Merge "Add ability to provide configdrive when rebuilding"

api-ref/source/baremetal-api-v1-node-management.inc

@@ -337,6 +337,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 <http://docs.openstack.org/ironic/latest/contributor/states.html>`_.
 
+.. versionadded:: 1.35
+   A ``configdrive`` can be provided when setting the node's provision target state to ``rebuild``.
+
 Normal response code: 202
 
 Error codes:

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

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)
 ------------------
 

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:
+        if configdrive:
-            msg = (_('Adding a config drive is only supported when setting '
+            api_utils.check_allow_configdrive(target)
-                     'provision state to %s') % ir_states.ACTIVE)
-            raise wsme.exc.ClientSideError(
-                msg, status_code=http_client.BAD_REQUEST)
 
         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:
+        if target in (ir_states.ACTIVE, ir_states.REBUILD):
-            pecan.request.rpcapi.do_node_deploy(pecan.request.context,
+            rebuild = (target == ir_states.REBUILD)
-                                                rpc_node.uuid, False,
+            pecan.request.rpcapi.do_node_deploy(context=pecan.request.context,
-                                                configdrive, topic)
+                                                node_id=rpc_node.uuid,
-        elif target == ir_states.REBUILD:
+                                                rebuild=rebuild,
-            pecan.request.rpcapi.do_node_deploy(pecan.request.context,
+                                                configdrive=configdrive,
-                                                rpc_node.uuid, True,
+                                                topic=topic)
-                                                None, topic)
         elif target == ir_states.DELETED:
             pecan.request.rpcapi.do_node_tear_down(
                 pecan.request.context, rpc_node.uuid, topic)

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.
 

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)

ironic/tests/unit/api/controllers/v1/test_node.py

@@ -2982,8 +2982,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(
+        self.mock_dnd.assert_called_once_with(context=mock.ANY,
-            mock.ANY, self.node.uuid, False, None, 'test-topic')
+                                              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
@@ -3002,16 +3005,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(
+        self.mock_dnd.assert_called_once_with(context=mock.ANY,
-            mock.ANY, self.node.uuid, False, 'foo', 'test-topic')
+                                              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
@@ -3097,8 +3158,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(
+        self.mock_dnd.assert_called_once_with(context=mock.ANY,
-            mock.ANY, node.uuid, False, None, 'test-topic')
+                                              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

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):
 

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.