From 329487843576f1685d82aabc04a86faf55f584d6 Mon Sep 17 00:00:00 2001 From: Tomas Hruby Date: Mon, 14 Sep 2026 12:55:43 -0700 Subject: [PATCH] Hold bpfKubeProxyHealthzPort=0 until every calico-node accepts it The operator defaults FelixConfiguration's bpfKubeProxyHealthzPort to 0 when BPF is on and it does not manage kube-proxy. 0 is only a valid value for Calico >= v3.32.0 and Enterprise >= v3.23.0-2.0; older calico-node rejects it at startup, so upgrading nodes CrashLoopBackOff, and a manifest-to-operator migration deadlocks because the nodes it has to replace cannot start. Gate the write on every calico-node already running the version the operator targets. A fresh install has nothing running and writes immediately; a pending kube-system migration means manifest-install pods are still serving nodes, so it waits; otherwise the status has to report both the target variant and version, which the operator writes together once the stack is available. The DaemonSet lookup that the bpfEnabled defaulting already did moves up so both checks share it. Cherry-pick of projectcalico/calico#13843 (0d65aae5de), which is the same change in the in-tree copy of this controller. CORE-13540 --- .../installation/core_controller.go | 52 +++++++++-- .../installation/core_controller_test.go | 86 ++++++++++++++++++- 2 files changed, 127 insertions(+), 11 deletions(-) diff --git a/pkg/controller/installation/core_controller.go b/pkg/controller/installation/core_controller.go index 3883eb9a20..03e28808e2 100644 --- a/pkg/controller/installation/core_controller.go +++ b/pkg/controller/installation/core_controller.go @@ -2094,12 +2094,28 @@ func (r *ReconcileInstallation) setDefaultsOnFelixConfiguration(ctx context.Cont } } + // A running calico-node DaemonSet decides both what version is out there and how + // bpfEnabled is 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, productVersion(&install.Spec)) && + install.Spec.BPFEnabled() && !install.Spec.KubeProxyManagementEnabled() && fc.Spec.BPFKubeProxyHealthzPort == nil { disableBPFKubeProxyHealthz(fc) updated = true } @@ -2143,15 +2159,9 @@ 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) + err := setBPFEnabledOnFelixConfiguration(fc, true) if err != nil { reqLogger.Error(err, "Unable to enable eBPF data plane with a fresh install") return false, err @@ -2177,6 +2187,30 @@ func (r *ReconcileInstallation) setDefaultsOnFelixConfiguration(ctx context.Cont return updated, nil } +// productVersion is the release the operator ships for this variant. +func productVersion(install *operatorv1.InstallationSpec) string { + if !install.Variant.IsEnterprise() { + return components.CalicoRelease + } + return components.EnterpriseRelease +} + +// 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, only once the stack was seen available on that version. A status + // reporting neither also fails. + 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 d652869e16..47d3b05987 100644 --- a/pkg/controller/installation/core_controller_test.go +++ b/pkg/controller/installation/core_controller_test.go @@ -77,10 +77,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 { @@ -1639,6 +1639,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 = cr.Spec.Variant + cr.Status.CalicoVersion = productVersion(&cr.Spec) + 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() { + // Manifest-install calico-node pods still serve nodes from kube-system, so the + // operator's 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() @@ -2576,6 +2634,30 @@ func (f *fakeComponentHandler) CreateOrUpdateOrDelete(ctx context.Context, compo return nil } +var _ = Describe("allNodesRunTargetVersion", func() { + const targetVersion = "v3.32.0" + + install := func(statusVariant operator.ProductVariant, statusVersion string) *operator.Installation { + return &operator.Installation{ + Spec: operator.InstallationSpec{Variant: operator.Calico}, + 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.Calico, targetVersion), true, true, false), + Entry("DaemonSet running, no version reported yet", install("", ""), false, true, false), + Entry("version upgrade in flight", install(operator.Calico, "v3.31.7"), false, true, false), + Entry("variant change in flight", install(operator.CalicoEnterprise, targetVersion), false, true, false), + Entry("target version rolled out", install(operator.Calico, targetVersion), false, true, true), + ) +}) + var _ = Describe("updateMutatingAdmissionPolicies", func() { var ( ctx context.Context