From abd36cece9a104e9ea9f0422af31d49b31683436 Mon Sep 17 00:00:00 2001 From: Tomas Hruby Date: Tue, 8 Sep 2026 18:07:57 -0700 Subject: [PATCH] fix(installation): hold bpfKubeProxyHealthzPort=0 until every node accepts it Connecting an OSS v3.31.x BPF cluster to Calico Cloud left calico-node in CrashLoopBackOff and deadlocked the manifest->operator migration: Unable to set global default configuration error=error with field BPFKubeProxyHealthzPort = '0' (Reason: failed to validate Field: BPFKubeProxyHealthzPort because of Tag: gte) 0 meaning "disabled" is only accepted from Calico v3.32.0 and Enterprise v3.23.0-2.0. Validation is client-side in libcalico-go, so an older node fails it while writing back the global defaults at startup and crashloops, even though the field means nothing to it. The operator stamped the value into the shared FelixConfiguration while v3.31.7 pods were still serving nodes out of kube-system. Gate the default on allNodesRunTargetVersion: skip the write while the kube-system migration is pending and, once an operator-managed DaemonSet exists, until Status.Variant and Status.CalicoVersion report the version this operator rolls out. A fresh install still writes it immediately, so the AKS case CORE-12755 fixed is unchanged. CORE-13540 --- .../installation/core_controller.go | 42 +++++++-- .../installation/core_controller_test.go | 86 ++++++++++++++++++- 2 files changed, 118 insertions(+), 10 deletions(-) diff --git a/pkg/controller/installation/core_controller.go b/pkg/controller/installation/core_controller.go index cfbe251fae..2fda493458 100644 --- a/pkg/controller/installation/core_controller.go +++ b/pkg/controller/installation/core_controller.go @@ -1699,12 +1699,28 @@ func (r *ReconcileInstallation) setDefaultsOnFelixConfiguration(ctx context.Cont } } + // Whether a calico-node DaemonSet is already running decides both what version is out + // there and how bpfEnabled has to be derived, below. + ds := &appsv1.DaemonSet{} + nodeDSExists := true + if err := r.client.Get(ctx, types.NamespacedName{Namespace: common.CalicoNamespace, Name: common.NodeDaemonSetName}, ds); err != nil { + if !apierrors.IsNotFound(err) { + reqLogger.Error(err, "An error occurred when getting the Daemonset resource") + return false, err + } + nodeDSExists = false + } + // When BPF is enabled but the operator is not managing kube-proxy (e.g. on AKS, where // the platform owns the kube-proxy DaemonSet), the platform's kube-proxy keeps the // default healthz port (10256), and Felix's BPF kube-proxy healthz server would fail // to bind. Default the port to 0 (disabled) so calico-node starts cleanly. Users can // still override by setting BPFKubeProxyHealthzPort explicitly on FelixConfiguration. - if install.Spec.BPFEnabled() && !install.Spec.KubeProxyManagementEnabled() && fc.Spec.BPFKubeProxyHealthzPort == nil { + // + // 0 is only a valid port for Calico >= v3.32.0 and Enterprise >= v3.23.0-2.0, so hold + // the write until every node can accept it. + if allNodesRunTargetVersion(install, needNsMigration, nodeDSExists, r.ext.ProductVersion(&install.Spec)) && + install.Spec.BPFEnabled() && !install.Spec.KubeProxyManagementEnabled() && fc.Spec.BPFKubeProxyHealthzPort == nil { disableBPFKubeProxyHealthz(fc) updated = true } @@ -1725,13 +1741,7 @@ func (r *ReconcileInstallation) setDefaultsOnFelixConfiguration(ctx context.Cont // If calico-node daemonset exists, we need to check the ENV VAR and set FelixConfiguration accordingly. // Otherwise, this is a fresh install in eBPF mode, set the felix config. - ds := &appsv1.DaemonSet{} - err = r.client.Get(ctx, types.NamespacedName{Namespace: common.CalicoNamespace, Name: common.NodeDaemonSetName}, ds) - if err != nil { - if !apierrors.IsNotFound(err) { - reqLogger.Error(err, "An error occurred when getting the Daemonset resource") - return false, err - } + if !nodeDSExists { if !needNsMigration && install.Spec.BPFEnabled() { err = setBPFEnabledOnFelixConfiguration(fc, true) if err != nil { @@ -1759,6 +1769,22 @@ func (r *ReconcileInstallation) setDefaultsOnFelixConfiguration(ctx context.Cont return updated, nil } +// allNodesRunTargetVersion reports whether every calico-node runs targetVersion. Gate +// FelixConfiguration writes that older nodes' validators reject on it. +func allNodesRunTargetVersion(install *operatorv1.Installation, needNsMigration, nodeDSExists bool, targetVersion string) bool { + if needNsMigration { + // Manifest-install pods still serve nodes out of kube-system. + return false + } + if !nodeDSExists { + // Fresh install: nothing is running yet. + return true + } + // Written together, and only once the stack was seen available on that version. A status + // reporting neither fails this too, which is what we want. + return install.Status.Variant == install.Spec.Variant && install.Status.CalicoVersion == targetVersion +} + // setClusterRoutingOnFelixConfiguration sets programClusterRoutes in the FelixConfiguration resource // based on the value of clusterRoutingMode in the install config. func setClusterRoutingOnFelixConfiguration( diff --git a/pkg/controller/installation/core_controller_test.go b/pkg/controller/installation/core_controller_test.go index a28a0d3876..9c2b5cc1b8 100644 --- a/pkg/controller/installation/core_controller_test.go +++ b/pkg/controller/installation/core_controller_test.go @@ -78,10 +78,10 @@ var ( var errMismatchedError = fmt.Errorf("installation spec.kubernetesProvider 'DockerEnterprise' does not match auto-detected value 'OpenShift'") -type fakeNamespaceMigration struct{} +type fakeNamespaceMigration struct{ needsMigration bool } func (f *fakeNamespaceMigration) NeedsCoreNamespaceMigration(ctx context.Context) (bool, error) { - return false, nil + return f.needsMigration, nil } func (f *fakeNamespaceMigration) Run(ctx context.Context, log logr.Logger) error { @@ -1652,6 +1652,64 @@ var _ = Describe("Testing core-controller installation", func() { Expect(*fc.Spec.BPFKubeProxyHealthzPort).To(Equal(12345)) }) + It("should not set BPFKubeProxyHealthzPort while calico-node is still on an older version", func() { + // A calico-node DaemonSet is already running and the status reports no version + // yet, so nodes may predate the value. + createNodeDaemonSet() + + network := operator.LinuxDataplaneBPF + cr.Spec.CalicoNetwork = &operator.CalicoNetworkSpec{LinuxDataplane: &network} + Expect(c.Create(ctx, cr)).NotTo(HaveOccurred()) + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + fc := &v3.FelixConfiguration{} + err = c.Get(ctx, types.NamespacedName{Name: "default"}, fc) + Expect(err).ShouldNot(HaveOccurred()) + + Expect(fc.Spec.BPFKubeProxyHealthzPort).To(BeNil()) + }) + + It("should set BPFKubeProxyHealthzPort once calico-node reports the target version", func() { + createNodeDaemonSet() + + network := operator.LinuxDataplaneBPF + cr.Spec.CalicoNetwork = &operator.CalicoNetworkSpec{LinuxDataplane: &network} + Expect(c.Create(ctx, cr)).NotTo(HaveOccurred()) + + cr.Status.Variant = operator.CalicoEnterprise + cr.Status.CalicoVersion = components.EnterpriseRelease + Expect(c.Status().Update(ctx, cr)).NotTo(HaveOccurred()) + + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + fc := &v3.FelixConfiguration{} + err = c.Get(ctx, types.NamespacedName{Name: "default"}, fc) + Expect(err).ShouldNot(HaveOccurred()) + + Expect(fc.Spec.BPFKubeProxyHealthzPort).NotTo(BeNil()) + Expect(*fc.Spec.BPFKubeProxyHealthzPort).To(Equal(0)) + }) + + It("should not set BPFKubeProxyHealthzPort while the kube-system migration is pending", func() { + // calico-node pods from the manifest install are still serving nodes out of + // kube-system, so the operator's own DaemonSet says nothing about them. + r.namespaceMigration = &fakeNamespaceMigration{needsMigration: true} + + network := operator.LinuxDataplaneBPF + cr.Spec.CalicoNetwork = &operator.CalicoNetworkSpec{LinuxDataplane: &network} + Expect(c.Create(ctx, cr)).NotTo(HaveOccurred()) + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + fc := &v3.FelixConfiguration{} + err = c.Get(ctx, types.NamespacedName{Name: "default"}, fc) + Expect(err).ShouldNot(HaveOccurred()) + + Expect(fc.Spec.BPFKubeProxyHealthzPort).To(BeNil()) + }) + It("should set BPFEnabled to ture on FelixConfiguration if BPF is enabled on installation", func() { createNodeDaemonSet() @@ -2563,6 +2621,30 @@ func (f *fakeComponentHandler) CreateOrUpdateOrDelete(ctx context.Context, compo return nil } +var _ = Describe("allNodesRunTargetVersion", func() { + const targetVersion = "v3.24.0-1.0" + + install := func(statusVariant operator.ProductVariant, statusVersion string) *operator.Installation { + return &operator.Installation{ + Spec: operator.InstallationSpec{Variant: operator.CalicoEnterprise}, + Status: operator.InstallationStatus{Variant: statusVariant, CalicoVersion: statusVersion}, + } + } + + DescribeTable("deciding whether older calico-node pods may still be running", + func(install *operator.Installation, needNsMigration, nodeDSExists, expected bool) { + Expect(allNodesRunTargetVersion(install, needNsMigration, nodeDSExists, targetVersion)).To(Equal(expected)) + }, + Entry("fresh install, no DaemonSet yet", install("", ""), false, false, true), + Entry("migration pending, no DaemonSet yet", install("", ""), true, false, false), + Entry("migration pending alongside a DaemonSet", install(operator.CalicoEnterprise, targetVersion), true, true, false), + Entry("DaemonSet running, no version reported yet", install("", ""), false, true, false), + Entry("version upgrade in flight", install(operator.CalicoEnterprise, "v3.23.0-1.1"), false, true, false), + Entry("variant change in flight", install(operator.Calico, targetVersion), false, true, false), + Entry("target version rolled out", install(operator.CalicoEnterprise, targetVersion), false, true, true), + ) +}) + var _ = Describe("updateMutatingAdmissionPolicies", func() { var ( ctx context.Context