diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index d37cdb3a67..81522fb5a8 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.45 (Rocky, master) +-------------------- + +Added ``reset_interfaces`` parameter to node's PATCH request, to specify +whether to reset hardware interfaces to their defaults on driver's update. + 1.44 (Rocky, master) -------------------- diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 7b3fac63a9..1a52e2fea9 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1934,7 +1934,7 @@ class NodesController(rest.RestController): return api_node # NOTE (yolanda): isolate validation to avoid patch too complex pep error - def _validate_patch(self, patch): + def _validate_patch(self, patch, reset_interfaces): if self.from_chassis: raise exception.OperationNotPermitted() @@ -1969,20 +1969,33 @@ class NodesController(rest.RestController): if b_interface and not api_utils.allow_bios_interface(): raise exception.NotAcceptable() + driver = api_utils.get_patch_values(patch, '/driver') + if reset_interfaces and not driver: + msg = _("The reset_interfaces parameter can only be used when " + "changing the node's driver.") + raise exception.Invalid(msg) + @METRICS.timer('NodesController.patch') - @wsme.validate(types.uuid, [NodePatchType]) - @expose.expose(Node, types.uuid_or_name, body=[NodePatchType]) - def patch(self, node_ident, patch): + @wsme.validate(types.uuid, types.boolean, [NodePatchType]) + @expose.expose(Node, types.uuid_or_name, types.boolean, + body=[NodePatchType]) + def patch(self, node_ident, reset_interfaces=None, patch=None): """Update an existing node. :param node_ident: UUID or logical name of a node. + :param reset_interfaces: whether to reset hardware interfaces to their + defaults. Only valid when updating the driver field. :param patch: a json PATCH document to apply to this node. """ context = pecan.request.context cdict = context.to_policy_values() policy.authorize('baremetal:node:update', cdict, cdict) - self._validate_patch(patch) + if (reset_interfaces is not None and not + api_utils.allow_reset_interfaces()): + raise exception.NotAcceptable() + + self._validate_patch(patch, reset_interfaces) rpc_node = api_utils.get_rpc_node_with_suffix(node_ident) @@ -2043,7 +2056,8 @@ class NodesController(rest.RestController): with notify.handle_error_notification(context, rpc_node, 'update', chassis_uuid=node.chassis_uuid): new_node = pecan.request.rpcapi.update_node(context, - rpc_node, topic) + rpc_node, topic, + reset_interfaces) api_node = Node.convert_with_links(new_node) notify.emit_end_notification(context, new_node, 'update', diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 070c72b64a..676734d28e 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -862,6 +862,12 @@ def allow_detail_query(): versions.MINOR_43_ENABLE_DETAIL_QUERY) +def allow_reset_interfaces(): + """Check if passing a reset_interfaces query string is allowed.""" + return (pecan.request.version.minor >= + versions.MINOR_45_RESET_INTERFACES) + + def get_request_return_fields(fields, detail, default_fields): """Calculate fields to return from an API request diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 965062f864..b0d336f760 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -82,6 +82,7 @@ BASE_VERSION = 1 # v1.42: Expose fault field to node. # v1.43: Add detail=True flag to all API endpoints # v1.44: Add node deploy_step field +# v1.45: reset_interfaces parameter to node's PATCH MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -128,6 +129,7 @@ MINOR_41_INSPECTION_ABORT = 41 MINOR_42_FAULT = 42 MINOR_43_ENABLE_DETAIL_QUERY = 43 MINOR_44_NODE_DEPLOY_STEP = 44 +MINOR_45_RESET_INTERFACES = 45 # When adding another version, update: # - MINOR_MAX_VERSION @@ -135,7 +137,7 @@ MINOR_44_NODE_DEPLOY_STEP = 44 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_44_NODE_DEPLOY_STEP +MINOR_MAX_VERSION = MINOR_45_RESET_INTERFACES # 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 01a626e3fd..dde8acd47c 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -115,8 +115,8 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.44', - 'rpc': '1.45', + 'api': '1.45', + 'rpc': '1.46', 'objects': { 'Node': ['1.26'], 'Conductor': ['1.2'], diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 5ddab21e4e..f760848e02 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -100,7 +100,7 @@ class ConductorManager(base_manager.BaseConductorManager): # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's. # NOTE(pas-ha): This also must be in sync with # ironic.common.release_mappings.RELEASE_MAPPING['master'] - RPC_API_VERSION = '1.45' + RPC_API_VERSION = '1.46' target = messaging.Target(version=RPC_API_VERSION) @@ -144,7 +144,7 @@ class ConductorManager(base_manager.BaseConductorManager): exception.NodeLocked, exception.InvalidState, exception.DriverNotFound) - def update_node(self, context, node_obj): + def update_node(self, context, node_obj, reset_interfaces=False): """Update a node with the supplied data. This method is the main "hub" for PUT and PATCH requests in the API. @@ -153,6 +153,8 @@ class ConductorManager(base_manager.BaseConductorManager): :param context: an admin context :param node_obj: a changed (but not saved) node object. + :param reset_interfaces: whether to reset hardware interfaces to their + defaults. :raises: NoValidDefaultForInterface if no default can be calculated for some interfaces, and explicit values must be provided. """ @@ -179,9 +181,13 @@ class ConductorManager(base_manager.BaseConductorManager): action = _("Node %(node)s can not have %(field)s " "updated unless it is in one of allowed " "(%(allowed)s) states or in maintenance mode.") + updating_driver = 'driver' in delta for iface in drivers_base.ALL_INTERFACES: interface_field = '%s_interface' % iface if interface_field not in delta: + if updating_driver and reset_interfaces: + setattr(node_obj, interface_field, None) + continue if not (node_obj.provision_state in allowed_update_states diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index c244a6623c..b644d8566f 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -94,13 +94,14 @@ class ConductorAPI(object): | 1.43 - Added do_node_rescue, do_node_unrescue and can_send_rescue | 1.44 - Added add_node_traits and remove_node_traits. | 1.45 - Added continue_node_deploy + | 1.46 - Added reset_interfaces to update_node """ # NOTE(rloo): This must be in sync with manager.ConductorManager's. # NOTE(pas-ha): This also must be in sync with # ironic.common.release_mappings.RELEASE_MAPPING['master'] - RPC_API_VERSION = '1.45' + RPC_API_VERSION = '1.46' def __init__(self, topic=None): super(ConductorAPI, self).__init__() @@ -186,7 +187,8 @@ class ConductorAPI(object): cctxt = self.client.prepare(topic=topic or self.topic, version='1.36') return cctxt.call(context, 'create_node', node_obj=node_obj) - def update_node(self, context, node_obj, topic=None): + def update_node(self, context, node_obj, topic=None, + reset_interfaces=False): """Synchronously, have a conductor update the node's information. Update the node's information in the database and return a node object. @@ -201,13 +203,16 @@ class ConductorAPI(object): :param context: request context. :param node_obj: a changed (but not saved) node object. :param topic: RPC topic. Defaults to self.topic. + :param reset_interfaces: whether to reset hardware interfaces to their + defaults. :returns: updated node object, including all fields. :raises: NoValidDefaultForInterface if no default can be calculated for some interfaces, and explicit values must be provided. """ cctxt = self.client.prepare(topic=topic or self.topic, version='1.1') - return cctxt.call(context, 'update_node', node_obj=node_obj) + return cctxt.call(context, 'update_node', node_obj=node_obj, + reset_interfaces=reset_interfaces) def change_node_power_state(self, context, node_id, new_state, topic=None, timeout=None): diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index ae10389033..e50bf87074 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -1586,7 +1586,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(self.mock_update_node.return_value.updated_at, timeutils.parse_isotime(response.json['updated_at'])) self.mock_update_node.assert_called_once_with( - mock.ANY, mock.ANY, 'test-topic') + mock.ANY, mock.ANY, 'test-topic', None) mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, @@ -1628,7 +1628,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(self.mock_update_node.return_value.updated_at, timeutils.parse_isotime(response.json['updated_at'])) self.mock_update_node.assert_called_once_with( - mock.ANY, mock.ANY, 'test-topic') + mock.ANY, mock.ANY, 'test-topic', None) def test_update_ok_by_name_with_json(self): self.mock_update_node.return_value = self.node @@ -1647,7 +1647,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(self.mock_update_node.return_value.updated_at, timeutils.parse_isotime(response.json['updated_at'])) self.mock_update_node.assert_called_once_with( - mock.ANY, mock.ANY, 'test-topic') + mock.ANY, mock.ANY, 'test-topic', None) def test_update_state(self): response = self.patch_json('/nodes/%s' % self.node.uuid, @@ -1675,7 +1675,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(http_client.BAD_REQUEST, response.status_code) self.mock_update_node.assert_called_once_with( - mock.ANY, mock.ANY, 'test-topic') + mock.ANY, mock.ANY, 'test-topic', None) mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, @@ -1697,6 +1697,42 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_code) + def test_update_with_reset_interfaces(self): + self.mock_update_node.return_value = self.node + (self + .mock_update_node + .return_value + .updated_at) = "2013-12-03T06:20:41.184720+00:00" + response = self.patch_json( + '/nodes/%s?reset_interfaces=True' % self.node.uuid, + [{'path': '/driver', 'value': 'ipmi', 'op': 'replace'}], + headers={api_base.Version.string: "1.45"}) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + self.assertEqual(self.mock_update_node.return_value.updated_at, + timeutils.parse_isotime(response.json['updated_at'])) + self.mock_update_node.assert_called_once_with( + mock.ANY, mock.ANY, 'test-topic', True) + + def test_reset_interfaces_without_driver(self): + response = self.patch_json( + '/nodes/%s?reset_interfaces=True' % self.node.uuid, + [{'path': '/name', 'value': 'new name', 'op': 'replace'}], + headers={api_base.Version.string: "1.45"}, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.BAD_REQUEST, response.status_code) + self.assertFalse(self.mock_update_node.called) + + def test_reset_interfaces_not_supported(self): + response = self.patch_json( + '/nodes/%s?reset_interfaces=True' % self.node.uuid, + [{'path': '/driver', 'value': 'ipmi', 'op': 'replace'}], + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + self.assertFalse(self.mock_update_node.called) + def test_add_ok(self): self.mock_update_node.return_value = self.node @@ -1708,7 +1744,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(http_client.OK, response.status_code) self.mock_update_node.assert_called_once_with( - mock.ANY, mock.ANY, 'test-topic') + mock.ANY, mock.ANY, 'test-topic', None) def test_add_root(self): self.mock_update_node.return_value = self.node @@ -1720,7 +1756,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.OK, response.status_code) self.mock_update_node.assert_called_once_with( - mock.ANY, mock.ANY, 'test-topic') + mock.ANY, mock.ANY, 'test-topic', None) def test_add_root_non_existent(self): response = self.patch_json('/nodes/%s' % self.node.uuid, @@ -1741,7 +1777,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(http_client.OK, response.status_code) self.mock_update_node.assert_called_once_with( - mock.ANY, mock.ANY, 'test-topic') + mock.ANY, mock.ANY, 'test-topic', None) def test_remove_non_existent_property_fail(self): response = self.patch_json('/nodes/%s' % self.node.uuid, @@ -1809,7 +1845,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.OK, response.status_code) self.mock_update_node.assert_called_once_with( - mock.ANY, mock.ANY, 'test-topic') + mock.ANY, mock.ANY, 'test-topic', None) def test_patch_ports_subresource_no_port_id(self): response = self.patch_json('/nodes/%s/ports' % self.node.uuid, @@ -2013,7 +2049,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(http_client.OK, response.status_code) self.mock_update_node.assert_called_once_with( - mock.ANY, mock.ANY, 'test-topic') + mock.ANY, mock.ANY, 'test-topic', None) def test_replace_maintenance_by_name(self): self.mock_update_node.return_value = self.node @@ -2027,7 +2063,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(http_client.OK, response.status_code) self.mock_update_node.assert_called_once_with( - mock.ANY, mock.ANY, 'test-topic') + mock.ANY, mock.ANY, 'test-topic', None) def test_replace_consoled_enabled(self): response = self.patch_json('/nodes/%s' % self.node.uuid, diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index a826e342d7..b0851297fc 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -567,17 +567,19 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.assertEqual(existing_driver, node.driver) UpdateInterfaces = namedtuple('UpdateInterfaces', ('old', 'new')) + # NOTE(dtantsur): "old" interfaces here do not match the defaults, so that + # we can test resetting them. IFACE_UPDATE_DICT = { - 'boot_interface': UpdateInterfaces(None, 'fake'), - 'console_interface': UpdateInterfaces(None, 'fake'), - 'deploy_interface': UpdateInterfaces(None, 'fake'), - 'inspect_interface': UpdateInterfaces(None, 'fake'), + 'boot_interface': UpdateInterfaces('pxe', 'fake'), + 'console_interface': UpdateInterfaces('no-console', 'fake'), + 'deploy_interface': UpdateInterfaces('iscsi', 'fake'), + 'inspect_interface': UpdateInterfaces('no-inspect', 'fake'), 'management_interface': UpdateInterfaces(None, 'fake'), - 'network_interface': UpdateInterfaces('flat', 'noop'), + 'network_interface': UpdateInterfaces('noop', 'flat'), 'power_interface': UpdateInterfaces(None, 'fake'), - 'raid_interface': UpdateInterfaces(None, 'fake'), - 'rescue_interface': UpdateInterfaces(None, 'no-rescue'), - 'storage_interface': UpdateInterfaces('noop', 'fake'), + 'raid_interface': UpdateInterfaces('no-raid', 'fake'), + 'rescue_interface': UpdateInterfaces('no-rescue', 'fake'), + 'storage_interface': UpdateInterfaces('fake', 'noop'), } def _create_node_with_interfaces(self, prov_state, maintenance=False): @@ -585,6 +587,7 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): for iface_name, ifaces in self.IFACE_UPDATE_DICT.items(): old_ifaces[iface_name] = ifaces.old node = obj_utils.create_test_node(self.context, driver='fake-hardware', + uuid=uuidutils.generate_uuid(), provision_state=prov_state, maintenance=maintenance, **old_ifaces) @@ -652,6 +655,30 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): for iface_name in self.IFACE_UPDATE_DICT: self._test_update_node_interface_invalid(node, iface_name) + def test_update_node_with_reset_interfaces(self): + # Modify only one interface at a time + for iface_name, ifaces in self.IFACE_UPDATE_DICT.items(): + node = self._create_node_with_interfaces(states.AVAILABLE) + setattr(node, iface_name, ifaces.new) + # Updating a driver is mandatory for reset_interfaces to work + node.driver = 'fake-hardware' + self.service.update_node(self.context, node, + reset_interfaces=True) + node.refresh() + self.assertEqual(ifaces.new, getattr(node, iface_name)) + # Other interfaces must be reset to their defaults + for other_iface_name, ifaces in self.IFACE_UPDATE_DICT.items(): + if other_iface_name == iface_name: + continue + # For this to work, the "old" interfaces in IFACE_UPDATE_DICT + # must not match the defaults. + self.assertNotEqual(ifaces.old, + getattr(node, other_iface_name), + "%s does not match the default after " + "reset with setting %s: %s" % + (other_iface_name, iface_name, + getattr(node, other_iface_name))) + def _test_update_node_change_resource_class(self, state, resource_class=None, new_resource_class='new', diff --git a/releasenotes/notes/reset-interface-e62036ac76b87486.yaml b/releasenotes/notes/reset-interface-e62036ac76b87486.yaml new file mode 100644 index 0000000000..77a1cf9595 --- /dev/null +++ b/releasenotes/notes/reset-interface-e62036ac76b87486.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Starting with API version 1.45, PATCH requests to ``/v1/nodes/`` + accept the new parameter ``reset_interfaces``. If can be provided whenever + the ``driver`` field is updated to reset all hardware interfaces to their + defaults, expect for ones updated in the same request.