Persist boot overrides during maintenance (#939)

Signed-off-by: Andreas Fritzler <andreas.fritzler@sap.com>

Author: afritzler

bmc/bmc.go

@@ -46,8 +46,16 @@ type BMC interface {
 	// Reset performs a reset on the system.
 	Reset(ctx context.Context, systemURI string, resetType schemas.ResetType) error
 
-	// SetPXEBootOnce sets the boot device for the next system boot.
-	SetPXEBootOnce(ctx context.Context, systemURI string) error
+	// SetBootOverride configures the system to network-boot on its next
+	// power-on, bypassing the persistent boot order. If persistent is false
+	// the override applies to a single boot only; if true it applies to every
+	// subsequent boot until ClearBootOverride is called.
+	SetBootOverride(ctx context.Context, systemURI string, persistent bool) error
+
+	// ClearBootOverride removes any active boot override so the system uses
+	// its persistent boot order on the next power-on. No-op when no override
+	// is currently set.
+	ClearBootOverride(ctx context.Context, systemURI string) error
 
 	// GetSystemInfo retrieves information about the system.
 	GetSystemInfo(ctx context.Context, systemURI string) (SystemInfo, error)

bmc/redfish.go

@@ -65,14 +65,9 @@ type RedfishBaseBMC struct {
 	manufacturer string
 }
 
-var pxeBootWithSettingUEFIBootMode = schemas.Boot{
-	BootSourceOverrideEnabled: schemas.OnceBootSourceOverrideEnabled,
-	BootSourceOverrideMode:    schemas.UEFIBootSourceOverrideMode,
-	BootSourceOverrideTarget:  schemas.PxeBootSource,
-}
-var pxeBootWithoutSettingUEFIBootMode = schemas.Boot{
-	BootSourceOverrideEnabled: schemas.OnceBootSourceOverrideEnabled,
-	BootSourceOverrideTarget:  schemas.PxeBootSource,
+var clearedBootOverride = schemas.Boot{
+	BootSourceOverrideEnabled: schemas.DisabledBootSourceOverrideEnabled,
+	BootSourceOverrideTarget:  schemas.NoneBootSource,
 }
 
 type InvalidBIOSSettingsError struct {
@@ -231,26 +226,65 @@ func (r *RedfishBaseBMC) GetSystems(ctx context.Context) ([]Server, error) {
 	return servers, nil
 }
 
-// SetPXEBootOnce sets the boot device for the next system boot using Redfish.
-func (r *RedfishBaseBMC) SetPXEBootOnce(ctx context.Context, systemURI string) error {
+// SetBootOverride sets a Redfish boot source override targeting network boot.
+// With persistent=false it sets BootSourceOverrideEnabled=Once; with
+// persistent=true it sets Continuous, which is preserved across power cycles.
+// When the requested persistent override already matches the current state
+// the call is a no-op.
+func (r *RedfishBaseBMC) SetBootOverride(ctx context.Context, systemURI string, persistent bool) error {
 	system, err := r.getSystemFromUri(ctx, systemURI)
 	if err != nil {
 		return fmt.Errorf("failed to get systems: %w", err)
 	}
-	var setBoot schemas.Boot
+	wantEnabled := schemas.OnceBootSourceOverrideEnabled
+	if persistent {
+		wantEnabled = schemas.ContinuousBootSourceOverrideEnabled
+		if system.Boot.BootSourceOverrideEnabled == schemas.ContinuousBootSourceOverrideEnabled &&
+			system.Boot.BootSourceOverrideTarget == schemas.PxeBootSource &&
+			(system.Boot.BootSourceOverrideMode == "" ||
+				system.Boot.BootSourceOverrideMode == schemas.UEFIBootSourceOverrideMode) {
+			return nil
+		}
+	}
+
+	setBoot := schemas.Boot{
+		BootSourceOverrideEnabled: wantEnabled,
+		BootSourceOverrideTarget:  schemas.PxeBootSource,
+	}
 	// TODO: cover setting BootSourceOverrideMode with BIOS settings profile
-	// Only skip setting BootSourceOverrideMode for older BMCs that don't report it
+	// Only set BootSourceOverrideMode when the BMC reports it; older BMCs that
+	// don't expose it will reject the field.
 	if system.Boot.BootSourceOverrideMode != "" && system.Boot.BootSourceOverrideMode != schemas.UEFIBootSourceOverrideMode {
-		setBoot = pxeBootWithSettingUEFIBootMode
-	} else {
-		setBoot = pxeBootWithoutSettingUEFIBootMode
+		setBoot.BootSourceOverrideMode = schemas.UEFIBootSourceOverrideMode
 	}
 
 	// TODO: pass logging context from caller
 	log := ctrl.LoggerFrom(ctx)
-	log.V(2).Info("Setting PXE boot once", "SystemURI", systemURI, "Boot settings", setBoot)
+	log.V(2).Info("Setting boot override", "SystemURI", systemURI, "Boot settings", setBoot)
+	if err := system.SetBoot(&setBoot); err != nil {
+		return fmt.Errorf("failed to set boot override: %w", err)
+	}
+	return nil
+}
+
+// ClearBootOverride disables any active Redfish boot source override so the
+// system uses its persistent boot order on the next power-on. No SetBoot call
+// is issued when the override is already disabled.
+func (r *RedfishBaseBMC) ClearBootOverride(ctx context.Context, systemURI string) error {
+	system, err := r.getSystemFromUri(ctx, systemURI)
+	if err != nil {
+		return fmt.Errorf("failed to get systems: %w", err)
+	}
+	if system.Boot.BootSourceOverrideEnabled == "" ||
+		system.Boot.BootSourceOverrideEnabled == schemas.DisabledBootSourceOverrideEnabled {
+		return nil
+	}
+
+	setBoot := clearedBootOverride
+	log := ctrl.LoggerFrom(ctx)
+	log.V(2).Info("Clearing boot override", "SystemURI", systemURI, "Boot settings", setBoot)
 	if err := system.SetBoot(&setBoot); err != nil {
-		return fmt.Errorf("failed to set the boot order: %w", err)
+		return fmt.Errorf("failed to clear boot override: %w", err)
 	}
 	return nil
 }

bmc/redfish_local.go

@@ -27,12 +27,12 @@ const (
 type RedfishLocalBMC struct {
 	*RedfishBaseBMC
 	// registryURL is an optional base URL (e.g. http://host:port). When set,
-	// SetPXEBootOnce posts dummy registration data to simulate a probe boot.
+	// SetBootOverride posts dummy registration data to simulate a probe boot.
 	registryURL string
 }
 
 // NewRedfishLocalBMCClientWithRegistry creates a RedfishLocalBMC that, after
-// SetPXEBootOnce, simulates probe registration by posting dummy network data to
+// SetBootOverride, simulates probe registration by posting dummy network data to
 // registryBaseURL/register. Use this in place of RedfishWithRegistryPatch for dev/tilt
 // environments where a real probe will not run.
 func NewRedfishLocalBMCClientWithRegistry(ctx context.Context, options Options, registryBaseURL string) (BMC, error) {
@@ -260,14 +260,16 @@ func (r *RedfishLocalBMC) CheckBMCPendingComponentUpgrade(_ context.Context, _ C
 	return false, nil
 }
 
-// SetPXEBootOnce sets the boot device for the next system boot using Redfish.
-// When a registryURL is configured, it additionally simulates probe registration
-// by posting dummy network data to the registry in a background goroutine.
-func (r *RedfishLocalBMC) SetPXEBootOnce(ctx context.Context, systemURI string) error {
-	if err := r.RedfishBaseBMC.SetPXEBootOnce(ctx, systemURI); err != nil {
+// SetBootOverride sets the network-boot override and, for one-shot overrides
+// only, simulates probe registration by posting dummy network data to the
+// configured registryURL in a background goroutine. The persistent override
+// path skips the registration simulation since maintenance flows do not go
+// through discovery.
+func (r *RedfishLocalBMC) SetBootOverride(ctx context.Context, systemURI string, persistent bool) error {
+	if err := r.RedfishBaseBMC.SetBootOverride(ctx, systemURI, persistent); err != nil {
 		return err
 	}
-	if r.registryURL == "" {
+	if persistent || r.registryURL == "" {
 		return nil
 	}
 	go func() {

bmc/redfish_supermicro.go

@@ -16,24 +16,34 @@ type SupermicroRedfishBMC struct {
 	*RedfishBaseBMC
 }
 
-// SetPXEBootOnce sets the boot device for the next system boot.
-// Supermicro requires explicitly setting BootSourceOverrideMode to UEFI.
-func (r *SupermicroRedfishBMC) SetPXEBootOnce(ctx context.Context, systemURI string) error {
+// SetBootOverride sets a network-boot override on Supermicro hardware, which
+// requires explicitly setting BootSourceOverrideMode to UEFI for the override
+// to take effect.
+func (r *SupermicroRedfishBMC) SetBootOverride(ctx context.Context, systemURI string, persistent bool) error {
 	system, err := r.getSystemFromUri(ctx, systemURI)
 	if err != nil {
 		return fmt.Errorf("failed to get systems: %w", err)
 	}
+	wantEnabled := schemas.OnceBootSourceOverrideEnabled
+	if persistent {
+		wantEnabled = schemas.ContinuousBootSourceOverrideEnabled
+		if system.Boot.BootSourceOverrideEnabled == schemas.ContinuousBootSourceOverrideEnabled &&
+			system.Boot.BootSourceOverrideTarget == schemas.PxeBootSource &&
+			system.Boot.BootSourceOverrideMode == schemas.UEFIBootSourceOverrideMode {
+			return nil
+		}
+	}
 
 	setBoot := &schemas.Boot{
-		BootSourceOverrideEnabled: schemas.OnceBootSourceOverrideEnabled,
+		BootSourceOverrideEnabled: wantEnabled,
 		BootSourceOverrideMode:    schemas.UEFIBootSourceOverrideMode,
 		BootSourceOverrideTarget:  schemas.PxeBootSource,
 	}
 
 	log := ctrl.LoggerFrom(ctx)
-	log.V(2).Info("Setting PXE boot once (Supermicro)", "SystemURI", systemURI, "Boot settings", setBoot)
+	log.V(2).Info("Setting boot override (Supermicro)", "SystemURI", systemURI, "Boot settings", setBoot)
 	if err := system.SetBoot(setBoot); err != nil {
-		return fmt.Errorf("failed to set the boot order: %w", err)
+		return fmt.Errorf("failed to set boot override: %w", err)
 	}
 	return nil
 }

internal/bmcutils/bmcutils.go

@@ -45,7 +45,7 @@ type createBMCClientConfig struct {
 }
 
 // WithRegistryURL configures the BMC client to POST dummy registration data to
-// the given base URL after SetPXEBootOnce. Used with ProtocolRedfishWithRegistryPatch to
+// the given base URL after SetBootOverride. Used with ProtocolRedfishWithRegistryPatch to
 // simulate probe boot registration without a real K8s Job.
 func WithRegistryURL(url string) CreateBMCClientOption {
 	return func(c *createBMCClientConfig) {

internal/controller/server_controller.go

@@ -424,6 +424,12 @@ func (r *ServerReconciler) handleDiscoveryState(ctx context.Context, bmcClient b
 
 func (r *ServerReconciler) handleAvailableState(ctx context.Context, bmcClient bmc.BMC, server *metalv1alpha1.Server) (bool, error) {
 	log := ctrl.LoggerFrom(ctx)
+	// Self-heal: drop any boot override that may have been left set by a
+	// previous Maintenance cycle that ended unexpectedly. ClearBootOverride
+	// is a no-op when no override is active.
+	if err := bmcClient.ClearBootOverride(ctx, server.Spec.SystemURI); err != nil {
+		return false, fmt.Errorf("failed to clear boot override: %w", err)
+	}
 	serverBase := server.DeepCopy()
 	if server.Status.PowerState != metalv1alpha1.ServerOffPowerState {
 		server.Spec.Power = metalv1alpha1.PowerOff
@@ -469,6 +475,12 @@ func (r *ServerReconciler) handleAvailableState(ctx context.Context, bmcClient b
 
 func (r *ServerReconciler) handleReservedState(ctx context.Context, bmcClient bmc.BMC, server *metalv1alpha1.Server) (bool, error) {
 	log := ctrl.LoggerFrom(ctx)
+	// Self-heal: drop any boot override that may have been left set by a
+	// previous Maintenance cycle that ended unexpectedly. ClearBootOverride
+	// is a no-op when no override is active.
+	if err := bmcClient.ClearBootOverride(ctx, server.Spec.SystemURI); err != nil {
+		return false, fmt.Errorf("failed to clear boot override: %w", err)
+	}
 	serverClaimRef := server.Spec.ServerClaimRef
 	if serverClaimRef == nil {
 		if modified, err := r.patchServerState(ctx, server, metalv1alpha1.ServerStateAvailable); err != nil || modified {
@@ -576,11 +588,21 @@ func (r *ServerReconciler) handleMaintenanceState(ctx context.Context, bmcClient
 		if err := r.updateServerStatusFromSystemInfo(ctx, bmcClient, server); err != nil {
 			return false, fmt.Errorf("failed to update server status system info: %w", err)
 		}
+		if err := bmcClient.ClearBootOverride(ctx, server.Spec.SystemURI); err != nil {
+			return false, fmt.Errorf("failed to clear boot override on maintenance exit: %w", err)
+		}
 		if server.Spec.ServerClaimRef == nil {
 			return r.patchServerState(ctx, server, metalv1alpha1.ServerStateInitial)
 		}
 		return r.patchServerState(ctx, server, metalv1alpha1.ServerStateReserved)
 	}
+	// Re-assert the persistent network-boot override on every reconcile while in Maintenance.
+	// This protects against reboots not driven by metal-operator (e.g. a vendor BIOS
+	// upgrade task rebooting the system itself) falling through to disk and starting
+	// the production OS while the host is still being worked on.
+	if err := bmcClient.SetBootOverride(ctx, server.Spec.SystemURI, true); err != nil {
+		return false, fmt.Errorf("failed to set persistent network boot for maintenance: %w", err)
+	}
 	if err := r.ensureServerPowerState(ctx, bmcClient, server); err != nil {
 		return false, fmt.Errorf("failed to ensure server power state: %w", err)
 	}
@@ -914,7 +936,7 @@ func (r *ServerReconciler) pxeBootServer(ctx context.Context, bmcClient bmc.BMC,
 		return fmt.Errorf("can only PXE boot server with valid BMC ref or inline BMC configuration")
 	}
 
-	if err := bmcClient.SetPXEBootOnce(ctx, server.Spec.SystemURI); err != nil {
+	if err := bmcClient.SetBootOverride(ctx, server.Spec.SystemURI, false); err != nil {
 		return fmt.Errorf("failed to set PXE boot one for server: %w", err)
 	}
 	return nil