🌱 re-read hardwareDetails

pkg/services/baremetal/host/host.go

@@ -29,6 +29,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/stoewer/go-strcase"
 	"github.com/syself/hrobot-go/models"
 	"golang.org/x/crypto/ssh"
@@ -698,13 +699,22 @@ func (s *Service) actionRegistering(ctx context.Context) actionResult {
 	}
 	record.Eventf(s.scope.HetznerBareMetalHost, "GetHardwareDetails", msg)
 
-	if s.scope.HetznerBareMetalHost.Spec.Status.HardwareDetails == nil {
-		hardwareDetails, err := getHardwareDetails(ctx, sshClient)
-		if err != nil {
-			return actionError{err: fmt.Errorf("failed to get hardware details: %w", err)}
+	hardwareDetails, err := getHardwareDetails(ctx, sshClient)
+	if err != nil {
+		return actionError{err: fmt.Errorf("failed to get hardware details: %w", err)}
+	}
+
+	if s.scope.HetznerBareMetalHost.Spec.Status.HardwareDetails != nil {
+		diff := cmp.Diff(*s.scope.HetznerBareMetalHost.Spec.Status.HardwareDetails, hardwareDetails)
+		if diff != "" {
+			s.scope.Info("HardwareDetails changed", "diff", diff)
+			record.Eventf(s.scope.HetznerBareMetalHost, "HardwareDetails changed", diff)
 		}
-		s.scope.HetznerBareMetalHost.Spec.Status.HardwareDetails = &hardwareDetails
 	}
+	// In case of a change in the disks, the WWNs got updated. This might lead to an outdated
+	// RootDeviceHints, which the user sets to decide which disk to use for provisioning, even if
+	// the server was already provisioned before. This will get caught by validateRootDeviceHints below.
+	s.scope.HetznerBareMetalHost.Spec.Status.HardwareDetails = &hardwareDetails
 
 	if s.scope.HetznerBareMetalHost.Spec.RootDeviceHints == nil {
 		v1beta1conditions.MarkFalse(

pkg/services/baremetal/host/host_suite_test.go

@@ -27,8 +27,10 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	clientgorecord "k8s.io/client-go/tools/record"
 	"k8s.io/klog/v2/textlogger"
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
+	"sigs.k8s.io/cluster-api/util/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 
@@ -40,6 +42,13 @@ import (
 	"github.com/syself/cluster-api-provider-hetzner/test/helpers"
 )
 
+var testEventRecorder *clientgorecord.FakeRecorder
+
+var _ = BeforeSuite(func() {
+	testEventRecorder = clientgorecord.NewFakeRecorder(100)
+	record.InitFromRecorder(testEventRecorder)
+})
+
 const (
 	sshFingerprint   = "my-fingerprint"
 	osSSHKeyName     = "os-sshkey"

pkg/services/baremetal/host/host_test.go

@@ -1665,6 +1665,80 @@ var _ = Describe("actionRegistering check RAID", func() {
 	})
 })
 
+var _ = Describe("actionRegistering emits event on hardwareDetails change", func() {
+	const storageStdOut = `NAME="nvme2n1" LABEL="" FSTYPE="" TYPE="disk" HCTL="" MODEL="SAMSUNG MZVL22T0HBLB-00B00" VENDOR="" SERIAL="S677NF0R402742" SIZE="2048408248320" WWN="eui.002538b411b2cee8" ROTA="0"
+NAME="nvme1n1" LABEL="" FSTYPE="" TYPE="disk" HCTL="" MODEL="SAMSUNG MZVLB512HAJQ-00000" VENDOR="" SERIAL="S3W8NX0N811178" SIZE="512110190592" WWN="eui.0025388801b4dff2" ROTA="0"`
+
+	ctx := context.Background()
+
+	It("emits HardwareDetails Changed event when existing details differ", func() {
+		// drain events from previous tests
+		for len(testEventRecorder.Events) > 0 {
+			<-testEventRecorder.Events
+		}
+
+		host := helpers.BareMetalHost(
+			"test-host",
+			"default",
+			helpers.WithRootDeviceHintWWN(),
+			helpers.WithIPv4(),
+			helpers.WithConsumerRef(),
+		)
+		host.Spec.Status.InstallImage = &infrav1.InstallImage{}
+		host.Spec.Status.HardwareDetails = &infrav1.HardwareDetails{
+			CPU: infrav1.CPU{Model: "old-model"},
+		}
+
+		sshMock := registeringSSHMock(storageStdOut)
+		service := newTestService(host, nil, bmmock.NewSSHFactory(sshMock, sshMock, sshMock), nil, helpers.GetDefaultSSHSecret(rescueSSHKeyName, "default"))
+
+		service.actionRegistering(ctx)
+
+		var events []string
+		for len(testEventRecorder.Events) > 0 {
+			events = append(events, <-testEventRecorder.Events)
+		}
+		Expect(events).To(ContainElement(ContainSubstring("HardwareDetails Changed")))
+	})
+
+	It("invalidates RootDeviceHints in cases where a hardware change leads to different wwns", func() {
+		host := helpers.BareMetalHost(
+			"test-host",
+			"default",
+			helpers.WithRootDeviceHintWWN(),
+			helpers.WithIPv4(),
+			helpers.WithConsumerRef(),
+		)
+		host.Spec.Status.InstallImage = &infrav1.InstallImage{}
+		// The previously read hardware had a disk matching the configured root device hint WWN.
+		host.Spec.Status.HardwareDetails = &infrav1.HardwareDetails{
+			Storage: []infrav1.Storage{
+				{WWN: helpers.DefaultWWN},
+			},
+		}
+
+		// The disk with the WWN referenced by RootDeviceHints is gone, e.g. because it was replaced.
+		const newStorageStdOut = `NAME="nvme2n1" LABEL="" FSTYPE="" TYPE="disk" HCTL="" MODEL="SAMSUNG MZVL22T0HBLB-00B00" VENDOR="" SERIAL="S677NF0R402742" SIZE="2048408248320" WWN="eui.002538b411b2cee2" ROTA="0"
+NAME="nvme1n1" LABEL="" FSTYPE="" TYPE="disk" HCTL="" MODEL="SAMSUNG MZVLB512HAJQ-00000" VENDOR="" SERIAL="S3W8NX0N811178" SIZE="512110190592" WWN="eui.0025388801b4dff2" ROTA="0"`
+
+		sshMock := registeringSSHMock(newStorageStdOut)
+		service := newTestService(host, nil, bmmock.NewSSHFactory(sshMock, sshMock, sshMock), nil, helpers.GetDefaultSSHSecret(rescueSSHKeyName, "default"))
+
+		actResult := service.actionRegistering(ctx)
+
+		Expect(actResult).To(BeAssignableToTypeOf(actionFailed{}))
+		Expect(host.Spec.Status.ErrorMessage).To(ContainSubstring("missing storage device for root device hint"))
+
+		// Even though the action failed, the freshly read hardware details must be persisted,
+		// so that the controller can still update the object (e.g. surface the new storage layout).
+		Expect(host.Spec.Status.HardwareDetails).ToNot(BeNil())
+		Expect(host.Spec.Status.HardwareDetails.Storage).To(ConsistOf(
+			infrav1.Storage{Model: "SAMSUNG MZVL22T0HBLB-00B00", SerialNumber: "S677NF0R402742", SizeBytes: 2048408248320, SizeGB: 2048, WWN: "eui.002538b411b2cee2"},
+			infrav1.Storage{Model: "SAMSUNG MZVLB512HAJQ-00000", SerialNumber: "S3W8NX0N811178", SizeBytes: 512110190592, SizeGB: 512, WWN: "eui.0025388801b4dff2"},
+		))
+	})
+})
+
 var _ = Describe("getImageDetails", func() {
 	type testCaseGetImageDetails struct {
 		image                 infrav1.Image