From 97f67dd6bfafb94918ca600ae3d235e5affc7408 Mon Sep 17 00:00:00 2001 From: Hardik Dodiya Date: Fri, 20 Mar 2026 14:15:05 +0100 Subject: [PATCH] Support dual-mode to run both httpboot and ipxeboot without race-condition --- cmd/main.go | 14 +++ .../controller/serverbootconfig_helpers.go | 23 ++++ ...serverbootconfiguration_http_controller.go | 44 +++++--- .../serverbootconfiguration_pxe_controller.go | 53 ++++++--- ...rbootconfiguration_readiness_controller.go | 103 ++++++++++++++++++ internal/controller/suite_test.go | 7 ++ 6 files changed, 212 insertions(+), 32 deletions(-) create mode 100644 internal/controller/serverbootconfiguration_readiness_controller.go diff --git a/cmd/main.go b/cmd/main.go index ab05ab0..eb10827 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -51,6 +51,7 @@ const ( serverBootConfigControllerPxe = "serverbootconfigpxe" httpBootConfigController = "httpbootconfig" serverBootConfigControllerHttp = "serverbootconfighttp" + serverBootConfigReadiness = "serverbootconfigreadiness" ) func init() { @@ -108,6 +109,7 @@ func main() { serverBootConfigControllerPxe, serverBootConfigControllerHttp, httpBootConfigController, + serverBootConfigReadiness, ) flag.Var(controllers, "controllers", @@ -284,6 +286,18 @@ func main() { } } + if controllers.Enabled(serverBootConfigReadiness) { + if err = (&controller.ServerBootConfigurationReadinessReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + RequireHTTPBoot: controllers.Enabled(serverBootConfigControllerHttp), + RequireIPXEBoot: controllers.Enabled(serverBootConfigControllerPxe), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "ServerBootConfigReadiness") + os.Exit(1) + } + } + //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { diff --git a/internal/controller/serverbootconfig_helpers.go b/internal/controller/serverbootconfig_helpers.go index af57c60..fc9a2de 100644 --- a/internal/controller/serverbootconfig_helpers.go +++ b/internal/controller/serverbootconfig_helpers.go @@ -245,3 +245,26 @@ func PatchServerBootConfigWithError( return c.Status().Patch(ctx, &cur, client.MergeFrom(base)) } + +// PatchServerBootConfigCondition patches a single condition on the ServerBootConfiguration status. +// Callers should only set condition types they own. +func PatchServerBootConfigCondition( + ctx context.Context, + c client.Client, + namespacedName types.NamespacedName, + condition metav1.Condition, +) error { + var cur metalv1alpha1.ServerBootConfiguration + if fetchErr := c.Get(ctx, namespacedName, &cur); fetchErr != nil { + return fmt.Errorf("failed to fetch ServerBootConfiguration: %w", fetchErr) + } + base := cur.DeepCopy() + + // Default to current generation if caller didn't set it. + if condition.ObservedGeneration == 0 { + condition.ObservedGeneration = cur.Generation + } + apimeta.SetStatusCondition(&cur.Status.Conditions, condition) + + return c.Status().Patch(ctx, &cur, client.MergeFrom(base)) +} diff --git a/internal/controller/serverbootconfiguration_http_controller.go b/internal/controller/serverbootconfiguration_http_controller.go index 2501dba..9b8cbc7 100644 --- a/internal/controller/serverbootconfiguration_http_controller.go +++ b/internal/controller/serverbootconfiguration_http_controller.go @@ -89,9 +89,16 @@ func (r *ServerBootConfigurationHTTPReconciler) reconcile(ctx context.Context, l ukiURL, err := r.constructUKIURL(ctx, config.Spec.Image) if err != nil { log.Error(err, "Failed to construct UKI URL") - if patchErr := PatchServerBootConfigWithError(ctx, r.Client, - types.NamespacedName{Name: config.Name, Namespace: config.Namespace}, err); patchErr != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch state to error: %w (original error: %w)", patchErr, err) + if patchErr := PatchServerBootConfigCondition(ctx, r.Client, + types.NamespacedName{Name: config.Name, Namespace: config.Namespace}, + metav1.Condition{ + Type: HTTPBootReadyConditionType, + Status: metav1.ConditionFalse, + Reason: "UKIURLConstructionFailed", + Message: err.Error(), + ObservedGeneration: config.Generation, + }); patchErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch %s condition: %w (original error: %w)", HTTPBootReadyConditionType, patchErr, err) } return ctrl.Result{}, err } @@ -134,16 +141,16 @@ func (r *ServerBootConfigurationHTTPReconciler) reconcile(ctx context.Context, l return ctrl.Result{}, fmt.Errorf("failed to get HTTPBoot config: %w", err) } - if err := r.patchConfigStateFromHTTPState(ctx, httpBootConfig, config); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch server boot config state to %s: %w", httpBootConfig.Status.State, err) + if err := r.patchHTTPBootReadyConditionFromHTTPState(ctx, httpBootConfig, config); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch %s condition from HTTPBootConfig state %s: %w", HTTPBootReadyConditionType, httpBootConfig.Status.State, err) } - log.V(1).Info("Patched server boot config state") + log.V(1).Info("Patched server boot config condition", "condition", HTTPBootReadyConditionType) log.V(1).Info("Reconciled ServerBootConfiguration") return ctrl.Result{}, nil } -func (r *ServerBootConfigurationHTTPReconciler) patchConfigStateFromHTTPState(ctx context.Context, httpBootConfig *bootv1alpha1.HTTPBootConfig, cfg *metalv1alpha1.ServerBootConfiguration) error { +func (r *ServerBootConfigurationHTTPReconciler) patchHTTPBootReadyConditionFromHTTPState(ctx context.Context, httpBootConfig *bootv1alpha1.HTTPBootConfig, cfg *metalv1alpha1.ServerBootConfiguration) error { key := types.NamespacedName{Name: cfg.Name, Namespace: cfg.Namespace} var cur metalv1alpha1.ServerBootConfiguration if err := r.Get(ctx, key, &cur); err != nil { @@ -151,19 +158,26 @@ func (r *ServerBootConfigurationHTTPReconciler) patchConfigStateFromHTTPState(ct } base := cur.DeepCopy() + cond := metav1.Condition{ + Type: HTTPBootReadyConditionType, + ObservedGeneration: cur.Generation, + } switch httpBootConfig.Status.State { case bootv1alpha1.HTTPBootConfigStateReady: - cur.Status.State = metalv1alpha1.ServerBootConfigurationStateReady - // Remove ImageValidation condition when transitioning to Ready - apimeta.RemoveStatusCondition(&cur.Status.Conditions, "ImageValidation") + cond.Status = metav1.ConditionTrue + cond.Reason = "BootConfigReady" + cond.Message = "HTTP boot configuration is ready." case bootv1alpha1.HTTPBootConfigStateError: - cur.Status.State = metalv1alpha1.ServerBootConfigurationStateError - } - - for _, c := range httpBootConfig.Status.Conditions { - apimeta.SetStatusCondition(&cur.Status.Conditions, c) + cond.Status = metav1.ConditionFalse + cond.Reason = "BootConfigError" + cond.Message = "HTTPBootConfig reported an error." + default: + cond.Status = metav1.ConditionUnknown + cond.Reason = "BootConfigPending" + cond.Message = "Waiting for HTTPBootConfig to become Ready." } + apimeta.SetStatusCondition(&cur.Status.Conditions, cond) return r.Status().Patch(ctx, &cur, client.MergeFrom(base)) } diff --git a/internal/controller/serverbootconfiguration_pxe_controller.go b/internal/controller/serverbootconfiguration_pxe_controller.go index 30ef100..94013b3 100644 --- a/internal/controller/serverbootconfiguration_pxe_controller.go +++ b/internal/controller/serverbootconfiguration_pxe_controller.go @@ -106,9 +106,16 @@ func (r *ServerBootConfigurationPXEReconciler) reconcile(ctx context.Context, lo kernelURL, initrdURL, squashFSURL, err := r.getImageDetailsFromConfig(ctx, bootConfig) if err != nil { - if patchErr := PatchServerBootConfigWithError(ctx, r.Client, - types.NamespacedName{Name: bootConfig.Name, Namespace: bootConfig.Namespace}, err); patchErr != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch server boot config state: %w (original error: %w)", patchErr, err) + if patchErr := PatchServerBootConfigCondition(ctx, r.Client, + types.NamespacedName{Name: bootConfig.Name, Namespace: bootConfig.Namespace}, + metav1.Condition{ + Type: IPXEBootReadyConditionType, + Status: metav1.ConditionFalse, + Reason: "ImageDetailsFailed", + Message: err.Error(), + ObservedGeneration: bootConfig.Generation, + }); patchErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch %s condition: %w (original error: %w)", IPXEBootReadyConditionType, patchErr, err) } return ctrl.Result{}, err } @@ -153,32 +160,44 @@ func (r *ServerBootConfigurationPXEReconciler) reconcile(ctx context.Context, lo return ctrl.Result{}, fmt.Errorf("failed to get IPXE config: %w", err) } - if err := r.patchConfigStateFromIPXEState(ctx, config, bootConfig); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch server boot config state to %s: %w", config.Status.State, err) + if err := r.patchIPXEBootReadyConditionFromIPXEState(ctx, config, bootConfig); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch %s condition from IPXEBootConfig state %s: %w", IPXEBootReadyConditionType, config.Status.State, err) } - log.V(1).Info("Patched server boot config state") + log.V(1).Info("Patched server boot config condition", "condition", IPXEBootReadyConditionType) log.V(1).Info("Reconciled ServerBootConfiguration") return ctrl.Result{}, nil } -func (r *ServerBootConfigurationPXEReconciler) patchConfigStateFromIPXEState(ctx context.Context, config *v1alpha1.IPXEBootConfig, bootConfig *metalv1alpha1.ServerBootConfiguration) error { - bootConfigBase := bootConfig.DeepCopy() +func (r *ServerBootConfigurationPXEReconciler) patchIPXEBootReadyConditionFromIPXEState(ctx context.Context, config *v1alpha1.IPXEBootConfig, bootConfig *metalv1alpha1.ServerBootConfiguration) error { + key := types.NamespacedName{Name: bootConfig.Name, Namespace: bootConfig.Namespace} + var cur metalv1alpha1.ServerBootConfiguration + if err := r.Get(ctx, key, &cur); err != nil { + return err + } + base := cur.DeepCopy() + cond := metav1.Condition{ + Type: IPXEBootReadyConditionType, + ObservedGeneration: cur.Generation, + } switch config.Status.State { case v1alpha1.IPXEBootConfigStateReady: - bootConfig.Status.State = metalv1alpha1.ServerBootConfigurationStateReady - // Remove ImageValidation condition when transitioning to Ready - apimeta.RemoveStatusCondition(&bootConfig.Status.Conditions, "ImageValidation") + cond.Status = metav1.ConditionTrue + cond.Reason = "BootConfigReady" + cond.Message = "IPXE boot configuration is ready." case v1alpha1.IPXEBootConfigStateError: - bootConfig.Status.State = metalv1alpha1.ServerBootConfigurationStateError - } - - for _, c := range config.Status.Conditions { - apimeta.SetStatusCondition(&bootConfig.Status.Conditions, c) + cond.Status = metav1.ConditionFalse + cond.Reason = "BootConfigError" + cond.Message = "IPXEBootConfig reported an error." + default: + cond.Status = metav1.ConditionUnknown + cond.Reason = "BootConfigPending" + cond.Message = "Waiting for IPXEBootConfig to become Ready." } - return r.Status().Patch(ctx, bootConfig, client.MergeFrom(bootConfigBase)) + apimeta.SetStatusCondition(&cur.Status.Conditions, cond) + return r.Status().Patch(ctx, &cur, client.MergeFrom(base)) } func (r *ServerBootConfigurationPXEReconciler) getSystemUUIDFromBootConfig(ctx context.Context, config *metalv1alpha1.ServerBootConfiguration) (string, error) { diff --git a/internal/controller/serverbootconfiguration_readiness_controller.go b/internal/controller/serverbootconfiguration_readiness_controller.go new file mode 100644 index 0000000..703c9ff --- /dev/null +++ b/internal/controller/serverbootconfiguration_readiness_controller.go @@ -0,0 +1,103 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" +) + +const ( + // Condition types written by the mode-specific converters. + HTTPBootReadyConditionType = "HTTPBootReady" + IPXEBootReadyConditionType = "IPXEBootReady" +) + +// ServerBootConfigurationReadinessReconciler aggregates mode-specific readiness conditions and is the +// single writer of ServerBootConfiguration.Status.State. +type ServerBootConfigurationReadinessReconciler struct { + client.Client + Scheme *runtime.Scheme + + // RequireHTTPBoot/RequireIPXEBoot are derived from boot-operator CLI controller enablement. + // There is currently no per-SBC spec hint for which boot modes should be considered active. + RequireHTTPBoot bool + RequireIPXEBoot bool +} + +//+kubebuilder:rbac:groups=metal.ironcore.dev,resources=serverbootconfigurations,verbs=get;list;watch +//+kubebuilder:rbac:groups=metal.ironcore.dev,resources=serverbootconfigurations/status,verbs=get;update;patch + +func (r *ServerBootConfigurationReadinessReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + cfg := &metalv1alpha1.ServerBootConfiguration{} + if err := r.Get(ctx, req.NamespacedName, cfg); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + // If no boot modes are required (because their converters are disabled), do not mutate status. + if !r.RequireHTTPBoot && !r.RequireIPXEBoot { + return ctrl.Result{}, nil + } + + desired := metalv1alpha1.ServerBootConfigurationStatePending + + allReady := true + hasError := false + + if r.RequireHTTPBoot { + c := apimeta.FindStatusCondition(cfg.Status.Conditions, HTTPBootReadyConditionType) + switch { + case c == nil: + allReady = false + case c.Status == metav1.ConditionFalse: + hasError = true + case c.Status != metav1.ConditionTrue: + allReady = false + } + } + + if r.RequireIPXEBoot { + c := apimeta.FindStatusCondition(cfg.Status.Conditions, IPXEBootReadyConditionType) + switch { + case c == nil: + allReady = false + case c.Status == metav1.ConditionFalse: + hasError = true + case c.Status != metav1.ConditionTrue: + allReady = false + } + } + + switch { + case hasError: + desired = metalv1alpha1.ServerBootConfigurationStateError + case allReady: + desired = metalv1alpha1.ServerBootConfigurationStateReady + } + + if cfg.Status.State == desired { + return ctrl.Result{}, nil + } + + base := cfg.DeepCopy() + cfg.Status.State = desired + if err := r.Status().Patch(ctx, cfg, client.MergeFrom(base)); err != nil { + return ctrl.Result{}, err + } + + return ctrl.Result{}, nil +} + +func (r *ServerBootConfigurationReadinessReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&metalv1alpha1.ServerBootConfiguration{}). + Complete(r) +} diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index e89ba00..b36dfc3 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -166,6 +166,13 @@ func SetupTest() *corev1.Namespace { RegistryValidator: registryValidator, }).SetupWithManager(k8sManager)).To(Succeed()) + Expect((&ServerBootConfigurationReadinessReconciler{ + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + RequireHTTPBoot: true, + RequireIPXEBoot: true, + }).SetupWithManager(k8sManager)).To(Succeed()) + go func() { defer GinkgoRecover() Expect(k8sManager.Start(mgrCtx)).To(Succeed(), "failed to start manager")