diff --git a/internal/controller/eviction_controller.go b/internal/controller/eviction_controller.go index b0fa33c9..b1ecd78e 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,32 @@ func moveToBack(instances []string) []string { return instances } +// evictionStatusCfg builds an EvictionStatusApplyConfiguration pre-populated +// 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 { + cfg := apiv1.EvictionStatus(). + WithHypervisorServiceId(eviction.Status.HypervisorServiceId). + WithOutstandingRamMb(eviction.Status.OutstandingRamMb) + cfg.OutstandingInstances = eviction.Status.OutstandingInstances + // 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 +} + +// applyStatus applies the eviction status via SSA with this controller as the +// field owner. +func (r *EvictionReconciler) applyStatus(ctx context.Context, name string, cfg *apiv1.EvictionStatusApplyConfiguration) error { + return r.Status().Apply(ctx, + apiv1.Eviction(name).WithStatus(cfg), + client.ForceOwnership, client.FieldOwner(EvictionControllerName)) +} + // +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 +145,14 @@ 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.applyStatus(ctx, eviction.Name, statusCfg) } switch statusCondition.Status { @@ -157,36 +184,16 @@ 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.applyStatus(ctx, eviction.Name, statusCfg) } func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv1.Eviction, hv *kvmv1.Hypervisor) (ctrl.Result, error) { @@ -194,14 +201,20 @@ 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.applyStatus(ctx, eviction.Name, statusCfg); err != nil { + return ctrl.Result{}, err + } } return ctrl.Result{RequeueAfter: defaultPollTime}, nil // Wait for hypervisor to be disabled } @@ -216,47 +229,50 @@ 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.applyStatus(ctx, eviction.Name, statusCfg) } + + // 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.applyStatus(ctx, eviction.Name, statusCfg) } + 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.applyStatus(ctx, eviction.Name, statusCfg) } // Tries to handle the NotFound-error by updating the status @@ -265,18 +281,20 @@ 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.applyStatus(ctx, eviction.Name, statusCfg) } func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Eviction) (ctrl.Result, error) { @@ -309,41 +327,25 @@ 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.applyStatus(ctx, eviction.Name, statusCfg) + 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 +360,50 @@ 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.applyStatus(ctx, eviction.Name, statusCfg) } 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.applyStatus(ctx, eviction.Name, statusCfg); 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 +411,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 +425,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 +434,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 }