From 85c46e5bbea3dbbdab484c8a487eeaade97b5128 Mon Sep 17 00:00:00 2001 From: Amit Uniyal Date: Wed, 27 Nov 2024 11:46:14 +0530 Subject: [PATCH] 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 --- controllers/nova_controller.go | 24 ++- test/functional/nova_multicell_test.go | 193 +++++++++++++++++++++++++ test/functional/suite_test.go | 4 +- 3 files changed, 217 insertions(+), 4 deletions(-) diff --git a/controllers/nova_controller.go b/controllers/nova_controller.go index 12d3ac523..37fe13070 100644 --- a/controllers/nova_controller.go +++ b/controllers/nova_controller.go @@ -18,7 +18,9 @@ package controllers import ( "context" + "errors" "fmt" + "sort" "strings" batchv1 "k8s.io/api/batch/v1" @@ -600,6 +602,16 @@ 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 { @@ -607,12 +619,18 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul 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") diff --git a/test/functional/nova_multicell_test.go b/test/functional/nova_multicell_test.go index 5f64b76d1..f69834ff0 100644 --- a/test/functional/nova_multicell_test.go +++ b/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) + + }) + + }) + +}) diff --git a/test/functional/suite_test.go b/test/functional/suite_test.go index 708976631..040eefd40 100644 --- a/test/functional/suite_test.go +++ b/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"] })