From 9fcd5f12840d0df3b15a8faac12ddb77a220f1f7 Mon Sep 17 00:00:00 2001 From: Daniel Erez Date: Mon, 19 Aug 2024 21:27:09 +0300 Subject: [PATCH] MGMT-18636: Handle status annotation in provisioned BMHs This change is introducing a solution for reprovisioned BMHs of restored spoke clusters. The suggested logic ensures that a 'status' annotation is always available for BMHs in state 'provisioned'. By that, as suggested in BMO docs[1] for moving BMHs bwtween hub, the status is restored from the annotation. I.e. When the status is available, BMHs reprovisioning by Ironic is prevented. See full flow in the enhancement[2], 'Solution for BMHs reconcile' section. [1] BMO documentation: https://github.com/metal3-io/metal3-docs/blob/6a656b3eb195c1b09ba35fcad4d011c6cb02dbc2/docs/user-guide/src/bmo/status_annotation.md [2] Enhancement: https://github.com/openshift/assisted-service/pull/6683 --- cmd/main.go | 2 + .../controllers/bmh_agent_controller.go | 90 ++++++- .../controllers/bmh_agent_controller_test.go | 221 ++++++++++++++++++ 3 files changed, 307 insertions(+), 6 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index c0207dae73fe..e545c8aeb05d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -161,6 +161,7 @@ var Options struct { ApproveCsrsRequeueDuration time.Duration `envconfig:"APPROVE_CSRS_REQUEUE_DURATION" default:"1m"` HTTPListenPort string `envconfig:"HTTP_LISTEN_PORT" default:""` AllowConvergedFlow bool `envconfig:"ALLOW_CONVERGED_FLOW" default:"true"` + PauseProvisionedBMHs bool `envconfig:"PAUSE_PROVISIONED_BMHS" default:"true"` PreprovisioningImageControllerConfig controllers.PreprovisioningImageControllerConfig BMACConfig controllers.BMACConfig @@ -618,6 +619,7 @@ func main() { Scheme: ctrlMgr.GetScheme(), SpokeK8sClientFactory: spoke_k8s_client.NewSpokeK8sClientFactory(log), ConvergedFlowEnabled: useConvergedFlow, + PauseProvisionedBMHs: Options.PauseProvisionedBMHs, Drainer: &controllers.KubectlDrainer{}, Config: &Options.BMACConfig, }).SetupWithManager(ctrlMgr), "unable to create controller BMH") diff --git a/internal/controller/controllers/bmh_agent_controller.go b/internal/controller/controllers/bmh_agent_controller.go index 51b537737207..6893fb5f79c2 100644 --- a/internal/controller/controllers/bmh_agent_controller.go +++ b/internal/controller/controllers/bmh_agent_controller.go @@ -69,6 +69,7 @@ type BMACReconciler struct { SpokeK8sClientFactory spoke_k8s_client.SpokeK8sClientFactory spokeClient client.Client ConvergedFlowEnabled bool + PauseProvisionedBMHs bool Drainer Drainer Config *BMACConfig } @@ -83,6 +84,8 @@ const ( BMH_ANNOTATION = "metal3.io/BareMetalHost" BMH_API_VERSION = "baremetal.cluster.k8s.io/v1alpha1" BMH_DETACHED_ANNOTATION = "baremetalhost.metal3.io/detached" + BMH_PAUSED_ANNOTATION = "baremetalhost.metal3.io/paused" + BMH_STATUS_ANNOTATION = "baremetalhost.metal3.io/status" BMH_INSPECT_ANNOTATION = "inspect.metal3.io" BMH_HARDWARE_DETAILS_ANNOTATION = "inspect.metal3.io/hardwaredetails" BMH_AGENT_IGNITION_CONFIG_OVERRIDES = "bmac.agent-install.openshift.io/ignition-config-overrides" @@ -346,9 +349,9 @@ func (r *BMACReconciler) handleBMHFinalizer(ctx context.Context, log logrus.Fiel return reconcileComplete{} } - // BMH is being deleted and finalizer is gone, ensure detached is gone and return + // BMH is being deleted and finalizer is gone, ensure detached and paused are gone and return if !bmhHasFinalizer { - dirty := removeBMHDetachedAnnotation(log, bmh) + dirty := removeBMHDetachedAndPausedAnnotations(log, bmh) return reconcileComplete{stop: true, dirty: dirty} } @@ -356,7 +359,7 @@ func (r *BMACReconciler) handleBMHFinalizer(ctx context.Context, log logrus.Fiel // agent could be nil here if it wasn't created yet, or if we deleted it, then failed to remove the finalizer for some reason // if the agent is missing, just allow the BMH to be removed if agent == nil { - removeBMHDetachedAnnotation(log, bmh) + removeBMHDetachedAndPausedAnnotations(log, bmh) controllerutil.RemoveFinalizer(bmh, BMH_FINALIZER_NAME) return reconcileComplete{stop: true, dirty: true} } @@ -785,6 +788,7 @@ func reconcileUnboundAgent(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHo // in case. They will be regenerated in the reconciles following the agent's // reboot delete(bmh.ObjectMeta.Annotations, BMH_DETACHED_ANNOTATION) + delete(bmh.ObjectMeta.Annotations, BMH_PAUSED_ANNOTATION) delete(bmh.ObjectMeta.Annotations, BMH_HARDWARE_DETAILS_ANNOTATION) bmh.Spec.Image = nil @@ -810,8 +814,9 @@ func reconcileUnboundAgent(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHo // always want the host attached if it's still unbinding pending user action if _, ok := bmh.GetAnnotations()[BMH_DETACHED_ANNOTATION]; ok { - log.Info("removing BMH detached annotation") + log.Info("removing BMH detached and paused annotations") delete(bmh.Annotations, BMH_DETACHED_ANNOTATION) + delete(bmh.Annotations, BMH_PAUSED_ANNOTATION) dirty = true } if _, ok := bmh.GetAnnotations()[BMH_HARDWARE_DETAILS_ANNOTATION]; ok { @@ -917,6 +922,23 @@ func (r *BMACReconciler) reconcileBMH(ctx context.Context, log logrus.FieldLogge return reconcileComplete{stop: true} } + if r.PauseProvisionedBMHs { + // Add 'status' and 'paused' annotations for provisioned BMHs + if bmh.Status.Provisioning.State == bmh_v1alpha1.StateProvisioned { + result := r.addBMHStatusAndPausedAnnotations(log, bmh) + if res := r.handleReconcileResult(ctx, log, result, bmh); res != nil { + return res + } + } + // Remove 'paused' annotation if BMH's provisioning state is missing + if bmh.Status.Provisioning.State == bmh_v1alpha1.StateNone { + result := r.removePausedAnnotation(log, bmh) + if res := r.handleReconcileResult(ctx, log, result, bmh); res != nil { + return res + } + } + } + // A detached BMH is considered to be unmanaged by the hub // cluster and, therefore, BMAC reconciles on this BMH should // not happen. @@ -1666,17 +1688,22 @@ func (r *BMACReconciler) getClusterDeploymentAndCheckIfInstalled(ctx context.Con return clusterDeployment, true, err } -func removeBMHDetachedAnnotation(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHost) (dirty bool) { +func removeBMHDetachedAndPausedAnnotations(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHost) (dirty bool) { if _, ok := bmh.GetAnnotations()[BMH_DETACHED_ANNOTATION]; ok { log.Info("removing BMH detached annotation") delete(bmh.Annotations, BMH_DETACHED_ANNOTATION) dirty = true } + if _, ok := bmh.GetAnnotations()[BMH_PAUSED_ANNOTATION]; ok { + log.Info("removing BMH paused annotation") + delete(bmh.Annotations, BMH_PAUSED_ANNOTATION) + dirty = true + } return } func configureBMHForCleaning(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHost) (dirty bool) { - dirty = removeBMHDetachedAnnotation(log, bmh) + dirty = removeBMHDetachedAndPausedAnnotations(log, bmh) if bmh.Spec.AutomatedCleaningMode != bmh_v1alpha1.CleaningModeMetadata { log.Infof("setting BMH cleaning mode to %s", bmh_v1alpha1.CleaningModeMetadata) @@ -1832,3 +1859,54 @@ func (r *BMACReconciler) drainAgentNode(ctx context.Context, log logrus.FieldLog return false, nil } + +// Adding 'status' and 'paused' annotations to the BMH. +// This is required to ensure that the BMH is keeping its status +// (when moving the BMH to another hub). +// I.e. when the BMH is applied on another hub without we need +// the 'status' annotation to restore it by BMO. +// See BMO docs for more details: https://github.com/metal3-io/metal3-docs/blob/6a656b3eb195c1b09ba35fcad4d011c6cb02dbc2/docs/user-guide/src/bmo/status_annotation.md +func (r *BMACReconciler) addBMHStatusAndPausedAnnotations(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHost) reconcileResult { + if bmh.ObjectMeta.Annotations == nil { + bmh.ObjectMeta.Annotations = make(map[string]string) + } + + // Convert BMH status to a JSON string + statusJson, err := json.Marshal(bmh.Status) + if err != nil { + return reconcileError{err: err} + } + desiredValue := string(statusJson) + + // Add 'status' annotation if missing + dirty := false + currentValue, hasAnnotation := bmh.ObjectMeta.Annotations[BMH_STATUS_ANNOTATION] + if !hasAnnotation || currentValue != desiredValue { + bmh.ObjectMeta.Annotations[BMH_STATUS_ANNOTATION] = desiredValue + log.Info("Added status annotation to BMH") + dirty = true + } + + // Add 'paused' annotation if missing + desiredValue = "assisted-service-controller" + currentValue, hasAnnotation = bmh.ObjectMeta.Annotations[BMH_PAUSED_ANNOTATION] + if !hasAnnotation || currentValue != desiredValue { + bmh.ObjectMeta.Annotations[BMH_PAUSED_ANNOTATION] = desiredValue + log.Info("Added paused annotation to BMH") + return reconcileComplete{dirty: true} + } + + return reconcileComplete{dirty: dirty, stop: true} +} + +// Removes 'paused' annotation for letting BMO to restore BMH's status (using the 'status' annotation). +func (r *BMACReconciler) removePausedAnnotation(log logrus.FieldLogger, bmh *bmh_v1alpha1.BareMetalHost) reconcileResult { + _, hasPausedAnnotation := bmh.ObjectMeta.Annotations[BMH_PAUSED_ANNOTATION] + _, hasStatusAnnotation := bmh.ObjectMeta.Annotations[BMH_STATUS_ANNOTATION] + if hasPausedAnnotation && hasStatusAnnotation { + delete(bmh.ObjectMeta.Annotations, BMH_PAUSED_ANNOTATION) + log.Info("Removed paused annotation from BMH") + return reconcileComplete{dirty: true, stop: true} + } + return reconcileComplete{} +} diff --git a/internal/controller/controllers/bmh_agent_controller_test.go b/internal/controller/controllers/bmh_agent_controller_test.go index 218b1905e097..abe162cdbed4 100644 --- a/internal/controller/controllers/bmh_agent_controller_test.go +++ b/internal/controller/controllers/bmh_agent_controller_test.go @@ -54,6 +54,7 @@ var _ = Describe("bmac reconcile", func() { Log: common.GetTestLog(), spokeClient: fakeclient.NewClientBuilder().WithScheme(schemes).Build(), ConvergedFlowEnabled: false, + PauseProvisionedBMHs: true, } }) @@ -1363,6 +1364,7 @@ var _ = Describe("bmac reconcile", func() { Expect(spokeBMH.ObjectMeta.Annotations).To(HaveKey(BMH_HARDWARE_DETAILS_ANNOTATION)) Expect(spokeBMH.ObjectMeta.Annotations[BMH_HARDWARE_DETAILS_ANNOTATION]).To(Equal(updatedHost.ObjectMeta.Annotations[BMH_HARDWARE_DETAILS_ANNOTATION])) Expect(spokeBMH.ObjectMeta.Annotations).ToNot(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(spokeBMH.ObjectMeta.Annotations).ToNot(HaveKey(BMH_PAUSED_ANNOTATION)) Expect(spokeBMH.ObjectMeta.Annotations[BMH_INSPECT_ANNOTATION]).To(Equal("disabled")) Expect(spokeBMH.Spec.Image).To(Equal(updatedHost.Spec.Image)) Expect(spokeBMH.Spec.ConsumerRef.Kind).To(Equal("Machine")) @@ -1431,6 +1433,7 @@ var _ = Describe("bmac reconcile", func() { Expect(spokeBMH.ObjectMeta.Annotations).To(HaveKey(BMH_HARDWARE_DETAILS_ANNOTATION)) Expect(spokeBMH.ObjectMeta.Annotations[BMH_HARDWARE_DETAILS_ANNOTATION]).To(Equal(updatedHost.ObjectMeta.Annotations[BMH_HARDWARE_DETAILS_ANNOTATION])) Expect(spokeBMH.ObjectMeta.Annotations).ToNot(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(spokeBMH.ObjectMeta.Annotations).ToNot(HaveKey(BMH_PAUSED_ANNOTATION)) Expect(spokeBMH.ObjectMeta.Annotations[BMH_INSPECT_ANNOTATION]).To(Equal("disabled")) Expect(spokeBMH.Spec.Image).To(Equal(updatedHost.Spec.Image)) Expect(spokeBMH.Spec.ConsumerRef.Kind).To(Equal("Machine")) @@ -1465,6 +1468,7 @@ var _ = Describe("bmac reconcile", func() { err := c.Get(ctx, types.NamespacedName{Name: host_day2.Name, Namespace: testNamespace}, updatedHost) Expect(err).To(BeNil()) Expect(updatedHost.ObjectMeta.Annotations).ToNot(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(updatedHost.ObjectMeta.Annotations).ToNot(HaveKey(BMH_PAUSED_ANNOTATION)) By("Checking the spoke BMH does not exist") machineName := fmt.Sprintf("%s-%s", cluster.Name, host_day2.Name) @@ -1495,6 +1499,7 @@ var _ = Describe("bmac reconcile", func() { err := c.Get(ctx, types.NamespacedName{Name: host_day2.Name, Namespace: testNamespace}, updatedHost) Expect(err).To(BeNil()) Expect(updatedHost.ObjectMeta.Annotations).ToNot(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(updatedHost.ObjectMeta.Annotations).ToNot(HaveKey(BMH_PAUSED_ANNOTATION)) By("Checking the spoke BMH does not exist") machineName := fmt.Sprintf("%s-%s", cluster.Name, host_day2.Name) @@ -1533,6 +1538,7 @@ var _ = Describe("bmac reconcile", func() { Expect(err).To(BeNil()) Expect(updatedHost.ObjectMeta.Annotations).To(HaveKey(BMH_HARDWARE_DETAILS_ANNOTATION)) Expect(updatedHost.ObjectMeta.Annotations).NotTo(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(updatedHost.ObjectMeta.Annotations).NotTo(HaveKey(BMH_PAUSED_ANNOTATION)) Expect(updatedHost.ObjectMeta.Annotations).To(HaveKey(BMH_AGENT_IGNITION_CONFIG_OVERRIDES)) Expect(updatedHost.ObjectMeta.Annotations[BMH_AGENT_IGNITION_CONFIG_OVERRIDES]).NotTo(Equal("")) Expect(updatedHost.ObjectMeta.Annotations[BMH_AGENT_IGNITION_CONFIG_OVERRIDES]).To(ContainSubstring("dGVzdA==")) @@ -1586,6 +1592,7 @@ var _ = Describe("bmac reconcile", func() { Expect(spokeBMH.ObjectMeta.Annotations).To(HaveKey(BMH_HARDWARE_DETAILS_ANNOTATION)) Expect(spokeBMH.ObjectMeta.Annotations[BMH_HARDWARE_DETAILS_ANNOTATION]).To(Equal(updatedHost.ObjectMeta.Annotations[BMH_HARDWARE_DETAILS_ANNOTATION])) Expect(spokeBMH.ObjectMeta.Annotations).ToNot(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(spokeBMH.ObjectMeta.Annotations).ToNot(HaveKey(BMH_PAUSED_ANNOTATION)) Expect(spokeBMH.ObjectMeta.Annotations[BMH_INSPECT_ANNOTATION]).To(Equal("disabled")) Expect(spokeBMH.Spec.Image).To(Equal(updatedHost.Spec.Image)) Expect(spokeBMH.Spec.ConsumerRef.Kind).To(Equal("Machine")) @@ -1757,6 +1764,7 @@ var _ = Describe("bmac reconcile", func() { err = c.Get(ctx, types.NamespacedName{Name: host.Name, Namespace: testNamespace}, updatedHost) Expect(err).To(BeNil()) Expect(updatedHost.ObjectMeta.Annotations).NotTo(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(updatedHost.ObjectMeta.Annotations).NotTo(HaveKey(BMH_PAUSED_ANNOTATION)) Expect(updatedHost.ObjectMeta.Annotations[BMH_DETACHED_ANNOTATION]).NotTo(Equal("assisted-service-controller")) }) It("should not set the detached annotation value for BMHs not labeled with an infraenv", func() { @@ -1837,6 +1845,7 @@ var _ = Describe("bmac reconcile", func() { err = c.Get(ctx, types.NamespacedName{Name: host.Name, Namespace: testNamespace}, updatedHost) Expect(err).To(BeNil()) Expect(updatedHost.ObjectMeta.Annotations).ToNot(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(updatedHost.ObjectMeta.Annotations).ToNot(HaveKey(BMH_PAUSED_ANNOTATION)) }) It("should not set the detached annotation if installation has not started", func() { @@ -1857,6 +1866,7 @@ var _ = Describe("bmac reconcile", func() { err = c.Get(ctx, types.NamespacedName{Name: host.Name, Namespace: testNamespace}, updatedHost) Expect(err).To(BeNil()) Expect(updatedHost.ObjectMeta.Annotations).ToNot(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(updatedHost.ObjectMeta.Annotations).ToNot(HaveKey(BMH_PAUSED_ANNOTATION)) }) }) @@ -1922,11 +1932,213 @@ var _ = Describe("bmac reconcile", func() { err = c.Get(ctx, types.NamespacedName{Name: host.Name, Namespace: testNamespace}, updatedHost) Expect(err).To(BeNil()) Expect(updatedHost.ObjectMeta.Annotations).NotTo(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(updatedHost.ObjectMeta.Annotations).NotTo(HaveKey(BMH_PAUSED_ANNOTATION)) Expect(updatedHost.ObjectMeta.Annotations).NotTo(HaveKey(BMH_HARDWARE_DETAILS_ANNOTATION)) Expect(updatedHost.Spec.Image).To(BeNil()) }) }) }) + + Describe("Handle status annotation for BMHs", func() { + var bmh, updatedBMH *bmh_v1alpha1.BareMetalHost + var infraEnv *v1beta1.InfraEnv + + BeforeEach(func() { + // Create test InfraEnv + infraEnv = newInfraEnvImage("testInfraEnv", testNamespace, v1beta1.InfraEnvSpec{}) + Expect(c.Create(ctx, infraEnv)).To(BeNil()) + + // Initialize test BMH + bmh = newBMH("testBMH", &bmh_v1alpha1.BareMetalHostSpec{}) + labels := make(map[string]string) + labels[BMH_INFRA_ENV_LABEL] = "testInfraEnv" + bmh.ObjectMeta.Labels = labels + }) + + Context("when BMH is in provisioned state", func() { + BeforeEach(func() { + bmh.Status.Provisioning.State = bmh_v1alpha1.StateProvisioned + bmh.ObjectMeta.Annotations = make(map[string]string) + }) + + It("should add paused and status annotations if missing", func() { + // Create BMH + Expect(c.Create(ctx, bmh)).To(Succeed()) + + // Reconcile BMH + result, err := bmhr.Reconcile(ctx, newBMHRequest(bmh)) + Expect(err).To(BeNil()) + Expect(result).To(Equal(ctrl.Result{})) + + // Get updated BMH + updatedBMH = &bmh_v1alpha1.BareMetalHost{} + err = c.Get(ctx, types.NamespacedName{Name: bmh.Name, Namespace: testNamespace}, updatedBMH) + Expect(err).To(BeNil()) + + // Ensure 'paused' and 'status' annotations are missing + Expect(bmh.ObjectMeta.Annotations).To(Not(HaveKey(BMH_PAUSED_ANNOTATION))) + Expect(bmh.ObjectMeta.Annotations).To(Not(HaveKey(BMH_STATUS_ANNOTATION))) + + // Ensure 'paused' annotation is added + Expect(updatedBMH.ObjectMeta.Annotations).To(HaveKey(BMH_PAUSED_ANNOTATION)) + Expect(updatedBMH.ObjectMeta.Annotations[BMH_PAUSED_ANNOTATION]).To(Equal("assisted-service-controller")) + + // Ensure 'status' annotation is added + statusJson, err := json.Marshal(updatedBMH.Status) + Expect(err).To(BeNil()) + Expect(updatedBMH.ObjectMeta.Annotations).To(HaveKey(BMH_STATUS_ANNOTATION)) + Expect(updatedBMH.ObjectMeta.Annotations[BMH_STATUS_ANNOTATION]).To(Equal(string(statusJson))) + }) + + It("should update status annotation if stale", func() { + // Create BMH + bmh.ObjectMeta.Annotations[BMH_PAUSED_ANNOTATION] = "assisted-service-controller" + originalStatusJson, err := json.Marshal(bmh.Status) + Expect(err).To(BeNil()) + bmh.ObjectMeta.Annotations[BMH_STATUS_ANNOTATION] = string(originalStatusJson) + Expect(c.Create(ctx, bmh)).To(Succeed()) + + // Reconcile BMH + result, err := bmhr.Reconcile(ctx, newBMHRequest(bmh)) + Expect(err).To(BeNil()) + Expect(result).To(Equal(ctrl.Result{})) + + // Update BMH + err = c.Get(ctx, types.NamespacedName{Name: bmh.Name, Namespace: testNamespace}, bmh) + Expect(err).To(BeNil()) + bmh.Status.OperationalStatus = "discovered" + Expect(c.Update(ctx, bmh)).To(BeNil()) + + // Reconcile updated BMH + result, err = bmhr.Reconcile(ctx, newBMHRequest(bmh)) + Expect(err).To(BeNil()) + Expect(result).To(Equal(ctrl.Result{})) + + // Get updated BMH + updatedBMH = &bmh_v1alpha1.BareMetalHost{} + err = c.Get(ctx, types.NamespacedName{Name: bmh.Name, Namespace: testNamespace}, updatedBMH) + Expect(err).To(BeNil()) + + // Ensure 'status' annotation is updated + updatedStatusJson, err := json.Marshal(updatedBMH.Status) + Expect(err).To(BeNil()) + Expect(updatedStatusJson).To(Not(Equal(originalStatusJson))) + Expect(updatedBMH.ObjectMeta.Annotations[BMH_STATUS_ANNOTATION]).To(Equal(string(updatedStatusJson))) + }) + + It("should keep paused and status annotations if already exist", func() { + // Create BMH + bmh.ObjectMeta.Annotations[BMH_PAUSED_ANNOTATION] = "assisted-service-controller" + statusJson, err := json.Marshal(bmh.Status) + Expect(err).To(BeNil()) + bmh.ObjectMeta.Annotations[BMH_STATUS_ANNOTATION] = string(statusJson) + Expect(c.Create(ctx, bmh)).To(Succeed()) + + // Reconcile BMH + result, err := bmhr.Reconcile(ctx, newBMHRequest(bmh)) + Expect(err).To(BeNil()) + Expect(result).To(Equal(ctrl.Result{})) + + // Get updated BMH + updatedBMH = &bmh_v1alpha1.BareMetalHost{} + err = c.Get(ctx, types.NamespacedName{Name: bmh.Name, Namespace: testNamespace}, updatedBMH) + Expect(err).To(BeNil()) + + // Ensure 'paused' annotation exists + Expect(updatedBMH.ObjectMeta.Annotations).To(HaveKey(BMH_PAUSED_ANNOTATION)) + Expect(updatedBMH.ObjectMeta.Annotations[BMH_PAUSED_ANNOTATION]).To(Equal("assisted-service-controller")) + + // Ensure 'status' annotation exists + statusJson, err = json.Marshal(updatedBMH.Status) + Expect(err).To(BeNil()) + Expect(updatedBMH.ObjectMeta.Annotations).To(HaveKey(BMH_STATUS_ANNOTATION)) + Expect(updatedBMH.ObjectMeta.Annotations[BMH_STATUS_ANNOTATION]).To(Equal(string(statusJson))) + }) + + It("should keep provisioned state when adding cluster-reference annotation", func() { + // Add 'cluster-reference' annotation + clusterRef := fmt.Sprintf("%s/%s", testNamespace, "testCluster") + bmh.ObjectMeta.Annotations[BMH_CLUSTER_REFERENCE] = clusterRef + + // Add 'paused' annotation + bmh.ObjectMeta.Annotations[BMH_PAUSED_ANNOTATION] = "assisted-service-controller" + + // Add 'status' annotation + originalStatusJson, err := json.Marshal(bmh.Status) + Expect(err).To(BeNil()) + bmh.ObjectMeta.Annotations[BMH_STATUS_ANNOTATION] = string(originalStatusJson) + + // Create BMH + Expect(c.Create(ctx, bmh)).To(Succeed()) + + // Reconcile BMH + result, err := bmhr.Reconcile(ctx, newBMHRequest(bmh)) + Expect(err).To(BeNil()) + Expect(result).To(Equal(ctrl.Result{})) + + // Get updated BMH + updatedBMH = &bmh_v1alpha1.BareMetalHost{} + err = c.Get(ctx, types.NamespacedName{Name: bmh.Name, Namespace: testNamespace}, updatedBMH) + Expect(err).To(BeNil()) + + // Ensure annotations exist + Expect(updatedBMH.ObjectMeta.Annotations).To(HaveKey(BMH_CLUSTER_REFERENCE)) + Expect(updatedBMH.ObjectMeta.Annotations).To(HaveKey(BMH_PAUSED_ANNOTATION)) + Expect(updatedBMH.ObjectMeta.Annotations).To(HaveKey(BMH_STATUS_ANNOTATION)) + + // Ensure BMH is in 'provisioned' state + Expect(updatedBMH.Status.Provisioning.State).To(Equal(bmh_v1alpha1.StateProvisioned)) + }) + }) + + Context("when BMH's provisioning state is missing", func() { + BeforeEach(func() { + bmh.Status.Provisioning.State = bmh_v1alpha1.StateNone + bmh.ObjectMeta.Annotations = make(map[string]string) + }) + + It("should remove paused annotation if also status annotation exists", func() { + // Create BMH + bmh.ObjectMeta.Annotations[BMH_PAUSED_ANNOTATION] = "assisted-service-controller" + statusJson, err := json.Marshal(bmh.Status) + Expect(err).To(BeNil()) + bmh.ObjectMeta.Annotations[BMH_STATUS_ANNOTATION] = string(statusJson) + Expect(c.Create(ctx, bmh)).To(Succeed()) + + // Reconcile BMH + result, err := bmhr.Reconcile(ctx, newBMHRequest(bmh)) + Expect(err).To(BeNil()) + Expect(result).To(Equal(ctrl.Result{})) + + // Get updated BMH + updatedBMH = &bmh_v1alpha1.BareMetalHost{} + err = c.Get(ctx, types.NamespacedName{Name: bmh.Name, Namespace: testNamespace}, updatedBMH) + Expect(err).To(BeNil()) + + // Ensure 'paused' annotation is removed + Expect(updatedBMH.ObjectMeta.Annotations).To(Not(HaveKey(BMH_PAUSED_ANNOTATION))) + }) + + It("should not remove paused annotation if status annotation doesn't exist", func() { + // Create BMH + bmh.ObjectMeta.Annotations[BMH_PAUSED_ANNOTATION] = "assisted-service-controller" + Expect(c.Create(ctx, bmh)).To(Succeed()) + + // Reconcile BMH + result, err := bmhr.Reconcile(ctx, newBMHRequest(bmh)) + Expect(err).To(BeNil()) + Expect(result).To(Equal(ctrl.Result{})) + + // Get updated BMH + updatedBMH = &bmh_v1alpha1.BareMetalHost{} + err = c.Get(ctx, types.NamespacedName{Name: bmh.Name, Namespace: testNamespace}, updatedBMH) + Expect(err).To(BeNil()) + + // Ensure 'paused' annotation exists + Expect(updatedBMH.ObjectMeta.Annotations).To(HaveKey(BMH_PAUSED_ANNOTATION)) + }) + }) + }) }) var _ = Describe("bmac reconcile - converged flow enabled", func() { @@ -1948,6 +2160,7 @@ var _ = Describe("bmac reconcile - converged flow enabled", func() { Log: common.GetTestLog(), spokeClient: fakeclient.NewClientBuilder().WithScheme(schemes).Build(), ConvergedFlowEnabled: true, + PauseProvisionedBMHs: true, } }) @@ -2140,6 +2353,7 @@ var _ = Describe("bmac reconcile - converged flow enabled", func() { updatedHost := &bmh_v1alpha1.BareMetalHost{} Expect(c.Get(ctx, types.NamespacedName{Name: "bmh-reconcile", Namespace: testNamespace}, updatedHost)).To(BeNil()) Expect(updatedHost.ObjectMeta.Annotations).ToNot(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(updatedHost.ObjectMeta.Annotations).ToNot(HaveKey(BMH_PAUSED_ANNOTATION)) Expect(updatedHost.Spec.CustomDeploy).To(BeNil()) }) It("resets customDeploy when the BMH is available", func() { @@ -2272,6 +2486,7 @@ var _ = Describe("handleBMHFinalizer", func() { Scheme: scheme.Scheme, Log: common.GetTestLog(), ConvergedFlowEnabled: true, + PauseProvisionedBMHs: true, Drainer: mockDrainer, SpokeK8sClientFactory: mockClientFactory, } @@ -2331,11 +2546,13 @@ var _ = Describe("handleBMHFinalizer", func() { data, err := json.Marshal(args) Expect(err).To(BeNil()) setAnnotation(&bmh.ObjectMeta, BMH_DETACHED_ANNOTATION, string(data)) + setAnnotation(&bmh.ObjectMeta, BMH_PAUSED_ANNOTATION, "assisted-service-controller") res := bmhr.handleBMHFinalizer(ctx, bmhr.Log, bmh, nil) Expect(res.Dirty()).To(BeTrue()) Expect(bmh.GetFinalizers()).NotTo(ContainElement(BMH_FINALIZER_NAME)) Expect(bmh.GetAnnotations()).NotTo(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(bmh.GetAnnotations()).NotTo(HaveKey(BMH_PAUSED_ANNOTATION)) _, err = res.Result() Expect(err).NotTo(HaveOccurred()) }) @@ -2347,11 +2564,13 @@ var _ = Describe("handleBMHFinalizer", func() { data, err := json.Marshal(args) Expect(err).To(BeNil()) setAnnotation(&bmh.ObjectMeta, BMH_DETACHED_ANNOTATION, string(data)) + setAnnotation(&bmh.ObjectMeta, BMH_PAUSED_ANNOTATION, "assisted-service-controller") bmh.ObjectMeta.Finalizers = nil res := bmhr.handleBMHFinalizer(ctx, bmhr.Log, bmh, nil) Expect(res.Dirty()).To(BeTrue()) Expect(bmh.GetAnnotations()).NotTo(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(bmh.GetAnnotations()).NotTo(HaveKey(BMH_PAUSED_ANNOTATION)) _, err = res.Result() Expect(err).NotTo(HaveOccurred()) }) @@ -2499,12 +2718,14 @@ var _ = Describe("handleBMHFinalizer", func() { It("sets the BMH to clean and deprovisions", func() { setAnnotation(&bmh.ObjectMeta, BMH_DETACHED_ANNOTATION, "assisted-service-controller") + setAnnotation(&bmh.ObjectMeta, BMH_PAUSED_ANNOTATION, "assisted-service-controller") bmh.Spec.AutomatedCleaningMode = bmh_v1alpha1.CleaningModeDisabled bmh.Spec.CustomDeploy = &bmh_v1alpha1.CustomDeploy{Method: ASSISTED_DEPLOY_METHOD} res := bmhr.handleBMHFinalizer(ctx, bmhr.Log, bmh, agent) Expect(res.Dirty()).To(BeTrue()) Expect(bmh.GetAnnotations()).NotTo(HaveKey(BMH_DETACHED_ANNOTATION)) + Expect(bmh.GetAnnotations()).NotTo(HaveKey(BMH_PAUSED_ANNOTATION)) Expect(bmh.Spec.AutomatedCleaningMode).To(Equal(bmh_v1alpha1.CleaningModeMetadata)) _, err := res.Result() Expect(err).NotTo(HaveOccurred())