diff --git a/hypershift-operator/controllers/hostedcluster/karpenter.go b/hypershift-operator/controllers/hostedcluster/karpenter.go index 7aa958deba54..339266948a57 100644 --- a/hypershift-operator/controllers/hostedcluster/karpenter.go +++ b/hypershift-operator/controllers/hostedcluster/karpenter.go @@ -25,6 +25,7 @@ import ( controlplanecomponent "github.com/openshift/hypershift/support/controlplane-component" "github.com/openshift/hypershift/support/k8sutil" karpenterutil "github.com/openshift/hypershift/support/karpenter" + "github.com/openshift/hypershift/support/statuspatching" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -48,16 +49,18 @@ func (r *HostedClusterReconciler) reconcileKarpenterOperator(cpContext controlpl return fmt.Errorf("failed to reconcile karpenter component: %w", err) } - // When Karpenter is disabled, clear stale node-counts from the HCP. - // HCP.Status.AutoNode holds NodeCount/NodeClaimCount written by the karpenter-operator - // while it was running. Since the karpenter-operator only runs when enabled, it cannot - // clear this itself. - if cpContext.HCP.Status.AutoNode != (hyperv1.AutoNodeStatus{}) { - patch := client.MergeFrom(cpContext.HCP.DeepCopy()) + // Clear stale node-counts from the HCP. HCP.Status.AutoNode holds NodeCount/NodeClaimCount + // written by the karpenter-operator while it was running. Since the karpenter-operator only + // runs when enabled, it cannot clear this itself. + // No pre-check on cpContext.HCP.Status.AutoNode here: it's a cache read from earlier in the + // reconcile loop and can be stale relative to the live object, silently skipping the patch. + // PatchStatus re-fetches and no-ops via DeepEqual when there's nothing to clear, so the guard + // only adds a staleness risk without saving real work. + if err := statuspatching.PatchStatus(cpContext, cpContext.Client, cpContext.HCP, func() error { cpContext.HCP.Status.AutoNode = hyperv1.AutoNodeStatus{} - if err := cpContext.Client.Status().Patch(cpContext, cpContext.HCP, patch); err != nil { - return fmt.Errorf("failed to clear AutoNode status: %w", err) - } + return nil + }); err != nil { + return fmt.Errorf("failed to clear AutoNode status: %w", err) } // Also delete the taint ConfigMap — it is only valid while Karpenter is enabled. if _, err := k8sutil.DeleteIfNeeded(cpContext, r.Client, &corev1.ConfigMap{ diff --git a/hypershift-operator/controllers/hostedcluster/karpenter_test.go b/hypershift-operator/controllers/hostedcluster/karpenter_test.go index e6e6ccb23922..5ba307e6d3cd 100644 --- a/hypershift-operator/controllers/hostedcluster/karpenter_test.go +++ b/hypershift-operator/controllers/hostedcluster/karpenter_test.go @@ -10,6 +10,7 @@ import ( karpenterv2 "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/karpenter" karpenteroperatorv2 "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/karpenteroperator" "github.com/openshift/hypershift/support/api" + controlplanecomponent "github.com/openshift/hypershift/support/controlplane-component" karpenterutil "github.com/openshift/hypershift/support/karpenter" appsv1 "k8s.io/api/apps/v1" @@ -459,3 +460,72 @@ func TestReconcileAutoNodeEnabledCondition(t *testing.T) { }) } } + +func TestReconcileKarpenterOperator(t *testing.T) { + tests := []struct { + name string + staleAutoNode hyperv1.AutoNodeStatus + wantAutoNode hyperv1.AutoNodeStatus + }{ + { + name: "When karpenter is disabled and AutoNode status is stale, it should clear the status", + staleAutoNode: hyperv1.AutoNodeStatus{NodeCount: ptr.To[int32](5)}, + wantAutoNode: hyperv1.AutoNodeStatus{}, + }, + { + name: "When karpenter is disabled and AutoNode status is already empty, it should no-op", + staleAutoNode: hyperv1.AutoNodeStatus{}, + wantAutoNode: hyperv1.AutoNodeStatus{}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-ns", + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(api.Scheme). + WithObjects(hcp). + WithStatusSubresource(hcp). + Build() + + // Set stale AutoNode status after creation (WithObjects doesn't persist status). + hcp.Status.AutoNode = tc.staleAutoNode + g.Expect(fakeClient.Status().Update(ctx, hcp)).To(Succeed()) + + r := &HostedClusterReconciler{ + Client: fakeClient, + } + + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "clusters", + }, + // Spec.AutoNode is zero value — karpenter disabled. + } + + cpContext := controlplanecomponent.ControlPlaneContext{ + Context: ctx, + Client: fakeClient, + HCP: hcp, + } + + err := r.reconcileKarpenterOperator(cpContext, hcluster, "image", "image") + g.Expect(err).ToNot(HaveOccurred()) + + updated := &hyperv1.HostedControlPlane{} + g.Expect(fakeClient.Get(ctx, crclient.ObjectKeyFromObject(hcp), updated)).To(Succeed()) + g.Expect(updated.Status.AutoNode).To(Equal(tc.wantAutoNode), + "stale AutoNode status should be cleared when karpenter is disabled") + }) + } +}