Merge lp:~ltrager/maas/lp1587896 into lp:~maas-committers/maas/trunk

[Lee Trager]

Don't gather swap information on controllers and only get the architecture, not the subarchitecture from archdetect.

[Gavin Panella (allenap)]

Seems good, but I want Blake to take a look at the architecture fix before you land it.

review: Approve

[Lee Trager (ltrager)]

The architecture fix was actually pointed out by Blake. archdetect outputs as amd64/efi on UEFI machines, I didn't notice this as I've been testing with libvirt which emulates a traditional BIOS. This modifies refreshes to mimic commissioning and only take the architecture part from archdetect, not the subarch.

http://bazaar.launchpad.net/~maas-committers/maas/trunk/view/head:/etc/maas/templates/commissioning-user-data/snippets/maas_enlist.sh#L72

[Blake Rouse (blake-rouse)]

review: Needs Fixing

[Lee Trager (ltrager)]

Updated to properly handle the subarch.

[Blake Rouse (blake-rouse)]

Thanks for the fix. Looks good!

review: Approve

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2016-06-06 18:28:04 +0000
+++ src/maasserver/models/node.py	2016-06-06 19:15:31 +0000
@@ -3641,8 +3641,6 @@
             self.osystem = response['osystem']
         if response['distro_series'] != '':
             self.distro_series = response['distro_series']
-        if response['swap_size'] > 0:
-            self.swap_size = response['swap_size']
         self.update_interfaces(response['interfaces'])
         self.save()
 

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2016-06-06 18:28:04 +0000
+++ src/maasserver/models/tests/test_node.py	2016-06-06 19:15:31 +0000
@@ -6908,7 +6908,6 @@
             'architecture': rackcontroller.architecture,
             'osystem': '',
             'distro_series': '',
-            'swap_size': 0,
             'interfaces': {},
         })
 
@@ -6934,7 +6933,6 @@
             'architecture': rackcontroller.architecture,
             'osystem': '',
             'distro_series': '',
-            'swap_size': 0,
             'interfaces': {},
         })
 
@@ -6951,7 +6949,6 @@
         rackcontroller = factory.make_RackController(status=NODE_STATUS.NEW)
         osystem = factory.make_name('osystem')
         distro_series = factory.make_name('distro_series')
-        swap_size = random.randint(1, 1024)
         interfaces = {
             "eth0": {
                 "type": "physical",
@@ -6971,7 +6968,6 @@
             'architecture': rackcontroller.architecture,
             'osystem': osystem,
             'distro_series': distro_series,
-            'swap_size': swap_size,
             'interfaces': interfaces
         })
 
@@ -6979,7 +6975,6 @@
         rackcontroller = reload_object(rackcontroller)
         self.assertEquals(osystem, rackcontroller.osystem)
         self.assertEquals(distro_series, rackcontroller.distro_series)
-        self.assertEquals(swap_size, rackcontroller.swap_size)
         self.assertIsNotNone(
             Interface.objects.filter(
                 node=rackcontroller,

=== modified file 'src/provisioningserver/refresh/__init__.py'
--- src/provisioningserver/refresh/__init__.py	2016-05-31 17:03:01 +0000
+++ src/provisioningserver/refresh/__init__.py	2016-06-06 19:15:31 +0000
@@ -16,6 +16,10 @@
     geturl,
 )
 from provisioningserver.refresh.node_info_scripts import NODE_INFO_SCRIPTS
+from provisioningserver.utils.shell import (
+    call_and_check,
+    ExternalProcessError,
+)
 from provisioningserver.utils.twisted import synchronous
 
 
@@ -24,20 +28,14 @@
 
 def get_architecture():
     """Get the architecture of the running system."""
-    proc = subprocess.Popen('archdetect', stdout=subprocess.PIPE)
-    stdout, stderr = proc.communicate()
-    if proc.returncode != 0:
+    try:
+        stdout = call_and_check('archdetect')
+    except ExternalProcessError:
         return ''
-    return stdout.strip().decode('utf-8')
-
-
-def get_swap_size():
-    """Get the current swap size of the running system."""
-    with open('/proc/meminfo') as f:
-        for line in f:
-            if 'SwapTotal' in line:
-                return int(line.split()[1]) * 1000
-    return 0
+    arch, subarch = stdout.strip().split('/')
+    if arch in ['i386', 'amd64', 'arm64', 'ppc64el']:
+        subarch = 'generic'
+    return '%s/%s' % (arch, subarch)
 
 
 def get_os_release():

=== modified file 'src/provisioningserver/refresh/tests/test_refresh.py'
--- src/provisioningserver/refresh/tests/test_refresh.py	2016-05-31 17:03:01 +0000
+++ src/provisioningserver/refresh/tests/test_refresh.py	2016-06-06 19:15:31 +0000
@@ -9,7 +9,6 @@
 import os
 from pathlib import Path
 import random
-import re
 import tempfile
 from textwrap import dedent
 from unittest.mock import sentinel
@@ -29,22 +28,21 @@
 
 
 class TestHelpers(MAASTestCase):
-    def test_get_architecture_returns_arch(self):
-        architecture = refresh.get_architecture()
-        self.assertIsInstance(architecture, str)
-        self.assertIsNot('', architecture)
+    def test_get_architecture_returns_arch_with_generic(self):
+        arch = random.choice(['i386', 'amd64', 'arm64', 'ppc64el'])
+        subarch = factory.make_name('subarch')
+        self.patch(refresh, 'call_and_check').return_value = (
+            "%s/%s" % (arch, subarch))
+        ret_arch = refresh.get_architecture()
+        self.assertEquals("%s/generic" % arch, ret_arch)
 
-    def test_get_swap_size_proc_meminfo_exists(self):
-        # This is a canary incase /proc/meminfo ever goes away
-        self.assertTrue(os.path.exists('/proc/meminfo'))
-        # Test that /proc/meminfo still provides SwapTotal
-        regex = re.compile('^SwapTotal:\s+[0-9]+ kB\n$')
-        with open('/proc/meminfo') as f:
-            for line in f:
-                result = regex.match(line)
-                if result is not None:
-                    return
-        self.assertTrue(False, '/proc/meminfo does not contain SwapTotal')
+    def test_get_architecture_returns_arch_with_subarch(self):
+        arch = factory.make_name('arch')
+        subarch = factory.make_name('subarch')
+        architecture = "%s/%s" % (arch, subarch)
+        self.patch(refresh, 'call_and_check').return_value = architecture
+        ret_arch = refresh.get_architecture()
+        self.assertEquals(architecture, ret_arch)
 
     def test_get_os_release_etc_os_release_exists(self):
         # This is a canary incase /etc/os-release ever goes away

=== modified file 'src/provisioningserver/rpc/cluster.py'
--- src/provisioningserver/rpc/cluster.py	2016-05-12 19:07:37 +0000
+++ src/provisioningserver/rpc/cluster.py	2016-06-06 19:15:31 +0000
@@ -453,7 +453,6 @@
         (b"architecture", amp.Unicode()),
         (b"osystem", amp.Unicode()),
         (b"distro_series", amp.Unicode()),
-        (b"swap_size", amp.Integer()),
         (b"interfaces", StructureAsJSON()),
     ]
     errors = {}

=== modified file 'src/provisioningserver/rpc/clusterservice.py'
--- src/provisioningserver/rpc/clusterservice.py	2016-05-12 19:07:37 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2016-06-06 19:15:31 +0000
@@ -39,7 +39,6 @@
 from provisioningserver.refresh import (
     get_architecture,
     get_os_release,
-    get_swap_size,
     refresh,
 )
 from provisioningserver.rpc import (
@@ -385,12 +384,11 @@
         def perform_refresh():
             architecture = get_architecture()
             os_release = get_os_release()
-            swap_size = get_swap_size()
             interfaces, _ = get_interfaces_definition()
-            return architecture, os_release, swap_size, interfaces
+            return architecture, os_release, interfaces
 
         def cb_result(result):
-            architecture, os_release, swap_size, interfaces = result
+            architecture, os_release, interfaces = result
             if 'ID' in os_release:
                 osystem = os_release['ID']
             elif 'NAME' in os_release:
@@ -407,7 +405,6 @@
                 'architecture': architecture,
                 'osystem': osystem,
                 'distro_series': distro_series,
-                'swap_size': swap_size,
                 'interfaces': interfaces
             }
 

=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2016-05-13 15:14:05 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2016-06-06 19:15:31 +0000
@@ -1956,7 +1956,7 @@
             clusterservice, 'deferToThread')
         mock_deferToThread.side_effect = [
             succeed(None),
-            succeed(('', {}, 0, {})),
+            succeed(('', {}, {})),
         ]
 
         system_id = factory.make_name('system_id')
@@ -1987,7 +1987,6 @@
         architecture = factory.make_name("architecture")
         osystem = factory.make_name("osystem")
         distro_series = factory.make_name("distro_series")
-        swap_size = random.randint(1, 100)
         os_release = {
             'ID': osystem,
             'UBUNTU_CODENAME': distro_series,
@@ -1996,7 +1995,6 @@
         self.patch(clusterservice, 'get_architecture').return_value = (
             architecture)
         self.patch(clusterservice, 'get_os_release').return_value = os_release
-        self.patch(clusterservice, 'get_swap_size').return_value = swap_size
 
         response = yield call_responder(
             Cluster(), cluster.RefreshRackControllerInfo, {
@@ -2010,7 +2008,6 @@
             {
                 'osystem': osystem,
                 'distro_series': distro_series,
-                'swap_size': swap_size,
                 'architecture': architecture,
                 'interfaces': get_all_interfaces_definition(),
             }, response)