diff --git a/internal/controller/aggregates_controller.go b/internal/controller/aggregates_controller.go index 50185b7..8eb0479 100644 --- a/internal/controller/aggregates_controller.go +++ b/internal/controller/aggregates_controller.go @@ -114,6 +114,7 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } + base := hv.DeepCopy() hv.Status.Aggregates = hv.Spec.Aggregates meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeAggregatesUpdated, @@ -121,7 +122,8 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request) Reason: kvmv1.ConditionReasonSucceeded, Message: "Aggregates updated successfully", }) - return ctrl.Result{}, ac.Status().Update(ctx, hv) + return ctrl.Result{}, ac.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(AggregatesControllerName)) } // setErrorCondition sets the error condition on the Hypervisor status, returns error if update fails @@ -133,8 +135,10 @@ func (ac *AggregatesController) setErrorCondition(ctx context.Context, hv *kvmv1 Message: msg, } + base := hv.DeepCopy() if meta.SetStatusCondition(&hv.Status.Conditions, condition) { - if err := ac.Status().Update(ctx, hv); err != nil { + if err := ac.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(AggregatesControllerName)); err != nil { return err } } diff --git a/internal/controller/decomission_controller.go b/internal/controller/decomission_controller.go index 543f72b..db28e5b 100644 --- a/internal/controller/decomission_controller.go +++ b/internal/controller/decomission_controller.go @@ -179,18 +179,21 @@ func (r *NodeDecommissionReconciler) removeFinalizer(ctx context.Context, node * nodeBase := node.DeepCopy() controllerutil.RemoveFinalizer(node, decommissionFinalizerName) - err := r.Patch(ctx, node, k8sclient.MergeFromWithOptions(nodeBase, k8sclient.MergeFromWithOptimisticLock{})) + err := r.Patch(ctx, node, k8sclient.MergeFromWithOptions(nodeBase, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(DecommissionControllerName)) return ctrl.Result{}, err } func (r *NodeDecommissionReconciler) setDecommissioningCondition(ctx context.Context, hv *kvmv1.Hypervisor, message string) (ctrl.Result, error) { + base := hv.DeepCopy() meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeReady, Status: metav1.ConditionFalse, Reason: "Decommissioning", Message: message, }) - if err := r.Status().Update(ctx, hv); err != nil { + if err := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(DecommissionControllerName)); err != nil { return ctrl.Result{}, fmt.Errorf("cannot update hypervisor status due to %w", err) } return ctrl.Result{}, nil diff --git a/internal/controller/eviction_controller.go b/internal/controller/eviction_controller.go index 528d4f6..b8287ba 100644 --- a/internal/controller/eviction_controller.go +++ b/internal/controller/eviction_controller.go @@ -100,14 +100,15 @@ func (r *EvictionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c statusCondition := meta.FindStatusCondition(eviction.Status.Conditions, kvmv1.ConditionTypeEvicting) if statusCondition == nil { - // No status condition, so we need to add it - if r.addCondition(ctx, eviction, metav1.ConditionTrue, "Running", kvmv1.ConditionReasonRunning) { - log.Info("running") - return ctrl.Result{}, nil - } - // We just checked if the condition is there, so this should never - // be reached, but let's cover our bass - return ctrl.Result{RequeueAfter: 1 * time.Second}, nil + base := eviction.DeepCopy() + 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, base) } switch statusCondition.Status { @@ -139,6 +140,7 @@ func (r *EvictionReconciler) handleRunning(ctx context.Context, eviction *kvmv1. return r.evictNext(ctx, eviction) } + base := eviction.DeepCopy() meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeEvicting, Status: metav1.ConditionFalse, @@ -148,10 +150,16 @@ func (r *EvictionReconciler) handleRunning(ctx context.Context, eviction *kvmv1. eviction.Status.OutstandingRamMb = 0 logger.FromContext(ctx).Info("succeeded") - return ctrl.Result{}, r.Status().Update(ctx, eviction) + return ctrl.Result{}, r.updateStatus(ctx, eviction, base) +} + +func (r *EvictionReconciler) updateStatus(ctx context.Context, eviction, base *kvmv1.Eviction) error { + return r.Status().Patch(ctx, eviction, client.MergeFromWithOptions(base, + client.MergeFromWithOptimisticLock{}), client.FieldOwner(EvictionControllerName)) } func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv1.Eviction) (ctrl.Result, error) { + base := eviction.DeepCopy() hypervisorName := eviction.Spec.Hypervisor // Does the hypervisor even exist? Is it enabled/disabled? @@ -176,7 +184,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv Message: err.Error(), Reason: kvmv1.ConditionReasonFailed, }) - return ctrl.Result{}, r.Status().Update(ctx, eviction) + return ctrl.Result{}, r.updateStatus(ctx, eviction, base) } else { // That is (likely) an eviction for a node that never registered // so we are good to go @@ -189,7 +197,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv }) eviction.Status.OutstandingRamMb = 0 logger.FromContext(ctx).Info(msg) - return ctrl.Result{}, r.Status().Update(ctx, eviction) + return ctrl.Result{}, r.updateStatus(ctx, eviction, base) } } @@ -204,7 +212,7 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv Message: err.Error(), Reason: kvmv1.ConditionReasonFailed, }) - return ctrl.Result{}, r.Status().Update(ctx, eviction) + return ctrl.Result{}, r.updateStatus(ctx, eviction, base) } if !meta.IsStatusConditionTrue(eviction.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) { @@ -235,15 +243,18 @@ func (r *EvictionReconciler) handlePreflight(ctx context.Context, eviction *kvmv Message: "Preflight checks passed, hypervisor is disabled and ready for eviction", Reason: kvmv1.ConditionReasonSucceeded, }) - return ctrl.Result{}, r.Status().Update(ctx, eviction) + return ctrl.Result{}, r.updateStatus(ctx, eviction, base) } // Tries to handle the NotFound-error by updating the status -func (r *EvictionReconciler) handleNotFound(ctx context.Context, eviction *kvmv1.Eviction, err error) error { +func (r *EvictionReconciler) handleNotFound(ctx context.Context, eviction, base *kvmv1.Eviction, err error) error { if !gophercloud.ResponseCodeIs(err, http.StatusNotFound) { return err } logger.FromContext(ctx).Info("Instance is gone") + if base == nil { + base = eviction.DeepCopy() + } instances := &eviction.Status.OutstandingInstances uuid := (*instances)[len(*instances)-1] *instances = (*instances)[:len(*instances)-1] @@ -253,10 +264,11 @@ func (r *EvictionReconciler) handleNotFound(ctx context.Context, eviction *kvmv1 Message: fmt.Sprintf("Instance %s is gone", uuid), Reason: kvmv1.ConditionReasonSucceeded, }) - return r.Status().Update(ctx, eviction) + return r.updateStatus(ctx, eviction, base) } func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Eviction) (ctrl.Result, error) { + base := eviction.DeepCopy() instances := &eviction.Status.OutstandingInstances uuid := (*instances)[len(*instances)-1] log := logger.FromContext(ctx).WithName("Evict").WithValues("server", uuid) @@ -266,7 +278,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic vm, err := res.Extract() if err != nil { - if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil { + if err2 := r.handleNotFound(ctx, eviction, base, err); err2 != nil { return ctrl.Result{}, err2 } else { return ctrl.Result{RequeueAfter: shortRetryTime}, nil @@ -293,7 +305,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic Message: fmt.Sprintf("Migration of instance %s failed: %s", vm.ID, vm.Fault.Message), Reason: kvmv1.ConditionReasonFailed, }) - if err := r.Status().Update(ctx, eviction); err != nil { + if err := r.updateStatus(ctx, eviction, base); err != nil { return ctrl.Result{}, err } @@ -314,7 +326,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic // 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() - if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil { + if err2 := r.handleNotFound(ctx, eviction, base, err); err2 != nil { // Retry confirm in next reconciliation return ctrl.Result{}, err2 } else { @@ -325,7 +337,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic // All done *instances = (*instances)[:len(*instances)-1] - return ctrl.Result{}, r.Status().Update(ctx, eviction) + return ctrl.Result{}, r.updateStatus(ctx, eviction, base) } if vm.TaskState == "deleting" { //nolint:gocritic @@ -339,14 +351,14 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic Message: fmt.Sprintf("Live migration of terminating instance %s skipped", vm.ID), Reason: kvmv1.ConditionReasonFailed, }) - if err2 := r.Status().Update(ctx, eviction); err2 != nil { + if err2 := r.updateStatus(ctx, eviction, base); err2 != nil { return ctrl.Result{}, fmt.Errorf("could update status due to %w", err2) } } else if vm.Status == "ACTIVE" || vm.PowerState == 1 { log.Info("trigger live-migration") err := r.liveMigrate(ctx, vm.ID, eviction) if err != nil { - if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil { + if err2 := r.handleNotFound(ctx, eviction, base, err); err2 != nil { return ctrl.Result{}, err2 } else { return ctrl.Result{RequeueAfter: shortRetryTime}, nil @@ -356,7 +368,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic log.Info("trigger cold-migration") err := r.coldMigrate(ctx, vm.ID, eviction) if err != nil { - if err2 := r.handleNotFound(ctx, eviction, err); err2 != nil { + if err2 := r.handleNotFound(ctx, eviction, base, err); err2 != nil { return ctrl.Result{}, err2 } else { return ctrl.Result{RequeueAfter: shortRetryTime}, nil @@ -389,9 +401,9 @@ func (r *EvictionReconciler) handleFinalizer(ctx context.Context, eviction *kvmv } } - evictionBase := eviction.DeepCopy() + base := eviction.DeepCopy() controllerutil.RemoveFinalizer(eviction, evictionFinalizerName) - return r.Patch(ctx, eviction, client.MergeFromWithOptions(evictionBase, client.MergeFromWithOptimisticLock{})) + return r.Patch(ctx, eviction, client.MergeFromWithOptions(base, client.MergeFromWithOptimisticLock{})) } func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, eviction *kvmv1.Eviction) error { @@ -400,6 +412,7 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti hypervisor, err := openstack.GetHypervisorByName(ctx, r.computeClient, eviction.Spec.Hypervisor, false) if err != nil { if errors.Is(err, openstack.ErrNoHypervisor) { + base := eviction.DeepCopy() changed := meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeHypervisorReEnabled, Status: metav1.ConditionTrue, @@ -407,11 +420,12 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti Reason: kvmv1.ConditionReasonSucceeded, }) if changed { - return r.Status().Update(ctx, eviction) + return r.updateStatus(ctx, eviction, base) } else { return nil } } else { + base := eviction.DeepCopy() errorMessage := fmt.Sprintf("failed to get hypervisor due to %s", err) // update the condition to reflect the error, and retry the reconciliation changed := meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ @@ -422,7 +436,7 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti }) if changed { - if err2 := r.Status().Update(ctx, eviction); err2 != nil { + if err2 := r.updateStatus(ctx, eviction, base); err2 != nil { log.Error(err, "failed to store error message in condition", "message", errorMessage) return err2 } @@ -433,6 +447,7 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti } if hypervisor.Service.DisabledReason != r.evictionReason(eviction) { + base := eviction.DeepCopy() changed := meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeHypervisorReEnabled, Status: metav1.ConditionTrue, @@ -440,7 +455,7 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti Reason: kvmv1.ConditionReasonSucceeded, }) if changed { - return r.Status().Update(ctx, eviction) + return r.updateStatus(ctx, eviction, base) } else { return nil } @@ -451,6 +466,7 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti _, err = services.Update(ctx, r.computeClient, hypervisor.Service.ID, enableService).Extract() if err != nil { + base := eviction.DeepCopy() errorMessage := fmt.Sprintf("failed to enable hypervisor due to %s", err) changed := meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeHypervisorReEnabled, @@ -459,12 +475,13 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti Reason: kvmv1.ConditionReasonFailed, }) if changed { - if err2 := r.Status().Update(ctx, eviction); err2 != nil { + if err2 := r.updateStatus(ctx, eviction, base); err2 != nil { log.Error(err, "failed to store error message in condition", "message", errorMessage) } } return ErrRetry } else { + base := eviction.DeepCopy() changed := meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeHypervisorReEnabled, Status: metav1.ConditionTrue, @@ -472,7 +489,7 @@ func (r *EvictionReconciler) enableHypervisorService(ctx context.Context, evicti Reason: kvmv1.ConditionReasonSucceeded, }) if changed { - return r.Status().Update(ctx, eviction) + return r.updateStatus(ctx, eviction, base) } else { return nil } @@ -486,6 +503,7 @@ func (r *EvictionReconciler) disableHypervisor(ctx context.Context, hypervisor * disabledReason := hypervisor.Service.DisabledReason if disabledReason != "" && disabledReason != evictionReason { + base := eviction.DeepCopy() // Disabled for another reason already meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeHypervisorDisabled, @@ -493,19 +511,20 @@ func (r *EvictionReconciler) disableHypervisor(ctx context.Context, hypervisor * Message: fmt.Sprintf("Hypervisor already disabled for reason %q", disabledReason), Reason: kvmv1.ConditionReasonSucceeded, }) - return r.Status().Update(ctx, eviction) + return r.updateStatus(ctx, eviction, base) } if !controllerutil.ContainsFinalizer(eviction, evictionFinalizerName) { - evictionBase := eviction.DeepCopy() + base := eviction.DeepCopy() controllerutil.AddFinalizer(eviction, evictionFinalizerName) - return r.Patch(ctx, eviction, client.MergeFromWithOptions(evictionBase, client.MergeFromWithOptimisticLock{})) + return r.Patch(ctx, eviction, client.MergeFromWithOptions(base, client.MergeFromWithOptimisticLock{}), client.FieldOwner(EvictionControllerName)) } disableService := services.UpdateOpts{Status: services.ServiceDisabled, DisabledReason: r.evictionReason(eviction)} _, err := services.Update(ctx, r.computeClient, hypervisor.Service.ID, disableService).Extract() + base := eviction.DeepCopy() if err != nil { // We expect OpenStack calls to be transient errors, so we retry for the next reconciliation meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ @@ -514,7 +533,7 @@ func (r *EvictionReconciler) disableHypervisor(ctx context.Context, hypervisor * Message: fmt.Sprintf("Failed to disable hypervisor: %v", err), Reason: kvmv1.ConditionReasonFailed, }) - return r.Status().Update(ctx, eviction) + return r.updateStatus(ctx, eviction, base) } meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ @@ -523,7 +542,7 @@ func (r *EvictionReconciler) disableHypervisor(ctx context.Context, hypervisor * Message: "Hypervisor disabled successfully", Reason: kvmv1.ConditionReasonSucceeded, }) - return r.Status().Update(ctx, eviction) + return r.updateStatus(ctx, eviction, base) } func (r *EvictionReconciler) liveMigrate(ctx context.Context, uuid string, eviction *kvmv1.Eviction) error { @@ -568,28 +587,6 @@ func (r *EvictionReconciler) coldMigrate(ctx context.Context, uuid string, evict return nil } -// addCondition adds a condition to the Eviction status and updates the status -func (r *EvictionReconciler) addCondition(ctx context.Context, eviction *kvmv1.Eviction, - status metav1.ConditionStatus, message string, reason string) bool { - - if !meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: status, - Message: message, - Reason: reason, - }) { - return false - } - - if err := r.Status().Update(ctx, eviction); err != nil { - log := logger.FromContext(ctx) - log.Error(err, "Failed to update conditions") - return false - } - - return true -} - // SetupWithManager sets up the controller with the Manager. func (r *EvictionReconciler) SetupWithManager(mgr ctrl.Manager) error { ctx := context.Background() diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index 087c15c..3ce5133 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -98,13 +98,12 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) // continue with creation } else { // update Status if needed - changed := false + base := hypervisor.DeepCopy() // transfer internal IP for _, address := range node.Status.Addresses { if address.Type == corev1.NodeInternalIP && hypervisor.Status.InternalIP != address.Address { hypervisor.Status.InternalIP = address.Address - changed = true break } } @@ -113,40 +112,42 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) nodeTerminationCondition := FindNodeStatusCondition(node.Status.Conditions, "Terminating") if nodeTerminationCondition != nil && nodeTerminationCondition.Status == corev1.ConditionTrue { // Node might be terminating, propagate condition to hypervisor - changed = meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ + meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeReady, Status: metav1.ConditionFalse, Reason: nodeTerminationCondition.Reason, Message: nodeTerminationCondition.Message, - }) || changed - changed = meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ + }) + meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeTerminating, Status: metav1.ConditionStatus(nodeTerminationCondition.Status), Reason: nodeTerminationCondition.Reason, Message: nodeTerminationCondition.Message, - }) || changed + }) } - if changed { - return ctrl.Result{}, hv.Status().Update(ctx, hypervisor) + if !equality.Semantic.DeepEqual(hypervisor, base) { + return ctrl.Result{}, hv.Status().Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorControllerName)) } - before := hypervisor.DeepCopy() syncLabelsAndAnnotations(nodeLabels, hypervisor, node) - if equality.Semantic.DeepEqual(hypervisor, before) { + if equality.Semantic.DeepEqual(hypervisor, base) { return ctrl.Result{}, nil } - return ctrl.Result{}, hv.Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(before, k8sclient.MergeFromWithOptimisticLock{})) + return ctrl.Result{}, hv.Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorControllerName)) } syncLabelsAndAnnotations(nodeLabels, hypervisor, node) - if err := controllerutil.SetOwnerReference(node, hypervisor, hv.Scheme, controllerutil.WithBlockOwnerDeletion(true)); err != nil { + if err := controllerutil.SetOwnerReference(node, hypervisor, hv.Scheme, + controllerutil.WithBlockOwnerDeletion(true)); err != nil { return ctrl.Result{}, fmt.Errorf("failed setting controller reference: %w", err) } - if err := hv.Create(ctx, hypervisor); err != nil { + if err := hv.Create(ctx, hypervisor, 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 0f3160f..5ca8fee 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -86,7 +86,8 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c return ctrl.Result{}, nil } - return ctrl.Result{}, hec.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(old, k8sclient.MergeFromWithOptimisticLock{})) + return ctrl.Result{}, hec.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(old, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(MaintenanceControllerName)) } func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor) error { diff --git a/internal/controller/hypervisor_taint_controller.go b/internal/controller/hypervisor_taint_controller.go index 17afd32..64a7e5e 100644 --- a/internal/controller/hypervisor_taint_controller.go +++ b/internal/controller/hypervisor_taint_controller.go @@ -84,7 +84,8 @@ func (r *HypervisorTaintController) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } - return ctrl.Result{}, r.Status().Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(before, k8sclient.MergeFromWithOptimisticLock{})) + return ctrl.Result{}, r.Status().Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(before, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorTaintControllerName)) } func (r *HypervisorTaintController) SetupWithManager(mgr ctrl.Manager) error { diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index 52d9392..e882c44 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -27,6 +27,7 @@ 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" @@ -107,6 +108,7 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) // check condition reason status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) if status == nil { + base := hv.DeepCopy() meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeReady, Status: metav1.ConditionFalse, @@ -120,7 +122,7 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) Reason: kvmv1.ConditionReasonInitial, Message: "Initial onboarding", }) - return ctrl.Result{}, r.Status().Update(ctx, hv) + return ctrl.Result{}, r.patchStatus(ctx, hv, base) } switch status.Reason { @@ -145,7 +147,7 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy return nil } - changed := false + base := hv.DeepCopy() ready := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeReady) if ready != nil { // Only undo ones own readiness status reporting @@ -156,23 +158,21 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy Reason: kvmv1.ConditionReasonOnboarding, Message: "Onboarding aborted", }) - changed = true } } - if meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionFalse, Reason: kvmv1.ConditionReasonAborted, Message: "Aborted due to LifecycleEnabled being false", - }) { - changed = true - } + }) - if !changed { + if equality.Semantic.DeepEqual(hv, base) { // Already aborted return nil } + if err := r.deleteTestServers(ctx, computeHost); err != nil { meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding, @@ -181,9 +181,14 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy Message: err.Error(), }) - return errors.Join(err, r.Status().Update(ctx, hv)) + if equality.Semantic.DeepEqual(hv, base) { + return err + } + + return errors.Join(err, r.patchStatus(ctx, hv, base)) } - return r.Status().Update(ctx, hv) + + return r.patchStatus(ctx, hv, base) } func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, host string) error { @@ -233,13 +238,14 @@ func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1. return result.Err } + base := hv.DeepCopy() meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, Reason: kvmv1.ConditionReasonTesting, Message: "Running onboarding tests", }) - return r.Status().Update(ctx, hv) + return r.patchStatus(ctx, hv, base) } func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervisor, host string) (ctrl.Result, error) { @@ -261,6 +267,7 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis return ctrl.Result{RequeueAfter: defaultWaitTime}, nil } + base := hv.DeepCopy() // Set condition back to testing meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding, @@ -268,7 +275,7 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis Reason: kvmv1.ConditionReasonTesting, Message: "Server ended up in error state: " + server.Fault.Message, }) - if err = r.Status().Update(ctx, hv); err != nil { + if err = r.patchStatus(ctx, hv, base); err != nil { return ctrl.Result{}, err } @@ -279,15 +286,18 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis return ctrl.Result{RequeueAfter: defaultWaitTime}, nil case "ACTIVE": - consoleOutput, err := servers.ShowConsoleOutput(ctx, r.testComputeClient, server.ID, servers.ShowConsoleOutputOpts{Length: 11}).Extract() + consoleOutput, err := servers. + ShowConsoleOutput(ctx, r.testComputeClient, server.ID, servers.ShowConsoleOutputOpts{Length: 11}). + Extract() if err != nil { + base := hv.DeepCopy() meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, Reason: kvmv1.ConditionReasonTesting, Message: fmt.Sprintf("could not get console output %v", err), }) - if err := r.Status().Update(ctx, hv); err != nil { + if err := r.patchStatus(ctx, hv, base); err != nil { return ctrl.Result{}, err } return ctrl.Result{RequeueAfter: defaultWaitTime}, nil @@ -298,13 +308,14 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis } if err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr(); err != nil { + base := hv.DeepCopy() meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, Reason: kvmv1.ConditionReasonTesting, Message: fmt.Sprintf("failed to terminate instance %v", err), }) - if err := r.Status().Update(ctx, hv); err != nil { + if err := r.patchStatus(ctx, hv, base); err != nil { return ctrl.Result{}, err } return ctrl.Result{RequeueAfter: defaultWaitTime}, nil @@ -321,13 +332,14 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri err := r.deleteTestServers(ctx, host) if err != nil { + base := hv.DeepCopy() if meta.SetStatusCondition(&hv.Status.Conditions, 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, r.Status().Update(ctx, hv)) + return ctrl.Result{}, errors.Join(err, r.patchStatus(ctx, hv, base)) } return ctrl.Result{}, err @@ -350,6 +362,7 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri return ctrl.Result{}, err } + base := hv.DeepCopy() // Set hypervisor ready condition meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeReady, @@ -365,7 +378,7 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri Reason: kvmv1.ConditionReasonSucceeded, Message: "Onboarding completed", }) - return ctrl.Result{}, r.Status().Update(ctx, hv) + return ctrl.Result{}, r.patchStatus(ctx, hv, base) } func (r *OnboardingController) deleteTestServers(ctx context.Context, host string) error { @@ -438,9 +451,10 @@ func (r *OnboardingController) ensureNovaProperties(ctx context.Context, hv *kvm return fmt.Errorf("could not find exact match for %v", shortHypervisorAddress) } + base := hv.DeepCopy() hv.Status.HypervisorID = myHypervisor.ID hv.Status.ServiceID = myHypervisor.Service.ID - return r.Status().Update(ctx, hv) + return r.patchStatus(ctx, hv, base) } func (r *OnboardingController) createOrGetTestServer(ctx context.Context, zone, computeHost string, nodeUid types.UID) (*servers.Server, error) { @@ -592,6 +606,11 @@ func (r *OnboardingController) findTestFlavor(ctx context.Context) (string, erro return "", errors.New("couldn't find flavor") } +func (r *OnboardingController) patchStatus(ctx context.Context, hv, base *kvmv1.Hypervisor) error { + return r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(OnboardingControllerName)) +} + // SetupWithManager sets up the controller with the Manager. func (r *OnboardingController) SetupWithManager(mgr ctrl.Manager) error { ctx := context.Background() diff --git a/internal/controller/traits_controller.go b/internal/controller/traits_controller.go index 40a72ad..d85f16f 100644 --- a/internal/controller/traits_controller.go +++ b/internal/controller/traits_controller.go @@ -93,9 +93,11 @@ 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 { + base := hv.DeepCopy() if meta.SetStatusCondition(&hv.Status.Conditions, getTraitCondition(err, "Failed to get current traits from placement")) { - err = errors.Join(tc.Status().Update(ctx, hv)) + err = errors.Join(tc.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(TraitsControllerName))) } return ctrl.Result{}, err } @@ -124,18 +126,22 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct if result.Err != nil { // set status condition + base := hv.DeepCopy() if meta.SetStatusCondition(&hv.Status.Conditions, getTraitCondition(err, "Failed to update traits in placement")) { - err = errors.Join(tc.Status().Update(ctx, hv)) + err = errors.Join(tc.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(TraitsControllerName))) } return ctrl.Result{}, err } } + base := hv.DeepCopy() // update status unconditionally, since we want always to propagate the current traits hv.Status.Traits = targetTraits meta.SetStatusCondition(&hv.Status.Conditions, getTraitCondition(nil, "Traits successfully updated")) - return ctrl.Result{}, tc.Status().Update(ctx, hv) + return ctrl.Result{}, tc.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(TraitsControllerName)) } // getTraitCondition creates a Condition object for trait updates