From cad47e7f9b6502070c33d09b72bcc6a4e1483c0e Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Mon, 13 Jul 2026 14:59:49 +0200 Subject: [PATCH] Migrate AggregatesController to SSA status updates Replace PatchHypervisorStatusWithRetry with Status().Apply() for ConditionTypeAggregatesUpdated and the Aggregates slice. The error path includes the current Aggregates in the apply to prevent SSA from pruning previously-set values on a transient OpenStack error. Clearing aggregates to an empty slice uses a targeted merge patch to work around the omitempty limitation in the generated apply configuration type. Seed only ConditionTypeAggregatesUpdated so SSA does not claim ownership over conditions written by other controllers. --- internal/controller/aggregates_controller.go | 90 ++++++++++++++++---- 1 file changed, 74 insertions(+), 16 deletions(-) diff --git a/internal/controller/aggregates_controller.go b/internal/controller/aggregates_controller.go index 30aa47e1..1f4b9997 100644 --- a/internal/controller/aggregates_controller.go +++ b/internal/controller/aggregates_controller.go @@ -19,6 +19,7 @@ package controller import ( "context" + "encoding/json" "errors" "fmt" @@ -26,6 +27,8 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -33,6 +36,7 @@ import ( "github.com/gophercloud/gophercloud/v2" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + apiv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/openstack" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -76,17 +80,32 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) // Apply aggregates to OpenStack and update status aggregates, err := openstack.ApplyAggregates(ctx, ac.computeClient, hv.Name, desiredAggregateNames) if err != nil { - // Set error condition with retry on conflict - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeAggregatesUpdated, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonFailed, - Message: fmt.Errorf("failed to apply aggregates: %w", err).Error(), + // Set error condition, preserving current aggregate ownership + statusCfg := apiv1.HypervisorStatus() + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeAggregatesUpdated). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonFailed). + WithMessage(fmt.Errorf("failed to apply aggregates: %w", err).Error())) + if len(hv.Status.Aggregates) > 0 { + aggCfgs := make([]apiv1.AggregateApplyConfiguration, len(hv.Status.Aggregates)) + for i, agg := range hv.Status.Aggregates { + a := apiv1.Aggregate().WithName(agg.Name).WithUUID(agg.UUID) + if len(agg.Metadata) > 0 { + a.WithMetadata(agg.Metadata) + } + aggCfgs[i] = *a + } + statusCfg.Aggregates = aggCfgs } - if err2 := utils.PatchHypervisorStatusWithRetry(ctx, ac.Client, hv.Name, AggregatesControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }); err2 != nil { + if err2 := ac.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(AggregatesControllerName)); err2 != nil { return ctrl.Result{}, errors.Join(err, err2) } return ctrl.Result{}, err @@ -106,15 +125,54 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, nil } - // Patch status with retry on conflict - err := utils.PatchHypervisorStatusWithRetry(ctx, ac.Client, hv.Name, AggregatesControllerName, func(h *kvmv1.Hypervisor) { - if aggregatesChanged { - h.Status.Aggregates = newAggregates + statusCfg := apiv1.HypervisorStatus() + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(desiredCondition.Type). + WithStatus(desiredCondition.Status). + WithReason(desiredCondition.Reason). + WithMessage(desiredCondition.Message)) + + if aggregatesChanged && len(newAggregates) > 0 { + aggCfgs := make([]apiv1.AggregateApplyConfiguration, len(newAggregates)) + for i, agg := range newAggregates { + a := apiv1.Aggregate().WithName(agg.Name).WithUUID(agg.UUID) + if len(agg.Metadata) > 0 { + a.WithMetadata(agg.Metadata) + } + aggCfgs[i] = *a + } + statusCfg.Aggregates = aggCfgs + } + + if err := ac.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(AggregatesControllerName)); err != nil { + return ctrl.Result{}, err + } + + // The Aggregates field uses omitempty in the generated apply config, so an + // empty slice cannot be sent via SSA. Use a targeted merge patch to clear it. + if aggregatesChanged && len(newAggregates) == 0 { + patch, err := json.Marshal(map[string]any{ + "status": map[string]any{ + "aggregates": []kvmv1.Aggregate{}, + }, + }) + if err != nil { + return ctrl.Result{}, err + } + fresh := &kvmv1.Hypervisor{} + if err := ac.Get(ctx, k8sclient.ObjectKey{Name: hv.Name}, fresh); err != nil { + return ctrl.Result{}, err } - meta.SetStatusCondition(&h.Status.Conditions, desiredCondition) - }) + return ctrl.Result{}, ac.Status().Patch(ctx, fresh, k8sclient.RawPatch(types.MergePatchType, patch)) + } - return ctrl.Result{}, err + return ctrl.Result{}, nil } // determineDesiredState returns the desired aggregates and the corresponding condition