MGMT-15639: Change detached annotation condition in non-converged flow

https://issues.redhat.com/browse/MGMT-15639 Day 2 workers create BMH and Machine CRs on the spoke cluster when the host starts installing. The non-converged flow initially added the detached annotation for the BMH when the host starts installing too. This causes the BMH to stop being reconciled so the BMH and Machine CRs aren't created in the spoke cluster. This change adds the detached annotation when the host reaches rebooting, joined, or failed instead of installing so that it doesn't conflict with adding the BMH/Machine to the spoke cluster.
openshift · Aug 31, 2023 · 0c6515d · 0c6515d
1 parent e9e5f18
commit 0c6515d
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 43 deletions.
diff --git a/internal/controller/controllers/bmh_agent_controller.go b/internal/controller/controllers/bmh_agent_controller.go
@@ -554,20 +554,11 @@ func (r *BMACReconciler) ensureBMHDetached(log logrus.FieldLogger, bmh *bmh_v1al
 }
 
 // The detached annotation is added if the installation of the agent associated with
-// the host has started.
+// the host has reached stages Failed, Rebooting, or Joined
 func (r *BMACReconciler) addBMHDetachedAnnotationIfAgentHasStartedInstallation(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHost, agent *aiv1beta1.Agent) reconcileResult {
-	c := conditionsv1.FindStatusCondition(agent.Status.Conditions, aiv1beta1.InstalledCondition)
-	if c == nil {
-		log.Debugf("Skipping adding detached annotation. missing install condition \n %v", agent)
-		return reconcileComplete{}
-	}
-	installConditionReason := c.Reason
-
-	// Do nothing if InstalledCondition is not in Installed, InProgress, or Failed
-	if installConditionReason != aiv1beta1.InstallationInProgressReason &&
-		installConditionReason != aiv1beta1.InstalledReason &&
-		installConditionReason != aiv1beta1.InstallationFailedReason {
-		log.Debugf("Skipping adding detached annotation. host not in proper installation condition \n %v", agent)
+	// Do nothing if host stage is not one of Failed, Rebooting, or Joined
+	if !funk.Contains([]models.HostStage{models.HostStageFailed, models.HostStageRebooting, models.HostStageJoined}, agent.Status.Progress.CurrentStage) {
+		log.Debugf("Skipping adding detached annotation. Host hasn't reached stage rebooted, joined, or failed. Current stage: %v", agent.Status.Progress.CurrentStage)
 		return reconcileComplete{}
 	}
 

diff --git a/internal/controller/controllers/bmh_agent_controller_test.go b/internal/controller/controllers/bmh_agent_controller_test.go
@@ -1331,14 +1331,8 @@ var _ = Describe("bmac reconcile", func() {
 				Expect(updatedHost.ObjectMeta.Annotations).To(HaveKey(BMH_DETACHED_ANNOTATION))
 				Expect(updatedHost.ObjectMeta.Annotations[BMH_DETACHED_ANNOTATION]).To(Equal("some-other-value"))
 			})
-			It("should set the detached annotation if agent is installed", func() {
-				agent.Status.Conditions = []conditionsv1.Condition{
-					{
-						Type:   v1beta1.InstalledCondition,
-						Status: corev1.ConditionTrue,
-						Reason: v1beta1.InstalledReason,
-					},
-				}
+			It("should set the detached annotation if agent started rebooting", func() {
+				agent.Status.Progress.CurrentStage = models.HostStageRebooting
 				Expect(c.Update(ctx, agent)).To(BeNil())
 
 				for range [3]int{} {
@@ -1354,14 +1348,9 @@ var _ = Describe("bmac reconcile", func() {
 				Expect(updatedHost.ObjectMeta.Annotations[BMH_DETACHED_ANNOTATION]).To(Equal("assisted-service-controller"))
 			})
 
-			It("should set the detached annotation if agent is installation is progressing", func() {
-				agent.Status.Conditions = []conditionsv1.Condition{
-					{
-						Type:   v1beta1.InstalledCondition,
-						Status: corev1.ConditionFalse,
-						Reason: v1beta1.InstallationInProgressReason,
-					},
-				}
+			It("should set the detached annotation if agent has joined cluster", func() {
+				agent.Status.Progress.CurrentStage = models.HostStageJoined
+
 				Expect(c.Update(ctx, agent)).To(BeNil())
 
 				for range [3]int{} {
@@ -1378,13 +1367,7 @@ var _ = Describe("bmac reconcile", func() {
 			})
 
 			It("should set the detached annotation if agent is installation has failed", func() {
-				agent.Status.Conditions = []conditionsv1.Condition{
-					{
-						Type:   v1beta1.InstalledCondition,
-						Status: corev1.ConditionFalse,
-						Reason: v1beta1.InstallationFailedReason,
-					},
-				}
+				agent.Status.Progress.CurrentStage = models.HostStageFailed
 				Expect(c.Update(ctx, agent)).To(BeNil())
 
 				for range [3]int{} {
@@ -1436,13 +1419,7 @@ var _ = Describe("bmac reconcile", func() {
 
 		Context("when BMH is detached", func() {
 			It("should not change the URL when it changes in the InfraEnv", func() {
-				agent.Status.Conditions = []conditionsv1.Condition{
-					{
-						Type:   v1beta1.InstalledCondition,
-						Status: corev1.ConditionFalse,
-						Reason: v1beta1.InstallationFailedReason,
-					},
-				}
+				agent.Status.Progress.CurrentStage = models.HostStageFailed
 				Expect(c.Update(ctx, agent)).To(BeNil())
 
 				for range [3]int{} {