search

nais / naisd

nais deployment daemon
https://nais.io
MIT License
27 stars 10 forks source link

Fix image version when default config is used #30

Closed davidsteinsland closed 7 years ago

davidsteinsland commented 7 years ago

Application version gets appended in createPodSpec, so if image is left out from nais.yaml, the resulting image will have the application version appended twice.

davidsteinsland commented 7 years ago

What should we do if no version is specified in nais.yaml, as the result would be docker.adeo.no:5000/appname?

jhrv commented 7 years ago

If no version is specified that is what is passed though all the way to the podspec, and it will use "latest" version from the repo (this is the default behaviour). I would argue this is slightly less magical. The drawback is that one has to manipulate nais.yaml for specifying version. Thoughts?

audunstrand commented 7 years ago

Version should come from deploymentrequest. We should fail if no version comes with deployment request.

davidsteinsland commented 7 years ago

I agree that the deployment request should be the place we get the version, as this will also allow us to deploy multiple versions without changing nais.yaml.

However, I think it would be nice to have the option to specify a full tag in nais.yaml (including version/or "latest") that would overwrite the deploymentrequest version. If not, we should produce an error saying that version is given in both deploymentrequest and nais.yaml

audunstrand commented 7 years ago

isn't that just making things more difficult. what's the problem you are trying to solve with this

davidsteinsland commented 7 years ago

good point. so image in nais.yaml is expected to be a name without a version. If no image is provided in nais.yaml, the GetDefaultAppConfig returns the default name (without version). The version given in the deployment request is always appended to the image name in createPodSpec.

audunstrand commented 7 years ago

its versjoned in git and nexus. we don't put versions in our code files in any other way.

version is a result of a build

davidsteinsland commented 7 years ago

yes, I've reverted the last commit so now the pull is as you propose.

jhrv commented 7 years ago

Sorry for the back and forth here. But I agree we've reached the best solution here by locking it down to a single version for the app-code/image and the configuration/nais.yaml. This simplifies a great deal, as well as it helps avoid file manipulation during build 👌

The drawback is that you have to push a new version of the image each time you change nais.yaml as you cannot override it. However this is usually handled automatically by the pipeline on push. Docker also handles this well, since if the artifact is unchanged, the layer is the same and a push is super fast as no new layers needs to be transferred.

Merging on monday