Delete next cell irrespective of last deletion

Right now when multiple cells gets deleted if any one the cell deletion
fails, the control exits with error msg.
This change stores the errors in a list and let next cells deleted.

Adds a functional test

Closes #OSPRH-10550

Author: auniyal61

controllers/nova_controller.go

@@ -18,7 +18,9 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"sort"
 	"strings"
 
 	batchv1 "k8s.io/api/batch/v1"
@@ -600,19 +602,35 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul
 		return ctrl.Result{}, err
 	}
 
+	sortNovaCellListByName := func(cellList *novav1.NovaCellList) {
+		sort.SliceStable(cellList.Items, func(i, j int) bool {
+			return cellList.Items[i].Name < cellList.Items[j].Name
+		})
+	}
+
+	sortNovaCellListByName(novaCellList)
+
+	var deleteErrs []error
+
 	for _, cr := range novaCellList.Items {
 		_, ok := instance.Spec.CellTemplates[cr.Spec.CellName]
 		if !ok {
 			err := r.ensureCellDeleted(ctx, h, instance,
 				cr.Spec.CellName, apiTransportURL,
 				secret, apiDB, cellDBs[novav1.Cell0Name].Database.GetDatabaseHostname(), cells[novav1.Cell0Name])
 			if err != nil {
-				return ctrl.Result{}, err
+				deleteErrs = append(deleteErrs, fmt.Errorf("Cell '%s' deletion failed, because: %w", cr.Spec.CellName, err))
+
+			} else {
+				Log.Info("Cell deleted", "cell", cr.Spec.CellName)
+				delete(instance.Status.RegisteredCells, cr.Name)
 			}
-			Log.Info("Cell deleted", "cell", cr.Spec.CellName)
-			delete(instance.Status.RegisteredCells, cr.Name)
 		}
+	}
 
+	if len(deleteErrs) > 0 {
+		delErrs := errors.Join(deleteErrs...)
+		return ctrl.Result{}, delErrs
 	}
 
 	Log.Info("Successfully reconciled")

test/functional/nova_multicell_test.go

@@ -960,3 +960,196 @@ var _ = Describe("Nova multi cell", func() {
 		})
 	})
 })
+
+func CreateNovaWith4CellsAndEnsureReady(novaNames NovaNames) {
+	cell0 := novaNames.Cells["cell0"]
+	cell1 := novaNames.Cells["cell1"]
+	cell2 := novaNames.Cells["cell2"]
+	cell3 := novaNames.Cells["cell3"]
+
+	DeferCleanup(k8sClient.Delete, ctx, CreateNovaSecret(novaNames.NovaName.Namespace, SecretName))
+	DeferCleanup(k8sClient.Delete, ctx, CreateNovaMessageBusSecret(cell0))
+	DeferCleanup(k8sClient.Delete, ctx, CreateNovaMessageBusSecret(cell1))
+	DeferCleanup(k8sClient.Delete, ctx, CreateNovaMessageBusSecret(cell2))
+	DeferCleanup(k8sClient.Delete, ctx, CreateNovaMessageBusSecret(cell3))
+
+	serviceSpec := corev1.ServiceSpec{Ports: []corev1.ServicePort{{Port: 3306}}}
+	DeferCleanup(
+		mariadb.DeleteDBService,
+		mariadb.CreateDBService(novaNames.APIMariaDBDatabaseName.Namespace, novaNames.APIMariaDBDatabaseName.Name, serviceSpec))
+	DeferCleanup(mariadb.DeleteDBService, mariadb.CreateDBService(cell0.MariaDBDatabaseName.Namespace, cell0.MariaDBDatabaseName.Name, serviceSpec))
+	DeferCleanup(mariadb.DeleteDBService, mariadb.CreateDBService(cell1.MariaDBDatabaseName.Namespace, cell1.MariaDBDatabaseName.Name, serviceSpec))
+	DeferCleanup(mariadb.DeleteDBService, mariadb.CreateDBService(cell2.MariaDBDatabaseName.Namespace, cell2.MariaDBDatabaseName.Name, serviceSpec))
+	DeferCleanup(mariadb.DeleteDBService, mariadb.CreateDBService(cell3.MariaDBDatabaseName.Namespace, cell3.MariaDBDatabaseName.Name, serviceSpec))
+
+	apiMariaDBAccount, apiMariaDBSecret := mariadb.CreateMariaDBAccountAndSecret(
+		novaNames.APIMariaDBDatabaseAccount, mariadbv1.MariaDBAccountSpec{})
+	DeferCleanup(k8sClient.Delete, ctx, apiMariaDBAccount)
+	DeferCleanup(k8sClient.Delete, ctx, apiMariaDBSecret)
+
+	cell0Account, cell0Secret := mariadb.CreateMariaDBAccountAndSecret(
+		cell0.MariaDBAccountName, mariadbv1.MariaDBAccountSpec{})
+	DeferCleanup(k8sClient.Delete, ctx, cell0Account)
+	DeferCleanup(k8sClient.Delete, ctx, cell0Secret)
+
+	cell1Account, cell1Secret := mariadb.CreateMariaDBAccountAndSecret(
+		cell1.MariaDBAccountName, mariadbv1.MariaDBAccountSpec{})
+	DeferCleanup(th.DeleteInstance, cell1Account)
+	DeferCleanup(
+		th.DeleteSecret,
+		types.NamespacedName{Name: cell1Secret.Name, Namespace: cell1Secret.Namespace})
+
+	cell2Account, cell2Secret := mariadb.CreateMariaDBAccountAndSecret(
+		cell2.MariaDBAccountName, mariadbv1.MariaDBAccountSpec{})
+	// DeferCleanup(k8sClient.Delete, ctx, cell2Account)
+	// DeferCleanup(k8sClient.Delete, ctx, cell2Secret)
+	logger.Info("Not Creating defercleanup for cell2 ...", " -- ", cell2Secret)
+
+	cell3Account, cell3Secret := mariadb.CreateMariaDBAccountAndSecret(
+		cell3.MariaDBAccountName, mariadbv1.MariaDBAccountSpec{})
+	// DeferCleanup(k8sClient.Delete, ctx, cell3Account)
+	// DeferCleanup(k8sClient.Delete, ctx, cell3Secret)
+	logger.Info("Not Creating defercleanup for cell3 ...", " -- ", cell3Secret)
+
+	spec := GetDefaultNovaSpec()
+	cell0Template := GetDefaultNovaCellTemplate()
+	cell0Template["cellDatabaseInstance"] = cell0.MariaDBDatabaseName.Name
+	cell0Template["cellDatabaseAccount"] = cell0Account.Name
+
+	cell1Template := GetDefaultNovaCellTemplate()
+	cell1Template["cellDatabaseInstance"] = cell1.MariaDBDatabaseName.Name
+	cell1Template["cellDatabaseAccount"] = cell1Account.Name
+	cell1Template["cellMessageBusInstance"] = cell1.TransportURLName.Name
+	cell1Template["novaComputeTemplates"] = map[string]interface{}{
+		ironicComputeName: GetDefaultNovaComputeTemplate(),
+	}
+
+	cell2Template := GetDefaultNovaCellTemplate()
+	cell2Template["cellDatabaseInstance"] = cell2.MariaDBDatabaseName.Name
+	cell2Template["cellDatabaseAccount"] = cell2Account.Name
+	cell2Template["cellMessageBusInstance"] = cell2.TransportURLName.Name
+	cell2Template["hasAPIAccess"] = false
+
+	cell3Template := GetDefaultNovaCellTemplate()
+	cell3Template["cellDatabaseInstance"] = cell3.MariaDBDatabaseName.Name
+	cell3Template["cellDatabaseAccount"] = cell3Account.Name
+	cell3Template["cellMessageBusInstance"] = cell3.TransportURLName.Name
+	cell3Template["hasAPIAccess"] = false
+
+	spec["cellTemplates"] = map[string]interface{}{
+		"cell0": cell0Template,
+		"cell1": cell1Template,
+		"cell2": cell2Template,
+		"cell3": cell3Template,
+	}
+	spec["apiDatabaseInstance"] = novaNames.APIMariaDBDatabaseName.Name
+	spec["apiMessageBusInstance"] = cell0.TransportURLName.Name
+
+	DeferCleanup(th.DeleteInstance, CreateNova(novaNames.NovaName, spec))
+	DeferCleanup(keystone.DeleteKeystoneAPI, keystone.CreateKeystoneAPI(novaNames.NovaName.Namespace))
+	memcachedSpec := memcachedv1.MemcachedSpec{
+		MemcachedSpecCore: memcachedv1.MemcachedSpecCore{
+			Replicas: ptr.To(int32(3)),
+		},
+	}
+
+	DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(novaNames.NovaName.Namespace, MemcachedInstance, memcachedSpec))
+	infra.SimulateMemcachedReady(novaNames.MemcachedNamespace)
+	keystone.SimulateKeystoneServiceReady(novaNames.KeystoneServiceName)
+	// END of common logic with Nova multi cell test
+
+	mariadb.SimulateMariaDBDatabaseCompleted(novaNames.APIMariaDBDatabaseName)
+	mariadb.SimulateMariaDBDatabaseCompleted(cell0.MariaDBDatabaseName)
+	mariadb.SimulateMariaDBDatabaseCompleted(cell1.MariaDBDatabaseName)
+	mariadb.SimulateMariaDBDatabaseCompleted(cell2.MariaDBDatabaseName)
+	mariadb.SimulateMariaDBDatabaseCompleted(cell3.MariaDBDatabaseName)
+
+	mariadb.SimulateMariaDBAccountCompleted(novaNames.APIMariaDBDatabaseAccount)
+	mariadb.SimulateMariaDBAccountCompleted(cell0.MariaDBAccountName)
+	mariadb.SimulateMariaDBAccountCompleted(cell1.MariaDBAccountName)
+	mariadb.SimulateMariaDBAccountCompleted(cell2.MariaDBAccountName)
+	mariadb.SimulateMariaDBAccountCompleted(cell3.MariaDBAccountName)
+
+	infra.SimulateTransportURLReady(cell0.TransportURLName)
+	infra.SimulateTransportURLReady(cell1.TransportURLName)
+	infra.SimulateTransportURLReady(cell2.TransportURLName)
+	infra.SimulateTransportURLReady(cell3.TransportURLName)
+
+	th.SimulateJobSuccess(cell0.DBSyncJobName)
+	th.SimulateStatefulSetReplicaReady(cell0.ConductorStatefulSetName)
+	th.SimulateJobSuccess(cell0.CellMappingJobName)
+
+	th.SimulateStatefulSetReplicaReady(cell1.NoVNCProxyStatefulSetName)
+	th.SimulateJobSuccess(cell1.DBSyncJobName)
+	th.SimulateStatefulSetReplicaReady(cell1.ConductorStatefulSetName)
+	th.SimulateStatefulSetReplicaReady(cell1.NovaComputeStatefulSetName)
+	th.SimulateJobSuccess(cell1.CellMappingJobName)
+	th.SimulateJobSuccess(cell1.HostDiscoveryJobName)
+
+	th.SimulateStatefulSetReplicaReady(cell2.NoVNCProxyStatefulSetName)
+	th.SimulateJobSuccess(cell2.DBSyncJobName)
+	th.SimulateStatefulSetReplicaReady(cell2.ConductorStatefulSetName)
+	th.SimulateJobSuccess(cell2.CellMappingJobName)
+
+	th.SimulateStatefulSetReplicaReady(cell3.NoVNCProxyStatefulSetName)
+	th.SimulateJobSuccess(cell3.DBSyncJobName)
+	th.SimulateStatefulSetReplicaReady(cell3.ConductorStatefulSetName)
+	th.SimulateJobSuccess(cell3.CellMappingJobName)
+
+	th.ExpectCondition(
+		novaNames.NovaName,
+		ConditionGetterFunc(NovaConditionGetter),
+		novav1.NovaAllCellsReadyCondition,
+		corev1.ConditionTrue,
+	)
+	SimulateReadyOfNovaTopServices()
+	th.ExpectCondition(
+		novaNames.NovaName,
+		ConditionGetterFunc(NovaConditionGetter),
+		condition.ReadyCondition,
+		corev1.ConditionTrue,
+	)
+}
+
+var _ = Describe("Nova multi cell deletion", func() {
+	BeforeEach(func() {
+
+		CreateNovaWith4CellsAndEnsureReady(novaNames)
+
+	})
+
+	When("Nova CR instance is created with 4 cells", func() {
+		It("delete cell2 and cell3, verify for cell2", func() {
+
+			nova := GetNova(novaNames.NovaName)
+			Expect(nova.Status.RegisteredCells).To(HaveKey(cell0.CellCRName.Name), "cell0 is not in the RegisteredCells", nova.Status.RegisteredCells)
+			Expect(nova.Status.RegisteredCells).To(HaveKey(cell1.CellCRName.Name), "cell1 is not in the RegisteredCells", nova.Status.RegisteredCells)
+			Expect(nova.Status.RegisteredCells).To(HaveKey(cell2.CellCRName.Name), "cell2 is not in the RegisteredCells", nova.Status.RegisteredCells)
+			Expect(nova.Status.RegisteredCells).To(HaveKey(cell3.CellCRName.Name), "cell3 is not in the RegisteredCells", nova.Status.RegisteredCells)
+
+			cell2Account := mariadb.GetMariaDBAccount(cell2.MariaDBAccountName)
+			cell2Account.Spec.Secret = ""
+			Expect(k8sClient.Update(ctx, cell2Account)).To(Succeed())
+
+			Eventually(func(g Gomega) {
+				// remove from cells CR
+				nova := GetNova(novaNames.NovaName)
+				delete(nova.Spec.CellTemplates, "cell2")
+				delete(nova.Spec.CellTemplates, "cell3")
+				g.Expect(k8sClient.Update(ctx, nova)).To(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				nova := GetNova(novaNames.NovaName)
+				g.Expect(nova.Status.RegisteredCells).To(HaveKey(cell2.CellCRName.Name))
+				g.Expect(nova.Status.RegisteredCells).NotTo(HaveKey(cell3.CellCRName.Name))
+			}, timeout, interval).Should(Succeed())
+
+			GetNovaCell(cell2.CellCRName)
+			NovaCellNotExists(cell3.CellCRName)
+
+		})
+
+	})
+
+})

test/functional/suite_test.go

@@ -84,6 +84,7 @@ var (
 	cell0     CellNames
 	cell1     CellNames
 	cell2     CellNames
+	cell3     CellNames
 )
 
 func TestAPIs(t *testing.T) {
@@ -283,8 +284,9 @@ var _ = BeforeEach(func() {
 		Name:      uuid.New().String()[:25],
 	}
 
-	novaNames = GetNovaNames(novaName, []string{"cell0", "cell1", "cell2"})
+	novaNames = GetNovaNames(novaName, []string{"cell0", "cell1", "cell2", "cell3"})
 	cell0 = novaNames.Cells["cell0"]
 	cell1 = novaNames.Cells["cell1"]
 	cell2 = novaNames.Cells["cell2"]
+	cell3 = novaNames.Cells["cell3"]
 })