VictoriaMetrics / operator

Kubernetes operator for Victoria Metrics
Apache License 2.0
422 stars 140 forks source link

Inconstencies in container image registry parameters #871

Closed skolyszewski closed 4 months ago

skolyszewski commented 6 months ago

For pulling container images, we use container registry with a pull-through-cache. This requires us to override the registry of a container image address, e.g. for images from dockerhub, we would use our-internal-registry-dns-name/dockerhub/<repository>:<tag>, e.g. our-internal-registry-dns-name/dockerhub/busybox:latest. We're unable to consistently and reliably set the registry using the Operator, as: 1) image section of CRDs contains repository and tag, and no registry (as it's usually done in the other resources, e.g. helm-charts of other upstream tools), so setting the registry requires to know the repository as well. This is also only true for the main containers, as the parameters for sidecars can't be modified from the Custom Resource level. 2) VM_CONTAINERREGISTRY environment variable for the Operator instance is a decent alternative, however, it only works for main containers as well, as some of the sidecar containers use e.g. quay.io, and setting this variable results in ErrImagePull due to a wrong address of the image 3) Env variables for sidecar images, e.g. VM_CUSTOMCONFIGRELOADERIMAGE contain registry (unsure), repository and tag part in a single value, so it would require tracking all of these parameters when modifying the installation, which is inconvenient.

Given that modifying CRDs would be a breaking change, perhaps adding better support in Operator's env variables is a better option?

f41gh7 commented 6 months ago

Hello,

I think this is a bug:

VM_CONTAINERREGISTRY environment variable for the Operator instance is a decent alternative, however, it only works > for main containers as well, as some of the sidecar containers use e.g. quay.io, and setting this variable results in > ErrImagePull due to a wrong address of the image

Operator must correctly set container registry for any images.

But keep in mind, that only strips a quay.io/ part and replaces it with defined registry variable.

E.g. VM_CONTAINERREGISTRY="our-internal-registry-dns-name/dockerhub"

Default image: - quay.io/prometheus-operator/prometheus-config-reloader:v0.68.0

Transforms into our-internal-registry-dns-name/dockerhub/prometheus-operator/prometheus-config-reloader:v0.68.0.

f41gh7 commented 4 months ago

I'd recommend to perform migration to the custom config-reloader image. It consistently uses own config-reloader image for any components.

VM_USECUSTOMCONFIGRELOADER=true
VM_CUSTOMCONFIGRELOADERIMAGE=victoriametrics/operator:config-reloader-v0.43.0