Skip to content

Hold bpfKubeProxyHealthzPort=0 until every calico-node accepts it #5303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tomastigera wants to merge 1 commit into from
Closed

Hold bpfKubeProxyHealthzPort=0 until every calico-node accepts it #5303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 34 additions & 8 deletions pkg/controller/installation/core_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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(
Expand Down
86 changes: 84 additions & 2 deletions pkg/controller/installation/core_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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
Expand Down