From 8ae7bf27dd53bf2c443ad091ee422281de7ba176 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Fri, 17 Jun 2016 07:57:04 -0400 Subject: [PATCH] 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 | 12 +++++++----- ironic/conductor/utils.py | 4 ---- ironic/tests/unit/api/v1/test_nodes.py | 19 +++++++++++++++++-- ironic/tests/unit/conductor/test_manager.py | 8 ++++++++ ...active-node-creation-a41c9869c966c82b.yaml | 5 +++-- 5 files changed, 35 insertions(+), 13 deletions(-) diff --git a/doc/source/deploy/adoption.rst b/doc/source/deploy/adoption.rst index 34fab413d8..aed8bd284f 100644 --- a/doc/source/deploy/adoption.rst +++ b/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 - defined, and no instance_info/image_checksum is defined so - any actual attempt to write the image out will fail. + The above example will fail a re-deployment as a fake image is + defined and no instance_info/image_checksum value is defined. + 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:: diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index f25340c223..89c296c612 100644 --- a/ironic/conductor/utils.py +++ b/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, diff --git a/ironic/tests/unit/api/v1/test_nodes.py b/ironic/tests/unit/api/v1/test_nodes.py index 5eabd00475..6a75ea0a96 100644 --- a/ironic/tests/unit/api/v1/test_nodes.py +++ b/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() diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index d53db2a753..c934a405f5 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/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, diff --git a/releasenotes/notes/active-node-creation-a41c9869c966c82b.yaml b/releasenotes/notes/active-node-creation-a41c9869c966c82b.yaml index 91abd8e912..7688a75a0b 100644 --- a/releasenotes/notes/active-node-creation-a41c9869c966c82b.yaml +++ b/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 - operator changes the power state in ironic. + that recorded power state will be enforced by ironic unless an + operator changes the power state in ironic. This was the default + behavior of ironic prior to the adoption feature.