redpanda-data / helm-charts

Redpanda Helm Chart
http://redpanda.com
Apache License 2.0
77 stars 97 forks source link

Install Redpanda CRDs when installing the Operator #967

Open voutilad opened 10 months ago

voutilad commented 10 months ago

What would you like to be added?

We should install the appropriate version of the Redpanda CRDs when installing the Operator via Helm.

Why is this needed?

Users need to be very careful in installing the matching CRD in case the one in the main branch of https://github.com/redpanda/redpanda-operator gets out of sync with what the current Operator release expects. Since the Operator is installed or upgraded via Helm, the Helm install should install the correct CRD version to prevent this possibility.

JIRA Link: K8S-92

chrisseto commented 9 months ago

@voutilad, is the issue you're referring this block from the readme?

https://github.com/redpanda-data/helm-charts/blob/6cc4328bd674e307a85c4f46f38e2102a121ae92/charts/operator/README.md?plain=1#L23-L27

voutilad commented 9 months ago

Maybe I should clarify my request that it's to install the CRD for the user and not require them to install the CRD manually.

chrisseto commented 9 months ago

Ah, I see!

It sounds like we have two issues in that case.

  1. Install the (correct) CRDs for users rather than having them run kubectl kustomize
  2. Update the README.md to specify the correct version of the CRDs to install

1 would solve 2 but it's going to take some time to get the helm chart to install CRDs automatically.

I'll leave this issue to track 1 and I've made https://github.com/redpanda-data/helm-charts/issues/970 to track 2.

RafalKorepta commented 9 months ago

The problem described in @chrisseto in number 1 is not a good option as helm doesn't support lifecycle for CRDs. We would have problems when user install CRDs using helm chart and then we would like to update them. It would end up being the same kustomzie ... | kubectl apply -f

Refs

https://helm.sh/docs/topics/charts/#limitations-on-crds

https://github.com/helm/helm-www/blob/d4d6b7448f1327e05aec227cdfbe44b28692a714/content/en/docs/topics/charts.md?plain=1#L1041-L1057

sebastiangaiser commented 9 months ago

@RafalKorepta the problems are the same for every operator. (Good) examples at least for me are kube-prometheus-stack and metallb. They ship the crds within the chart as "subchart". I think it should be up to the administrator to decide how to manage CRDs and the software (redpanda) should provide ways to do that. Speaking for me, using Flux, the CRD lifecycle is no big deal: https://fluxcd.io/flux/components/helm/helmreleases/#controlling-the-lifecycle-of-custom-resource-definitions

jonasbadstuebner commented 9 months ago

Effectively, if you would ship the CRDs as "templates/", they would be handled as normal resources and would be up- and downgraded as intended. The problem with that is, that CRDs have to exist in the cluster before the Helm dry-run would succeed. If you put the CRDs into a charts crds/ folder, they are respected in the dry-run and applied before everything else, so it works.

But if you want to update them, a sub chart in the way MetalLB does it, would work. (tested just now) Create a subchart, where the CRDs are inside the templates folder. If you make it one file that is the result of kubectl kustomize https://github.com/redpanda-data/redpanda-operator//src/go/k8s/config/crd, CRDs can be handled without manual steps. :rocket:

chrisseto commented 9 months ago

@sebastiangaiser and @DrBu7cher, thank you for providing those examples! I've been leaning towards using a sub chart though the easy of accidentally deleting one's cluster (The primary downside of using helm to manage CRDs) still makes me a bit uneasy.

As you mention @sebastiangaiser, the installation should be left up to the administrator. So we'll want to support multiple methods of installation similar to cert-manager.