From ee75070845baadce43977b2bbbc917a4789a0fcf Mon Sep 17 00:00:00 2001 From: janiskemper Date: Thu, 14 Sep 2023 15:17:30 +0200 Subject: [PATCH] :seedling: Refactor unit tests Refactoring unit tests - mostly improving table-driven test inputs --- pkg/services/baremetal/host/host_test.go | 1637 ++++++++--------- .../baremetal/host/state_machine_test.go | 196 +- pkg/services/baremetal/host/utils_test.go | 92 +- pkg/services/hcloud/server/server_test.go | 161 +- pkg/services/hcloud/util/utils_test.go | 2 + pkg/utils/utils_test.go | 207 ++- 6 files changed, 1198 insertions(+), 1097 deletions(-) diff --git a/pkg/services/baremetal/host/host_test.go b/pkg/services/baremetal/host/host_test.go index a98af96da..14bab7cfa 100644 --- a/pkg/services/baremetal/host/host_test.go +++ b/pkg/services/baremetal/host/host_test.go @@ -38,17 +38,19 @@ import ( var errTest = fmt.Errorf("test error") var _ = Describe("SetErrorMessage", func() { + type testCaseSetErrorMessage struct { + errorType infrav1.ErrorType + errorMessage string + hasErrorInStatus bool + expectedErrorCount int + expectedErrorType infrav1.ErrorType + expectedErrorMessage string + } + DescribeTable("SetErrorMessage", - func( - errorType infrav1.ErrorType, - errorMessage string, - hasErrorInStatus bool, - expectedErrorCount int, - expectedErrorType infrav1.ErrorType, - expectedErrorMessage string, - ) { + func(tc testCaseSetErrorMessage) { var host *infrav1.HetznerBareMetalHost - if hasErrorInStatus { + if tc.hasErrorInStatus { host = helpers.BareMetalHost( "test-host", "default", @@ -61,54 +63,54 @@ var _ = Describe("SetErrorMessage", func() { ) } - host.SetError(errorType, errorMessage) - Expect(host.Spec.Status.ErrorCount).To(Equal(expectedErrorCount)) - Expect(host.Spec.Status.ErrorMessage).To(Equal(expectedErrorMessage)) - Expect(host.Spec.Status.ErrorType).To(Equal(expectedErrorType)) + host.SetError(tc.errorType, tc.errorMessage) + Expect(host.Spec.Status.ErrorCount).To(Equal(tc.expectedErrorCount)) + Expect(host.Spec.Status.ErrorMessage).To(Equal(tc.expectedErrorMessage)) + Expect(host.Spec.Status.ErrorType).To(Equal(tc.expectedErrorType)) }, - Entry( - "new error with existing one", - infrav1.RegistrationError, // errorType infrav1.ErrorType - "new message", // errorMessage string - true, // hasErrorInStatus bool - 1, // expectedErrorCount int - infrav1.RegistrationError, // expectedErrorType - "new message", // expectedErrorMessage - ), - Entry( - "old error with existing one", - infrav1.PreparationError, // errorType infrav1.ErrorType - "first message", // errorMessage string - true, // hasErrorInStatus bool - 3, // expectedErrorCount int - infrav1.PreparationError, // expectedErrorType - "first message", // expectedErrorMessage - ), - Entry( - "new error without existing one", - infrav1.RegistrationError, // errorType infrav1.ErrorType - "new message", // errorMessage string - true, // hasErrorInStatus bool - 1, // expectedErrorCount int - infrav1.RegistrationError, // expectedErrorType - "new message", // expectedErrorMessage - ), + Entry("new error with existing one", testCaseSetErrorMessage{ + errorType: infrav1.RegistrationError, // errorType infrav1.ErrorType + errorMessage: "new message", // errorMessage string + hasErrorInStatus: true, // hasErrorInStatus bool + expectedErrorCount: 1, // expectedErrorCount int + expectedErrorType: infrav1.RegistrationError, // expectedErrorType + expectedErrorMessage: "new message", // expectedErrorMessage + }), + Entry("old error with existing one", testCaseSetErrorMessage{ + errorType: infrav1.PreparationError, // errorType infrav1.ErrorType + errorMessage: "first message", // errorMessage string + hasErrorInStatus: true, // hasErrorInStatus bool + expectedErrorCount: 3, // expectedErrorCount int + expectedErrorType: infrav1.PreparationError, // expectedErrorType + expectedErrorMessage: "first message", // expectedErrorMessage + }), + Entry("new error without existing one", testCaseSetErrorMessage{ + errorType: infrav1.RegistrationError, // errorType infrav1.ErrorType + errorMessage: "new message", // errorMessage string + hasErrorInStatus: true, // hasErrorInStatus bool + expectedErrorCount: 1, // expectedErrorCount int + expectedErrorType: infrav1.RegistrationError, // expectedErrorType + expectedErrorMessage: "new message", // expectedErrorMessage + }), ) }) var _ = Describe("obtainHardwareDetailsNics", func() { + type testCaseObtainHardwareDetailsNics struct { + stdout string + expectedOutput []infrav1.NIC + } DescribeTable("Complete successfully", - func(stdout string, expectedOutput []infrav1.NIC) { + func(tc testCaseObtainHardwareDetailsNics) { sshMock := &sshmock.Client{} - sshMock.On("GetHardwareDetailsNics").Return(sshclient.Output{StdOut: stdout}) + sshMock.On("GetHardwareDetailsNics").Return(sshclient.Output{StdOut: tc.stdout}) - Expect(obtainHardwareDetailsNics(sshMock)).Should(Equal(expectedOutput)) + Expect(obtainHardwareDetailsNics(sshMock)).Should(Equal(tc.expectedOutput)) }, - Entry( - "proper response", - `name="eth0" model="Realtek Semiconductor Co." mac="a8:a1:59:94:19:42" ip="23.88.6.239/26" speedMbps="1000" + Entry("proper response", testCaseObtainHardwareDetailsNics{ + stdout: `name="eth0" model="Realtek Semiconductor Co." mac="a8:a1:59:94:19:42" ip="23.88.6.239/26" speedMbps="1000" name="eth0" model="Realtek Semiconductor Co." mac="a8:a1:59:94:19:42" ip="2a01:4f8:272:3e0f::2/64" speedMbps="1000"`, - []infrav1.NIC{ + expectedOutput: []infrav1.NIC{ { Name: "eth0", Model: "Realtek Semiconductor Co.", @@ -122,30 +124,35 @@ var _ = Describe("obtainHardwareDetailsNics", func() { IP: "2a01:4f8:272:3e0f::2/64", SpeedMbps: 1000, }, - }), + }, + }), ) }) var _ = Describe("obtainHardwareDetailsStorage", func() { + type testCaseObtainHardwareDetailsStorage struct { + stdout string + expectedOutput []infrav1.Storage + expectedErrorMessage *string + } DescribeTable("Complete successfully", - func(stdout string, expectedOutput []infrav1.Storage, expectedErrorMessage *string) { + func(tc testCaseObtainHardwareDetailsStorage) { sshMock := &sshmock.Client{} - sshMock.On("GetHardwareDetailsStorage").Return(sshclient.Output{StdOut: stdout}) + sshMock.On("GetHardwareDetailsStorage").Return(sshclient.Output{StdOut: tc.stdout}) storageDevices, err := obtainHardwareDetailsStorage(sshMock) - Expect(storageDevices).Should(Equal(expectedOutput)) - if expectedErrorMessage != nil { - Expect(err.Error()).Should(ContainSubstring(*expectedErrorMessage)) + Expect(storageDevices).Should(Equal(tc.expectedOutput)) + if tc.expectedErrorMessage != nil { + Expect(err.Error()).Should(ContainSubstring(*tc.expectedErrorMessage)) } else { Expect(err).To(Succeed()) } }, - Entry( - "proper response", - `NAME="loop0" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="0" + Entry("proper response", testCaseObtainHardwareDetailsStorage{ + stdout: `NAME="loop0" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="0" NAME="nvme2n1" TYPE="disk" HCTL="" MODEL="SAMSUNG MZVL22T0HBLB-00B00" VENDOR="" SERIAL="S677NF0R402742" SIZE="2048408248320" WWN="eui.002538b411b2cee8" ROTA="0" NAME="nvme1n1" TYPE="disk" HCTL="" MODEL="SAMSUNG MZVLB512HAJQ-00000" VENDOR="" SERIAL="S3W8NX0N811178" SIZE="512110190592" WWN="eui.0025388801b4dff2" ROTA="0"`, - []infrav1.Storage{ + expectedOutput: []infrav1.Storage{ { Name: "nvme2n1", HCTL: "", @@ -169,30 +176,30 @@ NAME="nvme1n1" TYPE="disk" HCTL="" MODEL="SAMSUNG MZVLB512HAJQ-00000" VENDOR="" Rota: false, }, }, - nil, - ), - Entry( - "wrong rota", - `NAME="loop0" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="2" + expectedErrorMessage: nil, + }), + Entry("wrong rota", testCaseObtainHardwareDetailsStorage{ + stdout: `NAME="loop0" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="2" NAME="nvme2n1" TYPE="disk" HCTL="" MODEL="SAMSUNG MZVL22T0HBLB-00B00" VENDOR="" SERIAL="S677NF0R402742" SIZE="2048408248320" WWN="eui.002538b411b2cee8" ROTA="0" NAME="nvme1n1" TYPE="disk" HCTL="" MODEL="SAMSUNG MZVLB512HAJQ-00000" VENDOR="" SERIAL="S3W8NX0N811178" SIZE="512110190592" WWN="eui.0025388801b4dff2" ROTA="0"`, - nil, - pointer.String("unknown rota"), - ), + expectedOutput: nil, + expectedErrorMessage: pointer.String("unknown rota"), + }), ) }) var _ = Describe("handleIncompleteBoot", func() { Context("correct hostname == rescue", func() { + type testCaseHandleIncompleteBootCorrectHostname struct { + isRebootIntoRescue bool + isTimeOut bool + isConnectionRefused bool + hostErrorType infrav1.ErrorType + expectedReturnError error + expectedHostErrorType infrav1.ErrorType + } DescribeTable("hostName = rescue, varying error type and ssh client response - robot client giving all positive results, no timeouts", - func( - isRebootIntoRescue bool, - isTimeOut bool, - isConnectionRefused bool, - hostErrorType infrav1.ErrorType, - expectedReturnError error, - expectedHostErrorType infrav1.ErrorType, - ) { + func(tc testCaseHandleIncompleteBootCorrectHostname) { robotMock := robotmock.Client{} robotMock.On("SetBootRescue", bareMetalHostID, sshFingerprint).Return(nil, nil) robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: true}, nil) @@ -206,86 +213,87 @@ var _ = Describe("handleIncompleteBoot", func() { }), helpers.WithSSHSpec(), helpers.WithSSHStatus(), - helpers.WithError(hostErrorType, "", 1, metav1.Now()), + helpers.WithError(tc.hostErrorType, "", 1, metav1.Now()), ) service := newTestService(host, &robotMock, nil, nil, nil) - if expectedReturnError == nil { - Expect(service.handleIncompleteBoot(isRebootIntoRescue, isTimeOut, isConnectionRefused)).To(Succeed()) + if tc.expectedReturnError == nil { + Expect(service.handleIncompleteBoot(tc.isRebootIntoRescue, tc.isTimeOut, tc.isConnectionRefused)).To(Succeed()) } else { - Expect(service.handleIncompleteBoot(isRebootIntoRescue, isTimeOut, isConnectionRefused)).Should(Equal(expectedReturnError)) + Expect(service.handleIncompleteBoot(tc.isRebootIntoRescue, tc.isTimeOut, tc.isConnectionRefused)).Should(Equal(tc.expectedReturnError)) } - Expect(host.Spec.Status.ErrorType).To(Equal(expectedHostErrorType)) + Expect(host.Spec.Status.ErrorType).To(Equal(tc.expectedHostErrorType)) }, - Entry("timeout, no errorType", - true, // isRebootIntoRescue bool - true, // isTimeOut bool - false, // isConnectionRefused bool - infrav1.ErrorType(""), // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeSSHRebootTriggered, // expectedHostErrorType infrav1.ErrorType - ), - Entry("timeout,ErrorType == ErrorTypeSoftwareRebootTriggered", - true, // isRebootIntoRescue bool - true, // isTimeOut bool - false, // isConnectionRefused bool - infrav1.ErrorTypeSoftwareRebootTriggered, // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeSoftwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - ), - Entry("timeout,ErrorType == ErrorTypeHardwareRebootTriggered", - true, // isRebootIntoRescue bool - true, // isTimeOut bool - false, // isConnectionRefused bool - infrav1.ErrorTypeHardwareRebootTriggered, // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeHardwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - ), - Entry("timeout,ErrorType == ErrorTypeSoftwareRebootTriggered", - true, // isRebootIntoRescue bool - true, // isTimeOut bool - false, // isConnectionRefused bool - infrav1.ErrorTypeSoftwareRebootTriggered, // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeSoftwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - ), - Entry("timeout,ErrorType == ErrorTypeHardwareRebootTriggered", - true, // isRebootIntoRescue bool - true, // isTimeOut bool - false, // isConnectionRefused bool - infrav1.ErrorTypeHardwareRebootTriggered, // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeHardwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - ), - Entry("timeout,ErrorType == ErrorTypeSSHRebootTriggered", - true, // isRebootIntoRescue bool - false, // isTimeOut bool - false, // isConnectionRefused bool - infrav1.ErrorTypeSSHRebootTriggered, // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeSoftwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - ), - Entry("wrong boot", - false, // isRebootIntoRescue bool - false, // isTimeOut bool - false, // isConnectionRefused bool - infrav1.ErrorType(""), // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeSoftwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - ), + Entry("timeout, no errorType", testCaseHandleIncompleteBootCorrectHostname{ + isRebootIntoRescue: true, + isTimeOut: true, + isConnectionRefused: false, + hostErrorType: infrav1.ErrorType(""), + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeSSHRebootTriggered, + }), + Entry("timeout,ErrorType == ErrorTypeSoftwareRebootTriggered", testCaseHandleIncompleteBootCorrectHostname{ + isRebootIntoRescue: true, + isTimeOut: true, + isConnectionRefused: false, + hostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + }), + Entry("timeout,ErrorType == ErrorTypeHardwareRebootTriggered", testCaseHandleIncompleteBootCorrectHostname{ + isRebootIntoRescue: true, + isTimeOut: true, + isConnectionRefused: false, + hostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + }), + Entry("timeout,ErrorType == ErrorTypeSoftwareRebootTriggered", testCaseHandleIncompleteBootCorrectHostname{ + isRebootIntoRescue: true, + isTimeOut: true, + isConnectionRefused: false, + hostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + }), + Entry("timeout,ErrorType == ErrorTypeHardwareRebootTriggered", testCaseHandleIncompleteBootCorrectHostname{ + isRebootIntoRescue: true, + isTimeOut: true, + isConnectionRefused: false, + hostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + }), + Entry("timeout,ErrorType == ErrorTypeSSHRebootTriggered", testCaseHandleIncompleteBootCorrectHostname{ + isRebootIntoRescue: true, + isTimeOut: false, + isConnectionRefused: false, + hostErrorType: infrav1.ErrorTypeSSHRebootTriggered, + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + }), + Entry("wrong boot", testCaseHandleIncompleteBootCorrectHostname{ + isRebootIntoRescue: false, + isTimeOut: false, + isConnectionRefused: false, + hostErrorType: infrav1.ErrorType(""), + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + }), ) + type testCaseHandleIncompleteBootDifferentResetTypes struct { + isTimeOut bool + isConnectionRefused bool + rebootTypes []infrav1.RebootType + hostErrorType infrav1.ErrorType + expectedHostErrorType infrav1.ErrorType + expectedRebootType infrav1.RebootType + } // Test with different reset type only software on machine DescribeTable("Different reset types", - func( - isTimeOut bool, - isConnectionRefused bool, - rebootTypes []infrav1.RebootType, - hostErrorType infrav1.ErrorType, - expectedHostErrorType infrav1.ErrorType, - expectedRebootType infrav1.RebootType, - ) { + func(tc testCaseHandleIncompleteBootDifferentResetTypes) { robotMock := robotmock.Client{} robotMock.On("SetBootRescue", bareMetalHostID, sshFingerprint).Return(nil, nil) robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: true}, nil) @@ -295,77 +303,79 @@ var _ = Describe("handleIncompleteBoot", func() { helpers.WithSSHSpec(), helpers.WithSSHStatus(), // Make sure that timeouts are exceeded to trigger escalation step - helpers.WithError(hostErrorType, "", 1, metav1.NewTime(time.Now().Add(-time.Hour))), - helpers.WithRebootTypes(rebootTypes), + helpers.WithError(tc.hostErrorType, "", 1, metav1.NewTime(time.Now().Add(-time.Hour))), + helpers.WithRebootTypes(tc.rebootTypes), ) service := newTestService(host, &robotMock, nil, nil, nil) - Expect(service.handleIncompleteBoot(true, isTimeOut, isConnectionRefused)).To(Succeed()) - Expect(host.Spec.Status.ErrorType).To(Equal(expectedHostErrorType)) - if expectedRebootType != infrav1.RebootType("") { - Expect(robotMock.AssertCalled(GinkgoT(), "RebootBMServer", bareMetalHostID, expectedRebootType)).To(BeTrue()) + Expect(service.handleIncompleteBoot(true, tc.isTimeOut, tc.isConnectionRefused)).To(Succeed()) + Expect(host.Spec.Status.ErrorType).To(Equal(tc.expectedHostErrorType)) + if tc.expectedRebootType != infrav1.RebootType("") { + Expect(robotMock.AssertCalled(GinkgoT(), "RebootBMServer", bareMetalHostID, tc.expectedRebootType)).To(BeTrue()) } else { Expect(robotMock.AssertNotCalled(GinkgoT(), "RebootBMServer", bareMetalHostID, mock.Anything)).To(BeTrue()) } }, - Entry("timeout, no errorType, only hw reset", - true, // isTimeOut bool - false, // isConnectionRefused bool - []infrav1.RebootType{infrav1.RebootTypeHardware}, // rebootTypes []infrav1.RebootType - infrav1.ErrorTypeSSHRebootTriggered, // hostErrorType infrav1.ErrorType - infrav1.ErrorTypeHardwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - infrav1.RebootTypeHardware, // expectedRebootType infrav1.RebootType - ), - Entry("wrong boot, only hw reset", - false, // isTimeOut bool - false, // isConnectionRefused bool - []infrav1.RebootType{infrav1.RebootTypeHardware}, // rebootTypes []infrav1.RebootType - infrav1.ErrorType(""), // hostErrorType infrav1.ErrorType - infrav1.ErrorTypeHardwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - infrav1.RebootTypeHardware, // expectedRebootType infrav1.RebootType - ), - Entry("wrong boot, only hw reset, errorType =ErrorTypeSSHRebootTriggered", - false, // isTimeOut bool - false, // isConnectionRefused bool - []infrav1.RebootType{infrav1.RebootTypeHardware}, // rebootTypes []infrav1.RebootType - infrav1.ErrorTypeSSHRebootTriggered, // hostErrorType infrav1.ErrorType - infrav1.ErrorTypeHardwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - infrav1.RebootTypeHardware, // expectedRebootType infrav1.RebootType - ), - Entry("wrong boot, errorType =ErrorTypeSSHRebootTriggered", - false, // isTimeOut bool - false, // isConnectionRefused bool - []infrav1.RebootType{infrav1.RebootTypeSoftware, infrav1.RebootTypeHardware}, // rebootTypes []infrav1.RebootType - infrav1.ErrorTypeSSHRebootTriggered, // hostErrorType infrav1.ErrorType - infrav1.ErrorTypeSoftwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - infrav1.RebootTypeSoftware, // expectedRebootType infrav1.RebootType - ), - Entry("wrong boot, errorType =ErrorTypeSoftwareRebootTriggered", - false, // isTimeOut bool - false, // isConnectionRefused bool - []infrav1.RebootType{infrav1.RebootTypeSoftware, infrav1.RebootTypeHardware}, // rebootTypes []infrav1.RebootType - infrav1.ErrorTypeSoftwareRebootTriggered, // hostErrorType infrav1.ErrorType - infrav1.ErrorTypeHardwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - infrav1.RebootTypeHardware, // expectedRebootType infrav1.RebootType - ), - Entry("wrong boot, errorType =ErrorTypeHardwareRebootTriggered", - false, // isTimeOut bool - false, // isConnectionRefused bool - []infrav1.RebootType{infrav1.RebootTypeSoftware, infrav1.RebootTypeHardware}, // rebootTypes []infrav1.RebootType - infrav1.ErrorTypeHardwareRebootTriggered, // hostErrorType infrav1.ErrorType - infrav1.ErrorTypeHardwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - infrav1.RebootTypeHardware, // expectedRebootType infrav1.RebootType - ), + Entry("timeout, no errorType, only hw reset", testCaseHandleIncompleteBootDifferentResetTypes{ + isTimeOut: true, + isConnectionRefused: false, + rebootTypes: []infrav1.RebootType{infrav1.RebootTypeHardware}, + hostErrorType: infrav1.ErrorTypeSSHRebootTriggered, + expectedHostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedRebootType: infrav1.RebootTypeHardware, + }), + Entry("wrong boot, only hw reset", testCaseHandleIncompleteBootDifferentResetTypes{ + isTimeOut: false, + isConnectionRefused: false, + rebootTypes: []infrav1.RebootType{infrav1.RebootTypeHardware}, + hostErrorType: infrav1.ErrorType(""), + expectedHostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedRebootType: infrav1.RebootTypeHardware, + }), + Entry("wrong boot, only hw reset, errorType =ErrorTypeSSHRebootTriggered", testCaseHandleIncompleteBootDifferentResetTypes{ + isTimeOut: false, + isConnectionRefused: false, + rebootTypes: []infrav1.RebootType{infrav1.RebootTypeHardware}, + hostErrorType: infrav1.ErrorTypeSSHRebootTriggered, + expectedHostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedRebootType: infrav1.RebootTypeHardware, + }), + Entry("wrong boot, errorType =ErrorTypeSSHRebootTriggered", testCaseHandleIncompleteBootDifferentResetTypes{ + isTimeOut: false, + isConnectionRefused: false, + rebootTypes: []infrav1.RebootType{infrav1.RebootTypeSoftware, infrav1.RebootTypeHardware}, + hostErrorType: infrav1.ErrorTypeSSHRebootTriggered, + expectedHostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + expectedRebootType: infrav1.RebootTypeSoftware, + }), + Entry("wrong boot, errorType =ErrorTypeSoftwareRebootTriggered", testCaseHandleIncompleteBootDifferentResetTypes{ + isTimeOut: false, + isConnectionRefused: false, + rebootTypes: []infrav1.RebootType{infrav1.RebootTypeSoftware, infrav1.RebootTypeHardware}, + hostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + expectedHostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedRebootType: infrav1.RebootTypeHardware, + }), + Entry("wrong boot, errorType =ErrorTypeHardwareRebootTriggered", testCaseHandleIncompleteBootDifferentResetTypes{ + isTimeOut: false, + isConnectionRefused: false, + rebootTypes: []infrav1.RebootType{infrav1.RebootTypeSoftware, infrav1.RebootTypeHardware}, + hostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedHostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedRebootType: infrav1.RebootTypeHardware, + }), ) + type testCaseHandleIncompleteBootDifferentTimeouts struct { + hostErrorType infrav1.ErrorType + lastUpdated time.Time + expectedHostErrorType infrav1.ErrorType + expectedRebootType infrav1.RebootType + } + // Test with reached timeouts DescribeTable("Different timeouts", - func( - hostErrorType infrav1.ErrorType, - lastUpdated time.Time, - expectedHostErrorType infrav1.ErrorType, - expectedRebootType infrav1.RebootType, - ) { + func(tc testCaseHandleIncompleteBootDifferentTimeouts) { robotMock := robotmock.Client{} robotMock.On("SetBootRescue", bareMetalHostID, sshFingerprint).Return(nil, nil) robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: true}, nil) @@ -379,58 +389,56 @@ var _ = Describe("handleIncompleteBoot", func() { }), helpers.WithSSHSpec(), helpers.WithSSHStatus(), - helpers.WithError(hostErrorType, "", 1, metav1.Time{Time: lastUpdated}), + helpers.WithError(tc.hostErrorType, "", 1, metav1.Time{Time: tc.lastUpdated}), ) service := newTestService(host, &robotMock, nil, nil, nil) Expect(service.handleIncompleteBoot(true, true, false)).To(Succeed()) - Expect(host.Spec.Status.ErrorType).To(Equal(expectedHostErrorType)) - if expectedRebootType != infrav1.RebootType("") { - Expect(robotMock.AssertCalled(GinkgoT(), "RebootBMServer", bareMetalHostID, expectedRebootType)).To(BeTrue()) + Expect(host.Spec.Status.ErrorType).To(Equal(tc.expectedHostErrorType)) + if tc.expectedRebootType != infrav1.RebootType("") { + Expect(robotMock.AssertCalled(GinkgoT(), "RebootBMServer", bareMetalHostID, tc.expectedRebootType)).To(BeTrue()) } else { Expect(robotMock.AssertNotCalled(GinkgoT(), "RebootBMServer", bareMetalHostID, mock.Anything)).To(BeTrue()) } }, - Entry( - "timed out hw reset", // hostErrorType infrav1.ErrorType - infrav1.ErrorTypeHardwareRebootTriggered, // hostErrorType infrav1.ErrorType - time.Now().Add(-time.Hour), // lastUpdated time.Time - infrav1.ErrorTypeHardwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - infrav1.RebootTypeHardware, // expectedRebootType infrav1.RebootType - ), - Entry( - "timed out sw reset", // hostErrorType infrav1.ErrorType - infrav1.ErrorTypeSoftwareRebootTriggered, // hostErrorType infrav1.ErrorType - time.Now().Add(-5*time.Minute), // lastUpdated time.Time - infrav1.ErrorTypeHardwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - infrav1.RebootTypeHardware, // expectedRebootType infrav1.RebootType - ), - Entry( - "not timed out hw reset", // hostErrorType infrav1.ErrorType - infrav1.ErrorTypeHardwareRebootTriggered, // hostErrorType infrav1.ErrorType - time.Now().Add(-30*time.Minute), // lastUpdated time.Time - infrav1.ErrorTypeHardwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - infrav1.RebootType(""), // expectedRebootType infrav1.RebootType - ), - Entry( - "not timed out sw reset", - infrav1.ErrorTypeSoftwareRebootTriggered, // hostErrorType infrav1.ErrorType - time.Now().Add(-3*time.Minute), // lastUpdated time.Time - infrav1.ErrorTypeSoftwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - infrav1.RebootType(""), // expectedRebootType infrav1.RebootType - ), + Entry("timed out hw reset", testCaseHandleIncompleteBootDifferentTimeouts{ + hostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + lastUpdated: time.Now().Add(-time.Hour), + expectedHostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedRebootType: infrav1.RebootTypeHardware, + }), + Entry("timed out sw reset", testCaseHandleIncompleteBootDifferentTimeouts{ + hostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + lastUpdated: time.Now().Add(-5 * time.Minute), + expectedHostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedRebootType: infrav1.RebootTypeHardware, + }), + Entry("not timed out hw reset", testCaseHandleIncompleteBootDifferentTimeouts{ + hostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + lastUpdated: time.Now().Add(-30 * time.Minute), + expectedHostErrorType: infrav1.ErrorTypeHardwareRebootTriggered, + expectedRebootType: infrav1.RebootType(""), + }), + Entry("not timed out sw reset", testCaseHandleIncompleteBootDifferentTimeouts{ + hostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + lastUpdated: time.Now().Add(-3 * time.Minute), + expectedHostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + expectedRebootType: infrav1.RebootType(""), + }), ) }) Context("hostname rescue vs machinename", func() { + type testCaseHandleIncompleteBoot struct { + isRebootIntoRescue bool + hostErrorType infrav1.ErrorType + expectedReturnError error + expectedHostErrorType infrav1.ErrorType + expectsRescueCall bool + } + DescribeTable("vary hostname and see whether rescue gets triggered", - func( - isRebootIntoRescue bool, - hostErrorType infrav1.ErrorType, - expectedReturnError error, - expectedHostErrorType infrav1.ErrorType, - expectsRescueCall bool, - ) { + func(tc testCaseHandleIncompleteBoot) { robotMock := robotmock.Client{} robotMock.On("SetBootRescue", bareMetalHostID, sshFingerprint).Return(nil, nil) robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: true}, nil) @@ -444,50 +452,50 @@ var _ = Describe("handleIncompleteBoot", func() { }), helpers.WithSSHSpec(), helpers.WithSSHStatus(), - helpers.WithError(hostErrorType, "", 1, metav1.Now()), + helpers.WithError(tc.hostErrorType, "", 1, metav1.Now()), ) service := newTestService(host, &robotMock, nil, nil, nil) - if expectedReturnError == nil { - Expect(service.handleIncompleteBoot(isRebootIntoRescue, false, false)).To(Succeed()) + if tc.expectedReturnError == nil { + Expect(service.handleIncompleteBoot(tc.isRebootIntoRescue, false, false)).To(Succeed()) } else { - Expect(service.handleIncompleteBoot(isRebootIntoRescue, false, false)).Should(Equal(expectedReturnError)) + Expect(service.handleIncompleteBoot(tc.isRebootIntoRescue, false, false)).Should(Equal(tc.expectedReturnError)) } - Expect(host.Spec.Status.ErrorType).To(Equal(expectedHostErrorType)) - if expectsRescueCall { + Expect(host.Spec.Status.ErrorType).To(Equal(tc.expectedHostErrorType)) + if tc.expectsRescueCall { Expect(robotMock.AssertCalled(GinkgoT(), "GetBootRescue", bareMetalHostID)).To(BeTrue()) } else { Expect(robotMock.AssertNotCalled(GinkgoT(), "GetBootRescue", bareMetalHostID)).To(BeTrue()) } }, - Entry("hostname == rescue", - true, // isRebootIntoRescue bool - infrav1.ErrorType(""), // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeSoftwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - true, // expectsRescueCall bool - ), - Entry("hostname != rescue", - false, // isRebootIntoRescue bool - infrav1.ErrorType(""), // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeSoftwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - false, // expectsRescueCall bool - ), - Entry("hostname == rescue, ErrType == ErrorTypeSSHRebootTriggered", - true, // isRebootIntoRescue bool - infrav1.ErrorTypeSSHRebootTriggered, // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeSoftwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - true, // expectsRescueCall bool - ), - Entry("hostname != rescue, ErrType == ErrorTypeSSHRebootTriggered", - false, // isRebootIntoRescue bool - infrav1.ErrorTypeSSHRebootTriggered, // hostErrorType infrav1.ErrorType - nil, // expectedReturnError error - infrav1.ErrorTypeSoftwareRebootTriggered, // expectedHostErrorType infrav1.ErrorType - false, // expectsRescueCall bool - ), + Entry("hostname == rescue", testCaseHandleIncompleteBoot{ + isRebootIntoRescue: true, + hostErrorType: infrav1.ErrorType(""), + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + expectsRescueCall: true, + }), + Entry("hostname != rescue", testCaseHandleIncompleteBoot{ + isRebootIntoRescue: false, + hostErrorType: infrav1.ErrorType(""), + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + expectsRescueCall: false, + }), + Entry("hostname == rescue, ErrType == ErrorTypeSSHRebootTriggered", testCaseHandleIncompleteBoot{ + isRebootIntoRescue: true, + hostErrorType: infrav1.ErrorTypeSSHRebootTriggered, + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + expectsRescueCall: true, + }), + Entry("hostname != rescue, ErrType == ErrorTypeSSHRebootTriggered", testCaseHandleIncompleteBoot{ + isRebootIntoRescue: false, + hostErrorType: infrav1.ErrorTypeSSHRebootTriggered, + expectedReturnError: nil, + expectedHostErrorType: infrav1.ErrorTypeSoftwareRebootTriggered, + expectsRescueCall: false, + }), ) }) }) @@ -532,19 +540,22 @@ var _ = Describe("ensureSSHKey", func() { Expect(host.Spec.Status.ErrorType).To(Equal(infrav1.FatalError)) }) + type testCaseEnsureSSHKey struct { + hetznerSSHKeys []models.Key + sshSecretKeyRef infrav1.SSHSecretKeyRef + expectedFingerprint string + expectedActionResult actionResult + expectSetSSHKey bool + } + DescribeTable("ensureSSHKey", - func(hetznerSSHKeys []models.Key, - sshSecretKeyRef infrav1.SSHSecretKeyRef, - expectedFingerprint string, - expectedActionResult actionResult, - expectSetSSHKey bool, - ) { + func(tc testCaseEnsureSSHKey) { secret := helpers.GetDefaultSSHSecret("ssh-secret", "default") robotMock := robotmock.Client{} - robotMock.On("SetSSHKey", string(secret.Data[sshSecretKeyRef.Name]), mock.Anything).Return( - &models.Key{Name: sshSecretKeyRef.Name, Fingerprint: defaultFingerPrint}, nil, + robotMock.On("SetSSHKey", string(secret.Data[tc.sshSecretKeyRef.Name]), mock.Anything).Return( + &models.Key{Name: tc.sshSecretKeyRef.Name, Fingerprint: defaultFingerPrint}, nil, ) - robotMock.On("ListSSHKeys").Return(hetznerSSHKeys, nil) + robotMock.On("ListSSHKeys").Return(tc.hetznerSSHKeys, nil) host := helpers.BareMetalHost("test-host", "default") @@ -552,155 +563,153 @@ var _ = Describe("ensureSSHKey", func() { sshKey, actResult := service.ensureSSHKey(infrav1.SSHSecretRef{ Name: "secret-name", - Key: sshSecretKeyRef, + Key: tc.sshSecretKeyRef, }, secret) - Expect(sshKey.Fingerprint).To(Equal(expectedFingerprint)) - Expect(actResult).Should(BeAssignableToTypeOf(expectedActionResult)) - if expectSetSSHKey { - Expect(robotMock.AssertCalled(GinkgoT(), "SetSSHKey", string(secret.Data[sshSecretKeyRef.Name]), mock.Anything)).To(BeTrue()) + Expect(sshKey.Fingerprint).To(Equal(tc.expectedFingerprint)) + Expect(actResult).Should(BeAssignableToTypeOf(tc.expectedActionResult)) + if tc.expectSetSSHKey { + Expect(robotMock.AssertCalled(GinkgoT(), "SetSSHKey", string(secret.Data[tc.sshSecretKeyRef.Name]), mock.Anything)).To(BeTrue()) } else { - Expect(robotMock.AssertNotCalled(GinkgoT(), "SetSSHKey", string(secret.Data[sshSecretKeyRef.Name]), mock.Anything)).To(BeTrue()) + Expect(robotMock.AssertNotCalled(GinkgoT(), "SetSSHKey", string(secret.Data[tc.sshSecretKeyRef.Name]), mock.Anything)).To(BeTrue()) } }, - Entry( - "empty list", - nil, - infrav1.SSHSecretKeyRef{ + Entry("empty list", testCaseEnsureSSHKey{ + hetznerSSHKeys: nil, + sshSecretKeyRef: infrav1.SSHSecretKeyRef{ Name: "sshkey-name", PublicKey: "public-key", PrivateKey: "private-key", }, - defaultFingerPrint, - actionComplete{}, - true, - ), - Entry("secret in list", - []models.Key{ + expectedFingerprint: defaultFingerPrint, + expectedActionResult: actionComplete{}, + expectSetSSHKey: true, + }), + Entry("secret in list", testCaseEnsureSSHKey{ + hetznerSSHKeys: []models.Key{ { Name: "my-name", Fingerprint: "my-fingerprint", }, }, - infrav1.SSHSecretKeyRef{ + sshSecretKeyRef: infrav1.SSHSecretKeyRef{ Name: "sshkey-name", PublicKey: "public-key", PrivateKey: "private-key", }, - "my-fingerprint", - actionComplete{}, - false, - ), - Entry( - "secret not in list", - []models.Key{ - { - Name: "secret2", - Fingerprint: "my fingerprint", + expectedFingerprint: "my-fingerprint", + expectedActionResult: actionComplete{}, + expectSetSSHKey: false, + }), + Entry( + "secret not in list", testCaseEnsureSSHKey{ + hetznerSSHKeys: []models.Key{ + { + Name: "secret2", + Fingerprint: "my fingerprint", + }, + { + Name: "secret3", + Fingerprint: "my fingerprint", + }, }, - { - Name: "secret3", - Fingerprint: "my fingerprint", + sshSecretKeyRef: infrav1.SSHSecretKeyRef{ + Name: "sshkey-name", + PublicKey: "public-key", + PrivateKey: "private-key", }, - }, - infrav1.SSHSecretKeyRef{ - Name: "sshkey-name", - PublicKey: "public-key", - PrivateKey: "private-key", - }, - defaultFingerPrint, - actionComplete{}, - true, - ), + expectedFingerprint: defaultFingerPrint, + expectedActionResult: actionComplete{}, + expectSetSSHKey: true, + }), ) }) var _ = Describe("analyzeSSHOutputInstallImage", func() { + type testCaseAnalyzeSSHOutputInstallImageOutErr struct { + err error + rescueActive bool + expectedIsTimeout bool + expectedIsConnectionRefused bool + expectedErrMessage string + } + DescribeTable("analyzeSSHOutputInstallImage - out.Err", - func( - err error, - rescueActive bool, - expectedIsTimeout bool, - expectedIsConnectionRefused bool, - expectedErrMessage string, - ) { + func(tc testCaseAnalyzeSSHOutputInstallImageOutErr) { host := helpers.BareMetalHost( "test-host", "default", ) robotMock := robotmock.Client{} - robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: rescueActive}, nil) + robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: tc.rescueActive}, nil) service := newTestService(host, &robotMock, nil, nil, nil) - isTimeout, isConnectionRefused, err := service.analyzeSSHOutputRegistering(sshclient.Output{Err: err}) - Expect(isTimeout).To(Equal(expectedIsTimeout)) - Expect(isConnectionRefused).To(Equal(expectedIsConnectionRefused)) - if expectedErrMessage != "" { + isTimeout, isConnectionRefused, err := service.analyzeSSHOutputRegistering(sshclient.Output{Err: tc.err}) + Expect(isTimeout).To(Equal(tc.expectedIsTimeout)) + Expect(isConnectionRefused).To(Equal(tc.expectedIsConnectionRefused)) + if tc.expectedErrMessage != "" { Expect(err).To(Not(BeNil())) - Expect(err.Error()).To(ContainSubstring(expectedErrMessage)) + Expect(err.Error()).To(ContainSubstring(tc.expectedErrMessage)) } else { Expect(err).To(BeNil()) } }, - Entry( - "timeout error", - timeout, // err error - true, // rescueActive bool - true, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - "", // expectedErrMessage string - ), - Entry( - "authenticationFailed error, rescue active", - sshclient.ErrAuthenticationFailed, // err error - true, // rescueActive bool - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - "", // expectedErrMessage string - ), - Entry( - "authenticationFailed error, rescue not active", - sshclient.ErrAuthenticationFailed, // err error - false, // rescueActive bool - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - "wrong ssh key", // expectedErrMessage string - ), - Entry( - "connectionRefused error, rescue active", - sshclient.ErrConnectionRefused, // err error - true, // rescueActive bool - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - "", // expectedErrMessage string - ), - Entry( - "connectionRefused error, rescue not active", - sshclient.ErrConnectionRefused, // err error - false, // rescueActive bool - false, // expectedIsTimeout bool - true, // expectedIsConnectionRefused bool - "", // expectedErrMessage string - ), + Entry("timeout error", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: timeout, + rescueActive: true, + expectedIsTimeout: true, + expectedIsConnectionRefused: false, + expectedErrMessage: "", + }), + Entry("authenticationFailed error, rescue active", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: sshclient.ErrAuthenticationFailed, + rescueActive: true, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: "", + }), + Entry("authenticationFailed error, rescue not active", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: sshclient.ErrAuthenticationFailed, + rescueActive: false, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: "wrong ssh key", + }), + Entry("connectionRefused error, rescue active", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: sshclient.ErrConnectionRefused, + rescueActive: true, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: "", + }), + Entry("connectionRefused error, rescue not active", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: sshclient.ErrConnectionRefused, + rescueActive: false, + expectedIsTimeout: false, + expectedIsConnectionRefused: true, + expectedErrMessage: "", + }), ) + type testCaseAnalyzeSSHOutputInstallImageStdErr struct { + hasNilErr bool + stdErr string + hostName string + expectedErrMessage string + } + DescribeTable("analyzeSSHOutputRegistering - toggle stdErr and hostName", - func( - hasNilErr bool, - stdErr string, - hostName string, - expectedErrMessage string, - ) { + func(tc testCaseAnalyzeSSHOutputInstallImageStdErr) { var err error - if !hasNilErr { + if !tc.hasNilErr { err = errTest } out := sshclient.Output{ - StdOut: hostName, - StdErr: stdErr, + StdOut: tc.hostName, + StdErr: tc.stdErr, Err: err, } @@ -717,328 +726,314 @@ var _ = Describe("analyzeSSHOutputInstallImage", func() { isTimeout, isConnectionRefused, err := service.analyzeSSHOutputRegistering(out) Expect(isTimeout).To(Equal(false)) Expect(isConnectionRefused).To(Equal(false)) - if expectedErrMessage != "" { + if tc.expectedErrMessage != "" { Expect(err).To(Not(BeNil())) - Expect(err.Error()).To(ContainSubstring(expectedErrMessage)) + Expect(err.Error()).To(ContainSubstring(tc.expectedErrMessage)) } else { Expect(err).To(BeNil()) } }, - Entry( - "stderr not empty", - true, // hasNilErr bool - "command failed", // stdErr string - "hostName", // hostName string - "failed to get hostname via ssh: StdErr:", // expectedErrMessage string - ), - Entry( - "stderr not empty - err != nil", - false, // hasNilErr bool - "command failed", // stdErr string - "", // hostName string - "unhandled ssh error while getting hostname", // expectedErrMessage string - ), - Entry( - "stderr not empty - wrong hostName", - true, // hasNilErr bool - "command failed", // stdErr string - "", // hostName string - "failed to get hostname via ssh: StdErr:", // expectedErrMessage string - ), - Entry( - "stderr empty - wrong hostName", - true, // hasNilErr bool - "", // stdErr string - "", // hostName string - "hostname is empty", // expectedErrMessage string - ), + Entry("stderr not empty", testCaseAnalyzeSSHOutputInstallImageStdErr{ + hasNilErr: true, + stdErr: "command failed", + hostName: "hostName", + expectedErrMessage: "failed to get hostname via ssh: StdErr:", + }), + Entry("stderr not empty - err != nil", testCaseAnalyzeSSHOutputInstallImageStdErr{ + hasNilErr: false, + stdErr: "command failed", + hostName: "", + expectedErrMessage: "unhandled ssh error while getting hostname", + }), + Entry("stderr not empty - wrong hostName", testCaseAnalyzeSSHOutputInstallImageStdErr{ + hasNilErr: true, + stdErr: "command failed", + hostName: "", + expectedErrMessage: "failed to get hostname via ssh: StdErr:", + }), + Entry("stderr empty - wrong hostName", testCaseAnalyzeSSHOutputInstallImageStdErr{ + hasNilErr: true, + stdErr: "", + hostName: "", + expectedErrMessage: "hostname is empty", + }), ) }) var _ = Describe("analyzeSSHOutputInstallImage", func() { + type testCaseAnalyzeSSHOutputInstallImageOutErr struct { + err error + errFromGetHostNameNil bool + port int + expectedIsTimeout bool + expectedIsConnectionRefused bool + expectedErrMessage string + } + DescribeTable("analyzeSSHOutputInstallImage - out.Err", - func( - err error, - errFromGetHostNameNil bool, - port int, - expectedIsTimeout bool, - expectedIsConnectionRefused bool, - expectedErrMessage string, - ) { + func(tc testCaseAnalyzeSSHOutputInstallImageOutErr) { sshMock := &sshmock.Client{} var errFromGetHostName error - if !errFromGetHostNameNil { + if !tc.errFromGetHostNameNil { errFromGetHostName = errTest } sshMock.On("GetHostName").Return(sshclient.Output{Err: errFromGetHostName}) - isTimeout, isConnectionRefused, err := analyzeSSHOutputInstallImage(sshclient.Output{Err: err}, sshMock, port) - Expect(isTimeout).To(Equal(expectedIsTimeout)) - Expect(isConnectionRefused).To(Equal(expectedIsConnectionRefused)) - if expectedErrMessage != "" { + isTimeout, isConnectionRefused, err := analyzeSSHOutputInstallImage(sshclient.Output{Err: tc.err}, sshMock, tc.port) + Expect(isTimeout).To(Equal(tc.expectedIsTimeout)) + Expect(isConnectionRefused).To(Equal(tc.expectedIsConnectionRefused)) + if tc.expectedErrMessage != "" { Expect(err).To(Not(BeNil())) - Expect(err.Error()).To(ContainSubstring(expectedErrMessage)) + Expect(err.Error()).To(ContainSubstring(tc.expectedErrMessage)) } else { Expect(err).To(BeNil()) } }, - Entry( - "timeout error", - timeout, // err error - true, // errFromGetHostNameNil bool - 22, // port int - true, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - "", // expectedErrMessage string - ), - Entry( - "authenticationFailed error, port 22, no hostName error", - sshclient.ErrAuthenticationFailed, // err error - true, // errFromGetHostNameNil bool - 22, // port int - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - "", // expectedErrMessage string - ), - Entry( - "authenticationFailed error, port 22, hostName error", - sshclient.ErrAuthenticationFailed, // err error - false, // errFromGetHostNameNil bool - 22, // port int - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - "wrong ssh key", // expectedErrMessage string - ), - Entry( - "authenticationFailed error, port != 22", - sshclient.ErrAuthenticationFailed, // err error - true, // errFromGetHostNameNil bool - 23, // port int - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - "wrong ssh key", // expectedErrMessage string - ), - Entry( - "connectionRefused error, port 22", - sshclient.ErrConnectionRefused, // err error - true, // errFromGetHostNameNil bool - 22, // port int - false, // expectedIsTimeout bool - true, // expectedIsConnectionRefused bool - "", // expectedErrMessage string - ), - Entry( - "connectionRefused error, port != 22, hostname error", - sshclient.ErrConnectionRefused, // err error - false, // errFromGetHostNameNil bool - 23, // port int - false, // expectedIsTimeout bool - true, // expectedIsConnectionRefused bool - "", // expectedErrMessage string - ), - Entry( - "connectionRefused error, port != 22, no hostname error", - sshclient.ErrConnectionRefused, // err error - true, // errFromGetHostNameNil bool - 23, // port int - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - "", // expectedErrMessage string - ), + Entry("timeout error", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: timeout, + errFromGetHostNameNil: true, + port: 22, + expectedIsTimeout: true, + expectedIsConnectionRefused: false, + expectedErrMessage: "", + }), + Entry("authenticationFailed error, port 22, no hostName error", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: sshclient.ErrAuthenticationFailed, + errFromGetHostNameNil: true, + port: 22, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: "", + }), + Entry("authenticationFailed error, port 22, hostName error", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: sshclient.ErrAuthenticationFailed, + errFromGetHostNameNil: false, + port: 22, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: "wrong ssh key", + }), + Entry("authenticationFailed error, port != 22", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: sshclient.ErrAuthenticationFailed, + errFromGetHostNameNil: true, + port: 23, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: "wrong ssh key", + }), + Entry("connectionRefused error, port 22", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: sshclient.ErrConnectionRefused, + errFromGetHostNameNil: true, + port: 22, + expectedIsTimeout: false, + expectedIsConnectionRefused: true, + expectedErrMessage: "", + }), + Entry("connectionRefused error, port != 22, hostname error", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: sshclient.ErrConnectionRefused, + errFromGetHostNameNil: false, + port: 23, + expectedIsTimeout: false, + expectedIsConnectionRefused: true, + expectedErrMessage: "", + }), + Entry("connectionRefused error, port != 22, no hostname error", testCaseAnalyzeSSHOutputInstallImageOutErr{ + err: sshclient.ErrConnectionRefused, + errFromGetHostNameNil: true, + port: 23, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: "", + }), ) + type testCaseAnalyzeSSHOutputInstallImageStdErr struct { + hasNilErr bool + stdErr string + hasWrongHostName bool + expectedErrMessage string + } + DescribeTable("analyzeSSHOutputInstallImage - StdErr not empty", - func( - hasNilErr bool, - stdErr string, - hasWrongHostName bool, - expectedErrMessage string, - ) { + func(tc testCaseAnalyzeSSHOutputInstallImageStdErr) { var err error - if !hasNilErr { + if !tc.hasNilErr { err = errTest } hostName := "rescue" - if hasWrongHostName { + if tc.hasWrongHostName { hostName = "wrongHostName" } out := sshclient.Output{ StdOut: hostName, - StdErr: stdErr, + StdErr: tc.stdErr, Err: err, } isTimeout, isConnectionRefused, err := analyzeSSHOutputInstallImage(out, nil, 22) Expect(isTimeout).To(Equal(false)) Expect(isConnectionRefused).To(Equal(false)) - if expectedErrMessage != "" { + if tc.expectedErrMessage != "" { Expect(err).To(Not(BeNil())) - Expect(err.Error()).To(ContainSubstring(expectedErrMessage)) + Expect(err.Error()).To(ContainSubstring(tc.expectedErrMessage)) } else { Expect(err).To(BeNil()) } }, - Entry( - "stderr not empty", - true, // hasNilErr bool - "command failed", // stdErr string - false, // hasWrongHostName bool - "failed to get hostname via ssh: StdErr:", // expectedErrMessage string - ), - Entry( - "stderr not empty - err != nil", - false, // hasNilErr bool - "command failed", // stdErr string - false, // hasWrongHostName bool - "unhandled ssh error while getting hostname", // expectedErrMessage string - ), - Entry( - "stderr not empty - wrong hostName", - true, // hasNilErr bool - "command failed", // stdErr string - true, // hasWrongHostName bool - "failed to get hostname via ssh: StdErr:", // expectedErrMessage string - ), + Entry("stderr not empty", testCaseAnalyzeSSHOutputInstallImageStdErr{ + hasNilErr: true, + stdErr: "command failed", + hasWrongHostName: false, + expectedErrMessage: "failed to get hostname via ssh: StdErr:", + }), + Entry("stderr not empty - err != nil", testCaseAnalyzeSSHOutputInstallImageStdErr{ + hasNilErr: false, + stdErr: "command failed", + hasWrongHostName: false, + expectedErrMessage: "unhandled ssh error while getting hostname", + }), + Entry("stderr not empty - wrong hostName", testCaseAnalyzeSSHOutputInstallImageStdErr{ + hasNilErr: true, + stdErr: "command failed", + hasWrongHostName: true, + expectedErrMessage: "failed to get hostname via ssh: StdErr:", + }), ) + type testCaseAnalyzeSSHOutputInstallImageWrongHostname struct { + hasNilErr bool + stdErr string + hostName string + expectedErrMessage string + } + DescribeTable("analyzeSSHOutputInstallImage - wrong hostName", - func( - hasNilErr bool, - stdErr string, - hostName string, - expectedErrMessage string, - ) { + func(tc testCaseAnalyzeSSHOutputInstallImageWrongHostname) { var err error - if !hasNilErr { + if !tc.hasNilErr { err = errTest } out := sshclient.Output{ - StdOut: hostName, - StdErr: stdErr, + StdOut: tc.hostName, + StdErr: tc.stdErr, Err: err, } isTimeout, isConnectionRefused, err := analyzeSSHOutputInstallImage(out, nil, 22) Expect(isTimeout).To(Equal(false)) Expect(isConnectionRefused).To(Equal(false)) - if expectedErrMessage != "" { + if tc.expectedErrMessage != "" { Expect(err).To(Not(BeNil())) - Expect(err.Error()).To(ContainSubstring(expectedErrMessage)) + Expect(err.Error()).To(ContainSubstring(tc.expectedErrMessage)) } else { Expect(err).To(BeNil()) } }, - Entry( - "empty hostname", - true, // hasNilErr bool - "", // stdErr string - "", // hostName string - "hostname is empty", // expectedErrMessage string - ), - Entry( - "empty hostname - err not empty", - false, // hasNilErr bool - "", // stdErr string - "", // hostName string - "unhandled ssh error while getting hostname", // expectedErrMessage string - ), - Entry( - "empty hostname stderr not empty", - true, // hasNilErr bool - "command failed", // stdErr string - "", // hostName string - "failed to get hostname via ssh: StdErr:", // expectedErrMessage string - ), - Entry( - "hostname == rescue", - true, // hasNilErr bool - "", // stdErr string - "rescue", // hostName string - "", // expectedErrMessage string - ), - Entry( - "hostname == otherHostName", - true, // hasNilErr bool - "", // stdErr string - "otherHostName", // hostName string - "unexpected hostname", // expectedErrMessage string - ), + Entry("empty hostname", testCaseAnalyzeSSHOutputInstallImageWrongHostname{ + hasNilErr: true, + stdErr: "", + hostName: "", + expectedErrMessage: "hostname is empty", + }), + Entry("empty hostname - err not empty", testCaseAnalyzeSSHOutputInstallImageWrongHostname{ + hasNilErr: false, + stdErr: "", + hostName: "", + expectedErrMessage: "unhandled ssh error while getting hostname", + }), + Entry("empty hostname stderr not empty", testCaseAnalyzeSSHOutputInstallImageWrongHostname{ + hasNilErr: true, + stdErr: "command failed", + hostName: "", + expectedErrMessage: "failed to get hostname via ssh: StdErr:", + }), + Entry("hostname == rescue", testCaseAnalyzeSSHOutputInstallImageWrongHostname{ + hasNilErr: true, + stdErr: "", + hostName: "rescue", + expectedErrMessage: "", + }), + Entry("hostname == otherHostName", testCaseAnalyzeSSHOutputInstallImageWrongHostname{ + hasNilErr: true, + stdErr: "", + hostName: "otherHostName", + expectedErrMessage: "unexpected hostname", + }), ) }) var _ = Describe("analyzeSSHOutputProvisioned", func() { + type testCaseAnalyzeSSHOutputProvisioned struct { + out sshclient.Output + expectedIsTimeout bool + expectedIsConnectionRefused bool + expectedErrMessage *string + } + DescribeTable("analyzeSSHOutputProvisioned", - func(out sshclient.Output, - expectedIsTimeout bool, - expectedIsConnectionRefused bool, - expectedErrMessage *string, - ) { - isTimeout, isConnectionRefused, err := analyzeSSHOutputProvisioned(out) - Expect(isTimeout).To(Equal(expectedIsTimeout)) - Expect(isConnectionRefused).To(Equal(expectedIsConnectionRefused)) - if expectedErrMessage != nil { + func(tc testCaseAnalyzeSSHOutputProvisioned) { + isTimeout, isConnectionRefused, err := analyzeSSHOutputProvisioned(tc.out) + Expect(isTimeout).To(Equal(tc.expectedIsTimeout)) + Expect(isConnectionRefused).To(Equal(tc.expectedIsConnectionRefused)) + if tc.expectedErrMessage != nil { Expect(err).To(Not(BeNil())) - Expect(err.Error()).To(ContainSubstring(*expectedErrMessage)) + Expect(err.Error()).To(ContainSubstring(*tc.expectedErrMessage)) } else { Expect(err).To(BeNil()) } }, - Entry( - "incorrect boot", - sshclient.Output{StdOut: "wrong_hostname"}, // out sshclient.Output - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - pointer.String("unexpected hostname"), // expectedErrMessage *string - ), - Entry( - "timeout error", - sshclient.Output{Err: timeout}, // out sshclient.Output - true, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - nil, // expectedErrMessage *string - ), - Entry( - "stdErr non-empty", - sshclient.Output{StdErr: "some error"}, // out sshclient.Output - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - pointer.String("failed to get hostname via ssh: StdErr: some error"), // expectedErrMessage *string - ), - Entry( - "incorrect boot - empty hostname", - sshclient.Output{StdOut: ""}, // out sshclient.Output - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - pointer.String("hostname is empty"), // expectedErrMessage *string - ), - Entry( - "unable to authenticate", - sshclient.Output{Err: sshclient.ErrAuthenticationFailed}, // out sshclient.Output - false, // expectedIsTimeout bool - false, // expectedIsConnectionRefused bool - pointer.String("wrong ssh key"), // expectedErrMessage *string - ), - Entry( - "connection refused", - sshclient.Output{Err: sshclient.ErrConnectionRefused}, // out sshclient.Output - false, // expectedIsTimeout bool - true, // expectedIsConnectionRefused bool - nil, // expectedErrMessage *string - ), + Entry("incorrect boot", testCaseAnalyzeSSHOutputProvisioned{ + out: sshclient.Output{StdOut: "wrong_hostname"}, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: pointer.String("unexpected hostname"), + }), + Entry("timeout error", testCaseAnalyzeSSHOutputProvisioned{ + out: sshclient.Output{Err: timeout}, + expectedIsTimeout: true, + expectedIsConnectionRefused: false, + expectedErrMessage: nil, + }), + Entry("stdErr non-empty", testCaseAnalyzeSSHOutputProvisioned{ + out: sshclient.Output{StdErr: "some error"}, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: pointer.String("failed to get hostname via ssh: StdErr: some error"), + }), + Entry("incorrect boot - empty hostname", testCaseAnalyzeSSHOutputProvisioned{ + out: sshclient.Output{StdOut: ""}, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: pointer.String("hostname is empty"), + }), + Entry("unable to authenticate", testCaseAnalyzeSSHOutputProvisioned{ + out: sshclient.Output{Err: sshclient.ErrAuthenticationFailed}, + expectedIsTimeout: false, + expectedIsConnectionRefused: false, + expectedErrMessage: pointer.String("wrong ssh key"), + }), + Entry("connection refused", testCaseAnalyzeSSHOutputProvisioned{ + out: sshclient.Output{Err: sshclient.ErrConnectionRefused}, + expectedIsTimeout: false, + expectedIsConnectionRefused: true, + expectedErrMessage: nil, + }), ) }) var _ = Describe("actionRegistering", func() { + type testCaseActionRegistering struct { + storageStdOut string + includeRootDeviceHintWWN bool + includeRootDeviceHintRaid bool + expectedActionResult actionResult + expectedErrorMessage *string + } + DescribeTable("actionRegistering", - func( - storageStdOut string, - includeRootDeviceHintWWN bool, - includeRootDeviceHintRaid bool, - expectedActionResult actionResult, - expectedErrorMessage *string, - ) { + func(tc testCaseActionRegistering) { var host *infrav1.HetznerBareMetalHost - if includeRootDeviceHintWWN { + if tc.includeRootDeviceHintWWN { host = helpers.BareMetalHost( "test-host", "default", @@ -1046,7 +1041,7 @@ var _ = Describe("actionRegistering", func() { helpers.WithIPv4(), helpers.WithConsumerRef(), ) - } else if includeRootDeviceHintRaid { + } else if tc.includeRootDeviceHintRaid { host = helpers.BareMetalHost( "test-host", "default", @@ -1066,7 +1061,7 @@ var _ = Describe("actionRegistering", func() { sshMock.On("GetHostName").Return(sshclient.Output{StdOut: "rescue"}) sshMock.On("GetHardwareDetailsRAM").Return(sshclient.Output{StdOut: "10000"}) sshMock.On("GetHardwareDetailsStorage").Return(sshclient.Output{ - StdOut: storageStdOut, + StdOut: tc.storageStdOut, }) sshMock.On("GetHardwareDetailsNics").Return(sshclient.Output{ StdOut: `name="eth0" model="Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 15)" mac="a8:a1:59:94:19:42" ipv4="23.88.6.239/26" speedMbps="1000" @@ -1082,59 +1077,57 @@ var _ = Describe("actionRegistering", func() { service := newTestService(host, nil, bmmock.NewSSHFactory(sshMock, sshMock, sshMock), nil, helpers.GetDefaultSSHSecret(rescueSSHKeyName, "default")) actResult := service.actionRegistering() - Expect(actResult).Should(BeAssignableToTypeOf(expectedActionResult)) + Expect(actResult).Should(BeAssignableToTypeOf(tc.expectedActionResult)) Expect(host.Spec.Status.HardwareDetails).ToNot(BeNil()) - if expectedErrorMessage != nil { - Expect(host.Spec.Status.ErrorMessage).To(Equal(*expectedErrorMessage)) + if tc.expectedErrorMessage != nil { + Expect(host.Spec.Status.ErrorMessage).To(Equal(*tc.expectedErrorMessage)) } }, - Entry( - "working example", - `NAME="loop0" LABEL="" FSTYPE="ext2" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="0" + Entry("working example", testCaseActionRegistering{ + storageStdOut: `NAME="loop0" LABEL="" FSTYPE="ext2" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="0" 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"`, - true, - false, - actionComplete{}, - nil, - ), - Entry( - "working example - rootDeviceHints raid", - `NAME="loop0" LABEL="" FSTYPE="ext2" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="0" + includeRootDeviceHintWWN: true, + includeRootDeviceHintRaid: false, + expectedActionResult: actionComplete{}, + expectedErrorMessage: nil, + }), + Entry("working example - rootDeviceHints raid", testCaseActionRegistering{ + storageStdOut: `NAME="loop0" LABEL="" FSTYPE="ext2" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="0" 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"`, - false, - true, - actionComplete{}, - nil, - ), - Entry( - "wwn does not fit to storage devices", - `NAME="loop0" LABEL="" FSTYPE="ext2" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="0" + includeRootDeviceHintWWN: false, + includeRootDeviceHintRaid: true, + expectedActionResult: actionComplete{}, + expectedErrorMessage: nil, + }), + Entry("wwn does not fit to storage devices", testCaseActionRegistering{ + storageStdOut: `NAME="loop0" LABEL="" FSTYPE="ext2" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="0" 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"`, - true, - false, - actionFailed{}, - pointer.String("missing storage device for root device hint eui.002538b411b2cee8"), - ), - Entry( - "no root device hints", - `NAME="loop0" LABEL="" FSTYPE="ext2" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="0" + includeRootDeviceHintWWN: true, + includeRootDeviceHintRaid: false, + expectedActionResult: actionFailed{}, + expectedErrorMessage: pointer.String("missing storage device for root device hint eui.002538b411b2cee8"), + }), + Entry("no root device hints", testCaseActionRegistering{ + storageStdOut: `NAME="loop0" LABEL="" FSTYPE="ext2" TYPE="loop" HCTL="" MODEL="" VENDOR="" SERIAL="" SIZE="3068773888" WWN="" ROTA="0" 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"`, - false, - false, - actionFailed{}, - pointer.String(infrav1.ErrorMessageMissingRootDeviceHints), - ), + includeRootDeviceHintWWN: false, + includeRootDeviceHintRaid: false, + expectedActionResult: actionFailed{}, + expectedErrorMessage: pointer.String(infrav1.ErrorMessageMissingRootDeviceHints), + }), ) + type testCaseActionRegisteringIncompleteBoot struct { + getHostNameOutput sshclient.Output + expectedErrorType infrav1.ErrorType + } + DescribeTable("actionRegistering - incomplete reboot", - func( - getHostNameOutput sshclient.Output, - expectedErrorType infrav1.ErrorType, - ) { + func(tc testCaseActionRegisteringIncompleteBoot) { host := helpers.BareMetalHost( "test-host", "default", @@ -1145,7 +1138,7 @@ var _ = Describe("actionRegistering", func() { ) sshMock := &sshmock.Client{} - sshMock.On("GetHostName").Return(getHostNameOutput) + sshMock.On("GetHostName").Return(tc.getHostNameOutput) robotMock := robotmock.Client{} robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: false}, nil) @@ -1154,95 +1147,91 @@ var _ = Describe("actionRegistering", func() { actResult := service.actionRegistering() Expect(actResult).Should(BeAssignableToTypeOf(actionContinue{})) - if expectedErrorType != infrav1.ErrorType("") { - Expect(host.Spec.Status.ErrorType).To(Equal(expectedErrorType)) + if tc.expectedErrorType != infrav1.ErrorType("") { + Expect(host.Spec.Status.ErrorType).To(Equal(tc.expectedErrorType)) } }, - Entry( - "timeout", - sshclient.Output{Err: timeout}, // getHostNameOutput sshclient.Output - infrav1.ErrorTypeSSHRebootTriggered, // expectedErrorType string - ), - Entry( - "connectionRefused", - sshclient.Output{Err: sshclient.ErrConnectionRefused}, // getHostNameOutput sshclient.Output - infrav1.ErrorTypeConnectionError, // expectedErrorType string - ), + Entry("timeout", testCaseActionRegisteringIncompleteBoot{ + getHostNameOutput: sshclient.Output{Err: timeout}, + expectedErrorType: infrav1.ErrorTypeSSHRebootTriggered, + }), + Entry("connectionRefused", testCaseActionRegisteringIncompleteBoot{ + getHostNameOutput: sshclient.Output{Err: sshclient.ErrConnectionRefused}, + expectedErrorType: infrav1.ErrorTypeConnectionError, + }), ) }) var _ = Describe("getImageDetails", func() { + type testCaseGetImageDetails struct { + image infrav1.Image + expectedImagePath string + expectedNeedsDownload bool + expectedErrorMessage string + } + DescribeTable("getImageDetails", - func(image infrav1.Image, - expectedImagePath string, - expectedNeedsDownload bool, - expectedErrorMessage string, - ) { - imagePath, needsDownload, errorMessage := image.GetDetails() - Expect(imagePath).To(Equal(expectedImagePath)) - Expect(needsDownload).To(Equal(expectedNeedsDownload)) - Expect(errorMessage).To(Equal(expectedErrorMessage)) + func(tc testCaseGetImageDetails) { + imagePath, needsDownload, errorMessage := tc.image.GetDetails() + Expect(imagePath).To(Equal(tc.expectedImagePath)) + Expect(needsDownload).To(Equal(tc.expectedNeedsDownload)) + Expect(errorMessage).To(Equal(tc.expectedErrorMessage)) }, - Entry( - "name and url specified, tar.gz suffix", - infrav1.Image{ + Entry("name and url specified, tar.gz suffix", testCaseGetImageDetails{ + image: infrav1.Image{ Name: "imageName", URL: "https://mytargz.tar.gz", Path: "", - }, // image infrav1.Image - "/root/imageName.tar.gz", // expectedImagePath string - true, // expectedNeedsDownload bool - "", // expectedErrorMessage string - ), - Entry( - "name and url specified, tgz suffix", - infrav1.Image{ + }, + expectedImagePath: "/root/imageName.tar.gz", + expectedNeedsDownload: true, + expectedErrorMessage: "", + }), + Entry("name and url specified, tgz suffix", testCaseGetImageDetails{ + image: infrav1.Image{ Name: "imageName", URL: "https://mytargz.tgz", Path: "", - }, // image infrav1.Image - "/root/imageName.tgz", // expectedImagePath string - true, // expectedNeedsDownload bool - "", // expectedErrorMessage string - ), - Entry( - "name and url specified, wrong suffix", - infrav1.Image{ + }, + expectedImagePath: "/root/imageName.tgz", + expectedNeedsDownload: true, + expectedErrorMessage: "", + }), + Entry("name and url specified, wrong suffix", testCaseGetImageDetails{ + image: infrav1.Image{ Name: "imageName", URL: "https://mytargz.tgx", Path: "", - }, // image infrav1.Image - "", // expectedImagePath string - false, // expectedNeedsDownload bool - "wrong image url suffix", // expectedErrorMessage string - ), - Entry( - "path specified", - infrav1.Image{ + }, + expectedImagePath: "", + expectedNeedsDownload: false, + expectedErrorMessage: "wrong image url suffix", + }), + Entry("path specified", testCaseGetImageDetails{ + image: infrav1.Image{ Name: "", URL: "", Path: "imagePath", - }, // image infrav1.Image - "imagePath", // expectedImagePath string - false, // expectedNeedsDownload bool - "", // expectedErrorMessage string - ), - Entry( - "neither specified", - infrav1.Image{ + }, + expectedImagePath: "imagePath", + expectedNeedsDownload: false, + expectedErrorMessage: "", + }), + Entry("neither specified", testCaseGetImageDetails{ + image: infrav1.Image{ Name: "imageName", URL: "", Path: "", - }, // image infrav1.Image - "", // expectedImagePath string - false, // expectedNeedsDownload bool - "invalid image - need to specify either name and url or path", // expectedErrorMessage string - ), + }, + expectedImagePath: "", + expectedNeedsDownload: false, + expectedErrorMessage: "invalid image - need to specify either name and url or path", + }), ) }) var _ = Describe("actionEnsureProvisioned", func() { - type ensureProvisionedInputs struct { + type testCaseActionEnsureProvisioned struct { outSSHClientGetHostName sshclient.Output outSSHClientCloudInitStatus sshclient.Output outSSHClientCheckSigterm sshclient.Output @@ -1258,8 +1247,9 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallCheckSigterm bool expectsOldSSHClientCallReboot bool } + DescribeTable("actionEnsureProvisioned", - func(in ensureProvisionedInputs) { + func(in testCaseActionEnsureProvisioned) { var ( portAfterCloudInit = 24 portAfterInstallImage = 23 @@ -1332,9 +1322,8 @@ var _ = Describe("actionEnsureProvisioned", func() { Expect(oldSSHMock.AssertNotCalled(GinkgoT(), "Reboot")).To(BeTrue()) } }, - Entry( - "correct hostname, cloud init running", - ensureProvisionedInputs{ + Entry("correct hostname, cloud init running", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{StdOut: infrav1.BareMetalHostNamePrefix + "bm-machine"}, outSSHClientCloudInitStatus: sshclient.Output{StdOut: "status: running"}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1351,9 +1340,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "correct hostname, cloud init done, no SIGTERM", - ensureProvisionedInputs{ + Entry("correct hostname, cloud init done, no SIGTERM", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{StdOut: infrav1.BareMetalHostNamePrefix + "bm-machine"}, outSSHClientCloudInitStatus: sshclient.Output{StdOut: "status: done"}, outSSHClientCheckSigterm: sshclient.Output{StdOut: ""}, @@ -1370,9 +1358,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "correct hostname, cloud init done, SIGTERM", - ensureProvisionedInputs{ + Entry("correct hostname, cloud init done, SIGTERM", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{StdOut: infrav1.BareMetalHostNamePrefix + "bm-machine"}, outSSHClientCloudInitStatus: sshclient.Output{StdOut: "status: done"}, outSSHClientCheckSigterm: sshclient.Output{StdOut: "found SIGTERM in cloud init output logs"}, @@ -1389,9 +1376,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "correct hostname, cloud init error", - ensureProvisionedInputs{ + Entry("correct hostname, cloud init error", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{StdOut: infrav1.BareMetalHostNamePrefix + "bm-machine"}, outSSHClientCloudInitStatus: sshclient.Output{StdOut: "status: error"}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1408,9 +1394,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "correct hostname, cloud init disabled", - ensureProvisionedInputs{ + Entry("correct hostname, cloud init disabled", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{StdOut: infrav1.BareMetalHostNamePrefix + "bm-machine"}, outSSHClientCloudInitStatus: sshclient.Output{StdOut: "status: disabled"}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1427,9 +1412,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "connectionFailed, same ports", - ensureProvisionedInputs{ + Entry("connectionFailed, same ports", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{Err: sshclient.ErrConnectionRefused}, outSSHClientCloudInitStatus: sshclient.Output{}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1446,9 +1430,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "connectionFailed, different ports, connectionFailed of oldSSHClient", - ensureProvisionedInputs{ + Entry("connectionFailed, different ports, connectionFailed of oldSSHClient", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{Err: sshclient.ErrConnectionRefused}, outSSHClientCloudInitStatus: sshclient.Output{}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1465,9 +1448,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "connectionFailed, different ports, status running of oldSSHClient", - ensureProvisionedInputs{ + Entry("connectionFailed, different ports, status running of oldSSHClient", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{Err: sshclient.ErrConnectionRefused}, outSSHClientCloudInitStatus: sshclient.Output{}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1484,9 +1466,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "connectionFailed, different ports, status error of oldSSHClient", - ensureProvisionedInputs{ + Entry("connectionFailed, different ports, status error of oldSSHClient", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{Err: sshclient.ErrConnectionRefused}, outSSHClientCloudInitStatus: sshclient.Output{}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1503,9 +1484,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "connectionFailed, different ports, status disabled of oldSSHClient", - ensureProvisionedInputs{ + Entry("connectionFailed, different ports, status disabled of oldSSHClient", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{Err: sshclient.ErrConnectionRefused}, outSSHClientCloudInitStatus: sshclient.Output{}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1522,9 +1502,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: true, }, ), - Entry( - "connectionFailed, different ports, status done of oldSSHClient, SIGTERM of oldSSHClient", - ensureProvisionedInputs{ + Entry("connectionFailed, different ports, status done of oldSSHClient, SIGTERM of oldSSHClient", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{Err: sshclient.ErrConnectionRefused}, outSSHClientCloudInitStatus: sshclient.Output{}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1541,9 +1520,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: true, }, ), - Entry( - "connectionFailed, different ports, status done of oldSSHClient, no SIGTERM of oldSSHClient", - ensureProvisionedInputs{ + Entry("connectionFailed, different ports, status done of oldSSHClient, no SIGTERM of oldSSHClient", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{Err: sshclient.ErrConnectionRefused}, outSSHClientCloudInitStatus: sshclient.Output{}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1560,9 +1538,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "connectionFailed, different ports, timeout of oldSSHClient", - ensureProvisionedInputs{ + Entry("connectionFailed, different ports, timeout of oldSSHClient", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{Err: sshclient.ErrConnectionRefused}, outSSHClientCloudInitStatus: sshclient.Output{}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1579,9 +1556,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "correct hostname, cloud init done, no SIGTERM, ports different", - ensureProvisionedInputs{ + Entry("correct hostname, cloud init done, no SIGTERM, ports different", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{StdOut: infrav1.BareMetalHostNamePrefix + "bm-machine"}, outSSHClientCloudInitStatus: sshclient.Output{StdOut: "status: done"}, outSSHClientCheckSigterm: sshclient.Output{StdOut: ""}, @@ -1598,9 +1574,8 @@ var _ = Describe("actionEnsureProvisioned", func() { expectsOldSSHClientCallReboot: false, }, ), - Entry( - "timeout of sshclient", - ensureProvisionedInputs{ + Entry("timeout of sshclient", + testCaseActionEnsureProvisioned{ outSSHClientGetHostName: sshclient.Output{Err: timeout}, outSSHClientCloudInitStatus: sshclient.Output{}, outSSHClientCheckSigterm: sshclient.Output{}, @@ -1621,15 +1596,17 @@ var _ = Describe("actionEnsureProvisioned", func() { }) var _ = Describe("actionProvisioned", func() { + type testCaseActionProvisioned struct { + shouldHaveRebootAnnotation bool + rebooted bool + rebootFinished bool + expectedActionResult actionResult + expectRebootAnnotation bool + expectRebootInStatus bool + } + DescribeTable("actionProvisioned", - func( - shouldHaveRebootAnnotation bool, - rebooted bool, - rebootFinished bool, - expectedActionResult actionResult, - expectRebootAnnotation bool, - expectRebootInStatus bool, - ) { + func(tc testCaseActionProvisioned) { host := helpers.BareMetalHost( "test-host", "default", @@ -1638,15 +1615,15 @@ var _ = Describe("actionProvisioned", func() { helpers.WithConsumerRef(), ) - if shouldHaveRebootAnnotation { + if tc.shouldHaveRebootAnnotation { host.SetAnnotations(map[string]string{infrav1.RebootAnnotation: "reboot"}) } - host.Spec.Status.Rebooted = rebooted + host.Spec.Status.Rebooted = tc.rebooted sshMock := &sshmock.Client{} var hostNameOutput sshclient.Output - if rebootFinished { + if tc.rebootFinished { hostNameOutput = sshclient.Output{StdOut: infrav1.BareMetalHostNamePrefix + host.Spec.ConsumerRef.Name} } else { hostNameOutput = sshclient.Output{Err: timeout} @@ -1657,51 +1634,47 @@ var _ = Describe("actionProvisioned", func() { service := newTestService(host, nil, bmmock.NewSSHFactory(sshMock, sshMock, sshMock), helpers.GetDefaultSSHSecret(osSSHKeyName, "default"), helpers.GetDefaultSSHSecret(rescueSSHKeyName, "default")) actResult := service.actionProvisioned() - Expect(actResult).Should(BeAssignableToTypeOf(expectedActionResult)) - Expect(host.Spec.Status.Rebooted).To(Equal(expectRebootInStatus)) - Expect(host.HasRebootAnnotation()).To(Equal(expectRebootAnnotation)) + Expect(actResult).Should(BeAssignableToTypeOf(tc.expectedActionResult)) + Expect(host.Spec.Status.Rebooted).To(Equal(tc.expectRebootInStatus)) + Expect(host.HasRebootAnnotation()).To(Equal(tc.expectRebootAnnotation)) - if shouldHaveRebootAnnotation && !rebooted { + if tc.shouldHaveRebootAnnotation && !tc.rebooted { Expect(sshMock.AssertCalled(GinkgoT(), "Reboot")).To(BeTrue()) } else { Expect(sshMock.AssertNotCalled(GinkgoT(), "Reboot")).To(BeTrue()) } }, - Entry( - "reboot desired, but not performed yet", - true, // shouldHaveRebootAnnotation bool - false, // rebooted bool - false, // rebootFinished bool - actionContinue{}, // expectedActionResult actionResult - true, // expectRebootAnnotation bool - true, // expectRebootInStatus bool, - ), - Entry( - "reboot desired, and already performed, not finished", - true, // shouldHaveRebootAnnotation bool - true, // rebooted bool - false, // rebootFinished bool - actionContinue{}, // expectedActionResult actionResult - true, // expectRebootAnnotation bool - true, // expectRebootInStatus bool, - ), - Entry( - "reboot desired, and already performed, finished", - true, // shouldHaveRebootAnnotation bool - true, // rebooted bool - true, // rebootFinished bool - actionComplete{}, // expectedActionResult actionResult - false, // expectRebootAnnotation bool - false, // expectRebootInStatus bool, - ), - Entry( - "no reboot desired", - false, // shouldHaveRebootAnnotation bool - false, // rebooted bool - false, // rebootFinished bool - actionComplete{}, // expectedActionResult actionResult - false, // expectRebootAnnotation bool - false, // expectRebootInStatus bool, - ), + Entry("reboot desired, but not performed yet", testCaseActionProvisioned{ + shouldHaveRebootAnnotation: true, + rebooted: false, + rebootFinished: false, + expectedActionResult: actionContinue{}, + expectRebootAnnotation: true, + expectRebootInStatus: true, + }), + Entry("reboot desired, and already performed, not finished", testCaseActionProvisioned{ + shouldHaveRebootAnnotation: true, + rebooted: true, + rebootFinished: false, + expectedActionResult: actionContinue{}, + expectRebootAnnotation: true, + expectRebootInStatus: true, + }), + Entry("reboot desired, and already performed, finished", testCaseActionProvisioned{ + shouldHaveRebootAnnotation: true, + rebooted: true, + rebootFinished: true, + expectedActionResult: actionComplete{}, + expectRebootAnnotation: false, + expectRebootInStatus: false, + }), + Entry("no reboot desired", testCaseActionProvisioned{ + shouldHaveRebootAnnotation: false, + rebooted: false, + rebootFinished: false, + expectedActionResult: actionComplete{}, + expectRebootAnnotation: false, + expectRebootInStatus: false, + }), ) }) diff --git a/pkg/services/baremetal/host/state_machine_test.go b/pkg/services/baremetal/host/state_machine_test.go index 08d8cac3b..a2b5819be 100644 --- a/pkg/services/baremetal/host/state_machine_test.go +++ b/pkg/services/baremetal/host/state_machine_test.go @@ -28,16 +28,18 @@ import ( ) var _ = Describe("updateSSHKey", func() { + type testCaseUpdateSSHKey struct { + osSecretData map[string][]byte + rescueSecretData map[string][]byte + currentState infrav1.ProvisioningState + expectedActionResult actionResult + expectedNextState infrav1.ProvisioningState + expectedOSSecretData map[string][]byte + expectedRescueSecretData map[string][]byte + } + DescribeTable("updateSSHKey", - func( - osSecretData map[string][]byte, - rescueSecretData map[string][]byte, - currentState infrav1.ProvisioningState, - expectedActionResult actionResult, - expectedNextState infrav1.ProvisioningState, - expectedOSSecretData map[string][]byte, - expectedRescueSecretData map[string][]byte, - ) { + func(tc testCaseUpdateSSHKey) { host := helpers.BareMetalHost( "test-host", "default", @@ -45,16 +47,16 @@ var _ = Describe("updateSSHKey", func() { helpers.WithSSHSpecInclPorts(23, 24), ) - dataHashOS, err := infrav1.HashOfSecretData(osSecretData) + dataHashOS, err := infrav1.HashOfSecretData(tc.osSecretData) Expect(err).To(BeNil()) - dataHashRescue, err := infrav1.HashOfSecretData(rescueSecretData) + dataHashRescue, err := infrav1.HashOfSecretData(tc.rescueSecretData) Expect(err).To(BeNil()) - expectedDataHashOS, err := infrav1.HashOfSecretData(expectedOSSecretData) + expectedDataHashOS, err := infrav1.HashOfSecretData(tc.expectedOSSecretData) Expect(err).To(BeNil()) - expectedDataHashRescue, err := infrav1.HashOfSecretData(expectedRescueSecretData) + expectedDataHashRescue, err := infrav1.HashOfSecretData(tc.expectedRescueSecretData) Expect(err).To(BeNil()) host.Spec.Status.SSHStatus.CurrentOS = &infrav1.SecretStatus{ @@ -71,7 +73,7 @@ var _ = Describe("updateSSHKey", func() { }, DataHash: dataHashRescue, } - host.Spec.Status.ProvisioningState = currentState + host.Spec.Status.ProvisioningState = tc.currentState osSSHSecret := helpers.GetDefaultSSHSecret(osSSHKeyName, "default") osSSHSecret.ObjectMeta.ResourceVersion = "1" @@ -84,7 +86,7 @@ var _ = Describe("updateSSHKey", func() { actResult := hsm.updateSSHKey() - Expect(actResult).Should(BeAssignableToTypeOf(expectedActionResult)) + Expect(actResult).Should(BeAssignableToTypeOf(tc.expectedActionResult)) Expect(*host.Spec.Status.SSHStatus.CurrentRescue).Should(Equal(infrav1.SecretStatus{ Reference: &corev1.SecretReference{ Name: rescueSSHKeyName, @@ -99,163 +101,157 @@ var _ = Describe("updateSSHKey", func() { }, DataHash: expectedDataHashOS, })) - Expect(hsm.nextState).Should(Equal(expectedNextState)) + Expect(hsm.nextState).Should(Equal(tc.expectedNextState)) }, - Entry( - "nothing changed", - map[string][]byte{ + Entry("nothing changed", testCaseUpdateSSHKey{ + osSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // osSecretData string - map[string][]byte{ + }, + rescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // rescueSecretData string - infrav1.StateRegistering, // currentState infrav1.ProvisioningState - actionComplete{}, // expectedActionResult actionResult - infrav1.StateRegistering, // expectedNextState infrav1.ProvisioningState - map[string][]byte{ + }, + currentState: infrav1.StateRegistering, + expectedActionResult: actionComplete{}, + expectedNextState: infrav1.StateRegistering, + expectedOSSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedOSSecretData string - map[string][]byte{ + }, + expectedRescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedRescueSecretData string - ), - Entry( - "os secret changed - state available", - map[string][]byte{ + }, + }), + Entry("os secret changed - state available", testCaseUpdateSSHKey{ + osSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-old-name"), "public-key": []byte("my-old-public-key"), - }, // osSecretData string - map[string][]byte{ + }, + rescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // rescueSecretData string - infrav1.StateRegistering, // currentState infrav1.ProvisioningState - actionComplete{}, // expectedActionResult actionResult - infrav1.StateRegistering, // expectedNextState infrav1.ProvisioningState - map[string][]byte{ + }, + currentState: infrav1.StateRegistering, + expectedActionResult: actionComplete{}, + expectedNextState: infrav1.StateRegistering, + expectedOSSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedOSSecretData string - map[string][]byte{ + }, + expectedRescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedRescueSecretData string - ), - Entry( - "os secret changed - state provisioned", - map[string][]byte{ + }, + }), + Entry("os secret changed - state provisioned", testCaseUpdateSSHKey{ + osSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-old-name"), "public-key": []byte("my-old-public-key"), - }, // osSecretData string - map[string][]byte{ + }, + rescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // rescueSecretData string - infrav1.StateProvisioned, // currentState infrav1.ProvisioningState - actionFailed{}, // expectedActionResult actionResult - infrav1.StateProvisioned, // expectedNextState infrav1.ProvisioningState - map[string][]byte{ + }, + currentState: infrav1.StateProvisioned, + expectedActionResult: actionFailed{}, + expectedNextState: infrav1.StateProvisioned, + expectedOSSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-old-name"), "public-key": []byte("my-old-public-key"), - }, // expectedOSSecretData string - map[string][]byte{ + }, + expectedRescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedRescueSecretData string - ), - Entry( - "os secret changed - state provisioning", - map[string][]byte{ + }, + }), + Entry("os secret changed - state provisioning", testCaseUpdateSSHKey{ + osSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-old-name"), "public-key": []byte("my-old-public-key"), - }, // osSecretData string - map[string][]byte{ + }, + rescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // rescueSecretData string - infrav1.StateProvisioning, // currentState infrav1.ProvisioningState - actionComplete{}, // expectedActionResult actionResult - infrav1.StateImageInstalling, // expectedNextState infrav1.ProvisioningState - map[string][]byte{ + }, + currentState: infrav1.StateProvisioning, + expectedActionResult: actionComplete{}, + expectedNextState: infrav1.StateImageInstalling, + expectedOSSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedOSSecretData string - map[string][]byte{ + }, + expectedRescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedRescueSecretData string - ), - Entry( - "rescue secret changed - state provisioning", - map[string][]byte{ + }, + }), + Entry("rescue secret changed - state provisioning", testCaseUpdateSSHKey{ + osSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // osSecretData string - map[string][]byte{ + }, + rescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-old-name"), "public-key": []byte("my-old-public-key"), - }, // rescueSecretData string - infrav1.StateProvisioning, // currentState infrav1.ProvisioningState - actionComplete{}, // expectedActionResult actionResult - infrav1.StateProvisioning, // expectedNextState infrav1.ProvisioningState - map[string][]byte{ + }, + currentState: infrav1.StateProvisioning, + expectedActionResult: actionComplete{}, + expectedNextState: infrav1.StateProvisioning, + expectedOSSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedOSSecretData string - map[string][]byte{ + }, + expectedRescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedRescueSecretData string - ), - Entry( - "rescue secret changed - state available", - map[string][]byte{ + }, + }), + Entry("rescue secret changed - state available", testCaseUpdateSSHKey{ + osSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // osSecretData string - map[string][]byte{ + }, + rescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-old-name"), "public-key": []byte("my-old-public-key"), - }, // rescueSecretData string - infrav1.StateRegistering, // currentState infrav1.ProvisioningState - actionComplete{}, // expectedActionResult actionResult - infrav1.StateNone, // expectedNextState infrav1.ProvisioningState - map[string][]byte{ + }, + currentState: infrav1.StateRegistering, + expectedActionResult: actionComplete{}, + expectedNextState: infrav1.StateNone, + expectedOSSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", osSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedOSSecretData string - map[string][]byte{ + }, + expectedRescueSecretData: map[string][]byte{ "private-key": []byte(fmt.Sprintf("%s-private-key", rescueSSHKeyName)), "sshkey-name": []byte("my-name"), "public-key": []byte("my-public-key"), - }, // expectedRescueSecretData string - ), + }, + }), ) }) diff --git a/pkg/services/baremetal/host/utils_test.go b/pkg/services/baremetal/host/utils_test.go index 474e515b7..0065594e9 100644 --- a/pkg/services/baremetal/host/utils_test.go +++ b/pkg/services/baremetal/host/utils_test.go @@ -24,13 +24,17 @@ import ( ) var _ = Describe("buildAutoSetup", func() { + type testCaseBuildAutoSetup struct { + installImageSpec *infrav1.InstallImage + autoSetupInput autoSetupInput + expectedOutput string + } DescribeTable("buildAutoSetup", - func(installImageSpec *infrav1.InstallImage, autoSetupInput autoSetupInput, expectedOutput string) { - Expect(buildAutoSetup(installImageSpec, autoSetupInput)).Should(Equal(expectedOutput)) + func(tc testCaseBuildAutoSetup) { + Expect(buildAutoSetup(tc.installImageSpec, tc.autoSetupInput)).Should(Equal(tc.expectedOutput)) }, - Entry( - "multiple entries", - &infrav1.InstallImage{ + Entry("multiple entries", testCaseBuildAutoSetup{ + installImageSpec: &infrav1.InstallImage{ Partitions: []infrav1.Partition{ { Mount: "/boot", @@ -74,12 +78,12 @@ var _ = Describe("buildAutoSetup", func() { Swraid: 0, SwraidLevel: 1, }, - autoSetupInput{ + autoSetupInput: autoSetupInput{ image: "my-image", osDevices: []string{"device"}, hostName: "my-host", }, - `DRIVE1 /dev/device + expectedOutput: `DRIVE1 /dev/device HOSTNAME my-host SWRAID 0 @@ -93,10 +97,10 @@ LV vg0 swap swap swap 5G SUBVOL btrfs.1 @ / SUBVOL btrfs.1 @/usr /usr -IMAGE my-image`), - Entry( - "single entries", - &infrav1.InstallImage{ +IMAGE my-image`, + }), + Entry("single entries", testCaseBuildAutoSetup{ + installImageSpec: &infrav1.InstallImage{ Partitions: []infrav1.Partition{ { Mount: "/boot", @@ -123,12 +127,12 @@ IMAGE my-image`), Swraid: 1, SwraidLevel: 1, }, - autoSetupInput{ + autoSetupInput: autoSetupInput{ image: "my-image", osDevices: []string{"device"}, hostName: "my-host", }, - `DRIVE1 /dev/device + expectedOutput: `DRIVE1 /dev/device HOSTNAME my-host SWRAID 1 @@ -140,10 +144,10 @@ LV vg0 root / ext4 10G SUBVOL btrfs.1 @ / -IMAGE my-image`), - Entry( - "multiple drives", - &infrav1.InstallImage{ +IMAGE my-image`, + }), + Entry("multiple drives", testCaseBuildAutoSetup{ + installImageSpec: &infrav1.InstallImage{ Partitions: []infrav1.Partition{ { Mount: "/boot", @@ -170,12 +174,12 @@ IMAGE my-image`), Swraid: 0, SwraidLevel: 1, }, - autoSetupInput{ + autoSetupInput: autoSetupInput{ image: "my-image", osDevices: []string{"device1", "device2"}, hostName: "my-host", }, - `DRIVE1 /dev/device1 + expectedOutput: `DRIVE1 /dev/device1 DRIVE2 /dev/device2 HOSTNAME my-host @@ -187,10 +191,10 @@ LV vg0 root / ext4 10G SUBVOL btrfs.1 @ / -IMAGE my-image`), - Entry( - "proper response", - &infrav1.InstallImage{ +IMAGE my-image`, + }), + Entry("proper response", testCaseBuildAutoSetup{ + installImageSpec: &infrav1.InstallImage{ Partitions: []infrav1.Partition{ { Mount: "/boot", @@ -203,12 +207,12 @@ IMAGE my-image`), Swraid: 0, SwraidLevel: 1, }, - autoSetupInput{ + autoSetupInput: autoSetupInput{ image: "my-image", osDevices: []string{"device"}, hostName: "my-host", }, - `DRIVE1 /dev/device + expectedOutput: `DRIVE1 /dev/device HOSTNAME my-host SWRAID 0 @@ -217,28 +221,30 @@ PART /boot ext2 512M -IMAGE my-image`), +IMAGE my-image`, + }), ) }) var _ = Describe("validJSONFromSSHOutput", func() { - DescribeTable("validJSONFromSSHOutput", func(input, expectedOutput string) { - Expect(validJSONFromSSHOutput(input)).Should(Equal(expectedOutput)) + type testCaseValidJSONFromSSHOutput struct { + input string + expectedOutput string + } + DescribeTable("validJSONFromSSHOutput", func(tc testCaseValidJSONFromSSHOutput) { + Expect(validJSONFromSSHOutput(tc.input)).Should(Equal(tc.expectedOutput)) }, - Entry( - "working example", - `key1="string1" key2="string2" key3="string3"`, - `{"key1":"string1","key2":"string2","key3":"string3"}`, - ), - Entry( - "working example2", - `key1="string1"`, - `{"key1":"string1"}`, - ), - Entry( - "empty string", - ``, - `{}`, - ), + Entry("working example", testCaseValidJSONFromSSHOutput{ + input: `key1="string1" key2="string2" key3="string3"`, + expectedOutput: `{"key1":"string1","key2":"string2","key3":"string3"}`, + }), + Entry("working example2", testCaseValidJSONFromSSHOutput{ + input: `key1="string1"`, + expectedOutput: `{"key1":"string1"}`, + }), + Entry("empty string", testCaseValidJSONFromSSHOutput{ + input: ``, + expectedOutput: `{}`, + }), ) }) diff --git a/pkg/services/hcloud/server/server_test.go b/pkg/services/hcloud/server/server_test.go index 5a8abca8e..576fe1ab8 100644 --- a/pkg/services/hcloud/server/server_test.go +++ b/pkg/services/hcloud/server/server_test.go @@ -57,17 +57,22 @@ var _ = Describe("statusFromHCloudServer", func() { }) }) +type testCaseStatusFromHCloudServer struct { + isControlPlane bool + expectedOutput map[string]string +} + var _ = DescribeTable("createLabels", - func(hetznerClusterName, hcloudMachineName string, isControlPlane bool, expectedOutput map[string]string) { + func(tc testCaseStatusFromHCloudServer) { hcloudMachine := infrav1.HCloudMachine{} - hcloudMachine.Name = hcloudMachineName + hcloudMachine.Name = "hcloudMachine" hetznerCluster := infrav1.HetznerCluster{} - hetznerCluster.Name = hetznerClusterName + hetznerCluster.Name = "hcloudCluster" capiMachine := clusterv1.Machine{} - if isControlPlane { + if tc.isControlPlane { // set label on capi machine to mark it as control plane capiMachine.Labels = make(map[string]string) capiMachine.Labels[clusterv1.MachineControlPlaneLabel] = "control-plane" @@ -82,21 +87,32 @@ var _ = DescribeTable("createLabels", }, }, } - Expect(service.createLabels()).To(Equal(expectedOutput)) + Expect(service.createLabels()).To(Equal(tc.expectedOutput)) }, - Entry("is_controlplane", "hcloudCluster", "hcloudMachine", true, map[string]string{ - "caph-cluster-hcloudCluster": string(infrav1.ResourceLifecycleOwned), - infrav1.MachineNameTagKey: "hcloudMachine", - "machine_type": "control_plane", + Entry("is_controlplane", testCaseStatusFromHCloudServer{ + isControlPlane: true, + expectedOutput: map[string]string{ + "caph-cluster-hcloudCluster": string(infrav1.ResourceLifecycleOwned), + infrav1.MachineNameTagKey: "hcloudMachine", + "machine_type": "control_plane", + }, }), - Entry("is_worker", "hcloudCluster", "hcloudMachine", false, map[string]string{ - "caph-cluster-hcloudCluster": string(infrav1.ResourceLifecycleOwned), - infrav1.MachineNameTagKey: "hcloudMachine", - "machine_type": "worker", + Entry("is_worker", testCaseStatusFromHCloudServer{ + isControlPlane: false, + expectedOutput: map[string]string{ + "caph-cluster-hcloudCluster": string(infrav1.ResourceLifecycleOwned), + infrav1.MachineNameTagKey: "hcloudMachine", + "machine_type": "worker", + }, }), ) var _ = Describe("filterHCloudSSHKeys", func() { + type testCaseFilterHCloudSSHKeys struct { + sshKeysSpec []infrav1.SSHKey + expectedOutput []*hcloud.SSHKey + } + var sshKeysAPI []*hcloud.SSHKey BeforeEach(func() { sshKeysAPI = []*hcloud.SSHKey{ @@ -118,58 +134,64 @@ var _ = Describe("filterHCloudSSHKeys", func() { } }) _ = DescribeTable("no_error", - func(sshKeysSpec []infrav1.SSHKey, expectedOutput []*hcloud.SSHKey) { - Expect(filterHCloudSSHKeys(sshKeysAPI, sshKeysSpec)).Should(Equal(expectedOutput)) + func(tc testCaseFilterHCloudSSHKeys) { + Expect(filterHCloudSSHKeys(sshKeysAPI, tc.sshKeysSpec)).Should(Equal(tc.expectedOutput)) }, - Entry("no_error_same_length", []infrav1.SSHKey{ - { - Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:2f", - Name: "sshkey1", - }, - { - Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:3g", - Name: "sshkey2", - }, - { - Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:4f", - Name: "sshkey3", - }, - }, []*hcloud.SSHKey{ - { - Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:2f", - Name: "sshkey1", - ID: 42, - }, - { - Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:3g", - Name: "sshkey2", - ID: 43, + Entry("no_error_same_length", testCaseFilterHCloudSSHKeys{ + sshKeysSpec: []infrav1.SSHKey{ + { + Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:2f", + Name: "sshkey1", + }, + { + Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:3g", + Name: "sshkey2", + }, + { + Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:4f", + Name: "sshkey3", + }, }, - { - Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:4h", - Name: "sshkey3", - ID: 44, + expectedOutput: []*hcloud.SSHKey{ + { + Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:2f", + Name: "sshkey1", + ID: 42, + }, + { + Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:3g", + Name: "sshkey2", + ID: 43, + }, + { + Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:4h", + Name: "sshkey3", + ID: 44, + }, }, }), - Entry("no_error_different_length", []infrav1.SSHKey{ - { - Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:2f", - Name: "sshkey1", - }, - { - Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:4f", - Name: "sshkey3", - }, - }, []*hcloud.SSHKey{ - { - Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:2f", - Name: "sshkey1", - ID: 42, + Entry("no_error_different_length", testCaseFilterHCloudSSHKeys{ + sshKeysSpec: []infrav1.SSHKey{ + { + Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:2f", + Name: "sshkey1", + }, + { + Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:4f", + Name: "sshkey3", + }, }, - { - Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:4h", - Name: "sshkey3", - ID: 44, + expectedOutput: []*hcloud.SSHKey{ + { + Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:2f", + Name: "sshkey1", + ID: 42, + }, + { + Fingerprint: "b7:2f:30:a0:2f:6c:58:6c:21:04:58:61:ba:06:3b:4h", + Name: "sshkey3", + ID: 44, + }, }, }), ) @@ -213,25 +235,35 @@ var _ = Describe("handleServerStatusOff", func() { It("sets a condition if none is previously set", func() { service := newTestService(hcloudMachine, client) + res, err := service.handleServerStatusOff(context.Background(), server) Expect(err).To(Succeed()) + Expect(res).Should(Equal(reconcile.Result{RequeueAfter: 30 * time.Second})) + Expect(conditions.GetReason(hcloudMachine, infrav1.ServerAvailableCondition)).To(Equal(infrav1.ServerOffReason)) + Expect(server.Status).To(Equal(hcloud.ServerStatusRunning)) }) It("tries to power on server again if it is not timed out", func() { conditions.MarkFalse(hcloudMachine, infrav1.ServerAvailableCondition, infrav1.ServerOffReason, clusterv1.ConditionSeverityInfo, "") + service := newTestService(hcloudMachine, client) + res, err := service.handleServerStatusOff(context.Background(), server) Expect(err).To(Succeed()) + Expect(res).Should(Equal(reconcile.Result{RequeueAfter: 30 * time.Second})) + Expect(server.Status).To(Equal(hcloud.ServerStatusRunning)) + Expect(conditions.GetReason(hcloudMachine, infrav1.ServerAvailableCondition)).To(Equal(infrav1.ServerOffReason)) }) It("sets a failure message if it timed out", func() { conditions.MarkFalse(hcloudMachine, infrav1.ServerAvailableCondition, infrav1.ServerOffReason, clusterv1.ConditionSeverityInfo, "") + // manipulate lastTransitionTime conditionsList := hcloudMachine.GetConditions() for i, c := range conditionsList { @@ -239,22 +271,31 @@ var _ = Describe("handleServerStatusOff", func() { conditionsList[i].LastTransitionTime = metav1.NewTime(time.Now().Add(-time.Hour)) } } + service := newTestService(hcloudMachine, client) + res, err := service.handleServerStatusOff(context.Background(), server) Expect(err).To(Succeed()) + var emptyResult reconcile.Result Expect(res).Should(Equal(emptyResult)) + Expect(server.Status).To(Equal(hcloud.ServerStatusOff)) Expect(hcloudMachine.Status.FailureMessage).Should(Equal(pointer.String("reached timeout of waiting for machines that are switched off"))) }) It("tries to power on server and sets new condition if different one is set", func() { conditions.MarkTrue(hcloudMachine, infrav1.ServerAvailableCondition) + service := newTestService(hcloudMachine, client) + res, err := service.handleServerStatusOff(context.Background(), server) Expect(err).To(Succeed()) + Expect(res).Should(Equal(reconcile.Result{RequeueAfter: 30 * time.Second})) + Expect(conditions.GetReason(hcloudMachine, infrav1.ServerAvailableCondition)).To(Equal(infrav1.ServerOffReason)) + Expect(server.Status).To(Equal(hcloud.ServerStatusRunning)) }) }) diff --git a/pkg/services/hcloud/util/utils_test.go b/pkg/services/hcloud/util/utils_test.go index e839a368c..3dda2befa 100644 --- a/pkg/services/hcloud/util/utils_test.go +++ b/pkg/services/hcloud/util/utils_test.go @@ -40,6 +40,7 @@ var _ = Describe("Test ServerIDFromProviderID", func() { Expect(err).ToNot(BeNil()) Expect(err).To(MatchError(ErrNilProviderID)) }) + type testCaseServerIDFromProviderID struct { providerID string expectServerID int @@ -50,6 +51,7 @@ var _ = Describe("Test ServerIDFromProviderID", func() { func(tc testCaseServerIDFromProviderID) { serverID, err := ServerIDFromProviderID(&tc.providerID) Expect(serverID).Should(Equal(tc.expectServerID)) + if tc.expectError != nil { Expect(err).ToNot(BeNil()) Expect(err).Should(MatchError(tc.expectError)) diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 74412b6d2..0e2cb0a90 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -25,96 +25,179 @@ import ( "github.com/syself/cluster-api-provider-hetzner/pkg/utils" ) +type testCaseLabelsToLabelSelector struct { + labels map[string]string + expectedOutput string + expectedOutputInverse string +} + var _ = DescribeTable("LabelsToLabelSelector", - func(labels map[string]string, expectedOutput, expectedOutputInverse string) { - Expect(utils.LabelsToLabelSelector(labels)).To(Or(Equal(expectedOutput), Equal(expectedOutputInverse))) + func(tc testCaseLabelsToLabelSelector) { + Expect(utils.LabelsToLabelSelector(tc.labels)).To(Or(Equal(tc.expectedOutput), Equal(tc.expectedOutputInverse))) }, - Entry("existing keys", map[string]string{ - "key1": "label1", - "key2": "label2", - }, "key2==label2,key1==label1", "key1==label1,key2==label2"), - Entry("no keys", map[string]string{}, "", ""), + Entry("existing keys", testCaseLabelsToLabelSelector{ + labels: map[string]string{ + "key1": "label1", + "key2": "label2", + }, + expectedOutput: "key2==label2,key1==label1", + expectedOutputInverse: "key1==label1,key2==label2", + }), + Entry("no keys", testCaseLabelsToLabelSelector{ + labels: map[string]string{}, + expectedOutput: "", + expectedOutputInverse: "", + }), ) +type testCaseLabelSelectorToLabels struct { + str string + expectedOutput map[string]string +} + var _ = DescribeTable("LabelSelectorToLabels", - func(str string, expectedOutput map[string]string) { - Expect(utils.LabelSelectorToLabels(str)).To(Equal(expectedOutput)) + func(tc testCaseLabelSelectorToLabels) { + Expect(utils.LabelSelectorToLabels(tc.str)).To(Equal(tc.expectedOutput)) }, - Entry("existing keys", "key2==label2,key1==label1", map[string]string{ - "key1": "label1", - "key2": "label2", + Entry("existing keys", testCaseLabelSelectorToLabels{ + str: "key2==label2,key1==label1", + expectedOutput: map[string]string{ + "key1": "label1", + "key2": "label2", + }, + }), + Entry("no keys", testCaseLabelSelectorToLabels{ + str: "", + expectedOutput: map[string]string{}, }), - Entry("no keys", "", map[string]string{}), ) var _ = Describe("DifferenceOfStringSlices", func() { + type testCaseDifferenceOfStringSlices struct { + a []string + b []string + onlyInA []string + onlyInB []string + } + DescribeTable("Computing differences", - func(a, b, onlyInA, onlyInB []string) { - outA, outB := utils.DifferenceOfStringSlices(a, b) - Expect(outA).To(Equal(onlyInA)) - Expect(outB).To(Equal(onlyInB)) + func(tc testCaseDifferenceOfStringSlices) { + outA, outB := utils.DifferenceOfStringSlices(tc.a, tc.b) + Expect(outA).To(Equal(tc.onlyInA)) + Expect(outB).To(Equal(tc.onlyInB)) }, - Entry( - "entry1", - []string{"string1", "string2", "string3", "string4"}, - []string{"string1", "string2"}, - []string{"string3", "string4"}, - nil, - ), - Entry( - "entry2", - []string{"string1", "string2"}, - []string{"string1", "string2", "string3", "string4"}, - nil, - []string{"string3", "string4"}, - ), - Entry( - "entry3", - []string{"string1", "string2"}, - nil, - []string{"string1", "string2"}, - nil), - Entry( - "entry4", - nil, - []string{"string1", "string2"}, - nil, - []string{"string1", "string2"}, - ), + Entry("entry1", testCaseDifferenceOfStringSlices{ + a: []string{"string1", "string2", "string3", "string4"}, + b: []string{"string1", "string2"}, + onlyInA: []string{"string3", "string4"}, + onlyInB: nil, + }), + Entry("entry2", testCaseDifferenceOfStringSlices{ + a: []string{"string1", "string2"}, + b: []string{"string1", "string2", "string3", "string4"}, + onlyInA: nil, + onlyInB: []string{"string3", "string4"}, + }), + Entry("entry3", testCaseDifferenceOfStringSlices{ + a: []string{"string1", "string2"}, + b: nil, + onlyInA: []string{"string1", "string2"}, + onlyInB: nil, + }), + Entry("entry4", testCaseDifferenceOfStringSlices{ + a: nil, + b: []string{"string1", "string2"}, + onlyInA: nil, + onlyInB: []string{"string1", "string2"}, + }), ) }) var _ = Describe("DifferenceOfIntSlices", func() { + type testCaseDifferenceOfIntSlices struct { + a []int + b []int + onlyInA []int + onlyInB []int + } + DescribeTable("Computing differences", - func(a, b, onlyInA, onlyInB []int) { - outA, outB := utils.DifferenceOfIntSlices(a, b) - Expect(outA).To(Equal(onlyInA)) - Expect(outB).To(Equal(onlyInB)) + func(tc testCaseDifferenceOfIntSlices) { + outA, outB := utils.DifferenceOfIntSlices(tc.a, tc.b) + Expect(outA).To(Equal(tc.onlyInA)) + Expect(outB).To(Equal(tc.onlyInB)) }, - Entry("entry1", []int{1, 2, 3, 4}, []int{1, 2}, []int{3, 4}, nil), - Entry("entry2", []int{1, 2}, []int{1, 2, 3, 4}, nil, []int{3, 4}), - Entry("entry3", []int{1, 2}, nil, []int{1, 2}, nil), - Entry("entry4", nil, []int{1, 2}, nil, []int{1, 2})) + Entry("entry1", testCaseDifferenceOfIntSlices{ + a: []int{1, 2, 3, 4}, + b: []int{1, 2}, + onlyInA: []int{3, 4}, + onlyInB: nil, + }), + Entry("entry2", testCaseDifferenceOfIntSlices{ + a: []int{1, 2}, + b: []int{1, 2, 3, 4}, + onlyInA: nil, + onlyInB: []int{3, 4}, + }), + Entry("entry3", testCaseDifferenceOfIntSlices{ + a: []int{1, 2}, + b: nil, + onlyInA: []int{1, 2}, + onlyInB: nil, + }), + Entry("entry4", testCaseDifferenceOfIntSlices{ + a: nil, + b: []int{1, 2}, + onlyInA: nil, + onlyInB: []int{1, 2}, + })) }) var _ = Describe("StringInList", func() { + type testCaseStringInList struct { + list []string + str string + expectedOutcome bool + } + DescribeTable("Test string in list", - func(list []string, str string, expectedOutcome bool) { - out := utils.StringInList(list, str) - Expect(out).To(Equal(expectedOutcome)) + func(tc testCaseStringInList) { + out := utils.StringInList(tc.list, tc.str) + Expect(out).To(Equal(tc.expectedOutcome)) }, - Entry("entry1", []string{"a", "b", "c"}, "a", true), - Entry("entry2", []string{"a", "b", "c"}, "d", false)) + Entry("entry1", testCaseStringInList{ + list: []string{"a", "b", "c"}, + str: "a", + expectedOutcome: true, + }), + Entry("entry2", testCaseStringInList{ + list: []string{"a", "b", "c"}, + str: "d", + expectedOutcome: false, + })) }) var _ = Describe("FilterStringFromList", func() { + type testCaseFilterStringFromList struct { + list []string + str string + expectedOutcome []string + } DescribeTable("Test filter string from list", - func(list []string, str string, expectedOutcome []string) { - out := utils.FilterStringFromList(list, str) - Expect(out).To(Equal(expectedOutcome)) + func(tc testCaseFilterStringFromList) { + out := utils.FilterStringFromList(tc.list, tc.str) + Expect(out).To(Equal(tc.expectedOutcome)) }, - Entry("entry1", []string{"a", "b", "c"}, "a", []string{"b", "c"}), - Entry("entry2", []string{"a", "b", "c"}, "d", []string{"a", "b", "c"})) + Entry("entry1", testCaseFilterStringFromList{ + list: []string{"a", "b", "c"}, + str: "a", + expectedOutcome: []string{"b", "c"}, + }), + Entry("entry2", testCaseFilterStringFromList{ + list: []string{"a", "b", "c"}, + str: "d", + expectedOutcome: []string{"a", "b", "c"}, + })) }) var _ = Describe("Test removeOwnerRefFromList", func() {