Merge Cray/buildbot:openstack-block-device (PR buildbot#1221)

Conflicts:
	master/docs/manual/cfg-buildslaves.rst (merged to cfg-buildslaves-openstack.rst)

+autopep8

master/buildbot/buildslave/openstack.py

@@ -46,37 +46,73 @@ class OpenStackLatentBuildSlave(AbstractLatentBuildSlave):
 
     def __init__(self, name, password,
                  flavor,
-                 image,
                  os_username,
                  os_password,
                  os_tenant_name,
                  os_auth_url,
+                 block_devices=None,
+                 image=None,
                  meta=None,
                  max_builds=None, notify_on_missing=[], missing_timeout=60 * 20,
-                 build_wait_timeout=60 * 10, properties={}, locks=None):
+                 build_wait_timeout=60 * 10, properties={}, locks=None,
+                 # Have a nova_args parameter to allow passing things directly
+                 # to novaclient v1.1.
+                 nova_args=None):
 
         if not client or not nce:
             config.error("The python module 'novaclient' is needed  "
                          "to use a OpenStackLatentBuildSlave")
 
+        if not block_devices and not image:
+            raise ValueError('One of block_devices or image must be given')
+
         AbstractLatentBuildSlave.__init__(
             self, name, password, max_builds, notify_on_missing,
             missing_timeout, build_wait_timeout, properties, locks)
         self.flavor = flavor
-        self.image = image
         self.os_username = os_username
         self.os_password = os_password
         self.os_tenant_name = os_tenant_name
         self.os_auth_url = os_auth_url
+        if block_devices is not None:
+            self.block_devices = [self._parseBlockDevice(bd) for bd in block_devices]
+        else:
+            self.block_devices = None
+        self.image = image
         self.meta = meta
-
-    def _getImage(self, os_client):
-        # If self.image is a callable, then pass it the list of images. The
+        self.nova_args = nova_args if nova_args is not None else {}
+
+    def _parseBlockDevice(self, block_device):
+        """
+        Parse a higher-level view of the block device mapping into something
+        novaclient wants. This should be similar to how Horizon presents it.
+        Required keys:
+            device_name: The name of the device; e.g. vda or xda.
+            source_type: image, snapshot, volume, or blank/None.
+            destination_type: Destination of block device: volume or local.
+            delete_on_termination: True/False.
+            uuid: The image, snapshot, or volume id.
+            boot_index: Integer used for boot order.
+            volume_size: Size of the device in GiB.
+        """
+        client_block_device = {}
+        client_block_device['device_name'] = block_device.get('device_name', 'vda')
+        client_block_device['source_type'] = block_device.get('source_type', 'image')
+        client_block_device['destination_type'] = block_device.get('destination_type', 'volume')
+        client_block_device['delete_on_termination'] = bool(block_device.get('delete_on_termination', True))
+        client_block_device['uuid'] = block_device['uuid']
+        client_block_device['boot_index'] = int(block_device.get('boot_index', 0))
+        client_block_device['volume_size'] = block_device['volume_size']
+        return client_block_device
+
+    @staticmethod
+    def _getImage(os_client, image):
+        # If image is a callable, then pass it the list of images. The
         # function should return the image's UUID to use.
-        if callable(self.image):
-            image_uuid = self.image(os_client.images.list())
+        if callable(image):
+            image_uuid = image(os_client.images.list())
         else:
-            image_uuid = self.image
+            image_uuid = image
         return image_uuid
 
     def start_instance(self, build):
@@ -88,12 +124,12 @@ def _start_instance(self):
         # Authenticate to OpenStack.
         os_client = client.Client(self.os_username, self.os_password,
                                   self.os_tenant_name, self.os_auth_url)
-        image_uuid = self._getImage(os_client)
-        flavor_id = self.flavor
-        boot_args = [self.slavename, image_uuid, flavor_id]
-        boot_kwargs = {}
-        if self.meta is not None:
-            boot_kwargs['meta'] = self.meta
+        image_uuid = self._getImage(os_client, self.image)
+        boot_args = [self.slavename, image_uuid, self.flavor]
+        boot_kwargs = dict(
+            meta=self.meta,
+            block_device_mapping_v2=self.block_devices,
+            **self.nova_args)
         instance = os_client.servers.create(*boot_args, **boot_kwargs)
         self.instance = instance
         log.msg('%s %s starting instance %s (image %s)' %

master/buildbot/test/fake/openstack.py

@@ -42,9 +42,7 @@ class Servers():
     fail_to_get = False
     fail_to_start = False
     gets_until_active = 2
-
-    def __init__(self):
-        self.instances = {}
+    instances = {}
 
     def create(self, *boot_args, **boot_kwargs):
         instance_id = uuid.uuid4()
@@ -55,6 +53,8 @@ def create(self, *boot_args, **boot_kwargs):
     def get(self, instance_id):
         if not self.fail_to_get and instance_id in self.instances:
             inst = self.instances[instance_id]
+            if not inst.status.startswith('BUILD'):
+                return inst
             inst.gets += 1
             if inst.gets >= self.gets_until_active:
                 if not self.fail_to_start:

master/buildbot/test/unit/test_buildslave_openstack.py

@@ -20,10 +20,20 @@
 from buildbot import config
 from buildbot import interfaces
 from buildbot.buildslave import openstack
+from twisted.internet import defer
 from twisted.trial import unittest
 
 
 class TestOpenStackBuildSlave(unittest.TestCase):
+    os_auth = dict(
+        os_username='user',
+        os_password='pass',
+        os_tenant_name='tenant',
+        os_auth_url='auth')
+    bs_image_args = dict(
+        flavor=1,
+        image='image-uuid',
+        **os_auth)
 
     def setUp(self):
         self.patch(openstack, "nce", novaclient)
@@ -33,69 +43,76 @@ def test_constructor_nonova(self):
         self.patch(openstack, "nce", None)
         self.patch(openstack, "client", None)
         self.assertRaises(config.ConfigErrors,
-                          openstack.OpenStackLatentBuildSlave, 'bot', 'pass', flavor=1,
-                          image='image', os_username='user', os_password='pass',
-                          os_tenant_name='tenant', os_auth_url='auth')
+                          openstack.OpenStackLatentBuildSlave, 'bot', 'pass',
+                          **self.bs_image_args)
 
     def test_constructor_minimal(self):
-        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', flavor=1,
-                                                 image='image', os_username='user', os_password='pass',
-                                                 os_tenant_name='tenant', os_auth_url='auth')
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
         self.assertEqual(bs.slavename, 'bot')
         self.assertEqual(bs.password, 'pass')
         self.assertEqual(bs.flavor, 1)
-        self.assertEqual(bs.image, 'image')
+        self.assertEqual(bs.image, 'image-uuid')
+        self.assertEqual(bs.block_devices, None)
         self.assertEqual(bs.os_username, 'user')
         self.assertEqual(bs.os_password, 'pass')
         self.assertEqual(bs.os_tenant_name, 'tenant')
         self.assertEqual(bs.os_auth_url, 'auth')
 
-    def test_getImage_string(self):
+    def test_constructor_block_devices_default(self):
+        block_devices = [{'uuid': 'uuid', 'volume_size': 10}]
         bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', flavor=1,
-                                                 image='image-uuid', os_username='user', os_password='pass',
-                                                 os_tenant_name='tenant', os_auth_url='auth')
-        self.assertEqual('image-uuid', bs._getImage(None))
+                                                 block_devices=block_devices,
+                                                 **self.os_auth)
+        self.assertEqual(bs.image, None)
+        self.assertEqual(len(bs.block_devices), 1)
+        self.assertEqual(bs.block_devices, [{'boot_index': 0,
+                                             'delete_on_termination': True,
+                                             'destination_type': 'volume', 'device_name': 'vda',
+                                             'source_type': 'image', 'volume_size': 10, 'uuid': 'uuid'}])
+
+    def test_constructor_no_image(self):
+        """
+        Must have one of image or block_devices specified.
+        """
+        self.assertRaises(ValueError,
+                          openstack.OpenStackLatentBuildSlave, 'bot', 'pass',
+                          flavor=1, **self.os_auth)
+
+    def test_getImage_string(self):
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
+        self.assertEqual('image-uuid', bs._getImage(None, bs.image))
 
     def test_getImage_callable(self):
         def image_callable(images):
             return images[0]
 
         bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', flavor=1,
-                                                 image=image_callable, os_username='user', os_password='pass',
-                                                 os_tenant_name='tenant', os_auth_url='auth')
+                                                 image=image_callable, **self.os_auth)
         os_client = novaclient.Client('user', 'pass', 'tenant', 'auth')
         os_client.images.images = ['uuid1', 'uuid2', 'uuid2']
-        self.assertEqual('uuid1', bs._getImage(os_client))
+        self.assertEqual('uuid1', bs._getImage(os_client, image_callable))
 
     def test_start_instance_already_exists(self):
-        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', flavor=1,
-                                                 image='image-uuid', os_username='user', os_password='pass',
-                                                 os_tenant_name='tenant', os_auth_url='auth')
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
         bs.instance = mock.Mock()
         self.assertRaises(ValueError, bs.start_instance, None)
 
     def test_start_instance_fail_to_find(self):
-        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', flavor=1,
-                                                 image='image-uuid', os_username='user', os_password='pass',
-                                                 os_tenant_name='tenant', os_auth_url='auth')
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
         bs._poll_resolution = 0
         self.patch(novaclient.Servers, 'fail_to_get', True)
         self.assertRaises(interfaces.LatentBuildSlaveFailedToSubstantiate,
                           bs._start_instance)
 
     def test_start_instance_fail_to_start(self):
-        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', flavor=1,
-                                                 image='image-uuid', os_username='user', os_password='pass',
-                                                 os_tenant_name='tenant', os_auth_url='auth')
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
         bs._poll_resolution = 0
         self.patch(novaclient.Servers, 'fail_to_start', True)
         self.assertRaises(interfaces.LatentBuildSlaveFailedToSubstantiate,
                           bs._start_instance)
 
     def test_start_instance_success(self):
-        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', flavor=1,
-                                                 image='image-uuid', os_username='user', os_password='pass',
-                                                 os_tenant_name='tenant', os_auth_url='auth')
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
         bs._poll_resolution = 0
         uuid, image_uuid, time_waiting = bs._start_instance()
         self.assertTrue(uuid)
@@ -104,10 +121,60 @@ def test_start_instance_success(self):
 
     def test_start_instance_check_meta(self):
         meta_arg = {'some_key': 'some-value'}
-        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', flavor=1,
-                                                 image='image-uuid', os_username='user', os_password='pass',
-                                                 os_tenant_name='tenant', os_auth_url='auth', meta=meta_arg)
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', meta=meta_arg,
+                                                 **self.bs_image_args)
         bs._poll_resolution = 0
         uuid, image_uuid, time_waiting = bs._start_instance()
         self.assertIn('meta', bs.instance.boot_kwargs)
         self.assertIdentical(bs.instance.boot_kwargs['meta'], meta_arg)
+
+    @defer.inlineCallbacks
+    def test_stop_instance_not_set(self):
+        """
+        Test stopping the instance but with no instance to stop.
+        """
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
+        bs.instance = None
+        stopped = yield bs.stop_instance()
+        self.assertEqual(stopped, None)
+
+    def test_stop_instance_missing(self):
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
+        instance = mock.Mock()
+        instance.id = 'uuid'
+        bs.instance = instance
+        # TODO: Check log for instance not found.
+        bs.stop_instance()
+
+    def test_stop_instance_fast(self):
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
+        # Make instance immediately active.
+        self.patch(novaclient.Servers, 'gets_until_active', 0)
+        s = novaclient.Servers()
+        bs.instance = inst = s.create()
+        self.assertIn(inst.id, s.instances)
+        bs.stop_instance(fast=True)
+        self.assertNotIn(inst.id, s.instances)
+
+    def test_stop_instance_notfast(self):
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
+        # Make instance immediately active.
+        self.patch(novaclient.Servers, 'gets_until_active', 0)
+        s = novaclient.Servers()
+        bs.instance = inst = s.create()
+        self.assertIn(inst.id, s.instances)
+        bs.stop_instance(fast=False)
+        self.assertNotIn(inst.id, s.instances)
+
+    def test_stop_instance_unknown(self):
+        bs = openstack.OpenStackLatentBuildSlave('bot', 'pass', **self.bs_image_args)
+        # Make instance immediately active.
+        self.patch(novaclient.Servers, 'gets_until_active', 0)
+        s = novaclient.Servers()
+        bs.instance = inst = s.create()
+        # Set status to DELETED. Instance should not be deleted when shutting
+        # down as it already is.
+        inst.status = novaclient.DELETED
+        self.assertIn(inst.id, s.instances)
+        bs.stop_instance()
+        self.assertIn(inst.id, s.instances)

master/docs/manual/cfg-buildslaves-openstack.rst

@@ -58,10 +58,44 @@ These are the same details set in either environment variables or passed as opti
     The OpenStack authentication needed to create and delete instances.
     These are the same as the environment variables with uppercase names of the arguments.
 
+``block_devices``
+    A list of dictionaries.
+    Each dictionary specifies a block device to set up during instance creation.
+
+    Supported keys
+
+    ``uuid``
+        (required):
+        The image, snapshot, or volume UUID.
+    ``volume_size``
+        (required):
+        Size of the block device in GiB.
+    ``device_name``
+        (optional): defaults to ``vda``.
+        The name of the device in the instance; e.g. vda or xda.
+    ``source_type``
+        (optional): defaults to ``image``.
+        The origin of the block device.
+        Valid values are ``image``, ``snapshot``, or ``volume``.
+    ``destination_type``
+        (optional): defaults to ``volume``.
+        Destination of block device: ``volume`` or ``local``.
+    ``delete_on_termination``
+        (optional): defaults to ``True``.
+        Controls if the block device will be deleted when the instance terminates.
+    ``boot_index``
+        (optional): defaults to ``0``.
+        Integer used for boot order.
+
 ``meta``
     A dictionary of string key-value pairs to pass to the instance.
     These will be available under the ``metadata`` key from the metadata service.
 
+``nova_args``
+    (optional)
+    A dict that will be appended to the arguments when creating a VM.
+    Buildbot uses the OpenStack Nova version 1.1 API.
+
 Here is the simplest example of configuring an OpenStack latent buildslave.
 
 ::
@@ -99,6 +133,25 @@ The invocation happens in a separate thread to prevent blocking the build master
     ]
 
 
+The ``block_devices`` argument is minimally manipulated to provide some defaults and passed directly to novaclient.
+The simplest example is an image that is converted to a volume and the instance boots from that volume.
+When the instance is destroyed, the volume will be terminated as well.
+
+::
+
+    from buildbot.plugins import buildslave
+    c['slaves'] = [
+        buildslave.OpenStackLatentBuildSlave('bot2', 'sekrit',
+                    flavor=1, image='8ac9d4a4-5e03-48b0-acde-77a0345a9ab1',
+                    os_username='user', os_password='password',
+                    os_tenant_name='tenant',
+                    os_auth_url='http://127.0.0.1:35357/v2.0',
+                    block_devices=[
+                        {'uuid': '3f0b8868-67e7-4a5b-b685-2824709bd486',
+                        'volume_size': 10}])
+    ]
+
+
 :class:`OpenStackLatentBuildSlave` supports all other configuration from the standard :class:`BuildSlave`.
 The ``missing_timeout`` and ``notify_on_missing`` specify how long to wait for an OpenStack instance to attach before considering the attempt to have failed and email addresses to alert, respectively.
 ``missing_timeout`` defaults to 20 minutes.

master/docs/relnotes/index.rst

@@ -86,6 +86,8 @@ Features
 
 * :class:`~buildbot.status.status_gerrit.GerritStatusPush` supports specifying an SSH identity file explicitly.
 
+* OpenStack latent slaves now support block devices as a bootable volume.
+
 Fixes
 ~~~~~