Merge pull request #31 from metal-stack/fix-race-condition-in-readiness

fix: race condition in readiness #28
metal-stack · Jan 21, 2025 · 5574304 · 5574304
2 parents a3253ee + 0db4878
commit 5574304
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 8 deletions.
diff --git a/internal/controller/metalstackcluster_controller.go b/internal/controller/metalstackcluster_controller.go
@@ -732,6 +732,13 @@ func (r *clusterReconciler) status() error {
 	})
 
 	g.Go(func() error {
+		if r.infraCluster.Spec.Firewall == nil {
+			conditionUpdates <- func() {
+				conditions.MarkTrue(r.infraCluster, v1alpha1.ClusterFirewallDeploymentReady)
+			}
+			return nil
+		}
+
 		deploy := &fcmv2.FirewallDeployment{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      r.infraCluster.Name,
@@ -748,11 +755,6 @@ func (r *clusterReconciler) status() error {
 			}
 
 			if apierrors.IsNotFound(err) {
-				if r.infraCluster.Spec.Firewall == nil {
-					conditions.MarkTrue(r.infraCluster, v1alpha1.ClusterFirewallDeploymentReady)
-					return
-				}
-
 				conditions.MarkFalse(r.infraCluster, v1alpha1.ClusterFirewallDeploymentReady, "NotCreated", clusterv1.ConditionSeverityError, "firewall deployment was not yet created")
 				return
 			}
@@ -781,13 +783,14 @@ func (r *clusterReconciler) status() error {
 	}()
 
 	groupErr := g.Wait()
-	if groupErr == nil && allConditionsTrue() {
-		r.infraCluster.Status.Ready = true
-	}
 
 	close(conditionUpdates)
 	<-ready
 
+	if groupErr == nil && allConditionsTrue() {
+		r.infraCluster.Status.Ready = true
+	}
+
 	err := r.client.Status().Update(r.ctx, r.infraCluster)
 
 	return errors.Join(groupErr, err)

diff --git a/internal/controller/metalstackcluster_controller_test.go b/internal/controller/metalstackcluster_controller_test.go
@@ -273,6 +273,7 @@ var _ = Describe("MetalStackCluster Controller", func() {
 				"Type":   Equal(v1alpha1.ClusterControlPlaneEndpointEnsured),
 				"Status": Equal(corev1.ConditionTrue),
 			})))
+			Expect(resource.Status.Ready).To(BeTrue())
 
 			By("ssh keypair generation")
 			sshSecret := &corev1.Secret{
@@ -386,6 +387,8 @@ var _ = Describe("MetalStackCluster Controller", func() {
 						"Status": Equal(corev1.ConditionTrue),
 					}),
 				))
+
+				Expect(resource.Status.Ready).To(BeTrue())
 			})
 		})
 
@@ -460,6 +463,8 @@ var _ = Describe("MetalStackCluster Controller", func() {
 					"Reason":  Equal("InternalError"),
 					"Message": ContainSubstring("ip not found"),
 				})))
+
+				Expect(resource.Status.Ready).To(BeFalse())
 			})
 		})
 	})