projectcalico / calico

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

Error with defaultFelixConfiguration.enabled: true with Helm #9691

Open mnival opened 4 months ago

mnival commented 4 months ago

Expected Behavior

It should be possible to enable:

defaultFelixConfiguration:
  enabled: true

after a first installation done with the default values

Current Behavior

Currently, I receive this error:

$ helm upgrade --reset-values -n tigera-operator platform-nonprod-calico projectcalico/tigera-operator -f configuration/calico-values.yaml --version 3.28.1
Error: UPGRADE FAILED: Unable to continue with update: FelixConfiguration "default" in namespace "" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "platform-nonprod-calico"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "tigera-operator"

configuration/calico-values.yaml:

installation:
  cni:
    type: Calico
defaultFelixConfiguration:
  enabled: true
  wireguardEnabled: true

I executed these command before helm command:

kubectl label FelixConfiguration default app.kubernetes.io/managed-by=Helm
kubectl annotate FelixConfiguration default meta.helm.sh/release-namespace=tigera-operator
kubectl annotate FelixConfiguration default meta.helm.sh/release-name=platform-nonprod-calico

Possible Solution

Steps to Reproduce (for bugs)

1. 2. 3. 4.

Context

Your Environment

coutinhop commented 3 months ago

@mnival it doesn't look like you followed these steps when upgrading: https://docs.tigera.io/calico/3.28/operations/upgrading/kubernetes-upgrade#upgrading-an-installation-that-was-installed-using-helm

Could you try them and let us know if it works that way?

mnival commented 3 months ago

Hello @coutinhop, I haven't updated calico and all resources are already managed by Helm except FelixConfiguration. The page mentionned don't have any information about FelixConfiguration.

I'm on version 3.28.1, I just added the lines below in my values ​​file:

defaultFelixConfiguration:
  enabled: true

And I got the error mentioned in my first message Thank you

MichalFupso commented 3 months ago

@mnival I think that after installation when the Calico API server is running, use the v3 apiVersion to update felix configuration https://docs.tigera.io/calico/latest/reference/installation/helm_customization

caseydavenport commented 3 months ago

Error: UPGRADE FAILED: Unable to continue with update: FelixConfiguration "default" in namespace "" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "platform-nonprod-calico"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "tigera-operator"

@mnival was your FelixConfiguration originally created by Helm? Or might it have been created out of band?

@MichalFupso I wonder if this might be due to Calico's handling of labels on our underlying CRDs - we added this code recently to fix another issue: https://github.com/projectcalico/calico/blob/6dbc7d35243259474ff38029da08f7a726150d02/libcalico-go/lib/backend/k8s/resources/resources.go#L182-L193

We may need to make an exception there for helm labels

mnival commented 3 months ago

Hello @caseydavenport, It was created automatically by Calico but now I would like to be able to manage it by Helm to add configurations but this is not currently possible with the error that I indicated in my first message

caseydavenport commented 3 months ago

@mnival got it, ok.

This is a limitation in Helm - it refuses to update objects that it doesn't believe it owns. In this case, because Helm didn't originally create the FelixConfiguration, it's refusing to modify it.

There are some hacky ways to trick helm into assuming ownership of a resource by modifying the helm release ConfigMap, but it's a bit of a backdoor.

The other option would be to delete the FelixConfiguration and let Helm recreate it.

mnival commented 3 months ago

Hello @caseydavenport,

I added the annotations and labels but despite that helm doesn't want to manage it.

I tried to delete FelixConfiguration but it was recreated immediately. Do I need to stop something first?

I checked the commit here https://github.com/projectcalico/calico/commit/bb0ac52f607ee6fd6eee810c0a954c6b067b9ddb and it says:

if user wants to set some values, they should set enabled: true first and then add the properties and values.

But it doesn't work

caseydavenport commented 3 months ago

I tried to delete FelixConfiguration but it was recreated immediately. Do I need to stop something first?

Oh, yeah possible the tigera operator is recreating that.

mnival commented 2 months ago

Hello @caseydavenport ,

I understood my problem. I changed the label and annotations on FelixConfiguration.projectcalico.org but helm manages the resource FelixConfiguration.crd.projectcalico.org: https://github.com/projectcalico/calico/blob/master/charts/tigera-operator/templates/crs/configmap-felixconfiguration-templates.yaml#L4

I added the label and annotations and the resource is now manageable by Helm but reading https://github.com/projectcalico/calico/issues/6412 it seems that we should act on FelixConfiguration.projectcalico.org and not FelixConfiguration.crd.projectcalico.org, is there anything to change or with Helm we can only manage FelixConfiguration.crd.projectcalico.org.

Thank you

caseydavenport commented 2 months ago

@mnival yeah, you should use felixconfiguration.projectcalico.org - helm uses a backdoor due to a limitation.

The correct fix is in the Calico API server code - it needs to respect the helm labels and annotations. I will work on a fix for this.

caseydavenport commented 2 months ago

If you manage FelixConfiguration with helm, it's probably best to make all of your updates via the helm chart - i.e., modify values.yaml and upgrade your helm app.