Merge ~mpontillo/maas:ppc64-disable-smt-for-pod-deploy--bug-1797484 into maas:master

[Mike Pontillo]

LP: #1797484 - Support for PPC64 pod deployment

 * Add rc.local to disable SMT for ppc64el KVM pod deployments.
 * Fix UI pod deployment path to bridge all interfaces.

It's not really clear if this is the most correct change; I wonder about race conditions. We should investigate to see if adding a tag with `nosmt=force` is a better way to do this.

It's also not really clear if we need to restart libvirt in the rc file; we might want to drop that, too. Thoughts?

[MAAS Lander (maas-lander)]

UNIT TESTS
-b ppc64-disable-smt-for-pod-deploy--bug-1797484 lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 8965d5b463c71522dbc8e954726e3cc228b6a251

review: Approve

[Lee Trager (ltrager)]

Do you know why SMT needs to be disabled? I found this blog post from IBM saying it should be supported. [1]

Since this is for systemd based systems I think we should create a systemd unit in /etc/systemd/user. Something like this should work and will ensure smt is turned off before libvirtd starts so no restart is needed.

[Unit]
Description=Ensures SMT is disabled for KVM

[Service]
ExecStart=ppc64_cpu --smt=off
Before=libvirtd

[Install]
WantedBy=multi-user.target

[1] https://www.ibm.com/developerworks/community/blogs/fe313521-2e95-46f2-817d-44a4f27eba32/entry/enabling_smt_on_powerkvm_guests?lang=en_us

review: Needs Information

[Mike Pontillo (mpontillo)]

The article you linked to states that SMT can be enabled on /guests/, if I understand it correctly. This is for deploying the hypervisor. That said, I don't know specifically why SMT needs to be disabled on the hypervisor. I was asking about this on IRC last week, and heard from @rharper: "on certain levels of the power hardware, the kvm hypervisor cannot run the threads one the host which may be given to guests; (rather it ends up running poorly, so disabling the threads is needed)".

I like your systemd unit idea; thanks. I feel like the kernel parameter still might be better, but I don't like the idea of the manual tag management that would come with implementing it that way.

[Mike Pontillo (mpontillo)]

In other words, if I parsed what @rharper said correctly, by disabling SMT on the host, you ensure it's fully available on the guests?

ac39c3e... by Mike Pontillo on 2018-10-15

Fix end quote in cloud-config for PPC64.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b ppc64-disable-smt-for-pod-deploy--bug-1797484 lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/4199/console
COMMIT: 176915594810294556db88020ac1f83aa870962c

review: Needs Fixing

72c4d09... by Mike Pontillo on 2018-10-16

Fix test.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b ppc64-disable-smt-for-pod-deploy--bug-1797484 lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: e90dab3294e16885424b94bfe9d1c3f540caa949

review: Approve

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

2be7e2f... by Mike Pontillo on 2018-10-16

Use bridge_all when deploying from the UI for better UX.

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

diff --git a/src/maasserver/node_action.py b/src/maasserver/node_action.py
index f299700..edfdfe7 100644
--- a/src/maasserver/node_action.py
+++ b/src/maasserver/node_action.py
@@ -468,7 +468,9 @@ class Deploy(NodeAction):
         if self.node.owner is None:
             with locks.node_acquire:
                 try:
-                    self.node.acquire(self.user, token=None)
+                    bridge_all = True if install_kvm else False
+                    self.node.acquire(
+                        self.user, token=None, bridge_all=bridge_all)
                 except ValidationError as e:
                     raise NodeActionError(e)
         if install_kvm:
diff --git a/src/maasserver/tests/test_node_action.py b/src/maasserver/tests/test_node_action.py
index 4cf029e..ca37761 100644
--- a/src/maasserver/tests/test_node_action.py
+++ b/src/maasserver/tests/test_node_action.py
@@ -699,11 +699,12 @@ class TestDeployAction(MAASServerTestCase):
         request = factory.make_fake_request('/')
         request.user = user
         node = factory.make_Node(
-            interface=True, status=NODE_STATUS.ALLOCATED,
-            power_type='manual', owner=user)
+            interface=True, status=NODE_STATUS.READY,
+            power_type='manual', owner=None)
         mock_get_curtin_config = self.patch(
             node_action_module, 'get_curtin_config')
         mock_node_start = self.patch(node, 'start')
+        mock_node_acquire = self.patch(node, 'acquire')
         mock_validate_os = self.patch(
             node_action_module, 'validate_osystem_and_distro_series')
         mock_validate_os.side_effect = lambda os, release: (os, release)
@@ -715,6 +716,9 @@ class TestDeployAction(MAASServerTestCase):
         }
         Deploy(node, user, request).execute(**extra)
         self.expectThat(mock_get_curtin_config, MockCalledOnceWith(ANY, node))
+        # Make sure we set bridge_all when we acquire the Node.
+        self.expectThat(mock_node_acquire, MockCalledOnceWith(
+            user, token=None, bridge_all=True))
         self.expectThat(mock_node_start, MockCalledOnceWith(user))
         # Make sure ubuntu/bionic is selected if KVM pod deployment has been
         # selected.
diff --git a/src/metadataserver/tests/test_vendor_data.py b/src/metadataserver/tests/test_vendor_data.py
index 341717a..6f85f86 100644
--- a/src/metadataserver/tests/test_vendor_data.py
+++ b/src/metadataserver/tests/test_vendor_data.py
@@ -250,3 +250,21 @@ class TestGenerateRackControllerConfiguration(MAASServerTestCase):
         configuration = get_vendor_data(node)
         config = dict(configuration)
         self.assertThat(config['packages'], Contains("qemu-efi"))
+
+    def test_includes_smt_off_for_install_kvm_on_ppc64(self):
+        node = factory.make_Node(
+            osystem='ubuntu', netboot=False, architecture='ppc64el/generic')
+        node.install_kvm = True
+        configuration = get_vendor_data(node)
+        config = dict(configuration)
+        self.assertThat(
+            config['runcmd'], Contains([
+                'sh', '-c', 'printf "'
+                '#!/bin/sh\\n'
+                'ppc64_cpu --smt=off\\n'
+                'exit 0\\n'
+                '"  >> /etc/rc.local'
+            ]))
+        self.assertThat(
+            config['runcmd'], Contains(['chmod', '+x', '/etc/rc.local']))
+        self.assertThat(config['runcmd'], Contains(['/etc/rc.local']))
diff --git a/src/metadataserver/vendor_data.py b/src/metadataserver/vendor_data.py
index a7d942e..26810d3 100644
--- a/src/metadataserver/vendor_data.py
+++ b/src/metadataserver/vendor_data.py
@@ -103,7 +103,12 @@ def generate_rack_controller_configuration(node):
 def generate_kvm_pod_configuration(node):
     """Generate cloud-init configuration to install the node as a KVM pod."""
     if node.netboot is False and node.install_kvm is True:
-        yield "runcmd", [
+        architecture = None
+        if node.architecture is not None:
+            architecture = node.architecture
+            if '/' in architecture:
+                architecture = architecture.split('/')[0]
+        runcmd = [
             # Restrict the $PATH so that rbash can be used to limit what the
             # virsh user can do if they manage to get a shell.
             ['mkdir', '-p', '/home/virsh/bin'],
@@ -127,6 +132,21 @@ def generate_kvm_pod_configuration(node):
             # Ensure services are ready before cloud-init finishes.
             ['/bin/sleep', '10'],
         ]
+        if architecture == 'ppc64el':
+            # XXX mpontillo 2018-10-12 - we should investigate if it might be
+            # better to add a tag to the node that includes a kernel parameter
+            # such as nosmt=force. (The only problem being that we should
+            # probably also remove it after the machine is released.)
+            runcmd.append([
+                'sh', '-c', 'printf "'
+                '#!/bin/sh\\n'
+                'ppc64_cpu --smt=off\\n'
+                'exit 0\\n'
+                '"  >> /etc/rc.local'
+            ])
+            runcmd.append(['chmod', '+x', '/etc/rc.local'])
+            runcmd.append(['/etc/rc.local'])
+        yield "runcmd", runcmd
         # Generate a 32-character password by encoding 24 bytes as base64.
         virsh_password = b64encode(
             urandom(24), altchars=b'.!').decode('ascii')
@@ -150,10 +170,6 @@ def generate_kvm_pod_configuration(node):
             }
         ]
         packages = ["qemu-kvm", "libvirt-bin"]
-        if node.architecture is not None:
-            architecture = node.architecture
-            if '/' in architecture:
-                architecture = architecture.split('/')[0]
-            if architecture == 'amd64':
-                packages.append('qemu-efi')
+        if architecture == 'amd64':
+            packages.append('qemu-efi')
         yield "packages", packages