diff --git a/kwok/charts/crds/karpenter.sh_nodepools.yaml b/kwok/charts/crds/karpenter.sh_nodepools.yaml index 5ef3f3495a..521fbc5af9 100644 --- a/kwok/charts/crds/karpenter.sh_nodepools.yaml +++ b/kwok/charts/crds/karpenter.sh_nodepools.yaml @@ -41,6 +41,10 @@ spec: name: Memory priority: 1 type: string + - jsonPath: .status.resources.driftedNodeClaims + name: DriftedNodeClaims + priority: 1 + type: integer name: v1 schema: openAPIV3Schema: diff --git a/pkg/apis/crds/karpenter.sh_nodepools.yaml b/pkg/apis/crds/karpenter.sh_nodepools.yaml index 5a29c722f9..a81bb7b23e 100644 --- a/pkg/apis/crds/karpenter.sh_nodepools.yaml +++ b/pkg/apis/crds/karpenter.sh_nodepools.yaml @@ -41,6 +41,10 @@ spec: name: Memory priority: 1 type: string + - jsonPath: .status.resources.driftedNodeClaims + name: DriftedNodeClaims + priority: 1 + type: integer name: v1 schema: openAPIV3Schema: diff --git a/pkg/apis/v1/nodepool.go b/pkg/apis/v1/nodepool.go index d47a918268..e30b6078d4 100644 --- a/pkg/apis/v1/nodepool.go +++ b/pkg/apis/v1/nodepool.go @@ -301,6 +301,7 @@ type ObjectMeta struct { // +kubebuilder:printcolumn:name="Weight",type="integer",JSONPath=".spec.weight",priority=1,description="" // +kubebuilder:printcolumn:name="CPU",type="string",JSONPath=".status.resources.cpu",priority=1,description="" // +kubebuilder:printcolumn:name="Memory",type="string",JSONPath=".status.resources.memory",priority=1,description="" +// +kubebuilder:printcolumn:name="DriftedNodeClaims",type="integer",JSONPath=".status.resources.driftedNodeClaims",priority=1,description="" // +kubebuilder:subresource:status // +kubebuilder:subresource:scale:specpath=.spec.replicas,statuspath=.status.nodes type NodePool struct { diff --git a/pkg/apis/v1/nodepool_status.go b/pkg/apis/v1/nodepool_status.go index 7236418daf..f0ade91a57 100644 --- a/pkg/apis/v1/nodepool_status.go +++ b/pkg/apis/v1/nodepool_status.go @@ -29,6 +29,9 @@ const ( ConditionTypeNodeClassReady = "NodeClassReady" // ConditionTypeNodeRegistrationHealthy = "NodeRegistrationHealthy" condition indicates if a misconfiguration exists that is preventing successful node launch/registrations that requires manual investigation ConditionTypeNodeRegistrationHealthy = "NodeRegistrationHealthy" + // ConditionTypeNodeClaimsDrifted = "Drifted" condition indicates if any NodeClaims belonging to the NodePool have drifted + // from their expected configuration based on the NodePool and/or NodeClass hash + ConditionTypeNodeClaimsDrifted = "Drifted" ) // NodePoolStatus defines the observed state of NodePool diff --git a/pkg/controllers/nodepool/counter/controller.go b/pkg/controllers/nodepool/counter/controller.go index 35c7cc1a69..d2ab70bd44 100644 --- a/pkg/controllers/nodepool/counter/controller.go +++ b/pkg/controllers/nodepool/counter/controller.go @@ -18,6 +18,7 @@ package counter import ( "context" + "fmt" "time" "github.com/samber/lo" @@ -55,6 +56,7 @@ var BaseResources = corev1.ResourceList{ corev1.ResourcePods: resource.MustParse("0"), corev1.ResourceEphemeralStorage: resource.MustParse("0"), resources.Node: resource.MustParse("0"), + resources.DriftedNodeClaim: resource.MustParse("0"), } // NewController is a constructor @@ -88,6 +90,23 @@ func (c *Controller) Reconcile(ctx context.Context, nodePool *v1.NodePool) (reco nodePool.Status.Resources = lo.Assign(BaseResources, c.cluster.NodePoolResourcesFor(nodePool.Name)) nodeQuantity := nodePool.Status.Resources[resources.Node] nodePool.Status.Nodes = new(nodeQuantity.Value()) + + // Set condition Drifted + driftedNodeClaimCount := nodePool.Status.Resources[resources.DriftedNodeClaim] + if !driftedNodeClaimCount.IsZero() { + nodePool.StatusConditions().SetTrueWithReason( + v1.ConditionTypeNodeClaimsDrifted, + "NodeClaimsDriftedExist", + fmt.Sprintf("%d NodeClaim(s) managed by this NodePool have drifted", driftedNodeClaimCount.Value()), + ) + } else { + nodePool.StatusConditions().SetFalse( + v1.ConditionTypeNodeClaimsDrifted, + "ZeroNodeClaimsDrifted", + "All NodeClaims are in sync with the NodePool configuration", + ) + } + if !equality.Semantic.DeepEqual(stored, nodePool) { if err := c.kubeClient.Status().Patch(ctx, nodePool, client.MergeFrom(stored)); err != nil { return reconcile.Result{}, client.IgnoreNotFound(err) diff --git a/pkg/controllers/nodepool/counter/suite_test.go b/pkg/controllers/nodepool/counter/suite_test.go index 4eeb69240e..315978a909 100644 --- a/pkg/controllers/nodepool/counter/suite_test.go +++ b/pkg/controllers/nodepool/counter/suite_test.go @@ -428,4 +428,124 @@ var _ = Describe("Counter", func() { Expect(*staticNodePool.Spec.Replicas).To(Equal(int64(3))) }) }) + + Context("NodeClaims Drift Tracking", func() { + BeforeEach(func() { + nodePool = test.NodePool() + _ = nodePool.StatusConditions().Clear(v1.ConditionTypeNodeClaimsDrifted) + ExpectApplied(ctx, env.Client, nodePool) + ExpectReconcileSucceeded(ctx, nodePoolInformerController, client.ObjectKeyFromObject(nodePool)) + ExpectObjectReconciled(ctx, env.Client, nodePoolController, nodePool) + nodePool = ExpectExists(ctx, env.Client, nodePool) + }) + + It("should set Drifted to false if there are no owned nodeclaims", func() { + ExpectObjectReconciled(ctx, env.Client, nodePoolController, nodePool) + nodePool = ExpectExists(ctx, env.Client, nodePool) + driftedNodeClaimCount := nodePool.Status.Resources[resources.DriftedNodeClaim] + Expect(driftedNodeClaimCount.IsZero()).To(BeTrue()) + Expect(nodePool.StatusConditions().Get(v1.ConditionTypeNodeClaimsDrifted).IsFalse()).To(BeTrue()) + }) + + It("should set Drifted to false if the drifted condition of nodeclaim is nil", func() { + nodeClaim := test.NodeClaim(v1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + v1.NodePoolLabelKey: nodePool.Name, + }}, + Status: v1.NodeClaimStatus{ + ProviderID: test.RandomProviderID(), + }, + }) + ExpectApplied(ctx, env.Client, nodeClaim) + ExpectReconcileSucceeded(ctx, nodeClaimController, client.ObjectKeyFromObject(nodeClaim)) + ExpectObjectReconciled(ctx, env.Client, nodePoolController, nodePool) + nodePool = ExpectExists(ctx, env.Client, nodePool) + Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted)).To(BeNil()) + + driftedNodeClaimCount := nodePool.Status.Resources[resources.DriftedNodeClaim] + Expect(driftedNodeClaimCount.IsZero()).To(BeTrue()) + Expect(nodePool.StatusConditions().Get(v1.ConditionTypeNodeClaimsDrifted).IsFalse()).To(BeTrue()) + }) + + It("should set Drifted to false if all NodeClaims are not drifted", func() { + nodeClaim := test.NodeClaim(v1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + v1.NodePoolLabelKey: nodePool.Name, + }}, + Status: v1.NodeClaimStatus{ + ProviderID: test.RandomProviderID(), + }, + }) + nodeClaim.StatusConditions().SetFalse(v1.ConditionTypeDrifted, "NotDrifted", "NotDrifted") + Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsFalse()).To(BeTrue()) + + ExpectApplied(ctx, env.Client, nodeClaim) + ExpectReconcileSucceeded(ctx, nodeClaimController, client.ObjectKeyFromObject(nodeClaim)) + ExpectObjectReconciled(ctx, env.Client, nodePoolController, nodePool) + + nodePool = ExpectExists(ctx, env.Client, nodePool) + driftedNodeClaimCount := nodePool.Status.Resources[resources.DriftedNodeClaim] + Expect(driftedNodeClaimCount.IsZero()).To(BeTrue()) + Expect(nodePool.StatusConditions().Get(v1.ConditionTypeNodeClaimsDrifted).IsFalse()).To(BeTrue()) + }) + + It("should set Drifted to true if there are drifted NodeClaims", func() { + nodeClaim := test.NodeClaim(v1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + v1.NodePoolLabelKey: nodePool.Name, + }}, + Status: v1.NodeClaimStatus{ + ProviderID: test.RandomProviderID(), + }, + }) + + unknownNodeClaim := test.NodeClaim(v1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + v1.NodePoolLabelKey: nodePool.Name, + }}, + Status: v1.NodeClaimStatus{ + ProviderID: test.RandomProviderID(), + }, + }) + unknownNodeClaim.StatusConditions().SetUnknown(v1.ConditionTypeDrifted) + + notDriftedNodeClaim := test.NodeClaim(v1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + v1.NodePoolLabelKey: nodePool.Name, + }}, + Status: v1.NodeClaimStatus{ + ProviderID: test.RandomProviderID(), + }, + }) + notDriftedNodeClaim.StatusConditions().SetFalse(v1.ConditionTypeDrifted, "NotDrifted", "NotDrifted") + + driftedNodeClaim := test.NodeClaim(v1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + v1.NodePoolLabelKey: nodePool.Name, + }}, + Status: v1.NodeClaimStatus{ + ProviderID: test.RandomProviderID(), + }, + }) + driftedNodeClaim.StatusConditions().SetTrue(v1.ConditionTypeDrifted) + + Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted)).To(BeNil()) + Expect(unknownNodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsUnknown()).To(BeTrue()) + Expect(notDriftedNodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsFalse()).To(BeTrue()) + Expect(driftedNodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsTrue()).To(BeTrue()) + + ExpectApplied(ctx, env.Client, nodeClaim, unknownNodeClaim, driftedNodeClaim, notDriftedNodeClaim) + ExpectReconcileSucceeded(ctx, nodeClaimController, client.ObjectKeyFromObject(nodeClaim)) + ExpectReconcileSucceeded(ctx, nodeClaimController, client.ObjectKeyFromObject(unknownNodeClaim)) + ExpectReconcileSucceeded(ctx, nodeClaimController, client.ObjectKeyFromObject(driftedNodeClaim)) + ExpectReconcileSucceeded(ctx, nodeClaimController, client.ObjectKeyFromObject(notDriftedNodeClaim)) + + ExpectObjectReconciled(ctx, env.Client, nodePoolController, nodePool) + nodePool = ExpectExists(ctx, env.Client, nodePool) + + driftedNodeClaimCount := nodePool.Status.Resources[resources.DriftedNodeClaim] + Expect(driftedNodeClaimCount.Value()).To(Equal(int64(1))) + Expect(nodePool.StatusConditions().Get(v1.ConditionTypeNodeClaimsDrifted).IsTrue()).To(BeTrue()) + }) + }) }) diff --git a/pkg/controllers/state/cluster.go b/pkg/controllers/state/cluster.go index 3d332aee4b..9e27843aec 100644 --- a/pkg/controllers/state/cluster.go +++ b/pkg/controllers/state/cluster.go @@ -781,6 +781,23 @@ func (c *Cluster) updateNodePoolResources(oldNode, newNode *StateNode) { c.nodePoolResources[newNodePoolName][resourceName] = current } } + // Count drifted NodeClaims for each affected NodePool + for _, nodePoolName := range []string{oldNodePoolName, newNodePoolName} { + if nodePoolName == "" { + continue + } + driftedCount := 0 + for _, n := range c.nodes { + if n.Labels()[v1.NodePoolLabelKey] == nodePoolName { + if n.NodeClaim != nil && n.NodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsTrue() { + driftedCount++ + } + } + } + if _, ok := c.nodePoolResources[nodePoolName]; ok { + c.nodePoolResources[nodePoolName][resources.DriftedNodeClaim] = resource.MustParse(fmt.Sprintf("%d", driftedCount)) + } + } // Garbage collect any NodePool keys that no longer have any resources assigned to them. // We do this when there are no longer any NodeClaims that map to this NodePool // so that we don't leak NodePool keys in our nodePoolResources map diff --git a/pkg/utils/resources/resources.go b/pkg/utils/resources/resources.go index 2590963ee8..b90b205c5e 100644 --- a/pkg/utils/resources/resources.go +++ b/pkg/utils/resources/resources.go @@ -25,6 +25,7 @@ import ( ) var Node = v1.ResourceName("nodes") +var DriftedNodeClaim = v1.ResourceName("driftedNodeClaims") // RequestsForPods returns the total resources of a variadic list of podspecs. func RequestsForPods(pods ...*v1.Pod) v1.ResourceList {