Merge ~ack/maas:1984141-pod-console-log-no-duplicate into maas:master

[Alberto Donato]

LP:1984141 don't fail if pod-console-log tag exists with different kernel options

[MAAS Lander (maas-lander)]

UNIT TESTS
-b 1984141-pod-console-log-no-duplicate lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/416/consoleText
COMMIT: ffadcda0c6532a836002c76fc52fcc070cc9f6dd

review: Needs Fixing

[Alberto Donato (ack)]

jenkins: !test

[MAAS Lander (maas-lander)]

UNIT TESTS
-b 1984141-pod-console-log-no-duplicate lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: ffadcda0c6532a836002c76fc52fcc070cc9f6dd

review: Approve

[Alexsander de Souza (alexsander-souza)]

+1

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b 1984141-pod-console-log-no-duplicate lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci.internal:8080/job/maas-tester/422/consoleText

diff --git a/src/maasserver/forms/pods.py b/src/maasserver/forms/pods.py
index e7e1af6..05cb103 100644
--- a/src/maasserver/forms/pods.py
+++ b/src/maasserver/forms/pods.py
@@ -45,7 +45,6 @@ from maasserver.models import (
     Pod,
     PodStoragePool,
     ResourcePool,
-    Tag,
     Zone,
 )
 from maasserver.node_constraint_filter_forms import (
@@ -65,7 +64,10 @@ from maasserver.utils.dns import validate_hostname
 from maasserver.utils.forms import set_form_error
 from maasserver.utils.orm import post_commit_do, transactional
 from maasserver.utils.threads import deferToDatabase
-from maasserver.vmhost import request_commissioning_results
+from maasserver.vmhost import (
+    ensure_pod_console_logging_tag,
+    request_commissioning_results,
+)
 from provisioningserver.certificates import Certificate
 from provisioningserver.drivers import SETTING_SCOPE
 from provisioningserver.drivers.pod import (
@@ -303,13 +305,8 @@ class PodForm(MAASModelForm):
             )
 
         # Add tag for pod console logging with appropriate kernel parameters.
-        tag, _ = Tag.objects.get_or_create(
-            name="pod-console-logging",
-            kernel_opts="console=tty1 console=ttyS0",
-        )
-        # Add this tag to the pod.  This only adds the tag
-        # if it is not present on the pod.
-        self.instance.add_tag(tag.name)
+        self.instance.add_tag(ensure_pod_console_logging_tag().name)
+
         if self.is_new and power_type == "lxd":
             self.instance.created_with_trust_password = bool(
                 self.cleaned_data["password"]
diff --git a/src/maasserver/forms/tests/test_pods.py b/src/maasserver/forms/tests/test_pods.py
index 21c4708..12a70e0 100644
--- a/src/maasserver/forms/tests/test_pods.py
+++ b/src/maasserver/forms/tests/test_pods.py
@@ -227,6 +227,18 @@ class TestPodForm(MAASTransactionServerTestCase):
         pod = form.save()
         self.assertCountEqual(tags, pod.tags)
 
+    def test_creates_pod_with_existing_pod_console_logging_tag(self):
+        tag = factory.make_Tag(
+            name="pod-console-logging", kernel_opts="foo bar"
+        )
+        pod_info = self.make_pod_info()
+        form = PodForm(data=pod_info, request=self.request)
+        self.assertTrue(form.is_valid(), form._errors)
+        pod = form.save()
+        self.assertIn("pod-console-logging", pod.tags)
+        tag = reload_object(tag)
+        self.assertEqual(tag.kernel_opts, "foo bar")
+
     def test_creates_pod_with_zone(self):
         pod_info = self.make_pod_info()
         zone = factory.make_Zone()
diff --git a/src/maasserver/tests/test_vmhost.py b/src/maasserver/tests/test_vmhost.py
index cf9ea62..996e3a1 100644
--- a/src/maasserver/tests/test_vmhost.py
+++ b/src/maasserver/tests/test_vmhost.py
@@ -15,6 +15,7 @@ from maasserver.testing.testcase import (
     MAASServerTestCase,
     MAASTransactionServerTestCase,
 )
+from maasserver.utils.orm import reload_object
 from maasserver.utils.threads import deferToDatabase
 from maastesting.crochet import wait_for
 from provisioningserver.drivers.pod import (
@@ -421,6 +422,42 @@ class TestSyncVMCluster(MAASServerTestCase):
         for host in hosts:
             self.assertIn("pod-console-logging", host.tags)
 
+    def test_discovered_vmhosts_doesnt_update_console_logging_tag_if_exists(
+        self,
+    ):
+        tag = factory.make_Tag(
+            name="pod-console-logging", kernel_opts="foo bar"
+        )
+        (
+            discovered_cluster,
+            discovered_racks,
+            failed_racks,
+        ) = fake_cluster_discovery(self)
+        zone = factory.make_Zone()
+        pod_info = make_lxd_pod_info(url=discovered_cluster.pod_addresses[0])
+        power_parameters = {"power_address": pod_info["power_address"]}
+        orig_vmhost = factory.make_Pod(
+            zone=zone, pod_type=pod_info["type"], parameters=power_parameters
+        )
+        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,
+        )
+        vmhost = vmhost_module.discover_and_sync_vmhost(
+            orig_vmhost, factory.make_User()
+        )
+        hosts = vmhost.hints.cluster.hosts()
+        for host in hosts:
+            self.assertIn("pod-console-logging", host.tags)
+        tag = reload_object(tag)
+        self.assertEqual(tag.kernel_opts, "foo bar")
+
     def test_sync_vmcluster_adds_vmhost_zone_and_pool(self):
         (
             discovered_cluster,
diff --git a/src/maasserver/vmhost.py b/src/maasserver/vmhost.py
index 3c50f57..da4e18e 100644
--- a/src/maasserver/vmhost.py
+++ b/src/maasserver/vmhost.py
@@ -27,6 +27,14 @@ from provisioningserver.drivers.pod import DiscoveredCluster, DiscoveredPod
 from provisioningserver.events import EVENT_TYPES
 
 
+def ensure_pod_console_logging_tag() -> Tag:
+    tag, _ = Tag.objects.get_or_create(
+        name="pod-console-logging",
+        defaults={"kernel_opts": "console=tty1 console=ttyS0"},
+    )
+    return tag
+
+
 @inlineCallbacks
 def request_commissioning_results(pod):
     """Request commissioning results from machines associated with the Pod."""
@@ -291,11 +299,7 @@ async def sync_vmcluster_async(discovered_cluster, discovered, vmhost, user):
 
 
 def _update_db(discovered_pod, discovered, vmhost, user, cluster=None):
-    tag, _ = Tag.objects.get_or_create(
-        name="pod-console-logging",
-        kernel_opts="console=tty1 console=ttyS0",
-    )
-    vmhost.add_tag(tag.name)
+    vmhost.add_tag(ensure_pod_console_logging_tag().name)
 
     # If this is a new instance it will be stored in the database at the end of
     # sync.