Merge pull request #2754 from dtantsur/test-hwdata

✨ Allow recovering hardware details from existing HardwareData

Author: metal3-io-bot

internal/controller/metal3.io/baremetalhost_controller.go

@@ -274,17 +274,31 @@ func (r *BareMetalHostReconciler) Reconcile(ctx context.Context, request ctrl.Re
 func (r *BareMetalHostReconciler) updateHardwareDetails(ctx context.Context, request ctrl.Request, host *metal3api.BareMetalHost) (bool, error) {
 	updated := false
 	if host.Status.HardwareDetails == nil || host.InspectionDisabled() {
-		objHardwareDetails, err := r.getHardwareDetailsFromAnnotation(host)
-		if err != nil {
-			return updated, fmt.Errorf("error parsing HardwareDetails from annotation: %w", err)
+		hardwareData := &metal3api.HardwareData{}
+		hardwareDataKey := client.ObjectKey{
+			Name:      host.Name,
+			Namespace: host.Namespace,
+		}
+		err := r.Client.Get(ctx, hardwareDataKey, hardwareData)
+		if err != nil && !k8serrors.IsNotFound(err) {
+			return updated, fmt.Errorf("error loading HardwareData: %w", err)
 		}
-		if objHardwareDetails != nil {
+		objHardwareDetails := hardwareData.Spec.HardwareDetails
+
+		if objHardwareDetails == nil {
+			objHardwareDetails, err = r.getHardwareDetailsFromAnnotation(host)
+			if err != nil {
+				return updated, fmt.Errorf("error parsing HardwareDetails from annotation: %w", err)
+			}
+		}
+
+		if objHardwareDetails != nil && !reflect.DeepEqual(host.Status.HardwareDetails, objHardwareDetails) {
 			host.Status.HardwareDetails = objHardwareDetails
 			err = r.saveHostStatus(ctx, host)
 			if err != nil {
-				return updated, fmt.Errorf("could not update hardwaredetails from annotation: %w", err)
+				return updated, fmt.Errorf("could not update hardwaredetails from existing hardware data or annotation: %w", err)
 			}
-			r.publishEvent(ctx, request, host.NewEvent("UpdateHardwareDetails", "Set HardwareDetails from annotation"))
+			r.publishEvent(ctx, request, host.NewEvent("UpdateHardwareDetails", "Set HardwareDetails from hardware data or annotation"))
 			updated = true
 		}
 	}

internal/controller/metal3.io/baremetalhost_controller_test.go

@@ -274,6 +274,36 @@ func TestHardwareDetails_EmptyStatus(t *testing.T) {
 	)
 }
 
+// TestHardwareDetails_FromHardwareData ensures that hardware details in
+// the status field are populated when the HardwareData object
+// is present and no existing HarwareDetails are present.
+func TestHardwareDetails_FromHardwareData(t *testing.T) {
+	host := newDefaultHost(t)
+	hwdata := &metal3api.HardwareData{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      host.Name,
+			Namespace: host.Namespace,
+		},
+		Spec: metal3api.HardwareDataSpec{
+			HardwareDetails: &metal3api.HardwareDetails{},
+		},
+	}
+	err := json.Unmarshal([]byte(hwdAnnotation), hwdata.Spec.HardwareDetails)
+	require.NoError(t, err)
+
+	r := newTestReconciler(t, host, hwdata)
+
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			_, found := host.Annotations[metal3api.HardwareDetailsAnnotation]
+			if host.Status.HardwareDetails != nil && host.Status.HardwareDetails.Hostname == "hwdAnnotation-0" && !found {
+				return true
+			}
+			return false
+		},
+	)
+}
+
 // TestHardwareDetails_StatusPresent ensures that hardware details in
 // the hardwaredetails annotation is ignored with existing Status.
 func TestHardwareDetails_StatusPresent(t *testing.T) {
@@ -328,6 +358,43 @@ func TestHardwareDetails_StatusPresentInspectDisabled(t *testing.T) {
 	)
 }
 
+// TestHardwareDetails_StatusPresentInspectDisabled ensures that
+// hardware details in the HardwareData object are consumed
+// even when existing status exists, when inspection is disabled.
+func TestHardwareDetails_HardwareDataStatusPresentInspectDisabled(t *testing.T) {
+	host := newDefaultHost(t)
+	host.Spec.InspectionMode = metal3api.InspectionModeDisabled
+	time := metav1.Now()
+	host.Status.LastUpdated = &time
+	hwd := metal3api.HardwareDetails{}
+	hwd.Hostname = "existinghost"
+	host.Status.HardwareDetails = &hwd
+
+	hwdata := &metal3api.HardwareData{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      host.Name,
+			Namespace: host.Namespace,
+		},
+		Spec: metal3api.HardwareDataSpec{
+			HardwareDetails: &metal3api.HardwareDetails{},
+		},
+	}
+	err := json.Unmarshal([]byte(hwdAnnotation), hwdata.Spec.HardwareDetails)
+	require.NoError(t, err)
+
+	r := newTestReconciler(t, host, hwdata)
+
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			_, found := host.Annotations[metal3api.HardwareDetailsAnnotation]
+			if host.Status.HardwareDetails != nil && host.Status.HardwareDetails.Hostname == "hwdAnnotation-0" && !found {
+				return true
+			}
+			return false
+		},
+	)
+}
+
 // TestHardwareDetails_Invalid
 // Tests scenario where the hardwaredetails value is invalid.
 func TestHardwareDetails_Invalid(t *testing.T) {

test/e2e/external_inspection_test.go

@@ -249,6 +249,79 @@ var _ = Describe("External Inspection", Label("required", "external-inspection")
 		}, e2eConfig.GetIntervals(specName, "wait-bmh-deleted")...)
 	})
 
+	It("should skip inspection and become available when HardwareData exists and BMH has inspection disabled", func() {
+		By("Creating a secret with BMH credentials")
+		bmcCredentialsData := map[string]string{
+			"username": bmc.User,
+			"password": bmc.Password,
+		}
+		CreateSecret(ctx, clusterProxy.GetClient(), namespace.Name, secretName, bmcCredentialsData)
+
+		By("pre-creating a hardware data")
+		hwdata := metal3api.HardwareData{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      specName + "-inspect",
+				Namespace: namespace.Name,
+			},
+			Spec: metal3api.HardwareDataSpec{
+				HardwareDetails: &metal3api.HardwareDetails{},
+			},
+		}
+		err := json.Unmarshal([]byte(hardwareDetails), hwdata.Spec.HardwareDetails)
+		Expect(err).NotTo(HaveOccurred())
+		err = clusterProxy.GetClient().Create(ctx, &hwdata)
+		Expect(err).NotTo(HaveOccurred())
+
+		By("creating a BMH with inspection disabled and hardware details added")
+		bmh := metal3api.BareMetalHost{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      specName + "-inspect",
+				Namespace: namespace.Name,
+			},
+			Spec: metal3api.BareMetalHostSpec{
+				BMC: metal3api.BMCDetails{
+					Address:                        bmc.Address,
+					CredentialsName:                "bmc-credentials",
+					DisableCertificateVerification: bmc.DisableCertificateVerification,
+				},
+				BootMode:       metal3api.Legacy,
+				BootMACAddress: bmc.BootMacAddress,
+				InspectionMode: metal3api.InspectionModeDisabled,
+			},
+		}
+		err = clusterProxy.GetClient().Create(ctx, &bmh)
+		Expect(err).NotTo(HaveOccurred())
+
+		By("waiting for the BMH to become available")
+		WaitForBmhInProvisioningState(ctx, WaitForBmhInProvisioningStateInput{
+			Client:          clusterProxy.GetClient(),
+			Bmh:             bmh,
+			State:           metal3api.StateAvailable,
+			UndesiredStates: []metal3api.ProvisioningState{metal3api.StateInspecting},
+		}, e2eConfig.GetIntervals(specName, "wait-available")...)
+
+		By("checking that the BMH was not inspected")
+		key := types.NamespacedName{Namespace: bmh.Namespace, Name: bmh.Name}
+		Expect(clusterProxy.GetClient().Get(ctx, key, &bmh)).To(Succeed())
+		Expect(bmh.Status.OperationHistory.Inspect.Start.IsZero()).To(BeTrue())
+
+		By("checking that the hardware details match what was in hardware data")
+		hwStatusJSON, err := json.Marshal(bmh.Status.HardwareDetails)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(hwStatusJSON).To(MatchJSON(hardwareDetails))
+
+		By("Delete BMH")
+		err = clusterProxy.GetClient().Delete(ctx, &bmh)
+		Expect(err).NotTo(HaveOccurred())
+
+		By("Waiting for the BMH to be deleted")
+		WaitForBmhDeleted(ctx, WaitForBmhDeletedInput{
+			Client:    clusterProxy.GetClient(),
+			BmhName:   bmh.Name,
+			Namespace: bmh.Namespace,
+		}, e2eConfig.GetIntervals(specName, "wait-bmh-deleted")...)
+	})
+
 	AfterEach(func() {
 		DumpResources(ctx, e2eConfig, clusterProxy, path.Join(artifactFolder, specName))
 		if !skipCleanup {