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