Skip to content

Commit

Permalink
test: Add fix for nodes leaking into subsequent tests for budgets (#5662
Browse files Browse the repository at this point in the history
)
  • Loading branch information
jonathan-innis authored and jigisha620 committed Feb 16, 2024
1 parent 0e53855 commit e4bde9a
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 23 deletions.
2 changes: 1 addition & 1 deletion test/pkg/environment/common/expectations.go
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ func (env *Environment) printControllerLogs(options *v1.PodLogOptions) {
temp := options.DeepCopy() // local version of the log options

fmt.Printf("------- pod/%s -------\n", pod.Name)
if pod.Status.ContainerStatuses[0].RestartCount > 0 {
if len(pod.Status.ContainerStatuses) > 0 && pod.Status.ContainerStatuses[0].RestartCount > 0 {
fmt.Printf("[PREVIOUS CONTAINER LOGS]\n")
temp.Previous = true
}
Expand Down
21 changes: 14 additions & 7 deletions test/suites/consolidation/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,14 +283,15 @@ var _ = Describe("Consolidation", func() {

env.ExpectCreated(nodeClass, nodePool, deployments[0], deployments[1], deployments[2], deployments[3], deployments[4])

env.EventuallyExpectCreatedNodeClaimCount("==", 5)
nodes := env.EventuallyExpectCreatedNodeCount("==", 5)
originalNodeClaims := env.EventuallyExpectCreatedNodeClaimCount("==", 5)
originalNodes := env.EventuallyExpectCreatedNodeCount("==", 5)

// Check that all daemonsets and deployment pods are online
env.EventuallyExpectHealthyPodCount(selector, int(numPods)*2)

By("cordoning and adding finalizer to the nodes")
// Add a finalizer to each node so that we can stop termination disruptions
for _, node := range nodes {
for _, node := range originalNodes {
Expect(env.Client.Get(env.Context, client.ObjectKeyFromObject(node), node)).To(Succeed())
node.Finalizers = append(node.Finalizers, common.TestingFinalizer)
env.ExpectUpdated(node)
Expand All @@ -305,16 +306,22 @@ var _ = Describe("Consolidation", func() {
nodePool.Spec.Disruption.ConsolidateAfter = nil
env.ExpectUpdated(nodePool)

// Ensure that we get two nodes tainted, and they have overlap during the consolidation
// Ensure that we get three nodes tainted, and they have overlap during the consolidation
env.EventuallyExpectTaintedNodeCount("==", 3)
env.EventuallyExpectNodeClaimCount("==", 8)
env.EventuallyExpectNodeCount("==", 8)
nodes = env.ConsistentlyExpectDisruptionsWithNodeCount(3, 8, 5*time.Second)
env.ConsistentlyExpectDisruptionsWithNodeCount(3, 8, 5*time.Second)

for _, node := range nodes {
for _, node := range originalNodes {
Expect(env.ExpectTestingFinalizerRemoved(node)).To(Succeed())
}
env.EventuallyExpectNotFound(nodes[0], nodes[1], nodes[2])

// Eventually expect all the nodes to be rolled and completely removed
// Since this completes the disruption operation, this also ensures that we aren't leaking nodes into subsequent
// tests since nodeclaims that are actively replacing but haven't brought-up nodes yet can register nodes later
env.EventuallyExpectNotFound(lo.Map(originalNodes, func(n *v1.Node, _ int) client.Object { return n })...)
env.EventuallyExpectNotFound(lo.Map(originalNodeClaims, func(n *corev1beta1.NodeClaim, _ int) client.Object { return n })...)
env.ExpectNodeClaimCount("==", 5)
env.ExpectNodeCount("==", 5)
})
It("should not allow consolidation if the budget is fully blocking", func() {
Expand Down
26 changes: 14 additions & 12 deletions test/suites/drift/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ var _ = Describe("Drift", func() {
},
})
selector = labels.SelectorFromSet(dep.Spec.Selector.MatchLabels)

env.ExpectSettingsOverridden(v1.EnvVar{Name: "FEATURE_GATES", Value: "Drift=true"})
})
Context("Budgets", func() {
It("should respect budgets for empty drift", func() {
Expand Down Expand Up @@ -304,36 +302,40 @@ var _ = Describe("Drift", func() {

env.ExpectCreated(nodeClass, nodePool, deployments[0], deployments[1], deployments[2], deployments[3], deployments[4])

env.EventuallyExpectCreatedNodeClaimCount("==", 5)
nodes := env.EventuallyExpectCreatedNodeCount("==", 5)
originalNodeClaims := env.EventuallyExpectCreatedNodeClaimCount("==", 5)
originalNodes := env.EventuallyExpectCreatedNodeCount("==", 5)

// Check that all deployment pods are online
env.EventuallyExpectHealthyPodCount(selector, numPods)

By("cordoning and adding finalizer to the nodes")
// Add a finalizer to each node so that we can stop termination disruptions
for _, node := range nodes {
for _, node := range originalNodes {
Expect(env.Client.Get(env.Context, client.ObjectKeyFromObject(node), node)).To(Succeed())
node.Finalizers = append(node.Finalizers, common.TestingFinalizer)
env.ExpectUpdated(node)
}

// Check that all daemonsets and deployment pods are online
env.EventuallyExpectHealthyPodCount(selector, numPods)

By("drifting the nodepool")
nodePool.Spec.Template.Annotations = lo.Assign(nodePool.Spec.Template.Annotations, map[string]string{"test-annotation": "drift"})
env.ExpectUpdated(nodePool)

// Ensure that we get two nodes tainted, and they have overlap during the drift
// Ensure that we get three nodes tainted, and they have overlap during the drift
env.EventuallyExpectTaintedNodeCount("==", 3)
env.EventuallyExpectNodeClaimCount("==", 8)
env.EventuallyExpectNodeCount("==", 8)
nodes = env.ConsistentlyExpectDisruptionsWithNodeCount(3, 8, 5*time.Second)
env.ConsistentlyExpectDisruptionsWithNodeCount(3, 8, 5*time.Second)

for _, node := range nodes {
for _, node := range originalNodes {
Expect(env.ExpectTestingFinalizerRemoved(node)).To(Succeed())
}
env.EventuallyExpectNotFound(nodes[0], nodes[1], nodes[2])

// Eventually expect all the nodes to be rolled and completely removed
// Since this completes the disruption operation, this also ensures that we aren't leaking nodes into subsequent
// tests since nodeclaims that are actively replacing but haven't brought-up nodes yet can register nodes later
env.EventuallyExpectNotFound(lo.Map(originalNodes, func(n *v1.Node, _ int) client.Object { return n })...)
env.EventuallyExpectNotFound(lo.Map(originalNodeClaims, func(n *corev1beta1.NodeClaim, _ int) client.Object { return n })...)
env.ExpectNodeClaimCount("==", 5)
env.ExpectNodeCount("==", 5)
})
It("should not allow drift if the budget is fully blocking", func() {
Expand Down
4 changes: 1 addition & 3 deletions test/suites/expiration/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ var _ = Describe("Expiration", func() {

env.EventuallyExpectCreatedNodeClaimCount("==", 5)
nodes := env.EventuallyExpectCreatedNodeCount("==", 5)

// Check that all daemonsets and deployment pods are online
env.EventuallyExpectHealthyPodCount(selector, numPods)

Expand All @@ -396,9 +397,6 @@ var _ = Describe("Expiration", func() {
env.ExpectUpdated(node)
}

// Check that all daemonsets and deployment pods are online
env.EventuallyExpectHealthyPodCount(selector, numPods)

By("enabling expiration")
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.NillableDuration{Duration: lo.ToPtr(30 * time.Second)}
env.ExpectUpdated(nodePool)
Expand Down

0 comments on commit e4bde9a

Please sign in to comment.