diff --git a/pkg/services/baremetal/host/host.go b/pkg/services/baremetal/host/host.go index 1f47b7ed5..29e8c5e7a 100644 --- a/pkg/services/baremetal/host/host.go +++ b/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( diff --git a/pkg/services/baremetal/host/host_suite_test.go b/pkg/services/baremetal/host/host_suite_test.go index dff2c84f2..ec772d5c0 100644 --- a/pkg/services/baremetal/host/host_suite_test.go +++ b/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" diff --git a/pkg/services/baremetal/host/host_test.go b/pkg/services/baremetal/host/host_test.go index aa13ad769..8ddc28f03 100644 --- a/pkg/services/baremetal/host/host_test.go +++ b/pkg/services/baremetal/host/host_test.go @@ -1642,6 +1642,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