diff options
-rw-r--r-- | etc/ironic/ironic.conf.sample | 16 | ||||
-rw-r--r-- | ironic/api/controllers/v1/node.py | 26 | ||||
-rw-r--r-- | ironic/conductor/manager.py | 85 | ||||
-rw-r--r-- | ironic/conductor/rpcapi.py | 11 | ||||
-rw-r--r-- | ironic/tests/api/v1/test_nodes.py | 23 | ||||
-rw-r--r-- | ironic/tests/conductor/test_manager.py | 94 | ||||
-rw-r--r-- | ironic/tests/conductor/test_rpcapi.py | 5 |
7 files changed, 233 insertions, 27 deletions
diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 6f2302524..975833a36 100644 --- a/etc/ironic/ironic.conf.sample +++ b/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 diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 0299f9442..0e4359708 100644 --- a/ironic/api/controllers/v1/node.py +++ b/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) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 9fabff4b2..6ca5bad7d 100644 --- a/ironic/conductor/manager.py +++ b/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) diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index 68f7c469f..487335cf0 100644 --- a/ironic/conductor/rpcapi.py +++ b/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. diff --git a/ironic/tests/api/v1/test_nodes.py b/ironic/tests/api/v1/test_nodes.py index e2848a022..2bcdd1e8f 100644 --- a/ironic/tests/api/v1/test_nodes.py +++ b/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 diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py index bdb320473..cc8ad0a54 100644 --- a/ironic/tests/conductor/test_manager.py +++ b/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) diff --git a/ironic/tests/conductor/test_rpcapi.py b/ironic/tests/conductor/test_rpcapi.py index c177ec70a..d0fabe441 100644 --- a/ironic/tests/conductor/test_rpcapi.py +++ b/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', |