projectcalico / calico

Cloud native networking and network security
https://docs.tigera.io/calico/latest/about/
Apache License 2.0
6.02k stars 1.34k forks source link

Process for Upgrading CRDs when deploying Tigera operator through the Helm chart #6367

Open Chili-Man opened 2 years ago

Chili-Man commented 2 years ago

Currently I'm following the recommended directions for installing the Calico through the tigera-operator helm chart, https://docs.aws.amazon.com/eks/latest/userguide/calico.html. However, given that the the operator itself and Calico have CRDs (that are within the special crds helm directory) and that helm does not apply any updates to CRDs, what is the recommended way to have the CRDs get updated?

Expected Behavior

For Helm chart based deployments, I expect there to be documentation covering how to perform upgrades in particular around the CRDs (or at least what calico recommends).

Current Behavior

Since there is no recommendation from calico, simply doing the helm upgrade to a new chart won't upgrade the CRDs.

Possible Solution

Helm recommends that we have a separate helm chart for just the CRDs https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#method-2-separate-charts , that would get installed first before the tigera-operator helm chart. This approach works nicely as it works with helm based deployments and doesn't require additional tooling.

Another approach would be to use something like flux or argocd to deploy the helm chart with the --skip-crds option and alongside the helm chart release include the raw CRD manifests as part of that deployment.

However, to make the experience streamlined, it would ideally require some support from projectcalico to either create the separate helm chart with crds only / or have only the CRDs published somewhere by version tag.

lwr20 commented 2 years ago

Since there is no recommendation from calico

Have you seen this? https://projectcalico.docs.tigera.io/maintenance/kubernetes-upgrade#upgrading-an-installation-that-was-installed-using-helm

Or are you simply saying these upgrade instructions are wrong and will not work?

lwr20 commented 2 years ago

However, to make the experience streamlined, it would ideally require some support from projectcalico to either create the separate helm chart with crds only / or have only the CRDs published somewhere by version tag.

@caseydavenport ISTR you had some thoughts along those lines?

caseydavenport commented 2 years ago

Yeah, we need to do better here. The options laid out above are the only ways forward I think. We probably want to move the CRDs out of the chart IMO, and then provide both a manifest and a helm chart to install the CRDs separately.

Chili-Man commented 2 years ago

There might be a third option if there's appetite for it: we could have the tigera-operator fully manage the installation of the calico CRDs as well (the operator CRDs will still need to be installed by helm chart or through manifests). This would further serve the purpose of the operator and allow those CRDs in particular to be fully and automatically managed without concern from the end user, which would be a huge win for upgrades. In order to do this, the tigera-operator would need more RBAC permissions though to manage the CRDs (I don't see that as a big issue since it already has fairly broad cluster-role permissions to begin with)

caseydavenport commented 2 years ago

The operator does actually include that capability today, but it is turned off by default because of the RBAC concerns you mentioned. We could look at turning it on by default, though. Either way I think we will need options, as not everyone will agree on the desired approach.