grafana / mimir

Grafana Mimir provides horizontally scalable, highly available, multi-tenant, long-term storage for Prometheus.
https://grafana.com/oss/mimir/
GNU Affero General Public License v3.0
4.18k stars 537 forks source link

Add a global setting for image registry in our Mimir Helm chart #5449

Open camsellem opened 1 year ago

camsellem commented 1 year ago

Is your feature request related to a problem? Please describe.

It would be useful to have a setting in the global section of our Helm chart in order to point to a different image registry instead of having to modify all the charts manually.

Describe the solution you'd like

Just add a variable "image.registry" in the global section of "values.yaml" that will override existing values that are pointing to different external registry.

Describe alternatives you've considered

Modify the image repository for all images in the values.yaml.

Additional context

Requested by a customer who is using an internal artifact repository and had to update the Helm chart manually for all images.

dimitarvdimitrov commented 1 year ago

the mimir-distributed helm chart uses several images (grafana/mimir, grafana/enterprise-metrics, grafana/rollout-operator, grafana/mimir-continuous-test, nginx, memcached...). Would the global setting apply to only the GEM and Mimir images?

Are you talking about the helm chart in the context of being used as a subchart?

camsellem commented 1 year ago

Hi - don't know what the best design would be (not very familiar with Helm best practices) but I think that having a setting that would overwrite the repository for all images (not only Mimir/GEM) - in the case of my prospect, they pull all images in their internal artifact repository.

dimitarvdimitrov commented 1 year ago

Do you mean registry or repository?

Dalktor commented 1 year ago

+1 if it's a registry, this would make life easier deploying from a private container registry and not having to jump through hoops for each different image. Tempo Distributed and Loki has this already supported and Global Chart Values are "special" in helm and automatically will apply to sub-charts

camsellem commented 1 year ago

Sorry missed that one. But yes I was talking about the registry.

On Tue, 25 July 2023, 09:29 Dal Kang, @.***> wrote:

+1 if it's a registry. Tempo Distributed https://github.com/grafana/helm-charts/blob/main/charts/tempo-distributed/values.yaml#L4 and Loki https://github.com/grafana/loki/blob/main/production/helm/loki/values.yaml#L4 has this already supported And Global Chart Values https://helm.sh/docs/chart_template_guide/subcharts_and_globals/#global-chart-values are "special" in helm and automatically will apply to sub-charts

— Reply to this email directly, view it on GitHub https://github.com/grafana/mimir/issues/5449#issuecomment-1648756867, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5TBBAJC5NSQVRMDDBDQ3DXR4AOVANCNFSM6AAAAAA2EAUPYI . You are receiving this because you authored the thread.Message ID: @.***>

dimitarvdimitrov commented 1 year ago

I understand now. I think we can follow the loki and tempo formats of having global.image.registry. When a per-image registry is provided (such as gateway.nginx.image.registry), then then registry should take precedence, so that not all images have to be from the same registry. The new field will be empty by default to preserve the current behaviour of falling back to the implicit docker.io registry.

One slight complication is that not all images in the Mimir helm chart currently support the registry format. I think we can introduce a registry field for those without introducing breaking changes to the chart

AnthonyKeydel commented 7 months ago

I'm in direct need of this functionality, so I'll try to add it 👍

Ivalberto commented 5 months ago

@AnthonyKeydel Any update on this ?

Rohlik commented 1 month ago

We were able to partly solve this in a hackish way by utilizing existed repository property:

image:
  repository: ecr.example.corp/grafana/mimir

So all images, except a few of them, will be pulled from ecr.example.corp instead of the default one.