chore: Fix karpenter_nodeclaims_drifted metric (#875)

kubernetes-sigs · Dec 16, 2023 · 6703473 · 6703473
1 parent dbc3a2d
commit 6703473
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 4 deletions.
diff --git a/pkg/controllers/nodeclaim/disruption/drift.go b/pkg/controllers/nodeclaim/disruption/drift.go
@@ -86,7 +86,7 @@ func (d *Drift) Reconcile(ctx context.Context, nodePool *v1beta1.NodePool, nodeC
 		Reason:   string(driftedReason),
 	})
 	if !hasDriftedCondition {
-		logging.FromContext(ctx).Debugf("marking drifted")
+		logging.FromContext(ctx).With("reason", string(driftedReason)).Debugf("marking drifted")
 		nodeclaimutil.DisruptedCounter(nodeClaim, metrics.DriftReason).Inc()
 		nodeclaimutil.DriftedCounter(nodeClaim, string(driftedReason)).Inc()
 	}

diff --git a/pkg/controllers/nodeclaim/disruption/drift_test.go b/pkg/controllers/nodeclaim/disruption/drift_test.go
@@ -58,6 +58,59 @@ var _ = Describe("Drift", func() {
 		// NodeClaims are required to be launched before they can be evaluated for drift
 		nodeClaim.StatusConditions().MarkTrue(v1beta1.Launched)
 	})
+	Context("Metrics", func() {
+		It("should fire a karpenter_nodeclaims_drifted metric when drifted", func() {
+			cp.Drifted = "CloudProviderDrifted"
+			ExpectApplied(ctx, env.Client, nodePool, nodeClaim)
+			ExpectReconcileSucceeded(ctx, nodeClaimDisruptionController, client.ObjectKeyFromObject(nodeClaim))
+
+			nodeClaim = ExpectExists(ctx, env.Client, nodeClaim)
+			Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Drifted).IsTrue()).To(BeTrue())
+			metric, found := FindMetricWithLabelValues("karpenter_nodeclaims_drifted", map[string]string{
+				"type":     "CloudProviderDrifted",
+				"nodepool": nodePool.Name,
+			})
+			Expect(found).To(BeTrue())
+			Expect(metric.GetCounter().GetValue()).To(BeNumerically("==", 1))
+		})
+		It("should pass-through the correct drifted type value through the karpenter_nodeclaims_drifted metric", func() {
+			cp.Drifted = "drifted"
+			nodePool.Spec.Template.Spec.Requirements = []v1.NodeSelectorRequirement{
+				{
+					Key:      v1.LabelInstanceTypeStable,
+					Operator: v1.NodeSelectorOpDoesNotExist,
+				},
+			}
+			ExpectApplied(ctx, env.Client, nodePool, nodeClaim)
+			ExpectReconcileSucceeded(ctx, nodeClaimDisruptionController, client.ObjectKeyFromObject(nodeClaim))
+
+			nodeClaim = ExpectExists(ctx, env.Client, nodeClaim)
+			Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Drifted).IsTrue()).To(BeTrue())
+			Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Drifted).Reason).To(Equal(string(disruption.RequirementsDrifted)))
+
+			metric, found := FindMetricWithLabelValues("karpenter_nodeclaims_drifted", map[string]string{
+				"type":     "RequirementsDrifted",
+				"nodepool": nodePool.Name,
+			})
+			Expect(found).To(BeTrue())
+			Expect(metric.GetCounter().GetValue()).To(BeNumerically("==", 1))
+		})
+		It("should fire a karpenter_nodeclaims_disrupted metric when drifted", func() {
+			cp.Drifted = "drifted"
+			ExpectApplied(ctx, env.Client, nodePool, nodeClaim)
+			ExpectReconcileSucceeded(ctx, nodeClaimDisruptionController, client.ObjectKeyFromObject(nodeClaim))
+
+			nodeClaim = ExpectExists(ctx, env.Client, nodeClaim)
+			Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Drifted).IsTrue()).To(BeTrue())
+
+			metric, found := FindMetricWithLabelValues("karpenter_nodeclaims_disrupted", map[string]string{
+				"type":     "drift",
+				"nodepool": nodePool.Name,
+			})
+			Expect(found).To(BeTrue())
+			Expect(metric.GetCounter().GetValue()).To(BeNumerically("==", 1))
+		})
+	})
 	It("should detect drift", func() {
 		cp.Drifted = "drifted"
 		ExpectApplied(ctx, env.Client, nodePool, nodeClaim)

diff --git a/pkg/controllers/nodeclaim/disruption/emptiness_test.go b/pkg/controllers/nodeclaim/disruption/emptiness_test.go
@@ -51,7 +51,24 @@ var _ = Describe("Emptiness", func() {
 			},
 		})
 	})
-
+	Context("Metrics", func() {
+		It("should fire a karpenter_nodeclaims_disrupted metric when empty", func() {
+			ExpectApplied(ctx, env.Client, nodePool, nodeClaim, node)
+			ExpectMakeNodeClaimsInitialized(ctx, env.Client, nodeClaim)
+
+			ExpectReconcileSucceeded(ctx, nodeClaimDisruptionController, client.ObjectKeyFromObject(nodeClaim))
+
+			nodeClaim = ExpectExists(ctx, env.Client, nodeClaim)
+			Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Empty).IsTrue()).To(BeTrue())
+
+			metric, found := FindMetricWithLabelValues("karpenter_nodeclaims_disrupted", map[string]string{
+				"type":     "emptiness",
+				"nodepool": nodePool.Name,
+			})
+			Expect(found).To(BeTrue())
+			Expect(metric.GetCounter().GetValue()).To(BeNumerically("==", 1))
+		})
+	})
 	It("should mark NodeClaims as empty", func() {
 		ExpectApplied(ctx, env.Client, nodePool, nodeClaim, node)
 		ExpectMakeNodeClaimsInitialized(ctx, env.Client, nodeClaim)

diff --git a/pkg/controllers/nodeclaim/disruption/expiration_test.go b/pkg/controllers/nodeclaim/disruption/expiration_test.go
@@ -45,7 +45,26 @@ var _ = Describe("Expiration", func() {
 			},
 		})
 	})
-
+	Context("Metrics", func() {
+		It("should fire a karpenter_nodeclaims_disrupted metric when expired", func() {
+			nodePool.Spec.Disruption.ExpireAfter.Duration = lo.ToPtr(time.Second * 30)
+			ExpectApplied(ctx, env.Client, nodePool, nodeClaim)
+
+			// step forward to make the node expired
+			fakeClock.Step(60 * time.Second)
+			ExpectReconcileSucceeded(ctx, nodeClaimDisruptionController, client.ObjectKeyFromObject(nodeClaim))
+
+			nodeClaim = ExpectExists(ctx, env.Client, nodeClaim)
+			Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Expired).IsTrue()).To(BeTrue())
+
+			metric, found := FindMetricWithLabelValues("karpenter_nodeclaims_disrupted", map[string]string{
+				"type":     "expiration",
+				"nodepool": nodePool.Name,
+			})
+			Expect(found).To(BeTrue())
+			Expect(metric.GetCounter().GetValue()).To(BeNumerically("==", 1))
+		})
+	})
 	It("should remove the status condition from the NodeClaims when expiration is disabled", func() {
 		nodePool.Spec.Disruption.ExpireAfter.Duration = nil
 		nodeClaim.StatusConditions().MarkTrue(v1beta1.Expired)

diff --git a/pkg/utils/nodeclaim/nodeclaim.go b/pkg/utils/nodeclaim/nodeclaim.go
@@ -276,7 +276,7 @@ func DisruptedCounter(nodeClaim *v1beta1.NodeClaim, disruptionType string) prome
 }
 
 func DriftedCounter(nodeClaim *v1beta1.NodeClaim, driftType string) prometheus.Counter {
-	return metrics.NodeClaimsDisruptedCounter.With(prometheus.Labels{
+	return metrics.NodeClaimsDriftedCounter.With(prometheus.Labels{
 		metrics.TypeLabel:     driftType,
 		metrics.NodePoolLabel: nodeClaim.Labels[v1beta1.NodePoolLabelKey],
 	})