Add logic to store the config drive passed by Nova

This patch is extending the /nodes/<uuid>/provision API endpoint to accept
an optional "configdrive" parameter as part of the request BODY. If
present, Ironic will get the config drive and store it either directly
on the Node's instance_info field or it will upload it to Swift first,
generates a temp URL and then store it on Node's instance_info to be
processed later after the deployment started.

Two new config options were added to the conductor:

* configdrive_use_swift: Whether to upload the config drive to Swift ot
  not. Defaults to False

* configdrive_swift_container: The name of the container in Swift to
  store the config drive. Defaults to ironic_configdrive_container

Implements: blueprint expose-configdrive
Change-Id: Icc39af604af6439e85f14d1beb2c19b10e983635

etc/ironic/ironic.conf.sample

@@ -581,6 +581,13 @@
 # the check entirely. (integer value)
 #sync_local_state_interval=180
 
+# Whether upload the config drive to Swift. (boolean value)
+#configdrive_use_swift=false
+
+# The Swift config drive container to store data. (string
+# value)
+#configdrive_swift_container=ironic_configdrive_container
+
 
 [console]
 
@@ -817,6 +824,15 @@
 # (string value)
 #swift_container=glance
 
+# This should match a config by the same name in the Glance
+# configuration file. When set to 0, a single-tenant store
+# will only use one container to store all images. When set to
+# an integer value between 1 and 32, a single-tenant store
+# will use multiple containers to store images, and this value
+# will determine how many containers are created. (integer
+# value)
+#swift_store_multiple_containers_seed=0
+
 
 #
 # Options defined in ironic.common.image_service

ironic/api/controllers/v1/node.py

@@ -304,8 +304,9 @@ class NodeStatesController(rest.RestController):
         url_args = '/'.join([node_uuid, 'states'])
         pecan.response.location = link.build_url('nodes', url_args)
 
-    @wsme_pecan.wsexpose(None, types.uuid, wtypes.text, status_code=202)
-    def provision(self, node_uuid, target):
+    @wsme_pecan.wsexpose(None, types.uuid, wtypes.text, wtypes.text,
+                         status_code=202)
+    def provision(self, node_uuid, target, configdrive=None):
         """Asynchronous trigger the provisioning of the node.
 
         This will set the target provision state of the node, and a
@@ -317,6 +318,9 @@ class NodeStatesController(rest.RestController):
 
         :param node_uuid: UUID of a node.
         :param target: The desired provision state of the node.
+        :param configdrive: Optional. A gzipped and base64 encoded
+            configdrive. Only valid when setting provision state
+            to "active".
         :raises: ClientSideError (HTTP 409) if the node is already being
                  provisioned.
         :raises: ClientSideError (HTTP 400) if the node is already in
@@ -346,14 +350,22 @@ class NodeStatesController(rest.RestController):
                    % rpc_node.uuid)
             raise wsme.exc.ClientSideError(msg, status_code=409)  # Conflict
 
+        if configdrive and target != ir_states.ACTIVE:
+            msg = (_('Adding a config drive is only supported when setting '
+                     'provision state to %s') % ir_states.ACTIVE)
+            raise wsme.exc.ClientSideError(msg, status_code=400)
+
         # Note that there is a race condition. The node state(s) could change
         # by the time the RPC call is made and the TaskManager manager gets a
         # lock.
-
-        if target in (ir_states.ACTIVE, ir_states.REBUILD):
-            rebuild = (target == ir_states.REBUILD)
-            pecan.request.rpcapi.do_node_deploy(
-                    pecan.request.context, node_uuid, rebuild, topic)
+        if target == ir_states.ACTIVE:
+            pecan.request.rpcapi.do_node_deploy(pecan.request.context,
+                                                node_uuid, False,
+                                                configdrive, topic)
+        elif target == ir_states.REBUILD:
+            pecan.request.rpcapi.do_node_deploy(pecan.request.context,
+                                                node_uuid, True,
+                                                None, topic)
         elif target == ir_states.DELETED:
             pecan.request.rpcapi.do_node_tear_down(
                     pecan.request.context, node_uuid, topic)

ironic/conductor/manager.py

@@ -43,6 +43,7 @@ a change, etc.
 
 import collections
 import datetime
+import tempfile
 import threading
 
 import eventlet
@@ -65,6 +66,7 @@ from ironic.common.i18n import _LW
 from ironic.common import keystone
 from ironic.common import rpc
 from ironic.common import states
+from ironic.common import swift
 from ironic.common import utils as ironic_utils
 from ironic.conductor import task_manager
 from ironic.conductor import utils
@@ -152,6 +154,12 @@ conductor_opts = [
                         'conductor will check for nodes that it should '
                         '"take over". Set it to a negative value to disable '
                         'the check entirely.'),
+        cfg.BoolOpt('configdrive_use_swift',
+                    default=False,
+                    help='Whether upload the config drive to Swift.'),
+        cfg.StrOpt('configdrive_swift_container',
+                   default='ironic_configdrive_container',
+                   help='The Swift config drive container to store data.'),
 ]
 
 CONF = cfg.CONF
@@ -162,7 +170,7 @@ class ConductorManager(periodic_task.PeriodicTasks):
     """Ironic Conductor manager main class."""
 
     # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's.
-    RPC_API_VERSION = '1.21'
+    RPC_API_VERSION = '1.22'
 
     target = messaging.Target(version=RPC_API_VERSION)
 
@@ -593,7 +601,8 @@ class ConductorManager(periodic_task.PeriodicTasks):
                                    exception.InstanceDeployFailure,
                                    exception.InvalidParameterValue,
                                    exception.MissingParameterValue)
-    def do_node_deploy(self, context, node_id, rebuild=False):
+    def do_node_deploy(self, context, node_id, rebuild=False,
+                       configdrive=None):
         """RPC method to initiate deployment to a node.
 
         Initiate the deployment of a node. Validations are done
@@ -606,6 +615,7 @@ class ConductorManager(periodic_task.PeriodicTasks):
                         recreate the instance on the same node, overwriting
                         all disk. The ephemeral partition, if it exists, can
                         optionally be preserved.
+        :param configdrive: Optional. A gzipped and base64 encoded configdrive.
         :raises: InstanceDeployFailure
         :raises: NodeInMaintenance if the node is in maintenance mode.
         :raises: NoFreeConductorWorker when there is no free worker to start
@@ -649,7 +659,8 @@ class ConductorManager(periodic_task.PeriodicTasks):
                 task.process_event(event,
                                    callback=self._spawn_worker,
                                    call_args=(do_node_deploy, task,
-                                              self.conductor.id),
+                                              self.conductor.id,
+                                              configdrive),
                                    err_handler=provisioning_error_handler)
             except exception.InvalidState:
                 raise exception.InstanceDeployFailure(_(
@@ -1345,16 +1356,70 @@ def provisioning_error_handler(e, node, provision_state,
                      'tgt_prov_state': target_provision_state})
 
 
-def do_node_deploy(task, conductor_id):
+def _get_configdrive_obj_name(node):
+    """Generate the object name for the config drive."""
+    return 'configdrive-%s' % node.uuid
+
+
+def _store_configdrive(node, configdrive):
+    """Handle the storage of the config drive.
+
+    Whether update the Node's instance_info with the config driver
+    directly or upload it to Swift first and update the Node with an
+    temp URL pointing to the Swift object.
+
+    :param node: an Ironic node object.
+    :param configdrive: A gzipped and base64 encoded configdrive.
+    :raises: SwiftOperationError if an error occur when uploading the
+             config drive to Swift.
+
+    """
+    if CONF.conductor.configdrive_use_swift:
+        # NOTE(lucasagomes): No reason to use a different timeout than
+        # the one used for deploying the node
+        timeout = CONF.conductor.deploy_callback_timeout
+        container = CONF.conductor.configdrive_swift_container
+        object_name = _get_configdrive_obj_name(node)
+
+        object_headers = {'X-Delete-After': timeout}
+
+        with tempfile.NamedTemporaryFile() as fileobj:
+            fileobj.write(configdrive)
+            fileobj.flush()
+
+            swift_api = swift.SwiftAPI()
+            swift_api.create_object(container, object_name, fileobj.name,
+                                    object_headers=object_headers)
+            configdrive = swift_api.get_temp_url(container, object_name,
+                                                 timeout)
+
+    i_info = node.instance_info
+    i_info['configdrive'] = configdrive
+    node.instance_info = i_info
+
+
+def do_node_deploy(task, conductor_id, configdrive=None):
     """Prepare the environment and deploy a node."""
     node = task.node
+
+    def handle_failure(e, task, logmsg, errmsg):
+        # NOTE(deva): there is no need to clear conductor_affinity
+        task.process_event('fail')
+        args = {'node': task.node.uuid, 'err': e}
+        LOG.warning(logmsg, args)
+        node.last_error = errmsg % e
+
     try:
-        def handle_failure(e, task, logmsg, errmsg):
-            # NOTE(deva): there is no need to clear conductor_affinity
-            task.process_event('fail')
-            args = {'node': task.node.uuid, 'err': e}
-            LOG.warning(logmsg, args)
-            node.last_error = errmsg % e
+        try:
+            if configdrive:
+                _store_configdrive(node, configdrive)
+        except exception.SwiftOperationError as e:
+            with excutils.save_and_reraise_exception():
+                handle_failure(e, task,
+                    _LW('Error while uploading the configdrive for '
+                        '%(node)s to Swift'),
+                    _('Failed to upload the configdrive to Swift. '
+                      'Error %s'))
 
         try:
             task.driver.deploy.prepare(task)

ironic/conductor/rpcapi.py

@@ -64,11 +64,12 @@ class ConductorAPI(object):
     |           driver_vendor_passthru
     |    1.21 - Added get_node_vendor_passthru_methods and
     |           get_driver_vendor_passthru_methods
+    |    1.22 - Added configdrive parameter to do_node_deploy.
 
     """
 
     # NOTE(rloo): This must be in sync with manager.ConductorManager's.
-    RPC_API_VERSION = '1.21'
+    RPC_API_VERSION = '1.22'
 
     def __init__(self, topic=None):
         super(ConductorAPI, self).__init__()
@@ -260,12 +261,14 @@ class ConductorAPI(object):
         return cctxt.call(context, 'get_driver_vendor_passthru_methods',
                           driver_name=driver_name)
 
-    def do_node_deploy(self, context, node_id, rebuild, topic=None):
+    def do_node_deploy(self, context, node_id, rebuild, configdrive,
+                       topic=None):
         """Signal to conductor service to perform a deployment.
 
         :param context: request context.
         :param node_id: node id or uuid.
         :param rebuild: True if this is a rebuild request.
+        :param configdrive: Optional. A gzipped and base64 encoded configdrive.
         :param topic: RPC topic. Defaults to self.topic.
         :raises: InstanceDeployFailure
         :raises: InvalidParameterValue if validation fails
@@ -277,9 +280,9 @@ class ConductorAPI(object):
         undeployed state before this method is called.
 
         """
-        cctxt = self.client.prepare(topic=topic or self.topic, version='1.15')
+        cctxt = self.client.prepare(topic=topic or self.topic, version='1.22')
         return cctxt.call(context, 'do_node_deploy', node_id=node_id,
-                          rebuild=rebuild)
+                          rebuild=rebuild, configdrive=configdrive)
 
     def do_node_tear_down(self, context, node_id, topic=None):
         """Signal to conductor service to tear down a deployment.

ironic/tests/api/v1/test_nodes.py

@@ -1119,13 +1119,32 @@ class TestPut(api_base.FunctionalTest):
         self.assertEqual(202, ret.status_code)
         self.assertEqual('', ret.body)
         self.mock_dnd.assert_called_once_with(
-                mock.ANY, self.node.uuid, False, 'test-topic')
+                mock.ANY, self.node.uuid, False, None, 'test-topic')
         # Check location header
         self.assertIsNotNone(ret.location)
         expected_location = '/v1/nodes/%s/states' % self.node.uuid
         self.assertEqual(urlparse.urlparse(ret.location).path,
                          expected_location)
 
+    def test_provision_with_deploy_configdrive(self):
+        ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid,
+                            {'target': states.ACTIVE, 'configdrive': 'foo'})
+        self.assertEqual(202, ret.status_code)
+        self.assertEqual('', ret.body)
+        self.mock_dnd.assert_called_once_with(
+                mock.ANY, self.node.uuid, False, 'foo', 'test-topic')
+        # Check location header
+        self.assertIsNotNone(ret.location)
+        expected_location = '/v1/nodes/%s/states' % self.node.uuid
+        self.assertEqual(urlparse.urlparse(ret.location).path,
+                         expected_location)
+
+    def test_provision_with_configdrive_not_active(self):
+        ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid,
+                            {'target': states.DELETED, 'configdrive': 'foo'},
+                            expect_errors=True)
+        self.assertEqual(400, ret.status_code)
+
     def test_provision_with_tear_down(self):
         ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid,
                             {'target': states.DELETED})
@@ -1181,7 +1200,7 @@ class TestPut(api_base.FunctionalTest):
         self.assertEqual(202, ret.status_code)
         self.assertEqual('', ret.body)
         self.mock_dnd.assert_called_once_with(
-                mock.ANY, node.uuid, False, 'test-topic')
+                mock.ANY, node.uuid, False, None, 'test-topic')
         # Check location header
         self.assertIsNotNone(ret.location)
         expected_location = '/v1/nodes/%s/states' % node.uuid

ironic/tests/conductor/test_manager.py

@@ -31,6 +31,7 @@ from ironic.common import driver_factory
 from ironic.common import exception
 from ironic.common import keystone
 from ironic.common import states
+from ironic.common import swift
 from ironic.common import utils as ironic_utils
 from ironic.conductor import manager
 from ironic.conductor import task_manager
@@ -969,8 +970,9 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
         self.assertIsNotNone(node.last_error)
         mock_deploy.assert_called_once_with(mock.ANY)
 
+    @mock.patch.object(manager, '_store_configdrive')
     @mock.patch('ironic.drivers.modules.fake.FakeDeploy.deploy')
-    def test__do_node_deploy_ok(self, mock_deploy):
+    def test__do_node_deploy_ok(self, mock_deploy, mock_store):
         self._start_service()
         # test when driver.deploy.deploy returns DEPLOYDONE
         mock_deploy.return_value = states.DEPLOYDONE
@@ -985,6 +987,53 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
         self.assertEqual(states.NOSTATE, node.target_provision_state)
         self.assertIsNone(node.last_error)
         mock_deploy.assert_called_once_with(mock.ANY)
+        # assert _store_configdrive wasn't invoked
+        self.assertFalse(mock_store.called)
+
+    @mock.patch.object(manager, '_store_configdrive')
+    @mock.patch('ironic.drivers.modules.fake.FakeDeploy.deploy')
+    def test__do_node_deploy_ok_configdrive(self, mock_deploy, mock_store):
+        self._start_service()
+        # test when driver.deploy.deploy returns DEPLOYDONE
+        mock_deploy.return_value = states.DEPLOYDONE
+        node = obj_utils.create_test_node(self.context, driver='fake',
+                                          provision_state=states.DEPLOYING,
+                                          target_provision_state=states.ACTIVE)
+        task = task_manager.TaskManager(self.context, node.uuid)
+        configdrive = 'foo'
+
+        manager.do_node_deploy(task, self.service.conductor.id,
+                               configdrive=configdrive)
+        node.refresh()
+        self.assertEqual(states.ACTIVE, node.provision_state)
+        self.assertEqual(states.NOSTATE, node.target_provision_state)
+        self.assertIsNone(node.last_error)
+        mock_deploy.assert_called_once_with(mock.ANY)
+        mock_store.assert_called_once_with(task.node, configdrive)
+
+    @mock.patch.object(swift, 'SwiftAPI')
+    @mock.patch('ironic.drivers.modules.fake.FakeDeploy.deploy')
+    def test__do_node_deploy_configdrive_swift_error(self, mock_deploy,
+                                                     mock_swift):
+        CONF.set_override('configdrive_use_swift', True, group='conductor')
+        self._start_service()
+        # test when driver.deploy.deploy returns DEPLOYDONE
+        mock_deploy.return_value = states.DEPLOYDONE
+        node = obj_utils.create_test_node(self.context, driver='fake',
+                                          provision_state=states.DEPLOYING,
+                                          target_provision_state=states.ACTIVE)
+        task = task_manager.TaskManager(self.context, node.uuid)
+
+        mock_swift.side_effect = exception.SwiftOperationError('error')
+        self.assertRaises(exception.SwiftOperationError,
+                           manager.do_node_deploy, task,
+                           self.service.conductor.id,
+                           configdrive='fake config drive')
+        node.refresh()
+        self.assertEqual(states.DEPLOYFAIL, node.provision_state)
+        self.assertEqual(states.ACTIVE, node.target_provision_state)
+        self.assertIsNotNone(node.last_error)
+        self.assertFalse(mock_deploy.called)
 
     @mock.patch('ironic.drivers.modules.fake.FakeDeploy.deploy')
     def test__do_node_deploy_ok_2(self, mock_deploy):
@@ -1022,7 +1071,8 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
             self.assertIsNone(node.last_error)
             # Verify reservation has been cleared.
             self.assertIsNone(node.reservation)
-            mock_spawn.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY)
+            mock_spawn.assert_called_once_with(mock.ANY, mock.ANY,
+                                               mock.ANY, None)
 
     @mock.patch('ironic.drivers.modules.fake.FakeDeploy.deploy')
     def test_do_node_deploy_rebuild_active_state(self, mock_deploy):
@@ -2839,3 +2889,43 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
         self.task.spawn_after.assert_called_once_with(
                 self.service._spawn_worker,
                 self.service._do_takeover, self.task)
+
+
+@mock.patch.object(swift, 'SwiftAPI')
+class StoreConfigDriveTestCase(tests_base.TestCase):
+
+    def setUp(self):
+        super(StoreConfigDriveTestCase, self).setUp()
+        self.node = obj_utils.get_test_node(self.context, driver='fake',
+                                            instance_info=None)
+
+    def test_store_configdrive(self, mock_swift):
+        manager._store_configdrive(self.node, 'foo')
+        expected_instance_info = {'configdrive': 'foo'}
+        self.assertEqual(expected_instance_info, self.node.instance_info)
+        self.assertFalse(mock_swift.called)
+
+    def test_store_configdrive_swift(self, mock_swift):
+        container_name = 'foo_container'
+        timeout = 123
+        expected_obj_name = 'configdrive-%s' % self.node.uuid
+        expected_obj_header = {'X-Delete-After': timeout}
+        expected_instance_info = {'configdrive': 'http://1.2.3.4'}
+
+        # set configs and mocks
+        CONF.set_override('configdrive_use_swift', True, group='conductor')
+        CONF.set_override('configdrive_swift_container', container_name,
+                          group='conductor')
+        CONF.set_override('deploy_callback_timeout', timeout,
+                          group='conductor')
+        mock_swift.return_value.get_temp_url.return_value = 'http://1.2.3.4'
+
+        manager._store_configdrive(self.node, 'foo')
+
+        mock_swift.assert_called_once_with()
+        mock_swift.return_value.create_object.assert_called_once_with(
+            container_name, expected_obj_name, mock.ANY,
+            object_headers=expected_obj_header)
+        mock_swift.return_value.get_temp_url(container_name,
+            expected_obj_name, timeout)
+        self.assertEqual(expected_instance_info, self.node.instance_info)

ironic/tests/conductor/test_rpcapi.py

@@ -202,9 +202,10 @@ class RPCAPITestCase(base.DbTestCase):
     def test_do_node_deploy(self):
         self._test_rpcapi('do_node_deploy',
                           'call',
-                          version='1.15',
+                          version='1.22',
                           node_id=self.fake_node['uuid'],
-                          rebuild=False)
+                          rebuild=False,
+                          configdrive=None)
 
     def test_do_node_tear_down(self):
         self._test_rpcapi('do_node_tear_down',