Skip to content

Commit f7a3822

Browse files
committed
Fix two flaky tests that were failing intermittently due to race conditions
1. 'Failed deployment followed by completed deployment' test: - The test was creating both deployments simultaneously, causing a race where the test would try to update the status of a job that hadn't been created yet - Fixed by ensuring sequential execution: wait for the first deployment's job to fail before creating the second deployment 2. 'should not reconcile after failure' test: - The test was manually setting deployment status to 'backoff limit exceeded' without waiting for the controller to finish its initial reconciliation - Fixed by waiting for ObservedGeneration to match Generation before manually setting the failure status, ensuring the controller has completed processing Assisted-by: Claude-4.5-opus Signed-off-by: rabi <[email protected]>
1 parent 00c6858 commit f7a3822

File tree

2 files changed

+36
-15
lines changed

2 files changed

+36
-15
lines changed

test/functional/dataplane/openstackdataplanedeployment_controller_test.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1828,15 +1828,19 @@ var _ = Describe("Dataplane Deployment Test", func() {
18281828
})
18291829

18301830
It("should not reconcile after failure", func() {
1831-
// Directly set the deployment status to backoff limit exceeded
1832-
// This simulates a deployment that has failed after retries
1831+
// Wait for the controller to complete initial reconciliation and reach a stable state
1832+
// by waiting for the deployment to have conditions set and be in a known state
18331833
Eventually(func(g Gomega) {
18341834
deployment := GetDataplaneDeployment(dataplaneDeploymentName)
1835+
g.Expect(deployment.Status.Conditions).ToNot(BeEmpty())
1836+
// Wait for ObservedGeneration to be set, indicating controller has processed the resource
1837+
g.Expect(deployment.Status.ObservedGeneration).To(Equal(deployment.Generation))
1838+
}, th.Timeout, th.Interval).Should(Succeed())
18351839

1836-
// Initialize conditions if not present
1837-
if deployment.Status.Conditions == nil {
1838-
deployment.Status.Conditions = condition.Conditions{}
1839-
}
1840+
// Now manually set the deployment status to backoff limit exceeded
1841+
// This simulates a deployment that has failed after retries
1842+
Eventually(func(g Gomega) {
1843+
deployment := GetDataplaneDeployment(dataplaneDeploymentName)
18401844

18411845
// Initialize NodeSetConditions if not present
18421846
if deployment.Status.NodeSetConditions == nil {
@@ -1852,11 +1856,19 @@ var _ = Describe("Dataplane Deployment Test", func() {
18521856
"Simulated backoff limit exceeded for testing")
18531857

18541858
deployment.Status.Deployed = false
1855-
deployment.Status.ObservedGeneration = deployment.Generation
18561859

18571860
g.Expect(th.K8sClient.Status().Update(th.Ctx, deployment)).To(Succeed())
18581861
}, th.Timeout, th.Interval).Should(Succeed())
18591862

1863+
// Wait for the status to stabilize - ensure the controller recognizes the failure state
1864+
Eventually(func(g Gomega) {
1865+
deployment := GetDataplaneDeployment(dataplaneDeploymentName)
1866+
deploymentCondition := deployment.Status.Conditions.Get(condition.DeploymentReadyCondition)
1867+
g.Expect(deploymentCondition).ToNot(BeNil())
1868+
g.Expect(string(deploymentCondition.Severity)).To(Equal(string(condition.SeverityError)))
1869+
g.Expect(string(deploymentCondition.Reason)).To(Equal(string(condition.JobReasonBackoffLimitExceeded)))
1870+
}, th.Timeout, th.Interval).Should(Succeed())
1871+
18601872
// Verify the deployment has the correct failure condition
18611873
deployment := GetDataplaneDeployment(dataplaneDeploymentName)
18621874
deploymentCondition := deployment.Status.Conditions.Get(condition.DeploymentReadyCondition)

test/functional/dataplane/openstackdataplanenodeset_controller_test.go

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,16 +1734,11 @@ var _ = Describe("Dataplane NodeSet Test", func() {
17341734
DeferCleanup(th.DeleteInstance, CreateDNSMasq(dnsMasqName, DefaultDNSMasqSpec()))
17351735
DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(testNodeSetName, nodeSetSpec))
17361736

1737-
// Create first deployment (will fail)
1737+
// Create only the first deployment here (will fail)
17381738
firstDeploymentSpec := DefaultDataPlaneDeploymentSpec()
17391739
firstDeploymentSpec["nodeSets"] = []string{testNodeSetName.Name}
17401740
DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(dataplaneDeploymentName, firstDeploymentSpec))
17411741

1742-
// Create second deployment (will complete successfully)
1743-
secondDeploymentSpec := DefaultDataPlaneDeploymentSpec()
1744-
secondDeploymentSpec["nodeSets"] = []string{testNodeSetName.Name}
1745-
DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(secondDeploymentName, secondDeploymentSpec))
1746-
17471742
CreateSSHSecret(dataplaneSSHSecretName)
17481743
CreateCABundleSecret(caBundleSecretName)
17491744
SimulateDNSMasqComplete(dnsMasqName)
@@ -1752,7 +1747,7 @@ var _ = Describe("Dataplane NodeSet Test", func() {
17521747
})
17531748

17541749
It("Should show all deployments in status and process completed deployment", func() {
1755-
// Fail the first deployment
1750+
// Wait for first AnsibleEE job to be created, then fail it
17561751
Eventually(func(g Gomega) {
17571752
ansibleeeName := types.NamespacedName{
17581753
Name: "bootstrap-" + dataplaneDeploymentName.Name + "-" + testNodeSetName.Name,
@@ -1763,7 +1758,21 @@ var _ = Describe("Dataplane NodeSet Test", func() {
17631758
g.Expect(th.K8sClient.Status().Update(th.Ctx, ansibleEE)).To(Succeed())
17641759
}, th.Timeout, th.Interval).Should(Succeed())
17651760

1766-
// Complete the second deployment
1761+
// Wait for the first deployment to be marked as failed
1762+
Eventually(func(g Gomega) {
1763+
deployment := GetDataplaneDeployment(dataplaneDeploymentName)
1764+
g.Expect(deployment.Status.Conditions).ToNot(BeEmpty())
1765+
readyCondition := deployment.Status.Conditions.Get(condition.ReadyCondition)
1766+
g.Expect(readyCondition).ToNot(BeNil())
1767+
g.Expect(readyCondition.Status).To(Equal(corev1.ConditionFalse))
1768+
}, th.Timeout, th.Interval).Should(Succeed())
1769+
1770+
// Now create the second deployment (will complete successfully)
1771+
secondDeploymentSpec := DefaultDataPlaneDeploymentSpec()
1772+
secondDeploymentSpec["nodeSets"] = []string{testNodeSetName.Name}
1773+
DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(secondDeploymentName, secondDeploymentSpec))
1774+
1775+
// Wait for second AnsibleEE job to be created, then complete it
17671776
Eventually(func(g Gomega) {
17681777
ansibleeeName := types.NamespacedName{
17691778
Name: "bootstrap-" + secondDeploymentName.Name + "-" + testNodeSetName.Name,

0 commit comments

Comments
 (0)