diff --git a/bmc/mockup.go b/bmc/mockup.go new file mode 100644 index 00000000..39a97413 --- /dev/null +++ b/bmc/mockup.go @@ -0,0 +1,37 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package bmc + +// RedfishLocalBMC is an implementation of the BMC interface for Redfish. +type RedfishMockUps struct { + BIOSSettingAttr map[string]map[string]any + PendingBIOSSetting map[string]map[string]any +} + +func (r *RedfishMockUps) InitializeDefaults() { + r.BIOSSettingAttr = map[string]map[string]any{ + "abc": {"type": "string", "reboot": false, "value": "bar"}, + "fooreboot": {"type": "integer", "reboot": true, "value": 123}, + } + r.PendingBIOSSetting = map[string]map[string]any{} +} + +func (r *RedfishMockUps) ResetBIOSSettings() { + r.BIOSSettingAttr = map[string]map[string]any{ + "abc": {"type": "string", "reboot": false, "value": "bar"}, + "fooreboot": {"type": "integer", "reboot": true, "value": 123}, + } + r.PendingBIOSSetting = map[string]map[string]any{} +} + +func (r *RedfishMockUps) ResetPendingBIOSSetting() { + r.PendingBIOSSetting = map[string]map[string]any{} +} + +func InitMockUp() { + UnitTestMockUps = &RedfishMockUps{} + UnitTestMockUps.InitializeDefaults() +} + +var UnitTestMockUps *RedfishMockUps diff --git a/bmc/redfish_local.go b/bmc/redfish_local.go index 87bd6f99..d4d9de5e 100644 --- a/bmc/redfish_local.go +++ b/bmc/redfish_local.go @@ -6,24 +6,17 @@ package bmc import ( "context" "fmt" + "time" "github.com/stmcginnis/gofish/redfish" + ctrl "sigs.k8s.io/controller-runtime" ) var _ BMC = (*RedfishLocalBMC)(nil) -var defaultMockedBIOSSetting = map[string]map[string]any{ - "abc": {"type": "string", "reboot": false, "value": "bar"}, - "fooreboot": {"type": "integer", "reboot": true, "value": 123}, -} - -var pendingMockedBIOSSetting = map[string]map[string]any{} - // RedfishLocalBMC is an implementation of the BMC interface for Redfish. type RedfishLocalBMC struct { *RedfishBMC - StoredBIOSSettingData map[string]map[string]any - StoredBMCSettingData map[string]map[string]any } // NewRedfishLocalBMCClient creates a new RedfishLocalBMC with the given connection details. @@ -39,40 +32,57 @@ func NewRedfishLocalBMCClient( } func (r RedfishLocalBMC) PowerOn(ctx context.Context, systemUUID string) error { - system, err := r.getSystemByUUID(ctx, systemUUID) - if err != nil { - return fmt.Errorf("failed to get system: %w", err) - } - system.PowerState = redfish.OnPowerState - systemURI := fmt.Sprintf("/redfish/v1/Systems/%s", system.ID) - if err := system.Patch(systemURI, system); err != nil { - return fmt.Errorf("failed to set power state %s for system %s: %w", redfish.OnPowerState, systemUUID, err) - } - // mock the bmc update here - if len(pendingMockedBIOSSetting) > 0 { + go func() { + time.Sleep(250 * time.Millisecond) + system, err := r.getSystemByUUID(ctx, systemUUID) + if err != nil { + log := ctrl.LoggerFrom(ctx) + log.V(1).Error(err, "failed to get system") + return + } + system.PowerState = redfish.OnPowerState + system.RawData = nil + systemURI := fmt.Sprintf("/redfish/v1/Systems/%s", system.ID) + if err := system.Patch(systemURI, system); err != nil { + log := ctrl.LoggerFrom(ctx) + log.V(1).Error(err, "failed to Patch system to power on", "systemUUID", systemUUID) + return + } - for key, data := range pendingMockedBIOSSetting { - if _, ok := defaultMockedBIOSSetting[key]; ok { - defaultMockedBIOSSetting[key] = data + // mock the bmc update here + if len(UnitTestMockUps.PendingBIOSSetting) > 0 { + time.Sleep(150 * time.Millisecond) + for key, data := range UnitTestMockUps.PendingBIOSSetting { + if _, ok := UnitTestMockUps.BIOSSettingAttr[key]; ok { + UnitTestMockUps.BIOSSettingAttr[key] = data + } } + UnitTestMockUps.ResetPendingBIOSSetting() } - pendingMockedBIOSSetting = map[string]map[string]any{} - r.StoredBIOSSettingData = defaultMockedBIOSSetting - } + }() return nil } func (r RedfishLocalBMC) PowerOff(ctx context.Context, systemUUID string) error { - system, err := r.getSystemByUUID(ctx, systemUUID) - if err != nil { - return fmt.Errorf("failed to get system: %w", err) - } - system.PowerState = redfish.OffPowerState - systemURI := fmt.Sprintf("/redfish/v1/Systems/%s", system.ID) - if err := system.Patch(systemURI, system); err != nil { - return fmt.Errorf("failed to set power state %s for system %s: %w", redfish.OffPowerState, systemUUID, err) - } + + go func() { + time.Sleep(250 * time.Millisecond) + system, err := r.getSystemByUUID(ctx, systemUUID) + if err != nil { + log := ctrl.LoggerFrom(ctx) + log.V(1).Error(err, "failed to get system") + return + } + system.PowerState = redfish.OffPowerState + system.RawData = nil + systemURI := fmt.Sprintf("/redfish/v1/Systems/%s", system.ID) + if err := system.Patch(systemURI, system); err != nil { + log := ctrl.LoggerFrom(ctx) + log.V(1).Error(err, "failed to Patch system to power off", "systemUUID", systemUUID) + return + } + }() return nil } @@ -83,13 +93,13 @@ func (r *RedfishLocalBMC) GetBiosPendingAttributeValues( redfish.SettingsAttributes, error, ) { - if len(pendingMockedBIOSSetting) == 0 { + if len(UnitTestMockUps.PendingBIOSSetting) == 0 { return redfish.SettingsAttributes{}, nil } - result := make(redfish.SettingsAttributes, len(pendingMockedBIOSSetting)) + result := make(redfish.SettingsAttributes, len(UnitTestMockUps.PendingBIOSSetting)) - for key, data := range pendingMockedBIOSSetting { + for key, data := range UnitTestMockUps.PendingBIOSSetting { result[key] = data["value"] } @@ -102,19 +112,16 @@ func (r *RedfishLocalBMC) SetBiosAttributesOnReset( systemUUID string, attributes redfish.SettingsAttributes, ) error { - if len(defaultMockedBIOSSetting) == 0 { - defaultMockedBIOSSetting = map[string]map[string]any{} - } - pendingMockedBIOSSetting = map[string]map[string]any{} + UnitTestMockUps.ResetPendingBIOSSetting() for key, attrData := range attributes { - if AttributesData, ok := defaultMockedBIOSSetting[key]; ok { + if AttributesData, ok := UnitTestMockUps.BIOSSettingAttr[key]; ok { if reboot, ok := AttributesData["reboot"]; ok && !reboot.(bool) { // if reboot not needed, set the attribute immediately. AttributesData["value"] = attrData } else { // if reboot needed, set the attribute at next power on. - pendingMockedBIOSSetting[key] = map[string]any{ + UnitTestMockUps.PendingBIOSSetting[key] = map[string]any{ "type": AttributesData["type"], "reboot": AttributesData["reboot"], "value": attrData, @@ -122,20 +129,9 @@ func (r *RedfishLocalBMC) SetBiosAttributesOnReset( } } } - r.StoredBIOSSettingData = defaultMockedBIOSSetting - return nil } -func (r *RedfishLocalBMC) getMockedBIOSSettingData() map[string]map[string]any { - - if len(r.StoredBIOSSettingData) > 0 { - return r.StoredBIOSSettingData - } - return defaultMockedBIOSSetting - -} - func (r *RedfishLocalBMC) GetBiosAttributeValues( ctx context.Context, systemUUID string, @@ -149,8 +145,6 @@ func (r *RedfishLocalBMC) GetBiosAttributeValues( return nil, nil } - mockedAttributes := r.getMockedBIOSSettingData() - filteredAttr, err := r.getFilteredBiosRegistryAttributes(false, false) if err != nil { return nil, err @@ -158,7 +152,7 @@ func (r *RedfishLocalBMC) GetBiosAttributeValues( result := make(redfish.SettingsAttributes, len(attributes)) for _, name := range attributes { if _, ok := filteredAttr[name]; ok { - if AttributesData, ok := mockedAttributes[name]; ok { + if AttributesData, ok := UnitTestMockUps.BIOSSettingAttr[name]; ok { result[name] = AttributesData["value"] } } @@ -173,12 +167,11 @@ func (r *RedfishLocalBMC) getFilteredBiosRegistryAttributes( map[string]RegistryEntryAttributes, error, ) { - mockedAttributes := r.getMockedBIOSSettingData() filtered := make(map[string]RegistryEntryAttributes) - if len(mockedAttributes) == 0 { + if len(UnitTestMockUps.BIOSSettingAttr) == 0 { return filtered, fmt.Errorf("no bmc setting attributes found") } - for name, AttributesData := range mockedAttributes { + for name, AttributesData := range UnitTestMockUps.BIOSSettingAttr { data := RegistryEntryAttributes{} data.AttributeName = name data.Immutable = immutable diff --git a/internal/controller/biossettings_controller_test.go b/internal/controller/biossettings_controller_test.go index dfb9c382..f0586a23 100644 --- a/internal/controller/biossettings_controller_test.go +++ b/internal/controller/biossettings_controller_test.go @@ -6,6 +6,7 @@ package controller import ( "github.com/ironcore-dev/controller-utils/metautils" metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" + "github.com/ironcore-dev/metal-operator/bmc" . "sigs.k8s.io/controller-runtime/pkg/envtest/komega" @@ -24,6 +25,13 @@ var _ = Describe("BIOSSettings Controller", func() { var server *metalv1alpha1.Server BeforeEach(func(ctx SpecContext) { + + By("Ensuring clean state") + var serverList metalv1alpha1.ServerList + Eventually(ObjectList(&serverList)).Should(HaveField("Items", (BeEmpty()))) + var biosList metalv1alpha1.BIOSSettingsList + Eventually(ObjectList(&biosList)).Should(HaveField("Items", (BeEmpty()))) + By("Creating a BMCSecret") bmcSecret := &metalv1alpha1.BMCSecret{ ObjectMeta: metav1.ObjectMeta{ @@ -38,6 +46,7 @@ var _ = Describe("BIOSSettings Controller", func() { Expect(k8sClient.Create(ctx, bmcSecret)).To(Succeed()) By("Creating a Server") + server = &metalv1alpha1.Server{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns.Name, @@ -58,16 +67,12 @@ var _ = Describe("BIOSSettings Controller", func() { }, }, } - Expect(k8sClient.Create(ctx, server)).Should(Succeed()) - - By("update the server state to Available state") - Eventually(UpdateStatus(server, func() { - server.Status.State = metalv1alpha1.ServerStateAvailable - })).Should(Succeed()) + TransistionServerFromInitialToAvailableState(ctx, k8sClient, server, ns.Name) }) AfterEach(func(ctx SpecContext) { DeleteAllMetalResources(ctx, ns.Name) + bmc.UnitTestMockUps.ResetBIOSSettings() }) It("should successfully patch its reference to referred server", func(ctx SpecContext) { @@ -76,14 +81,14 @@ var _ = Describe("BIOSSettings Controller", func() { BIOSSetting := make(map[string]string) // no setting to apply By("Ensuring that the server has Available state") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Status.State", metalv1alpha1.ServerStateAvailable), - )) + ) By("Creating a BIOSSetting V1") biosSettingsV1 := &metalv1alpha1.BIOSSettings{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns.Name, - GenerateName: "test-", + GenerateName: "test-reference", }, Spec: metalv1alpha1.BIOSSettingsSpec{ Version: "P79 v1.45 (12/06/2017)", @@ -95,19 +100,19 @@ var _ = Describe("BIOSSettings Controller", func() { Expect(k8sClient.Create(ctx, biosSettingsV1)).To(Succeed()) By("Ensuring that the Server has the correct server bios setting ref") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", &v1.LocalObjectReference{Name: biosSettingsV1.Name}), - )) + ) - Eventually(Object(biosSettingsV1)).Should(SatisfyAny( + Eventually(Object(biosSettingsV1)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), - )) + ) By("Creating a BIOSSetting V2") biosSettingsV2 := &metalv1alpha1.BIOSSettings{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns.Name, - GenerateName: "test-", + GenerateName: "test-reference-dup", }, Spec: metalv1alpha1.BIOSSettingsSpec{ Version: "P79 v2.45 (12/06/2017)", @@ -119,21 +124,21 @@ var _ = Describe("BIOSSettings Controller", func() { Expect(k8sClient.Create(ctx, biosSettingsV2)).To(Succeed()) By("Ensuring that the Server has the correct server bios setting ref") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", &v1.LocalObjectReference{Name: biosSettingsV2.Name}), - )) + ) - Eventually(Object(biosSettingsV2)).Should(SatisfyAny( + Eventually(Object(biosSettingsV2)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), - )) + ) By("Deleting the BIOSSettings V1 (old)") Expect(k8sClient.Delete(ctx, biosSettingsV1)).To(Succeed()) By("Ensuring that the Server has the correct server bios setting ref") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", &v1.LocalObjectReference{Name: biosSettingsV2.Name}), - )) + ) }) @@ -147,7 +152,7 @@ var _ = Describe("BIOSSettings Controller", func() { biosSettings := &metalv1alpha1.BIOSSettings{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns.Name, - GenerateName: "test-", + GenerateName: "test-no-change", }, Spec: metalv1alpha1.BIOSSettingsSpec{ Version: "P79 v1.45 (12/06/2017)", @@ -159,21 +164,21 @@ var _ = Describe("BIOSSettings Controller", func() { Expect(k8sClient.Create(ctx, biosSettings)).To(Succeed()) By("Ensuring that the Server has the bios setting ref") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", &v1.LocalObjectReference{Name: biosSettings.Name}), - )) + ) - Eventually(Object(biosSettings)).Should(SatisfyAll( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), - )) + ) By("Deleting the BIOSSettings") Expect(k8sClient.Delete(ctx, biosSettings)).To(Succeed()) By("Ensuring that the bios ref is empty") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", BeNil()), - )) + ) }) It("should update the setting without maintenance if setting requested needs no server reboot", func(ctx SpecContext) { @@ -184,13 +189,14 @@ var _ = Describe("BIOSSettings Controller", func() { // mock BIOSSettings to not request maintenance by powering on the system (mock no need of power change on system) // note: cant be in Available state as it will power off automatically. - _ = transitionServerToReserved(ctx, ns, server, metalv1alpha1.PowerOn) + serverClaim := BuildServerClaim(ctx, k8sClient, *server, ns.Name, nil, metalv1alpha1.PowerOn, "foo:bar") + TransistionServerToReserveredState(ctx, k8sClient, serverClaim, server, ns.Name) By("Creating a BIOS settings") biosSettings := &metalv1alpha1.BIOSSettings{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns.Name, - GenerateName: "test-bios-change", + GenerateName: "test-bios-change-noreboot-", }, Spec: metalv1alpha1.BIOSSettingsSpec{ Version: "P79 v1.45 (12/06/2017)", @@ -218,22 +224,22 @@ var _ = Describe("BIOSSettings Controller", func() { By("Ensuring that the Maintenance resource has not been created") var serverMaintenanceList metalv1alpha1.ServerMaintenanceList Consistently(ObjectList(&serverMaintenanceList)).Should(HaveField("Items", BeEmpty())) - Consistently(Object(biosSettings)).Should(SatisfyAll( + Consistently(Object(biosSettings)).Should( HaveField("Spec.ServerMaintenanceRef", BeNil()), - )) + ) By("Ensuring that the BIOS setting has reached next state: stateSynced") - Eventually(Object(biosSettings)).Should(SatisfyAll( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), - )) + ) By("Deleting the BIOSSettings") Expect(k8sClient.Delete(ctx, biosSettings)).To(Succeed()) By("Ensuring that the Server BIOSSettings ref is empty") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", BeNil()), - )) + ) }) It("should request maintenance when changing power status of server, even if bios settings update does not need it", func(ctx SpecContext) { @@ -247,13 +253,14 @@ var _ = Describe("BIOSSettings Controller", func() { // Reserved state is needed to transition through the unit test step by step // else, unit test finishes the state very fast without being able to check the transition // creating OwnerApproval through reserved state gives more control when to approve the maintenance - serverClaim := transitionServerToReserved(ctx, ns, server, metalv1alpha1.PowerOff) + serverClaim := BuildServerClaim(ctx, k8sClient, *server, ns.Name, nil, metalv1alpha1.PowerOff, "foo:bar") + TransistionServerToReserveredState(ctx, k8sClient, serverClaim, server, ns.Name) By("Creating a BIOS settings") biosSettings := &metalv1alpha1.BIOSSettings{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns.Name, - GenerateName: "test-bios-change", + GenerateName: "test-bios-change-poweron", }, Spec: metalv1alpha1.BIOSSettingsSpec{ Version: "P79 v1.45 (12/06/2017)", @@ -265,9 +272,9 @@ var _ = Describe("BIOSSettings Controller", func() { Expect(k8sClient.Create(ctx, biosSettings)).To(Succeed()) By("Ensuring that the BIOS setting has reached next state: inProgress") - Eventually(Object(biosSettings)).Should(SatisfyAny( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateInProgress), - )) + ) By("Ensuring that the Maintenance resource has been created") var serverMaintenanceList metalv1alpha1.ServerMaintenanceList @@ -299,9 +306,9 @@ var _ = Describe("BIOSSettings Controller", func() { )) By("Ensuring that the BIOS setting has state: inProgress") - Eventually(Object(biosSettings)).Should(SatisfyAny( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateInProgress), - )) + ) By("Approving the maintenance") Eventually(Update(serverClaim, func() { @@ -316,21 +323,21 @@ var _ = Describe("BIOSSettings Controller", func() { )) By("Ensuring that the Server is in correct power state") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Status.PowerState", metalv1alpha1.ServerOnPowerState), - )) + ) // because of how we mock the setting update, it applied immediately and hence will not go through reboots to apply setting // this is the eventual state we would need to reach By("Ensuring that the BIOS setting has reached next state: Completed") - Eventually(Object(biosSettings)).Should(SatisfyAll( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), - )) + ) By("Ensuring that the BIOS setting has not referenced serverMaintenance anymore") - Eventually(Object(biosSettings)).Should(SatisfyAll( + Eventually(Object(biosSettings)).Should( HaveField("Spec.ServerMaintenanceRef", BeNil()), - )) + ) By("Ensuring that the Maintenance resource has been deleted") Eventually(Get(serverMaintenance)).Should(Satisfy(apierrors.IsNotFound)) @@ -340,9 +347,9 @@ var _ = Describe("BIOSSettings Controller", func() { Expect(k8sClient.Delete(ctx, biosSettings)).To(Succeed()) By("Ensuring that the Server BIOSSettings ref is empty") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", BeNil()), - )) + ) }) It("should create maintenance if setting update needs reboot", func(ctx SpecContext) { @@ -357,7 +364,8 @@ var _ = Describe("BIOSSettings Controller", func() { // else, unit test finishes the state very fast without being able to check the transition // creating OwnerApproval through reserved state gives more control when to approve the maintenance By("update the server powerstate to Off and reserved state") - serverClaim := transitionServerToReserved(ctx, ns, server, metalv1alpha1.PowerOff) + serverClaim := BuildServerClaim(ctx, k8sClient, *server, ns.Name, nil, metalv1alpha1.PowerOff, "foo:bar") + TransistionServerToReserveredState(ctx, k8sClient, serverClaim, server, ns.Name) By("Creating a BIOS settings") biosSettings := &metalv1alpha1.BIOSSettings{ @@ -375,10 +383,9 @@ var _ = Describe("BIOSSettings Controller", func() { Expect(k8sClient.Create(ctx, biosSettings)).To(Succeed()) By("Ensuring that the BIOS setting has reached next state: inProgress") - Eventually(Object(biosSettings)).Should(SatisfyAny( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateInProgress), - )) - + ) By("Ensuring that the Maintenance resource has been created") var serverMaintenanceList metalv1alpha1.ServerMaintenanceList Eventually(ObjectList(&serverMaintenanceList)).Should(HaveField("Items", Not(BeEmpty()))) @@ -409,9 +416,9 @@ var _ = Describe("BIOSSettings Controller", func() { )) By("Ensuring that the BIOS setting has state: inProgress") - Eventually(Object(biosSettings)).Should(SatisfyAny( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateInProgress), - )) + ) By("Approving the maintenance") Eventually(Update(serverClaim, func() { @@ -426,34 +433,28 @@ var _ = Describe("BIOSSettings Controller", func() { )) By("Ensuring that the Server is in Maintenance") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Status.State", metalv1alpha1.ServerStateMaintenance), - )) - - // due to issue with serverClaim, which forces the power state on the server even during maintenance we need this - By("Ensuring that the Server is in correct power state") - Eventually(Object(server)).Should(SatisfyAll( - HaveField("Status.PowerState", metalv1alpha1.ServerOnPowerState), - )) + ) - By("Ensuring that the BIOS setting has reached next state: issue/reboot") + By("Ensuring that the BIOS setting has reached next state: issue/reboot or verification state") Eventually(Object(biosSettings)).Should(SatisfyAny( HaveField("Status.UpdateSettingState", metalv1alpha1.BIOSSettingUpdateWaitOnServerRebootPowerOn), HaveField("Status.UpdateSettingState", metalv1alpha1.BIOSSettingUpdateWaitOnServerRebootPowerOff), - HaveField("Status.UpdateSettingState", metalv1alpha1.BIOSSettingUpdateStateIssue), + HaveField("Status.UpdateSettingState", metalv1alpha1.BIOSSettingUpdateStateVerification), )) // because of how we mock the setting update, it applied immediately and hence will not go through reboots to apply setting // this is the eventual state we would need to reach By("Ensuring that the BIOS setting has reached next state: Completed") - Eventually(Object(biosSettings)).Should(SatisfyAll( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), - )) + ) By("Ensuring that the BIOS setting has not referenced serverMaintenance anymore") - Eventually(Object(biosSettings)).Should(SatisfyAll( + Eventually(Object(biosSettings)).Should( HaveField("Spec.ServerMaintenanceRef", BeNil()), - )) + ) By("Ensuring that the Maintenance resource has been deleted") Eventually(Get(serverMaintenance)).Should(Satisfy(apierrors.IsNotFound)) @@ -476,7 +477,7 @@ var _ = Describe("BIOSSettings Controller", func() { biosSettings := &metalv1alpha1.BIOSSettings{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns.Name, - GenerateName: "test-", + GenerateName: "test-from-server-avail", }, Spec: metalv1alpha1.BIOSSettingsSpec{ Version: "P79 v1.45 (12/06/2017)", @@ -488,14 +489,14 @@ var _ = Describe("BIOSSettings Controller", func() { Expect(k8sClient.Create(ctx, biosSettings)).To(Succeed()) By("Ensuring that the BIOS setting has reached next state: inProgress") - Eventually(Object(biosSettings)).Should(SatisfyAny( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateInProgress), - )) + ) By("Ensuring that the Server has the bios setting ref") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", &v1.LocalObjectReference{Name: biosSettings.Name}), - )) + ) By("Ensuring that the Maintenance resource has been created") var serverMaintenanceList metalv1alpha1.ServerMaintenanceList @@ -525,34 +526,31 @@ var _ = Describe("BIOSSettings Controller", func() { APIVersion: "metal.ironcore.dev/v1alpha1", }), )) - // because of the mocking, the transistions are super fast here. can not determine the exact states - By("Ensuring that the BIOS setting has reached next state: issue/reboot") - Eventually(Object(biosSettings)).Should(SatisfyAny( - HaveField("Status.UpdateSettingState", metalv1alpha1.BIOSSettingUpdateStateIssue), - HaveField("Status.UpdateSettingState", metalv1alpha1.BIOSSettingUpdateWaitOnServerRebootPowerOff), - HaveField("Status.UpdateSettingState", metalv1alpha1.BIOSSettingUpdateWaitOnServerRebootPowerOn), - HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), - )) - // because of the mocking, the transistions are super fast here. can not determine the exact states + By("Ensuring that the Server is in Maintenance") + Eventually(Object(server)).Should( + HaveField("Status.State", metalv1alpha1.ServerStateMaintenance), + ) + + By("Ensuring that the BIOS setting has reached next state: issue/reboot or verification state") Eventually(Object(biosSettings)).Should(SatisfyAny( HaveField("Status.UpdateSettingState", metalv1alpha1.BIOSSettingUpdateWaitOnServerRebootPowerOn), + HaveField("Status.UpdateSettingState", metalv1alpha1.BIOSSettingUpdateWaitOnServerRebootPowerOff), HaveField("Status.UpdateSettingState", metalv1alpha1.BIOSSettingUpdateStateVerification), - HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), )) // because of the mocking, the transistions are super fast here. - Eventually(Object(biosSettings)).Should(SatisfyAll( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), - )) + ) By("Deleting the BIOSSettings") Expect(k8sClient.Delete(ctx, biosSettings)).To(Succeed()) By("Ensuring that the bios ref is empty") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", BeNil()), - )) + ) }) It("should wait for upgrade and reconcile when biosSettings version is correct", func(ctx SpecContext) { @@ -564,13 +562,14 @@ var _ = Describe("BIOSSettings Controller", func() { // Reserved state is needed to as Available state will turn off the power automatically. // powerOn is needed to skip the change in power on system, Hence skip maintenance. By("update the server powerstate to On and reserved state") - _ = transitionServerToReserved(ctx, ns, server, metalv1alpha1.PowerOn) + serverClaim := BuildServerClaim(ctx, k8sClient, *server, ns.Name, nil, metalv1alpha1.PowerOn, "foo:bar") + TransistionServerToReserveredState(ctx, k8sClient, serverClaim, server, ns.Name) By("Creating a BMCSetting") biosSettings := &metalv1alpha1.BIOSSettings{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns.Name, - GenerateName: "test-bmc-upgrade", + GenerateName: "test-bios-upgrade-", }, Spec: metalv1alpha1.BIOSSettingsSpec{ Version: "2.45.455b66-rev4", @@ -588,17 +587,17 @@ var _ = Describe("BIOSSettings Controller", func() { )) By("Ensuring that the biosSettings resource state is correct State inVersionUpgrade") - Eventually(Object(biosSettings)).Should(SatisfyAny( + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateInProgress), - )) + ) By("Ensuring that the serverMaintenance not ref.") - Eventually(Object(biosSettings)).Should(SatisfyAll( + Eventually(Object(biosSettings)).Should( HaveField("Spec.ServerMaintenanceRef", BeNil()), - )) - Consistently(Object(biosSettings)).Should(SatisfyAll( + ) + Consistently(Object(biosSettings)).Should( HaveField("Spec.ServerMaintenanceRef", BeNil()), - )) + ) By("Simulate the server biosSettings version update by matching the spec version") Eventually(Update(biosSettings, func() { @@ -610,17 +609,18 @@ var _ = Describe("BIOSSettings Controller", func() { HaveField("Status.State", metalv1alpha1.BIOSSettingsStateInProgress), HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), )) - Eventually(Object(biosSettings)).Should(SatisfyAll( + // due to nature of mocking, we cant not determine few steps here. hence need a longer wait time + Eventually(Object(biosSettings)).Should( HaveField("Status.State", metalv1alpha1.BIOSSettingsStateApplied), - )) + ) By("Ensuring that the serverMaintenance not ref.") - Eventually(Object(biosSettings)).Should(SatisfyAll( + Eventually(Object(biosSettings)).Should( HaveField("Spec.ServerMaintenanceRef", BeNil()), - )) - Consistently(Object(biosSettings)).Should(SatisfyAll( + ) + Consistently(Object(biosSettings)).Should( HaveField("Spec.ServerMaintenanceRef", BeNil()), - )) + ) By("Deleting the BMCSetting resource") Expect(k8sClient.Delete(ctx, biosSettings)).To(Succeed()) @@ -630,74 +630,11 @@ var _ = Describe("BIOSSettings Controller", func() { Consistently(Get(biosSettings)).Should(Satisfy(apierrors.IsNotFound)) By("Ensuring that the Server biosSettings ref is empty on BMC") - Eventually(Object(server)).Should(SatisfyAll( + Eventually(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", BeNil()), - )) - Consistently(Object(server)).Should(SatisfyAll( + ) + Consistently(Object(server)).Should( HaveField("Spec.BIOSSettingsRef", BeNil()), - )) + ) }) }) - -func transitionServerToReserved(ctx SpecContext, ns *v1.Namespace, server *metalv1alpha1.Server, powerState metalv1alpha1.Power) *metalv1alpha1.ServerClaim { - - By("Creating an Ignition secret") - ignitionSecret := &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns.Name, - GenerateName: "test-", - }, - Data: map[string][]byte{ - "foo": []byte("bar"), - }, - } - Expect(k8sClient.Create(ctx, ignitionSecret)).To(Succeed()) - - By("Creating a ServerClaim") - serverClaim := &metalv1alpha1.ServerClaim{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns.Name, - GenerateName: "test-", - }, - Spec: metalv1alpha1.ServerClaimSpec{ - Power: powerState, - ServerRef: &v1.LocalObjectReference{Name: server.Name}, - IgnitionSecretRef: &v1.LocalObjectReference{Name: ignitionSecret.Name}, - Image: "foo:bar", - }, - } - Expect(k8sClient.Create(ctx, serverClaim)).To(Succeed()) - - By("Patching the Server to available state") - Eventually(UpdateStatus(server, func() { - server.Status.State = metalv1alpha1.ServerStateAvailable - })).Should(Succeed()) - - // unfortunately, ServerClaim force creates the bootconfig and that does not transition to completed state. - // in reserved state, Hence, manually move bootconfig to completed to be able to put server in powerOn state. - bootConfig := &metalv1alpha1.ServerBootConfiguration{} - bootConfig.Name = serverClaim.Name - bootConfig.Namespace = serverClaim.Namespace - - Eventually(Get(bootConfig)).Should(Succeed()) - - By("Patching the Server to available state") - Eventually(UpdateStatus(bootConfig, func() { - bootConfig.Status.State = metalv1alpha1.ServerBootConfigurationStateReady - })).Should(Succeed()) - - Eventually(Get(server)).Should(Succeed()) - - By("Ensuring that the Server has the spec and state") - Eventually(Object(server)).Should(SatisfyAll( - HaveField("Spec.ServerClaimRef.Name", serverClaim.Name), - HaveField("Spec.Power", powerState), - HaveField("Status.State", metalv1alpha1.ServerStateReserved), - )) - - By("Patching the Server to required power state state") - Eventually(UpdateStatus(server, func() { - server.Status.PowerState = metalv1alpha1.ServerPowerState(powerState) - })).Should(Succeed()) - return serverClaim -} diff --git a/internal/controller/common_test_helpers.go b/internal/controller/common_test_helpers.go new file mode 100644 index 00000000..610712f7 --- /dev/null +++ b/internal/controller/common_test_helpers.go @@ -0,0 +1,253 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" // nolint: staticcheck + . "github.com/onsi/gomega" // nolint: staticcheck + . "sigs.k8s.io/controller-runtime/pkg/envtest/komega" // nolint: staticcheck + + metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" + "github.com/ironcore-dev/metal-operator/internal/probe" + v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// TransistionServerFromInitialToAvailableState transistions the server to AvailableState +func TransistionServerFromInitialToAvailableState( + ctx context.Context, + k8sClient client.Client, + server *metalv1alpha1.Server, + BootConfigNameSpace string, +) { + if server.ResourceVersion == "" && server.Name == "" { + By("Ensuring the server has been created") + Expect(k8sClient.Create(ctx, server)).Should(SatisfyAny( + BeNil(), + Satisfy(apierrors.IsAlreadyExists), + ), fmt.Sprintf("server is not created %v", server)) + } + Eventually(Get(server)).Should(Succeed()) + + if server.Status.State == metalv1alpha1.ServerStateAvailable { + By("Ensuring the server in Available state consistently") + Consistently(Object(server)).Should( + HaveField("Status.State", metalv1alpha1.ServerStateAvailable), + fmt.Sprintf("Expected server to be consistently in Available State %v", server.Status.State)) + Eventually(Object(server)).Should( + HaveField("Status.PowerState", metalv1alpha1.ServerOffPowerState), + fmt.Sprintf("Expected Server to be in PowerState 'off' in Available state %v", server)) + return + } + + By("Ensuring the server's Initial state") + Eventually(Object(server)).Should(SatisfyAny( + HaveField("Status.State", metalv1alpha1.ServerStateInitial), + HaveField("Status.State", metalv1alpha1.ServerStateDiscovery), + ), fmt.Sprintf("Expected server to be in Initial State and transisitiong to powerOff %v", server.Status.State)) + if server.Status.State == metalv1alpha1.ServerStateInitial { + Eventually(Object(server)).Should(SatisfyAll( + HaveField("Spec.Power", metalv1alpha1.PowerOff), + HaveField("Status.PowerState", metalv1alpha1.ServerOffPowerState), + ), fmt.Sprintf("Expected Server to be in PowerState 'off' in Initial state %v", server)) + } + + By("Ensuring the server's bootconfig is ref") + Eventually(Object(server)).Should( + HaveField("Spec.BootConfigurationRef", Not(BeNil())), + ) + + // need long time to create boot config + // as we go through multiple reconcile before creating the boot config + By("Ensuring the boot configuration has been created") + bootConfig := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: BootConfigNameSpace, + Name: server.Name, + }, + } + Eventually(Get(bootConfig)).Should( + Succeed(), + fmt.Sprintf("Expected to get the bootConfig %v, created by Server %v", bootConfig, server.Name), + ) + Eventually(Object(bootConfig)).Should( + HaveField("Status.State", metalv1alpha1.ServerBootConfigurationStatePending), + "Expected to get the bootConfig to reach pending state") + + By("Patching the boot configuration to a Ready state") + Eventually(UpdateStatus(bootConfig, func() { + bootConfig.Status.State = metalv1alpha1.ServerBootConfigurationStateReady + })).Should(Succeed(), fmt.Sprintf("Unable to set the bootconfig %v to Ready State", bootConfig)) + + Eventually(Object(bootConfig)).Should(SatisfyAll( + HaveField("Status.State", metalv1alpha1.ServerBootConfigurationStateReady), + HaveField("Spec.IgnitionSecretRef", Not(BeNil())), + ), "Expected the bootConfig to reach ready state") + + By("Ensuring that the Server is set to discovery and powered on") + Eventually(Object(server)).Should( + HaveField("Status.State", metalv1alpha1.ServerStateDiscovery), + fmt.Sprintf("Expected Server to be in Discovery state %v", server)) + Eventually(Object(server)).Should(SatisfyAll( + HaveField("Spec.Power", metalv1alpha1.PowerOn), + HaveField("Status.PowerState", metalv1alpha1.ServerOnPowerState), + ), fmt.Sprintf("Expected Server to be in PowerState 'on' in discovery state %v", server)) + + By("Starting the probe agent") + probeAgent := probe.NewAgent(server.Spec.SystemUUID, "http://localhost:30000", 50*time.Millisecond) + go func() { + defer GinkgoRecover() + Expect(probeAgent.Start(ctx)).To(Succeed(), "failed to start probe agent") + }() + + By("Ensuring that the server is set to available and powered off") + // here, the Registry agent check sometimes fails (checkLastStatusUpdateAfter), need longer wait time, + // to give chance to reach available incase it was reset to Initial state + Eventually(Object(server)).Should(SatisfyAny( + HaveField("Status.State", metalv1alpha1.ServerStateAvailable), + HaveField("Status.State", metalv1alpha1.ServerStateInitial), + ), fmt.Sprintf("Expected Server to be in Available or Initial State %v", server)) + + // give it one more chance to reach Available state before declaring an error + if server.Status.State == metalv1alpha1.ServerStateInitial { + Eventually(Object(server)).Should(SatisfyAny( + HaveField("Status.State", metalv1alpha1.ServerStateDiscovery), + HaveField("Status.State", metalv1alpha1.ServerStateAvailable), + ), fmt.Sprintf("Expected Server to be in Available or Discovery State %v", server)) + } + Eventually(Object(server)).Should( + HaveField("Status.State", metalv1alpha1.ServerStateAvailable), + fmt.Sprintf("Expected Server to be in Available State %v", server)) + Eventually(Object(server)).Should(SatisfyAll( + HaveField("Spec.Power", metalv1alpha1.PowerOff), + HaveField("Status.PowerState", metalv1alpha1.ServerOffPowerState), + ), fmt.Sprintf("Expected Server to be Powered Off in Available State %v", server)) +} + +// TransistionServerToReserveredState transistions the server to Reserved +func TransistionServerToReserveredState( + ctx context.Context, + k8sClient client.Client, + serverClaim *metalv1alpha1.ServerClaim, + server *metalv1alpha1.Server, + nameSpace string, +) { + if server.Status.State == metalv1alpha1.ServerStateReserved { + By("Ensuring the server in Reserevd state consistently") + Consistently(Object(server)).Should( + HaveField("Status.State", metalv1alpha1.ServerStateReserved), + fmt.Sprintf("Expected server to be consistently in Reserved State %v", server.Status.State)) + return + } + TransistionServerFromInitialToAvailableState(ctx, k8sClient, server, nameSpace) + + if serverClaim.ResourceVersion == "" && serverClaim.Name == "" { + Expect(k8sClient.Create(ctx, serverClaim)).Should(SatisfyAny( + BeNil(), + Satisfy(apierrors.IsAlreadyExists), + ), fmt.Sprintf("serverClaim is not created %v", serverClaim)) + } + Eventually(Get(serverClaim)).Should(Succeed()) + + By("Ensuring that the Server has the correct State and Claim ref") + Eventually(Object(server)).Should(SatisfyAll( + HaveField("Spec.ServerClaimRef", Not(BeNil())), + HaveField("Spec.ServerClaimRef.Name", serverClaim.Name), + ), fmt.Sprintf("Expected Server %v to be referenced by serverClaim %v", server, serverClaim)) + Eventually(Object(server)).Should( + HaveField("Status.State", metalv1alpha1.ServerStateReserved), + fmt.Sprintf("Expected Server to be in Reserved state %v", server)) + + By("Ensuring that the ServerClaim is bound") + Eventually(Object(serverClaim)).Should(SatisfyAll( + HaveField("Finalizers", ContainElement(ServerClaimFinalizer)), + HaveField("Status.Phase", metalv1alpha1.PhaseBound), + HaveField("Spec.ServerRef", Not(BeNil())), + HaveField("Spec.ServerRef.Name", server.Name), + ), fmt.Sprintf("Expected serverClaim %v to be bound", serverClaim)) + + By("Ensuring that the ServerBootConfiguration has been created") + claimConfig := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: serverClaim.Namespace, + Name: serverClaim.Name, + }, + } + Eventually(Get(claimConfig)).Should(Succeed()) + + By("Ensuring that the server has a correct boot configuration ref") + Eventually(Object(server)).Should( + HaveField("Spec.BootConfigurationRef", &v1.ObjectReference{ + APIVersion: "metal.ironcore.dev/v1alpha1", + Kind: "ServerBootConfiguration", + Namespace: claimConfig.Namespace, + Name: claimConfig.Name, + UID: claimConfig.UID, + }), + fmt.Sprintf("Expected Server to have ref for BootConfig %v created by serverClaim %v", + claimConfig, + serverClaim), + ) + + By("Patching the boot configuration to a Ready state") + Eventually(UpdateStatus(claimConfig, func() { + claimConfig.Status.State = metalv1alpha1.ServerBootConfigurationStateReady + })).Should(Succeed(), fmt.Sprintf("Unable to set the bootconfig %v to Ready State", claimConfig)) + + By("Ensuring that the Server has the correct PowerState") + Eventually(Object(server)).Should( + HaveField("Spec.Power", serverClaim.Spec.Power), + fmt.Sprintf("Expected Server to be in Power %v in Reserved state %v", serverClaim.Spec.Power, server.Status), + ) + Eventually(Object(server)).Should( + HaveField("Status.PowerState", metalv1alpha1.ServerPowerState(serverClaim.Spec.Power)), + fmt.Sprintf("Expected Server to be in PowerState %v in Reserved state %v", + serverClaim.Spec.Power, + server.Status), + ) +} + +func BuildServerClaim( + ctx context.Context, + k8sClient client.Client, + server metalv1alpha1.Server, + nameSpace string, + ignitionData map[string][]byte, + requiredPower metalv1alpha1.Power, + claimImage string, +) *metalv1alpha1.ServerClaim { + if ignitionData == nil { + ignitionData = make(map[string][]byte, 1) + } + By("Creating an Ignition secret") + ignitionSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: nameSpace, + GenerateName: "test-", + }, + Data: ignitionData, + } + Expect(k8sClient.Create(ctx, ignitionSecret)).To(Succeed()) + + By("Creating a ServerClaim object") + serverClaim := &metalv1alpha1.ServerClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: nameSpace, + GenerateName: "test-", + }, + Spec: metalv1alpha1.ServerClaimSpec{ + Power: requiredPower, + ServerRef: &v1.LocalObjectReference{Name: server.Name}, + IgnitionSecretRef: &v1.LocalObjectReference{Name: ignitionSecret.Name}, + Image: claimImage, + }, + } + return serverClaim +} diff --git a/internal/controller/server_controller.go b/internal/controller/server_controller.go index 03e1e131..835569e2 100644 --- a/internal/controller/server_controller.go +++ b/internal/controller/server_controller.go @@ -189,7 +189,7 @@ func (r *ServerReconciler) reconcile(ctx context.Context, log logr.Logger, serve if err := r.updateServerStatus(ctx, log, server); err != nil { return ctrl.Result{}, fmt.Errorf("failed to update server status: %w", err) } - log.V(1).Info("Updated Server status", "Status", server.Status.State) + log.V(1).Info("Updated Server status") if err := r.applyBootOrder(ctx, log, server); err != nil { return ctrl.Result{}, fmt.Errorf("failed to update server bios boot order: %w", err) @@ -198,6 +198,11 @@ func (r *ServerReconciler) reconcile(ctx context.Context, log logr.Logger, serve requeue, err := r.ensureServerStateTransition(ctx, log, server) if requeue && err == nil { + // we need to update the ServerStatus after state transition to make sure it reflects the changes done + if err := r.updateServerStatus(ctx, log, server); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to update server status: %w", err) + } + log.V(1).Info("Updated Server status after state transition") return ctrl.Result{Requeue: requeue, RequeueAfter: r.ResyncInterval}, nil } if err != nil && !apierrors.IsNotFound(err) { @@ -208,7 +213,7 @@ func (r *ServerReconciler) reconcile(ctx context.Context, log logr.Logger, serve if err := r.updateServerStatus(ctx, log, server); err != nil { return ctrl.Result{}, fmt.Errorf("failed to update server status: %w", err) } - log.V(1).Info("Updated Server status after transistions", "Status", server.Status.State) + log.V(1).Info("Updated Server status after state transition") log.V(1).Info("Reconciled Server") return ctrl.Result{}, nil @@ -523,6 +528,10 @@ func (r *ServerReconciler) updateServerStatus(ctx context.Context, log logr.Logg return fmt.Errorf("failed to patch Server status: %w", err) } + log.V(1).Info("Updated Server status", + "Status", server.Status.State, + "powerState", server.Status.PowerState) + return nil } @@ -695,6 +704,9 @@ func (r *ServerReconciler) setAndPatchServerPowerState(ctx context.Context, log } if op == controllerutil.OperationResultUpdated { log.V(1).Info("Server updated to power off state.") + if err := r.ensureServerPowerState(ctx, log, server); err != nil { + log.V(1).Info("ensuring power state failed.") + } return true, nil } return false, nil @@ -808,7 +820,7 @@ func (r *ServerReconciler) ensureServerPowerState(ctx context.Context, log logr. } if powerOp == powerOpNoOP { - log.V(1).Info("Server already in target power state") + log.V(1).Info("Server already in target power state", "powerState", server.Status.PowerState) return nil } @@ -824,6 +836,7 @@ func (r *ServerReconciler) ensureServerPowerState(ctx context.Context, log logr. switch powerOp { case powerOpOn: + log.V(1).Info("Server Power On") if err := bmcClient.PowerOn(ctx, server.Spec.SystemUUID); err != nil { return fmt.Errorf("failed to power on server: %w", err) } @@ -831,6 +844,7 @@ func (r *ServerReconciler) ensureServerPowerState(ctx context.Context, log logr. return fmt.Errorf("failed to wait for server power on server: %w", err) } case powerOpOff: + log.V(1).Info("Server Power Off") powerOffType := bmcClient.PowerOff if err := powerOffType(ctx, server.Spec.SystemUUID); err != nil { diff --git a/internal/controller/serverclaim_controller.go b/internal/controller/serverclaim_controller.go index b0071fbc..df513032 100644 --- a/internal/controller/serverclaim_controller.go +++ b/internal/controller/serverclaim_controller.go @@ -351,8 +351,8 @@ func (r *ServerClaimReconciler) claimServer(ctx context.Context, log logr.Logger if err != nil { return nil, false, err } - versions := make(chan string) - defer close(versions) + ServerRef := make(chan *v1.ObjectReference) + defer close(ServerRef) handler := toolscache.ResourceEventHandlerFuncs{ UpdateFunc: func(oldObj, newObj any) { @@ -360,7 +360,10 @@ func (r *ServerClaimReconciler) claimServer(ctx context.Context, log logr.Logger if newServer.Name != server.Name || newServer.Namespace != server.Namespace { return } - versions <- newServer.ResourceVersion + if newServer.Spec.ServerClaimRef == nil { + return + } + ServerRef <- newServer.Spec.ServerClaimRef }, } // The watch is initialized before calling ensureObjectRefForServer to ensure that the @@ -381,8 +384,9 @@ func (r *ServerClaimReconciler) claimServer(ctx context.Context, log logr.Logger } for { select { - case cachedVersion := <-versions: - if cachedVersion == server.ResourceVersion { + case cachedServerRef := <-ServerRef: + if cachedServerRef.Name == server.Spec.ServerClaimRef.Name && + cachedServerRef.Namespace == server.Spec.ServerClaimRef.Namespace { return server, modified, nil } case <-time.After(cacheUpdateTimeout): diff --git a/internal/controller/serverclaim_controller_test.go b/internal/controller/serverclaim_controller_test.go index a2975ff1..9b439783 100644 --- a/internal/controller/serverclaim_controller_test.go +++ b/internal/controller/serverclaim_controller_test.go @@ -5,6 +5,7 @@ package controller import ( "context" + "fmt" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -24,6 +25,12 @@ var _ = Describe("ServerClaim Controller", func() { var server *metalv1alpha1.Server BeforeEach(func(ctx SpecContext) { + By("Ensuring clean state") + var serverList metalv1alpha1.ServerList + Eventually(ObjectList(&serverList)).Should(HaveField("Items", (BeEmpty()))) + var claimList metalv1alpha1.ServerClaimList + Eventually(ObjectList(&claimList)).Should(HaveField("Items", (BeEmpty()))) + By("Creating a BMCSecret") bmcSecret := &metalv1alpha1.BMCSecret{ ObjectMeta: metav1.ObjectMeta{ @@ -56,7 +63,7 @@ var _ = Describe("ServerClaim Controller", func() { }, }, } - Expect(k8sClient.Create(ctx, server)).Should(Succeed()) + TransistionServerFromInitialToAvailableState(ctx, k8sClient, server, ns.Name) }) AfterEach(func(ctx SpecContext) { @@ -91,15 +98,9 @@ var _ = Describe("ServerClaim Controller", func() { } Expect(k8sClient.Create(ctx, claim)).To(Succeed()) - By("Patching the Server to available state") - Eventually(UpdateStatus(server, func() { - server.Status.State = metalv1alpha1.ServerStateAvailable - })).Should(Succeed()) - By("Ensuring that the Server has the correct claim ref") Eventually(Object(server)).Should(SatisfyAll( HaveField("Spec.ServerClaimRef.Name", claim.Name), - HaveField("Spec.Power", metalv1alpha1.PowerOn), HaveField("Status.State", metalv1alpha1.ServerStateReserved), )) @@ -142,6 +143,15 @@ var _ = Describe("ServerClaim Controller", func() { UID: config.UID, }), )) + By("Patching the boot configuration to a Ready state") + Eventually(UpdateStatus(config, func() { + config.Status.State = metalv1alpha1.ServerBootConfigurationStateReady + })).Should(Succeed(), fmt.Sprintf("Unable to set the bootconfig %v to Ready State", config)) + + By("Ensuring that the Server has the correct PowerStatus") + Eventually(Object(server)).Should(SatisfyAll( + HaveField("Status.PowerState", metalv1alpha1.ServerPowerState(claim.Spec.Power)), + )) By("Deleting the ServerClaim") Expect(k8sClient.Delete(ctx, claim)).To(Succeed()) @@ -186,11 +196,6 @@ var _ = Describe("ServerClaim Controller", func() { } Expect(k8sClient.Create(ctx, claim)).To(Succeed()) - By("Patching the Server to available state") - Eventually(UpdateStatus(server, func() { - server.Status.State = metalv1alpha1.ServerStateAvailable - })).Should(Succeed()) - By("Ensuring that the Server has the correct claim ref") Eventually(Object(server)).Should(SatisfyAll( HaveField("Spec.ServerClaimRef.Name", claim.Name), @@ -227,16 +232,6 @@ var _ = Describe("ServerClaim Controller", func() { } })).Should(Succeed()) - By("Patching the Server to available state") - Eventually(UpdateStatus(server, func() { - server.Status.State = metalv1alpha1.ServerStateAvailable - })).Should(Succeed()) - - Eventually(Object(server)).Should(SatisfyAll( - HaveField("Spec.ServerClaimRef", BeNil()), - HaveField("Status.State", metalv1alpha1.ServerStateAvailable), - )) - By("Creating a ServerClaim") claim := &metalv1alpha1.ServerClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -293,7 +288,7 @@ var _ = Describe("ServerClaim Controller", func() { }) It("should not claim a server in a non-available state", func(ctx SpecContext) { - By("Patching the Server to available state") + By("Patching the Server to Initial state") Eventually(UpdateStatus(server, func() { server.Status.State = metalv1alpha1.ServerStateInitial })).Should(Succeed()) diff --git a/internal/controller/servermaintenance_controller_test.go b/internal/controller/servermaintenance_controller_test.go index 805db023..0f59638d 100644 --- a/internal/controller/servermaintenance_controller_test.go +++ b/internal/controller/servermaintenance_controller_test.go @@ -21,6 +21,12 @@ var _ = Describe("ServerMaintenance Controller", func() { var server *metalv1alpha1.Server BeforeEach(func(ctx SpecContext) { + By("Ensuring clean state") + var serverList metalv1alpha1.ServerList + Eventually(ObjectList(&serverList)).Should(HaveField("Items", (BeEmpty()))) + var maintenanceList metalv1alpha1.ServerMaintenanceList + Eventually(ObjectList(&maintenanceList)).Should(HaveField("Items", (BeEmpty()))) + By("Creating a BMCSecret") bmcSecret := &metalv1alpha1.BMCSecret{ ObjectMeta: metav1.ObjectMeta{ @@ -53,16 +59,21 @@ var _ = Describe("ServerMaintenance Controller", func() { }, }, } - Expect(k8sClient.Create(ctx, server)).Should(Succeed()) + TransistionServerFromInitialToAvailableState(ctx, k8sClient, server, ns.Name) }) AfterEach(func(ctx SpecContext) { DeleteAllMetalResources(ctx, ns.Name) }) - It("Should force a Server into maintenance", func(ctx SpecContext) { - By("Creating an ServerMaintenance object") + It("Should force a Server into maintenance from Initial State", func(ctx SpecContext) { + + By("patching server to Initial State") + Eventually(UpdateStatus(server, func() { + server.Status.State = metalv1alpha1.ServerStateInitial + })).Should(Succeed()) + By("Creating an ServerMaintenance object") serverMaintenance := &metalv1alpha1.ServerMaintenance{ ObjectMeta: metav1.ObjectMeta{ Name: "test-server-maintenance", @@ -96,6 +107,10 @@ var _ = Describe("ServerMaintenance Controller", func() { }) It("Should wait to put a Server into maintenance until approval", func(ctx SpecContext) { + + serverClaim := BuildServerClaim(ctx, k8sClient, *server, ns.Name, nil, metalv1alpha1.PowerOff, "abc:abc") + TransistionServerToReserveredState(ctx, k8sClient, serverClaim, server, ns.Name) + By("Creating an ServerMaintenance object") serverMaintenance := &metalv1alpha1.ServerMaintenance{ ObjectMeta: metav1.ObjectMeta{ @@ -118,26 +133,6 @@ var _ = Describe("ServerMaintenance Controller", func() { }, }, } - serverClaim := &metalv1alpha1.ServerClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-claim", - Namespace: ns.Name, - }, - Spec: metalv1alpha1.ServerClaimSpec{ - ServerRef: &v1.LocalObjectReference{Name: server.Name}, - }, - } - Expect(k8sClient.Create(ctx, serverClaim)).To(Succeed()) - By("Patching the Server to reserved state") - Eventually(Update(server, func() { - server.Spec.ServerClaimRef = &v1.ObjectReference{ - Name: serverClaim.Name, - Namespace: serverClaim.Namespace, - } - })).Should(Succeed()) - Eventually(UpdateStatus(server, func() { - server.Status.State = metalv1alpha1.ServerStateReserved - })).Should(Succeed()) Expect(k8sClient.Create(ctx, serverMaintenance)).To(Succeed()) Eventually(Object(serverMaintenance)).Should(SatisfyAll( HaveField("Status.State", metalv1alpha1.ServerMaintenanceStatePending), @@ -282,7 +277,7 @@ var _ = Describe("ServerMaintenance Controller", func() { HaveField("Status.State", metalv1alpha1.ServerMaintenanceStatePending), )) - By("Deleting first ServerMaintenance to finish the maintennce on the server") + By("Deleting first ServerMaintenance to finish the maintenance on the server") Eventually(k8sClient.Delete).WithArguments(ctx, serverMaintenance01).Should(Succeed()) Eventually(Object(serverMaintenance02)).Should(SatisfyAll( diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 30849495..d9ca194d 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -135,6 +135,8 @@ var _ = BeforeSuite(func() { defer GinkgoRecover() Expect(registryServer.Start(mgrCtx)).To(Succeed(), "failed to start registry server") }() + + bmc.InitMockUp() }) func SetupTest() *corev1.Namespace { @@ -219,7 +221,7 @@ func SetupTest() *corev1.Namespace { PowerPollingTimeout: 200 * time.Millisecond, BasicAuth: true, }, - DiscoveryTimeout: 500 * time.Millisecond, // Force timeout to be quick for tests + DiscoveryTimeout: time.Second, // Force timeout to be quick for tests }).SetupWithManager(k8sManager)).To(Succeed()) Expect((&ServerClaimReconciler{