Skip to content

Commit

Permalink
Expose drifted nodeclaim condition status to printer column output
Browse files Browse the repository at this point in the history
Signed-off-by: Cameron McAvoy <[email protected]>
  • Loading branch information
cnmcavoy committed Nov 26, 2024
1 parent 6efbae4 commit b14a101
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 10 deletions.
3 changes: 3 additions & 0 deletions kwok/charts/crds/karpenter.sh_nodeclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/crds/karpenter.sh_nodeclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/v1/nodeclaim.go
Original file line number Diff line number Diff line change
Expand Up @@ -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=""
Expand Down
11 changes: 6 additions & 5 deletions pkg/controllers/nodeclaim/disruption/drift.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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
Expand Down
18 changes: 14 additions & 4 deletions pkg/controllers/nodeclaim/disruption/drift_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -179,15 +179,15 @@ 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"
ExpectApplied(ctx, env.Client, nodeClaim)
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 = ""
Expand All @@ -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("",
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/nodeclaim/disruption/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})

0 comments on commit b14a101

Please sign in to comment.