diff --git a/api/v1alpha1/constants.go b/api/v1alpha1/constants.go index 94da8a52a..57f2ef7e1 100644 --- a/api/v1alpha1/constants.go +++ b/api/v1alpha1/constants.go @@ -99,6 +99,11 @@ const ( ForceOnServerPower = "force-on-server" // GracefulRestartBMC indicates to gracefully restart the baremetal server's BMC's power. GracefulRestartBMC = "graceful-restart-bmc" + + // ForceSSHResetBMC is set internally by the BMCReconciler on the OperationAnnotation when a + // Redfish reset fails with a 5xx error. On the next reconcile the controller detects this + // annotation and performs the SSH reset directly. + ForceSSHResetBMC = "force-ssh-reset-bmc" ) var AnnotationToRedfishMapping = map[string]schemas.ResetType{ diff --git a/cmd/main.go b/cmd/main.go index 876dbaf9e..1dc540cc3 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -102,6 +102,7 @@ func main() { // nolint: gocyclo bmcFailureResetDelay time.Duration bmcResetResyncInterval time.Duration bmcResetWaitingInterval time.Duration + sshResetTimeout time.Duration serverMaxConcurrentReconciles int serverClaimMaxConcurrentReconciles int dnsRecordTemplatePath string @@ -133,6 +134,8 @@ func main() { // nolint: gocyclo "Defines the interval at which the bmc is polled when bmc reset is in-progress.") flag.DurationVar(&bmcResetWaitingInterval, "bmc-reset-waiting-interval", 2*time.Minute, "Defines the duration which the bmc waits before reconciling again when bmc has been reset.") + flag.DurationVar(&sshResetTimeout, "ssh-reset-timeout", 2*time.Minute, + "Timeout for SSH reset operations.") flag.DurationVar(&maintenanceResyncInterval, "maintenance-resync-interval", 2*time.Minute, "Defines the interval at which the CRD performing maintenance is polled during server maintenance task.") flag.StringVar(&discoveryIgnitionPath, "discovery-ignition-path", "/etc/metal-operator/ignition-template.yaml", @@ -436,6 +439,7 @@ func main() { // nolint: gocyclo EventURL: eventURL, DNSRecordTemplate: dnsRecordTemplate, Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}), + SSHResetTimeout: sshResetTimeout, BMCOptions: bmc.Options{ BasicAuth: true, }, diff --git a/internal/controller/bmc_controller.go b/internal/controller/bmc_controller.go index 65688bade..f4e0b273f 100644 --- a/internal/controller/bmc_controller.go +++ b/internal/controller/bmc_controller.go @@ -60,6 +60,9 @@ type BMCReconciler struct { // DNSRecordTemplatePath is the path to the file containing the DNSRecord template. DNSRecordTemplate string Conditions *conditionutils.Accessor + + // SSHResetTimeout defines the timeout for SSH reset operations (dial + command execution). + SSHResetTimeout time.Duration } // +kubebuilder:rbac:groups=metal.ironcore.dev,resources=endpoints,verbs=get;list;watch @@ -122,6 +125,21 @@ func (r *BMCReconciler) reconcile(ctx context.Context, bmcObj *metalv1alpha1.BMC log.V(1).Info("Skipped BMC reconciliation") return ctrl.Result{}, nil } + // SSH reset takes priority — process it even during the waitForBMCReset window so the + // reset actually runs instead of being deferred until the window expires. + if r.hasSSHResetAnnotation(bmcObj) { + log.V(1).Info("SSH reset annotation detected on unresponsive BMC", "BMC", bmcObj.Name) + bmcBase := bmcObj.DeepCopy() + metautils.DeleteAnnotation(bmcObj, metalv1alpha1.OperationAnnotation) + if err := r.Patch(ctx, bmcObj, client.MergeFrom(bmcBase)); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to remove SSH reset annotation: %w", err) + } + if err := r.resetBMCViaSSH(ctx, bmcObj.Name); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to reset BMC via SSH: %w", err) + } + log.V(1).Info("BMC SSH reset completed", "BMC", bmcObj.Name) + return ctrl.Result{RequeueAfter: r.BMCClientRetryInterval}, nil + } if r.waitForBMCReset(bmcObj, r.BMCResetWaitTime) { log.V(1).Info("Skipped BMC reconciliation while waiting for BMC reset to complete") if err := r.patchBMCStatePending(ctx, bmcObj); err != nil { @@ -135,7 +153,7 @@ func (r *BMCReconciler) reconcile(ctx context.Context, bmcObj *metalv1alpha1.BMC if err != nil { if r.shouldResetBMC(bmcObj) { log.V(1).Info("BMC needs reset, resetting", "BMC", bmcObj.Name) - if err := r.resetBMC(ctx, bmcObj, bmcClient, ReasonAutoReset, bmcAutoResetMessage); err != nil { + if err := r.resetBMC(ctx, bmcObj, bmcClient, nil, ReasonAutoReset, bmcAutoResetMessage); err != nil { return ctrl.Result{}, fmt.Errorf("failed to reset BMC: %w", err) } log.V(1).Info("BMC reset initiated", "BMC", bmcObj.Name) @@ -143,6 +161,20 @@ func (r *BMCReconciler) reconcile(ctx context.Context, bmcObj *metalv1alpha1.BMC RequeueAfter: r.BMCClientRetryInterval, }, nil } + // User-requested reset annotation while BMC is offline — attempt Redfish (will 5xx → schedule SSH). + if r.hasGracefulRestartAnnotation(bmcObj) { + log.V(1).Info("Reset annotation detected on unresponsive BMC, attempting reset", "BMC", bmcObj.Name) + connErr := err // preserve before inner scopes shadow it + bmcBase := bmcObj.DeepCopy() + metautils.DeleteAnnotation(bmcObj, metalv1alpha1.OperationAnnotation) + if err := r.Patch(ctx, bmcObj, client.MergeFrom(bmcBase)); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to remove operation annotation: %w", err) + } + if err := r.resetBMC(ctx, bmcObj, bmcClient, connErr, ReasonUserReset, bmcUserResetMessage); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to reset BMC: %w", err) + } + return ctrl.Result{RequeueAfter: r.BMCClientRetryInterval}, nil + } return ctrl.Result{RequeueAfter: r.BMCClientRetryInterval}, r.updateReadyConditionOnBMCFailure(ctx, bmcObj, err) } defer bmcClient.Logout() @@ -159,7 +191,7 @@ func (r *BMCReconciler) reconcile(ctx context.Context, bmcObj *metalv1alpha1.BMC if err := r.updateConditions(ctx, bmcObj, true, ConditionReady, corev1.ConditionTrue, ReasonConnected, "BMC is connected"); err != nil { return ctrl.Result{}, fmt.Errorf("failed to set BMC connected condition: %w", err) } - if err := r.updateConditions(ctx, bmcObj, false, ConditionReset, corev1.ConditionFalse, "ResetComplete", "BMC reset is complete"); err != nil { + if err := r.updateConditions(ctx, bmcObj, false, ConditionReset, corev1.ConditionFalse, ReasonResetComplete, "BMC reset is complete"); err != nil { return ctrl.Result{}, fmt.Errorf("failed to set BMC reset complete condition: %w", err) } @@ -362,7 +394,7 @@ func (r *BMCReconciler) handleAnnotationOperations(ctx context.Context, bmcObj * switch value { case schemas.GracefulRestartResetType: log.V(1).Info("Handling operation", "Operation", operation, "RedfishResetType", value) - if err := r.resetBMC(ctx, bmcObj, bmcClient, ReasonUserReset, bmcUserResetMessage); err != nil { + if err := r.resetBMC(ctx, bmcObj, bmcClient, nil, ReasonUserReset, bmcUserResetMessage); err != nil { return false, fmt.Errorf("failed to reset BMC: %w", err) } log.Info("Handled operation", "Operation", operation) @@ -436,7 +468,8 @@ func (r *BMCReconciler) handlePreviousBMCResetAnnotations(ctx context.Context, b return false, nil } if condition.Status == metav1.ConditionTrue { - if operation, ok := bmcObj.GetAnnotations()[metalv1alpha1.OperationAnnotation]; ok && operation == metalv1alpha1.GracefulRestartBMC { + if operation, ok := bmcObj.GetAnnotations()[metalv1alpha1.OperationAnnotation]; ok && + (operation == metalv1alpha1.GracefulRestartBMC || operation == metalv1alpha1.ForceSSHResetBMC) { bmcBase := bmcObj.DeepCopy() metautils.DeleteAnnotation(bmcObj, metalv1alpha1.OperationAnnotation) if err := r.Patch(ctx, bmcObj, client.MergeFrom(bmcBase)); err != nil { @@ -483,15 +516,26 @@ func (r *BMCReconciler) patchBMCStatePending(ctx context.Context, bmcObj *metalv return nil } -func (r *BMCReconciler) resetBMC(ctx context.Context, bmcObj *metalv1alpha1.BMC, bmcClient bmc.BMC, reason, message string) error { +func (r *BMCReconciler) resetBMC(ctx context.Context, bmcObj *metalv1alpha1.BMC, bmcClient bmc.BMC, clientErr error, reason, message string) error { log := ctrl.LoggerFrom(ctx) if err := r.updateConditions(ctx, bmcObj, true, ConditionReset, corev1.ConditionTrue, reason, message); err != nil { return fmt.Errorf("failed to set BMC resetting condition: %w", err) } if bmcClient == nil { - // No client connection at all (e.g. auto-reset reached with a nil - // client from GetBMCClientFromBMC). No Redfish call possible — surface - // it instead of logging a nil error. + // No Redfish client available. If the connection error was a 5xx (e.g. 503 + // from the BMC during gofish connect), schedule an SSH retry. + if clientErr != nil { + if httpErr, ok := errors.AsType[*schemas.Error](clientErr); ok && + httpErr.HTTPReturnedStatusCode >= 500 && httpErr.HTTPReturnedStatusCode < 600 { + bmcBase := bmcObj.DeepCopy() + metautils.SetAnnotation(bmcObj, metalv1alpha1.OperationAnnotation, metalv1alpha1.ForceSSHResetBMC) + if patchErr := r.Patch(ctx, bmcObj, client.MergeFrom(bmcBase)); patchErr != nil { + return fmt.Errorf("failed to set SSH reset annotation: %w", patchErr) + } + log.Info("Scheduled SSH-based BMC reset due to connection 5xx error", "BMC", bmcObj.Name) + return r.patchBMCStatePending(ctx, bmcObj) + } + } return errors.Join( r.patchBMCStatePending(ctx, bmcObj), fmt.Errorf("could not reset BMC %s: no client connection", bmcObj.Name), @@ -500,17 +544,85 @@ func (r *BMCReconciler) resetBMC(ctx context.Context, bmcObj *metalv1alpha1.BMC, if err := bmcClient.ResetManager(ctx, bmcObj.Spec.BMCUUID, schemas.GracefulRestartResetType); err == nil { log.Info("Successfully reset BMC via Redfish", "BMC", bmcObj.Name) return r.patchBMCStatePending(ctx, bmcObj) + } else if httpErr, ok := errors.AsType[*schemas.Error](err); ok { + // only retryable on 5xx; anything else is a permanent failure for this attempt + if httpErr.HTTPReturnedStatusCode < 500 || httpErr.HTTPReturnedStatusCode >= 600 { + return errors.Join(r.patchBMCStatePending(ctx, bmcObj), fmt.Errorf("could not reset BMC: %w", err)) + } + // 5xx — schedule SSH reset via annotation so the next reconcile handles it + // in-cluster state drives the retry; no separate goroutine needed. + bmcBase := bmcObj.DeepCopy() + metautils.SetAnnotation(bmcObj, metalv1alpha1.OperationAnnotation, metalv1alpha1.ForceSSHResetBMC) + if patchErr := r.Patch(ctx, bmcObj, client.MergeFrom(bmcBase)); patchErr != nil { + return fmt.Errorf("failed to set SSH reset annotation: %w", patchErr) + } + log.Info("Scheduled SSH-based BMC reset due to Redfish 5xx error", "BMC", bmcObj.Name) + return r.patchBMCStatePending(ctx, bmcObj) } else { - if httpErr, ok := errors.AsType[*schemas.Error](err); ok { - // only retryable on 5xx; anything else is a permanent failure for this attempt - if httpErr.HTTPReturnedStatusCode < 500 || httpErr.HTTPReturnedStatusCode >= 600 { - return errors.Join(r.patchBMCStatePending(ctx, bmcObj), fmt.Errorf("could not reset BMC: %w", err)) - } - } else { - return fmt.Errorf("could not reset BMC, unknown error: %w", err) + return fmt.Errorf("could not reset BMC, unknown error: %w", err) + } +} + +func (r *BMCReconciler) hasSSHResetAnnotation(bmcObj *metalv1alpha1.BMC) bool { + operation, ok := bmcObj.GetAnnotations()[metalv1alpha1.OperationAnnotation] + return ok && operation == metalv1alpha1.ForceSSHResetBMC +} + +func (r *BMCReconciler) hasGracefulRestartAnnotation(bmcObj *metalv1alpha1.BMC) bool { + operation, ok := bmcObj.GetAnnotations()[metalv1alpha1.OperationAnnotation] + return ok && operation == metalv1alpha1.GracefulRestartBMC +} + +func (r *BMCReconciler) resetBMCViaSSH(ctx context.Context, bmcName string) error { + log := ctrl.LoggerFrom(ctx).WithValues("BMC", bmcName) + log.V(1).Info("Starting SSH-based BMC reset") + + currentBMC := &metalv1alpha1.BMC{} + if err := r.Get(ctx, client.ObjectKey{Name: bmcName}, currentBMC); err != nil { + return fmt.Errorf("failed to fetch BMC object for SSH reset: %w", err) + } + address, err := bmcutils.GetBMCAddressForBMC(ctx, r.Client, currentBMC) + if err != nil { + _ = r.updateConditions(ctx, currentBMC, true, ConditionReset, corev1.ConditionFalse, ReasonConnectionFailed, fmt.Sprintf("Failed to get BMC address: %v", err)) + return fmt.Errorf("failed to get BMC address for SSH reset: %w", err) + } + manufacturer := currentBMC.Status.Manufacturer + if manufacturer == "" { + log.V(1).Info("BMC manufacturer not available, attempting to get manufacturer from Server") + serverList := &metalv1alpha1.ServerList{} + if err := r.List(ctx, serverList, client.MatchingFields{bmcRefField: currentBMC.Name}); err != nil { + log.Error(err, "Failed to list Servers for BMC to get manufacturer fallback") + } else if len(serverList.Items) > 0 && serverList.Items[0].Status.Manufacturer != "" { + manufacturer = serverList.Items[0].Status.Manufacturer + log.V(1).Info("Using manufacturer from Server as fallback", "manufacturer", manufacturer, "server", serverList.Items[0].Name) } } - log.V(1).Info("BMC reset returned a retryable 5xx, leaving reset condition set", "BMC", bmcObj.Name) + if manufacturer == "" { + _ = r.updateConditions(ctx, currentBMC, true, ConditionReset, corev1.ConditionFalse, ReasonInternalError, "BMC manufacturer not available") + return fmt.Errorf("BMC manufacturer not available for SSH reset") + } + username, password, err := bmcutils.GetBMCCredentialsForBMCSecretName(ctx, r.Client, currentBMC.Spec.BMCSecretRef.Name) + if err != nil { + _ = r.updateConditions(ctx, currentBMC, true, ConditionReset, corev1.ConditionFalse, ReasonAuthenticationFailed, fmt.Sprintf("Failed to get credentials: %v", err)) + return fmt.Errorf("failed to get BMC credentials for SSH reset: %w", err) + } + if err := bmcutils.SSHResetBMCFunc(ctx, address, manufacturer, username, password, r.SSHResetTimeout); err != nil { + _ = r.updateConditions(ctx, currentBMC, true, ConditionReset, corev1.ConditionFalse, ReasonInternalError, fmt.Sprintf("SSH reset failed: %v", err)) + return fmt.Errorf("SSH reset failed: %w", err) + } + log.Info("Successfully reset BMC via SSH") + + // ConditionReset is not cleared here — it is cleared when the BMC reconnects + // successfully via handlePreviousBMCResetAnnotations. + if err := r.Get(ctx, client.ObjectKey{Name: bmcName}, currentBMC); err != nil { + return fmt.Errorf("failed to re-fetch BMC after SSH reset: %w", err) + } + bmcBase := currentBMC.DeepCopy() + now := metav1.Now() + currentBMC.Status.LastResetTime = &now + if err := r.Status().Patch(ctx, currentBMC, client.MergeFrom(bmcBase)); err != nil { + return fmt.Errorf("failed to patch LastResetTime after SSH reset: %w", err) + } return nil } diff --git a/internal/controller/bmc_controller_test.go b/internal/controller/bmc_controller_test.go index c19dc1087..125497a44 100644 --- a/internal/controller/bmc_controller_test.go +++ b/internal/controller/bmc_controller_test.go @@ -4,7 +4,10 @@ package controller import ( + "context" + "fmt" "maps" + "sync" "time" metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" @@ -673,7 +676,7 @@ var _ = Describe("BMC Conditions", func() { SatisfyAll( HaveField("Type", ConditionReset), HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", "ResetComplete"), + HaveField("Reason", ReasonResetComplete), ), )), ) @@ -692,3 +695,220 @@ var _ = Describe("BMC Conditions", func() { Eventually(Get(server)).Should(Satisfy(apierrors.IsNotFound)) }) }) + +// createBMCSecretForSSHTest creates a BMCSecret with known credentials for SSH reset tests. +func createBMCSecretForSSHTest(ctx SpecContext) *metalv1alpha1.BMCSecret { + secret := &metalv1alpha1.BMCSecret{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "test-ssh-"}, + Data: map[string][]byte{ + metalv1alpha1.BMCSecretUsernameKeyName: []byte("foo"), + metalv1alpha1.BMCSecretPasswordKeyName: []byte("bar"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + return secret +} + +// createBMCForSSHTest creates a BMC pointing at the mock server for SSH reset tests. +func createBMCForSSHTest(ctx SpecContext, secret *metalv1alpha1.BMCSecret) *metalv1alpha1.BMC { + b := &metalv1alpha1.BMC{ + ObjectMeta: metav1.ObjectMeta{GenerateName: "test-ssh-bmc-"}, + Spec: metalv1alpha1.BMCSpec{ + Endpoint: &metalv1alpha1.InlineEndpoint{ + IP: metalv1alpha1.MustParseIP(MockServerIP), + MACAddress: "aa:bb:cc:dd:ee:ff", + }, + Protocol: metalv1alpha1.Protocol{ + Name: metalv1alpha1.ProtocolRedfishLocal, + Port: MockServerPort, + }, + BMCSecretRef: v1.LocalObjectReference{Name: secret.Name}, + }, + } + Expect(k8sClient.Create(ctx, b)).To(Succeed()) + return b +} + +var _ = Describe("BMC SSH Reset", func() { + _ = SetupTest(nil) + + AfterEach(func(ctx SpecContext) { + bmcutils.SSHResetBMCFunc = bmcutils.SSHResetBMC + mockServers[0].SetUnavailable(false) + // Use a longer timeout since BMC may be in a reset-wait state that delays cleanup. + Eventually(func(g Gomega) { + list := &metalv1alpha1.BMCList{} + g.Expect(k8sClient.List(ctx, list)).To(Succeed()) + g.Expect(list.Items).To(BeEmpty()) + }).WithTimeout(30 * time.Second).Should(Succeed()) + Eventually(func(g Gomega) { + list := &metalv1alpha1.ServerList{} + g.Expect(k8sClient.List(ctx, list)).To(Succeed()) + g.Expect(list.Items).To(BeEmpty()) + }).WithTimeout(30 * time.Second).Should(Succeed()) + }) + + It("Should successfully perform SSH reset after Redfish 5xx error", func(ctx SpecContext) { + By("Setting up mock SSH function") + var mu sync.Mutex + sshResetCalled := false + var capturedIP, capturedManufacturer, capturedUsername, capturedPassword string + var capturedTimeout time.Duration + bmcutils.SSHResetBMCFunc = func(_ context.Context, ip, manufacturer, username, password string, timeout time.Duration) error { + mu.Lock() + defer mu.Unlock() + sshResetCalled = true + capturedIP = ip + capturedManufacturer = manufacturer + capturedUsername = username + capturedPassword = password + capturedTimeout = timeout + return nil + } + + By("Creating BMC and waiting for it to become Ready with manufacturer info") + bmcSecret := createBMCSecretForSSHTest(ctx) + bmc := createBMCForSSHTest(ctx, bmcSecret) + Eventually(Object(bmc)).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", "Ready"), + HaveField("Status", metav1.ConditionTrue), + )))) + Eventually(Object(bmc)).WithTimeout(10 * time.Second).Should(HaveField("Status.Manufacturer", Not(BeEmpty()))) + + By("Simulating Redfish 503 and adding reset annotation") + mockServers[0].SetUnavailable(true) + Eventually(Update(bmc, func() { + if bmc.Annotations == nil { + bmc.Annotations = map[string]string{} + } + bmc.Annotations[metalv1alpha1.OperationAnnotation] = metalv1alpha1.GracefulRestartBMC + })).Should(Succeed()) + + By("Waiting for SSH reset to be triggered") + Eventually(func() bool { + mu.Lock() + defer mu.Unlock() + return sshResetCalled + }).WithTimeout(15 * time.Second).WithPolling(100 * time.Millisecond).Should(BeTrue()) + + By("Verifying SSH was called with correct parameters") + mu.Lock() + Expect(capturedIP).To(Equal(MockServerIP)) + Expect(capturedManufacturer).NotTo(BeEmpty()) + Expect(capturedUsername).To(Equal("foo")) + Expect(capturedPassword).To(Equal("bar")) + Expect(capturedTimeout).To(Equal(1 * time.Second)) + mu.Unlock() + + By("Verifying Reset condition is True and annotation was removed") + Eventually(Object(bmc)).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", "Reset"), + HaveField("Status", metav1.ConditionTrue), + )))) + Eventually(Object(bmc)).Should(HaveField("Annotations", Not(HaveKey(metalv1alpha1.OperationAnnotation)))) + + By("Verifying LastResetTime was updated") + Eventually(Object(bmc)).Should(HaveField("Status.LastResetTime", Not(BeNil()))) + + mockServers[0].SetUnavailable(false) + server := &metalv1alpha1.Server{ObjectMeta: metav1.ObjectMeta{Name: bmcutils.GetServerNameFromBMCandIndex(0, bmc)}} + Expect(k8sClient.Delete(ctx, bmc)).To(Succeed()) + Expect(k8sClient.Delete(ctx, bmcSecret)).To(Succeed()) + _ = k8sClient.Delete(ctx, server) + }) + + It("Should handle SSH reset connection failure gracefully", func(ctx SpecContext) { + By("Setting up mock SSH function to return error") + bmcutils.SSHResetBMCFunc = func(_ context.Context, ip, manufacturer, username, password string, timeout time.Duration) error { + return fmt.Errorf("connection refused") + } + + By("Creating BMC and waiting for manufacturer info") + bmcSecret := createBMCSecretForSSHTest(ctx) + bmc := createBMCForSSHTest(ctx, bmcSecret) + Eventually(Object(bmc)).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", "Ready"), + HaveField("Status", metav1.ConditionTrue), + )))) + Eventually(Object(bmc)).WithTimeout(10 * time.Second).Should(HaveField("Status.Manufacturer", Not(BeEmpty()))) + + By("Simulating Redfish unavailability and triggering reset") + mockServers[0].SetUnavailable(true) + Eventually(Update(bmc, func() { + if bmc.Annotations == nil { + bmc.Annotations = map[string]string{} + } + bmc.Annotations[metalv1alpha1.OperationAnnotation] = metalv1alpha1.GracefulRestartBMC + })).Should(Succeed()) + + By("Waiting for SSH reset failure condition") + Eventually(Object(bmc), 10*time.Second).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", "Reset"), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", "InternalServerError"), + HaveField("Message", ContainSubstring("SSH reset failed")), + )))) + + mockServers[0].SetUnavailable(false) + server := &metalv1alpha1.Server{ObjectMeta: metav1.ObjectMeta{Name: bmcutils.GetServerNameFromBMCandIndex(0, bmc)}} + Expect(k8sClient.Delete(ctx, bmc)).To(Succeed()) + Expect(k8sClient.Delete(ctx, bmcSecret)).To(Succeed()) + _ = k8sClient.Delete(ctx, server) + }) + + It("Should not trigger duplicate SSH resets during wait period", func(ctx SpecContext) { + By("Setting up mock SSH function with call counter") + var mu sync.Mutex + callCount := 0 + bmcutils.SSHResetBMCFunc = func(_ context.Context, ip, manufacturer, username, password string, timeout time.Duration) error { + mu.Lock() + defer mu.Unlock() + callCount++ + return nil + } + + By("Creating BMC and waiting for Ready + manufacturer") + bmcSecret := createBMCSecretForSSHTest(ctx) + bmc := createBMCForSSHTest(ctx, bmcSecret) + Eventually(Object(bmc)).Should(SatisfyAll( + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", "Ready"), + HaveField("Status", metav1.ConditionTrue), + ))), + HaveField("Status.Manufacturer", Not(BeEmpty())), + )) + + By("Simulating Redfish unavailability and triggering reset") + mockServers[0].SetUnavailable(true) + Eventually(Update(bmc, func() { + if bmc.Annotations == nil { + bmc.Annotations = map[string]string{} + } + bmc.Annotations[metalv1alpha1.OperationAnnotation] = metalv1alpha1.GracefulRestartBMC + })).Should(Succeed()) + + By("Waiting for first SSH reset") + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return callCount + }).Should(Equal(1)) + + By("Verifying Reset condition True prevents re-triggering") + Eventually(Object(bmc)).Should(HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", "Reset"), + HaveField("Status", metav1.ConditionTrue), + )))) + + time.Sleep(500 * time.Millisecond) + mu.Lock() + Expect(callCount).To(Equal(1)) + mu.Unlock() + + mockServers[0].SetUnavailable(false) + server := &metalv1alpha1.Server{ObjectMeta: metav1.ObjectMeta{Name: bmcutils.GetServerNameFromBMCandIndex(0, bmc)}} + Expect(k8sClient.Delete(ctx, bmc)).To(Succeed()) + Expect(k8sClient.Delete(ctx, bmcSecret)).To(Succeed()) + _ = k8sClient.Delete(ctx, server) + }) +}) diff --git a/internal/controller/conditions.go b/internal/controller/conditions.go index fbb34302d..546114f27 100644 --- a/internal/controller/conditions.go +++ b/internal/controller/conditions.go @@ -87,4 +87,6 @@ const ( ReasonWaitingForPowerOff = "WaitingForPowerOff" // ReasonPowerOffConfirmed marks WaitingForPowerOff as resolved. ReasonPowerOffConfirmed = "PowerOffConfirmed" + // ReasonResetComplete indicates a BMC reset has completed successfully. + ReasonResetComplete = "ResetComplete" ) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 4c6837750..cfc13122c 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -185,6 +185,7 @@ func SetupTest(redfishMockServers []netip.AddrPort) *corev1.Namespace { ManagerNamespace: ns.Name, BMCResetWaitTime: 400 * time.Millisecond, BMCClientRetryInterval: 25 * time.Millisecond, + SSHResetTimeout: 1 * time.Second, EventURL: "http://localhost:8008", DNSRecordTemplate: dnsTemplate, Conditions: accessor, diff --git a/pkg/bmcutils/bmcutils.go b/pkg/bmcutils/bmcutils.go index 4a9c6731e..b84a30fd6 100644 --- a/pkg/bmcutils/bmcutils.go +++ b/pkg/bmcutils/bmcutils.go @@ -261,6 +261,10 @@ func GetServerNameFromBMCandIndex(index int, bmcObj *metalv1alpha1.BMC) string { return fmt.Sprintf("%s-%s-%d", bmcObj.Name, "system", index) } +// SSHResetBMCFunc is the function used to perform SSH-based BMC resets. +// It can be replaced in tests to inject a mock implementation. +var SSHResetBMCFunc = SSHResetBMC + func SSHResetBMC(ctx context.Context, ip, manufacturer, username, password string, timeout time.Duration) error { // If Redfish reset fails, try SSH-based reset for known manufacturers config := &ssh.ClientConfig{