Follow-up to Active Node Creation

Follow-up change to the Active Node Creation revision in order
to address some requested documentation and test changes for the
original revision Ib3eadf4172e93add9a9855582f56cbb3707f3d39.

Change-Id: I962b3d9e4b40acd92446813792c9d968fac3a170
Partial-Bug: #1526315

doc/source/deploy/adoption.rst

@@ -101,7 +101,7 @@ Requirements for use are essentially the same as to deploy a node:
 * Sufficient driver information to allow for a successful
   power management validation.
 
-* Sufficient instance_info to pass deploy driver validation.
+* Sufficient instance_info to pass deploy driver preparation.
 
 Each driver may have additional requirements dependent upon the
 configuration that is supplied. An example of this would be defining
@@ -143,9 +143,11 @@ from the ``manageable`` state to ``active`` state.::
    image or file, however that image or file can ultimately be empty.
 
 .. NOTE::
-   The above example will naturally fail as a fake image is
+   The above example will fail a re-deployment as a fake image is
-   defined, and no instance_info/image_checksum is defined so
+   defined and no instance_info/image_checksum value is defined.
-   any actual attempt to write the image out will fail.
+   As such any actual attempt to write the image out will fail as the
+   image_checksum value is only validated at time of an actual
+   deployment operation.
 
 .. NOTE::
    A user may wish to assign an instance_uuid to a node, which could be
@@ -176,7 +178,7 @@ the node back to ``manageable`` from ``adopt failed`` state by issuing the
 
 If all else fails the hardware node can be removed from the Bare Metal
 service.  The ``node-delete`` command, which is **not** the same as setting
-the provision state to ``delete``, can be used while the node is in
+the provision state to ``deleted``, can be used while the node is in
 ``adopt failed`` state. This will delete the node without cleaning
 occurring to preserve the node's current state. Example::
 

ironic/conductor/utils.py

@@ -59,10 +59,6 @@ def node_set_boot_device(task, device, persistent=False):
     """
     if getattr(task.driver, 'management', None):
         task.driver.management.validate(task)
-        # NOTE(TheJulia): When a node is in the ADOPTING state, we must
-        # not attempt to change the default boot device as a side effect
-        # of a driver's node takeover process as it is modifying
-        # a working machine.
         if task.node.provision_state != states.ADOPTING:
             task.driver.management.set_boot_device(task,
                                                    device=device,

ironic/tests/unit/api/v1/test_nodes.py

@@ -2208,7 +2208,8 @@ class TestPut(test_api_base.BaseApiTest):
                                          'test-topic')
 
     @mock.patch.object(rpcapi.ConductorAPI, 'do_provisioning_action')
-    def test_adopt_from_adoption_failed(self, mock_dpa):
+    def test_adopt_from_adoptfail(self, mock_dpa):
+        """Test that a node in ADOPTFAIL can be adopted"""
         self.node.provision_state = states.ADOPTFAIL
         self.node.save()
 
@@ -2223,6 +2224,7 @@ class TestPut(test_api_base.BaseApiTest):
 
     @mock.patch.object(rpcapi.ConductorAPI, 'do_provisioning_action')
     def test_adopt_from_active_fails(self, mock_dpa):
+        """Test that an ACTIVE node cannot be adopted"""
         self.node.provision_state = states.ACTIVE
         self.node.save()
 
@@ -2234,7 +2236,8 @@ class TestPut(test_api_base.BaseApiTest):
         self.assertEqual(0, mock_dpa.call_count)
 
     @mock.patch.object(rpcapi.ConductorAPI, 'do_provisioning_action')
-    def test_manage_from_adoption_failed(self, mock_dpa):
+    def test_manage_from_adoptfail(self, mock_dpa):
+        """Test that a node can be sent to MANAGEABLE from ADOPTFAIL"""
         self.node.provision_state = states.ADOPTFAIL
         self.node.save()
 
@@ -2249,6 +2252,12 @@ class TestPut(test_api_base.BaseApiTest):
 
     @mock.patch.object(rpcapi.ConductorAPI, 'do_provisioning_action')
     def test_bad_requests_in_adopting_state(self, mock_dpa):
+        """Test that a node in ADOPTING fails with invalid requests
+
+        Verify that an API request fails if the ACTIVE, REBUILD, or DELETED
+        state is requested by an API client when the node is in ADOPTING
+        state.
+        """
         self.node.provision_state = states.ADOPTING
         self.node.save()
 
@@ -2261,6 +2270,12 @@ class TestPut(test_api_base.BaseApiTest):
 
     @mock.patch.object(rpcapi.ConductorAPI, 'do_provisioning_action')
     def test_bad_requests_in_adoption_failed_state(self, mock_dpa):
+        """Test that a node in ADOPTFAIL fails with invalid requests
+
+        Verify that an API request fails if the ACTIVE, REBUILD, or DELETED
+        state is requested by an API client when the node is in ADOPTFAIL
+        state.
+        """
         self.node.provision_state = states.ADOPTFAIL
         self.node.save()
 

ironic/tests/unit/conductor/test_manager.py

@@ -4788,6 +4788,7 @@ class DoNodeAdoptionTestCase(
                                         mock_start_console,
                                         mock_boot_validate,
                                         mock_power_validate):
+        """Test a successful node adoption"""
         self._start_service()
         node = obj_utils.create_test_node(
             self.context, driver='fake',
@@ -4804,6 +4805,7 @@ class DoNodeAdoptionTestCase(
         mock_take_over.assert_called_once_with(mock.ANY)
         self.assertFalse(mock_start_console.called)
         self.assertTrue(mock_boot_validate.called)
+        self.assertIn('is_whole_disk_image', task.node.driver_internal_info)
 
     @mock.patch('ironic.drivers.modules.fake.FakeBoot.validate')
     @mock.patch('ironic.drivers.modules.fake.FakeConsole.start_console')
@@ -4814,6 +4816,7 @@ class DoNodeAdoptionTestCase(
                                             mock_take_over,
                                             mock_start_console,
                                             mock_boot_validate):
+        """Test that adoption failed if an exception is raised"""
         # Note(TheJulia): Use of an actual possible exception that
         # can be raised due to a misconfiguration.
         mock_take_over.side_effect = exception.IPMIFailure(
@@ -4835,6 +4838,7 @@ class DoNodeAdoptionTestCase(
         mock_take_over.assert_called_once_with(mock.ANY)
         self.assertFalse(mock_start_console.called)
         self.assertTrue(mock_boot_validate.called)
+        self.assertIn('is_whole_disk_image', task.node.driver_internal_info)
 
     @mock.patch('ironic.drivers.modules.fake.FakeBoot.validate')
     @mock.patch('ironic.drivers.modules.fake.FakeConsole.start_console')
@@ -4845,6 +4849,7 @@ class DoNodeAdoptionTestCase(
                                                 mock_take_over,
                                                 mock_start_console,
                                                 mock_boot_validate):
+        """Test that adoption fails if the boot validation fails"""
         # Note(TheJulia): Use of an actual possible exception that
         # can be raised due to a misconfiguration.
         mock_boot_validate.side_effect = exception.MissingParameterValue(
@@ -4869,6 +4874,7 @@ class DoNodeAdoptionTestCase(
 
     @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker')
     def test_do_provisioning_action_adopt_node(self, mock_spawn):
+        """Test an adoption request results in the node in ADOPTING"""
         node = obj_utils.create_test_node(
             self.context, driver='fake',
             provision_state=states.MANAGEABLE,
@@ -4884,6 +4890,7 @@ class DoNodeAdoptionTestCase(
 
     @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker')
     def test_do_provisioning_action_adopt_node_retry(self, mock_spawn):
+        """Test a retried adoption from ADOPTFAIL results in ADOPTING state"""
         node = obj_utils.create_test_node(
             self.context, driver='fake',
             provision_state=states.ADOPTFAIL,
@@ -4898,6 +4905,7 @@ class DoNodeAdoptionTestCase(
         mock_spawn.assert_called_with(self.service._do_adoption, mock.ANY)
 
     def test_do_provisioning_action_manage_of_failed_adoption(self):
+        """Test a node in ADOPTFAIL can be taken to MANAGEABLE"""
         node = obj_utils.create_test_node(
             self.context, driver='fake',
             provision_state=states.ADOPTFAIL,

releasenotes/notes/active-node-creation-a41c9869c966c82b.yaml

@@ -10,5 +10,6 @@ other:
   - When a node is enrolled into ironic, upon transition to the
     ``manageable`` state, the current power state of the node is
     recorded.  Once the node is adopted and in an ``active`` state,
-    that recorded power state will be enfored by ironic unless an
+    that recorded power state will be enforced by ironic unless an
-    operator changes the power state in ironic.
+    operator changes the power state in ironic. This was the default
+    behavior of ironic prior to the adoption feature.