Merge lp:~freyes/maas-deployer/lp1570218 into lp:~maas-deployers/maas-deployer/next

[Felipe Reyes]

Split 'arch' into 'build_arch' and 'kernel_arch'

This allows to make a clear differentiation what's the expected 'arch'
in libvirt (kernel arch) versus the 'arch' expected by MAAS (build arch).

So users can define the arch in deployment.yaml as 'ppc64el' or 'pp64' and get a functional deployment, as the juju/maas vms will be launched using 'ppc64' and using the ubuntu ppc64el cloud image.

[lp:~freyes/maas-deployer/lp1570218]

79. By Felipe Reyes on 2016-08-01

Mock virsh

[Felipe Reyes (freyes)]

A user with access to a power8 machine is testing this patch (available at https://launchpad.net/~freyes/+archive/ubuntu/lp1570218 )

[Edward Hope-Morley (hopem)]

review: Needs Fixing

[lp:~freyes/maas-deployer/lp1570218]

80. By Felipe Reyes on 2016-08-19

Fix command line parsing

[Felipe Reyes (freyes)]

https://bugs.launchpad.net/maas-deployer/+bug/1570218/comments/6

confirmed that this fixes pc64el

[Billy Olsen (billy-olsen)]

LGTM, thanks Felipe!

review: Approve

=== modified file 'examples/deployment.yaml'
--- examples/deployment.yaml	2016-04-27 12:53:58 +0000
+++ examples/deployment.yaml	2016-08-19 13:25:54 +0000
@@ -8,7 +8,7 @@
         interfaces: ['bridge=virbr0,model=virtio']
         memory: 2048
         vcpus: 2
-        arch: amd64
+        arch: amd64  # build arch
         pool: default
         disk_size: 20G
         sticky_ip_address:
@@ -29,7 +29,7 @@
         interfaces: ['bridge=virbr0,model=virtio']
         memory: 4096
         vcpus: 2
-        arch: amd64
+        arch: amd64  # build arch
         pool: default
         disk_size: 60G
         #release: trusty

=== modified file 'maas_deployer/tests/test_vm.py'
--- maas_deployer/tests/test_vm.py	2015-09-11 10:09:16 +0000
+++ maas_deployer/tests/test_vm.py	2016-08-19 13:25:54 +0000
@@ -3,8 +3,10 @@
 #
 
 import unittest
+
+import mock
+
 from maas_deployer.vmaas import vm
-from mock import patch, MagicMock
 
 
 VIRSH_LIST_ALL = """
@@ -15,15 +17,20 @@
  -     juju-boot-vm-dc1               shut off
  -     fooX                           shut off
 """
+VIRSH_POOL_LIST = """ Name                 State      Autostart 
+-------------------------------------------
+ default              active     yes       
+ uvtool               active     yes       
+"""  # noqa
 
 
 class TestVM(unittest.TestCase):
 
-    @patch.object(vm, 'log', MagicMock())
-    @patch.object(vm.Instance, 'assert_pool_exists', lambda *args: None)
-    @patch.object(vm, 'cfg')
-    @patch.object(vm, 'virsh')
-    @patch.object(vm.libvirt, 'open', lambda *args: None)
+    @mock.patch.object(vm, 'log', mock.MagicMock())
+    @mock.patch.object(vm.Instance, 'assert_pool_exists', lambda *args: None)
+    @mock.patch.object(vm, 'cfg')
+    @mock.patch.object(vm, 'virsh')
+    @mock.patch.object(vm.libvirt, 'open', lambda *args: None)
     def test_instance_domain_exists(self, mock_virsh, mock_cfg):
         mock_cfg.use_existing = False
         mock_cfg.remote = None
@@ -36,3 +43,41 @@
         inst = vm.Instance({})
         self.assertFalse(inst._domain_exists('foo'))
         self.assertTrue(inst._domain_exists('fooX'))
+
+
[e-mail](vm, 'virsh')
[e-mail]('libvirt.open')
+class TestInstance(unittest.TestCase):
+    def setUp(self):
+        self.INSTANCE_PARAMS = {
+            'name': 'foo',
+            'interfaces': ['bridge=virbr0,model=virtio'],
+            'memory': 2048,
+            'vcpus': 2,
+            'arch': 'amd64',
+            'pool': 'default',
+            'disk_size': '20G',
+        }
+        setattr(vm.cfg, 'remote', 'qemu:///system')
+
+    def test_instance_arch(self, libvirt_open, virsh):
+        virsh.return_value = (VIRSH_POOL_LIST, 0)
+        instance = vm.Instance(self.INSTANCE_PARAMS)
+
+        self.assertEqual(instance.kernel_arch, 'x86_64')
+
+    def test_instance_pp64(self, libvirt_open, virsh):
+        virsh.return_value = (VIRSH_POOL_LIST, 0)
+        self.INSTANCE_PARAMS['arch'] = 'ppc64'
+        instance = vm.Instance(self.INSTANCE_PARAMS)
+
+        self.assertEqual(instance.build_arch, 'ppc64el')
+        self.assertEqual(instance.kernel_arch, 'ppc64')
+
+    def test_instance_pp64el(self, libvirt_open, virsh):
+        virsh.return_value = (VIRSH_POOL_LIST, 0)
+        self.INSTANCE_PARAMS['arch'] = 'ppc64el'
+        instance = vm.Instance(self.INSTANCE_PARAMS)
+
+        self.assertEqual(instance.build_arch, 'ppc64el')
+        self.assertEqual(instance.kernel_arch, 'ppc64')

=== modified file 'maas_deployer/vmaas/engine.py'
--- maas_deployer/vmaas/engine.py	2016-08-01 20:02:22 +0000
+++ maas_deployer/vmaas/engine.py	2016-08-19 13:25:54 +0000
@@ -88,7 +88,7 @@
         """
         node = {
             'name': juju_domain.name,
-            'architecture': juju_config.get('arch'),
+            'architecture': juju_domain.build_arch,
             'mac_addresses': [x for x in juju_domain.mac_addresses],
             'tags': 'bootstrap'
         }

=== modified file 'maas_deployer/vmaas/templates/cloud-init.cfg'
--- maas_deployer/vmaas/templates/cloud-init.cfg	2016-03-31 17:31:45 +0000
+++ maas_deployer/vmaas/templates/cloud-init.cfg	2016-08-19 13:25:54 +0000
@@ -44,7 +44,7 @@
   # Used to manage KVM instances via libvirt
   - libvirt-bin
   # Below package does not exist on Power8 architecture.
-{%- if arch != 'ppc64' %}
+{%- if arch != 'ppc64el' %}
   - linux-image-extra-virtual
 {% endif %}
   - jq

=== modified file 'maas_deployer/vmaas/util.py'
--- maas_deployer/vmaas/util.py	2016-04-27 19:41:53 +0000
+++ maas_deployer/vmaas/util.py	2016-08-19 13:25:54 +0000
@@ -9,6 +9,7 @@
 import logging
 import os
 import subprocess
+import sys
 import time
 
 log = logging.getLogger('vmaas.main')
@@ -202,8 +203,8 @@
         raise AttributeError("%r object has no attribute %r" %
                              (self.__class__, attr))
 
-    def parse_args(self):
-        self._args = self._parser.parse_args()
+    def parse_args(self, argv=None):
+        self._args = self._parser.parse_args(argv)
 
     @property
     def args(self):

=== modified file 'maas_deployer/vmaas/vm.py'
--- maas_deployer/vmaas/vm.py	2016-04-27 19:41:53 +0000
+++ maas_deployer/vmaas/vm.py	2016-08-19 13:25:54 +0000
@@ -34,6 +34,8 @@
 
 
 log = logging.getLogger('vmaas.main')
+ARCH_BUILD_TO_KERNEL = {'ppc64el': 'ppc64',
+                        'amd64': 'x86_64'}
 
 
 class Instance(object):
@@ -41,7 +43,17 @@
     def __init__(self, params, autostart=False):
         self.name = params.get('name')
         self.interfaces = params.get('interfaces')
-        self.arch = params.get('arch', 'amd64')
+        self.build_arch = params.get('arch', 'amd64')
+
+        if self.build_arch == 'ppc64':
+            # see LP: #1570218
+            log.warning(("`arch' should be the build arch instead of the "
+                         "architecture expected by the kernel. Using `ppc64el'"
+                         "instead of `ppc64'"))
+            self.build_arch = 'ppc64el'
+
+        self.kernel_arch = ARCH_BUILD_TO_KERNEL.get(self.build_arch,
+                                                    self.build_arch)
         self.disk_size = params.get('disk_size', '20G')
         self.vcpus = params.get('vcpus', 1)
         self.memory = params.get('memory', 1024)
@@ -128,18 +140,13 @@
                  create the domain.
         """
 
-        # NOTE: virt-install appears to prefer x86_64 over amd64
-        arch = self.arch
-        if arch == 'amd64':
-            arch = 'x86_64'
-
         cmd = ['virt-install',
                '--connect', cfg.remote,
                '--name', self.name,
                '--ram', str(self.memory),
                '--vcpus', str(self.vcpus),
                '--video', str(self.video),
-               '--arch', str(arch)]
+               '--arch', str(self.kernel_arch)]
 
         for disk in self._get_disks():
             cmd.extend(['--disk', disk])
@@ -298,7 +305,6 @@
         super(CloudInstance, self).__init__(params, autostart)
         self.name = params.get('name')
         self.interfaces = params.get('interfaces')
-        self.arch = params.get('arch', 'amd64')
         self.disk_size = params.get('disk_size', '40G')
         self.vcpus = params.get('vcpus', 2)
         self.memory = params.get('memory', 4096)
@@ -316,14 +322,10 @@
         Returns a tuple with the cloud-image url and the file it
         should be saved as.
         """
-        if self.arch == "ppc64":
-            url = ('https://cloud-images.ubuntu.com/{release}/current/'
-                   '{release}-server-cloudimg-{arch}el-disk1.img')
-        else:
-            url = ('https://cloud-images.ubuntu.com/{release}/current/'
-                   '{release}-server-cloudimg-{arch}-disk1.img')
+        url = ('https://cloud-images.ubuntu.com/{release}/current/'
+               '{release}-server-cloudimg-{arch}-disk1.img')
 
-        url = url.format(release=self.release, arch=self.arch)
+        url = url.format(release=self.release, arch=self.build_arch)
         f = url.split('/')[-1]
         return (url, f)
 
@@ -394,7 +396,7 @@
         """
         storage_pool = self.conn.storagePoolLookupByName(self.pool)
         existing_vols = [v.name() for v in storage_pool.listAllVolumes()]
-        basevol = "%s-%s-base" % (self.release, self.arch)
+        basevol = "%s-%s-base" % (self.release, self.build_arch)
         self._create_base_volume(basevol, existing_vols)
         root_img_name = '{}-root.img'.format(self.name)
         self._create_root_volume(root_img_name, basevol, existing_vols,
@@ -457,7 +459,7 @@
         etc_net_interfaces = ["%s%s" % (indent, l.rstrip()) for l in
                               etc_net_interfaces]
 
-        parms = {
+        params = {
             'hostname': self.name,
             'user': self.user,
             'password': self.password,
@@ -465,21 +467,21 @@
             'apt_http_proxy': self.apt_http_proxy,
             'apt_sources': self.apt_sources,
             'network_config': '\n'.join(etc_net_interfaces),
-            'arch': self.arch
+            'arch': self.build_arch
         }
-        content = template.load('cloud-init.cfg', parms)
+        content = template.load('cloud-init.cfg', params)
         with open(base_file, 'w+') as f:
             f.write(content)
             f.flush()
 
         # Generate the script file...
         config_maas_script = os.path.join(self.working_dir, 'config-maas.sh')
-        parms = {
+        params = {
             'user': self.user,
             'password': self.password,
             'node_group_ifaces': self.node_group_ifaces,
         }
-        content = template.load('config-maas.sh', parms)
+        content = template.load('config-maas.sh', params)
         with open(config_maas_script, 'w+') as f:
             f.write(content)
             f.flush()