diff --git a/kwok/charts/crds/karpenter.sh_nodeclaims.yaml b/kwok/charts/crds/karpenter.sh_nodeclaims.yaml index dbb84c33a0..0f8489c79e 100644 --- a/kwok/charts/crds/karpenter.sh_nodeclaims.yaml +++ b/kwok/charts/crds/karpenter.sh_nodeclaims.yaml @@ -32,6 +32,9 @@ spec: - jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string + - jsonPath: .status.conditions[?(@.type=="Drifted")].status + name: Drifted + type: string - jsonPath: .metadata.creationTimestamp name: Age type: date diff --git a/pkg/apis/crds/karpenter.sh_nodeclaims.yaml b/pkg/apis/crds/karpenter.sh_nodeclaims.yaml index 517088614f..35c12bfb69 100644 --- a/pkg/apis/crds/karpenter.sh_nodeclaims.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeclaims.yaml @@ -32,6 +32,9 @@ spec: - jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string + - jsonPath: .status.conditions[?(@.type=="Drifted")].status + name: Drifted + type: string - jsonPath: .metadata.creationTimestamp name: Age type: date diff --git a/pkg/apis/v1/nodeclaim.go b/pkg/apis/v1/nodeclaim.go index ccaf4df97e..5bf040e15b 100644 --- a/pkg/apis/v1/nodeclaim.go +++ b/pkg/apis/v1/nodeclaim.go @@ -132,6 +132,7 @@ type Provider = runtime.RawExtension // +kubebuilder:printcolumn:name="Zone",type="string",JSONPath=".metadata.labels.topology\\.kubernetes\\.io/zone",description="" // +kubebuilder:printcolumn:name="Node",type="string",JSONPath=".status.nodeName",description="" // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="" +// +kubebuilder:printcolumn:name="Drifted",type="string",JSONPath=".status.conditions[?(@.type==\"Drifted\")].status",description="" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="" // +kubebuilder:printcolumn:name="ID",type="string",JSONPath=".status.providerID",priority=1,description="" // +kubebuilder:printcolumn:name="NodePool",type="string",JSONPath=".metadata.labels.karpenter\\.sh/nodepool",priority=1,description="" diff --git a/pkg/controllers/nodeclaim/disruption/drift.go b/pkg/controllers/nodeclaim/disruption/drift.go index 879433b19a..22dcb341af 100644 --- a/pkg/controllers/nodeclaim/disruption/drift.go +++ b/pkg/controllers/nodeclaim/disruption/drift.go @@ -44,12 +44,13 @@ type Drift struct { } func (d *Drift) Reconcile(ctx context.Context, nodePool *v1.NodePool, nodeClaim *v1.NodeClaim) (reconcile.Result, error) { - hasDriftedCondition := nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted) != nil + hasDriftedCondition := nodeClaim.StatusConditions().IsTrue(v1.ConditionTypeDrifted) + // Prefer to set the Drifted condition to false instead of clearing for the printer column output. // From here there are three scenarios to handle: - // 1. If NodeClaim is not launched, remove the drift status condition + // 1. If NodeClaim is not launched, set the drift status condition false if !nodeClaim.StatusConditions().Get(v1.ConditionTypeLaunched).IsTrue() { - _ = nodeClaim.StatusConditions().Clear(v1.ConditionTypeDrifted) + _ = nodeClaim.StatusConditions().SetFalse(v1.ConditionTypeDrifted, v1.ConditionTypeDrifted, "") if hasDriftedCondition { log.FromContext(ctx).V(1).Info("removing drift status condition, isn't launched") } @@ -59,10 +60,10 @@ func (d *Drift) Reconcile(ctx context.Context, nodePool *v1.NodePool, nodeClaim if err != nil { return reconcile.Result{}, cloudprovider.IgnoreNodeClaimNotFoundError(fmt.Errorf("getting drift, %w", err)) } - // 2. Otherwise, if the NodeClaim isn't drifted, but has the status condition, remove it. + // 2. Otherwise, if the NodeClaim isn't drifted, but has the status condition, unset it if driftedReason == "" { if hasDriftedCondition { - _ = nodeClaim.StatusConditions().Clear(v1.ConditionTypeDrifted) + _ = nodeClaim.StatusConditions().SetFalse(v1.ConditionTypeDrifted, v1.ConditionTypeDrifted, "") log.FromContext(ctx).V(1).Info("removing drifted status condition, not drifted") } return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil diff --git a/pkg/controllers/nodeclaim/disruption/drift_test.go b/pkg/controllers/nodeclaim/disruption/drift_test.go index 7ba3dd13f0..fbae6ed25e 100644 --- a/pkg/controllers/nodeclaim/disruption/drift_test.go +++ b/pkg/controllers/nodeclaim/disruption/drift_test.go @@ -167,7 +167,7 @@ var _ = Describe("Drift", func() { ExpectObjectReconciled(ctx, env.Client, nodeClaimDisruptionController, nodeClaim) nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) - Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted)).To(BeNil()) + Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsTrue()).To(BeFalse()) }) It("should remove the status condition from the nodeClaim when the nodeClaim launch condition is false", func() { cp.Drifted = "drifted" @@ -179,7 +179,7 @@ var _ = Describe("Drift", func() { ExpectObjectReconciled(ctx, env.Client, nodeClaimDisruptionController, nodeClaim) nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) - Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted)).To(BeNil()) + Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsTrue()).To(BeFalse()) }) It("should not detect drift if the nodePool does not exist", func() { cp.Drifted = "drifted" @@ -187,7 +187,7 @@ var _ = Describe("Drift", func() { ExpectObjectReconciled(ctx, env.Client, nodeClaimDisruptionController, nodeClaim) nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) - Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted)).To(BeNil()) + Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsTrue()).To(BeFalse()) }) It("should remove the status condition from the nodeClaim if the nodeClaim is no longer drifted", func() { cp.Drifted = "" @@ -197,7 +197,17 @@ var _ = Describe("Drift", func() { ExpectObjectReconciled(ctx, env.Client, nodeClaimDisruptionController, nodeClaim) nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) - Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted)).To(BeNil()) + Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsTrue()).To(BeFalse()) + }) + It("should set the drifted condition to false if unset after reconcile", func() { + cp.Drifted = "" + _ = nodeClaim.StatusConditions().Clear(v1.ConditionTypeDrifted) + ExpectApplied(ctx, env.Client, nodePool, nodeClaim) + + ExpectObjectReconciled(ctx, env.Client, nodeClaimDisruptionController, nodeClaim) + + nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) + Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsTrue()).To(BeFalse()) }) Context("NodeRequirement Drift", func() { DescribeTable("", diff --git a/pkg/controllers/nodeclaim/disruption/suite_test.go b/pkg/controllers/nodeclaim/disruption/suite_test.go index 23fbc1641b..13df0c0ebb 100644 --- a/pkg/controllers/nodeclaim/disruption/suite_test.go +++ b/pkg/controllers/nodeclaim/disruption/suite_test.go @@ -153,7 +153,7 @@ var _ = Describe("Disruption", func() { ExpectObjectReconciled(ctx, env.Client, nodeClaimDisruptionController, nodeClaim) nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) - Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted)).To(BeNil()) + Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeDrifted).IsTrue()).To(BeFalse()) Expect(nodeClaim.StatusConditions().Get(v1.ConditionTypeConsolidatable)).To(BeNil()) }) })