helm charts: unexpected image name splicing

[STRRL]

Feature Request

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

Yes.

make install could not works well on the current master (bfee629), after #2462 merged.

After we migrated the docker images from dockerhub to ghcr.io, the way to render the image name is not suitable anymore, because the ghcr.io repository domain has been brought by default:

Ref:

chaos-mesh/helm/chaos-mesh/templates/chaos-dashboard-deployment.yaml

Line 56 in 0d683c6

image: {{template "registry-prefix" .}}{{ .Values.dashboard.image }}

We are now using {{template "registry-prefix" .}}{{ .Values.dashboard.image }}, but the registry-prefix should already be a part of image name.

Our official image is ghcr.io/chaos-mesh/chaos-mesh/chao-mesh/:latest, if the user has a self-hosted mirror with domain mirror.self.hosted, and the user setup registry in the helm values, the final result is mirror.self.hosted/ghcr.io/chaos-mesh/chaos-mesh/chao-mesh/:latest, which is incorrect.

Describe the feature you'd like:

Updated:

Pattern B with registry, repository, tag is chosen to use.

I'm not sure about which is the most fluency way to configure the image. I would give some available solutions in the comments. :)

Feel free to leave any comments!

Describe alternatives you've considered:

No other alternatives.

Teachability, Documentation, Adoption, Migration Strategy:

[STRRL]

Pattern A:

Remove registry directly, only use image.

If user use

helm upgrade --set registry: mirrors.self.hosted

before, he should use

helm upgrade --set controllerManager.image ghcr.io/chaos-mesh/chaos-mesh:latest \
  --set controllerManager.image ghcr.io/chaos-mesh/chaos-mesh:latest \
  --set chaosDaemon.image ghcr.io/chaos-mesh/chaos-daemon:latest \
  --set dashboard.image ghcr.io/chaos-mesh/chaos-dashboard:latest \
  --set bpfki.image ghcr.io/chaos-mesh/chaos-kernel:latest

after the updates.

Many flags need to be set, but if the user using a values.yaml to setup the helm installation, the operation is not so different.

Well-known projects use this pattern:

• traefik https://artifacthub.io/packages/helm/bitnami/nginx-ingress-controller?modal=template&template=controller-deployment.yaml
• cilium https://artifacthub.io/packages/helm/cilium/cilium?modal=template&template=cilium-nodeinit/daemonset.yaml
• tidb-operator https://github.com/pingcap/tidb-operator/blob/master/charts/tidb-operator/templates/advanced-statefulset-deployment.yaml

Pattern B:

Use three parts (registry, repository, tag) to define the image name, and

• setup the default registry to ghcr.io
• setup each repository for each component, like controllerManager.repository, dashboard.repository
• setup the default tag to the release version

The disadvantage is there is no completed docker image name in the configuration file.

Well-known projects use this pattern:

• cert-manager https://artifacthub.io/packages/helm/cert-manager/cert-manager?modal=template&template=cainjector-deployment.yaml
• most of bitnami images: https://github.com/bitnami/charts (defined in https://github.com/bitnami/charts/blob/master/bitnami/common/templates/_images.tpl) thanks @iguoyr
  • bitnami/nginx https://artifacthub.io/packages/helm/bitnami/nginx?modal=template&template=deployment.yaml
  • bitnami/etcd https://artifacthub.io/packages/helm/bitnami/etcd?modal=template&template=statefulset.yaml
  • etc.

Pattern C:

Use two parts (repository, tag) to define the image name. It's both similar to pattern A and pattern B. But I think this pattern is not suitable for Chaos Mesh, because:

• Chaos Mesh Helm Charts's version upgrades with appVersion, so you should switch to another version of charts if you want to upgrade/downgrade, not only replace the image tag
• As the former says, if tag is consistent, pattern C is extremely the same as pattern A.

Well-known projects use this pattern:

• harbor https://artifacthub.io/packages/helm/harbor/harbor?modal=template&template=chartmuseum/chartmuseum-dpl.yaml
• kube-prometheus-stack https://artifacthub.io/packages/helm/prometheus-community/kube-prometheus-stack?modal=template&template=prometheus-operator/deployment.yaml
• calico https://artifacthub.io/packages/helm/arhatdev/calico?modal=template&template=kube-controllers/deployment.yaml
• rook-ceph https://artifacthub.io/packages/helm/rook/rook-ceph?modal=template&template=deployment.yaml

Pattern D(which is not recommended personally):

Replace the repository domain in <component>.image with the given registry.

Input:

Input:
	registry: mirror.self.hosted
	image: ghcr.io/chaos-mesh/chaos-mesh:latest
Output:
	mirror.self.hosted/chaos-mesh/chaos-mesh:latest

Input:
	registry: 
	image: ghcr.io/chaos-mesh/chaos-mesh:latest
Output:
	ghcr.io/chaos-mesh/chaos-mesh:latest

I do not recommend it because it's not simple enough, including replacing and string cutting.

[cwen0]

Pattern A:

Remove registry directly, only use image.

If user use

helm upgrade --set registry: mirrors.self.hosted

before, he should use

helm upgrade --set controllerManager.image ghcr.io/chaos-mesh/chaos-mesh:latest \
  --set controllerManager.image ghcr.io/chaos-mesh/chaos-mesh:latest \
  --set chaosDaemon.image ghcr.io/chaos-mesh/chaos-daemon:latest \
  --set dashboard.image ghcr.io/chaos-mesh/chaos-dashboard:latest \
  --set bpfki.image ghcr.io/chaos-mesh/chaos-kernel:latest

after the updates.

Many flags need to be set, but if the user using a values.yaml to setup the helm installation, the operation is not so different.

Well-known projects use this pattern:

• most of bitnami images: https://github.com/bitnami/charts

  • bitnami/nginx https://artifacthub.io/packages/helm/bitnami/nginx?modal=template&template=deployment.yaml
  • bitnami/etcd https://artifacthub.io/packages/helm/bitnami/etcd?modal=template&template=statefulset.yaml
  • etc.

• traefik https://artifacthub.io/packages/helm/bitnami/nginx-ingress-controller?modal=template&template=controller-deployment.yaml

• cilium https://artifacthub.io/packages/helm/cilium/cilium?modal=template&template=cilium-nodeinit/daemonset.yaml

• tidb-operator https://github.com/pingcap/tidb-operator/blob/master/charts/tidb-operator/templates/advanced-statefulset-deployment.yaml

Pattern B:

Use three parts (registry, repository, tag) to define the image name, and

• setup the default registry to ghcr.io
• setup each repository for each component, like controllerManager.repository, dashboard.repository
• setup the default tag to the release version

The disadvantage is there is no completed docker image name in the configuration file.

Well-known projects use this pattern:

• cert-manager https://artifacthub.io/packages/helm/cert-manager/cert-manager?modal=template&template=cainjector-deployment.yaml

Pattern C:

Use two parts (repository, tag) to define the image name. It's both similar to pattern A and pattern B. But I think this pattern is not suitable for Chaos Mesh, because:

• Chaos Mesh Helm Charts's version upgrades with appVersion, so you should switch to another version of charts if you want to upgrade/downgrade, not only replace the image tag
• As the former says, if tag is consistent, pattern C is extremely the same as pattern A.

Well-known projects use this pattern:

• harbor https://artifacthub.io/packages/helm/harbor/harbor?modal=template&template=chartmuseum/chartmuseum-dpl.yaml
• kube-prometheus-stack https://artifacthub.io/packages/helm/prometheus-community/kube-prometheus-stack?modal=template&template=prometheus-operator/deployment.yaml
• calico https://artifacthub.io/packages/helm/arhatdev/calico?modal=template&template=kube-controllers/deployment.yaml
• rook-ceph https://artifacthub.io/packages/helm/rook/rook-ceph?modal=template&template=deployment.yaml

Pattern D(which is not recommended personally):

Replace the repository domain in <component>.image with the given registry.

Input:

Input:
	registry: mirror.self.hosted
	image: ghcr.io/chaos-mesh/chaos-mesh:latest
Output:
	mirror.self.hosted/chaos-mesh/chaos-mesh:latest

Input:
	registry: 
	image: ghcr.io/chaos-mesh/chaos-mesh:latest
Output:
	ghcr.io/chaos-mesh/chaos-mesh:latest

I do not recommend it because it's not simple enough, including replacing and string cutting.

I prefer pattern A. This pattern is more simple.

[iguoyr]

traefik https://artifacthub.io/packages/helm/bitnami/nginx-ingress-controller?modal=template&template=controller-deployment.yaml
cilium https://artifacthub.io/packages/helm/cilium/cilium?modal=template&template=cilium-nodeinit/daemonset.yaml

@STRRL Hi, it has some errors here, traefik is belong to patternB, and cilium is belong to patternC. BTW, reading the value.yaml and xxx.tpl is the best way to figure it out :)

And about patternA, it's applicable to charts with single service component, while patternB is more applicable to charts with multi-service components. More specifically, when using patternB, users basically only need to --set the registry and tag manually, while patternA needs to specify more parameters which will increase with the increase of service components.

[Hexilee]

I prefer pattern B as registry is an appropriate abstraction. Pattern A or pattern C is more appropriate only when we plan to publish images to different registries.

[STRRL]

I would work on pattern B from the above comments. :D