search

karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.48k stars 887 forks source link

Helm chart testing enhancement #5436

Open RainbowMango opened 2 months ago

RainbowMango commented 2 months ago

What would you like to be added:

  1. Make images based on PR or the latest code and then test with them
  2. Make sure to cover all components

Why is this needed:

Currently, helm testing does not test against the latest code but the last images on image registry. When we introduce new parameters to a component the chart test will fail due to unrecognized parameters, see the example here: https://github.com/karmada-io/karmada/pull/5290#issuecomment-2311992249.

In addition, the helm chart test coverage is not enough, it just tests part of the components, for example, the karmada-scheduler-estimator is not included in the test, that's why it passes #5273.

RainbowMango commented 2 months ago

May be #4045 addresses part of this.

chaosi-zju commented 2 months ago

here is another shortage I want to raise:

karmada-scheduler-estimator has installation problem when installMode=component.

just like:

I firstly execute helm install karmada -n karmada-system to install karmada main components.

then all karmada certs is stored in secret/{{.Release.Name}}-cert, since {{.Release.Name}} is karmada, so secret name is karmada-cert.

https://github.com/karmada-io/karmada/blob/295dee80dc8f9cf21b152048dfc7c4660cd4f6a3/charts/karmada/templates/pre-install-job.yaml#L249-L253

Then I execute helm install karmada-scheduler-estimator -n karmada-system --set installMode=component,components={"schedulerEstimator"} to install scheduler estimator independently.

the estimator needs to mount that secret/{{.Release.Name}}-cert to obtain karmada certs, however, now {{.Release.Name}} is karmada-scheduler-estimator (host cluster does not allow duplicate release names), so it will try to mount secret/karmada-scheduler-estimator-cert, which doesn't exist.

https://github.com/karmada-io/karmada/blob/295dee80dc8f9cf21b152048dfc7c4660cd4f6a3/charts/karmada/templates/_helpers.tpl#L352-L356

Actually, metricsAdapter,search,descheduler when installMode=component has same problem, the original author gives a rough circumvention:

https://github.com/karmada-io/karmada/blob/295dee80dc8f9cf21b152048dfc7c4660cd4f6a3/charts/karmada/values.yaml#L969-L972

that is, redeclare the secret name in value.yaml to overwrite the original {{.Release.Name}}-cert.

but I think the best way is fix the secret name, don't prefix it with {{.Release.Name}}, which is mentioned in #5363.