From 6a8b38a2caafe89ebd73fda8c72b8f4fbbdc9f6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathieu=20Gagne=CC=81?= Date: Thu, 7 Sep 2017 10:34:45 -0400 Subject: [PATCH] Add ability to provide configdrive when rebuilding Previously, the configdrive could only be set when setting the node's provisioning state to "active". When rebuilding, the old configdrive was used and therefore was never updated with latest content. This change introduces the API microversion 1.35 which will allow configdrive to be provided when setting the node's provisioning state to "rebuild". Closes-bug: #1575935 Change-Id: I9a5529f9fa796c75621e9f4354886bf3032cc248 --- .../baremetal-api-v1-node-management.inc | 3 + api-ref/source/parameters.yaml | 2 +- .../contributor/webapi-version-history.rst | 6 ++ ironic/api/controllers/v1/node.py | 24 +++--- ironic/api/controllers/v1/utils.py | 21 +++++ ironic/api/controllers/v1/versions.py | 4 +- .../unit/api/controllers/v1/test_node.py | 80 +++++++++++++++++-- .../unit/api/controllers/v1/test_utils.py | 25 ++++++ .../rebuild-configdrive-f52479fd55b0f5ce.yaml | 10 +++ 9 files changed, 151 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/rebuild-configdrive-f52479fd55b0f5ce.yaml diff --git a/api-ref/source/baremetal-api-v1-node-management.inc b/api-ref/source/baremetal-api-v1-node-management.inc index c443a73ea1..eb620a510c 100644 --- a/api-ref/source/baremetal-api-v1-node-management.inc +++ b/api-ref/source/baremetal-api-v1-node-management.inc @@ -335,6 +335,9 @@ Acceptable target states depend on the Node's current provision state. More detailed documentation of the Ironic State Machine is available `in the developer docs `_. +.. versionadded:: 1.35 + A ``configdrive`` can be provided when setting the node's provision target state to ``rebuild``. + Normal response code: 202 Error codes: diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index bf181db420..071464ac16 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -364,7 +364,7 @@ configdrive: description: | A gzip'ed and base-64 encoded config drive, to be written to a partition on the Node's boot disk. This parameter is only accepted when setting the - state to "active". + state to "active" or "rebuild". in: body required: false type: string or gzip+b64 blob diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index 4038a524d1..38116aa93a 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,12 @@ REST API Version History ======================== +1.35 (Queens, 10.0.0) +--------------------- + +Added ability to provide ``configdrive`` when node is updated +to ``rebuild`` provision state. + 1.34 (Pike, 9.0.0) ------------------ diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index d3106f4153..8ac2fb4a68 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -560,7 +560,7 @@ class NodeStatesController(rest.RestController): :param target: The desired provision state of the node or verb. :param configdrive: Optional. A gzipped and base64 encoded configdrive. Only valid when setting provision state - to "active". + to "active" or "rebuild". :param clean_steps: An ordered list of cleaning steps that will be performed on the node. A cleaning step is a dictionary with required keys 'interface' and 'step', and optional key 'args'. If @@ -622,11 +622,8 @@ class NodeStatesController(rest.RestController): action=target, node=rpc_node.uuid, state=rpc_node.provision_state) - 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=http_client.BAD_REQUEST) + if configdrive: + api_utils.check_allow_configdrive(target) if clean_steps and target != ir_states.VERBS['clean']: msg = (_('"clean_steps" is only valid when setting target ' @@ -637,14 +634,13 @@ class NodeStatesController(rest.RestController): # 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 == ir_states.ACTIVE: - pecan.request.rpcapi.do_node_deploy(pecan.request.context, - rpc_node.uuid, False, - configdrive, topic) - elif target == ir_states.REBUILD: - pecan.request.rpcapi.do_node_deploy(pecan.request.context, - rpc_node.uuid, True, - None, topic) + if target in (ir_states.ACTIVE, ir_states.REBUILD): + rebuild = (target == ir_states.REBUILD) + pecan.request.rpcapi.do_node_deploy(context=pecan.request.context, + node_id=rpc_node.uuid, + rebuild=rebuild, + configdrive=configdrive, + topic=topic) elif target == ir_states.DELETED: pecan.request.rpcapi.do_node_tear_down( pecan.request.context, rpc_node.uuid, topic) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 8f90d26475..39ea32e5c2 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -392,6 +392,18 @@ def check_allow_driver_detail(detail): 'opr': versions.MINOR_30_DYNAMIC_DRIVERS}) +def check_allow_configdrive(target): + allowed_targets = [states.ACTIVE] + if allow_node_rebuild_with_configdrive(): + allowed_targets.append(states.REBUILD) + + if target not in allowed_targets: + msg = (_('Adding a config drive is only supported when setting ' + 'provision state to %s') % ', '.join(allowed_targets)) + raise wsme.exc.ClientSideError( + msg, status_code=http_client.BAD_REQUEST) + + def initial_node_provision_state(): """Return node state to use by default when creating new nodes. @@ -581,6 +593,15 @@ def allow_port_physical_network(): objects.Port.supports_physical_network()) +def allow_node_rebuild_with_configdrive(): + """Check if we should support node rebuild with configdrive. + + Version 1.35 of the API added support for node rebuild with configdrive. + """ + return (pecan.request.version.minor >= + versions.MINOR_35_REBUILD_CONFIG_DRIVE) + + def get_controller_reserved_names(cls): """Get reserved names for a given controller. diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index a55d0727e4..9e2340f084 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -65,6 +65,7 @@ BASE_VERSION = 1 # v1.32: Add volume support. # v1.33: Add node storage interface # v1.34: Add physical network field to port. +# v1.35: Add ability to provide configdrive when rebuilding node. MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -101,11 +102,12 @@ MINOR_31_DYNAMIC_INTERFACES = 31 MINOR_32_VOLUME = 32 MINOR_33_STORAGE_INTERFACE = 33 MINOR_34_PORT_PHYSICAL_NETWORK = 34 +MINOR_35_REBUILD_CONFIG_DRIVE = 35 # When adding another version, update MINOR_MAX_VERSION and also update # doc/source/dev/webapi-version-history.rst with a detailed explanation of # what the version has changed. -MINOR_MAX_VERSION = MINOR_34_PORT_PHYSICAL_NETWORK +MINOR_MAX_VERSION = MINOR_35_REBUILD_CONFIG_DRIVE # String representations of the minor and maximum versions MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 84d6c1c4f0..79e46dc4d6 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -2966,8 +2966,11 @@ class TestPut(test_api_base.BaseApiTest): {'target': states.ACTIVE}) self.assertEqual(http_client.ACCEPTED, ret.status_code) self.assertEqual(b'', ret.body) - self.mock_dnd.assert_called_once_with( - mock.ANY, self.node.uuid, False, None, 'test-topic') + self.mock_dnd.assert_called_once_with(context=mock.ANY, + node_id=self.node.uuid, + rebuild=False, + configdrive=None, + topic='test-topic') # Check location header self.assertIsNotNone(ret.location) expected_location = '/v1/nodes/%s/states' % self.node.uuid @@ -2986,16 +2989,74 @@ class TestPut(test_api_base.BaseApiTest): headers={api_base.Version.string: "1.5"}) self.assertEqual(http_client.ACCEPTED, ret.status_code) self.assertEqual(b'', ret.body) - self.mock_dnd.assert_called_once_with( - mock.ANY, self.node.uuid, False, None, 'test-topic') + + self.mock_dnd.assert_called_once_with(context=mock.ANY, + node_id=self.node.uuid, + rebuild=False, + configdrive=None, + topic='test-topic') 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(http_client.ACCEPTED, ret.status_code) self.assertEqual(b'', ret.body) - self.mock_dnd.assert_called_once_with( - mock.ANY, self.node.uuid, False, 'foo', 'test-topic') + self.mock_dnd.assert_called_once_with(context=mock.ANY, + node_id=self.node.uuid, + rebuild=False, + configdrive='foo', + topic='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_rebuild(self): + node = self.node + node.provision_state = states.ACTIVE + node.target_provision_state = states.NOSTATE + node.save() + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.REBUILD}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dnd.assert_called_once_with(context=mock.ANY, + node_id=self.node.uuid, + rebuild=True, + configdrive=None, + topic='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_rebuild_unsupported_configdrive(self): + node = self.node + node.provision_state = states.ACTIVE + node.target_provision_state = states.NOSTATE + node.save() + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.REBUILD, 'configdrive': 'foo'}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + + def test_provision_with_rebuild_configdrive(self): + node = self.node + node.provision_state = states.ACTIVE + node.target_provision_state = states.NOSTATE + node.save() + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.REBUILD, 'configdrive': 'foo'}, + headers={api_base.Version.string: '1.35'}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dnd.assert_called_once_with(context=mock.ANY, + node_id=self.node.uuid, + rebuild=True, + configdrive='foo', + topic='test-topic') # Check location header self.assertIsNotNone(ret.location) expected_location = '/v1/nodes/%s/states' % self.node.uuid @@ -3081,8 +3142,11 @@ class TestPut(test_api_base.BaseApiTest): {'target': states.ACTIVE}) self.assertEqual(http_client.ACCEPTED, ret.status_code) self.assertEqual(b'', ret.body) - self.mock_dnd.assert_called_once_with( - mock.ANY, node.uuid, False, None, 'test-topic') + self.mock_dnd.assert_called_once_with(context=mock.ANY, + node_id=self.node.uuid, + rebuild=False, + configdrive=None, + topic='test-topic') # Check location header self.assertIsNotNone(ret.location) expected_location = '/v1/nodes/%s/states' % node.uuid diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index 4ffb02556c..54f038da4f 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/ironic/tests/unit/api/controllers/v1/test_utils.py @@ -25,6 +25,7 @@ import wsme from ironic.api.controllers.v1 import node as api_node from ironic.api.controllers.v1 import utils from ironic.common import exception +from ironic.common import states from ironic import objects from ironic.tests import base from ironic.tests.unit.api import utils as test_api_utils @@ -444,6 +445,30 @@ class TestApiUtils(base.TestCase): mock_request.version.minor = 33 self.assertFalse(utils.allow_port_physical_network()) + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_allow_node_rebuild_with_configdrive(self, mock_request): + mock_request.version.minor = 35 + self.assertTrue(utils.allow_node_rebuild_with_configdrive()) + mock_request.version.minor = 34 + self.assertFalse(utils.allow_node_rebuild_with_configdrive()) + + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_check_allow_configdrive_fails(self, mock_request): + mock_request.version.minor = 35 + self.assertRaises(wsme.exc.ClientSideError, + utils.check_allow_configdrive, states.DELETED) + mock_request.version.minor = 34 + self.assertRaises(wsme.exc.ClientSideError, + utils.check_allow_configdrive, states.REBUILD) + + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_check_allow_configdrive(self, mock_request): + mock_request.version.minor = 35 + utils.check_allow_configdrive(states.ACTIVE) + utils.check_allow_configdrive(states.REBUILD) + mock_request.version.minor = 34 + utils.check_allow_configdrive(states.ACTIVE) + class TestNodeIdent(base.TestCase): diff --git a/releasenotes/notes/rebuild-configdrive-f52479fd55b0f5ce.yaml b/releasenotes/notes/rebuild-configdrive-f52479fd55b0f5ce.yaml new file mode 100644 index 0000000000..45c671f1ce --- /dev/null +++ b/releasenotes/notes/rebuild-configdrive-f52479fd55b0f5ce.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Starting with REST API version 1.35, it is possible to provide + a configdrive when rebuilding a node. +fixes: + - | + Fixes the problem of an old configdrive (used for deploying the node) + being used again when rebuilding the node. Starting with REST API 1.35, + it is possible to specify a different configdrive when rebuilding a node.