Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow image tags/name/repository to be configured separately #644

Open
cucxabong opened this issue Jun 28, 2021 · 9 comments
Open

Allow image tags/name/repository to be configured separately #644

cucxabong opened this issue Jun 28, 2021 · 9 comments
Labels
area/chart-only Related to changes that simply require yaml Helm chart changes, e.g. exposing a new field type/enhancement New feature or request

Comments

@cucxabong
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

We have an in-house tool that will fetch latest tags for an image in a (local docker registry) & then fill values (using --set) file on deployment, so this tool can not work when current chart requires input image as a single string (with registry/name/tag in a single line)

Contributions

I'm happy to work on this

@thisisnotashwin
Copy link
Contributor

Hey @cucxabong !! Thanks for filing an issue. I do understand the appeal of having the tags, name and repository being separate fields that are individually configurable, it would make it a little more complicated to parse the helm config for most of our users.
Would it be possible for the in-house tool's output to be configured in the repository/name:tag format so that it can be provided to the --set flag? Because unfortunately, if we don't see more community support for this (with 👍 ), we might not want to make this update as I fear it will make the values file more complicated to understand as most users are familiar with images in the repo/name:tag format and might find them being in separate field non-intuitive.

@TJM
Copy link
Contributor

TJM commented Jul 1, 2021

I second this... We have "air-gapped" environments, where we cannot reach "docker" proper, if we could just set something like global.registry to internal-mirror.domain.com, and not mess with the image name or tag, that would make things easier. Now, whenever we upgrade the helm chart, we have to go back in and verify that we have the correct version of consul-k8s and consul ...

Additionally, I think it would be nice if there was a way of just setting global.enterprise: true instead of overriding the image name manually... or even just automatically select the enterprise image if they provide a license?

@thisisnotashwin
Copy link
Contributor

thisisnotashwin commented Jul 1, 2021

@TJM thanks for the feedback. This definitely makes sense in an airgapped env where all one wants to update is the repository but retain the name of the image and tag. Given docker works without a repo makes sense. A tentative solution would be to have a global flag for repo which defaults to index.docker.io which would work with docker but could be swapped for internal-mirror.domain.com or any other repo that the images have been migrated to.

re license: we have separate binaries for consul and consul-enterprise and hence separate images as well. hence the differing image name. i do agree that ideally, if the image was the same, the presence of a license should just lead to enterprise features being toggled on but that is not the case here unfortunately.

@t-eckert t-eckert changed the title Make image tags/name/repository configurable separately helm:Make image tags/name/repository configurable separately Aug 24, 2021
@t-eckert t-eckert transferred this issue from hashicorp/consul-helm Aug 24, 2021
lawliet89 pushed a commit to lawliet89/consul-k8s that referenced this issue Sep 13, 2021
@lkysow
Copy link
Member

lkysow commented Nov 4, 2021

I don't think we can support this without breaking backwards compatibility. Say we do as @thisisnotashwin suggests and have:

# values.yaml
global:
  imageRepo: docker.io
  imageK8S: hashicorp/consul-k8s:0.36.0

And then in our templates we use:

image: {{ .Values.global.imageRepo }}/{{ .Values.imageK8S}}

This would break anyone that was previously setting the full image, e.g.:

global:
  imageK8S: my.internal.repo/hashicorp/consul-k8s:0.36.0

Since it would now render as:

image: docker.io/my.internal.repo/hashicorp/consul-k8s:0.36.0

Given that this is a nice-to-have and that there should be a way to work around it in tooling and that I don't see a way to implement this without breaking backwards compatibility I'm not sure it makes sense to implement this.

@lkysow lkysow added area/chart-only Related to changes that simply require yaml Helm chart changes, e.g. exposing a new field type/enhancement New feature or request labels Nov 4, 2021
@TJM
Copy link
Contributor

TJM commented Nov 4, 2021

Without breaking backwards compatibility... If the parameter to set the "full image name" (global.imageK8S) is set, use it, otherwise construct an image name using the component parts. Just don't use the same parameter for the full image name as the component parts. The "image" that is passed to the kubernetes resource can all be magic'd out in a helpers template.

NOTE: you can even deprecate the old "full image name" parameter, such that some time in the future, you could remove it in a major release. It really isn't too difficult to use though. Take a look at the "fullNameOverride" in _helpers.tpl which defines the name of various parts of a typical helm template (helm create example)

@david-yu david-yu changed the title helm:Make image tags/name/repository configurable separately Allow image tags/name/repository to be configured separately Jul 5, 2023
@david-yu
Copy link
Contributor

david-yu commented Jul 5, 2023

This was discussed recently whether it would be viable to make this change. Currently would introduce a lot of changes for the integration testing side of Consul-k8s so we've delayed working on this as it is a breaking change.

@ebachle
Copy link

ebachle commented Oct 2, 2023

hey @david-yu I wanted to bump this one. Looking at the chart I can see how this would be a challenging refactor, but I do want to highlight how worthwhile my organization would find it.

In our case, we're angling to get full off of Dockerhub and onto the Public ECR Gallery for images so we don't leave the AWS network to pull images.

In the case of the vault chart, it was fairly simple to make this migration. We replaced the repository on each component and we were good to go.

https://github.com/hashicorp/vault-helm/blob/main/values.yaml#L68-L71

  image:
    repository: "hashicorp/vault-k8s"
    tag: "1.2.1"

With consul however, the part that somewhat holds me back from taking the leap is that I would then end up having to take up tracing both the versions of the chart and the images. While that is doable and if I want off of Dockerhub bad enough, I'll probably do it. But it puts me in a position of not just worrying about my chart version and what the associated default version of Consul is for that chart, but matching those up and keeping them up-to-date.

From my end, it would definitely be preferable if I was able to update the repository without having to take up the additional task of keeping an eye on the version of the image as a side-effect.

My thought on a possible contract that might work would be to do something like Vault does, but test for if image is set to a string or a map in a template helper and if it's a string, take that value, otherwise compose it from the map. That might make it less breaking, though not less work from an integration testing side.

Thanks for the consideration!

@dmpe
Copy link

dmpe commented May 21, 2024

related #3425

@ebachle
Copy link

ebachle commented May 21, 2024

I also found myself itching for this one again as the 1.16.7 image didn't make it to dockerhub, so I was considering changing the registry.

Though the core fix probably needs to be to ensure the images get published.

Ref: https://hub.docker.com/r/hashicorp/consul/tags?page=&page_size=&ordering=&name=1.16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/chart-only Related to changes that simply require yaml Helm chart changes, e.g. exposing a new field type/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants