From 49fabe6d7b7051cb78d6f7904a62ab6ae2bd8c0f Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Wed, 3 Aug 2016 17:11:33 -0700 Subject: [PATCH] Add API methods for [un]rescue Adds API methods to support rescue and unrescue. After rescuing a node, it will be left running a rescue ramdisk, configured with the rescue_password, and listening with ssh on the specified network interfaces. Unrescuing a node will return the node to Active. Change-Id: I3953ff0b1ca000f8ae83fb7b3c663f848a149345 Partial-bug: #1526449 Co-Authored-By: Jay Faulkner Co-Authored-By: Josh Gachnang Co-Authored-By: Jesse J. Cook Co-Authored-By: Mario Villaplana Co-Authored-By: Aparna Co-Authored-By: Shivanand Tendulker --- .../contributor/webapi-version-history.rst | 16 +- ironic/api/controllers/v1/driver.py | 6 + ironic/api/controllers/v1/node.py | 42 ++- ironic/api/controllers/v1/ramdisk.py | 3 +- ironic/api/controllers/v1/utils.py | 12 + ironic/api/controllers/v1/versions.py | 5 +- ironic/common/release_mappings.py | 2 +- ironic/conductor/manager.py | 5 - .../unit/api/controllers/v1/test_driver.py | 41 ++- .../unit/api/controllers/v1/test_node.py | 278 +++++++++++++++++- .../unit/api/controllers/v1/test_utils.py | 15 + ironic/tests/unit/conductor/test_manager.py | 31 +- .../notes/rescue-node-87e3b673c61ef628.yaml | 39 +++ tox.ini | 2 +- 14 files changed, 454 insertions(+), 43 deletions(-) create mode 100644 releasenotes/notes/rescue-node-87e3b673c61ef628.yaml diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index bfd7eac5f6..033335cce0 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,20 @@ REST API Version History ======================== +1.38 (Queens, 10.1.0) +--------------------- + +Added provision_state verbs ``rescue`` and ``unrescue`` along with +the following states: ``rescue``, ``rescue failed``, ``rescue wait``, +``rescuing``, ``unrescue failed``, and ``unrescuing``. After rescuing +a node, it will be left in the ``rescue`` state running a rescue +ramdisk, configured with the ``rescue_password``, and listening with +ssh on the specified network interfaces. Unrescuing a node will return +it to ``active``. + +Added ``rescue_interface`` to the node object, to +allow setting the rescue interface for a dynamic driver. + 1.37 (Queens, 10.1.0) --------------------- @@ -36,7 +50,7 @@ Added ``agent_version`` parameter to deploy heartbeat request for version negotiation with Ironic Python Agent features. 1.35 (Queens, 9.2.0) ---------------------- +-------------------- Added ability to provide ``configdrive`` when node is updated to ``rebuild`` provision state. diff --git a/ironic/api/controllers/v1/driver.py b/ironic/api/controllers/v1/driver.py index 85ee7e2b2d..3a93efbcee 100644 --- a/ironic/api/controllers/v1/driver.py +++ b/ironic/api/controllers/v1/driver.py @@ -75,6 +75,10 @@ def hide_fields_in_newer_versions(obj): obj.default_storage_interface = wsme.Unset obj.enabled_storage_interfaces = wsme.Unset + if not api_utils.allow_rescue_interface(): + obj.default_rescue_interface = wsme.Unset + obj.enabled_rescue_interfaces = wsme.Unset + class Driver(base.APIBase): """API representation of a driver.""" @@ -103,6 +107,7 @@ class Driver(base.APIBase): default_network_interface = wtypes.text default_power_interface = wtypes.text default_raid_interface = wtypes.text + default_rescue_interface = wtypes.text default_storage_interface = wtypes.text default_vendor_interface = wtypes.text @@ -115,6 +120,7 @@ class Driver(base.APIBase): enabled_network_interfaces = [wtypes.text] enabled_power_interfaces = [wtypes.text] enabled_raid_interfaces = [wtypes.text] + enabled_rescue_interfaces = [wtypes.text] enabled_storage_interfaces = [wtypes.text] enabled_vendor_interfaces = [wtypes.text] diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 429e92a7f2..38a1c34c74 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -159,6 +159,9 @@ def hide_fields_in_newer_versions(obj): if not api_utils.allow_traits(): obj.traits = wsme.Unset + if not api_utils.allow_rescue_interface(): + obj.rescue_interface = wsme.Unset + def update_state_in_older_versions(obj): """Change provision state names for API backwards compatibility. @@ -546,10 +549,10 @@ class NodeStatesController(rest.RestController): @METRICS.timer('NodeStatesController.provision') @expose.expose(None, types.uuid_or_name, wtypes.text, - wtypes.text, types.jsontype, + wtypes.text, types.jsontype, wtypes.text, status_code=http_client.ACCEPTED) def provision(self, node_ident, target, configdrive=None, - clean_steps=None): + clean_steps=None, rescue_password=None): """Asynchronous trigger the provisioning of the node. This will set the target provision state of the node, and a @@ -582,6 +585,9 @@ class NodeStatesController(rest.RestController): 'args': {'force': True} } This is required (and only valid) when target is "clean". + :param rescue_password: A string representing the password to be set + inside the rescue environment. This is required (and only valid), + when target is "rescue". :raises: NodeLocked (HTTP 409) if the node is currently locked. :raises: ClientSideError (HTTP 409) if the node is already being provisioned. @@ -634,6 +640,13 @@ class NodeStatesController(rest.RestController): raise wsme.exc.ClientSideError( msg, status_code=http_client.BAD_REQUEST) + if (rescue_password is not None and + target != ir_states.VERBS['rescue']): + msg = (_('"rescue_password" is only valid when setting target ' + 'provision state to %s') % ir_states.VERBS['rescue']) + raise wsme.exc.ClientSideError( + msg, status_code=http_client.BAD_REQUEST) + # 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. @@ -644,6 +657,18 @@ class NodeStatesController(rest.RestController): rebuild=rebuild, configdrive=configdrive, topic=topic) + elif (target == ir_states.VERBS['unrescue']): + pecan.request.rpcapi.do_node_unrescue( + pecan.request.context, rpc_node.uuid, topic) + elif (target == ir_states.VERBS['rescue']): + if not (rescue_password and rescue_password.strip()): + msg = (_('A non-empty "rescue_password" is required when ' + 'setting target provision state to %s') % + ir_states.VERBS['rescue']) + raise wsme.exc.ClientSideError( + msg, status_code=http_client.BAD_REQUEST) + pecan.request.rpcapi.do_node_rescue( + pecan.request.context, rpc_node.uuid, rescue_password, topic) elif target == ir_states.DELETED: pecan.request.rpcapi.do_node_tear_down( pecan.request.context, rpc_node.uuid, topic) @@ -947,6 +972,9 @@ class Node(base.APIBase): raid_interface = wsme.wsattr(wtypes.text) """The raid interface to be used for this node""" + rescue_interface = wsme.wsattr(wtypes.text) + """The rescue interface to be used for this node""" + storage_interface = wsme.wsattr(wtypes.text) """The storage interface to be used for this node""" @@ -1110,7 +1138,7 @@ class Node(base.APIBase): deploy_interface=None, inspect_interface=None, management_interface=None, power_interface=None, raid_interface=None, vendor_interface=None, - storage_interface=None, traits=[]) + storage_interface=None, traits=[], rescue_interface=None) # NOTE(matty_dubs): The chassis_uuid getter() is based on the # _chassis_uuid variable: sample._chassis_uuid = 'edcad704-b2da-41d5-96d9-afd580ecfa12' @@ -1748,6 +1776,10 @@ class NodesController(rest.RestController): "be set via the node traits API.") raise exception.Invalid(msg) + if (not api_utils.allow_rescue_interface() and + node.rescue_interface is not wtypes.Unset): + raise exception.NotAcceptable() + # NOTE(deva): get_topic_for checks if node.driver is in the hash ring # and raises NoValidHost if it is not. # We need to ensure that node has a UUID before it can @@ -1825,6 +1857,10 @@ class NodesController(rest.RestController): "should be updated via the node traits API.") raise exception.Invalid(msg) + r_interface = api_utils.get_patch_values(patch, '/rescue_interface') + if r_interface and not api_utils.allow_rescue_interface(): + raise exception.NotAcceptable() + rpc_node = api_utils.get_rpc_node(node_ident) remove_inst_uuid_patch = [{'op': 'remove', 'path': '/instance_uuid'}] diff --git a/ironic/api/controllers/v1/ramdisk.py b/ironic/api/controllers/v1/ramdisk.py index a2afc0e72e..9d0c5e82b0 100644 --- a/ironic/api/controllers/v1/ramdisk.py +++ b/ironic/api/controllers/v1/ramdisk.py @@ -38,7 +38,8 @@ _LOOKUP_RETURN_FIELDS = ('uuid', 'properties', 'instance_info', 'driver_internal_info') _LOOKUP_ALLOWED_STATES = {states.DEPLOYING, states.DEPLOYWAIT, states.CLEANING, states.CLEANWAIT, - states.INSPECTING} + states.INSPECTING, + states.RESCUING, states.RESCUEWAIT} def config(): diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 4c50bb8bb9..cc5ba12208 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -54,6 +54,8 @@ MIN_VERB_VERSIONS = { states.VERBS['abort']: versions.MINOR_13_ABORT_VERB, states.VERBS['clean']: versions.MINOR_15_MANUAL_CLEAN, states.VERBS['adopt']: versions.MINOR_17_ADOPT_VERB, + states.VERBS['rescue']: versions.MINOR_38_RESCUE_INTERFACE, + states.VERBS['unrescue']: versions.MINOR_38_RESCUE_INTERFACE, } V31_FIELDS = [ @@ -325,6 +327,8 @@ def check_allowed_fields(fields): raise exception.NotAcceptable() if 'traits' in fields and not allow_traits(): raise exception.NotAcceptable() + if 'rescue_interface' in fields and not allow_rescue_interface(): + raise exception.NotAcceptable() def check_allowed_portgroup_fields(fields): @@ -635,6 +639,14 @@ def allow_agent_version_in_heartbeat(): versions.MINOR_36_AGENT_VERSION_HEARTBEAT) +def allow_rescue_interface(): + """Check if we should support rescue and unrescue operations and interface. + + Version 1.38 of the API added support for rescue and unrescue. + """ + return pecan.request.version.minor >= versions.MINOR_38_RESCUE_INTERFACE + + 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 f81e5e8289..7166bafb57 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -74,6 +74,8 @@ BASE_VERSION = 1 # v1.35: Add ability to provide configdrive when rebuilding node. # v1.36: Add Ironic Python Agent version support. # v1.37: Add node traits. +# v1.38: Add rescue and unrescue provision states +# Add rescue_interface to the node object MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -113,6 +115,7 @@ MINOR_34_PORT_PHYSICAL_NETWORK = 34 MINOR_35_REBUILD_CONFIG_DRIVE = 35 MINOR_36_AGENT_VERSION_HEARTBEAT = 36 MINOR_37_NODE_TRAITS = 37 +MINOR_38_RESCUE_INTERFACE = 38 # When adding another version, update: # - MINOR_MAX_VERSION @@ -120,7 +123,7 @@ MINOR_37_NODE_TRAITS = 37 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_37_NODE_TRAITS +MINOR_MAX_VERSION = MINOR_38_RESCUE_INTERFACE # String representations of the minor and maximum versions _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 6b0e944a72..ba6a1131f1 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -121,7 +121,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.37', + 'api': '1.38', 'rpc': '1.44', 'objects': { 'Node': ['1.23'], diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index e178179dd0..fde7cabc09 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -1820,11 +1820,6 @@ class ConductorManager(base_manager.BaseConductorManager): task.node.instance_info) task.node.driver_internal_info['is_whole_disk_image'] = iwdi for iface_name in task.driver.non_vendor_interfaces: - # TODO(stendulker): Remove this check in 'rescue' API patch - # Do not have to return the validation result for 'rescue' - # interface. - if iface_name == 'rescue': - continue iface = getattr(task.driver, iface_name, None) result = reason = None if iface: diff --git a/ironic/tests/unit/api/controllers/v1/test_driver.py b/ironic/tests/unit/api/controllers/v1/test_driver.py index 3499806767..ed53ce58ea 100644 --- a/ironic/tests/unit/api/controllers/v1/test_driver.py +++ b/ironic/tests/unit/api/controllers/v1/test_driver.py @@ -48,7 +48,7 @@ class TestListDrivers(base.BaseApiTest): self.dbapi.register_conductor_hardware_interfaces( c.id, self.d3, 'deploy', ['iscsi', 'direct'], 'direct') - def _test_drivers(self, use_dynamic, detail=False, storage_if=False): + def _test_drivers(self, use_dynamic, detail=False, latest_if=False): self.register_fake_conductors() headers = {} expected = [ @@ -58,8 +58,8 @@ class TestListDrivers(base.BaseApiTest): ] expected = sorted(expected, key=lambda d: d['name']) if use_dynamic: - if storage_if: - headers[api_base.Version.string] = '1.33' + if latest_if: + headers[api_base.Version.string] = '1.38' else: headers[api_base.Version.string] = '1.30' @@ -86,10 +86,14 @@ class TestListDrivers(base.BaseApiTest): # as this case can't actually happen. if detail: self.assertIn('default_deploy_interface', d) - if storage_if: + if latest_if: + self.assertIn('default_rescue_interface', d) + self.assertIn('enabled_rescue_interfaces', d) self.assertIn('default_storage_interface', d) self.assertIn('enabled_storage_interfaces', d) else: + self.assertNotIn('default_rescue_interface', d) + self.assertNotIn('enabled_rescue_interfaces', d) self.assertNotIn('default_storage_interface', d) self.assertNotIn('enabled_storage_interfaces', d) else: @@ -103,7 +107,7 @@ class TestListDrivers(base.BaseApiTest): def test_drivers_with_dynamic(self): self._test_drivers(True) - def _test_drivers_with_dynamic_detailed(self, storage_if=False): + def _test_drivers_with_dynamic_detailed(self, latest_if=False): with mock.patch.object(self.dbapi, 'list_hardware_type_interfaces', autospec=True) as mock_hw: mock_hw.return_value = [ @@ -121,13 +125,13 @@ class TestListDrivers(base.BaseApiTest): }, ] - self._test_drivers(True, detail=True, storage_if=storage_if) + self._test_drivers(True, detail=True, latest_if=latest_if) def test_drivers_with_dynamic_detailed(self): self._test_drivers_with_dynamic_detailed() def test_drivers_with_dynamic_detailed_storage_interface(self): - self._test_drivers_with_dynamic_detailed(storage_if=True) + self._test_drivers_with_dynamic_detailed(latest_if=True) def _test_drivers_type_filter(self, requested_type): self.register_fake_conductors() @@ -179,7 +183,7 @@ class TestListDrivers(base.BaseApiTest): @mock.patch.object(rpcapi.ConductorAPI, 'get_driver_properties') def _test_drivers_get_one_ok(self, use_dynamic, mock_driver_properties, - storage_if=False): + latest_if=False): # get_driver_properties mock is required by validate_link() self.register_fake_conductors() @@ -193,8 +197,8 @@ class TestListDrivers(base.BaseApiTest): hosts = [self.h1] headers = {} - if storage_if: - headers[api_base.Version.string] = '1.33' + if latest_if: + headers[api_base.Version.string] = '1.38' else: headers[api_base.Version.string] = '1.30' @@ -208,12 +212,7 @@ class TestListDrivers(base.BaseApiTest): if use_dynamic: for iface in driver_base.ALL_INTERFACES: - # TODO(stendulker): Remove this check in 'rescue' API - # patch. - if iface == 'rescue': - self.assertNotIn('default_rescue_interface', data) - self.assertNotIn('enabled_rescue_interfaces', data) - elif storage_if or iface != 'storage': + if latest_if or iface not in ['rescue', 'storage']: self.assertIn('default_%s_interface' % iface, data) self.assertIn('enabled_%s_interfaces' % iface, data) self.assertIsNotNone(data['default_deploy_interface']) @@ -230,7 +229,7 @@ class TestListDrivers(base.BaseApiTest): def test_drivers_get_one_ok_classic(self): self._test_drivers_get_one_ok(False) - def _test_drivers_get_one_ok_dynamic(self, storage_if=False): + def _test_drivers_get_one_ok_dynamic(self, latest_if=False): with mock.patch.object(self.dbapi, 'list_hardware_type_interfaces', autospec=True) as mock_hw: mock_hw.return_value = [ @@ -248,14 +247,14 @@ class TestListDrivers(base.BaseApiTest): }, ] - self._test_drivers_get_one_ok(True, storage_if=storage_if) + self._test_drivers_get_one_ok(True, latest_if=latest_if) mock_hw.assert_called_once_with([self.d3]) - def test_drivers_get_one_ok_dynamic(self): + def test_drivers_get_one_ok_dynamic_base_interfaces(self): self._test_drivers_get_one_ok_dynamic() - def test_drivers_get_one_ok_dynamic_storage_interface(self): - self._test_drivers_get_one_ok_dynamic(storage_if=True) + def test_drivers_get_one_ok_dynamic_latest_interfaces(self): + self._test_drivers_get_one_ok_dynamic(latest_if=True) def test_driver_properties_hidden_in_lower_version(self): self.register_fake_conductors() diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 5b0490a26d..2a00752dee 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -2250,7 +2250,7 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('neutron', result['network_interface']) def test_create_node_specify_interfaces(self): - headers = {api_base.Version.string: '1.33'} + headers = {api_base.Version.string: '1.38'} all_interface_fields = api_utils.V31_FIELDS + ['network_interface', 'rescue_interface', 'storage_interface'] @@ -2268,11 +2268,6 @@ class TestPost(test_api_base.BaseApiTest): expected = 'flat' elif field == 'storage_interface': expected = 'noop' - elif field == 'rescue_interface': - # TODO(stendulker): Enable testing of rescue interface - # in its API patch. - continue - node = { 'uuid': uuidutils.generate_uuid(), field: expected, @@ -2955,6 +2950,12 @@ class TestPut(test_api_base.BaseApiTest): p = mock.patch.object(rpcapi.ConductorAPI, 'inspect_hardware') self.mock_dnih = p.start() self.addCleanup(p.stop) + p = mock.patch.object(rpcapi.ConductorAPI, 'do_node_rescue') + self.mock_dnr = p.start() + self.addCleanup(p.stop) + p = mock.patch.object(rpcapi.ConductorAPI, 'do_node_unrescue') + self.mock_dnur = p.start() + self.addCleanup(p.stop) def _test_power_state_success(self, target_state, timeout, api_version): if timeout is None: @@ -3326,6 +3327,271 @@ class TestPut(test_api_base.BaseApiTest): states.VERBS['provide'], 'test-topic') + def test_rescue_raises_error_before_1_38(self): + """Test that a lower API client cannot use the rescue verb""" + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['rescue'], + 'rescue_password': 'password'}, + headers={api_base.Version.string: "1.37"}, + expect_errors=True) + self.assertEqual(http_client.NOT_ACCEPTABLE, ret.status_code) + + def test_unrescue_raises_error_before_1_38(self): + """Test that a lower API client cannot use the unrescue verb""" + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['unrescue']}, + headers={api_base.Version.string: "1.37"}, + expect_errors=True) + self.assertEqual(http_client.NOT_ACCEPTABLE, ret.status_code) + + def test_provision_unexpected_rescue_password(self): + self.node.provision_state = states.AVAILABLE + self.node.save() + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.ACTIVE, + 'rescue_password': 'password'}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertFalse(self.mock_dnr.called) + + def test_provision_rescue_no_password(self): + self.node.provision_state = states.ACTIVE + self.node.save() + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['rescue']}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertFalse(self.mock_dnr.called) + + def test_provision_rescue_empty_password(self): + self.node.provision_state = states.ACTIVE + self.node.save() + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['rescue'], + 'rescue_password': ' '}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertFalse(self.mock_dnr.called) + + def test_provision_rescue_in_active(self): + self.node.provision_state = states.ACTIVE + self.node.save() + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['rescue'], + 'rescue_password': 'password'}, + headers={api_base.Version.string: "1.38"}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dnr.assert_called_once_with( + mock.ANY, self.node.uuid, 'password', 'test-topic') + + def test_provision_rescue_in_deleting(self): + node = self.node + node.provision_state = states.DELETING + node.target_provision_state = states.AVAILABLE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['rescue'], + 'rescue_password': 'password'}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertFalse(self.mock_dnr.called) + + def test_provision_rescue_in_rescue(self): + node = self.node + node.provision_state = states.RESCUE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['rescue'], + 'rescue_password': 'password'}, + headers={api_base.Version.string: "1.38"}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dnr.assert_called_once_with( + mock.ANY, self.node.uuid, 'password', 'test-topic') + + def test_provision_rescue_in_rescuefail(self): + node = self.node + node.provision_state = states.RESCUEFAIL + node.target_provision_state = states.RESCUE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['rescue'], + 'rescue_password': 'password'}, + headers={api_base.Version.string: "1.38"}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dnr.assert_called_once_with( + mock.ANY, self.node.uuid, 'password', 'test-topic') + + def test_provision_rescue_in_rescuewait(self): + node = self.node + node.provision_state = states.RESCUEWAIT + node.target_provision_state = states.RESCUE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['rescue'], + 'rescue_password': 'password'}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertFalse(self.mock_dnr.called) + + def test_provision_rescue_in_rescuing(self): + node = self.node + node.provision_state = states.RESCUING + node.target_provision_state = states.RESCUE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['rescue'], + 'rescue_password': 'password'}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertFalse(self.mock_dnr.called) + + def test_provision_rescue_in_unrescuefail(self): + node = self.node + node.provision_state = states.UNRESCUEFAIL + node.target_provision_state = states.ACTIVE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['rescue'], + 'rescue_password': 'password'}, + headers={api_base.Version.string: "1.38"}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dnr.assert_called_once_with( + mock.ANY, self.node.uuid, 'password', 'test-topic') + + def test_provision_rescue_in_unrescuing(self): + node = self.node + node.provision_state = states.UNRESCUING + node.target_provision_state = states.ACTIVE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['rescue']}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertFalse(self.mock_dnr.called) + + def test_provision_unrescue_in_active(self): + node = self.node + node.provision_state = states.ACTIVE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['unrescue']}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertFalse(self.mock_dnur.called) + + def test_provision_unrescue_in_deleting(self): + node = self.node + node.provision_state = states.DELETING + node.target_provision_state = states.AVAILABLE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['unrescue']}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertFalse(self.mock_dnur.called) + + def test_provision_unrescue_in_rescue(self): + node = self.node + node.provision_state = states.RESCUE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['unrescue']}, + headers={api_base.Version.string: "1.38"}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dnur.assert_called_once_with( + mock.ANY, self.node.uuid, 'test-topic') + + def test_provision_unrescue_in_rescuefail(self): + node = self.node + node.provision_state = states.RESCUEFAIL + node.target_provision_state = states.RESCUE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['unrescue']}, + headers={api_base.Version.string: "1.38"}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dnur.assert_called_once_with( + mock.ANY, self.node.uuid, 'test-topic') + + def test_provision_unrescue_in_rescuewait(self): + node = self.node + node.provision_state = states.RESCUEWAIT + node.target_provision_state = states.RESCUE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['unrescue']}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertFalse(self.mock_dnur.called) + + def test_provision_unrescue_in_rescuing(self): + node = self.node + node.provision_state = states.RESCUING + node.target_provision_state = states.RESCUE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['unrescue']}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertFalse(self.mock_dnur.called) + + def test_provision_unrescue_in_unrescuefail(self): + node = self.node + node.provision_state = states.UNRESCUEFAIL + node.target_provision_state = states.ACTIVE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['unrescue']}, + headers={api_base.Version.string: "1.38"}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dnur.assert_called_once_with( + mock.ANY, self.node.uuid, 'test-topic') + + def test_provision_unrescue_in_unrescuing(self): + node = self.node + node.provision_state = states.UNRESCUING + node.target_provision_state = states.ACTIVE + node.reservation = 'fake-host' + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.VERBS['unrescue']}, + headers={api_base.Version.string: "1.38"}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertFalse(self.mock_dnur.called) + def test_inspect_already_in_progress(self): node = self.node node.provision_state = states.INSPECTING diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index 96d8d6e583..cc962a6bac 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/ironic/tests/unit/api/controllers/v1/test_utils.py @@ -201,6 +201,14 @@ class TestApiUtils(base.TestCase): utils.check_allowed_fields, ['resource_class']) + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_check_allowed_fields_rescue_interface_fail(self, mock_request): + mock_request.version.minor = 31 + self.assertRaises( + exception.NotAcceptable, + utils.check_allowed_fields, + ['rescue_interface']) + @mock.patch.object(pecan, 'request', spec_set=['version']) def test_check_allowed_portgroup_fields_mode_properties(self, mock_request): @@ -499,6 +507,13 @@ class TestApiUtils(base.TestCase): mock_request.version.minor = 34 utils.check_allow_configdrive(states.ACTIVE) + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_allow_rescue_interface(self, mock_request): + mock_request.version.minor = 38 + self.assertTrue(utils.allow_rescue_interface()) + mock_request.version.minor = 37 + self.assertFalse(utils.allow_rescue_interface()) + class TestNodeIdent(base.TestCase): diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index d513cead21..793ba933bc 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -3223,12 +3223,13 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn, 'otherdriver')) @mock.patch.object(images, 'is_whole_disk_image') - def test_validate_driver_interfaces(self, mock_iwdi): + def test_validate_dynamic_driver_interfaces(self, mock_iwdi): mock_iwdi.return_value = False target_raid_config = {'logical_disks': [{'size_gb': 1, 'raid_level': '1'}]} node = obj_utils.create_test_node( - self.context, driver='fake', target_raid_config=target_raid_config, + self.context, driver='fake-hardware', + target_raid_config=target_raid_config, network_interface='noop') ret = self.service.validate_driver_interfaces(self.context, node.uuid) @@ -3240,8 +3241,32 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn, 'raid': {'result': True}, 'deploy': {'result': True}, 'network': {'result': True}, - 'storage': {'result': True}} + 'storage': {'result': True}, + 'rescue': {'result': True}} + self.assertEqual(expected, ret) + mock_iwdi.assert_called_once_with(self.context, node.instance_info) + @mock.patch.object(images, 'is_whole_disk_image') + def test_validate_driver_interfaces(self, mock_iwdi): + mock_iwdi.return_value = False + target_raid_config = {'logical_disks': [{'size_gb': 1, + 'raid_level': '1'}]} + node = obj_utils.create_test_node( + self.context, driver='fake', target_raid_config=target_raid_config, + network_interface='noop') + ret = self.service.validate_driver_interfaces(self.context, + node.uuid) + reason = ('not supported') + expected = {'console': {'result': True}, + 'power': {'result': True}, + 'inspect': {'result': True}, + 'management': {'result': True}, + 'boot': {'result': True}, + 'raid': {'result': True}, + 'deploy': {'result': True}, + 'network': {'result': True}, + 'storage': {'result': True}, + 'rescue': {'reason': reason, 'result': None}} self.assertEqual(expected, ret) mock_iwdi.assert_called_once_with(self.context, node.instance_info) diff --git a/releasenotes/notes/rescue-node-87e3b673c61ef628.yaml b/releasenotes/notes/rescue-node-87e3b673c61ef628.yaml new file mode 100644 index 0000000000..f57b76aed2 --- /dev/null +++ b/releasenotes/notes/rescue-node-87e3b673c61ef628.yaml @@ -0,0 +1,39 @@ +--- +features: + - | + Adds version 1.38 of the Bare Metal API, which provides supports for + rescuing (and unrescuing) a node. This includes: + + * A node in the ``active`` provision state can be rescued via the + ``GET /v1/nodes/{node_ident}/states/provision`` API, by specifying + ``rescue`` as the ``target`` value, and a ``rescue_password`` + value. When the node has been rescued, it will be in the ``rescue`` + provision state. A rescue ramdisk will be running, configured with + the specified ``rescue_password``, and listening with ssh on the + tenant network. + + * A node in the ``rescue`` provision state can be unrescued (to the + ``active`` state) via the + ``GET /v1/nodes/{node_ident}/states/provision`` API, by specifying + ``unrescue`` as the ``target`` value. + + * The ``rescue_interface`` field of the node resource. A rescue + interface can be set when creating or updating a node. + + * It also exposes ``default_rescue_interface`` and + ``enable_rescue_interfaces`` fields of the driver resource. + + * Adds new configuration options ``[DEFAULT]/enabled_rescue_interfaces`` + and ``[DEFAULT]/default_rescue_interface``. Rescue interfaces are + enabled via the ``[DEFAULT]/enabled_rescue_interfaces``. A default + rescue interface to use when creating or updating nodes can be + specified with the ``[DEFAULT]/enabled_rescue_interfaces``. + + * Adds new options ``[conductor]/check_rescue_state_interval`` and + ``[conductor]/rescue_callback_timeout`` to fail the rescue operation + upon timeout, for the nodes that are stuck in the rescue wait state. + + * Adds support for providing separate ``rescuing`` network with its + security groups using new options ``[neutron]/rescuing_network`` and + ``[neutron]/rescuing_network_security_groups`` respectively. It is + required to provide ``[neutron]/rescuing_network``. diff --git a/tox.ini b/tox.ini index 38e7884bf2..9f026b0e03 100644 --- a/tox.ini +++ b/tox.ini @@ -105,7 +105,7 @@ filename = *.py,app.wsgi exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build import-order-style = pep8 application-import-names = ironic -max-complexity=17 +max-complexity=18 # [H106] Don’t put vim configuration in source files. # [H203] Use assertIs(Not)None to check for None. # [H204] Use assert(Not)Equal to check for equality.