openstack/ironic
Code Issues Proposed changes

Add reset_interfaces parameter to node's PATCH

Browse Source 
Setting it to true when updating the node's driver resets all hardware
interfaces to their defaults, unless they're also updated.

Change-Id: Ie70dbbed2be0a46f024b859e8141572500fb47c6
Story: #2002868
Task: #22818
This commit is contained in:
Dmitry Tantsur 2018-07-16 15:03:42 +02:00 committed by Jim Rollenhagen
parent 072cf9b560
commit 597d8a727b
10 changed files with 141 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
doc/source/contributor/webapi-version-history.rst
View File

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

26
ironic/api/controllers/v1/node.py
View File

@ -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',

6
ironic/api/controllers/v1/utils.py
View File

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

4
ironic/api/controllers/v1/versions.py
View File

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

4
ironic/common/release_mappings.py
View File

@ -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'],

10
ironic/conductor/manager.py
View File

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

11
ironic/conductor/rpcapi.py
View File

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

56
ironic/tests/unit/api/controllers/v1/test_node.py
View File

@ -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,

43
ironic/tests/unit/conductor/test_manager.py
View File

@ -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',

7
releasenotes/notes/reset-interface-e62036ac76b87486.yaml Normal file
View File

@ -0,0 +1,7 @@
---
features:
- |
Starting with API version 1.45, PATCH requests to ``/v1/nodes/<NODE>``
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.