Fix cell0 access while cell0 waits for DB

There was a bug in the nova-controller in the ensureCell loop. If the
cell0 was still waiting for the DB to be created but cell1 had all the
prerequisites are done (DB, MQ) then the code tried to look up cell0
which was not in the internal cell map causing panic.

Author: gibizer

controllers/nova_controller.go

@@ -392,9 +392,10 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul
 		}
 
 		// The cell0 is always handled first in the loop as we iterate on
-		// orderedCellNames. So for any other cells we can get cell0 from cells
-		// as it is already there.
-		if cellName != Cell0Name && cellTemplate.HasAPIAccess && !cells[Cell0Name].IsReady() {
+		// orderedCellNames. So for any other cells we can assume that if cell0
+		// is not in the list then cell0 is not ready
+		cell0Ready := (cells[Cell0Name] != nil && cells[Cell0Name].IsReady())
+		if cellName != Cell0Name && cellTemplate.HasAPIAccess && !cell0Ready {
 			allCellsReady = false
 			skippedCells = append(skippedCells, cellName)
 			util.LogForObject(

test/functional/base_test.go

@@ -533,6 +533,7 @@ func GetCellNames(novaName types.NamespacedName, cell string) CellNames {
 }
 
 type NovaNames struct {
+	Namespace                       string
 	NovaName                        types.NamespacedName
 	InternalNovaServiceName         types.NamespacedName
 	PublicNovaServiceName           types.NamespacedName
@@ -599,7 +600,8 @@ func GetNovaNames(novaName types.NamespacedName, cellNames []string) NovaNames {
 	}
 
 	return NovaNames{
-		NovaName: novaName,
+		Namespace: novaName.Namespace,
+		NovaName:  novaName,
 		InternalNovaServiceName: types.NamespacedName{ // TODO replace for nova-internal
 			Namespace: novaName.Namespace,
 			Name:      novaName.Name + "-internal",

test/functional/nova_multicell_test.go

@@ -652,55 +652,65 @@ var _ = Describe("Nova multicell", func() {
 	})
 	When("cell1 DB and MQ create finishes before cell0 DB create", func() {
 		BeforeEach(func() {
-			DeferCleanup(k8sClient.Delete, ctx, CreateNovaSecret(namespace, SecretName))
+			DeferCleanup(k8sClient.Delete, ctx, CreateNovaSecret(novaNames.Namespace, SecretName))
 			DeferCleanup(
 				k8sClient.Delete,
 				ctx,
-				CreateNovaMessageBusSecret(namespace, "mq-for-cell1-secret"),
+				CreateNovaMessageBusSecret(cell0.CellName.Namespace, fmt.Sprintf("%s-secret", cell0.TransportURLName.Name)),
 			)
 			serviceSpec := corev1.ServiceSpec{Ports: []corev1.ServicePort{{Port: 3306}}}
-			DeferCleanup(th.DeleteDBService, th.CreateDBService(namespace, "db-for-api", serviceSpec))
-			DeferCleanup(th.DeleteDBService, th.CreateDBService(namespace, "db-for-cell1", serviceSpec))
+			DeferCleanup(
+				th.DeleteDBService,
+				th.CreateDBService(
+					novaNames.APIMariaDBDatabaseName.Namespace,
+					novaNames.APIMariaDBDatabaseName.Name,
+					serviceSpec))
+			DeferCleanup(
+				th.DeleteDBService,
+				th.CreateDBService(
+					cell1.MariaDBDatabaseName.Namespace,
+					cell1.MariaDBDatabaseName.Name,
+					serviceSpec))
 
 			spec := GetDefaultNovaSpec()
-			cell0 := GetDefaultNovaCellTemplate()
-			cell0["cellName"] = "cell0"
-			cell0["cellDatabaseInstance"] = "db-for-api"
-			cell0["cellDatabaseUser"] = "nova_cell0"
+			cell0Template := GetDefaultNovaCellTemplate()
+			cell0Template["cellDatabaseInstance"] = cell0.MariaDBDatabaseName.Name
+			cell0Template["cellDatabaseUser"] = "nova_cell0"
 
-			cell1 := GetDefaultNovaCellTemplate()
-			cell1["cellName"] = "cell1"
-			cell1["cellDatabaseInstance"] = "db-for-cell1"
-			cell1["cellDatabaseUser"] = "nova_cell1"
-			cell1["cellMessageBusInstance"] = "mq-for-cell1"
+			cell1Template := GetDefaultNovaCellTemplate()
+			cell1Template["cellDatabaseInstance"] = cell1.MariaDBDatabaseName.Name
+			cell1Template["cellDatabaseUser"] = "nova_cell1"
+			cell1Template["cellMessageBusInstance"] = cell1.TransportURLName.Name
 
 			spec["cellTemplates"] = map[string]interface{}{
-				"cell0": cell0,
-				"cell1": cell1,
+				"cell0": cell0Template,
+				"cell1": cell1Template,
 			}
-			spec["apiDatabaseInstance"] = "db-for-api"
-			spec["apiMessageBusInstance"] = "mq-for-api"
+			spec["apiDatabaseInstance"] = novaNames.APIMariaDBDatabaseName.Name
+			spec["apiMessageBusInstance"] = cell0.TransportURLName.Name
 
-			DeferCleanup(DeleteInstance, CreateNova(novaName, spec))
-			keystoneAPIName := th.CreateKeystoneAPI(namespace)
+			DeferCleanup(th.DeleteInstance, CreateNova(novaNames.NovaName, spec))
+			keystoneAPIName := th.CreateKeystoneAPI(novaNames.Namespace)
 			DeferCleanup(th.DeleteKeystoneAPI, keystoneAPIName)
 			keystoneAPI := th.GetKeystoneAPI(keystoneAPIName)
 			keystoneAPI.Status.APIEndpoints["internal"] = "http://keystone-internal-openstack.testing"
 			Eventually(func(g Gomega) {
 				g.Expect(k8sClient.Status().Update(ctx, keystoneAPI.DeepCopy())).Should(Succeed())
 			}, timeout, interval).Should(Succeed())
-			th.SimulateKeystoneServiceReady(novaKeystoneServiceName)
+			th.SimulateKeystoneServiceReady(novaNames.KeystoneServiceName)
 		})
 
-		// We cannot merge this test as is because if the tests run
-		// in parallel then this test hangs forever (not even the global
-		// ginkgo --timeout stops it). If you run it with --procs 1 locally
-		// then it shows the panic and terminates
-		// It("panics", func(ctx SpecContext) {
-		// 	th.SimulateMariaDBDatabaseCompleted(cell1.MariaDBDatabaseName)
-		// 	th.SimulateTransportURLReady(cell1.TransportURLName)
-
-		// 	// BUG: the nova-controller-manager panics but it should not
-		// })
+		It("waits for cell0 DB to be created", func(ctx SpecContext) {
+			th.SimulateMariaDBDatabaseCompleted(cell1.MariaDBDatabaseName)
+			th.SimulateTransportURLReady(cell1.TransportURLName)
+			// NOTE(gibi): before the fix https://github.com/openstack-k8s-operators/nova-operator/pull/356
+			// nova-controller panic at this point and test would hang
+			th.ExpectCondition(
+				novaNames.NovaName,
+				ConditionGetterFunc(NovaConditionGetter),
+				novav1.NovaAllCellsReadyCondition,
+				corev1.ConditionFalse,
+			)
+		})
 	})
 })