Skip to content

Commit

Permalink
MGMT-15559: Change detached annotation condition in non-converged flow (
Browse files Browse the repository at this point in the history 
#5445) (#5507)

https://issues.redhat.com/browse/MGMT-15559
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.
  • Loading branch information
@CrystalChun
CrystalChun committed Sep 27, 2023
1 parent 024c88b commit 91d09f5
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 52 deletions.
28 changes: 6 additions & 22 deletions internal/controller/controllers/bmh_agent_controller.go
View file
Expand Up @@ -306,7 +306,7 @@ func (r *BMACReconciler) Reconcile(origCtx context.Context, req ctrl.Request) (c
} else {
// After the agent has started installation, Ironic should not manage the host.
// Adding the detached annotation to the BMH stops Ironic from managing it.
result = r.addBMHDetachedAnnotationIfAgentHasStartedInstallation(log, bmh, agent)
result = r.addBMHDetachedAnnotationIfHostIsRebooting(log, bmh, agent)
}
if result.Dirty() {
err := r.Client.Update(ctx, bmh)
Expand Down Expand Up @@ -465,27 +465,11 @@ func shouldSkipDetach(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHost, a
return false
}

// The detached annotation is added if the installation of the agent associated with
// the host has started.
func (r *BMACReconciler) addBMHDetachedAnnotationIfAgentHasStartedInstallation(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHost, agent *aiv1beta1.Agent) reconcileResult {

shouldSkip := shouldSkipDetach(log, bmh, agent)
if shouldSkip {
return reconcileComplete{}
}

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)
// the host has reached stages Failed, Rebooting, or Joined
func (r *BMACReconciler) addBMHDetachedAnnotationIfHostIsRebooting(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHost, agent *aiv1beta1.Agent) reconcileResult {
// 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{}
}

Expand Down
52 changes: 22 additions & 30 deletions internal/controller/controllers/bmh_agent_controller_test.go
View file
Expand Up @@ -1178,14 +1178,23 @@ var _ = Describe("bmac reconcile", func() {
Expect(updatedHost.ObjectMeta.Annotations).NotTo(HaveKey(BMH_DETACHED_ANNOTATION))
Expect(updatedHost.ObjectMeta.Annotations[BMH_DETACHED_ANNOTATION]).NotTo(Equal("assisted-service-controller"))
})
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 not set the detached annotation value for BMHs not labeled with an infraenv", func() {
host.SetAnnotations(map[string]string{BMH_DETACHED_ANNOTATION: "some-other-value"})
delete(host.Labels, BMH_INFRA_ENV_LABEL)
Expect(c.Update(ctx, host)).To(BeNil())

result, err := bmhr.Reconcile(ctx, newBMHRequest(host))
Expect(err).To(BeNil())
Expect(result).To(Equal(ctrl.Result{}))

updatedHost := &bmh_v1alpha1.BareMetalHost{}
err = c.Get(ctx, types.NamespacedName{Name: host.Name, Namespace: testNamespace}, updatedHost)
Expect(err).To(BeNil())
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 started rebooting", func() {
agent.Status.Progress.CurrentStage = models.HostStageRebooting
Expect(c.Update(ctx, agent)).To(BeNil())

for range [3]int{} {
Expand All @@ -1201,14 +1210,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{} {
Expand All @@ -1225,13 +1229,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{} {
Expand Down Expand Up @@ -1283,13 +1281,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{} {
Expand Down

0 comments on commit 91d09f5

Please sign in to comment.