Merge ~cgrabowski/maas:handle_existing_cluster_returning_no_name_on_discover into maas:master

[Christian Grabowski]

use existing cluster name when discovered_cluster does not return one

[Alexsander de Souza (alexsander-souza)]

+1

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b handle_existing_cluster_returning_no_name_on_discover lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: e6c81ff28a749535d63adb4caf0351bf4880bfe7

review: Approve

diff --git a/src/maasserver/tests/test_vmhost.py b/src/maasserver/tests/test_vmhost.py
index f33bbab..b0a4843 100644
--- a/src/maasserver/tests/test_vmhost.py
+++ b/src/maasserver/tests/test_vmhost.py
@@ -464,6 +464,48 @@ class TestSyncVMCluster(MAASServerTestCase):
             cluster.hosts(), updated_vmhost.hints.cluster.hosts()
         )
 
+    def test_sync_vmcluster_uses_existing_cluster_name_when_discovered_cluster_has_none(
+        self,
+    ):
+        (
+            discovered_cluster,
+            discovered_racks,
+            failed_racks,
+        ) = fake_cluster_discovery(self)
+        discovered_cluster.name = None
+        zone = factory.make_Zone()
+        pool = factory.make_ResourcePool()
+        cluster = factory.make_VMCluster(
+            name=factory.make_name("cluster"),
+            project=discovered_cluster.project,
+            zone=zone,
+            pool=pool,
+            pods=0,
+        )
+        vmhosts = [
+            factory.make_Pod(
+                cluster=cluster,
+                parameters={"power_address": address},
+                zone=zone,
+                pool=pool,
+            )
+            for address in discovered_cluster.pod_addresses
+        ]
+        successes = {
+            rack_id: discovered_cluster for rack_id in discovered_racks
+        }
+        failures = {
+            rack_id: factory.make_exception() for rack_id in failed_racks
+        }
+        self.patch(vmhost_module, "discover_pod").return_value = (
+            successes,
+            failures,
+        )
+        updated_vmhost = vmhost_module.discover_and_sync_vmhost(
+            vmhosts[0], factory.make_User()
+        )
+        self.assertEqual(updated_vmhost.hints.cluster.name, cluster.name)
+
 
 class TestSyncVMClusterAsync(MAASTransactionServerTestCase):
 
@@ -670,3 +712,56 @@ class TestSyncVMClusterAsync(MAASTransactionServerTestCase):
             )
 
         await deferToDatabase(_compare_cluster)
+
+    @wait_for_reactor
+    async def test_sync_vmcluster_uses_existing_cluster_name_when_discovered_cluster_has_none_async(
+        self,
+    ):
+        (
+            discovered_cluster,
+            discovered_racks,
+            failed_racks,
+        ) = await deferToDatabase(fake_cluster_discovery, self)
+        discovered_cluster.name = None
+        zone = await deferToDatabase(factory.make_Zone)
+        pool = await deferToDatabase(factory.make_ResourcePool)
+        cluster = await deferToDatabase(
+            factory.make_VMCluster,
+            name=factory.make_name("cluster"),
+            project=discovered_cluster.project,
+            zone=zone,
+            pool=pool,
+            pods=0,
+        )
+
+        def _create_vmhosts():
+            return [
+                factory.make_Pod(
+                    cluster=cluster,
+                    parameters={"power_address": address},
+                    zone=zone,
+                    pool=pool,
+                )
+                for address in discovered_cluster.pod_addresses
+            ]
+
+        vmhosts = await deferToDatabase(_create_vmhosts)
+        successes = {
+            rack_id: discovered_cluster for rack_id in discovered_racks
+        }
+        failures = {
+            rack_id: factory.make_exception() for rack_id in failed_racks
+        }
+        vmhost_module.discover_pod.return_value = succeed(
+            (successes, failures)
+        )
+        user = await deferToDatabase(factory.make_User)
+        updated_vmhost = await vmhost_module.discover_and_sync_vmhost_async(
+            vmhosts[0], user
+        )
+
+        def _get_name():
+            return updated_vmhost.hints.cluster.name
+
+        name = await deferToDatabase(_get_name)
+        self.assertEqual(name, cluster.name)
diff --git a/src/maasserver/vmhost.py b/src/maasserver/vmhost.py
index 2fc5a11..2a67f64 100644
--- a/src/maasserver/vmhost.py
+++ b/src/maasserver/vmhost.py
@@ -179,7 +179,10 @@ def _get_or_create_clustered_host(
 
 def sync_vmcluster(discovered_cluster, discovered, vmhost, user):
     cluster, _ = VMCluster.objects.get_or_create(
-        name=discovered_cluster.name or vmhost.name,
+        name=discovered_cluster.name
+        or (
+            vmhost.hints.cluster.name if vmhost.hints.cluster else vmhost.name
+        ),
         project=discovered_cluster.project,
         pool=vmhost.pool,
         zone=vmhost.zone,
@@ -215,7 +218,12 @@ def sync_vmcluster(discovered_cluster, discovered, vmhost, user):
 async def sync_vmcluster_async(discovered_cluster, discovered, vmhost, user):
     def _transaction(discovered_cluster, discovered, vmhost, user):
         cluster, _ = VMCluster.objects.get_or_create(
-            name=discovered_cluster.name or vmhost.name,
+            name=discovered_cluster.name
+            or (
+                vmhost.hints.cluster.name
+                if vmhost.hints.cluster
+                else vmhost.name
+            ),
             project=discovered_cluster.project,
             pool=vmhost.pool,
             zone=vmhost.zone,