From 74efc346e15ba518585bee4605f93f43bc3e2e92 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 5 May 2026 13:12:43 +0200 Subject: [PATCH 01/20] Migrate EvictionReconciler to SSA status updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace MergeFromWithOptimisticLock + retry with Status().Apply() for all eviction status updates. Introduces ConditionsFromStatus and SetApplyConfigurationStatusCondition helpers to utils — mirroring meta.SetStatusCondition semantics — as the foundation for all SSA condition management. The evictionStatusCfg helper seeds every apply with the full set of owned fields so SSA never prunes values between reconcile cycles. --- internal/controller/eviction_controller.go | 317 +++++++++++---------- internal/utils/conditions.go | 116 ++++++++ 2 files changed, 284 insertions(+), 149 deletions(-) create mode 100644 internal/utils/conditions.go diff --git a/internal/controller/eviction_controller.go b/internal/controller/eviction_controller.go index b0fa33c..fc66f09 100644 --- a/internal/controller/eviction_controller.go +++ b/internal/controller/eviction_controller.go @@ -32,12 +32,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/util/retry" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" logger "sigs.k8s.io/controller-runtime/pkg/log" 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" ) @@ -85,6 +86,19 @@ func moveToBack(instances []string) []string { return instances } +// evictionStatusCfg builds an EvictionStatusApplyConfiguration pre-populated +// with the current scalar fields and conditions from the fetched eviction. +// Call sites upsert the single condition they are setting via +// utils.SetApplyConfigurationStatusCondition, then apply. +func evictionStatusCfg(eviction *kvmv1.Eviction) *apiv1.EvictionStatusApplyConfiguration { + cfg := apiv1.EvictionStatus(). + WithHypervisorServiceId(eviction.Status.HypervisorServiceId). + WithOutstandingRamMb(eviction.Status.OutstandingRamMb) + cfg.OutstandingInstances = eviction.Status.OutstandingInstances + cfg.Conditions = utils.ConditionsFromStatus(eviction.Status.Conditions) + return cfg +} + // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=evictions,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=evictions/status,verbs=get;update;patch // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=evictions/finalizers,verbs=update @@ -118,14 +132,16 @@ func (r *EvictionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c statusCondition := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeEvicting) if statusCondition == nil { - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: metav1.ConditionTrue, - Message: "Running", - Reason: kvmv1.ConditionReasonRunning, - }) - - return ctrl.Result{}, r.updateStatus(ctx, eviction) + statusCfg := evictionStatusCfg(eviction) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonRunning). + WithMessage("Running")) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } switch statusCondition.Status { @@ -157,36 +173,18 @@ func (r *EvictionReconciler) handleRunning(ctx context.Context, eviction *kvmv1. return r.evictNext(ctx, eviction) } - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: metav1.ConditionFalse, - Message: "eviction completed successfully", - Reason: kvmv1.ConditionReasonSucceeded, - }) - - eviction.Status.OutstandingRamMb = 0 logger.FromContext(ctx).Info("succeeded") - return ctrl.Result{}, r.updateStatus(ctx, eviction) -} - -func (r *EvictionReconciler) updateStatus(ctx context.Context, eviction *kvmv1.Eviction) error { - desiredStatus := eviction.Status.DeepCopy() - return retry.RetryOnConflict(utils.StatusPatchBackoff, func() error { - freshEviction := &kvmv1.Eviction{} - if err := r.Get(ctx, client.ObjectKeyFromObject(eviction), freshEviction); err != nil { - return err - } - freshBase := freshEviction.DeepCopy() - // Apply desired conditions and scalar fields onto the fresh status - for _, c := range desiredStatus.Conditions { - meta.SetStatusCondition(&freshEviction.Status.Conditions, c) - } - freshEviction.Status.OutstandingInstances = desiredStatus.OutstandingInstances - freshEviction.Status.OutstandingRamMb = desiredStatus.OutstandingRamMb - freshEviction.Status.HypervisorServiceId = desiredStatus.HypervisorServiceId - return r.Status().Patch(ctx, freshEviction, client.MergeFromWithOptions(freshBase, - client.MergeFromWithOptimisticLock{}), client.FieldOwner(EvictionControllerName)) - }) + statusCfg := evictionStatusCfg(eviction) + statusCfg.WithOutstandingRamMb(0) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage("eviction completed successfully")) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv1.Eviction, hv *kvmv1.Hypervisor) (ctrl.Result, error) { @@ -194,14 +192,22 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv // If the hypervisor should exist, then we need to ensure it is disabled before we start evicting if expectHypervisor && !meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) { - // Hypervisor is not disabled (yet?), reflect that as a failing preflight check - if meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypePreflight, - Status: metav1.ConditionFalse, - Message: "hypervisor not disabled", - Reason: kvmv1.ConditionReasonFailed, - }) { - return ctrl.Result{}, r.updateStatus(ctx, eviction) + // Hypervisor is not disabled (yet?), reflect that as a failing preflight check. + // Only apply if the condition is new or changed, to avoid unnecessary API calls on repeated requeues. + existing := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypePreflight) + if existing == nil || existing.Status != metav1.ConditionFalse { + statusCfg := evictionStatusCfg(eviction) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypePreflight). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonFailed). + WithMessage("hypervisor not disabled")) + if err := r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)); err != nil { + return ctrl.Result{}, err + } } return ctrl.Result{RequeueAfter: defaultPollTime}, nil // Wait for hypervisor to be disabled } @@ -216,47 +222,56 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv if expectHypervisor { // Abort eviction - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: metav1.ConditionFalse, - Message: fmt.Sprintf("failed to get hypervisor %v", err), - Reason: kvmv1.ConditionReasonFailed, - }) - return ctrl.Result{}, r.updateStatus(ctx, eviction) - } else { - // That is (likely) an eviction for a node that never registered - // so we are good to go - msg := "eviction completed successfully due to expected case of no hypervisor" - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: metav1.ConditionFalse, - Message: msg, - Reason: kvmv1.ConditionReasonSucceeded, - }) - eviction.Status.OutstandingRamMb = 0 - logger.FromContext(ctx).Info(msg) - return ctrl.Result{}, r.updateStatus(ctx, eviction) + statusCfg := evictionStatusCfg(eviction) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonFailed). + WithMessage(fmt.Sprintf("failed to get hypervisor %v", err))) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } + + // That is (likely) an eviction for a node that never registered + // so we are good to go + msg := "eviction completed successfully due to expected case of no hypervisor" + logger.FromContext(ctx).Info(msg) + statusCfg := evictionStatusCfg(eviction) + statusCfg.WithOutstandingRamMb(0) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage(msg)) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } + var instances []string if hypervisor.Servers != nil { - uuids := make([]string, len(*hypervisor.Servers)) + instances = make([]string, len(*hypervisor.Servers)) for i, server := range *hypervisor.Servers { - uuids[i] = server.UUID + instances[i] = server.UUID } - eviction.Status.OutstandingInstances = uuids } - // Update status - eviction.Status.HypervisorServiceId = hypervisor.ID - eviction.Status.OutstandingRamMb = int64(hypervisor.MemoryMBUsed) - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypePreflight, - Status: metav1.ConditionTrue, - Message: "Preflight checks passed, hypervisor is disabled and ready for eviction", - Reason: kvmv1.ConditionReasonSucceeded, - }) - return ctrl.Result{}, r.updateStatus(ctx, eviction) + statusCfg := evictionStatusCfg(eviction) + statusCfg.WithHypervisorServiceId(hypervisor.ID) + statusCfg.WithOutstandingRamMb(int64(hypervisor.MemoryMBUsed)) + statusCfg.OutstandingInstances = instances + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypePreflight). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage("Preflight checks passed, hypervisor is disabled and ready for eviction")) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } // Tries to handle the NotFound-error by updating the status @@ -265,18 +280,22 @@ func (r *EvictionReconciler) handleNotFound(ctx context.Context, eviction *kvmv1 return err } logger.FromContext(ctx).Info("Instance is gone") - var uuid string - eviction.Status.OutstandingInstances, uuid = popInstance(eviction.Status.OutstandingInstances) + remaining, uuid := popInstance(eviction.Status.OutstandingInstances) if uuid == "" { return nil } - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: fmt.Sprintf("Instance %s is gone", uuid), - Reason: kvmv1.ConditionReasonSucceeded, - }) - return r.updateStatus(ctx, eviction) + + statusCfg := evictionStatusCfg(eviction) + statusCfg.OutstandingInstances = remaining + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeMigration). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage(fmt.Sprintf("Instance %s is gone", uuid))) + return r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Eviction) (ctrl.Result, error) { @@ -309,41 +328,28 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic case "ERROR": // Needs manual intervention (or another operator fixes it) // put it at the end of the list (beginning of array) - eviction.Status.OutstandingInstances = moveToBack(eviction.Status.OutstandingInstances) + reordered := moveToBack(eviction.Status.OutstandingInstances) log.Info("error", "faultMessage", vm.Fault.Message) - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: fmt.Sprintf("Migration of instance %s failed: %s", vm.ID, vm.Fault.Message), - Reason: kvmv1.ConditionReasonFailed, - }) - - return ctrl.Result{}, errors.Join(fmt.Errorf("error migrating instance %v", uuid), - r.updateStatus(ctx, eviction)) + + statusCfg := evictionStatusCfg(eviction) + statusCfg.OutstandingInstances = reordered + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeMigration). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonFailed). + WithMessage(fmt.Sprintf("Migration of instance %s failed: %s", vm.ID, vm.Fault.Message))) + applyErr := r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) + return ctrl.Result{}, errors.Join(fmt.Errorf("error migrating instance %v", uuid), applyErr) } currentHypervisor, _, _ := strings.Cut(vm.HypervisorHostname, ".") if currentHypervisor != eviction.Spec.Hypervisor { log.Info("migrated") - // Don't overwrite a sticky Failed migration condition with Succeeded - // while there are still other outstanding VMs - an earlier ERROR VM - // has been moved to the back of the queue and the eviction is not - // actually clean. The condition is reset only when the whole - // eviction completes (OutstandingInstances becomes empty). - remaining, _ := popInstance(eviction.Status.OutstandingInstances) - prior := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeMigration) - stickyFailure := len(remaining) > 0 && prior != nil && - prior.Status == metav1.ConditionFalse && - prior.Reason == kvmv1.ConditionReasonFailed - if !stickyFailure { - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: fmt.Sprintf("Migration of instance %s finished", vm.ID), - Reason: kvmv1.ConditionReasonSucceeded, - }) - } + // So, it is already off this one, do we need to verify it? if vm.Status == "VERIFY_RESIZE" { @@ -358,27 +364,54 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic } // All done - eviction.Status.OutstandingInstances, _ = popInstance(eviction.Status.OutstandingInstances) - return ctrl.Result{}, r.updateStatus(ctx, eviction) + popped, _ := popInstance(eviction.Status.OutstandingInstances) + + // Don't overwrite a sticky Failed migration condition with Succeeded + // while there are still other outstanding VMs - an earlier ERROR VM + // has been moved to the back of the queue and the eviction is not + // actually clean. The condition is reset only when the whole + // eviction completes (OutstandingInstances becomes empty). + prior := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeMigration) + stickyFailure := len(popped) > 0 && prior != nil && + prior.Status == metav1.ConditionFalse && + prior.Reason == kvmv1.ConditionReasonFailed + + statusCfg := evictionStatusCfg(eviction) + statusCfg.OutstandingInstances = popped + if !stickyFailure { + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeMigration). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage(fmt.Sprintf("Migration of instance %s finished", vm.ID))) + } + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } if vm.TaskState == "deleting" { //nolint:gocritic // We just have to wait for it to be gone. Try the next one. - eviction.Status.OutstandingInstances = moveToBack(eviction.Status.OutstandingInstances) - - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: fmt.Sprintf("Live migration of terminating instance %s skipped", vm.ID), - Reason: kvmv1.ConditionReasonFailed, - }) - if err := r.updateStatus(ctx, eviction); err != nil { + reordered := moveToBack(eviction.Status.OutstandingInstances) + + statusCfg := evictionStatusCfg(eviction) + statusCfg.OutstandingInstances = reordered + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeMigration). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonFailed). + WithMessage(fmt.Sprintf("Live migration of terminating instance %s skipped", vm.ID))) + if err := r.Status().Apply(ctx, + apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)); err != nil { return ctrl.Result{}, fmt.Errorf("could not update status due to %w", err) } return ctrl.Result{RequeueAfter: defaultPollTime}, nil } else if vm.Status == "ACTIVE" || vm.PowerState == 1 { log.Info("trigger live-migration") - if err := r.liveMigrate(ctx, vm.ID, eviction); err != nil { + if err := r.liveMigrate(ctx, vm.ID, eviction.Spec.Hypervisor); err != nil { if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil { return ctrl.Result{}, err2 } @@ -386,7 +419,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic } } else { log.Info("trigger cold-migration") - if err := r.coldMigrate(ctx, vm.ID, eviction); err != nil { + if err := r.coldMigrate(ctx, vm.ID, eviction.Spec.Hypervisor); err != nil { if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil { return ctrl.Result{}, err2 } @@ -400,7 +433,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic return ctrl.Result{RequeueAfter: defaultPollTime}, nil } -func (r *EvictionReconciler) liveMigrate(ctx context.Context, uuid string, eviction *kvmv1.Eviction) error { +func (r *EvictionReconciler) liveMigrate(ctx context.Context, uuid, hypervisorName string) error { log := logger.FromContext(ctx) liveMigrateOpts := servers.LiveMigrateOpts{ @@ -409,36 +442,22 @@ func (r *EvictionReconciler) liveMigrate(ctx context.Context, uuid string, evict res := servers.LiveMigrate(ctx, r.computeClient, uuid, liveMigrateOpts) if res.Err != nil { - err := fmt.Errorf("failed to evict VM %s due to %w", uuid, res.Err) - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: err.Error(), - Reason: kvmv1.ConditionReasonFailed, - }) - return err + return fmt.Errorf("failed to evict VM %s due to %w", uuid, res.Err) } - log.Info("Live migrating server", "server", uuid, "source", eviction.Spec.Hypervisor, "X-Openstack-Request-Id", res.Header.Get("X-Openstack-Request-Id")) + log.Info("Live migrating server", "server", uuid, "source", hypervisorName, "X-Openstack-Request-Id", res.Header.Get("X-Openstack-Request-Id")) return nil } -func (r *EvictionReconciler) coldMigrate(ctx context.Context, uuid string, eviction *kvmv1.Eviction) error { +func (r *EvictionReconciler) coldMigrate(ctx context.Context, uuid, hypervisorName string) error { log := logger.FromContext(ctx) res := servers.Migrate(ctx, r.computeClient, uuid) if res.Err != nil { - err := fmt.Errorf("failed to evict stopped server %s due to %w", uuid, res.Err) - meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeMigration, - Status: metav1.ConditionFalse, - Message: err.Error(), - Reason: kvmv1.ConditionReasonFailed, - }) - return err + return fmt.Errorf("failed to evict stopped server %s due to %w", uuid, res.Err) } - log.Info("Cold-migrating server", "server", uuid, "source", eviction.Spec.Hypervisor, "X-Openstack-Request-Id", res.Header.Get("X-Openstack-Request-Id")) + log.Info("Cold-migrating server", "server", uuid, "source", hypervisorName, "X-Openstack-Request-Id", res.Header.Get("X-Openstack-Request-Id")) return nil } diff --git a/internal/utils/conditions.go b/internal/utils/conditions.go new file mode 100644 index 0000000..2eef3a2 --- /dev/null +++ b/internal/utils/conditions.go @@ -0,0 +1,116 @@ +/* +SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" +) + +// ConditionsFromStatus converts a []metav1.Condition (as stored in a status +// subresource) into a []k8sacmetav1.ConditionApplyConfiguration suitable for +// use with server-side apply. All fields including LastTransitionTime are +// preserved verbatim. +func ConditionsFromStatus(conditions []metav1.Condition) []k8sacmetav1.ConditionApplyConfiguration { + result := make([]k8sacmetav1.ConditionApplyConfiguration, len(conditions)) + for i := range conditions { + c := &conditions[i] + result[i] = k8sacmetav1.ConditionApplyConfiguration{ + Type: &c.Type, + Status: &c.Status, + Reason: &c.Reason, + Message: &c.Message, + LastTransitionTime: &c.LastTransitionTime, + } + } + return result +} + +// SetApplyConfigurationStatusCondition sets the corresponding condition in +// conditions to newCondition and returns true if the conditions are changed by +// this call. It mirrors the behaviour of k8s.io/apimachinery/pkg/api/meta.SetStatusCondition: +// +// 1. If a condition of the specified type does not exist, newCondition is +// appended. LastTransitionTime is set to now if not provided. +// 2. If a condition of the specified type already exists, all fields are +// updated. LastTransitionTime is updated to now (or the provided value) +// only when Status changes; otherwise the existing LastTransitionTime is +// preserved. +func SetApplyConfigurationStatusCondition(conditions *[]k8sacmetav1.ConditionApplyConfiguration, newCondition k8sacmetav1.ConditionApplyConfiguration) (changed bool) { + if conditions == nil { + return false + } + + // Find existing entry by type + var existing *k8sacmetav1.ConditionApplyConfiguration + for i := range *conditions { + if (*conditions)[i].Type != nil && *(*conditions)[i].Type == *newCondition.Type { + existing = &(*conditions)[i] + break + } + } + + if existing == nil { + // New condition: set LastTransitionTime if not provided + if newCondition.LastTransitionTime == nil { + now := metav1.Now() + newCondition.LastTransitionTime = &now + } + *conditions = append(*conditions, newCondition) + return true + } + + // Existing condition: update fields, handle LastTransitionTime + statusChanged := existing.Status == nil || newCondition.Status == nil || + *existing.Status != *newCondition.Status + + if statusChanged { + existing.Status = newCondition.Status + if newCondition.LastTransitionTime != nil { + existing.LastTransitionTime = newCondition.LastTransitionTime + } else { + now := metav1.Now() + existing.LastTransitionTime = &now + } + changed = true + } + // When status is unchanged, LastTransitionTime is intentionally preserved. + + if strChanged(&existing.Reason, newCondition.Reason) { + existing.Reason = newCondition.Reason + changed = true + } + if strChanged(&existing.Message, newCondition.Message) { + existing.Message = newCondition.Message + changed = true + } + + return changed +} + +// strChanged reports whether the pointer value of b differs from the current +// value at a. +func strChanged(a **string, b *string) bool { + if *a == nil && b == nil { + return false + } + if *a == nil || b == nil { + return true + } + return **a != *b +} From 35d43a41942aed0eeae858d78468ca24f5909c4f Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 5 May 2026 17:39:46 +0200 Subject: [PATCH 02/20] Migrate HypervisorController to SSA status updates Replace PatchHypervisorStatusWithRetry with Status().Apply() for the InternalIP scalar and ConditionTypeTerminating condition. Re-fetch the hypervisor after the status apply so the subsequent spec patch sees a fresh resourceVersion. --- internal/controller/hypervisor_controller.go | 90 +++++++++++--------- 1 file changed, 48 insertions(+), 42 deletions(-) diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index d174eb6..2a583ca 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -41,6 +42,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" 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/global" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" ) @@ -105,27 +107,13 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) } // continue with creation } else { - // First, propagate spec/metadata derived from the Node (labels, - // annotations -> aggregates/traits, lifecycle). This must run on - // every reconcile, including those where status will also change - // (e.g. AgentPodsEvicted=False during termination); otherwise the - // Hypervisor spec/labels go stale. - specBase := hypervisor.DeepCopy() - syncLabelsAndAnnotations(nodeLabels, hypervisor, node) - if !equality.Semantic.DeepEqual(hypervisor, specBase) { - if err := hv.Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(specBase, - k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorControllerName)); err != nil { - return ctrl.Result{}, err - } - } - - // Then, compute and persist any status changes derived from the Node. - statusBase := hypervisor.DeepCopy() + // update Status if needed // transfer internal IP + var newInternalIP string for _, address := range node.Status.Addresses { - if address.Type == corev1.NodeInternalIP && hypervisor.Status.InternalIP != address.Address { - hypervisor.Status.InternalIP = address.Address + if address.Type == corev1.NodeInternalIP { + newInternalIP = address.Address break } } @@ -133,14 +121,19 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) // update terminating condition — fired by either the legacy gardener // node condition or the node's deletion timestamp (gardener's future path) nodeTerminationCondition := FindNodeStatusCondition(node.Status.Conditions, "Terminating") + + // Capture values to apply - only mutate fields this controller owns + statusCfg := apiv1.HypervisorStatus().WithInternalIP(newInternalIP) + statusCfg.Conditions = utils.ConditionsFromStatus(hypervisor.Status.Conditions) + // Node might be terminating, propagate condition to hypervisor if (nodeTerminationCondition != nil && nodeTerminationCondition.Status == corev1.ConditionTrue) || !node.DeletionTimestamp.IsZero() { - meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeTerminating, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTerminating, - Message: "Node is terminating", - }) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeTerminating). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTerminating). + WithMessage("Node is terminating")) } // Only evaluate after VM eviction; a spurious True on a fresh node @@ -153,33 +146,46 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to compute %s condition: %w", kvmv1.ConditionTypeAgentPodsEvicted, err) } - meta.SetStatusCondition(&hypervisor.Status.Conditions, cond) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(cond.Type). + WithStatus(metav1.ConditionStatus(cond.Status)). + WithReason(cond.Reason). + WithMessage(cond.Message)) if cond.Status == metav1.ConditionFalse { // No pod watch — rely on periodic requeue. statusRequeueAfter = defaultPollTime } } - if !equality.Semantic.DeepEqual(hypervisor, statusBase) { - // Capture values to apply - only mutate fields this controller owns - newInternalIP := hypervisor.Status.InternalIP - terminatingCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTerminating) - agentPodsCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeAgentPodsEvicted) + if err := hv.Status().Apply(ctx, + apiv1.Hypervisor(hypervisor.Name, "").WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorControllerName)); err != nil { + return ctrl.Result{}, err + } - if err := utils.PatchHypervisorStatusWithRetry(ctx, hv.Client, hypervisor.Name, HypervisorControllerName, func(h *kvmv1.Hypervisor) { - h.Status.InternalIP = newInternalIP - if terminatingCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *terminatingCondition) - } - if agentPodsCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *agentPodsCondition) - } - }); err != nil { - return ctrl.Result{}, err - } + if statusRequeueAfter > 0 { + return ctrl.Result{RequeueAfter: statusRequeueAfter}, nil + } + + // Re-fetch after status apply so the spec patch has a fresh resourceVersion + if err := hv.Get(ctx, k8sclient.ObjectKeyFromObject(hypervisor), hypervisor); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to re-fetch hypervisor after status apply: %w", err) + } + + // First, propagate spec/metadata derived from the Node (labels, + // annotations -> aggregates/traits, lifecycle). This must run on + // every reconcile, including those where status will also change + // (e.g. AgentPodsEvicted=False during termination); otherwise the + // Hypervisor spec/labels go stale. + base := hypervisor.DeepCopy() + syncLabelsAndAnnotations(nodeLabels, hypervisor, node) + if equality.Semantic.DeepEqual(hypervisor, base) { + return ctrl.Result{}, nil } - return ctrl.Result{RequeueAfter: statusRequeueAfter}, nil + return ctrl.Result{}, hv.Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorControllerName)) } syncLabelsAndAnnotations(nodeLabels, hypervisor, node) From 004de66e9e15f1b221e744429c8938f8c2cf8909 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 5 May 2026 17:40:04 +0200 Subject: [PATCH 03/20] Migrate HypervisorTaintController to SSA status updates Replace PatchHypervisorStatusWithRetry with Status().Apply() for the ConditionTypeTainted condition. --- .../controller/hypervisor_taint_controller.go | 64 +++++++++---------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/internal/controller/hypervisor_taint_controller.go b/internal/controller/hypervisor_taint_controller.go index 7822b34..3fefe5c 100644 --- a/internal/controller/hypervisor_taint_controller.go +++ b/internal/controller/hypervisor_taint_controller.go @@ -20,16 +20,17 @@ package controller import ( "context" - "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + 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" "sigs.k8s.io/controller-runtime/pkg/predicate" 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/utils" ) @@ -46,52 +47,45 @@ type HypervisorTaintController struct { // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors/status,verbs=get;update;patch func (r *HypervisorTaintController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - hypervisor := &kvmv1.Hypervisor{ - ObjectMeta: metav1.ObjectMeta{ - Name: req.Name, - Labels: map[string]string{}, - }, - Spec: kvmv1.HypervisorSpec{ - HighAvailability: true, - InstallCertificate: true, - }, - } - - // Check if hypervisor already exists + hypervisor := &kvmv1.Hypervisor{} if err := r.Get(ctx, req.NamespacedName, hypervisor); err != nil { return ctrl.Result{}, k8sclient.IgnoreNotFound(err) } - before := hypervisor.DeepCopy() + var condStatus metav1.ConditionStatus + var reason, message string if HasKubectlManagedFields(&hypervisor.ObjectMeta) { - meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeTainted, - Status: metav1.ConditionTrue, - Reason: "Kubectl", - Message: "⚠️", - ObservedGeneration: hypervisor.Generation, - }) + condStatus = metav1.ConditionTrue + reason = "Kubectl" + message = "⚠️" } else { - meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeTainted, - Status: metav1.ConditionFalse, - Reason: "NoKubectl", - Message: "🟢", - ObservedGeneration: hypervisor.Generation, - }) + condStatus = metav1.ConditionFalse + reason = "NoKubectl" + message = "🟢" } - if equality.Semantic.DeepEqual(hypervisor, before) { + // Skip if already at the desired state + existing := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTainted) + if existing != nil && existing.Status == condStatus && + existing.Reason == reason && existing.Message == message && + existing.ObservedGeneration == hypervisor.Generation { return ctrl.Result{}, nil } // Only set the Tainted condition this controller owns - taintedCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTainted) - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hypervisor.Name, HypervisorTaintControllerName, func(h *kvmv1.Hypervisor) { - if taintedCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *taintedCondition) - } - }) + statusCfg := apiv1.HypervisorStatus() + statusCfg.Conditions = utils.ConditionsFromStatus(hypervisor.Status.Conditions) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeTainted). + WithStatus(condStatus). + WithReason(reason). + WithMessage(message). + WithObservedGeneration(hypervisor.Generation)) + + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Hypervisor(hypervisor.Name, "").WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorTaintControllerName)) } func (r *HypervisorTaintController) SetupWithManager(mgr ctrl.Manager) error { From 40f517b4aa886852b7e1ee4ad04267cfd7d77c95 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 5 May 2026 17:40:23 +0200 Subject: [PATCH 04/20] Migrate HypervisorMaintenanceController to SSA status updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace PatchHypervisorStatusWithRetry with Status().Apply() for ConditionTypeHypervisorDisabled, ConditionTypeEvicting, and the Evicted scalar. ConditionTypeEvicting is removed by omitting it from the apply config — SSA prunes map-list entries no longer claimed by the sole owner. --- .../hypervisor_maintenance_controller.go | 183 ++++++++++-------- 1 file changed, 104 insertions(+), 79 deletions(-) diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index 5c82487..cc16013 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -24,11 +24,11 @@ import ( "context" "fmt" - "k8s.io/apimachinery/pkg/api/equality" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -38,6 +38,7 @@ import ( "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/services" 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" ) @@ -58,7 +59,6 @@ type HypervisorMaintenanceController struct { func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { hv := &kvmv1.Hypervisor{} if err := hec.Get(ctx, req.NamespacedName, hv); err != nil { - // OnboardingReconciler not found errors, could be deleted return ctrl.Result{}, k8sclient.IgnoreNotFound(err) } @@ -69,59 +69,74 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c return ctrl.Result{}, nil } - old := hv.DeepCopy() - - if err := hec.reconcileComputeService(ctx, hv); err != nil { + // Determine desired disabled condition and eviction state + disabledCond, evictingCond, evicted, err := hec.reconcileComputeService(ctx, hv) + if err != nil { return ctrl.Result{}, err } - if err := hec.reconcileEviction(ctx, hv); err != nil { + evictingCond, evicted, err = hec.reconcileEviction(ctx, hv, evictingCond, evicted) + if err != nil { return ctrl.Result{}, err } - if equality.Semantic.DeepEqual(hv, old) { - return ctrl.Result{}, nil - } + // Build status apply config: always include conditions this controller owns; + // omit ConditionTypeEvicting when it should be removed (SSA prunes it). + statusCfg := apiv1.HypervisorStatus().WithEvicted(evicted) + statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) - // Capture only the fields this controller owns - disabledCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) - evictingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting) - evicted := hv.Status.Evicted + if disabledCond != nil { + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *disabledCond) + } - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, hec.Client, hv.Name, HypervisorMaintenanceControllerName, func(h *kvmv1.Hypervisor) { - if disabledCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *disabledCondition) - } - if evictingCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *evictingCondition) - } else { - meta.RemoveStatusCondition(&h.Status.Conditions, kvmv1.ConditionTypeEvicting) + if evictingCond != nil { + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *evictingCond) + } else { + // Remove ConditionTypeEvicting by omitting it — SSA prunes sole-owned entries. + filtered := statusCfg.Conditions[:0] + for _, c := range statusCfg.Conditions { + if c.Type == nil || *c.Type != kvmv1.ConditionTypeEvicting { + filtered = append(filtered, c) + } } - h.Status.Evicted = evicted - }) + statusCfg.Conditions = filtered + } + + return ctrl.Result{}, hec.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorMaintenanceControllerName)) } -func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor) error { +// reconcileComputeService enables/disables the nova-compute service based on +// hv.Spec.Maintenance. Returns the desired HypervisorDisabled condition (nil if +// service ID is unset) and the initial evicting/evicted state. +func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor) ( + disabledCond *k8sacmetav1.ConditionApplyConfiguration, + evictingCond *k8sacmetav1.ConditionApplyConfiguration, + evicted bool, + err error, +) { log := logger.FromContext(ctx) serviceId := hv.Status.ServiceID - // We can only do something here, if there is a service to begin with. - // The onboarding should take care of that if serviceId == "" { - return nil + // We can only do something here, if there is a service to begin with. + // The onboarding should take care of that. + return nil, nil, false, nil } switch hv.Spec.Maintenance { case kvmv1.MaintenanceUnset: - // Enable the compute service (in case we haven't done so already) - if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeHypervisorDisabled, - Status: metav1.ConditionFalse, - Message: "Hypervisor is enabled", - Reason: kvmv1.ConditionReasonSucceeded, - }) { - // Spec matches status - return nil + cond := k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeHypervisorDisabled). + WithStatus(metav1.ConditionFalse). + WithMessage("Hypervisor is enabled"). + WithReason(kvmv1.ConditionReasonSucceeded) + + existing := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + if existing != nil && existing.Status == metav1.ConditionFalse { + // Already enabled, nothing to do + return cond, nil, false, nil } // We need to enable the host as per spec and clear forced_down @@ -132,88 +147,95 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context. ForcedDown: &falseVal, } log.Info("Enabling hypervisor", "id", serviceId) - _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract() - if err != nil { - return fmt.Errorf("failed to enable hypervisor due to %w", err) + if _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract(); err != nil { + return nil, nil, false, fmt.Errorf("failed to enable hypervisor due to %w", err) } + return cond, nil, false, nil + case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceTermination: // Disable the compute service. - if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeHypervisorDisabled, - Status: metav1.ConditionTrue, - Message: "Hypervisor is disabled", - Reason: kvmv1.ConditionReasonSucceeded, - }) { - // Spec matches status - return nil + cond := k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeHypervisorDisabled). + WithStatus(metav1.ConditionTrue). + WithMessage("Hypervisor is disabled"). + WithReason(kvmv1.ConditionReasonSucceeded) + + existing := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + if existing != nil && existing.Status == metav1.ConditionTrue { + // Already disabled, nothing to do + return cond, nil, false, nil } - // We need to disable the host as per spec - enableService := services.UpdateOpts{ + disableService := services.UpdateOpts{ Status: services.ServiceDisabled, DisabledReason: "Hypervisor CRD: spec.maintenance=" + hv.Spec.Maintenance, } + // We need to disable the host as per spec log.Info("Disabling hypervisor", "id", serviceId) - _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract() - if err != nil { - return fmt.Errorf("failed to disable hypervisor due to %w", err) + if _, err := services.Update(ctx, hec.computeClient, serviceId, disableService).Extract(); err != nil { + return nil, nil, false, fmt.Errorf("failed to disable hypervisor due to %w", err) } + return cond, nil, false, nil } - return nil + return nil, nil, false, nil } -func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Context, hv *kvmv1.Hypervisor) error { +// reconcileEviction creates/deletes the Eviction CR and returns the desired +// ConditionTypeEvicting apply configuration (nil means remove the condition). +func (hec *HypervisorMaintenanceController) reconcileEviction( + ctx context.Context, + hv *kvmv1.Hypervisor, + evictingCond *k8sacmetav1.ConditionApplyConfiguration, + evicted bool, +) (*k8sacmetav1.ConditionApplyConfiguration, bool, error) { eviction := &kvmv1.Eviction{ - ObjectMeta: metav1.ObjectMeta{ - Name: hv.Name, - }, + ObjectMeta: metav1.ObjectMeta{Name: hv.Name}, } switch hv.Spec.Maintenance { case kvmv1.MaintenanceUnset: // Avoid deleting the eviction over and over. - if hv.Status.Evicted || meta.RemoveStatusCondition(&hv.Status.Conditions, kvmv1.ConditionTypeEvicting) { - err := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)) - hv.Status.Evicted = false - return err + if !hv.Status.Evicted && meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting) == nil { + return nil, false, nil } - return nil + err := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)) + return nil, false, err // nil evictingCond → condition omitted from apply → SSA prunes it + case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceTermination: // In case of "ha", the host gets emptied from the HA service if cond := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting); cond != nil { if cond.Reason == kvmv1.ConditionReasonSucceeded { // We are done here, no need to look at the eviction any more - return nil + return evictingCond, evicted, nil } } + status, err := hec.ensureEviction(ctx, eviction, hv) if err != nil { - return err + return evictingCond, evicted, err } - var reason, message string + var reason, message string if status == metav1.ConditionFalse { message = "Evicted" reason = kvmv1.ConditionReasonSucceeded - hv.Status.Evicted = true + evicted = true } else { message = "Evicting" reason = kvmv1.ConditionReasonRunning - hv.Status.Evicted = false + evicted = false } - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: status, - Reason: reason, - Message: message, - }) - - return nil + cond := k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(status). + WithReason(reason). + WithMessage(message) + return cond, evicted, nil } - return nil + return evictingCond, evicted, nil } func (hec *HypervisorMaintenanceController) ensureEviction(ctx context.Context, eviction *kvmv1.Eviction, hypervisor *kvmv1.Hypervisor) (metav1.ConditionStatus, error) { @@ -242,9 +264,8 @@ func (hec *HypervisorMaintenanceController) ensureEviction(ctx context.Context, // check if we are still evicting (defaulting to yes) if meta.IsStatusConditionFalse(eviction.Status.Conditions, kvmv1.ConditionTypeEvicting) { return metav1.ConditionFalse, nil - } else { - return metav1.ConditionTrue, nil } + return metav1.ConditionTrue, nil } // registerWithManager registers the controller with the Manager without acquiring OpenStack clients. @@ -267,5 +288,9 @@ func (hec *HypervisorMaintenanceController) SetupWithManager(mgr ctrl.Manager) e } hec.computeClient.Microversion = "2.90" // Xena (or later) - return hec.registerWithManager(mgr) + return ctrl.NewControllerManagedBy(mgr). + Named(HypervisorMaintenanceControllerName). + For(&kvmv1.Hypervisor{}). + Owns(&kvmv1.Eviction{}). + Complete(hec) } From 723b232c51c2cf3b92ed7e7e90bf91e9e265d1b5 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 5 May 2026 17:40:34 +0200 Subject: [PATCH 05/20] Migrate OnboardingController to SSA status updates Replace PatchHypervisorStatusWithRetry with Status().Apply(). The Nova ID lookup is inlined into Reconcile and combined with the initial condition in a single apply, avoiding two applies from the same field manager in one reconcile cycle which would cause SSA to prune the IDs. HypervisorID and ServiceID are included in every subsequent apply to retain ownership. --- internal/controller/onboarding_controller.go | 266 ++++++++----------- 1 file changed, 113 insertions(+), 153 deletions(-) diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index 1f52d0e..574e1b2 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -26,11 +26,11 @@ import ( "time" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" "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" "k8s.io/utils/clock" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -45,6 +45,7 @@ import ( "github.com/gophercloud/utils/v2/openstack/clientconfig" 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" ) @@ -107,28 +108,30 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) // We bail here out, because the openstack api is not the best to poll if hv.Status.HypervisorID == "" || hv.Status.ServiceID == "" { - if err := r.ensureNovaProperties(ctx, hv); err != nil { + hypervisorID, serviceID, err := r.lookupNovaProperties(ctx, hv) + if err != nil { if errors.Is(err, errRequeue) { return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } return ctrl.Result{}, err } + statusCfg := apiv1.HypervisorStatus(). + WithHypervisorID(hypervisorID). + WithServiceID(serviceID) + statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonInitial). + WithMessage("Initial onboarding")) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(OnboardingControllerName)) } // check condition reason status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) - if status == nil { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonInitial, - Message: "Initial onboarding", - } - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) - } - switch status.Reason { case kvmv1.ConditionReasonInitial: return ctrl.Result{}, r.initialOnboarding(ctx, hv) @@ -146,6 +149,19 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) } } +// applyOnboardingCondition applies a single onboarding condition via SSA, +// carrying all existing conditions and scalar fields forward. +func (r *OnboardingController) applyOnboardingCondition(ctx context.Context, hv *kvmv1.Hypervisor, cond k8sacmetav1.ConditionApplyConfiguration) error { + statusCfg := apiv1.HypervisorStatus(). + WithHypervisorID(hv.Status.HypervisorID). + WithServiceID(hv.Status.ServiceID) + statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, cond) + return r.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(OnboardingControllerName)) +} + func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, computeHost, message string) error { status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) // Never onboarded @@ -153,42 +169,28 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy return nil } - base := hv.DeepCopy() - - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonAborted, - Message: message, - }) - - if equality.Semantic.DeepEqual(hv, base) { - // Already aborted + // Already aborted with the same message — nothing to do + if status.Status == metav1.ConditionFalse && + status.Reason == kvmv1.ConditionReasonAborted && + status.Message == message { return nil } - condition := *meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) if err := r.deleteTestServers(ctx, computeHost); err != nil { - condition = metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, // No cleanup, so we are still "onboarding" - Reason: kvmv1.ConditionReasonAborted, - Message: err.Error(), - } - - meta.SetStatusCondition(&hv.Status.Conditions, condition) - if equality.Semantic.DeepEqual(hv, base) { - return err - } - - return errors.Join(err, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - })) - } - - return utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) + return errors.Join(err, r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). // No cleanup, so we are still "onboarding" + WithReason(kvmv1.ConditionReasonAborted). + WithMessage(err.Error()))) + } + + return r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonAborted). + WithMessage(message)) } func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1.Hypervisor) error { @@ -198,7 +200,6 @@ func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1. } // Wait for aggregates controller to apply the desired state (zone and test aggregate) - // Extract aggregate names for comparison currentAggregateNames := make([]string, len(hv.Status.Aggregates)) for i, agg := range hv.Status.Aggregates { currentAggregateNames[i] = agg.Name @@ -220,15 +221,12 @@ func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1. return result.Err } - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: "Running onboarding tests", - } - return utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) + return r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage("Running onboarding tests")) } func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervisor, host string) (ctrl.Result, error) { @@ -245,21 +243,17 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis id := server.ID server, err = servers.Get(ctx, r.testComputeClient, id).Extract() if err != nil { - // should not happened log.Error(err, "failed to get test instance, instance vanished", "id", id) return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } // Set condition back to testing - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: "Server ended up in error state: " + server.Fault.Message, - } - if err = utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }); err != nil { + if err = r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage("Server ended up in error state: "+server.Fault.Message)); err != nil { return ctrl.Result{}, err } @@ -268,38 +262,33 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis log.Error(err, "failed to delete test instance", "id", id) } - return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil + return ctrl.Result{RequeueAfter: defaultWaitTime}, nil + case "ACTIVE": consoleOutput, err := servers. ShowConsoleOutput(ctx, r.testComputeClient, server.ID, servers.ShowConsoleOutputOpts{Length: 11}). Extract() if err != nil { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: fmt.Sprintf("could not get console output %v", err), - } - if err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }); err != nil { - return ctrl.Result{}, err + if err2 := r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage(fmt.Sprintf("could not get console output %v", err))); err2 != nil { + return ctrl.Result{}, err2 } return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } if !strings.Contains(consoleOutput, server.Name) { if !server.LaunchedAt.IsZero() && r.Clock.Now().After(server.LaunchedAt.Add(smokeTestTimeout)) { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: fmt.Sprintf("timeout waiting for console output since %v", server.LaunchedAt), - } - if err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }); err != nil { - return ctrl.Result{}, err + if err2 := r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage(fmt.Sprintf("timeout waiting for console output since %v", server.LaunchedAt))); err2 != nil { + return ctrl.Result{}, err2 } if err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr(); err != nil { if !gophercloud.ResponseCodeIs(err, http.StatusNotFound) { @@ -311,28 +300,25 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis } if err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr(); err != nil { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: fmt.Sprintf("failed to terminate instance %v", err), - } - if err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }); err != nil { - return ctrl.Result{}, err + if err2 := r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage(fmt.Sprintf("failed to terminate instance %v", err))); err2 != nil { + return ctrl.Result{}, err2 } return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } return r.completeOnboarding(ctx, host, hv) + default: return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } } func (r *OnboardingController) completeOnboarding(ctx context.Context, host string, hv *kvmv1.Hypervisor) (ctrl.Result, error) { - // Check if we're in the RemovingTestAggregate phase onboardingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) if onboardingCondition != nil && onboardingCondition.Reason == kvmv1.ConditionReasonHandover { // We're waiting for aggregates and traits controllers to sync @@ -348,44 +334,32 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri return ctrl.Result{}, nil } - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonSucceeded, - Message: "Onboarding completed", - } - - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) + return ctrl.Result{}, r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonSucceeded). + WithMessage("Onboarding completed")) } // First time in completeOnboarding - clean up and prepare for aggregate sync - err := r.deleteTestServers(ctx, host) - if err != nil { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, // No cleanup, so we are still "onboarding" - Reason: kvmv1.ConditionReasonAborted, - Message: err.Error(), - } - return ctrl.Result{}, errors.Join(err, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - })) + if err := r.deleteTestServers(ctx, host); err != nil { + return ctrl.Result{}, errors.Join(err, r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). // No cleanup, so we are still "onboarding" + WithReason(kvmv1.ConditionReasonAborted). + WithMessage(err.Error()))) } // Mark onboarding as almost complete, triggers other controllers to do their part - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonHandover, - Message: "Waiting for other controllers to take over", - } - // Patch status to signal aggregates controller - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) + return ctrl.Result{}, r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonHandover). + WithMessage("Waiting for other controllers to take over")) } func (r *OnboardingController) deleteTestServers(ctx context.Context, host string) error { @@ -417,10 +391,10 @@ func (r *OnboardingController) deleteTestServers(ctx context.Context, host strin return errors.Join(errs...) } -func (r *OnboardingController) ensureNovaProperties(ctx context.Context, hv *kvmv1.Hypervisor) error { +func (r *OnboardingController) lookupNovaProperties(ctx context.Context, hv *kvmv1.Hypervisor) (hypervisorID, serviceID string, err error) { hypervisorAddress := hv.Labels[corev1.LabelHostname] if hypervisorAddress == "" { - return errRequeue + return "", "", errRequeue } shortHypervisorAddress := strings.SplitN(hypervisorAddress, ".", 2)[0] @@ -429,41 +403,27 @@ func (r *OnboardingController) ensureNovaProperties(ctx context.Context, hv *kvm hypervisorPages, err := hypervisors.List(r.computeClient, hypervisorQuery).AllPages(ctx) if err != nil { if gophercloud.ResponseCodeIs(err, http.StatusNotFound) { - return errRequeue + return "", "", errRequeue } - return err + return "", "", err } hs, err := hypervisors.ExtractHypervisors(hypervisorPages) if err != nil { - return err + return "", "", err } if len(hs) < 1 { - return errRequeue + return "", "", errRequeue } - var found = false - var myHypervisor hypervisors.Hypervisor for _, h := range hs { - short := strings.SplitN(h.HypervisorHostname, ".", 2)[0] - if short == shortHypervisorAddress { - myHypervisor = h - found = true - break + if strings.SplitN(h.HypervisorHostname, ".", 2)[0] == shortHypervisorAddress { + return h.ID, h.Service.ID, nil } } - if !found { - return fmt.Errorf("could not find exact match for %v", shortHypervisorAddress) - } - - hypervisorID := myHypervisor.ID - serviceID := myHypervisor.Service.ID - return utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OnboardingControllerName, func(h *kvmv1.Hypervisor) { - h.Status.HypervisorID = hypervisorID - h.Status.ServiceID = serviceID - }) + return "", "", fmt.Errorf("could not find exact match for %v", shortHypervisorAddress) } func (r *OnboardingController) createOrGetTestServer(ctx context.Context, zone, computeHost string, nodeUid types.UID) (*servers.Server, error) { @@ -484,9 +444,9 @@ func (r *OnboardingController) createOrGetTestServer(ctx context.Context, zone, } log := logger.FromContext(ctx) + var foundServer *servers.Server // Cleanup all other server with the same test prefix, except for the exact match // as the cleanup after onboarding may leak resources - var foundServer *servers.Server for _, server := range serverList { // The query is a substring search, we are looking for a prefix if !strings.HasPrefix(server.Name, serverPrefix) { From 2f41ef9d1cececd6043de35ee56a0e17824ba939 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 5 May 2026 17:40:41 +0200 Subject: [PATCH 06/20] Migrate HypervisorOffboardingReconciler to SSA status updates Replace PatchHypervisorStatusWithRetry with Status().Apply() for the ConditionTypeOffboarded condition. --- internal/controller/offboarding_controller.go | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/internal/controller/offboarding_controller.go b/internal/controller/offboarding_controller.go index eee2cfe..46860af 100644 --- a/internal/controller/offboarding_controller.go +++ b/internal/controller/offboarding_controller.go @@ -29,11 +29,13 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" logger "sigs.k8s.io/controller-runtime/pkg/log" 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" ) @@ -157,15 +159,21 @@ func (r *HypervisorOffboardingReconciler) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, r.markOffboarded(ctx, hv) } +func (r *HypervisorOffboardingReconciler) applyStatus(ctx context.Context, hv *kvmv1.Hypervisor, cond *k8sacmetav1.ConditionApplyConfiguration) error { + statusCfg := apiv1.HypervisorStatus() + statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *cond) + return r.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(OffboardingControllerName)) +} + func (r *HypervisorOffboardingReconciler) setOffboardingCondition(ctx context.Context, hv *kvmv1.Hypervisor, message string) (ctrl.Result, error) { - err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OffboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeOffboarded, - Status: metav1.ConditionFalse, - Reason: "Offboarding", - Message: message, - }) - }) + err := r.applyStatus(ctx, hv, k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOffboarded). + WithStatus(metav1.ConditionFalse). + WithReason("Offboarding"). + WithMessage(message)) if err != nil { return ctrl.Result{}, fmt.Errorf("cannot update hypervisor status due to %w", err) } @@ -173,14 +181,11 @@ func (r *HypervisorOffboardingReconciler) setOffboardingCondition(ctx context.Co } func (r *HypervisorOffboardingReconciler) markOffboarded(ctx context.Context, hv *kvmv1.Hypervisor) error { - err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OffboardingControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeOffboarded, - Status: metav1.ConditionTrue, - Reason: "Offboarded", - Message: "Offboarding successful", - }) - }) + err := r.applyStatus(ctx, hv, k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOffboarded). + WithStatus(metav1.ConditionTrue). + WithReason("Offboarded"). + WithMessage("Offboarding successful")) if err != nil { return fmt.Errorf("cannot update hypervisor status due to %w", err) } From 35efa4f2ad7ab385d39ee54a465159da6ebcec23 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 5 May 2026 17:40:50 +0200 Subject: [PATCH 07/20] 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. --- internal/controller/aggregates_controller.go | 86 ++++++++++++++++---- 1 file changed, 70 insertions(+), 16 deletions(-) diff --git a/internal/controller/aggregates_controller.go b/internal/controller/aggregates_controller.go index 30aa47e..5f40a4f 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,30 @@ 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() + statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + 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 +123,52 @@ 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() + statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + 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 } - meta.SetStatusCondition(&h.Status.Conditions, desiredCondition) - }) + fresh := &kvmv1.Hypervisor{} + if err := ac.Get(ctx, k8sclient.ObjectKey{Name: hv.Name}, fresh); err != nil { + return ctrl.Result{}, err + } + 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 From cc33b1685abe83d1e3a9145987b3dc5b45dcfd54 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 5 May 2026 17:40:59 +0200 Subject: [PATCH 08/20] Migrate TraitsController to SSA status updates Replace PatchHypervisorStatusWithRetry with Status().Apply() for ConditionTypeTraitsUpdated and the Traits slice. Error paths pass the current hv.Status.Traits to retain ownership and avoid SSA releasing it on transient placement API failures. --- internal/controller/traits_controller.go | 42 ++++++++++++++---------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/internal/controller/traits_controller.go b/internal/controller/traits_controller.go index 4e91e21..fff3aa8 100644 --- a/internal/controller/traits_controller.go +++ b/internal/controller/traits_controller.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -34,6 +35,7 @@ import ( "github.com/gophercloud/gophercloud/v2/openstack/placement/v1/resourceproviders" 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" ) @@ -103,11 +105,8 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct // fetch current traits, to ensure we don't add duplicates current, err := resourceproviders.GetTraits(ctx, tc.serviceClient, hv.Status.HypervisorID).Extract() if err != nil { - condition := getTraitCondition(err, "Failed to get current traits from placement") - patchErr := utils.PatchHypervisorStatusWithRetry(ctx, tc.Client, hv.Name, TraitsControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) - return ctrl.Result{}, errors.Join(err, patchErr) + return ctrl.Result{}, errors.Join(err, + tc.applyTraitsStatus(ctx, hv, hv.Status.Traits, getTraitCondition(err, "Failed to get current traits from placement"))) } var targetTraits []string @@ -131,22 +130,31 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct ResourceProviderGeneration: current.ResourceProviderGeneration, Traits: targetTraits, }) - err = result.Err - if err != nil { - condition := getTraitCondition(err, "Failed to update traits in placement") - patchErr := utils.PatchHypervisorStatusWithRetry(ctx, tc.Client, hv.Name, TraitsControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) - return ctrl.Result{}, errors.Join(err, patchErr) + if result.Err != nil { + return ctrl.Result{}, errors.Join(result.Err, + tc.applyTraitsStatus(ctx, hv, hv.Status.Traits, getTraitCondition(result.Err, "Failed to update traits in placement"))) } } // update status unconditionally, since we want always to propagate the current traits - err = utils.PatchHypervisorStatusWithRetry(ctx, tc.Client, hv.Name, TraitsControllerName, func(h *kvmv1.Hypervisor) { - h.Status.Traits = targetTraits - meta.SetStatusCondition(&h.Status.Conditions, getTraitCondition(nil, "Traits successfully updated")) - }) - return ctrl.Result{}, err + return ctrl.Result{}, tc.applyTraitsStatus(ctx, hv, targetTraits, getTraitCondition(nil, "Traits successfully updated")) +} + +func (tc *TraitsController) applyTraitsStatus(ctx context.Context, hv *kvmv1.Hypervisor, traits []string, cond metav1.Condition) error { + statusCfg := apiv1.HypervisorStatus() + statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(cond.Type). + WithStatus(cond.Status). + WithReason(cond.Reason). + WithMessage(cond.Message)) + if traits != nil { + statusCfg.WithTraits(traits...) + } + return tc.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(TraitsControllerName)) } // getTraitCondition creates a Condition object for trait updates From db4720d0b01082f834a87b3d5bf2a90ad3f1bfa6 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 5 May 2026 17:41:07 +0200 Subject: [PATCH 09/20] Migrate ready.Controller to SSA status updates; remove PatchHypervisorStatusWithRetry Replace the last caller of PatchHypervisorStatusWithRetry with Status().Apply() for ConditionTypeReady. With no remaining callers, PatchHypervisorStatusWithRetry and StatusPatchBackoff are removed. --- internal/controller/ready/controller.go | 23 ++++++----- internal/utils/status_patch.go | 55 ------------------------- 2 files changed, 13 insertions(+), 65 deletions(-) delete mode 100644 internal/utils/status_patch.go diff --git a/internal/controller/ready/controller.go b/internal/controller/ready/controller.go index 813d583..29293ba 100644 --- a/internal/controller/ready/controller.go +++ b/internal/controller/ready/controller.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + 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" @@ -32,6 +33,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" 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/utils" ) @@ -104,20 +106,21 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, k8sclient.IgnoreNotFound(err) } - base := hv.DeepCopy() - // Compute Ready condition based on other conditions readyCondition := ComputeReadyCondition(hv) - meta.SetStatusCondition(&hv.Status.Conditions, readyCondition) - - if equality.Semantic.DeepEqual(hv.Status, base.Status) { - return ctrl.Result{}, nil - } log.Info("Updating Ready condition", "status", readyCondition.Status, "reason", readyCondition.Reason) - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, req.Name, ControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, readyCondition) - }) + statusCfg := apiv1.HypervisorStatus() + statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(readyCondition.Type). + WithStatus(readyCondition.Status). + WithReason(readyCondition.Reason). + WithMessage(readyCondition.Message)) + return ctrl.Result{}, r.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(ControllerName)) } // ComputeReadyCondition determines the Ready condition based on other conditions diff --git a/internal/utils/status_patch.go b/internal/utils/status_patch.go deleted file mode 100644 index e353f87..0000000 --- a/internal/utils/status_patch.go +++ /dev/null @@ -1,55 +0,0 @@ -/* -SPDX-FileCopyrightText: Copyright 2025 SAP SE or an SAP affiliate company and cobaltcore-dev contributors -SPDX-License-Identifier: Apache-2.0 - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package utils - -import ( - "context" - "time" - - "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/client-go/util/retry" - k8sclient "sigs.k8s.io/controller-runtime/pkg/client" - - kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" -) - -// StatusPatchBackoff is a retry backoff for status patches that may conflict -// with other controllers. Uses exponential backoff with more retries than the -// default to handle high contention scenarios. -var StatusPatchBackoff = wait.Backoff{ - Steps: 10, - Duration: 50 * time.Millisecond, - Factor: 2.0, - Jitter: 0.2, -} - -// PatchHypervisorStatusWithRetry patches hypervisor status with retry on conflict. -// The updateFn receives a fresh copy of the hypervisor and should apply status changes to it. -// It re-fetches the resource before each retry attempt to get the latest resourceVersion. -func PatchHypervisorStatusWithRetry(ctx context.Context, c k8sclient.Client, name, fieldOwner string, updateFn func(*kvmv1.Hypervisor)) error { - return retry.RetryOnConflict(StatusPatchBackoff, func() error { - hv := &kvmv1.Hypervisor{} - if err := c.Get(ctx, k8sclient.ObjectKey{Name: name}, hv); err != nil { - return err - } - base := hv.DeepCopy() - updateFn(hv) - return c.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, - k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(fieldOwner)) - }) -} From 61e89a893a7f8940454ce9129f7e7a1553b7fb00 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Wed, 6 May 2026 11:10:55 +0200 Subject: [PATCH 10/20] Migrate HypervisorMaintenanceController Eviction creation to SSA Replace Create + SetControllerReference with Apply for the Eviction CR. The owner reference and labels are set in the apply configuration metadata, and SSA handles the upsert. A Get is still performed after the apply to read the current eviction status conditions. --- .../hypervisor_maintenance_controller.go | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index cc16013..349d3ac 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -24,14 +24,12 @@ import ( "context" "fmt" - k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logger "sigs.k8s.io/controller-runtime/pkg/log" "github.com/gophercloud/gophercloud/v2" @@ -240,25 +238,39 @@ func (hec *HypervisorMaintenanceController) reconcileEviction( func (hec *HypervisorMaintenanceController) ensureEviction(ctx context.Context, eviction *kvmv1.Eviction, hypervisor *kvmv1.Hypervisor) (metav1.ConditionStatus, error) { log := logger.FromContext(ctx) - if err := hec.Get(ctx, k8sclient.ObjectKeyFromObject(eviction), eviction); err != nil { - if !k8serrors.IsNotFound(err) { - return metav1.ConditionUnknown, fmt.Errorf("failed to get eviction due to %w", err) - } - if err := controllerutil.SetControllerReference(hypervisor, eviction, hec.Scheme); err != nil { - return metav1.ConditionUnknown, err - } - log.Info("Creating new eviction", "name", eviction.Name) - eviction.Spec = kvmv1.EvictionSpec{ - Hypervisor: hypervisor.Name, - Reason: "openstack-hypervisor-operator maintenance", + + // Build labels to transport from hypervisor (e.g. label-selector, if set) + evictionLabels := make(map[string]string) + for _, label := range transferLabels { + if v, ok := hypervisor.Labels[label]; ok { + evictionLabels[label] = v } + } - // This also transports the label-selector, if set - transportLabels(&hypervisor.ObjectMeta, &eviction.ObjectMeta) + ownerRef := k8sacmetav1.OwnerReference(). + WithAPIVersion(kvmv1.GroupVersion.String()). + WithKind("Hypervisor"). + WithName(hypervisor.Name). + WithUID(hypervisor.UID). + WithController(true). + WithBlockOwnerDeletion(true) + + evictionApplyCfg := apiv1.Eviction(eviction.Name, eviction.Namespace). + WithLabels(evictionLabels). + WithOwnerReferences(ownerRef). + WithSpec(apiv1.EvictionSpec(). + WithHypervisor(hypervisor.Name). + WithReason("openstack-hypervisor-operator maintenance")) + + log.Info("Applying eviction", "name", eviction.Name) + if err := hec.Apply(ctx, evictionApplyCfg, + k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorMaintenanceControllerName)); err != nil { + return metav1.ConditionUnknown, fmt.Errorf("failed to apply eviction due to %w", err) + } - if err = hec.Create(ctx, eviction); err != nil { - return metav1.ConditionUnknown, fmt.Errorf("failed to create eviction due to %w", err) - } + // Re-fetch to read current eviction status + if err := hec.Get(ctx, k8sclient.ObjectKeyFromObject(eviction), eviction); err != nil { + return metav1.ConditionUnknown, fmt.Errorf("failed to get eviction status due to %w", err) } // check if we are still evicting (defaulting to yes) From d06dd1f94e170d69431b560db29cf8b2c1ba69bf Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Wed, 6 May 2026 11:10:55 +0200 Subject: [PATCH 11/20] Migrate NodeCertificateController Certificate creation to SSA Replace CreateOrUpdate + SetOwnerReference with Apply for the cert-manager Certificate CR. The owner reference is set in the apply configuration metadata. The retry-on-conflict loop is no longer needed since SSA handles concurrent updates correctly. The controller now applies the full certificate spec on every reconcile so IP/DNS changes are picked up immediately. --- .../controller/node_certificate_controller.go | 142 ++++++++---------- 1 file changed, 60 insertions(+), 82 deletions(-) diff --git a/internal/controller/node_certificate_controller.go b/internal/controller/node_certificate_controller.go index b8da375..8287c48 100644 --- a/internal/controller/node_certificate_controller.go +++ b/internal/controller/node_certificate_controller.go @@ -24,14 +24,14 @@ import ( "time" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + cmapplyv1 "github.com/cert-manager/cert-manager/pkg/client/applyconfigurations/certmanager/v1" + cmapplymetav1 "github.com/cert-manager/cert-manager/pkg/client/applyconfigurations/meta/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/util/retry" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ctrl "sigs.k8s.io/controller-runtime" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logger "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" ) @@ -59,95 +59,77 @@ func (r *NodeCertificateController) ensureCertificate(ctx context.Context, node secretName, certName := getSecretAndCertName(node.Name) - certificate := &cmapi.Certificate{ - TypeMeta: metav1.TypeMeta{ - Kind: cmapi.CertificateKind, - APIVersion: cmapi.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: certName, - Namespace: r.namespace, - }, - } - - update, err := controllerutil.CreateOrUpdate(ctx, r.Client, certificate, func() error { - if err := controllerutil.SetOwnerReference(node, certificate, r.Scheme); err != nil { - return err - } - - ipAddressSet := make(map[string]bool) - dnsNameSet := make(map[string]bool) - - dnsNameSet[computeHost] = true + ipAddressSet := make(map[string]bool) + dnsNameSet := make(map[string]bool) - for _, addr := range node.Status.Addresses { - if addr.Address == "" { - continue - } + dnsNameSet[computeHost] = true - switch addr.Type { - case corev1.NodeHostName, corev1.NodeInternalDNS, corev1.NodeExternalDNS: - dnsNameSet[addr.Address] = true - case corev1.NodeInternalIP, corev1.NodeExternalIP: - ipAddressSet[addr.Address] = true - } + for _, addr := range node.Status.Addresses { + if addr.Address == "" { + continue } - ipAddresses := make([]string, 0, len(ipAddressSet)) - for k := range ipAddressSet { - ipAddresses = append(ipAddresses, k) - } - - slices.Sort(ipAddresses) - - dnsNames := make([]string, 0, len(dnsNameSet)) - for k := range dnsNameSet { - dnsNames = append(dnsNames, k) + switch addr.Type { + case corev1.NodeHostName, corev1.NodeInternalDNS, corev1.NodeExternalDNS: + dnsNameSet[addr.Address] = true + case corev1.NodeInternalIP, corev1.NodeExternalIP: + ipAddressSet[addr.Address] = true } + } - slices.Sort(dnsNames) + ipAddresses := make([]string, 0, len(ipAddressSet)) + for k := range ipAddressSet { + ipAddresses = append(ipAddresses, k) + } + slices.Sort(ipAddresses) - certificate.Spec = cmapi.CertificateSpec{ - SecretName: secretName, - PrivateKey: &cmapi.CertificatePrivateKey{ - Algorithm: cmapi.RSAKeyAlgorithm, - Encoding: cmapi.PKCS1, - Size: 4096, - }, + dnsNames := make([]string, 0, len(dnsNameSet)) + for k := range dnsNameSet { + dnsNames = append(dnsNames, k) + } + slices.Sort(dnsNames) + + ownerRef := k8sacmetav1.OwnerReference(). + WithAPIVersion(corev1.SchemeGroupVersion.String()). + WithKind("Node"). + WithName(node.Name). + WithUID(node.UID) + + certApplyCfg := cmapplyv1.Certificate(certName, r.namespace). + WithOwnerReferences(ownerRef). + WithSpec(cmapplyv1.CertificateSpec(). + WithSecretName(secretName). + WithPrivateKey(cmapplyv1.CertificatePrivateKey(). + WithAlgorithm(cmapi.RSAKeyAlgorithm). + WithEncoding(cmapi.PKCS1). + WithSize(4096)). // Matching the CA/Browser Forum's maximum duration for 2029 - Duration: &metav1.Duration{Duration: 47 * 24 * time.Hour}, - RenewBefore: &metav1.Duration{Duration: 37 * 24 * time.Hour}, - IsCA: false, - Usages: []cmapi.KeyUsage{ + WithDuration(metav1.Duration{Duration: 47 * 24 * time.Hour}). + WithRenewBefore(metav1.Duration{Duration: 37 * 24 * time.Hour}). + WithIsCA(false). + WithUsages( cmapi.UsageServerAuth, cmapi.UsageClientAuth, cmapi.UsageCertSign, // Really? cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, - }, - Subject: &cmapi.X509Subject{ - Organizations: []string{"nova"}, - }, - CommonName: computeHost, - DNSNames: dnsNames, - IPAddresses: ipAddresses, - IssuerRef: cmmeta.IssuerReference{ - Name: r.issuerName, - Kind: cmapi.IssuerKind, - Group: cmapi.SchemeGroupVersion.Group, - }, - } - return nil - }) - - if err != nil { + ). + WithSubject(cmapplyv1.X509Subject(). + WithOrganizations("nova")). + WithCommonName(computeHost). + WithDNSNames(dnsNames...). + WithIPAddresses(ipAddresses...). + WithIssuerRef(cmapplymetav1.IssuerReference(). + WithName(r.issuerName). + WithKind(cmapi.IssuerKind). + WithGroup(cmapi.SchemeGroupVersion.Group))) + + if err := r.Apply(ctx, certApplyCfg, + k8sclient.ForceOwnership, k8sclient.FieldOwner(NodeCertificateControllerName)); err != nil { return err } - if update != controllerutil.OperationResultNone { - log.Info(fmt.Sprintf("Certificate %s %s", certName, update)) - } - + log.Info("Applied Certificate " + certName) return nil } @@ -166,12 +148,8 @@ func (r *NodeCertificateController) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, k8sclient.IgnoreNotFound(err) } - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - return r.ensureCertificate(ctx, node, node.Name) - }) - - if err != nil { - return ctrl.Result{}, fmt.Errorf("could not create certificate: %w", err) + if err := r.ensureCertificate(ctx, node, node.Name); err != nil { + return ctrl.Result{}, fmt.Errorf("could not apply certificate: %w", err) } return ctrl.Result{}, nil From 0816907778b4714fa8e72c795eb35b1c7851b562 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Wed, 6 May 2026 12:14:24 +0200 Subject: [PATCH 12/20] Refactor HypervisorMaintenanceController: pass statusCfg into sub-functions With SSA the status config is built once in Reconcile and passed to reconcileComputeService and reconcileEviction which mutate it directly. This eliminates the intermediate condition return values and makes the single-apply-per-reconcile explicit. The early-return guard for already-enabled/disabled state is inlined into each branch. --- .../hypervisor_maintenance_controller.go | 168 +++++++----------- 1 file changed, 67 insertions(+), 101 deletions(-) diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index 349d3ac..2e1a254 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -67,37 +67,16 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c return ctrl.Result{}, nil } - // Determine desired disabled condition and eviction state - disabledCond, evictingCond, evicted, err := hec.reconcileComputeService(ctx, hv) - if err != nil { - return ctrl.Result{}, err - } - - evictingCond, evicted, err = hec.reconcileEviction(ctx, hv, evictingCond, evicted) - if err != nil { - return ctrl.Result{}, err - } - - // Build status apply config: always include conditions this controller owns; - // omit ConditionTypeEvicting when it should be removed (SSA prunes it). - statusCfg := apiv1.HypervisorStatus().WithEvicted(evicted) + // Build status apply config upfront; sub-functions mutate it directly. + statusCfg := apiv1.HypervisorStatus().WithEvicted(hv.Status.Evicted) statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) - if disabledCond != nil { - utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *disabledCond) + if err := hec.reconcileComputeService(ctx, hv, statusCfg); err != nil { + return ctrl.Result{}, err } - if evictingCond != nil { - utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *evictingCond) - } else { - // Remove ConditionTypeEvicting by omitting it — SSA prunes sole-owned entries. - filtered := statusCfg.Conditions[:0] - for _, c := range statusCfg.Conditions { - if c.Type == nil || *c.Type != kvmv1.ConditionTypeEvicting { - filtered = append(filtered, c) - } - } - statusCfg.Conditions = filtered + if err := hec.reconcileEviction(ctx, hv, statusCfg); err != nil { + return ctrl.Result{}, err } return ctrl.Result{}, hec.Status().Apply(ctx, @@ -106,87 +85,64 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c } // reconcileComputeService enables/disables the nova-compute service based on -// hv.Spec.Maintenance. Returns the desired HypervisorDisabled condition (nil if -// service ID is unset) and the initial evicting/evicted state. -func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor) ( - disabledCond *k8sacmetav1.ConditionApplyConfiguration, - evictingCond *k8sacmetav1.ConditionApplyConfiguration, - evicted bool, - err error, -) { +// hv.Spec.Maintenance and sets the HypervisorDisabled condition on statusCfg. +func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor, statusCfg *apiv1.HypervisorStatusApplyConfiguration) error { log := logger.FromContext(ctx) serviceId := hv.Status.ServiceID if serviceId == "" { // We can only do something here, if there is a service to begin with. // The onboarding should take care of that. - return nil, nil, false, nil + return nil } switch hv.Spec.Maintenance { case kvmv1.MaintenanceUnset: - cond := k8sacmetav1.Condition(). - WithType(kvmv1.ConditionTypeHypervisorDisabled). - WithStatus(metav1.ConditionFalse). - WithMessage("Hypervisor is enabled"). - WithReason(kvmv1.ConditionReasonSucceeded) - existing := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) - if existing != nil && existing.Status == metav1.ConditionFalse { - // Already enabled, nothing to do - return cond, nil, false, nil - } - - // We need to enable the host as per spec and clear forced_down - // in case it was set by the HA service during maintenance. - falseVal := false - enableService := openstack.UpdateServiceOpts{ - Status: services.ServiceEnabled, - ForcedDown: &falseVal, - } - log.Info("Enabling hypervisor", "id", serviceId) - if _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract(); err != nil { - return nil, nil, false, fmt.Errorf("failed to enable hypervisor due to %w", err) + if existing == nil || existing.Status != metav1.ConditionFalse { + // We need to enable the host as per spec + enableService := services.UpdateOpts{Status: services.ServiceEnabled} + log.Info("Enabling hypervisor", "id", serviceId) + if _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract(); err != nil { + return fmt.Errorf("failed to enable hypervisor due to %w", err) + } } - return cond, nil, false, nil + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeHypervisorDisabled). + WithStatus(metav1.ConditionFalse). + WithMessage("Hypervisor is enabled"). + WithReason(kvmv1.ConditionReasonSucceeded)) case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceTermination: // Disable the compute service. - cond := k8sacmetav1.Condition(). - WithType(kvmv1.ConditionTypeHypervisorDisabled). - WithStatus(metav1.ConditionTrue). - WithMessage("Hypervisor is disabled"). - WithReason(kvmv1.ConditionReasonSucceeded) - existing := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) - if existing != nil && existing.Status == metav1.ConditionTrue { - // Already disabled, nothing to do - return cond, nil, false, nil - } - - disableService := services.UpdateOpts{ - Status: services.ServiceDisabled, - DisabledReason: "Hypervisor CRD: spec.maintenance=" + hv.Spec.Maintenance, - } - // We need to disable the host as per spec - log.Info("Disabling hypervisor", "id", serviceId) - if _, err := services.Update(ctx, hec.computeClient, serviceId, disableService).Extract(); err != nil { - return nil, nil, false, fmt.Errorf("failed to disable hypervisor due to %w", err) + if existing == nil || existing.Status != metav1.ConditionTrue { + disableService := services.UpdateOpts{ + Status: services.ServiceDisabled, + DisabledReason: "Hypervisor CRD: spec.maintenance=" + hv.Spec.Maintenance, + } + // We need to disable the host as per spec + log.Info("Disabling hypervisor", "id", serviceId) + if _, err := services.Update(ctx, hec.computeClient, serviceId, disableService).Extract(); err != nil { + return fmt.Errorf("failed to disable hypervisor due to %w", err) + } } - return cond, nil, false, nil + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeHypervisorDisabled). + WithStatus(metav1.ConditionTrue). + WithMessage("Hypervisor is disabled"). + WithReason(kvmv1.ConditionReasonSucceeded)) } - return nil, nil, false, nil + return nil } -// reconcileEviction creates/deletes the Eviction CR and returns the desired -// ConditionTypeEvicting apply configuration (nil means remove the condition). -func (hec *HypervisorMaintenanceController) reconcileEviction( - ctx context.Context, - hv *kvmv1.Hypervisor, - evictingCond *k8sacmetav1.ConditionApplyConfiguration, - evicted bool, -) (*k8sacmetav1.ConditionApplyConfiguration, bool, error) { +// reconcileEviction creates/deletes the Eviction CR and sets the ConditionTypeEvicting +// condition and Evicted scalar on statusCfg. When eviction should be removed, the +// condition entry is filtered out so SSA prunes it. +func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Context, hv *kvmv1.Hypervisor, statusCfg *apiv1.HypervisorStatusApplyConfiguration) error { eviction := &kvmv1.Eviction{ ObjectMeta: metav1.ObjectMeta{Name: hv.Name}, } @@ -195,45 +151,55 @@ func (hec *HypervisorMaintenanceController) reconcileEviction( case kvmv1.MaintenanceUnset: // Avoid deleting the eviction over and over. if !hv.Status.Evicted && meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting) == nil { - return nil, false, nil + return nil } - err := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)) - return nil, false, err // nil evictingCond → condition omitted from apply → SSA prunes it + if err := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)); err != nil { + return err + } + // Remove ConditionTypeEvicting by omitting it — SSA prunes sole-owned entries. + filtered := statusCfg.Conditions[:0] + for _, c := range statusCfg.Conditions { + if c.Type == nil || *c.Type != kvmv1.ConditionTypeEvicting { + filtered = append(filtered, c) + } + } + statusCfg.Conditions = filtered + statusCfg.WithEvicted(false) case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceTermination: // In case of "ha", the host gets emptied from the HA service if cond := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting); cond != nil { if cond.Reason == kvmv1.ConditionReasonSucceeded { // We are done here, no need to look at the eviction any more - return evictingCond, evicted, nil + return nil } } status, err := hec.ensureEviction(ctx, eviction, hv) if err != nil { - return evictingCond, evicted, err + return err } var reason, message string if status == metav1.ConditionFalse { message = "Evicted" reason = kvmv1.ConditionReasonSucceeded - evicted = true + statusCfg.WithEvicted(true) } else { message = "Evicting" reason = kvmv1.ConditionReasonRunning - evicted = false + statusCfg.WithEvicted(false) } - cond := k8sacmetav1.Condition(). - WithType(kvmv1.ConditionTypeEvicting). - WithStatus(status). - WithReason(reason). - WithMessage(message) - return cond, evicted, nil + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(status). + WithReason(reason). + WithMessage(message)) } - return evictingCond, evicted, nil + return nil } func (hec *HypervisorMaintenanceController) ensureEviction(ctx context.Context, eviction *kvmv1.Eviction, hypervisor *kvmv1.Hypervisor) (metav1.ConditionStatus, error) { From 0d19f14575b6699940677df9fba2d9ad9fead542 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Wed, 6 May 2026 12:14:24 +0200 Subject: [PATCH 13/20] Refactor OnboardingController: statusCfg passed through reconcile chain With SSA the status config is built once at the top of Reconcile and threaded through to all sub-functions (abortOnboarding, initialOnboarding, smokeTest, completeOnboarding) which mutate it directly. An apply closure is also passed so each function calls apply() when it has determined the desired condition, rather than building and applying the config itself. This removes the applyOnboardingCondition helper and makes the flow fully declarative: compute desired state, then apply once. --- internal/controller/onboarding_controller.go | 117 +++++++++---------- 1 file changed, 54 insertions(+), 63 deletions(-) diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index 574e1b2..15e3fb1 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -96,14 +96,27 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) computeHost := hv.Name + // Build the status apply config upfront; sub-functions mutate it. + // HypervisorID and ServiceID are always retained so they are never pruned. + statusCfg := apiv1.HypervisorStatus(). + WithHypervisorID(hv.Status.HypervisorID). + WithServiceID(hv.Status.ServiceID) + statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + + apply := func() error { + return r.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(OnboardingControllerName)) + } + // check if lifecycle management is enabled if !hv.Spec.LifecycleEnabled { - return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost, "Aborted due to LifecycleEnabled being false") + return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost, "Aborted due to LifecycleEnabled being false", statusCfg, apply) } // check if hv is terminating if hv.Spec.Maintenance == kvmv1.MaintenanceTermination { - return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost, "Aborted due to MaintenanceTermination") + return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost, "Aborted due to MaintenanceTermination", statusCfg, apply) } // We bail here out, because the openstack api is not the best to poll @@ -115,54 +128,36 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) } return ctrl.Result{}, err } - statusCfg := apiv1.HypervisorStatus(). - WithHypervisorID(hypervisorID). - WithServiceID(serviceID) - statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + statusCfg.WithHypervisorID(hypervisorID).WithServiceID(serviceID) utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(kvmv1.ConditionTypeOnboarding). WithStatus(metav1.ConditionTrue). WithReason(kvmv1.ConditionReasonInitial). WithMessage("Initial onboarding")) - return ctrl.Result{}, r.Status().Apply(ctx, - apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), - k8sclient.ForceOwnership, k8sclient.FieldOwner(OnboardingControllerName)) + return ctrl.Result{}, apply() } // check condition reason status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) switch status.Reason { case kvmv1.ConditionReasonInitial: - return ctrl.Result{}, r.initialOnboarding(ctx, hv) + return ctrl.Result{}, r.initialOnboarding(ctx, hv, statusCfg, apply) case kvmv1.ConditionReasonTesting: if hv.Spec.SkipTests { - return r.completeOnboarding(ctx, computeHost, hv) + return r.completeOnboarding(ctx, computeHost, hv, statusCfg, apply) } else { - return r.smokeTest(ctx, hv, computeHost) + return r.smokeTest(ctx, hv, computeHost, statusCfg, apply) } case kvmv1.ConditionReasonHandover: - return r.completeOnboarding(ctx, computeHost, hv) + return r.completeOnboarding(ctx, computeHost, hv, statusCfg, apply) default: // Nothing to be done return ctrl.Result{}, nil } } -// applyOnboardingCondition applies a single onboarding condition via SSA, -// carrying all existing conditions and scalar fields forward. -func (r *OnboardingController) applyOnboardingCondition(ctx context.Context, hv *kvmv1.Hypervisor, cond k8sacmetav1.ConditionApplyConfiguration) error { - statusCfg := apiv1.HypervisorStatus(). - WithHypervisorID(hv.Status.HypervisorID). - WithServiceID(hv.Status.ServiceID) - statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) - utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, cond) - return r.Status().Apply(ctx, - apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), - k8sclient.ForceOwnership, k8sclient.FieldOwner(OnboardingControllerName)) -} - -func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, computeHost, message string) error { +func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, computeHost, message string, statusCfg *apiv1.HypervisorStatusApplyConfiguration, apply func() error) error { status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) // Never onboarded if status == nil { @@ -177,23 +172,25 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy } if err := r.deleteTestServers(ctx, computeHost); err != nil { - return errors.Join(err, r.applyOnboardingCondition(ctx, hv, + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(kvmv1.ConditionTypeOnboarding). WithStatus(metav1.ConditionTrue). // No cleanup, so we are still "onboarding" WithReason(kvmv1.ConditionReasonAborted). - WithMessage(err.Error()))) + WithMessage(err.Error())) + return errors.Join(err, apply()) } - return r.applyOnboardingCondition(ctx, hv, + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(kvmv1.ConditionTypeOnboarding). WithStatus(metav1.ConditionFalse). WithReason(kvmv1.ConditionReasonAborted). WithMessage(message)) + return apply() } -func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1.Hypervisor) error { +func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, statusCfg *apiv1.HypervisorStatusApplyConfiguration, apply func() error) error { zone, found := hv.Labels[corev1.LabelTopologyZone] if !found || zone == "" { return fmt.Errorf("cannot find availability-zone label %v on node", corev1.LabelTopologyZone) @@ -221,15 +218,16 @@ func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1. return result.Err } - return r.applyOnboardingCondition(ctx, hv, + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(kvmv1.ConditionTypeOnboarding). WithStatus(metav1.ConditionTrue). WithReason(kvmv1.ConditionReasonTesting). WithMessage("Running onboarding tests")) + return apply() } -func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervisor, host string) (ctrl.Result, error) { +func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervisor, host string, statusCfg *apiv1.HypervisorStatusApplyConfiguration, apply func() error) (ctrl.Result, error) { log := logger.FromContext(ctx) zone := hv.Labels[corev1.LabelTopologyZone] server, err := r.createOrGetTestServer(ctx, zone, host, hv.UID) @@ -237,6 +235,16 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis return ctrl.Result{}, fmt.Errorf("failed to create or get test instance: %w", err) } + setTestingCondition := func(message string) error { + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage(message)) + return apply() + } + switch server.Status { case "ERROR": // servers.List doesn't provide the fault field, so fetch the server again @@ -248,12 +256,7 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis } // Set condition back to testing - if err = r.applyOnboardingCondition(ctx, hv, - *k8sacmetav1.Condition(). - WithType(kvmv1.ConditionTypeOnboarding). - WithStatus(metav1.ConditionTrue). - WithReason(kvmv1.ConditionReasonTesting). - WithMessage("Server ended up in error state: "+server.Fault.Message)); err != nil { + if err = setTestingCondition("Server ended up in error state: " + server.Fault.Message); err != nil { return ctrl.Result{}, err } @@ -269,12 +272,7 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis ShowConsoleOutput(ctx, r.testComputeClient, server.ID, servers.ShowConsoleOutputOpts{Length: 11}). Extract() if err != nil { - if err2 := r.applyOnboardingCondition(ctx, hv, - *k8sacmetav1.Condition(). - WithType(kvmv1.ConditionTypeOnboarding). - WithStatus(metav1.ConditionTrue). - WithReason(kvmv1.ConditionReasonTesting). - WithMessage(fmt.Sprintf("could not get console output %v", err))); err2 != nil { + if err2 := setTestingCondition(fmt.Sprintf("could not get console output %v", err)); err2 != nil { return ctrl.Result{}, err2 } return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil @@ -282,12 +280,7 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis if !strings.Contains(consoleOutput, server.Name) { if !server.LaunchedAt.IsZero() && r.Clock.Now().After(server.LaunchedAt.Add(smokeTestTimeout)) { - if err2 := r.applyOnboardingCondition(ctx, hv, - *k8sacmetav1.Condition(). - WithType(kvmv1.ConditionTypeOnboarding). - WithStatus(metav1.ConditionTrue). - WithReason(kvmv1.ConditionReasonTesting). - WithMessage(fmt.Sprintf("timeout waiting for console output since %v", server.LaunchedAt))); err2 != nil { + if err2 := setTestingCondition(fmt.Sprintf("timeout waiting for console output since %v", server.LaunchedAt)); err2 != nil { return ctrl.Result{}, err2 } if err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr(); err != nil { @@ -300,25 +293,20 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis } if err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr(); err != nil { - if err2 := r.applyOnboardingCondition(ctx, hv, - *k8sacmetav1.Condition(). - WithType(kvmv1.ConditionTypeOnboarding). - WithStatus(metav1.ConditionTrue). - WithReason(kvmv1.ConditionReasonTesting). - WithMessage(fmt.Sprintf("failed to terminate instance %v", err))); err2 != nil { + if err2 := setTestingCondition(fmt.Sprintf("failed to terminate instance %v", err)); err2 != nil { return ctrl.Result{}, err2 } return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } - return r.completeOnboarding(ctx, host, hv) + return r.completeOnboarding(ctx, host, hv, statusCfg, apply) default: return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } } -func (r *OnboardingController) completeOnboarding(ctx context.Context, host string, hv *kvmv1.Hypervisor) (ctrl.Result, error) { +func (r *OnboardingController) completeOnboarding(ctx context.Context, host string, hv *kvmv1.Hypervisor, statusCfg *apiv1.HypervisorStatusApplyConfiguration, apply func() error) (ctrl.Result, error) { onboardingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) if onboardingCondition != nil && onboardingCondition.Reason == kvmv1.ConditionReasonHandover { // We're waiting for aggregates and traits controllers to sync @@ -334,32 +322,35 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri return ctrl.Result{}, nil } - return ctrl.Result{}, r.applyOnboardingCondition(ctx, hv, + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(kvmv1.ConditionTypeOnboarding). WithStatus(metav1.ConditionFalse). WithReason(kvmv1.ConditionReasonSucceeded). WithMessage("Onboarding completed")) + return ctrl.Result{}, apply() } // First time in completeOnboarding - clean up and prepare for aggregate sync if err := r.deleteTestServers(ctx, host); err != nil { - return ctrl.Result{}, errors.Join(err, r.applyOnboardingCondition(ctx, hv, + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(kvmv1.ConditionTypeOnboarding). WithStatus(metav1.ConditionTrue). // No cleanup, so we are still "onboarding" WithReason(kvmv1.ConditionReasonAborted). - WithMessage(err.Error()))) + WithMessage(err.Error())) + return ctrl.Result{}, errors.Join(err, apply()) } // Mark onboarding as almost complete, triggers other controllers to do their part // Patch status to signal aggregates controller - return ctrl.Result{}, r.applyOnboardingCondition(ctx, hv, + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(kvmv1.ConditionTypeOnboarding). WithStatus(metav1.ConditionTrue). WithReason(kvmv1.ConditionReasonHandover). WithMessage("Waiting for other controllers to take over")) + return ctrl.Result{}, apply() } func (r *OnboardingController) deleteTestServers(ctx context.Context, host string) error { From ec266e52d4a2845e03c0cc1738cd41288d918657 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Wed, 8 Jul 2026 14:13:59 +0200 Subject: [PATCH 14/20] Fix conditions helpers and HypervisorController status apply issues conditions.go (cherry-pick from rework-onboarding): - ConditionsFromStatus: preserve ObservedGeneration; the taint controller's skip-check gates on it and it was silently dropped. - SetApplyConfigurationStatusCondition: guard against nil/empty Type to prevent a panic when a caller omits the Type field. - SetApplyConfigurationStatusCondition: propagate ObservedGeneration when updating an existing condition entry. hypervisor_controller.go: - Do not call WithInternalIP("") when the node has no NodeInternalIP address yet (e.g. during kubelet restart or bootstrap). WithInternalIP combined with ForceOwnership would overwrite a previously stored valid IP with an empty string. - Remove unnecessary metav1.ConditionStatus() conversion (unconvert lint). eviction_controller.go: - Remove extra blank line flagged by gofmt. Note: the 3 pre-existing test failures in hypervisor_maintenance_controller_test.go and hypervisor_controller_test.go are unrelated to these changes. --- internal/controller/eviction_controller.go | 1 - internal/controller/hypervisor_controller.go | 13 ++++++++--- internal/utils/conditions.go | 24 ++++++++++++++++++-- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/internal/controller/eviction_controller.go b/internal/controller/eviction_controller.go index fc66f09..11454d1 100644 --- a/internal/controller/eviction_controller.go +++ b/internal/controller/eviction_controller.go @@ -350,7 +350,6 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic if currentHypervisor != eviction.Spec.Hypervisor { log.Info("migrated") - // So, it is already off this one, do we need to verify it? if vm.Status == "VERIFY_RESIZE" { err := servers.ConfirmResize(ctx, r.computeClient, vm.ID).ExtractErr() diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index 2a583ca..23d5822 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -122,8 +122,15 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) // node condition or the node's deletion timestamp (gardener's future path) nodeTerminationCondition := FindNodeStatusCondition(node.Status.Conditions, "Terminating") - // Capture values to apply - only mutate fields this controller owns - statusCfg := apiv1.HypervisorStatus().WithInternalIP(newInternalIP) + // Capture values to apply - only mutate fields this controller owns. + // Only set InternalIP when non-empty: an empty string combined with + // ForceOwnership would overwrite a valid previously-stored IP during + // transient node states (kubelet restart, bootstrap) where no + // NodeInternalIP address is present yet. + statusCfg := apiv1.HypervisorStatus() + if newInternalIP != "" { + statusCfg.WithInternalIP(newInternalIP) + } statusCfg.Conditions = utils.ConditionsFromStatus(hypervisor.Status.Conditions) // Node might be terminating, propagate condition to hypervisor if (nodeTerminationCondition != nil && nodeTerminationCondition.Status == corev1.ConditionTrue) || @@ -149,7 +156,7 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(cond.Type). - WithStatus(metav1.ConditionStatus(cond.Status)). + WithStatus(cond.Status). WithReason(cond.Reason). WithMessage(cond.Message)) if cond.Status == metav1.ConditionFalse { diff --git a/internal/utils/conditions.go b/internal/utils/conditions.go index 2eef3a2..3955cfd 100644 --- a/internal/utils/conditions.go +++ b/internal/utils/conditions.go @@ -24,8 +24,8 @@ import ( // ConditionsFromStatus converts a []metav1.Condition (as stored in a status // subresource) into a []k8sacmetav1.ConditionApplyConfiguration suitable for -// use with server-side apply. All fields including LastTransitionTime are -// preserved verbatim. +// use with server-side apply. All fields including LastTransitionTime and +// ObservedGeneration are preserved verbatim. func ConditionsFromStatus(conditions []metav1.Condition) []k8sacmetav1.ConditionApplyConfiguration { result := make([]k8sacmetav1.ConditionApplyConfiguration, len(conditions)) for i := range conditions { @@ -36,6 +36,7 @@ func ConditionsFromStatus(conditions []metav1.Condition) []k8sacmetav1.Condition Reason: &c.Reason, Message: &c.Message, LastTransitionTime: &c.LastTransitionTime, + ObservedGeneration: &c.ObservedGeneration, } } return result @@ -55,6 +56,9 @@ func SetApplyConfigurationStatusCondition(conditions *[]k8sacmetav1.ConditionApp if conditions == nil { return false } + if newCondition.Type == nil || *newCondition.Type == "" { + return false + } // Find existing entry by type var existing *k8sacmetav1.ConditionApplyConfiguration @@ -99,6 +103,10 @@ func SetApplyConfigurationStatusCondition(conditions *[]k8sacmetav1.ConditionApp existing.Message = newCondition.Message changed = true } + if int64Changed(&existing.ObservedGeneration, newCondition.ObservedGeneration) { + existing.ObservedGeneration = newCondition.ObservedGeneration + changed = true + } return changed } @@ -114,3 +122,15 @@ func strChanged(a **string, b *string) bool { } return **a != *b } + +// int64Changed reports whether the pointer value of b differs from the current +// value at a. +func int64Changed(a **int64, b *int64) bool { + if *a == nil && b == nil { + return false + } + if *a == nil || b == nil { + return true + } + return **a != *b +} From 9aec81635902634be66bbea35253dc8fc7384145 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Wed, 8 Jul 2026 16:29:02 +0200 Subject: [PATCH 15/20] Fix SSA condition seeding: scope each controller to its own conditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With +listType=map / +listMapKey=type on HypervisorStatus.Conditions and EvictionStatus.Conditions, the API server merges the conditions list by the 'type' key on Apply. This means: - Conditions owned by other field managers are untouched (correct). - Conditions previously written by this field manager that are absent from the Apply payload are pruned (also correct — used to delete conditions). The previous ConditionsFromStatus helper round-tripped ALL conditions from status into the apply payload. This was wrong: it caused each controller to claim field ownership over conditions it does not own, interfering with other controllers on the next Apply. Replace ConditionsFromStatus with ConditionFromStatus (single condition), and update every controller to seed only the condition(s) it owns before calling SetApplyConfigurationStatusCondition. For controllers that need to delete a condition by omission (HypervisorMaintenanceController removing ConditionTypeEvicting on MaintenanceUnset), the owned conditions are seeded selectively so the target condition is simply absent from the payload. No behaviour change for the eviction_controller: it is the sole owner of all conditions on the Eviction CR, so seeding all three types is correct. --- internal/controller/aggregates_controller.go | 8 +++-- internal/controller/eviction_controller.go | 9 +++-- internal/controller/hypervisor_controller.go | 7 +++- .../hypervisor_maintenance_controller.go | 20 ++++++----- .../controller/hypervisor_taint_controller.go | 4 ++- internal/controller/offboarding_controller.go | 4 ++- internal/controller/onboarding_controller.go | 5 ++- internal/controller/ready/controller.go | 4 ++- internal/controller/traits_controller.go | 4 ++- internal/utils/conditions.go | 36 ++++++++++--------- 10 files changed, 65 insertions(+), 36 deletions(-) diff --git a/internal/controller/aggregates_controller.go b/internal/controller/aggregates_controller.go index 5f40a4f..9a36bf1 100644 --- a/internal/controller/aggregates_controller.go +++ b/internal/controller/aggregates_controller.go @@ -82,7 +82,9 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) if err != nil { // Set error condition, preserving current aggregate ownership statusCfg := apiv1.HypervisorStatus() - statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + 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). @@ -124,7 +126,9 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) } statusCfg := apiv1.HypervisorStatus() - statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + 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). diff --git a/internal/controller/eviction_controller.go b/internal/controller/eviction_controller.go index 11454d1..2664cc7 100644 --- a/internal/controller/eviction_controller.go +++ b/internal/controller/eviction_controller.go @@ -87,7 +87,7 @@ func moveToBack(instances []string) []string { } // evictionStatusCfg builds an EvictionStatusApplyConfiguration pre-populated -// with the current scalar fields and conditions from the fetched eviction. +// with the current scalar fields and the conditions this controller owns. // Call sites upsert the single condition they are setting via // utils.SetApplyConfigurationStatusCondition, then apply. func evictionStatusCfg(eviction *kvmv1.Eviction) *apiv1.EvictionStatusApplyConfiguration { @@ -95,7 +95,12 @@ func evictionStatusCfg(eviction *kvmv1.Eviction) *apiv1.EvictionStatusApplyConfi WithHypervisorServiceId(eviction.Status.HypervisorServiceId). WithOutstandingRamMb(eviction.Status.OutstandingRamMb) cfg.OutstandingInstances = eviction.Status.OutstandingInstances - cfg.Conditions = utils.ConditionsFromStatus(eviction.Status.Conditions) + // Seed all conditions this controller owns so they are not pruned on Apply. + for _, t := range []string{kvmv1.ConditionTypeEvicting, kvmv1.ConditionTypePreflight, kvmv1.ConditionTypeMigration} { + if c := meta.FindStatusCondition(eviction.Status.Conditions, t); c != nil { + cfg.WithConditions(utils.ConditionFromStatus(*c)) + } + } return cfg } diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index 23d5822..0e03d53 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -131,7 +131,12 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) if newInternalIP != "" { statusCfg.WithInternalIP(newInternalIP) } - statusCfg.Conditions = utils.ConditionsFromStatus(hypervisor.Status.Conditions) + // Seed the conditions this controller owns so they are not pruned. + for _, t := range []string{kvmv1.ConditionTypeTerminating, kvmv1.ConditionTypeAgentPodsEvicted} { + if c := meta.FindStatusCondition(hypervisor.Status.Conditions, t); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + } // Node might be terminating, propagate condition to hypervisor if (nodeTerminationCondition != nil && nodeTerminationCondition.Status == corev1.ConditionTrue) || !node.DeletionTimestamp.IsZero() { diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index 2e1a254..3486e97 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -68,8 +68,14 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c } // Build status apply config upfront; sub-functions mutate it directly. + // Seed only the HypervisorDisabled condition (the only one always retained). + // reconcileEviction conditionally seeds ConditionTypeEvicting: it is included + // when maintenance is active, and intentionally omitted when MaintenanceUnset + // so that SSA prunes it from the field manager's managed fields. statusCfg := apiv1.HypervisorStatus().WithEvicted(hv.Status.Evicted) - statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } if err := hec.reconcileComputeService(ctx, hv, statusCfg); err != nil { return ctrl.Result{}, err @@ -156,14 +162,8 @@ func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Contex if err := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)); err != nil { return err } - // Remove ConditionTypeEvicting by omitting it — SSA prunes sole-owned entries. - filtered := statusCfg.Conditions[:0] - for _, c := range statusCfg.Conditions { - if c.Type == nil || *c.Type != kvmv1.ConditionTypeEvicting { - filtered = append(filtered, c) - } - } - statusCfg.Conditions = filtered + // ConditionTypeEvicting is intentionally absent from statusCfg — SSA + // will prune it from this field manager's managed fields on Apply. statusCfg.WithEvicted(false) case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceTermination: @@ -173,6 +173,8 @@ func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Contex // We are done here, no need to look at the eviction any more return nil } + // Seed the existing evicting condition so it is not pruned + statusCfg.WithConditions(utils.ConditionFromStatus(*cond)) } status, err := hec.ensureEviction(ctx, eviction, hv) diff --git a/internal/controller/hypervisor_taint_controller.go b/internal/controller/hypervisor_taint_controller.go index 3fefe5c..3342427 100644 --- a/internal/controller/hypervisor_taint_controller.go +++ b/internal/controller/hypervisor_taint_controller.go @@ -74,7 +74,9 @@ func (r *HypervisorTaintController) Reconcile(ctx context.Context, req ctrl.Requ // Only set the Tainted condition this controller owns statusCfg := apiv1.HypervisorStatus() - statusCfg.Conditions = utils.ConditionsFromStatus(hypervisor.Status.Conditions) + if c := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTainted); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(kvmv1.ConditionTypeTainted). diff --git a/internal/controller/offboarding_controller.go b/internal/controller/offboarding_controller.go index 46860af..f649495 100644 --- a/internal/controller/offboarding_controller.go +++ b/internal/controller/offboarding_controller.go @@ -161,7 +161,9 @@ func (r *HypervisorOffboardingReconciler) Reconcile(ctx context.Context, req ctr func (r *HypervisorOffboardingReconciler) applyStatus(ctx context.Context, hv *kvmv1.Hypervisor, cond *k8sacmetav1.ConditionApplyConfiguration) error { statusCfg := apiv1.HypervisorStatus() - statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *cond) return r.Status().Apply(ctx, apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index 15e3fb1..a2bff44 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -101,7 +101,10 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) statusCfg := apiv1.HypervisorStatus(). WithHypervisorID(hv.Status.HypervisorID). WithServiceID(hv.Status.ServiceID) - statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + // Seed the Onboarding condition this controller owns so it is not pruned. + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } apply := func() error { return r.Status().Apply(ctx, diff --git a/internal/controller/ready/controller.go b/internal/controller/ready/controller.go index 29293ba..394bbd1 100644 --- a/internal/controller/ready/controller.go +++ b/internal/controller/ready/controller.go @@ -111,7 +111,9 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu log.Info("Updating Ready condition", "status", readyCondition.Status, "reason", readyCondition.Reason) statusCfg := apiv1.HypervisorStatus() - statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeReady); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(readyCondition.Type). diff --git a/internal/controller/traits_controller.go b/internal/controller/traits_controller.go index fff3aa8..5fb675f 100644 --- a/internal/controller/traits_controller.go +++ b/internal/controller/traits_controller.go @@ -142,7 +142,9 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct func (tc *TraitsController) applyTraitsStatus(ctx context.Context, hv *kvmv1.Hypervisor, traits []string, cond metav1.Condition) error { statusCfg := apiv1.HypervisorStatus() - statusCfg.Conditions = utils.ConditionsFromStatus(hv.Status.Conditions) + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *k8sacmetav1.Condition(). WithType(cond.Type). diff --git a/internal/utils/conditions.go b/internal/utils/conditions.go index 3955cfd..d4f4a89 100644 --- a/internal/utils/conditions.go +++ b/internal/utils/conditions.go @@ -22,24 +22,26 @@ import ( k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" ) -// ConditionsFromStatus converts a []metav1.Condition (as stored in a status -// subresource) into a []k8sacmetav1.ConditionApplyConfiguration suitable for -// use with server-side apply. All fields including LastTransitionTime and -// ObservedGeneration are preserved verbatim. -func ConditionsFromStatus(conditions []metav1.Condition) []k8sacmetav1.ConditionApplyConfiguration { - result := make([]k8sacmetav1.ConditionApplyConfiguration, len(conditions)) - for i := range conditions { - c := &conditions[i] - result[i] = k8sacmetav1.ConditionApplyConfiguration{ - Type: &c.Type, - Status: &c.Status, - Reason: &c.Reason, - Message: &c.Message, - LastTransitionTime: &c.LastTransitionTime, - ObservedGeneration: &c.ObservedGeneration, - } +// ConditionFromStatus converts a single metav1.Condition into a +// *k8sacmetav1.ConditionApplyConfiguration, preserving all fields verbatim +// (including LastTransitionTime and ObservedGeneration). +// +// Controllers must seed their apply-config payload with the conditions they +// own before mutating them. With +listType=map / +listMapKey=type on the CRD, +// SSA merges conditions by the "type" key: conditions owned by other field +// managers are left untouched, and conditions previously written by this +// field manager that are absent from the payload are pruned. Therefore each +// controller should include all conditions it has previously set — and only +// those — in every Apply call. +func ConditionFromStatus(c metav1.Condition) *k8sacmetav1.ConditionApplyConfiguration { + return &k8sacmetav1.ConditionApplyConfiguration{ + Type: &c.Type, + Status: &c.Status, + Reason: &c.Reason, + Message: &c.Message, + LastTransitionTime: &c.LastTransitionTime, + ObservedGeneration: &c.ObservedGeneration, } - return result } // SetApplyConfigurationStatusCondition sets the corresponding condition in From bc85729944725296f72af9016789e02f1c9bbc6c Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Thu, 9 Jul 2026 10:31:12 +0200 Subject: [PATCH 16/20] Adapt SSA call sites to controller-gen v0.21.0 signatures Cluster-scoped resources (Hypervisor, Eviction) now take only a name parameter in their apply configuration constructors. Remove the previously required but always-empty namespace argument. --- internal/controller/aggregates_controller.go | 4 ++-- internal/controller/eviction_controller.go | 20 +++++++++---------- internal/controller/hypervisor_controller.go | 2 +- .../hypervisor_maintenance_controller.go | 4 ++-- .../controller/hypervisor_taint_controller.go | 2 +- internal/controller/offboarding_controller.go | 2 +- internal/controller/onboarding_controller.go | 2 +- internal/controller/ready/controller.go | 2 +- internal/controller/traits_controller.go | 2 +- 9 files changed, 20 insertions(+), 20 deletions(-) diff --git a/internal/controller/aggregates_controller.go b/internal/controller/aggregates_controller.go index 9a36bf1..1f4b999 100644 --- a/internal/controller/aggregates_controller.go +++ b/internal/controller/aggregates_controller.go @@ -104,7 +104,7 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) } if err2 := ac.Status().Apply(ctx, - apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), k8sclient.ForceOwnership, k8sclient.FieldOwner(AggregatesControllerName)); err2 != nil { return ctrl.Result{}, errors.Join(err, err2) } @@ -149,7 +149,7 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) } if err := ac.Status().Apply(ctx, - apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), k8sclient.ForceOwnership, k8sclient.FieldOwner(AggregatesControllerName)); err != nil { return ctrl.Result{}, err } diff --git a/internal/controller/eviction_controller.go b/internal/controller/eviction_controller.go index 2664cc7..acb5c95 100644 --- a/internal/controller/eviction_controller.go +++ b/internal/controller/eviction_controller.go @@ -145,7 +145,7 @@ func (r *EvictionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c WithReason(kvmv1.ConditionReasonRunning). WithMessage("Running")) return ctrl.Result{}, r.Status().Apply(ctx, - apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } @@ -188,7 +188,7 @@ func (r *EvictionReconciler) handleRunning(ctx context.Context, eviction *kvmv1. WithReason(kvmv1.ConditionReasonSucceeded). WithMessage("eviction completed successfully")) return ctrl.Result{}, r.Status().Apply(ctx, - apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } @@ -209,7 +209,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv WithReason(kvmv1.ConditionReasonFailed). WithMessage("hypervisor not disabled")) if err := r.Status().Apply(ctx, - apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), client.ForceOwnership, client.FieldOwner(EvictionControllerName)); err != nil { return ctrl.Result{}, err } @@ -235,7 +235,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv WithReason(kvmv1.ConditionReasonFailed). WithMessage(fmt.Sprintf("failed to get hypervisor %v", err))) return ctrl.Result{}, r.Status().Apply(ctx, - apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } @@ -252,7 +252,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv WithReason(kvmv1.ConditionReasonSucceeded). WithMessage(msg)) return ctrl.Result{}, r.Status().Apply(ctx, - apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } @@ -275,7 +275,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv WithReason(kvmv1.ConditionReasonSucceeded). WithMessage("Preflight checks passed, hypervisor is disabled and ready for eviction")) return ctrl.Result{}, r.Status().Apply(ctx, - apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } @@ -299,7 +299,7 @@ func (r *EvictionReconciler) handleNotFound(ctx context.Context, eviction *kvmv1 WithReason(kvmv1.ConditionReasonSucceeded). WithMessage(fmt.Sprintf("Instance %s is gone", uuid))) return r.Status().Apply(ctx, - apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } @@ -345,7 +345,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic WithReason(kvmv1.ConditionReasonFailed). WithMessage(fmt.Sprintf("Migration of instance %s failed: %s", vm.ID, vm.Fault.Message))) applyErr := r.Status().Apply(ctx, - apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), client.ForceOwnership, client.FieldOwner(EvictionControllerName)) return ctrl.Result{}, errors.Join(fmt.Errorf("error migrating instance %v", uuid), applyErr) } @@ -391,7 +391,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic WithMessage(fmt.Sprintf("Migration of instance %s finished", vm.ID))) } return ctrl.Result{}, r.Status().Apply(ctx, - apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), client.ForceOwnership, client.FieldOwner(EvictionControllerName)) } @@ -408,7 +408,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic WithReason(kvmv1.ConditionReasonFailed). WithMessage(fmt.Sprintf("Live migration of terminating instance %s skipped", vm.ID))) if err := r.Status().Apply(ctx, - apiv1.Eviction(eviction.Name, eviction.Namespace).WithStatus(statusCfg), + apiv1.Eviction(eviction.Name).WithStatus(statusCfg), client.ForceOwnership, client.FieldOwner(EvictionControllerName)); err != nil { return ctrl.Result{}, fmt.Errorf("could not update status due to %w", err) } diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index 0e03d53..b6565a7 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -171,7 +171,7 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) } if err := hv.Status().Apply(ctx, - apiv1.Hypervisor(hypervisor.Name, "").WithStatus(statusCfg), + apiv1.Hypervisor(hypervisor.Name).WithStatus(statusCfg), k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorControllerName)); err != nil { return ctrl.Result{}, err } diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index 3486e97..d74cc02 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -86,7 +86,7 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c } return ctrl.Result{}, hec.Status().Apply(ctx, - apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorMaintenanceControllerName)) } @@ -223,7 +223,7 @@ func (hec *HypervisorMaintenanceController) ensureEviction(ctx context.Context, WithController(true). WithBlockOwnerDeletion(true) - evictionApplyCfg := apiv1.Eviction(eviction.Name, eviction.Namespace). + evictionApplyCfg := apiv1.Eviction(eviction.Name). WithLabels(evictionLabels). WithOwnerReferences(ownerRef). WithSpec(apiv1.EvictionSpec(). diff --git a/internal/controller/hypervisor_taint_controller.go b/internal/controller/hypervisor_taint_controller.go index 3342427..d15fc6e 100644 --- a/internal/controller/hypervisor_taint_controller.go +++ b/internal/controller/hypervisor_taint_controller.go @@ -86,7 +86,7 @@ func (r *HypervisorTaintController) Reconcile(ctx context.Context, req ctrl.Requ WithObservedGeneration(hypervisor.Generation)) return ctrl.Result{}, r.Status().Apply(ctx, - apiv1.Hypervisor(hypervisor.Name, "").WithStatus(statusCfg), + apiv1.Hypervisor(hypervisor.Name).WithStatus(statusCfg), k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorTaintControllerName)) } diff --git a/internal/controller/offboarding_controller.go b/internal/controller/offboarding_controller.go index f649495..776aa93 100644 --- a/internal/controller/offboarding_controller.go +++ b/internal/controller/offboarding_controller.go @@ -166,7 +166,7 @@ func (r *HypervisorOffboardingReconciler) applyStatus(ctx context.Context, hv *k } utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, *cond) return r.Status().Apply(ctx, - apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), k8sclient.ForceOwnership, k8sclient.FieldOwner(OffboardingControllerName)) } diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index a2bff44..b238a51 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -108,7 +108,7 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) apply := func() error { return r.Status().Apply(ctx, - apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), k8sclient.ForceOwnership, k8sclient.FieldOwner(OnboardingControllerName)) } diff --git a/internal/controller/ready/controller.go b/internal/controller/ready/controller.go index 394bbd1..5ef66fb 100644 --- a/internal/controller/ready/controller.go +++ b/internal/controller/ready/controller.go @@ -121,7 +121,7 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu WithReason(readyCondition.Reason). WithMessage(readyCondition.Message)) return ctrl.Result{}, r.Status().Apply(ctx, - apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), k8sclient.ForceOwnership, k8sclient.FieldOwner(ControllerName)) } diff --git a/internal/controller/traits_controller.go b/internal/controller/traits_controller.go index 5fb675f..40c84d0 100644 --- a/internal/controller/traits_controller.go +++ b/internal/controller/traits_controller.go @@ -155,7 +155,7 @@ func (tc *TraitsController) applyTraitsStatus(ctx context.Context, hv *kvmv1.Hyp statusCfg.WithTraits(traits...) } return tc.Status().Apply(ctx, - apiv1.Hypervisor(hv.Name, "").WithStatus(statusCfg), + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), k8sclient.ForceOwnership, k8sclient.FieldOwner(TraitsControllerName)) } From 6d0a83aefed6f187056ab4ffd90819f63272cc42 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Thu, 9 Jul 2026 13:03:06 +0200 Subject: [PATCH 17/20] Fix label propagation and forced_down clearing on service enable HypervisorController: the AgentPodsEvicted=False branch was returning early with a requeue before running syncLabelsAndAnnotations, so node label changes were not propagated to the Hypervisor while terminating. Move the requeue after the spec patch so labels stay in sync. HypervisorMaintenanceController: when enabling a compute service, also clear the forced_down flag. Otherwise, an earlier HA event that set forced_down=true would leave the service reachable but treated as down. --- internal/controller/hypervisor_controller.go | 14 ++++++-------- .../hypervisor_maintenance_controller.go | 9 +++++++-- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index b6565a7..c673092 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -176,10 +176,6 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } - if statusRequeueAfter > 0 { - return ctrl.Result{RequeueAfter: statusRequeueAfter}, nil - } - // Re-fetch after status apply so the spec patch has a fresh resourceVersion if err := hv.Get(ctx, k8sclient.ObjectKeyFromObject(hypervisor), hypervisor); err != nil { return ctrl.Result{}, fmt.Errorf("failed to re-fetch hypervisor after status apply: %w", err) @@ -192,12 +188,14 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) // Hypervisor spec/labels go stale. base := hypervisor.DeepCopy() syncLabelsAndAnnotations(nodeLabels, hypervisor, node) - if equality.Semantic.DeepEqual(hypervisor, base) { - return ctrl.Result{}, nil + if !equality.Semantic.DeepEqual(hypervisor, base) { + if err := hv.Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorControllerName)); err != nil { + return ctrl.Result{}, err + } } - return ctrl.Result{}, hv.Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(base, - k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorControllerName)) + return ctrl.Result{RequeueAfter: statusRequeueAfter}, nil } syncLabelsAndAnnotations(nodeLabels, hypervisor, node) diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index d74cc02..ce3463a 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -106,8 +106,13 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context. case kvmv1.MaintenanceUnset: existing := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) if existing == nil || existing.Status != metav1.ConditionFalse { - // We need to enable the host as per spec - enableService := services.UpdateOpts{Status: services.ServiceEnabled} + // We need to enable the host as per spec. + // Also clear forced_down in case a previous HA event set it. + falseVal := false + enableService := openstack.UpdateServiceOpts{ + Status: services.ServiceEnabled, + ForcedDown: &falseVal, + } log.Info("Enabling hypervisor", "id", serviceId) if _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract(); err != nil { return fmt.Errorf("failed to enable hypervisor due to %w", err) From df5d971b2685da315e37d3af3a27d0acc573d601 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Thu, 9 Jul 2026 20:29:09 +0200 Subject: [PATCH 18/20] HypervisorMaintenanceController: delegate SetupWithManager to registerWithManager SetupWithManager was inlining the same ctrl.NewControllerManagedBy call that registerWithManager already provides, leaving registerWithManager unreachable and triggering the unused lint error. --- internal/controller/hypervisor_maintenance_controller.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index ce3463a..1d0da1a 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -273,9 +273,5 @@ func (hec *HypervisorMaintenanceController) SetupWithManager(mgr ctrl.Manager) e } hec.computeClient.Microversion = "2.90" // Xena (or later) - return ctrl.NewControllerManagedBy(mgr). - Named(HypervisorMaintenanceControllerName). - For(&kvmv1.Hypervisor{}). - Owns(&kvmv1.Eviction{}). - Complete(hec) + return hec.registerWithManager(mgr) } From 3182f107f09ac22ce9cb0f8f860eca87a50b57a2 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 19 May 2026 13:44:27 +0200 Subject: [PATCH 19/20] Onboarding: keep retrying after instance creation/error failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the test instance creation in the onboarding smoke test fails, the controller used to return a bare error from Reconcile and never updated the Onboarding condition. Operators saw a stale message frozen for days while the loop kept silently retrying — combined with the SSA condition helper preserving LastTransitionTime when Status does not change, the resource looked stuck even though reconciliation was healthy. This change: - In smokeTest, when createOrGetTestServer fails, set the Onboarding condition to Testing with the current error message and requeue with defaultWaitTime instead of returning the error. This makes each retry observable in the resource itself and matches the style used by every other smokeTest error path. - Include the failed server's UUID in the ERROR-state, console-output-failure, timeout, and terminate-failure messages. A fresh attempt allocates a new instance UUID, so each retry yields a different message even when Nova keeps returning the same fault text preventing SetApplyConfigurationStatusCondition from treating successive failures as no-op updates. - Tests: add Ginkgo specs covering POST /servers returning 500 and a server in ERROR state, asserting the condition message, the requeue, the absence of a returned error, and (for ERROR) that the delete is issued and the UUID is in the message. --- internal/controller/onboarding_controller.go | 65 ++++++++-- .../controller/onboarding_controller_test.go | 121 +++++++++++++++++- 2 files changed, 172 insertions(+), 14 deletions(-) diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index b238a51..4d67133 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -235,17 +235,19 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis zone := hv.Labels[corev1.LabelTopologyZone] server, err := r.createOrGetTestServer(ctx, zone, host, hv.UID) if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to create or get test instance: %w", err) - } - - setTestingCondition := func(message string) error { - utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + // Surface the failure in the Onboarding condition so the user can see + // the latest error and confirm the controller is still retrying. + // Returning a bare error here would leave the previously-set message + // in place and the resource looks frozen even though we keep retrying. + if err2 := r.applyOnboardingCondition(ctx, hv, *k8sacmetav1.Condition(). WithType(kvmv1.ConditionTypeOnboarding). WithStatus(metav1.ConditionTrue). WithReason(kvmv1.ConditionReasonTesting). - WithMessage(message)) - return apply() + WithMessage(fmt.Sprintf("failed to create or get test instance: %v", err))); err2 != nil { + return ctrl.Result{}, err2 + } + return ctrl.Result{RequeueAfter: defaultWaitTime}, nil } switch server.Status { @@ -258,8 +260,16 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil } - // Set condition back to testing - if err = setTestingCondition("Server ended up in error state: " + server.Fault.Message); err != nil { + // Set condition back to testing. Include the server ID so each retry + // (which creates a fresh instance with a new UUID) yields a different + // message — otherwise SetApplyConfigurationStatusCondition would treat + // the update as a no-op when Nova keeps reporting the same fault text. + if err = r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage(fmt.Sprintf("Server %s ended up in error state: %s", id, server.Fault.Message))); err != nil { return ctrl.Result{}, err } @@ -275,7 +285,12 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis ShowConsoleOutput(ctx, r.testComputeClient, server.ID, servers.ShowConsoleOutputOpts{Length: 11}). Extract() if err != nil { - if err2 := setTestingCondition(fmt.Sprintf("could not get console output %v", err)); err2 != nil { + if err2 := r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage(fmt.Sprintf("could not get console output for server %s: %v", server.ID, err))); err2 != nil { return ctrl.Result{}, err2 } return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil @@ -283,7 +298,12 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis if !strings.Contains(consoleOutput, server.Name) { if !server.LaunchedAt.IsZero() && r.Clock.Now().After(server.LaunchedAt.Add(smokeTestTimeout)) { - if err2 := setTestingCondition(fmt.Sprintf("timeout waiting for console output since %v", server.LaunchedAt)); err2 != nil { + if err2 := r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage(fmt.Sprintf("timeout waiting for console output of server %s since %v", server.ID, server.LaunchedAt))); err2 != nil { return ctrl.Result{}, err2 } if err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr(); err != nil { @@ -296,7 +316,12 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis } if err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr(); err != nil { - if err2 := setTestingCondition(fmt.Sprintf("failed to terminate instance %v", err)); err2 != nil { + if err2 := r.applyOnboardingCondition(ctx, hv, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeOnboarding). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonTesting). + WithMessage(fmt.Sprintf("failed to terminate instance %s: %v", server.ID, err))); err2 != nil { return ctrl.Result{}, err2 } return ctrl.Result{RequeueAfter: r.getRequeueInterval()}, nil @@ -356,6 +381,22 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri return ctrl.Result{}, apply() } +// applyOnboardingCondition applies a single Onboarding condition via SSA. +// It seeds the previously-stored Onboarding condition so it is not pruned, +// then upserts the new value preserving LastTransitionTime when Status is unchanged. +func (r *OnboardingController) applyOnboardingCondition(ctx context.Context, hv *kvmv1.Hypervisor, cond k8sacmetav1.ConditionApplyConfiguration) error { + statusCfg := apiv1.HypervisorStatus(). + WithHypervisorID(hv.Status.HypervisorID). + WithServiceID(hv.Status.ServiceID) + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) + } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, cond) + return r.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(OnboardingControllerName)) +} + func (r *OnboardingController) deleteTestServers(ctx context.Context, host string) error { log := logger.FromContext(ctx) serverPrefix := fmt.Sprintf("%v-%v", testPrefixName, host) diff --git a/internal/controller/onboarding_controller_test.go b/internal/controller/onboarding_controller_test.go index 8c0003d..3afd612 100644 --- a/internal/controller/onboarding_controller_test.go +++ b/internal/controller/onboarding_controller_test.go @@ -336,6 +336,7 @@ var _ = Describe("Onboarding Controller", func() { Context("running tests after initial setup", func() { var ( serverActionHandler func(http.ResponseWriter, *http.Request) + serverCreateHandler func(http.ResponseWriter, *http.Request) serverDeleteHandler func(http.ResponseWriter, *http.Request) serverDetailHandler func(http.ResponseWriter, *http.Request) ) @@ -397,11 +398,14 @@ var _ = Describe("Onboarding Controller", func() { Expect(err).NotTo(HaveOccurred()) }) - fakeServer.Mux.HandleFunc("POST /servers", func(w http.ResponseWriter, r *http.Request) { + serverCreateHandler = func(w http.ResponseWriter, _ *http.Request) { w.Header().Add("Content-Type", "application/json") w.WriteHeader(http.StatusOK) _, err := fmt.Fprint(w, createServerBody) Expect(err).NotTo(HaveOccurred()) + } + fakeServer.Mux.HandleFunc("POST /servers", func(w http.ResponseWriter, r *http.Request) { + serverCreateHandler(w, r) }) serverActionHandler = func(w http.ResponseWriter, _ *http.Request) { @@ -632,13 +636,126 @@ var _ = Describe("Onboarding Controller", func() { By("Verifying the timed-out server was deleted") Expect(serverDeletedCalled).To(BeTrue()) - By("Verifying the onboarding condition message indicates a timeout") + By("Verifying the onboarding condition message indicates a timeout and includes the server ID") hv := &kvmv1.Hypervisor{} Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) onboardingCond := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) Expect(onboardingCond).NotTo(BeNil()) Expect(onboardingCond.Reason).To(Equal(kvmv1.ConditionReasonTesting)) Expect(onboardingCond.Message).To(ContainSubstring("timeout")) + Expect(onboardingCond.Message).To(ContainSubstring("server-id")) + }) + }) + + When("test server creation fails", func() { + BeforeEach(func(ctx SpecContext) { + By("Overriding HV status to Testing state") + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonTesting, + Message: "previously stuck on something else", + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + }) + + It("should surface the error in the Onboarding condition and requeue without returning an error", func(ctx SpecContext) { + By("Overriding the POST /servers handler to return 500") + serverCreateHandler = func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, err := fmt.Fprint(w, `{"computeFault": {"message": "Cinder volume backend is degraded"}}`) + Expect(err).NotTo(HaveOccurred()) + } + + By("Reconciling: createOrGetTestServer should fail at POST /servers") + result, err := onboardingReconciler.Reconcile(ctx, reconcileReq) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(defaultWaitTime)) + + By("Verifying the Onboarding condition reflects the create error") + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + onboardingCond := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) + Expect(onboardingCond).NotTo(BeNil()) + Expect(onboardingCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(onboardingCond.Reason).To(Equal(kvmv1.ConditionReasonTesting)) + Expect(onboardingCond.Message).To(ContainSubstring("failed to create or get test instance")) + }) + }) + + When("test server is in ERROR state", func() { + var ( + serverGetCalled bool + serverDeleteCalled bool + ) + + BeforeEach(func(ctx SpecContext) { + By("Overriding HV status to Testing state") + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonTesting, + Message: "Running onboarding tests", + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + + serverName := fmt.Sprintf("%s-%s-%s", testPrefixName, hypervisorName, string(hv.UID)) + serverGetCalled = false + serverDeleteCalled = false + + // GET /servers/detail returns a single ERROR-state server with the + // expected name so createOrGetTestServer returns it as foundServer. + serverDetailHandler = func(w http.ResponseWriter, _ *http.Request) { + _, err := fmt.Fprintf(w, + `{"servers": [{"id": "server-id", "name": %q, "status": "ERROR"}], "servers_links": []}`, + serverName) + Expect(err).NotTo(HaveOccurred()) + } + + // GET /servers/server-id returns the ERROR server with a fault + // message so smokeTest can record it. + fakeServer.Mux.HandleFunc("GET /servers/server-id", func(w http.ResponseWriter, _ *http.Request) { + serverGetCalled = true + w.Header().Add("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := fmt.Fprintf(w, + `{"server": {"id": "server-id", "name": %q, "status": "ERROR", "fault": {"message": "Build of instance aborted: volume creation failed"}}}`, + serverName) + Expect(err).NotTo(HaveOccurred()) + }) + + serverDeleteHandler = func(w http.ResponseWriter, _ *http.Request) { + serverDeleteCalled = true + w.WriteHeader(http.StatusAccepted) + } + }) + + It("should record the failure with the server UUID and issue a delete, without returning an error", func(ctx SpecContext) { + By("Reconciling once to enter the ERROR branch") + result, err := onboardingReconciler.Reconcile(ctx, reconcileReq) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(defaultWaitTime)) + + By("Verifying GET on the server happened so the fault could be read") + Expect(serverGetCalled).To(BeTrue()) + + By("Verifying the server delete was attempted") + Expect(serverDeleteCalled).To(BeTrue()) + + By("Verifying the Onboarding condition message includes the server UUID and the fault text") + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + onboardingCond := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) + Expect(onboardingCond).NotTo(BeNil()) + Expect(onboardingCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(onboardingCond.Reason).To(Equal(kvmv1.ConditionReasonTesting)) + Expect(onboardingCond.Message).To(ContainSubstring("server-id")) + Expect(onboardingCond.Message).To(ContainSubstring("ended up in error state")) + Expect(onboardingCond.Message).To(ContainSubstring("Build of instance aborted")) }) }) From 95fad4d8aa7caba21dd662c5c3d047c4eea603d0 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Thu, 9 Jul 2026 14:27:44 +0200 Subject: [PATCH 20/20] Add unit tests for conditions helpers Covers ConditionFromStatus (including ObservedGeneration preservation) and SetApplyConfigurationStatusCondition (append, update, status-change, LastTransitionTime preservation, no-op detection, multi-condition isolation). --- internal/utils/conditions_test.go | 138 ++++++++++++++++++++++++++++++ internal/utils/suite_test.go | 30 +++++++ 2 files changed, 168 insertions(+) create mode 100644 internal/utils/conditions_test.go create mode 100644 internal/utils/suite_test.go diff --git a/internal/utils/conditions_test.go b/internal/utils/conditions_test.go new file mode 100644 index 0000000..8d43476 --- /dev/null +++ b/internal/utils/conditions_test.go @@ -0,0 +1,138 @@ +/* +SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sacmetav1 "k8s.io/client-go/applyconfigurations/meta/v1" + + "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" +) + +var _ = Describe("ConditionFromStatus", func() { + It("preserves all fields verbatim, including ObservedGeneration", func() { + ts := metav1.Now() + c := metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionTrue, + Reason: "AllGood", + Message: "everything is fine", + LastTransitionTime: ts, + ObservedGeneration: 42, + } + got := utils.ConditionFromStatus(c) + Expect(got.Type).NotTo(BeNil()) + Expect(*got.Type).To(Equal("Ready")) + Expect(*got.Status).To(Equal(metav1.ConditionTrue)) + Expect(*got.Reason).To(Equal("AllGood")) + Expect(*got.Message).To(Equal("everything is fine")) + Expect(*got.LastTransitionTime).To(Equal(ts)) + Expect(got.ObservedGeneration).NotTo(BeNil(), "ObservedGeneration must be copied") + Expect(*got.ObservedGeneration).To(Equal(int64(42))) + }) +}) + +var _ = Describe("SetApplyConfigurationStatusCondition", func() { + makeCondition := func(condType string, status metav1.ConditionStatus, reason, message string) k8sacmetav1.ConditionApplyConfiguration { + return *k8sacmetav1.Condition(). + WithType(condType). + WithStatus(status). + WithReason(reason). + WithMessage(message) + } + + It("appends a new condition and sets LastTransitionTime", func() { + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + changed := utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Ready", metav1.ConditionTrue, "AllGood", "ok")) + Expect(changed).To(BeTrue()) + Expect(conditions).To(HaveLen(1)) + Expect(*conditions[0].Type).To(Equal("Ready")) + Expect(conditions[0].LastTransitionTime).NotTo(BeNil(), "LastTransitionTime must be set on append") + }) + + It("returns false and does not modify for nil conditions slice", func() { + changed := utils.SetApplyConfigurationStatusCondition(nil, makeCondition("Ready", metav1.ConditionTrue, "R", "m")) + Expect(changed).To(BeFalse()) + }) + + It("returns false for a condition with nil Type", func() { + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + blank := k8sacmetav1.ConditionApplyConfiguration{} + changed := utils.SetApplyConfigurationStatusCondition(&conditions, blank) + Expect(changed).To(BeFalse()) + Expect(conditions).To(BeEmpty()) + }) + + It("updates fields and marks changed when status changes", func() { + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Ready", metav1.ConditionFalse, "NotReady", "broken")) + + changed := utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Ready", metav1.ConditionTrue, "AllGood", "fixed")) + Expect(changed).To(BeTrue()) + Expect(*conditions[0].Status).To(Equal(metav1.ConditionTrue)) + Expect(*conditions[0].Reason).To(Equal("AllGood")) + Expect(*conditions[0].Message).To(Equal("fixed")) + }) + + It("preserves LastTransitionTime when status is unchanged", func() { + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Ready", metav1.ConditionTrue, "R", "m")) + original := *conditions[0].LastTransitionTime + + // Same status — only message changes. + changed := utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Ready", metav1.ConditionTrue, "R", "updated message")) + Expect(changed).To(BeTrue(), "message change is still a change") + Expect(*conditions[0].LastTransitionTime).To(Equal(original), "LastTransitionTime must not advance when status is unchanged") + }) + + It("returns false when nothing changes", func() { + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Ready", metav1.ConditionTrue, "R", "m")) + + changed := utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Ready", metav1.ConditionTrue, "R", "m")) + Expect(changed).To(BeFalse()) + }) + + It("updates ObservedGeneration independently of status", func() { + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Ready", metav1.ConditionTrue, "R", "m")) + + next := makeCondition("Ready", metav1.ConditionTrue, "R", "m") + gen := int64(7) + next.ObservedGeneration = &gen + changed := utils.SetApplyConfigurationStatusCondition(&conditions, next) + Expect(changed).To(BeTrue()) + Expect(conditions[0].ObservedGeneration).NotTo(BeNil()) + Expect(*conditions[0].ObservedGeneration).To(Equal(int64(7))) + }) + + It("manages multiple conditions independently", func() { + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Ready", metav1.ConditionTrue, "R", "m")) + utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Onboarding", metav1.ConditionFalse, "Initial", "pending")) + Expect(conditions).To(HaveLen(2)) + + utils.SetApplyConfigurationStatusCondition(&conditions, makeCondition("Ready", metav1.ConditionFalse, "Degraded", "issue")) + Expect(conditions).To(HaveLen(2)) + Expect(*conditions[0].Status).To(Equal(metav1.ConditionFalse)) + Expect(*conditions[1].Status).To(Equal(metav1.ConditionFalse)) + }) +}) diff --git a/internal/utils/suite_test.go b/internal/utils/suite_test.go new file mode 100644 index 0000000..b2476dc --- /dev/null +++ b/internal/utils/suite_test.go @@ -0,0 +1,30 @@ +/* +SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestUtils(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Utils Suite") +}