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