Clean up scaled down services based on hostname

- Update service objects in functional test api_fixture
- Adds a check to wait for readycount to become equal to replica
- Delete service host from nova DB by hostname on replica count update

controllers/common.go

@@ -21,6 +21,8 @@ import (
 	"fmt"
 	"net/url"
 	"sort"
+	"strconv"
+	"strings"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -115,6 +117,8 @@ func cleanNovaServiceFromNovaDb(
 	computeClient *gophercloud.ServiceClient,
 	serviceName string,
 	l logr.Logger,
+	replicaCount int32,
+	cellName string,
 ) error {
 	opts := services.ListOpts{
 		Binary: serviceName,
@@ -131,7 +135,26 @@ func cleanNovaServiceFromNovaDb(
 	}
 
 	for _, service := range allServices {
-		if service.State == "down" {
+		// delete only if serviceHost is for our cell. If no cell is
+		// provided (e.g. scheduler cleanup case) then this check is
+		// non-operational (noop)
+		if !strings.Contains(service.Host, cellName) {
+			continue
+		}
+
+		// extract 0 (suffix) from hostname nova-scheduler-0
+		hostSplits := strings.Split(service.Host, "-")
+		hostIndexStr := hostSplits[len(hostSplits)-1]
+
+		hostIndex, err := strconv.Atoi(hostIndexStr)
+		if err != nil {
+			return err
+		}
+
+		// name index start from 0
+		// which means if replicaCount is 1, only nova-scheduler-0 is valid case
+		// so delete >= 1
+		if hostIndex >= int(replicaCount) {
 			rsp := services.Delete(computeClient, service.ID)
 			if rsp.Err != nil {
 				l.Error(rsp.Err, "Failed to delete service", "service", service, "response", rsp)

controllers/novaconductor_controller.go

@@ -280,6 +280,10 @@ func (r *NovaConductorReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, err
 	}
 
+	if *instance.Spec.Replicas != instance.Status.ReadyCount {
+		Log.Info("Waiting for the ReadyCount to become equal to Replicas before doing service cleanup in nova database.")
+		return ctrl.Result{}, nil
+	}
 	// clean up nova services from nova db should be always a last step in reconcile
 	err = r.cleanServiceFromNovaDb(ctx, h, instance, secret, Log)
 	if err != nil {
@@ -614,8 +618,10 @@ func (r *NovaConductorReconciler) cleanServiceFromNovaDb(
 	if err != nil {
 		return err
 	}
+	replicaCount := instance.Spec.Replicas
+	cellName := instance.Spec.CellName
 
-	return cleanNovaServiceFromNovaDb(computeClient, "nova-conductor", l)
+	return cleanNovaServiceFromNovaDb(computeClient, "nova-conductor", l, *replicaCount, cellName)
 }
 
 func (r *NovaConductorReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request {

controllers/novascheduler_controller.go

@@ -278,6 +278,11 @@ func (r *NovaSchedulerReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return result, err
 	}
 
+	if *instance.Spec.Replicas != instance.Status.ReadyCount {
+		Log.Info("Waiting for the ReadyCount to become equal to Replicas before doing service cleanup in nova database.")
+		return ctrl.Result{}, nil
+	}
+
 	// clean up nova services from nova db should be always a last step in reconcile
 	// to make sure that
 	err = r.cleanServiceFromNovaDb(ctx, h, instance, secret, Log)
@@ -668,6 +673,7 @@ func (r *NovaSchedulerReconciler) cleanServiceFromNovaDb(
 	if err != nil {
 		return err
 	}
+	replicaCount := instance.Spec.Replicas
 
-	return cleanNovaServiceFromNovaDb(computeClient, "nova-scheduler", l)
+	return cleanNovaServiceFromNovaDb(computeClient, "nova-scheduler", l, *replicaCount, "")
 }

test/functional/api_fixture.go

@@ -82,39 +82,63 @@ func AddNovaAPIFixture(log logr.Logger, server *api.FakeAPIServer) *NovaAPIFixtu
 			{
 				ID:         "1",
 				Binary:     "nova-scheduler",
-				Host:       "host1",
+				Host:       "nova-scheduler-0",
 				State:      "up",
 				Status:     "disabled",
 				ForcedDown: false,
 			},
 			{
 				ID:         "2",
 				Binary:     "nova-compute",
-				Host:       "host1",
+				Host:       "nova-compute-0",
 				State:      "up",
 				Status:     "disabled",
 				ForcedDown: false,
 			},
 			{
 				ID:         "3",
 				Binary:     "nova-scheduler",
-				Host:       "host2",
+				Host:       "nova-scheduler-1",
 				State:      "down",
 				Status:     "enabled",
 				ForcedDown: false,
 			},
 			{
 				ID:         "4",
 				Binary:     "nova-compute",
-				Host:       "host2",
+				Host:       "nova-compute-1",
 				State:      "down",
 				Status:     "disabled",
 				ForcedDown: false,
 			},
 			{
 				ID:         "5",
 				Binary:     "nova-conductor",
-				Host:       "host2",
+				Host:       "nova-cell0-conductor-0",
+				State:      "up",
+				Status:     "disabled",
+				ForcedDown: false,
+			},
+			{
+				ID:         "6",
+				Binary:     "nova-conductor",
+				Host:       "nova-cell1-conductor-0",
+				State:      "down",
+				Status:     "disabled",
+				ForcedDown: false,
+			},
+			{
+				ID:         "7",
+				Binary:     "nova-conductor",
+				Host:       "nova-cell1-conductor-1",
+				State:      "up",
+				Status:     "disabled",
+				ForcedDown: false,
+			},
+			{
+				ID:         "8",
+				Binary:     "nova-conductor",
+				Host:       "nova-cell0-conductor-1",
 				State:      "down",
 				Status:     "disabled",
 				ForcedDown: false,

test/functional/novaconductor_controller_test.go

@@ -781,8 +781,12 @@ var _ = Describe("NovaConductor controller cleaning", func() {
 				corev1.ConditionTrue,
 			)
 			Expect(novaAPIServer.HasRequest("GET", "/compute/os-services/", "binary=nova-conductor")).To(BeTrue())
-			Expect(novaAPIServer.HasRequest("DELETE", "/compute/os-services/5", "")).To(BeTrue())
-			req := novaAPIServer.FindRequest("DELETE", "/compute/os-services/5", "")
+			Expect(novaAPIServer.HasRequest("DELETE", "/compute/os-services/5", "")).NotTo(BeTrue())
+			// ID(6, 7)should not delete as these are from cell1 and not from cell0
+			Expect(novaAPIServer.HasRequest("DELETE", "/compute/os-services/6", "")).NotTo(BeTrue())
+			Expect(novaAPIServer.HasRequest("DELETE", "/compute/os-services/7", "")).NotTo(BeTrue())
+			Expect(novaAPIServer.HasRequest("DELETE", "/compute/os-services/8", "")).To(BeTrue())
+			req := novaAPIServer.FindRequest("DELETE", "/compute/os-services/8", "")
 			Expect(req.Header.Get("X-OpenStack-Nova-API-Version")).To(Equal("2.95"))
 		})
 	})