diff --git a/api/v1alpha1/applyconfiguration/api/v1alpha1/bmcstatus.go b/api/v1alpha1/applyconfiguration/api/v1alpha1/bmcstatus.go
index 724ab33d1..a0dafefc9 100644
--- a/api/v1alpha1/applyconfiguration/api/v1alpha1/bmcstatus.go
+++ b/api/v1alpha1/applyconfiguration/api/v1alpha1/bmcstatus.go
@@ -30,8 +30,7 @@ type BMCStatusApplyConfiguration struct {
SerialNumber *string `json:"serialNumber,omitempty"`
// FirmwareVersion is the version of the firmware currently running on the BMC.
FirmwareVersion *string `json:"firmwareVersion,omitempty"`
- // State represents the current state of the BMC.
- // kubebuilder:validation:Enum=Enabled;Error;Pending
+ // State represents the current state of the BMC as reported by Redfish.
State *apiv1alpha1.BMCState `json:"state,omitempty"`
// PowerState represents the current power state of the BMC.
PowerState *apiv1alpha1.BMCPowerState `json:"powerState,omitempty"`
diff --git a/api/v1alpha1/applyconfiguration/internal/internal.go b/api/v1alpha1/applyconfiguration/internal/internal.go
index 26302b2d4..40dbab1df 100644
--- a/api/v1alpha1/applyconfiguration/internal/internal.go
+++ b/api/v1alpha1/applyconfiguration/internal/internal.go
@@ -592,7 +592,6 @@ var schemaYAML = typed.YAMLObject(`types:
- name: state
type:
namedType: com.github.ironcore-dev.metal-operator.api.v1alpha1.BMCState
- default: Pending
- name: com.github.ironcore-dev.metal-operator.api.v1alpha1.BMCUser
map:
fields:
diff --git a/api/v1alpha1/bmc_types.go b/api/v1alpha1/bmc_types.go
index 2668a951f..8c65bd339 100644
--- a/api/v1alpha1/bmc_types.go
+++ b/api/v1alpha1/bmc_types.go
@@ -197,9 +197,7 @@ type BMCStatus struct {
// +optional
FirmwareVersion string `json:"firmwareVersion,omitempty"`
- // State represents the current state of the BMC.
- // kubebuilder:validation:Enum=Enabled;Error;Pending
- // +kubebuilder:default=Pending
+ // State represents the current state of the BMC as reported by Redfish.
// +optional
State BMCState `json:"state,omitempty"`
@@ -232,12 +230,6 @@ type BMCState string
const (
// BMCStateEnabled indicates that the BMC is enabled and functioning correctly.
BMCStateEnabled BMCState = "Enabled"
-
- // BMCStateError indicates that there is an error with the BMC.
- BMCStateError BMCState = "Error"
-
- // BMCStatePending indicates that there is an error connecting with the BMC.
- BMCStatePending BMCState = "Pending"
)
// +kubebuilder:object:root=true
diff --git a/cmd/main.go b/cmd/main.go
index 876dbaf9e..0b7f268db 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -100,7 +100,7 @@ func main() { // nolint: gocyclo
discoveryTimeout time.Duration
biosSettingsApplyTimeout time.Duration
bmcFailureResetDelay time.Duration
- bmcResetResyncInterval time.Duration
+ bmcReconnectInterval time.Duration
bmcResetWaitingInterval time.Duration
serverMaxConcurrentReconciles int
serverClaimMaxConcurrentReconciles int
@@ -129,8 +129,8 @@ func main() { // nolint: gocyclo
"Defines the interval at which the server is polled.")
flag.DurationVar(&bmcFailureResetDelay, "bmc-failure-reset-delay", 0,
"Reset the BMC after this duration of consecutive failures. 0 to disable.")
- flag.DurationVar(&bmcResetResyncInterval, "bmc-reset-resync-interval", 2*time.Minute,
- "Defines the interval at which the bmc is polled when bmc reset is in-progress.")
+ flag.DurationVar(&bmcReconnectInterval, "bmc-reconnect-interval", 2*time.Minute,
+ "Defines the interval at which the BMC is requeued after a connection failure or while a 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(&maintenanceResyncInterval, "maintenance-resync-interval", 2*time.Minute,
@@ -425,18 +425,18 @@ func main() { // nolint: gocyclo
os.Exit(1)
}
if err = (&controller.BMCReconciler{
- Client: mgr.GetClient(),
- Scheme: mgr.GetScheme(),
- DefaultProtocol: effectiveProtocol,
- SkipCertValidation: effectiveSkipCert,
- BMCFailureResetDelay: bmcFailureResetDelay,
- BMCResetWaitTime: bmcResetWaitingInterval,
- BMCClientRetryInterval: bmcResetResyncInterval,
- ManagerNamespace: managerNamespace,
- EventURL: eventURL,
- DNSRecordTemplate: dnsRecordTemplate,
- Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
- BMCOptions: bmc.Options{
+ Client: mgr.GetClient(),
+ Scheme: mgr.GetScheme(),
+ DefaultProtocol: effectiveProtocol,
+ SkipCertValidation: effectiveSkipCert,
+ FailureResetDelay: bmcFailureResetDelay,
+ ResetWaitTime: bmcResetWaitingInterval,
+ ReconnectInterval: bmcReconnectInterval,
+ ManagerNamespace: managerNamespace,
+ EventURL: eventURL,
+ DNSRecordTemplate: dnsRecordTemplate,
+ Conditions: conditionutils.NewAccessor(conditionutils.AccessorOptions{}),
+ Options: bmc.Options{
BasicAuth: true,
},
}).SetupWithManager(mgr); err != nil {
diff --git a/config/crd/bases/metal.ironcore.dev_bmcs.yaml b/config/crd/bases/metal.ironcore.dev_bmcs.yaml
index 3503c3097..391a5215b 100644
--- a/config/crd/bases/metal.ironcore.dev_bmcs.yaml
+++ b/config/crd/bases/metal.ironcore.dev_bmcs.yaml
@@ -291,10 +291,8 @@ spec:
description: SKU is the stock keeping unit identifier for the BMC.
type: string
state:
- default: Pending
- description: |-
- State represents the current state of the BMC.
- kubebuilder:validation:Enum=Enabled;Error;Pending
+ description: State represents the current state of the BMC as reported
+ by Redfish.
type: string
type: object
type: object
diff --git a/dist/chart/templates/crd/bmcs.metal.ironcore.dev.yaml b/dist/chart/templates/crd/bmcs.metal.ironcore.dev.yaml
index 00cfc597f..aeaf5245e 100644
--- a/dist/chart/templates/crd/bmcs.metal.ironcore.dev.yaml
+++ b/dist/chart/templates/crd/bmcs.metal.ironcore.dev.yaml
@@ -305,10 +305,8 @@ spec:
description: SKU is the stock keeping unit identifier for the BMC.
type: string
state:
- default: Pending
- description: |-
- State represents the current state of the BMC.
- kubebuilder:validation:Enum=Enabled;Error;Pending
+ description: State represents the current state of the BMC as reported
+ by Redfish.
type: string
type: object
type: object
diff --git a/dist/install.yaml b/dist/install.yaml
index 256c91668..e4f0fcf58 100644
--- a/dist/install.yaml
+++ b/dist/install.yaml
@@ -1344,10 +1344,8 @@ spec:
description: SKU is the stock keeping unit identifier for the BMC.
type: string
state:
- default: Pending
- description: |-
- State represents the current state of the BMC.
- kubebuilder:validation:Enum=Enabled;Error;Pending
+ description: State represents the current state of the BMC as reported
+ by Redfish.
type: string
type: object
type: object
diff --git a/docs/api-reference/api.md b/docs/api-reference/api.md
index d7da7319b..c8f00a0d4 100644
--- a/docs/api-reference/api.md
+++ b/docs/api-reference/api.md
@@ -670,8 +670,6 @@ _Appears in:_
| Field | Description |
| --- | --- |
| `Enabled` | BMCStateEnabled indicates that the BMC is enabled and functioning correctly.
|
-| `Error` | BMCStateError indicates that there is an error with the BMC.
|
-| `Pending` | BMCStatePending indicates that there is an error connecting with the BMC.
|
#### BMCStatus
@@ -694,7 +692,7 @@ _Appears in:_
| `sku` _string_ | SKU is the stock keeping unit identifier for the BMC. | | |
| `serialNumber` _string_ | SerialNumber is the serial number of the BMC. | | |
| `firmwareVersion` _string_ | FirmwareVersion is the version of the firmware currently running on the BMC. | | |
-| `state` _[BMCState](#bmcstate)_ | State represents the current state of the BMC.
kubebuilder:validation:Enum=Enabled;Error;Pending | Pending | |
+| `state` _[BMCState](#bmcstate)_ | State represents the current state of the BMC as reported by Redfish. | | |
| `powerState` _[BMCPowerState](#bmcpowerstate)_ | PowerState represents the current power state of the BMC. | | |
| `lastResetTime` _[Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.36/#time-v1-meta)_ | LastResetTime is the timestamp of the last reset operation performed on the BMC. | | |
| `metricsReportSubscriptionLink` _string_ | MetricsReportSubscriptionLink is the link to the metrics report subscription of the bmc. | | |
diff --git a/internal/controller/bmc_controller.go b/internal/controller/bmc_controller.go
index 09abbc281..278040040 100644
--- a/internal/controller/bmc_controller.go
+++ b/internal/controller/bmc_controller.go
@@ -48,18 +48,14 @@ type BMCReconciler struct {
Scheme *runtime.Scheme
DefaultProtocol metalv1alpha1.ProtocolScheme
SkipCertValidation bool
- // BMCFailureResetDelay defines the duration after which a BMC will be reset upon repeated connection failures.
- BMCFailureResetDelay time.Duration
- BMCOptions bmc.Options
- ManagerNamespace string
- EventURL string
- // BMCResetWaitTime defines the duration to wait after a BMC reset before attempting reconciliation again.
- BMCResetWaitTime time.Duration
- // BMCClientRetryInterval defines the duration to requeue reconciliation after a BMC client error/reset/unavailablility.
- BMCClientRetryInterval time.Duration
- // DNSRecordTemplatePath is the path to the file containing the DNSRecord template.
- DNSRecordTemplate string
- Conditions *conditionutils.Accessor
+ FailureResetDelay time.Duration
+ Options bmc.Options
+ ManagerNamespace string
+ EventURL string
+ ResetWaitTime time.Duration
+ ReconnectInterval time.Duration
+ DNSRecordTemplate string
+ Conditions *conditionutils.Accessor
}
// +kubebuilder:rbac:groups=metal.ironcore.dev,resources=endpoints,verbs=get;list;watch
@@ -83,6 +79,7 @@ func (r *BMCReconciler) reconcileExists(ctx context.Context, bmcObj *metalv1alph
if !bmcObj.DeletionTimestamp.IsZero() {
return r.delete(ctx, bmcObj)
}
+
return r.reconcile(ctx, bmcObj)
}
@@ -99,7 +96,7 @@ func (r *BMCReconciler) delete(ctx context.Context, bmcObj *metalv1alpha1.BMC) (
}
}
- bmcClient, err := bmcutils.GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions)
+ bmcClient, err := bmcutils.GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.DefaultProtocol, r.SkipCertValidation, r.Options)
if err == nil {
defer bmcClient.Logout()
if err := r.deleteEventSubscription(ctx, bmcClient, bmcObj); err != nil {
@@ -122,16 +119,21 @@ func (r *BMCReconciler) reconcile(ctx context.Context, bmcObj *metalv1alpha1.BMC
log.V(1).Info("Skipped BMC reconciliation")
return ctrl.Result{}, nil
}
- if r.waitForBMCReset(bmcObj, r.BMCResetWaitTime) {
+
+ switch r.resetWaitState(bmcObj) {
+ case resetWaitPending:
log.V(1).Info("Skipped BMC reconciliation while waiting for BMC reset to complete")
- if err := r.patchBMCStatePending(ctx, bmcObj); err != nil {
+ return ctrl.Result{RequeueAfter: r.ReconnectInterval}, nil
+ case resetWaitExpired:
+ // The reset wait window elapsed but ConditionReset is still True. Clear
+ // it so shouldResetBMC can schedule future auto-resets; otherwise the
+ // stuck True condition blocks all recovery while the BMC stays down.
+ if err := r.clearResetCondition(ctx, bmcObj, ReasonResetTimeout, "BMC reset timed out waiting for reconnect"); err != nil {
return ctrl.Result{}, err
}
- return ctrl.Result{
- RequeueAfter: r.BMCClientRetryInterval,
- }, nil
}
- bmcClient, err := bmcutils.GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.DefaultProtocol, r.SkipCertValidation, r.BMCOptions, bmcutils.BMCConnectivityCheckOption)
+
+ bmcClient, err := bmcutils.GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.DefaultProtocol, r.SkipCertValidation, r.Options, bmcutils.BMCConnectivityCheckOption)
if err != nil {
if r.shouldResetBMC(bmcObj) {
log.V(1).Info("BMC needs reset, resetting", "BMC", bmcObj.Name)
@@ -140,30 +142,44 @@ func (r *BMCReconciler) reconcile(ctx context.Context, bmcObj *metalv1alpha1.BMC
}
log.V(1).Info("BMC reset initiated", "BMC", bmcObj.Name)
return ctrl.Result{
- RequeueAfter: r.BMCClientRetryInterval,
+ RequeueAfter: r.ReconnectInterval,
}, nil
}
- return ctrl.Result{RequeueAfter: r.BMCClientRetryInterval}, r.updateReadyConditionOnBMCFailure(ctx, bmcObj, err)
+ return ctrl.Result{RequeueAfter: r.ReconnectInterval}, r.updateReadyConditionOnBMCFailure(ctx, bmcObj, err)
}
defer bmcClient.Logout()
- // if BMC reset was issued and is successful, ensure to remove previous reset annotation
- if modified, err := r.handlePreviousBMCResetAnnotations(ctx, bmcObj); err != nil || modified {
- return ctrl.Result{}, err
- }
-
if modified, err := r.handleAnnotationOperations(ctx, bmcObj, bmcClient); err != nil || modified {
return ctrl.Result{}, err
}
- if err := r.updateConditions(ctx, bmcObj, true, ConditionReady, corev1.ConditionTrue, ReasonConnected, "BMC is connected"); err != nil {
+ // Mark BMC connected and clear any in-flight reset in a single status patch.
+ bmcBase := bmcObj.DeepCopy()
+ if err := r.Conditions.UpdateSlice(
+ &bmcObj.Status.Conditions,
+ ConditionReady,
+ conditionutils.UpdateStatus(corev1.ConditionTrue),
+ conditionutils.UpdateReason(ReasonConnected),
+ conditionutils.UpdateMessage("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 {
- return ctrl.Result{}, fmt.Errorf("failed to set BMC reset complete condition: %w", err)
+ if found, _ := r.Conditions.FindSlice(bmcObj.Status.Conditions, ConditionReset, &metav1.Condition{}); found {
+ if err := r.Conditions.UpdateSlice(
+ &bmcObj.Status.Conditions,
+ ConditionReset,
+ conditionutils.UpdateStatus(corev1.ConditionFalse),
+ conditionutils.UpdateReason(ReasonResetComplete),
+ conditionutils.UpdateMessage("BMC reset is complete"),
+ ); err != nil {
+ return ctrl.Result{}, fmt.Errorf("failed to set BMC reset complete condition: %w", err)
+ }
+ }
+ if err := r.Status().Patch(ctx, bmcObj, client.MergeFrom(bmcBase)); err != nil {
+ return ctrl.Result{}, fmt.Errorf("failed to patch BMC conditions: %w", err)
}
- if err := r.updateBMCStatusDetails(ctx, bmcClient, bmcObj); err != nil {
+ if err := r.patchBMCStatus(ctx, bmcClient, bmcObj); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to update BMC status: %w", err)
}
log.V(1).Info("Updated BMC status", "State", bmcObj.Status.State)
@@ -188,7 +204,7 @@ func (r *BMCReconciler) reconcile(ctx context.Context, bmcObj *metalv1alpha1.BMC
return ctrl.Result{}, nil
}
-func (r *BMCReconciler) updateBMCStatusDetails(ctx context.Context, bmcClient bmc.BMC, bmcObj *metalv1alpha1.BMC) error {
+func (r *BMCReconciler) patchBMCStatus(ctx context.Context, bmcClient bmc.BMC, bmcObj *metalv1alpha1.BMC) error {
log := ctrl.LoggerFrom(ctx)
var (
ip metalv1alpha1.IP
@@ -359,171 +375,135 @@ func (r *BMCReconciler) handleAnnotationOperations(ctx context.Context, bmcObj *
log.V(1).Info("Unknown operation annotation, ignoring", "Operation", operation, "Supported Operations", schemas.GracefulRestartResetType)
return false, nil
}
- 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 {
- return false, fmt.Errorf("failed to reset BMC: %w", err)
- }
- log.Info("Handled operation", "Operation", operation)
- default:
+ if value != schemas.GracefulRestartResetType {
log.V(1).Info("Unsupported operation annotation", "Operation", operation, "RedfishResetType", value)
return false, nil
}
+
+ // Consume the annotation before issuing the side-effecting reset, so a
+ // crash or failed Redfish call cannot leave it behind as stale state. Once
+ // resetBMC sets ConditionReset, the condition is the sole source of truth
+ // for the in-flight reset.
bmcBase := bmcObj.DeepCopy()
metautils.DeleteAnnotation(bmcObj, metalv1alpha1.OperationAnnotation)
if err := r.Patch(ctx, bmcObj, client.MergeFrom(bmcBase)); err != nil {
return false, fmt.Errorf("failed to remove operation annotation: %w", err)
}
log.V(1).Info("Removed operation annotation", "Operation", operation)
+
+ log.V(1).Info("Handling operation", "Operation", operation, "RedfishResetType", value)
+ if err := r.resetBMC(ctx, bmcObj, bmcClient, ReasonUserReset, bmcUserResetMessage); err != nil {
+ return false, fmt.Errorf("failed to reset BMC: %w", err)
+ }
+ log.Info("Handled operation", "Operation", operation)
return true, nil
}
func (r *BMCReconciler) updateReadyConditionOnBMCFailure(ctx context.Context, bmcObj *metalv1alpha1.BMC, err error) error {
- httpErr := &schemas.Error{}
- if errors.As(err, &httpErr) {
- // only handle 5xx errors
+ reason, message := ReasonUnknownError, fmt.Sprintf("BMC connection error: %v", err)
+ if httpErr, ok := errors.AsType[*schemas.Error](err); ok {
switch httpErr.HTTPReturnedStatusCode {
case 401:
- // Unauthorized error, likely due to bad credentials
- if err := r.updateConditions(ctx, bmcObj, true, ConditionReady, corev1.ConditionFalse, ReasonAuthenticationFailed, "BMC credentials are invalid"); err != nil {
- return fmt.Errorf("failed to set BMC unauthorized condition: %w", err)
- }
-
+ reason, message = ReasonAuthenticationFailed, "BMC credentials are invalid"
case 500:
- // Internal Server Error, might be transient
- if err := r.updateConditions(ctx, bmcObj, true, ConditionReady, corev1.ConditionFalse, ReasonInternalError, "BMC internal server error"); err != nil {
- return fmt.Errorf("failed to set BMC internal server error condition: %w", err)
- }
+ reason, message = ReasonInternalError, "BMC internal server error"
case 503:
- // Service Unavailable, might be transient
- if err := r.updateConditions(ctx, bmcObj, true, ConditionReady, corev1.ConditionFalse, ReasonConnectionFailed, "BMC service unavailable"); err != nil {
- return fmt.Errorf("failed to set BMC service unavailable condition: %w", err)
- }
- default:
- if err := r.updateConditions(ctx, bmcObj, true, ConditionReady, corev1.ConditionFalse, ReasonUnknownError, fmt.Sprintf("BMC connection error: %v", err)); err != nil {
- return fmt.Errorf("failed to set BMC error condition: %w", err)
- }
- }
- } else {
- if err := r.updateConditions(ctx, bmcObj, true, ConditionReady, corev1.ConditionFalse, ReasonUnknownError, fmt.Sprintf("BMC connection error: %v", err)); err != nil {
- return fmt.Errorf("failed to set BMC error condition: %w", err)
+ reason, message = ReasonConnectionFailed, "BMC service unavailable"
}
}
+ if err := r.patchCondition(ctx, bmcObj, ConditionReady, corev1.ConditionFalse, reason, message); err != nil {
+ return fmt.Errorf("failed to set BMC ready condition: %w", err)
+ }
return err
}
-func (r *BMCReconciler) waitForBMCReset(bmcObj *metalv1alpha1.BMC, delay time.Duration) bool {
- condition := &metav1.Condition{}
- found, err := r.Conditions.FindSlice(bmcObj.Status.Conditions, ConditionReset, condition)
- if err != nil || !found {
- return false
- }
- if condition.Status == metav1.ConditionTrue {
- // give bmc some time to start the reset process
- if time.Since(condition.LastTransitionTime.Time) < delay {
- return true
- }
- }
- return false
-}
+// resetWaitState reports the state of an in-flight BMC reset.
+type resetWaitState int
-func (r *BMCReconciler) handlePreviousBMCResetAnnotations(ctx context.Context, bmcObj *metalv1alpha1.BMC) (bool, error) {
- log := ctrl.LoggerFrom(ctx)
+const (
+ resetWaitNone resetWaitState = iota // no ConditionReset, or it is not True
+ resetWaitPending // ConditionReset is True and within the wait window
+ resetWaitExpired // ConditionReset is True but the wait window has elapsed
+)
+
+func (r *BMCReconciler) resetWaitState(bmcObj *metalv1alpha1.BMC) resetWaitState {
condition := &metav1.Condition{}
found, err := r.Conditions.FindSlice(bmcObj.Status.Conditions, ConditionReset, condition)
- if err != nil || !found {
- return false, nil
+ if err != nil || !found || condition.Status != metav1.ConditionTrue {
+ return resetWaitNone
}
- if condition.Status == metav1.ConditionTrue {
- if operation, ok := bmcObj.GetAnnotations()[metalv1alpha1.OperationAnnotation]; ok && operation == metalv1alpha1.GracefulRestartBMC {
- bmcBase := bmcObj.DeepCopy()
- metautils.DeleteAnnotation(bmcObj, metalv1alpha1.OperationAnnotation)
- if err := r.Patch(ctx, bmcObj, client.MergeFrom(bmcBase)); err != nil {
- return false, fmt.Errorf("failed to remove operation annotation from previous reset: %w", err)
- }
- log.V(1).Info("Removed operation annotation from previous reset", "Operation", operation)
- return true, nil
- }
+ // give bmc some time to start the reset process
+ if time.Since(condition.LastTransitionTime.Time) < r.ResetWaitTime {
+ return resetWaitPending
}
- return false, nil
+ return resetWaitExpired
}
func (r *BMCReconciler) shouldResetBMC(bmcObj *metalv1alpha1.BMC) bool {
- if r.BMCFailureResetDelay == 0 {
+ if r.FailureResetDelay == 0 {
return false
}
- bmcResetCondition := &metav1.Condition{}
- found, err := r.Conditions.FindSlice(bmcObj.Status.Conditions, ConditionReset, bmcResetCondition)
- if err != nil || (found && bmcResetCondition.Status == metav1.ConditionTrue) {
+ if resetStatus, _ := r.Conditions.FindSliceStatus(bmcObj.Status.Conditions, ConditionReset); resetStatus == corev1.ConditionTrue {
return false
}
readyCondition := &metav1.Condition{}
- found, err = r.Conditions.FindSlice(bmcObj.Status.Conditions, ConditionReady, readyCondition)
+ found, err := r.Conditions.FindSlice(bmcObj.Status.Conditions, ConditionReady, readyCondition)
if err != nil || !found {
return false
}
- if readyCondition.Status == metav1.ConditionFalse && (readyCondition.Reason == ReasonInternalError || readyCondition.Reason == ReasonConnectionFailed) {
- if time.Since(readyCondition.LastTransitionTime.Time) > r.BMCFailureResetDelay {
- return true
- }
+ if readyCondition.Status == metav1.ConditionFalse &&
+ (readyCondition.Reason == ReasonInternalError || readyCondition.Reason == ReasonConnectionFailed) &&
+ time.Since(readyCondition.LastTransitionTime.Time) > r.FailureResetDelay {
+ return true
}
return false
}
-func (r *BMCReconciler) patchBMCStatePending(ctx context.Context, bmcObj *metalv1alpha1.BMC) error {
- if bmcObj.Status.State == metalv1alpha1.BMCStatePending {
- return nil
- }
- bmcBase := bmcObj.DeepCopy()
- bmcObj.Status.State = metalv1alpha1.BMCStatePending
- if err := r.Status().Patch(ctx, bmcObj, client.MergeFrom(bmcBase)); err != nil {
- return fmt.Errorf("failed to patch BMC state to Pending: %w", err)
- }
- return nil
-}
-
func (r *BMCReconciler) resetBMC(ctx context.Context, bmcObj *metalv1alpha1.BMC, bmcClient bmc.BMC, reason, message string) error {
log := ctrl.LoggerFrom(ctx)
- if err := r.updateConditions(ctx, bmcObj, true, ConditionReset, corev1.ConditionTrue, reason, message); err != nil {
+ if err := r.patchCondition(ctx, bmcObj, 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.
- return errors.Join(
- r.patchBMCStatePending(ctx, bmcObj),
- fmt.Errorf("could not reset BMC %s: no client connection", bmcObj.Name),
- )
- }
- if err := bmcClient.ResetManager(ctx, bmcObj.Spec.BMCUUID, schemas.GracefulRestartResetType); err == nil {
+ return r.failReset(ctx, bmcObj, fmt.Errorf("could not reset BMC %s: no client connection", bmcObj.Name))
+ }
+ err := bmcClient.ResetManager(ctx, bmcObj.Spec.BMCUUID, schemas.GracefulRestartResetType)
+ if 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))
- }
- } else {
- return fmt.Errorf("could not reset BMC, unknown error: %w", err)
+ return nil
+ }
+ 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 {
+ log.V(1).Info("BMC reset returned a retryable 5xx, leaving reset condition set", "BMC", bmcObj.Name)
+ return nil
}
+ return r.failReset(ctx, bmcObj, fmt.Errorf("could not reset BMC: %w", err))
}
- log.V(1).Info("BMC reset returned a retryable 5xx, leaving reset condition set", "BMC", bmcObj.Name)
- return nil
+ return r.failReset(ctx, bmcObj, fmt.Errorf("could not reset BMC, unknown error: %w", err))
}
-func (r *BMCReconciler) updateConditions(ctx context.Context, bmcObj *metalv1alpha1.BMC, createIfNotFound bool, conditionType string, status corev1.ConditionStatus, reason, message string) error {
- condition := &metav1.Condition{}
- ok, err := r.Conditions.FindSlice(bmcObj.Status.Conditions, conditionType, condition)
+// failReset clears the in-flight reset condition and joins the clearing error
+// with the reset failure.
+func (r *BMCReconciler) failReset(ctx context.Context, bmcObj *metalv1alpha1.BMC, err error) error {
+ return errors.Join(
+ r.clearResetCondition(ctx, bmcObj, ReasonResetFailed, "BMC reset failed"),
+ err,
+ )
+}
+
+func (r *BMCReconciler) clearResetCondition(ctx context.Context, bmcObj *metalv1alpha1.BMC, reason, message string) error {
+ found, err := r.Conditions.FindSlice(bmcObj.Status.Conditions, ConditionReset, &metav1.Condition{})
if err != nil {
- return fmt.Errorf("failed to find condition %s: %w", conditionType, err)
+ return fmt.Errorf("failed to find condition %s: %w", ConditionReset, err)
}
- if !ok && !createIfNotFound {
- // condition not found and not allowed to create
+ if !found {
return nil
}
+ return r.patchCondition(ctx, bmcObj, ConditionReset, corev1.ConditionFalse, reason, message)
+}
+
+func (r *BMCReconciler) patchCondition(ctx context.Context, bmcObj *metalv1alpha1.BMC, conditionType string, status corev1.ConditionStatus, reason, message string) error {
bmcBase := bmcObj.DeepCopy()
if err := r.Conditions.UpdateSlice(
&bmcObj.Status.Conditions,
diff --git a/internal/controller/bmc_controller_test.go b/internal/controller/bmc_controller_test.go
index c19dc1087..267cb0294 100644
--- a/internal/controller/bmc_controller_test.go
+++ b/internal/controller/bmc_controller_test.go
@@ -673,7 +673,7 @@ var _ = Describe("BMC Conditions", func() {
SatisfyAll(
HaveField("Type", ConditionReset),
HaveField("Status", metav1.ConditionFalse),
- HaveField("Reason", "ResetComplete"),
+ HaveField("Reason", ReasonResetComplete),
),
)),
)
diff --git a/internal/controller/conditions.go b/internal/controller/conditions.go
index fbb34302d..82a083bb6 100644
--- a/internal/controller/conditions.go
+++ b/internal/controller/conditions.go
@@ -54,9 +54,16 @@ const (
ReasonVersionUpgradePending = "VersionUpgradePending"
// ReasonResetIssued indicates a reset has been issued.
ReasonResetIssued = "ResetIssued"
- // ReasonResetRequired indicates a BMC reset is needed but not yet issued.
+ // ReasonResetComplete indicates a reset has completed.
+ ReasonResetComplete = "ResetComplete"
+ // ReasonResetFailed indicates a reset attempt failed.
+ ReasonResetFailed = "ResetFailed"
+ // ReasonResetTimeout indicates a reset was issued but the BMC did not
+ // reconnect within the reset wait window.
+ ReasonResetTimeout = "ResetTimeout"
+ // ReasonResetRequired indicates a reset is needed but not yet issued.
ReasonResetRequired = "ResetRequired"
- // ReasonNoResetRequired indicates no BMC reset is needed for this apply cycle.
+ // ReasonNoResetRequired indicates no reset is needed for this apply cycle.
ReasonNoResetRequired = "NoResetRequired"
// ReasonAuthenticationFailed indicates authentication has failed.
ReasonAuthenticationFailed = "AuthenticationFailed"
diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go
index 4c6837750..16957d392 100644
--- a/internal/controller/suite_test.go
+++ b/internal/controller/suite_test.go
@@ -178,17 +178,17 @@ func SetupTest(redfishMockServers []netip.AddrPort) *corev1.Namespace {
Expect(err).NotTo(HaveOccurred())
Expect((&BMCReconciler{
- Client: k8sManager.GetClient(),
- Scheme: k8sManager.GetScheme(),
- DefaultProtocol: metalv1alpha1.HTTPProtocolScheme,
- SkipCertValidation: true,
- ManagerNamespace: ns.Name,
- BMCResetWaitTime: 400 * time.Millisecond,
- BMCClientRetryInterval: 25 * time.Millisecond,
- EventURL: "http://localhost:8008",
- DNSRecordTemplate: dnsTemplate,
- Conditions: accessor,
- BMCOptions: bmc.Options{
+ Client: k8sManager.GetClient(),
+ Scheme: k8sManager.GetScheme(),
+ DefaultProtocol: metalv1alpha1.HTTPProtocolScheme,
+ SkipCertValidation: true,
+ ManagerNamespace: ns.Name,
+ ResetWaitTime: 400 * time.Millisecond,
+ ReconnectInterval: 25 * time.Millisecond,
+ EventURL: "http://localhost:8008",
+ DNSRecordTemplate: dnsTemplate,
+ Conditions: accessor,
+ Options: bmc.Options{
PowerPollingInterval: 50 * time.Millisecond,
PowerPollingTimeout: 200 * time.Millisecond,
BasicAuth: true,