Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions pkg/controllers/disruption/consolidation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2302,7 +2302,7 @@ var _ = Describe("Consolidation", func() {
// and delete the old one
ExpectNotFound(ctx, env.Client, nodeClaims[1], nodes[1])
})
It("does not delete nodes when there is pod churn", func() {
It("does not delete nodes with pod churn, deletes nodes without pod churn", func() {
// create our RS so we can link a pod to it
ExpectApplied(ctx, env.Client, nodePool)
for i := range 2 {
Expand All @@ -2329,7 +2329,10 @@ var _ = Describe("Consolidation", func() {
wg.Wait()
Expect(err).To(Succeed())
Expect(results).To(Equal(pscheduling.Results{}))
Expect(cmd).To(Equal(disruption.Command{}))
Expect(cmd.Candidates()).To(HaveLen(1))
// the test validator manually binds a pod to nodes[0], causing it to no longer be eligible
Expect(cmd.Candidates()[0].StateNode.Node.Name).To(Equal(nodes[1].Name))
Expect(cmd.Decision()).To(Equal(disruption.DeleteDecision))

Expect(emptyConsolidation.IsConsolidated()).To(BeFalse())

Expand Down
4 changes: 4 additions & 0 deletions pkg/controllers/disruption/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ func (c Command) Decision() Decision {
}
}

func (c Command) Candidates() []*Candidate {
return c.candidates
}

func (c Command) LogValues() []any {
podCount := lo.Reduce(c.candidates, func(_ int, cd *Candidate, _ int) int { return len(cd.reschedulablePods) }, 0)

Expand Down
28 changes: 28 additions & 0 deletions pkg/controllers/disruption/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"time"

"github.com/samber/lo"
"k8s.io/utils/clock"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand Down Expand Up @@ -149,6 +150,33 @@ func (c *ConsolidationValidator) isValid(ctx context.Context, cmd Command, valid
return nil
}

func (e *EmptinessValidator) validateCandidates(ctx context.Context, candidates ...*Candidate) ([]*Candidate, error) {
validatedCandidates, err := GetCandidates(ctx, e.cluster, e.kubeClient, e.recorder, e.clock, e.cloudProvider, e.filter, GracefulDisruptionClass, e.queue)
if err != nil {
return nil, fmt.Errorf("constructing validation candidates, %w", err)
}
validatedCandidates = mapCandidates(candidates, validatedCandidates)
if len(validatedCandidates) == 0 {
return nil, NewValidationError(fmt.Errorf("%d candidates are no longer valid", len(candidates)))
}
disruptionBudgetMapping, err := BuildDisruptionBudgetMapping(ctx, e.cluster, e.clock, e.kubeClient, e.cloudProvider, e.recorder, e.reason)
if err != nil {
return nil, fmt.Errorf("building disruption budgets, %w", err)
}

if valid := lo.Filter(validatedCandidates, func(cn *Candidate, _ int) bool {
if e.cluster.IsNodeNominated(cn.ProviderID()) || disruptionBudgetMapping[cn.NodePool.Name] == 0 {
return false
}
disruptionBudgetMapping[cn.NodePool.Name]--
return true
}); len(valid) > 0 {
return valid, nil
}
return nil, NewValidationError(fmt.Errorf("a candidate failed validation because it was nominated for a pod or would violate disruption budgets"))

}

// ValidateCandidates gets the current representation of the provided candidates and ensures that they are all still valid.
// For a candidate to still be valid, the following conditions must be met:
//
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/nodeclaim/disruption/drift.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func instanceTypeNotFound(its []*cloudprovider.InstanceType, nodeClaim *v1.NodeC
}

// Eligible fields for drift are described in the docs
// https://karpenter.sh/docs/concepts/deprovisioning/#drift
// https://karpenter.sh/docs/concepts/disruption/#drift
func areStaticFieldsDrifted(nodePool *v1.NodePool, nodeClaim *v1.NodeClaim) cloudprovider.DriftReason {
nodePoolHash, foundNodePoolHash := nodePool.Annotations[v1.NodePoolHashAnnotationKey]
nodePoolHashVersion, foundNodePoolHashVersion := nodePool.Annotations[v1.NodePoolHashVersionAnnotationKey]
Expand Down
Loading