diff --git a/api/v1/eviction_types.go b/api/v1/eviction_types.go index b96ad00..5f6039f 100644 --- a/api/v1/eviction_types.go +++ b/api/v1/eviction_types.go @@ -53,6 +53,13 @@ const ( // ConditionTypeHypervisorDisabled is the type of condition for hypervisor disabled status ConditionTypeHypervisorDisabled = "HypervisorDisabled" + // ConditionTypeIncomingMigrationsSettled is set on the Hypervisor CR by the + // maintenance controller. True means no live-migrations or evacuations are + // currently in flight targeting this host. False/Unknown/Missing means the + // controller has observed outstanding incoming migrations (or hasn't checked yet) + // and the host must not be declared evicted. + ConditionTypeIncomingMigrationsSettled = "IncomingMigrationsSettled" + // ConditionTypeHaEnabled is the type of condition for signalling if HA is enabled / disabled for the hypervisor ConditionTypeHaEnabled = "HaEnabled" @@ -70,6 +77,15 @@ const ( // ConditionReasonSucceeded means the eviction has succeeded ConditionReasonSucceeded string = "Succeeded" + + // ConditionReasonSettled means all incoming migrations have reached a terminal state + ConditionReasonSettled string = "Settled" + + // ConditionReasonAborting means the controller is aborting incoming migrations + ConditionReasonAborting string = "AbortingIncomingMigrations" + + // ConditionReasonWaiting means the controller is waiting for non-abortable incoming migrations + ConditionReasonWaiting string = "WaitingForIncomingMigrations" ) // EvictionStatus defines the observed state of Eviction diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index 5c82487..25bbc72 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -23,27 +23,31 @@ package controller import ( "context" "fmt" + "time" - "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" logger "sigs.k8s.io/controller-runtime/pkg/log" "github.com/gophercloud/gophercloud/v2" "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" ) const ( HypervisorMaintenanceControllerName = "HypervisorMaintenance" + + // settleRequeueInterval is the interval at which the controller re-checks + // incoming migrations when they haven't settled yet. + settleRequeueInterval = 10 * time.Second ) type HypervisorMaintenanceController struct { @@ -58,7 +62,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,182 +72,299 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c return ctrl.Result{}, nil } - old := hv.DeepCopy() - - if err := hec.reconcileComputeService(ctx, hv); err != nil { - return ctrl.Result{}, err + // Build status apply config upfront; sub-functions mutate it directly. + // Seed only the HypervisorDisabled condition (the only one always retained). + // reconcileEviction conditionally seeds ConditionTypeEvicting and + // ConditionTypeIncomingMigrationsSettled: they are included when maintenance + // is active, and intentionally omitted when MaintenanceUnset so that SSA + // prunes them from the field manager's managed fields. + statusCfg := apiv1.HypervisorStatus().WithEvicted(hv.Status.Evicted) + if c := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled); c != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*c)) } - if err := hec.reconcileEviction(ctx, hv); err != nil { + if err := hec.reconcileComputeService(ctx, hv, statusCfg); err != nil { return ctrl.Result{}, err } - if equality.Semantic.DeepEqual(hv, old) { - return ctrl.Result{}, nil + result, err := hec.reconcileEviction(ctx, hv, statusCfg) + if err != nil { + return ctrl.Result{}, err } - // 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 - - 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) - } - h.Status.Evicted = evicted - }) + applyErr := hec.Status().Apply(ctx, + apiv1.Hypervisor(hv.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, k8sclient.FieldOwner(HypervisorMaintenanceControllerName)) + return result, applyErr } -func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor) error { +// reconcileComputeService enables/disables the nova-compute service based on +// 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 - // We can only do something here, if there is a service to begin with. - // The onboarding should take care of that 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 } 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 + existing := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + if existing == nil || existing.Status != metav1.ConditionFalse { + // 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) + } } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeHypervisorDisabled). + WithStatus(metav1.ConditionFalse). + WithMessage("Hypervisor is enabled"). + WithReason(kvmv1.ConditionReasonSucceeded)) - // 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) - _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract() - if err != nil { - return fmt.Errorf("failed to enable hypervisor due to %w", err) - } 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 - } - - // We need to disable the host as per spec - enableService := services.UpdateOpts{ - Status: services.ServiceDisabled, - DisabledReason: "Hypervisor CRD: spec.maintenance=" + hv.Spec.Maintenance, - } - 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) + existing := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled) + 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) + } } + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeHypervisorDisabled). + WithStatus(metav1.ConditionTrue). + WithMessage("Hypervisor is disabled"). + WithReason(kvmv1.ConditionReasonSucceeded)) } return nil } -func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Context, hv *kvmv1.Hypervisor) error { +// reconcileEviction creates/deletes the Eviction CR and sets the ConditionTypeEvicting +// and ConditionTypeIncomingMigrationsSettled conditions plus the Evicted scalar on +// statusCfg. When eviction should be removed, the condition entries are filtered out +// so SSA prunes them. +func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Context, hv *kvmv1.Hypervisor, statusCfg *apiv1.HypervisorStatusApplyConfiguration) (ctrl.Result, 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 && + meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeIncomingMigrationsSettled) == nil { + return ctrl.Result{}, nil } - return nil + if err := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)); err != nil { + return ctrl.Result{}, err + } + // ConditionTypeEvicting and ConditionTypeIncomingMigrationsSettled are + // intentionally absent from statusCfg — SSA will prune them from this + // field manager's managed fields on Apply. + statusCfg.WithEvicted(false) + case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceTermination: - // In case of "ha", the host gets emptied from the HA service + // Gate: ensure all incoming migrations are settled before proceeding. + // This runs on every reconcile (defense in depth). + settled, err := hec.settleIncomingMigrations(ctx, hv, statusCfg) + if err != nil { + return ctrl.Result{}, err + } + if !settled { + // Cannot proceed with eviction while incoming migrations are outstanding. + // Seed the existing evicting condition so SSA does not prune it. + if cond := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting); cond != nil { + statusCfg.WithConditions(utils.ConditionFromStatus(*cond)) + } + return ctrl.Result{RequeueAfter: settleRequeueInterval}, nil + } + + // Terminal guard: if Evicting was already Succeeded, verify the host is truly empty. 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 + if hv.Status.NumInstances > 0 { + // Instance appeared after eviction completed. + // Delete the Eviction CR so ensureEviction recreates a fresh one. + log := logger.FromContext(ctx) + log.Info("Eviction reported succeeded but instances remain on host; re-entering drain", + "numInstances", hv.Status.NumInstances) + if delErr := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)); delErr != nil { + return ctrl.Result{}, delErr + } + statusCfg.WithEvicted(false) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonRunning). + WithMessage(fmt.Sprintf("Re-entering drain: %d instance(s) still on host", hv.Status.NumInstances))) + return ctrl.Result{}, nil + } + // Truly done: host is empty and migrations are settled. + statusCfg.WithConditions(utils.ConditionFromStatus(*cond)) + return ctrl.Result{}, nil } } + status, err := hec.ensureEviction(ctx, eviction, hv) if err != nil { - return err + return ctrl.Result{}, err } - var reason, message string + var reason, message string if status == metav1.ConditionFalse { + if hv.Status.NumInstances > 0 { + // Eviction CR says done but instances exist (race with in-flight migration). + // Delete the Eviction CR and restart drain. + log := logger.FromContext(ctx) + log.Info("Eviction finished but instances remain on host; restarting drain", + "numInstances", hv.Status.NumInstances) + if delErr := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)); delErr != nil { + return ctrl.Result{}, delErr + } + statusCfg.WithEvicted(false) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonRunning). + WithMessage(fmt.Sprintf("Restarting drain: %d instance(s) still on host", hv.Status.NumInstances))) + return ctrl.Result{}, nil + } message = "Evicted" reason = kvmv1.ConditionReasonSucceeded - hv.Status.Evicted = true + statusCfg.WithEvicted(true) } else { message = "Evicting" reason = kvmv1.ConditionReasonRunning - hv.Status.Evicted = false + statusCfg.WithEvicted(false) } - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: status, - Reason: reason, - Message: message, - }) - - return nil + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeEvicting). + WithStatus(status). + WithReason(reason). + WithMessage(message)) } - return nil + return ctrl.Result{}, nil } -func (hec *HypervisorMaintenanceController) ensureEviction(ctx context.Context, eviction *kvmv1.Eviction, hypervisor *kvmv1.Hypervisor) (metav1.ConditionStatus, error) { +// settleIncomingMigrations checks for in-flight migrations targeting this host, +// aborts those that can be aborted, and reports whether the host is settled. +// It sets the IncomingMigrationsSettled condition on statusCfg. +// Returns true if settled (no incoming migrations), false otherwise. +func (hec *HypervisorMaintenanceController) settleIncomingMigrations(ctx context.Context, hv *kvmv1.Hypervisor, statusCfg *apiv1.HypervisorStatusApplyConfiguration) (bool, 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 + + aborted, waiting, err := openstack.SettleIncomingMigrations(ctx, hec.computeClient, hv.Name) + if err != nil { + return false, fmt.Errorf("settling incoming migrations for %s: %w", hv.Name, err) + } + + if len(waiting) > 0 { + instanceUUIDs := make([]string, len(waiting)) + for i, m := range waiting { + instanceUUIDs[i] = m.InstanceUUID } - log.Info("Creating new eviction", "name", eviction.Name) - eviction.Spec = kvmv1.EvictionSpec{ - Hypervisor: hypervisor.Name, - Reason: "openstack-hypervisor-operator maintenance", + log.Info("Waiting for non-abortable incoming migrations", "host", hv.Name, "instances", instanceUUIDs) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeIncomingMigrationsSettled). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonWaiting). + WithMessage(fmt.Sprintf("Waiting for %d non-abortable incoming migration(s) to complete", len(waiting)))) + return false, nil + } + + if len(aborted) > 0 { + instanceUUIDs := make([]string, len(aborted)) + for i, m := range aborted { + instanceUUIDs[i] = m.InstanceUUID } + log.Info("Aborted incoming migrations", "host", hv.Name, "instances", instanceUUIDs) + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeIncomingMigrationsSettled). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonAborting). + WithMessage(fmt.Sprintf("Aborted %d incoming migration(s); verifying on next reconcile", len(aborted)))) + return false, nil + } - // This also transports the label-selector, if set - transportLabels(&hypervisor.ObjectMeta, &eviction.ObjectMeta) + // No incoming migrations — settled. + utils.SetApplyConfigurationStatusCondition(&statusCfg.Conditions, + *k8sacmetav1.Condition(). + WithType(kvmv1.ConditionTypeIncomingMigrationsSettled). + WithStatus(metav1.ConditionTrue). + WithReason(kvmv1.ConditionReasonSettled). + WithMessage("No incoming migrations targeting this host")) + return true, nil +} - if err = hec.Create(ctx, eviction); err != nil { - return metav1.ConditionUnknown, fmt.Errorf("failed to create eviction due to %w", err) +func (hec *HypervisorMaintenanceController) ensureEviction(ctx context.Context, eviction *kvmv1.Eviction, hypervisor *kvmv1.Hypervisor) (metav1.ConditionStatus, error) { + log := logger.FromContext(ctx) + + // 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 } } + ownerRef := k8sacmetav1.OwnerReference(). + WithAPIVersion(kvmv1.GroupVersion.String()). + WithKind("Hypervisor"). + WithName(hypervisor.Name). + WithUID(hypervisor.UID). + WithController(true). + WithBlockOwnerDeletion(true) + + evictionApplyCfg := apiv1.Eviction(eviction.Name). + 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) + } + + // 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) 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. diff --git a/internal/controller/hypervisor_maintenance_controller_test.go b/internal/controller/hypervisor_maintenance_controller_test.go index bdbe615..7120b78 100644 --- a/internal/controller/hypervisor_maintenance_controller_test.go +++ b/internal/controller/hypervisor_maintenance_controller_test.go @@ -20,6 +20,7 @@ package controller import ( "fmt" "net/http" + "time" "github.com/gophercloud/gophercloud/v2/testhelper" "github.com/gophercloud/gophercloud/v2/testhelper/client" @@ -28,18 +29,22 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + applymetav1 "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" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + applyv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/applyconfigurations/api/v1" ) var _ = Describe("HypervisorMaintenanceController", func() { var ( - controller *HypervisorMaintenanceController - fakeServer testhelper.FakeServer - hypervisorName = types.NamespacedName{Name: "hv-test"} + controller *HypervisorMaintenanceController + fakeServer testhelper.FakeServer + hypervisorName = types.NamespacedName{Name: "hv-test"} + migrationsResponse string // mutable: tests can set this to control /os-migrations responses + deletedMigrations []string ) const ( @@ -83,6 +88,28 @@ var _ = Describe("HypervisorMaintenanceController", func() { fakeServer = testhelper.SetupHTTP() DeferCleanup(fakeServer.Teardown) + // Default: no incoming migrations + migrationsResponse = `{"migrations": []}` + deletedMigrations = nil + + fakeServer.Mux.HandleFunc("GET /os-migrations", func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + mt := r.URL.Query().Get("migration_type") + // Only return migrations for the live-migration type query; + // evacuation query returns empty by default. + if mt == "evacuation" { + fmt.Fprint(w, `{"migrations": []}`) + } else { + fmt.Fprint(w, migrationsResponse) + } + }) + + fakeServer.Mux.HandleFunc("DELETE /servers/", func(w http.ResponseWriter, r *http.Request) { + deletedMigrations = append(deletedMigrations, r.URL.Path) + w.WriteHeader(http.StatusAccepted) + }) + By("Creating the HypervisorMaintenanceController") controller = &HypervisorMaintenanceController{ Client: k8sClient, @@ -238,6 +265,42 @@ var _ = Describe("HypervisorMaintenanceController", func() { }) }) // Spec.Maintenance="" + Context("Spec.Maintenance=\"\" with only stale IncomingMigrationsSettled condition", func() { + BeforeEach(func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + + // First, simulate that the controller previously owned the condition + // by applying it via SSA with the same field manager. + statusCfg := applyv1.HypervisorStatus(). + WithEvicted(false). + WithConditions(applymetav1.Condition(). + WithType(kvmv1.ConditionTypeIncomingMigrationsSettled). + WithStatus(metav1.ConditionFalse). + WithReason(kvmv1.ConditionReasonWaiting). + WithMessage("incoming migrations still pending"). + WithLastTransitionTime(metav1.Now())) + Expect(k8sClient.Status().Apply(ctx, + applyv1.Hypervisor(hypervisor.Name).WithStatus(statusCfg), + k8sclient.ForceOwnership, + k8sclient.FieldOwner(HypervisorMaintenanceControllerName), + )).To(Succeed()) + + // Now cancel maintenance. + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + hypervisor.Spec.Maintenance = "" + Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) + expectedBody := `{"status": "enabled", "forced_down": false}` + mockServiceUpdate(expectedBody) + }) + + It("should remove the stale IncomingMigrationsSettled condition", func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + Expect(meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeIncomingMigrationsSettled)).To(BeNil()) + }) + }) + Context("Spec.Maintenance=\"ha\"", func() { BeforeEach(func(ctx SpecContext) { hypervisor := &kvmv1.Hypervisor{} @@ -396,6 +459,29 @@ var _ = Describe("HypervisorMaintenanceController", func() { HaveField("Status", metav1.ConditionFalse), ))) }) + + It("should keep the evicting condition on a repeated reconcile (must not be pruned by SSA)", func(ctx SpecContext) { + // Reconciling again with the succeeded state already + // recorded on the Hypervisor takes the early-return + // branch in reconcileEviction. The apply must still + // seed the succeeded evicting condition — otherwise + // SSA prunes it because this controller is its sole + // owner. + req := ctrl.Request{NamespacedName: hypervisorName} + _, err := controller.Reconcile(ctx, req) + Expect(err).NotTo(HaveOccurred()) + + updated := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) + Expect(updated.Status.Conditions).To(ContainElement( + SatisfyAll( + HaveField("Type", kvmv1.ConditionTypeEvicting), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", kvmv1.ConditionReasonSucceeded), + ), + )) + Expect(updated.Status.Evicted).To(BeTrue()) + }) }) }) // Spec.Maintenance="" } @@ -459,4 +545,247 @@ var _ = Describe("HypervisorMaintenanceController", func() { Expect(hypervisor.Status.ServiceID).To(BeEmpty()) }) }) + + Context("Incoming migrations settling", func() { + BeforeEach(func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + hypervisor.Status.ServiceID = "1234" + meta.SetStatusCondition(&hypervisor.Status.Conditions, + metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: metav1.StatusSuccess, + Message: "Onboarded", + }, + ) + Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) + + // Re-read to get fresh resourceVersion, then update spec + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + hypervisor.Spec.Maintenance = "auto" + Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) + + // Permissive service mock: accept any enable/disable call + fakeServer.Mux.HandleFunc("PUT /os-services/1234", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, ServiceEnabledResponse) + }) + }) + + Context("when there are no incoming migrations", func() { + It("should set IncomingMigrationsSettled=True and proceed to create eviction", func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + Expect(meta.IsStatusConditionTrue(hypervisor.Status.Conditions, kvmv1.ConditionTypeIncomingMigrationsSettled)).To(BeTrue()) + // Eviction should be created + eviction := &kvmv1.Eviction{} + Expect(k8sClient.Get(ctx, hypervisorName, eviction)).To(Succeed()) + }) + }) + + Context("when there is a running incoming migration", func() { + BeforeEach(func(_ SpecContext) { + migrationsResponse = `{"migrations": [ + { + "id": 42, + "uuid": "mig-uuid-1", + "instance_uuid": "inst-uuid-1", + "status": "running", + "source_compute": "node003", + "dest_compute": "hv-test", + "migration_type": "live-migration" + } + ]}` + }) + + It("should set IncomingMigrationsSettled=False with Aborting reason", func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + cond := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeIncomingMigrationsSettled) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(kvmv1.ConditionReasonAborting)) + }) + + It("should issue a DELETE to abort the migration", func(_ SpecContext) { + Expect(deletedMigrations).To(HaveLen(1)) + Expect(deletedMigrations[0]).To(ContainSubstring("inst-uuid-1")) + Expect(deletedMigrations[0]).To(ContainSubstring("42")) + }) + + It("should not create an eviction resource", func(ctx SpecContext) { + eviction := &kvmv1.Eviction{} + err := k8sClient.Get(ctx, hypervisorName, eviction) + Expect(err).To(HaveOccurred()) + Expect(k8sclient.IgnoreNotFound(err)).To(Succeed()) + }) + + It("should requeue after settleRequeueInterval", func(ctx SpecContext) { + req := ctrl.Request{NamespacedName: hypervisorName} + result, err := controller.Reconcile(ctx, req) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(10 * time.Second)) + }) + }) + + Context("when there is a post-migrating incoming migration", func() { + BeforeEach(func(_ SpecContext) { + migrationsResponse = `{"migrations": [ + { + "id": 99, + "uuid": "mig-uuid-2", + "instance_uuid": "inst-uuid-2", + "status": "post-migrating", + "source_compute": "node003", + "dest_compute": "hv-test", + "migration_type": "live-migration" + } + ]}` + }) + + It("should set IncomingMigrationsSettled=False with Waiting reason", func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + cond := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeIncomingMigrationsSettled) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(kvmv1.ConditionReasonWaiting)) + }) + + It("should not issue any DELETE", func(_ SpecContext) { + Expect(deletedMigrations).To(BeEmpty()) + }) + + It("should not create an eviction resource", func(ctx SpecContext) { + eviction := &kvmv1.Eviction{} + err := k8sClient.Get(ctx, hypervisorName, eviction) + Expect(err).To(HaveOccurred()) + Expect(k8sclient.IgnoreNotFound(err)).To(Succeed()) + }) + + It("should requeue after settleRequeueInterval", func(ctx SpecContext) { + req := ctrl.Request{NamespacedName: hypervisorName} + result, err := controller.Reconcile(ctx, req) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(10 * time.Second)) + }) + }) + + Context("when eviction succeeded but instances remain (incident regression)", func() { + BeforeEach(func(ctx SpecContext) { + // Simulate: eviction CR exists and reports Succeeded + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + + eviction := &kvmv1.Eviction{ + ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name}, + Spec: kvmv1.EvictionSpec{ + Hypervisor: hypervisorName.Name, + Reason: "test", + }, + } + Expect(controllerutil.SetControllerReference(hypervisor, eviction, controller.Scheme)).To(Succeed()) + Expect(k8sClient.Create(ctx, eviction)).To(Succeed()) + + meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: metav1.ConditionFalse, + Message: "done", + Reason: kvmv1.ConditionReasonSucceeded, + }) + Expect(k8sClient.Status().Update(ctx, eviction)).To(Succeed()) + + // Set NumInstances=1 on the Hypervisor (simulating late-arriving instance) + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + hypervisor.Status.NumInstances = 1 + meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonSucceeded, + Message: "Evicted", + }) + hypervisor.Status.Evicted = true + Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) + }) + + It("should flip Evicted back to false", func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + Expect(hypervisor.Status.Evicted).To(BeFalse()) + }) + + It("should set Evicting back to Running", func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + cond := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeEvicting) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(kvmv1.ConditionReasonRunning)) + }) + + It("should delete the eviction CR to re-enter drain", func(ctx SpecContext) { + eviction := &kvmv1.Eviction{} + err := k8sClient.Get(ctx, hypervisorName, eviction) + Expect(err).To(HaveOccurred()) + Expect(k8sclient.IgnoreNotFound(err)).To(Succeed()) + }) + }) + + Context("when eviction just finished but instances remain (no pre-existing Succeeded condition)", func() { + BeforeEach(func(ctx SpecContext) { + // Simulate: eviction CR exists and reports finished, but the + // Hypervisor does NOT have Evicting=Succeeded yet (first reconcile + // after the Eviction CR transitioned). + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + + eviction := &kvmv1.Eviction{ + ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name}, + Spec: kvmv1.EvictionSpec{ + Hypervisor: hypervisorName.Name, + Reason: "test", + }, + } + Expect(controllerutil.SetControllerReference(hypervisor, eviction, controller.Scheme)).To(Succeed()) + Expect(k8sClient.Create(ctx, eviction)).To(Succeed()) + + meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: metav1.ConditionFalse, + Message: "done", + Reason: kvmv1.ConditionReasonSucceeded, + }) + Expect(k8sClient.Status().Update(ctx, eviction)).To(Succeed()) + + // Set NumInstances=1 but leave Evicting condition as Running (not Succeeded) + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + hypervisor.Status.NumInstances = 1 + meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonRunning, + Message: "Evicting", + }) + Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) + }) + + It("should set Evicting to True/Running and not declare Evicted", func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + Expect(hypervisor.Status.Evicted).To(BeFalse()) + cond := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeEvicting) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(kvmv1.ConditionReasonRunning)) + }) + + It("should delete the eviction CR to restart drain", func(ctx SpecContext) { + eviction := &kvmv1.Eviction{} + err := k8sClient.Get(ctx, hypervisorName, eviction) + Expect(err).To(HaveOccurred()) + Expect(k8sclient.IgnoreNotFound(err)).To(Succeed()) + }) + }) + }) }) diff --git a/internal/openstack/migrations.go b/internal/openstack/migrations.go new file mode 100644 index 0000000..9a92ee2 --- /dev/null +++ b/internal/openstack/migrations.go @@ -0,0 +1,173 @@ +/* +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 openstack + +import ( + "context" + "fmt" + "net/http" + "net/url" + "strconv" + "time" + + "github.com/gophercloud/gophercloud/v2" +) + +// IncomingMigrationsLookbackWindow defines how far back we query Nova for +// migrations. Nova's live-migration monitor updates Migration.updated_at every +// ~5 seconds on both libvirt/kvm and libvirt/cloud-hypervisor backends, so any +// in-flight migration is guaranteed to appear in this window. +const IncomingMigrationsLookbackWindow = 6 * time.Hour + +// MigrationInfo holds the subset of Nova migration fields relevant to settling +// incoming migrations on a host being drained. +type MigrationInfo struct { + ID int `json:"id"` + UUID string `json:"uuid"` + InstanceUUID string `json:"instance_uuid"` + Status string `json:"status"` + SourceCompute string `json:"source_compute"` + DestCompute string `json:"dest_compute"` + MigrationType string `json:"migration_type"` +} + +// migrationsResponse mirrors Nova's GET /os-migrations JSON envelope. +type migrationsResponse struct { + Migrations []MigrationInfo `json:"migrations"` +} + +// terminalStatuses are migration statuses that no longer represent an in-flight +// migration. Derived from nova/db/main/api.py migration_get_in_progress_by_host_and_node. +var terminalStatuses = map[string]bool{ + "confirmed": true, + "reverted": true, + "error": true, + "failed": true, + "completed": true, + "cancelled": true, + "done": true, +} + +// abortableStatuses are migration statuses where a DELETE (abort) is supported. +// queued and preparing require microversion >= 2.65; running is always abortable. +// The operator uses microversion 2.90 which covers all three. +var abortableStatuses = map[string]bool{ + "queued": true, + "preparing": true, + "running": true, +} + +// ListActiveIncomingMigrations queries Nova for migrations that target the +// given host and are still in a non-terminal state. It issues a single call +// with a changes-since window of IncomingMigrationsLookbackWindow, then +// filters client-side for dest_compute==host and non-terminal status. +func ListActiveIncomingMigrations(ctx context.Context, sc *gophercloud.ServiceClient, host string) ([]MigrationInfo, error) { + changesSince := time.Now().UTC().Add(-IncomingMigrationsLookbackWindow).Format(time.RFC3339) + + migrations, err := listMigrationsForHost(ctx, sc, host, changesSince) + if err != nil { + return nil, fmt.Errorf("listing migrations for host %s: %w", host, err) + } + + var result []MigrationInfo + for _, m := range migrations { + if m.DestCompute != host { + continue + } + if terminalStatuses[m.Status] { + continue + } + result = append(result, m) + } + + return result, nil +} + +// AbortMigration issues DELETE /servers/{instanceUUID}/migrations/{migrationID} +// to abort a live-migration. Treats HTTP 404 and 409 as non-fatal (the +// migration already transitioned past an abortable state or was deleted). +func AbortMigration(ctx context.Context, sc *gophercloud.ServiceClient, instanceUUID string, migrationID int) error { + deleteURL := sc.ServiceURL("servers", instanceUUID, "migrations", strconv.Itoa(migrationID)) + + resp, err := sc.Delete(ctx, deleteURL, &gophercloud.RequestOpts{ + OkCodes: []int{http.StatusAccepted, http.StatusNoContent}, + }) + if err != nil { + if gophercloud.ResponseCodeIs(err, http.StatusNotFound) || gophercloud.ResponseCodeIs(err, http.StatusConflict) { + // Migration already completed/cancelled or not found — non-fatal. + return nil + } + return fmt.Errorf("aborting migration %d for instance %s: %w", migrationID, instanceUUID, err) + } + // gophercloud closes the response body when JSONResponse is nil, + // but close defensively if it wasn't. + if resp != nil && resp.Body != nil { + resp.Body.Close() + } + return nil +} + +// SettleIncomingMigrations combines listing and aborting: it lists all active +// incoming migrations for the host, aborts those in abortable states, and +// returns both the aborted and the waiting (post-migrating) sets. +// +// The caller should treat: +// - len(aborted) > 0 as "just issued aborts, recheck on next reconcile" +// - len(waiting) > 0 as "cannot abort, must wait for completion" +// - both empty as "no incoming migrations; host is settled" +func SettleIncomingMigrations(ctx context.Context, sc *gophercloud.ServiceClient, host string) (aborted, waiting []MigrationInfo, err error) { + active, err := ListActiveIncomingMigrations(ctx, sc, host) + if err != nil { + return nil, nil, err + } + + for _, m := range active { + if abortableStatuses[m.Status] && m.MigrationType == "live-migration" { + if abortErr := AbortMigration(ctx, sc, m.InstanceUUID, m.ID); abortErr != nil { + return aborted, waiting, abortErr + } + aborted = append(aborted, m) + } else { + // Non-abortable: either post-migrating, or an evacuation (which Nova + // does not support aborting). Must wait for completion. + waiting = append(waiting, m) + } + } + + return aborted, waiting, nil +} + +// listMigrationsForHost queries GET /os-migrations with host and changes-since filters. +func listMigrationsForHost(ctx context.Context, sc *gophercloud.ServiceClient, host, changesSince string) ([]MigrationInfo, error) { + query := url.Values{} + query.Set("host", host) + query.Set("changes-since", changesSince) + + requestURL := sc.ServiceURL("os-migrations") + "?" + query.Encode() + + var parsed migrationsResponse + //nolint:bodyclose // gophercloud closes the body when JSONResponse is non-nil + _, err := sc.Get(ctx, requestURL, &parsed, &gophercloud.RequestOpts{ + OkCodes: []int{http.StatusOK}, + }) + if err != nil { + return nil, err + } + + return parsed.Migrations, nil +} diff --git a/internal/openstack/migrations_test.go b/internal/openstack/migrations_test.go new file mode 100644 index 0000000..d22b916 --- /dev/null +++ b/internal/openstack/migrations_test.go @@ -0,0 +1,618 @@ +/* +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 openstack + +import ( + "context" + "fmt" + "net/http" + + "github.com/gophercloud/gophercloud/v2/testhelper" + "github.com/gophercloud/gophercloud/v2/testhelper/client" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Migrations", func() { + var ( + fakeServer testhelper.FakeServer + ctx context.Context + ) + + BeforeEach(func() { + fakeServer = testhelper.SetupHTTP() + ctx = context.Background() + }) + + AfterEach(func() { + fakeServer.Teardown() + }) + + // migrationsHandler returns a handler that responds to GET /os-migrations. + migrationsHandler := func(data string) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, data) + } + } + + emptyMigrations := `{"migrations": []}` + + Describe("ListActiveIncomingMigrations", func() { + Context("when there are no migrations", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(emptyMigrations)) + }) + + It("should return an empty list", func() { + sc := client.ServiceClient(fakeServer) + migrations, err := ListActiveIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(migrations).To(BeEmpty()) + }) + }) + + Context("when there is a running migration targeting the host", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(`{"migrations": [ + { + "id": 42, + "uuid": "mig-uuid-1", + "instance_uuid": "inst-uuid-1", + "status": "running", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + } + ]}`)) + }) + + It("should return the migration", func() { + sc := client.ServiceClient(fakeServer) + migrations, err := ListActiveIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(migrations).To(HaveLen(1)) + Expect(migrations[0].ID).To(Equal(42)) + Expect(migrations[0].InstanceUUID).To(Equal("inst-uuid-1")) + Expect(migrations[0].Status).To(Equal("running")) + Expect(migrations[0].DestCompute).To(Equal("node009-bb549")) + }) + }) + + Context("when there is a post-migrating migration targeting the host", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(`{"migrations": [ + { + "id": 99, + "uuid": "mig-uuid-2", + "instance_uuid": "inst-uuid-2", + "status": "post-migrating", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + } + ]}`)) + }) + + It("should return the migration as active incoming", func() { + sc := client.ServiceClient(fakeServer) + migrations, err := ListActiveIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(migrations).To(HaveLen(1)) + Expect(migrations[0].Status).To(Equal("post-migrating")) + }) + }) + + Context("when there is an outbound migration (source_compute == host)", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(`{"migrations": [ + { + "id": 50, + "uuid": "mig-uuid-3", + "instance_uuid": "inst-uuid-3", + "status": "running", + "source_compute": "node009-bb549", + "dest_compute": "node005-bb549", + "migration_type": "live-migration" + } + ]}`)) + }) + + It("should ignore outbound migrations", func() { + sc := client.ServiceClient(fakeServer) + migrations, err := ListActiveIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(migrations).To(BeEmpty()) + }) + }) + + Context("when there is an incoming evacuation", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(`{"migrations": [ + { + "id": 77, + "uuid": "mig-uuid-4", + "instance_uuid": "inst-uuid-4", + "status": "running", + "source_compute": "node001-bb549", + "dest_compute": "node009-bb549", + "migration_type": "evacuation" + } + ]}`)) + }) + + It("should include incoming evacuations", func() { + sc := client.ServiceClient(fakeServer) + migrations, err := ListActiveIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(migrations).To(HaveLen(1)) + Expect(migrations[0].MigrationType).To(Equal("evacuation")) + Expect(migrations[0].DestCompute).To(Equal("node009-bb549")) + }) + }) + + Context("when migrations have terminal statuses", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(`{"migrations": [ + { + "id": 10, + "uuid": "mig-t1", + "instance_uuid": "inst-t1", + "status": "completed", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 11, + "uuid": "mig-t2", + "instance_uuid": "inst-t2", + "status": "cancelled", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 12, + "uuid": "mig-t3", + "instance_uuid": "inst-t3", + "status": "error", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 13, + "uuid": "mig-t4", + "instance_uuid": "inst-t4", + "status": "failed", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 14, + "uuid": "mig-t5", + "instance_uuid": "inst-t5", + "status": "confirmed", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 15, + "uuid": "mig-t6", + "instance_uuid": "inst-t6", + "status": "reverted", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 16, + "uuid": "mig-t7", + "instance_uuid": "inst-t7", + "status": "done", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + } + ]}`)) + }) + + It("should filter out all terminal statuses", func() { + sc := client.ServiceClient(fakeServer) + migrations, err := ListActiveIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(migrations).To(BeEmpty()) + }) + }) + + Context("when Nova returns a server error", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, `{"error": "Internal Server Error"}`) + }) + }) + + It("should propagate the error", func() { + sc := client.ServiceClient(fakeServer) + _, err := ListActiveIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).To(HaveOccurred()) + }) + }) + + Context("with a mixed set of migrations", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(`{"migrations": [ + { + "id": 1, + "uuid": "mig-1", + "instance_uuid": "inst-1", + "status": "running", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 2, + "uuid": "mig-2", + "instance_uuid": "inst-2", + "status": "running", + "source_compute": "node007-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 3, + "uuid": "mig-3", + "instance_uuid": "inst-3", + "status": "post-migrating", + "source_compute": "node002-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 4, + "uuid": "mig-4", + "instance_uuid": "inst-4", + "status": "running", + "source_compute": "node009-bb549", + "dest_compute": "node005-bb549", + "migration_type": "live-migration" + }, + { + "id": 5, + "uuid": "mig-5", + "instance_uuid": "inst-5", + "status": "completed", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + } + ]}`)) + }) + + It("should return only active incoming migrations", func() { + sc := client.ServiceClient(fakeServer) + migrations, err := ListActiveIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(migrations).To(HaveLen(3)) + ids := []int{migrations[0].ID, migrations[1].ID, migrations[2].ID} + Expect(ids).To(ConsistOf(1, 2, 3)) + }) + }) + + Context("verifies query parameters", func() { + var receivedQueries []string + + BeforeEach(func() { + receivedQueries = nil + fakeServer.Mux.HandleFunc("GET /os-migrations", func(w http.ResponseWriter, r *http.Request) { + receivedQueries = append(receivedQueries, r.URL.RawQuery) + w.Header().Add("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, `{"migrations": []}`) + }) + }) + + It("should send host and changes-since parameters", func() { + sc := client.ServiceClient(fakeServer) + _, err := ListActiveIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + // Single call without migration_type filter + Expect(receivedQueries).To(HaveLen(1)) + Expect(receivedQueries[0]).To(ContainSubstring("host=node009-bb549")) + Expect(receivedQueries[0]).To(ContainSubstring("changes-since=")) + Expect(receivedQueries[0]).NotTo(ContainSubstring("migration_type")) + }) + }) + }) + + Describe("AbortMigration", func() { + Context("when abort succeeds", func() { + var deleteCalled bool + + BeforeEach(func() { + deleteCalled = false + fakeServer.Mux.HandleFunc("DELETE /servers/inst-uuid-1/migrations/42", func(w http.ResponseWriter, r *http.Request) { + deleteCalled = true + w.WriteHeader(http.StatusAccepted) + }) + }) + + It("should issue DELETE and return nil", func() { + sc := client.ServiceClient(fakeServer) + err := AbortMigration(ctx, sc, "inst-uuid-1", 42) + Expect(err).NotTo(HaveOccurred()) + Expect(deleteCalled).To(BeTrue()) + }) + }) + + Context("when Nova returns 404 (migration already gone)", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("DELETE /servers/inst-uuid-1/migrations/42", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + fmt.Fprint(w, `{"itemNotFound": {"message": "Migration not found", "code": 404}}`) + }) + }) + + It("should treat as non-fatal", func() { + sc := client.ServiceClient(fakeServer) + err := AbortMigration(ctx, sc, "inst-uuid-1", 42) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + Context("when Nova returns 409 (migration not abortable)", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("DELETE /servers/inst-uuid-1/migrations/42", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusConflict) + fmt.Fprint(w, `{"conflictingRequest": {"message": "Migration status is post-migrating", "code": 409}}`) + }) + }) + + It("should treat as non-fatal", func() { + sc := client.ServiceClient(fakeServer) + err := AbortMigration(ctx, sc, "inst-uuid-1", 42) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + Context("when Nova returns a server error", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("DELETE /servers/inst-uuid-1/migrations/42", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, `{"error": "Internal Server Error"}`) + }) + }) + + It("should propagate the error", func() { + sc := client.ServiceClient(fakeServer) + err := AbortMigration(ctx, sc, "inst-uuid-1", 42) + Expect(err).To(HaveOccurred()) + }) + }) + }) + + Describe("SettleIncomingMigrations", func() { + Context("when there are no incoming migrations", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(emptyMigrations)) + }) + + It("should return empty aborted and waiting", func() { + sc := client.ServiceClient(fakeServer) + aborted, waiting, err := SettleIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(aborted).To(BeEmpty()) + Expect(waiting).To(BeEmpty()) + }) + }) + + Context("when there are abortable migrations", func() { + var deleteCalls int + + BeforeEach(func() { + deleteCalls = 0 + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(`{"migrations": [ + { + "id": 1, + "uuid": "mig-1", + "instance_uuid": "inst-1", + "status": "running", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 2, + "uuid": "mig-2", + "instance_uuid": "inst-2", + "status": "queued", + "source_compute": "node007-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + } + ]}`)) + + fakeServer.Mux.HandleFunc("DELETE /servers/inst-1/migrations/1", func(w http.ResponseWriter, r *http.Request) { + deleteCalls++ + w.WriteHeader(http.StatusAccepted) + }) + + fakeServer.Mux.HandleFunc("DELETE /servers/inst-2/migrations/2", func(w http.ResponseWriter, r *http.Request) { + deleteCalls++ + w.WriteHeader(http.StatusAccepted) + }) + }) + + It("should abort all and return them in aborted list", func() { + sc := client.ServiceClient(fakeServer) + aborted, waiting, err := SettleIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(aborted).To(HaveLen(2)) + Expect(waiting).To(BeEmpty()) + Expect(deleteCalls).To(Equal(2)) + }) + }) + + Context("when there is a post-migrating migration (not abortable)", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(`{"migrations": [ + { + "id": 99, + "uuid": "mig-99", + "instance_uuid": "inst-99", + "status": "post-migrating", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + } + ]}`)) + }) + + It("should return in waiting list without issuing DELETE", func() { + sc := client.ServiceClient(fakeServer) + aborted, waiting, err := SettleIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(aborted).To(BeEmpty()) + Expect(waiting).To(HaveLen(1)) + Expect(waiting[0].ID).To(Equal(99)) + }) + }) + + Context("with a mixed set (abortable + post-migrating + outbound + completed)", func() { + var deleteCalls int + + BeforeEach(func() { + deleteCalls = 0 + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(`{"migrations": [ + { + "id": 1, + "uuid": "mig-1", + "instance_uuid": "inst-1", + "status": "running", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 2, + "uuid": "mig-2", + "instance_uuid": "inst-2", + "status": "preparing", + "source_compute": "node007-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 3, + "uuid": "mig-3", + "instance_uuid": "inst-3", + "status": "post-migrating", + "source_compute": "node002-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + }, + { + "id": 4, + "uuid": "mig-4", + "instance_uuid": "inst-4", + "status": "running", + "source_compute": "node009-bb549", + "dest_compute": "node005-bb549", + "migration_type": "live-migration" + }, + { + "id": 5, + "uuid": "mig-5", + "instance_uuid": "inst-5", + "status": "completed", + "source_compute": "node003-bb549", + "dest_compute": "node009-bb549", + "migration_type": "live-migration" + } + ]}`)) + + fakeServer.Mux.HandleFunc("DELETE /servers/inst-1/migrations/1", func(w http.ResponseWriter, r *http.Request) { + deleteCalls++ + w.WriteHeader(http.StatusAccepted) + }) + + fakeServer.Mux.HandleFunc("DELETE /servers/inst-2/migrations/2", func(w http.ResponseWriter, r *http.Request) { + deleteCalls++ + w.WriteHeader(http.StatusAccepted) + }) + }) + + It("should abort 2, wait on 1, ignore outbound and completed", func() { + sc := client.ServiceClient(fakeServer) + aborted, waiting, err := SettleIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(aborted).To(HaveLen(2)) + Expect(waiting).To(HaveLen(1)) + Expect(waiting[0].ID).To(Equal(3)) + Expect(deleteCalls).To(Equal(2)) + }) + }) + + Context("when Nova list returns an error", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, `{"error": "Internal Server Error"}`) + }) + }) + + It("should propagate the error", func() { + sc := client.ServiceClient(fakeServer) + _, _, err := SettleIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).To(HaveOccurred()) + }) + }) + + Context("when there is a running evacuation (not abortable via migration-abort)", func() { + BeforeEach(func() { + fakeServer.Mux.HandleFunc("GET /os-migrations", migrationsHandler(`{"migrations": [ + { + "id": 88, + "uuid": "mig-evac-1", + "instance_uuid": "inst-evac-1", + "status": "running", + "source_compute": "node001-bb549", + "dest_compute": "node009-bb549", + "migration_type": "evacuation" + } + ]}`)) + }) + + It("should place the evacuation in waiting without issuing DELETE", func() { + sc := client.ServiceClient(fakeServer) + aborted, waiting, err := SettleIncomingMigrations(ctx, sc, "node009-bb549") + Expect(err).NotTo(HaveOccurred()) + Expect(aborted).To(BeEmpty()) + Expect(waiting).To(HaveLen(1)) + Expect(waiting[0].ID).To(Equal(88)) + Expect(waiting[0].MigrationType).To(Equal("evacuation")) + }) + }) + }) +}) diff --git a/internal/utils/conditions.go b/internal/utils/conditions.go new file mode 100644 index 0000000..98f5e7d --- /dev/null +++ b/internal/utils/conditions.go @@ -0,0 +1,136 @@ +/* +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" +) + +// 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, + } +} + +// 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 + } + if newCondition.Type == nil || *newCondition.Type == "" { + 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 != 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 + } + if int64Changed(existing.ObservedGeneration, newCondition.ObservedGeneration) { + existing.ObservedGeneration = newCondition.ObservedGeneration + changed = true + } + + return changed +} + +// strChanged reports whether the pointer value of b differs from a. +func strChanged(a, b *string) bool { + if a == nil && b == nil { + return false + } + if a == nil || b == nil { + return true + } + return *a != *b +} + +// int64Changed reports whether the pointer value of b differs from a. +func int64Changed(a, b *int64) bool { + if a == nil && b == nil { + return false + } + if a == nil || b == nil { + return true + } + return *a != *b +} diff --git a/internal/utils/conditions_test.go b/internal/utils/conditions_test.go new file mode 100644 index 0000000..b9a4f80 --- /dev/null +++ b/internal/utils/conditions_test.go @@ -0,0 +1,285 @@ +/* +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 ( + "testing" + "time" + + . "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" +) + +func TestUtils(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Utils Suite") +} + +var _ = Describe("ConditionFromStatus", func() { + It("copies all fields verbatim", func() { + ts := metav1.NewTime(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)) + gen := int64(42) + c := metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionTrue, + Reason: "AllGood", + Message: "everything is fine", + LastTransitionTime: ts, + ObservedGeneration: gen, + } + got := ConditionFromStatus(c) + 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).To(Equal(gen)) + }) + + It("includes ObservedGeneration even when zero", func() { + c := metav1.Condition{Type: "T", Status: metav1.ConditionFalse, Reason: "R"} + got := ConditionFromStatus(c) + Expect(got.ObservedGeneration).NotTo(BeNil()) + Expect(*got.ObservedGeneration).To(Equal(int64(0))) + }) +}) + +var _ = Describe("SetApplyConfigurationStatusCondition", func() { + ptr := func(s string) *string { return &s } + condStatus := func(s metav1.ConditionStatus) *metav1.ConditionStatus { return &s } + + Describe("nil / empty-type guards", func() { + It("returns false and does not panic on nil slice pointer", func() { + changed := SetApplyConfigurationStatusCondition(nil, + *k8sacmetav1.Condition().WithType("T").WithStatus(metav1.ConditionTrue).WithReason("R")) + Expect(changed).To(BeFalse()) + }) + + It("returns false when Type is nil", func() { + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + changed := SetApplyConfigurationStatusCondition(&conditions, + k8sacmetav1.ConditionApplyConfiguration{}) + Expect(changed).To(BeFalse()) + Expect(conditions).To(BeEmpty()) + }) + + It("returns false when Type is empty string", func() { + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + changed := SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition().WithType("")) + Expect(changed).To(BeFalse()) + Expect(conditions).To(BeEmpty()) + }) + }) + + Describe("appending a new condition", func() { + It("appends when the type is not yet present", func() { + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + changed := SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("Ready"). + WithStatus(metav1.ConditionTrue). + WithReason("OK")) + Expect(changed).To(BeTrue()) + Expect(conditions).To(HaveLen(1)) + Expect(*conditions[0].Type).To(Equal("Ready")) + }) + + It("sets LastTransitionTime to now when not provided", func() { + before := time.Now() + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("T"). + WithStatus(metav1.ConditionTrue). + WithReason("R")) + Expect(conditions[0].LastTransitionTime).NotTo(BeNil()) + Expect(conditions[0].LastTransitionTime.Time).To(BeTemporally(">=", before)) + }) + + It("preserves a caller-supplied LastTransitionTime", func() { + ts := metav1.NewTime(time.Date(2020, 6, 1, 0, 0, 0, 0, time.UTC)) + conditions := []k8sacmetav1.ConditionApplyConfiguration{} + SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("T"). + WithStatus(metav1.ConditionTrue). + WithReason("R"). + WithLastTransitionTime(ts)) + Expect(*conditions[0].LastTransitionTime).To(Equal(ts)) + }) + }) + + Describe("updating an existing condition", func() { + var ( + oldTS metav1.Time + conditions []k8sacmetav1.ConditionApplyConfiguration + ) + + BeforeEach(func() { + oldTS = metav1.NewTime(time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)) + conditions = []k8sacmetav1.ConditionApplyConfiguration{ + { + Type: ptr("Ready"), + Status: condStatus(metav1.ConditionFalse), + Reason: ptr("NotReady"), + Message: ptr("waiting"), + LastTransitionTime: &oldTS, + }, + } + }) + + It("updates Status and refreshes LastTransitionTime when Status changes", func() { + before := time.Now() + changed := SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("Ready"). + WithStatus(metav1.ConditionTrue). + WithReason("OK"). + WithMessage("done")) + Expect(changed).To(BeTrue()) + Expect(*conditions[0].Status).To(Equal(metav1.ConditionTrue)) + Expect(conditions[0].LastTransitionTime.Time).To(BeTemporally(">=", before)) + }) + + It("preserves LastTransitionTime when Status is unchanged", func() { + changed := SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("Ready"). + WithStatus(metav1.ConditionFalse). + WithReason("StillNotReady"). + WithMessage("still waiting")) + Expect(changed).To(BeTrue()) // Reason/Message changed + Expect(*conditions[0].LastTransitionTime).To(Equal(oldTS)) + }) + + It("returns false when nothing changed", func() { + changed := SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("Ready"). + WithStatus(metav1.ConditionFalse). + WithReason("NotReady"). + WithMessage("waiting")) + Expect(changed).To(BeFalse()) + Expect(*conditions[0].LastTransitionTime).To(Equal(oldTS)) + }) + + It("updates Reason independently of Status", func() { + changed := SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("Ready"). + WithStatus(metav1.ConditionFalse). + WithReason("DifferentReason"). + WithMessage("waiting")) + Expect(changed).To(BeTrue()) + Expect(*conditions[0].Reason).To(Equal("DifferentReason")) + Expect(*conditions[0].LastTransitionTime).To(Equal(oldTS)) + }) + + It("uses a caller-supplied LastTransitionTime when Status changes", func() { + newTS := metav1.NewTime(time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC)) + SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("Ready"). + WithStatus(metav1.ConditionTrue). + WithReason("OK"). + WithLastTransitionTime(newTS)) + Expect(*conditions[0].LastTransitionTime).To(Equal(newTS)) + }) + + It("does not affect other conditions in the slice", func() { + otherTS := metav1.NewTime(time.Date(2019, 1, 1, 0, 0, 0, 0, time.UTC)) + conditions = append(conditions, k8sacmetav1.ConditionApplyConfiguration{ + Type: ptr("Other"), + Status: condStatus(metav1.ConditionTrue), + LastTransitionTime: &otherTS, + }) + SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("Ready"). + WithStatus(metav1.ConditionTrue). + WithReason("OK")) + Expect(*conditions[1].LastTransitionTime).To(Equal(otherTS)) + }) + + It("updates ObservedGeneration and reports changed", func() { + gen := int64(5) + changed := SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("Ready"). + WithStatus(metav1.ConditionFalse). + WithReason("NotReady"). + WithMessage("waiting"). + WithObservedGeneration(gen)) + Expect(changed).To(BeTrue()) + Expect(*conditions[0].ObservedGeneration).To(Equal(gen)) + Expect(*conditions[0].LastTransitionTime).To(Equal(oldTS)) + }) + + It("detects change when existing Status is nil", func() { + conditions[0].Status = nil + changed := SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("Ready"). + WithStatus(metav1.ConditionFalse). + WithReason("NotReady"). + WithMessage("waiting")) + Expect(changed).To(BeTrue()) + Expect(*conditions[0].Status).To(Equal(metav1.ConditionFalse)) + }) + + It("does not detect a change when both Status values are nil", func() { + conditions[0].Status = nil + changed := SetApplyConfigurationStatusCondition(&conditions, + k8sacmetav1.ConditionApplyConfiguration{ + Type: ptr("Ready"), + Reason: ptr("NotReady"), + Message: ptr("waiting"), + // Status intentionally nil + }) + Expect(changed).To(BeFalse()) + Expect(*conditions[0].LastTransitionTime).To(Equal(oldTS)) + }) + + It("detects change when existing Reason is nil", func() { + conditions[0].Reason = nil + changed := SetApplyConfigurationStatusCondition(&conditions, + *k8sacmetav1.Condition(). + WithType("Ready"). + WithStatus(metav1.ConditionFalse). + WithReason("NotReady"). + WithMessage("waiting")) + Expect(changed).To(BeTrue()) + Expect(*conditions[0].Reason).To(Equal("NotReady")) + }) + + It("detects change when new Message is nil but existing is not", func() { + changed := SetApplyConfigurationStatusCondition(&conditions, + k8sacmetav1.ConditionApplyConfiguration{ + Type: ptr("Ready"), + Status: condStatus(metav1.ConditionFalse), + Reason: ptr("NotReady"), + // Message intentionally nil + }) + Expect(changed).To(BeTrue()) + Expect(conditions[0].Message).To(BeNil()) + }) + }) +})