diff --git a/internal/controller/eviction_controller.go b/internal/controller/eviction_controller.go index b8fc678..cf06015 100644 --- a/internal/controller/eviction_controller.go +++ b/internal/controller/eviction_controller.go @@ -53,6 +53,36 @@ const ( defaultPollTime = 10 * time.Second ) +// popInstance removes and returns the last instance UUID from the slice. +// Returns the modified slice and the UUID (empty string if slice was empty). +func popInstance(instances []string) (remaining []string, uuid string) { + if len(instances) == 0 { + return instances, "" + } + return instances[:len(instances)-1], instances[len(instances)-1] +} + +// peekInstance returns the last instance UUID without removing it. +// Returns empty string if the slice is empty. +func peekInstance(instances []string) string { + if len(instances) == 0 { + return "" + } + return instances[len(instances)-1] +} + +// moveToBack moves the last instance to the front of the slice, +// effectively deprioritizing it. Returns the modified slice. +func moveToBack(instances []string) []string { + if len(instances) < 2 { + return instances + } + uuid := instances[len(instances)-1] + copy(instances[1:], instances[:len(instances)-1]) + instances[0] = uuid + return instances +} + // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=evictions,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=evictions/status,verbs=get;update;patch // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=evictions/finalizers,verbs=update @@ -224,9 +254,11 @@ func (r *EvictionReconciler) handleNotFound(ctx context.Context, eviction, base if base == nil { base = eviction.DeepCopy() } - instances := &eviction.Status.OutstandingInstances - uuid := (*instances)[len(*instances)-1] - *instances = (*instances)[:len(*instances)-1] + var uuid string + eviction.Status.OutstandingInstances, uuid = popInstance(eviction.Status.OutstandingInstances) + if uuid == "" { + return nil + } meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeMigration, Status: metav1.ConditionFalse, @@ -238,8 +270,10 @@ func (r *EvictionReconciler) handleNotFound(ctx context.Context, 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] + uuid := peekInstance(eviction.Status.OutstandingInstances) + if uuid == "" { + return ctrl.Result{}, nil + } log := logger.FromContext(ctx).WithName("Evict").WithValues("server", uuid) logger.IntoContext(ctx, log) @@ -265,8 +299,7 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic case "ERROR": // Needs manual intervention (or another operator fixes it) // put it at the end of the list (beginning of array) - copy((*instances)[1:], (*instances)[:len(*instances)-1]) - (*instances)[0] = uuid + eviction.Status.OutstandingInstances = moveToBack(eviction.Status.OutstandingInstances) log.Info("error", "faultMessage", vm.Fault.Message) meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeMigration, @@ -303,14 +336,13 @@ func (r *EvictionReconciler) evictNext(ctx context.Context, eviction *kvmv1.Evic } // All done - *instances = (*instances)[:len(*instances)-1] + eviction.Status.OutstandingInstances, _ = popInstance(eviction.Status.OutstandingInstances) return ctrl.Result{}, r.updateStatus(ctx, eviction, base) } if vm.TaskState == "deleting" { //nolint:gocritic // We just have to wait for it to be gone. Try the next one. - copy((*instances)[1:], (*instances)[:len(*instances)-1]) - (*instances)[0] = uuid + eviction.Status.OutstandingInstances = moveToBack(eviction.Status.OutstandingInstances) meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeMigration, diff --git a/internal/controller/eviction_controller_test.go b/internal/controller/eviction_controller_test.go index d70d855..85606ee 100644 --- a/internal/controller/eviction_controller_test.go +++ b/internal/controller/eviction_controller_test.go @@ -35,6 +35,85 @@ import ( kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) +var _ = Describe("Instance slice helpers", func() { + Describe("peekInstance", func() { + It("returns empty string for empty slice", func() { + Expect(peekInstance([]string{})).To(Equal("")) + }) + + It("returns empty string for nil slice", func() { + Expect(peekInstance(nil)).To(Equal("")) + }) + + It("returns the last element", func() { + Expect(peekInstance([]string{"a", "b", "c"})).To(Equal("c")) + }) + + It("returns the only element for single-element slice", func() { + Expect(peekInstance([]string{"only"})).To(Equal("only")) + }) + + It("does not modify the slice", func() { + s := []string{"a", "b", "c"} + peekInstance(s) + Expect(s).To(Equal([]string{"a", "b", "c"})) + }) + }) + + Describe("popInstance", func() { + It("returns empty string and unchanged slice for empty slice", func() { + s, uuid := popInstance([]string{}) + Expect(uuid).To(Equal("")) + Expect(s).To(BeEmpty()) + }) + + It("returns empty string and unchanged slice for nil slice", func() { + s, uuid := popInstance(nil) + Expect(uuid).To(Equal("")) + Expect(s).To(BeNil()) + }) + + It("removes and returns the last element", func() { + s, uuid := popInstance([]string{"a", "b", "c"}) + Expect(uuid).To(Equal("c")) + Expect(s).To(Equal([]string{"a", "b"})) + }) + + It("returns empty slice when popping last element", func() { + s, uuid := popInstance([]string{"only"}) + Expect(uuid).To(Equal("only")) + Expect(s).To(BeEmpty()) + }) + }) + + Describe("moveToBack", func() { + It("returns unchanged slice for empty slice", func() { + s := moveToBack([]string{}) + Expect(s).To(BeEmpty()) + }) + + It("returns unchanged slice for nil slice", func() { + s := moveToBack(nil) + Expect(s).To(BeNil()) + }) + + It("returns unchanged slice for single-element slice", func() { + s := moveToBack([]string{"only"}) + Expect(s).To(Equal([]string{"only"})) + }) + + It("moves last element to front for two elements", func() { + s := moveToBack([]string{"a", "b"}) + Expect(s).To(Equal([]string{"b", "a"})) + }) + + It("moves last element to front for multiple elements", func() { + s := moveToBack([]string{"a", "b", "c", "d"}) + Expect(s).To(Equal([]string{"d", "a", "b", "c"})) + }) + }) +}) + var _ = Describe("Eviction Controller", func() { const ( resourceName = "test-resource"