Skip to content

test: deflake NodeClaim and presubmit tests #2240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 22, 2025
Merged

test: deflake NodeClaim and presubmit tests #2240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74f48cd
test: deflake tests
rschalo May 21, 2025
ade1dd8
test: deflake presubmit
rschalo May 22, 2025
0a825ce
Merge branch 'kubernetes-sigs:main' into main
rschalo May 22, 2025
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
11 changes: 8 additions & 3 deletions pkg/controllers/disruption/consolidation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,10 @@ var _ = Describe("Consolidation", func() {
})
})
DescribeTable("should correctly report invalidated commands for emptiness disruption", func(validatorOpt TestEmptinessValidatorOption) {
nodes := []*corev1.Node{node}
nodeClaims := []*v1.NodeClaim{nodeClaim}
ExpectApplied(ctx, env.Client, nodePool, nodeClaim, node)
ExpectMakeNodesAndNodeClaimsInitializedAndStateUpdated(ctx, env.Client, nodeStateController, nodeClaimStateController, []*corev1.Node{node}, []*v1.NodeClaim{nodeClaim})
ExpectMakeNodesAndNodeClaimsInitializedAndStateUpdated(ctx, env.Client, nodeStateController, nodeClaimStateController, nodes, nodeClaims)

emptyConsolidation := disruption.NewEmptiness(disruption.MakeConsolidation(fakeClock, cluster, env.Client, prov, cloudProvider, recorder, queue))
budgets, err := disruption.BuildDisruptionBudgetMapping(ctx, cluster, fakeClock, env.Client, cloudProvider, recorder, emptyConsolidation.Reason())
Expand All @@ -228,7 +230,7 @@ var _ = Describe("Consolidation", func() {
candidates, err := disruption.GetCandidates(ctx, cluster, env.Client, recorder, fakeClock, cloudProvider, emptyConsolidation.ShouldDisrupt, emptyConsolidation.Class(), queue)
Expect(err).To(Succeed())

emptyConsolidation.Validator = NewTestEmptinessValidator(cluster, nodePool, emptyConsolidation.Validator.(*disruption.EmptinessValidator), validatorOpt)
emptyConsolidation.Validator = NewTestEmptinessValidator(nodes, nodeClaims, nodePool, emptyConsolidation.Validator.(*disruption.EmptinessValidator), validatorOpt)

fakeClock.Step(10 * time.Minute)

Expand Down Expand Up @@ -2446,6 +2448,9 @@ var _ = Describe("Consolidation", func() {
for i := range 2 {
ExpectApplied(ctx, env.Client, nodeClaims[i], nodes[i])
}
sort.Slice(nodes, func(i, j int) bool {
return nodes[i].Name < nodes[j].Name
})

ExpectMakeNodesAndNodeClaimsInitializedAndStateUpdated(ctx, env.Client, nodeStateController, nodeClaimStateController, nodes, nodeClaims)

Expand All @@ -2457,7 +2462,7 @@ var _ = Describe("Consolidation", func() {
Expect(err).To(Succeed())

// this test validator invalidates the command because it creates pod churn during validaiton
emptyConsolidation.Validator = NewTestEmptinessValidator(cluster, nodePool, emptyConsolidation.Validator.(*disruption.EmptinessValidator), WithEmptinessChurn())
emptyConsolidation.Validator = NewTestEmptinessValidator(nodes, nodeClaims, nodePool, emptyConsolidation.Validator.(*disruption.EmptinessValidator), WithEmptinessChurn())

fakeClock.Step(10 * time.Minute)

Expand Down
39 changes: 17 additions & 22 deletions pkg/controllers/disruption/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@ import (
)

type TestEmptinessValidator struct {
blocked bool
churn bool
nominated bool
cluster *state.Cluster
nodePool *v1.NodePool
emptiness *disruption.EmptinessValidator
blocked bool
churn bool
nominated bool
nodes []*corev1.Node
nodeClaims []*v1.NodeClaim
nodePool *v1.NodePool
emptiness *disruption.EmptinessValidator
}

type TestEmptinessValidatorOption func(*TestEmptinessValidator)
Expand All @@ -62,11 +63,12 @@ func WithEmptinessNodeNomination() TestEmptinessValidatorOption {
}
}

func NewTestEmptinessValidator(cluster *state.Cluster, nodePool *v1.NodePool, e *disruption.EmptinessValidator, opts ...TestEmptinessValidatorOption) disruption.Validator {
func NewTestEmptinessValidator(nodes []*corev1.Node, nodeClaims []*v1.NodeClaim, nodePool *v1.NodePool, e *disruption.EmptinessValidator, opts ...TestEmptinessValidatorOption) disruption.Validator {
v := &TestEmptinessValidator{
cluster: cluster,
nodePool: nodePool,
emptiness: e,
nodes: nodes,
nodeClaims: nodeClaims,
nodePool: nodePool,
emptiness: e,
}
for _, opt := range opts {
opt(v)
Expand All @@ -75,21 +77,14 @@ func NewTestEmptinessValidator(cluster *state.Cluster, nodePool *v1.NodePool, e
}

func (t *TestEmptinessValidator) Validate(ctx context.Context, cmd disruption.Command, _ time.Duration) (disruption.Command, error) {
stateNodes := t.cluster.Nodes()
nodes := make([]*corev1.Node, len(stateNodes))
nodeClaims := make([]*v1.NodeClaim, len(stateNodes))
for i, stateNode := range stateNodes {
nodes[i] = stateNode.Node
nodeClaims[i] = stateNode.NodeClaim
}
if t.blocked {
blockingBudget(nodes, nodeClaims, t.nodePool)
blockingBudget(t.nodes, t.nodeClaims, t.nodePool)
}
if t.churn {
churn(nodes, nodeClaims)
churn(t.nodes, t.nodeClaims)
}
if t.nominated {
nominated(nodes, nodeClaims, t.cluster)
nominated(t.nodes, t.nodeClaims)
}
return t.emptiness.Validate(ctx, cmd, 0)
}
Expand Down Expand Up @@ -150,7 +145,7 @@ func (t *TestConsolidationValidator) Validate(ctx context.Context, cmd disruptio
churn(nodes, nodeClaims)
}
if t.nominated {
nominated(nodes, nodeClaims, t.cluster)
nominated(nodes, nodeClaims)
}
return t.consolidation.Validate(ctx, cmd, 0)
}
Expand Down Expand Up @@ -193,7 +188,7 @@ func blockingBudget(nodes []*corev1.Node, nodeClaims []*v1.NodeClaim, nodePool *
ExpectApplied(ctx, env.Client, nodePool)
}

func nominated(nodes []*corev1.Node, nodeClaims []*v1.NodeClaim, cluster *state.Cluster) {
func nominated(nodes []*corev1.Node, nodeClaims []*v1.NodeClaim) {
ExpectMakeNodesAndNodeClaimsInitializedAndStateUpdated(ctx, env.Client, nodeStateController, nodeClaimStateController, nodes, nodeClaims)
for i := range nodes {
cluster.NominateNodeForPod(ctx, nodes[i].Spec.ProviderID)
Expand Down
6 changes: 3 additions & 3 deletions test/suites/regression/nodeclaim_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ var _ = Describe("NodeClaim", func() {
// Expect that the nodeClaim is eventually de-provisioned due to the registration timeout
Eventually(func(g Gomega) {
g.Expect(errors.IsNotFound(env.Client.Get(env.Context, client.ObjectKeyFromObject(nodeClaim), nodeClaim))).To(BeTrue())
}).WithTimeout(time.Minute * 16).Should(Succeed())
}).WithTimeout(time.Minute * 17).Should(Succeed())
})
It("should delete a NodeClaim if it references a NodeClass that doesn't exist", func() {
nodeClaim := test.NodeClaim(v1.NodeClaim{
Expand Down Expand Up @@ -291,8 +291,8 @@ var _ = Describe("NodeClaim", func() {
nodeClaim = env.ExpectExists(nodeClaim).(*v1.NodeClaim)

By("Updated NodeClaim Status")
nodeClaim.Status.ProviderID = "test-provider-id"
nodeClaim.Status.NodeName = "test-node-name"
nodeClaim.Status.ProviderID = ""
nodeClaim.Status.NodeName = ""
nodeClaim.StatusConditions().SetTrue(v1.ConditionTypeLaunched)
nodeClaim.StatusConditions().SetTrue(v1.ConditionTypeRegistered)
nodeClaim.StatusConditions().SetTrue(v1.ConditionTypeInitialized)
Expand Down
Loading