From 4789d3b41ad045f201351ea40097dfd895bfc8a9 Mon Sep 17 00:00:00 2001 From: Sam Betts Date: Wed, 30 Nov 2016 18:43:35 +0000 Subject: [PATCH] Add Virtual Network Interface REST APIs This patch adds the REST APIs for the virtual network interface API in order to abstract the task of assigning logical network interfaces to physical network interfaces. Since Newton Ironic provides an interface for pluggable network implementations. Different network implementations may want to handle how logical to physical network interface assignment happens. To do this the new API calls into new functions on the network implementation loaded for the specified node. This is part 3 of 3, and adds the node vif subcontroller to expose the /nodes//vifs REST API endpoint. API version is bumped to 1.28. Co-Authored-By: Vasyl Saienko (vsaienko@mirantis.com) Change-Id: I70f1166a15a26f392734e21d6bc30a03da4e5486 Partial-Bug: #1582188 --- doc/source/dev/webapi-version-history.rst | 4 + etc/ironic/policy.json.sample | 6 + ironic/api/controllers/v1/node.py | 88 ++++++- ironic/api/controllers/v1/port.py | 3 + ironic/api/controllers/v1/types.py | 30 +++ ironic/api/controllers/v1/utils.py | 10 + ironic/api/controllers/v1/versions.py | 4 +- ironic/common/policy.py | 9 + ironic/common/utils.py | 12 + ironic/drivers/modules/network/common.py | 5 + ironic/tests/unit/api/v1/test_nodes.py | 224 ++++++++++++++++++ ironic/tests/unit/api/v1/test_ports.py | 16 +- ironic/tests/unit/api/v1/test_types.py | 25 ++ ironic/tests/unit/common/test_utils.py | 10 + .../drivers/modules/network/test_common.py | 38 ++- .../services/baremetal/base.py | 27 ++- .../baremetal/v1/json/baremetal_client.py | 40 ++++ .../tests/api/admin/test_nodes.py | 31 +++ .../tests/scenario/baremetal_manager.py | 5 + .../scenario/test_baremetal_basic_ops.py | 10 +- ...ttach-detach-support-99eca43eea6e5a30.yaml | 8 + 21 files changed, 589 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/add-vif-attach-detach-support-99eca43eea6e5a30.yaml diff --git a/doc/source/dev/webapi-version-history.rst b/doc/source/dev/webapi-version-history.rst index 2f8e333226..1b99e7c0b9 100644 --- a/doc/source/dev/webapi-version-history.rst +++ b/doc/source/dev/webapi-version-history.rst @@ -2,6 +2,10 @@ REST API Version History ======================== +**1.28** (Ocata) + + Add '/v1/nodes//vifs' endpoint for attach, detach and list of VIFs. + **1.27** (Ocata) Add ``soft rebooting`` and ``soft power off`` as possible values diff --git a/etc/ironic/policy.json.sample b/etc/ironic/policy.json.sample index e328eade25..1b196b2752 100644 --- a/etc/ironic/policy.json.sample +++ b/etc/ironic/policy.json.sample @@ -42,6 +42,12 @@ "baremetal:node:get_console": "rule:is_admin" # Change Node console status "baremetal:node:set_console_state": "rule:is_admin" +# List VIFs attached to node +"baremetal:node:vif:list": "rule:is_admin" +# Attach a VIF to a node +"baremetal:node:vif:attach": "rule:is_admin" +# Detach a VIF from a node +"baremetal:node:vif:detach": "rule:is_admin" # Retrieve Port records "baremetal:port:get": "rule:is_admin or rule:is_observer" # Create Port records diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 9885f6f244..548594782d 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1071,6 +1071,76 @@ class NodeMaintenanceController(rest.RestController): self._set_maintenance(node_ident, False) +# NOTE(vsaienko) We don't support pagination with VIFs, so we don't use +# collection.Collection here. +class VifCollection(wtypes.Base): + """API representation of a collection of VIFs. """ + + vifs = [types.viftype] + """A list containing VIFs objects""" + + @staticmethod + def collection_from_list(vifs): + col = VifCollection() + col.vifs = [types.VifType.frombasetype(vif) for vif in vifs] + return col + + +class NodeVIFController(rest.RestController): + + def __init__(self, node_ident): + self.node_ident = node_ident + + def _get_node_and_topic(self): + rpc_node = api_utils.get_rpc_node(self.node_ident) + try: + return rpc_node, pecan.request.rpcapi.get_topic_for(rpc_node) + except exception.NoValidHost as e: + e.code = http_client.BAD_REQUEST + raise + + @METRICS.timer('NodeVIFController.get_all') + @expose.expose(VifCollection) + def get_all(self): + """Get a list of attached VIFs""" + cdict = pecan.request.context.to_policy_values() + policy.authorize('baremetal:node:vif:list', cdict, cdict) + rpc_node, topic = self._get_node_and_topic() + vifs = pecan.request.rpcapi.vif_list(pecan.request.context, + rpc_node.uuid, topic=topic) + return VifCollection.collection_from_list(vifs) + + @METRICS.timer('NodeVIFController.post') + @expose.expose(None, body=types.viftype, + status_code=http_client.NO_CONTENT) + def post(self, vif): + """Attach a VIF to this node + + :param vif_info: a dictionary of information about a VIF. + It must have an 'id' key, whose value is a unique identifier + for that VIF. + """ + cdict = pecan.request.context.to_policy_values() + policy.authorize('baremetal:node:vif:attach', cdict, cdict) + rpc_node, topic = self._get_node_and_topic() + pecan.request.rpcapi.vif_attach(pecan.request.context, rpc_node.uuid, + vif_info=vif, topic=topic) + + @METRICS.timer('NodeVIFController.delete') + @expose.expose(None, types.uuid_or_name, + status_code=http_client.NO_CONTENT) + def delete(self, vif_id): + """Detach a VIF from this node + + :param vif_id: The ID of a VIF to detach + """ + cdict = pecan.request.context.to_policy_values() + policy.authorize('baremetal:node:vif:detach', cdict, cdict) + rpc_node, topic = self._get_node_and_topic() + pecan.request.rpcapi.vif_detach(pecan.request.context, rpc_node.uuid, + vif_id=vif_id, topic=topic) + + class NodesController(rest.RestController): """REST controller for Nodes.""" @@ -1109,6 +1179,7 @@ class NodesController(rest.RestController): _subcontroller_map = { 'ports': port.PortsController, 'portgroups': portgroup.PortgroupsController, + 'vifs': NodeVIFController, } @pecan.expose() @@ -1117,13 +1188,16 @@ class NodesController(rest.RestController): ident = types.uuid_or_name.validate(ident) except exception.InvalidUuidOrName as e: pecan.abort(http_client.BAD_REQUEST, e.args[0]) - if remainder: - subcontroller = self._subcontroller_map.get(remainder[0]) - if subcontroller: - if (remainder[0] == 'portgroups' and - not api_utils.allow_portgroups_subcontrollers()): - pecan.abort(http_client.NOT_FOUND) - return subcontroller(node_ident=ident), remainder[1:] + if not remainder: + return + if ((remainder[0] == 'portgroups' and + not api_utils.allow_portgroups_subcontrollers()) or + (remainder[0] == 'vifs' and + not api_utils.allow_vifs_subcontroller())): + pecan.abort(http_client.NOT_FOUND) + subcontroller = self._subcontroller_map.get(remainder[0]) + if subcontroller: + return subcontroller(node_ident=ident), remainder[1:] def _get_nodes_collection(self, chassis_uuid, instance_uuid, associated, maintenance, provision_state, marker, limit, diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index 512179e923..019c124be8 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -33,6 +33,7 @@ from ironic.api import expose from ironic.common import exception from ironic.common.i18n import _ from ironic.common import policy +from ironic.common import utils as common_utils from ironic import objects METRICS = metrics_utils.get_metrics_logger(__name__) @@ -512,6 +513,8 @@ class PortsController(rest.RestController): extra = pdict.get('extra') vif = extra.get('vif_port_id') if extra else None + if vif: + common_utils.warn_about_deprecated_extra_vif_port_id() if (pdict.get('portgroup_uuid') and (pdict.get('pxe_enabled') or vif)): rpc_pg = objects.Portgroup.get_by_uuid(context, diff --git a/ironic/api/controllers/v1/types.py b/ironic/api/controllers/v1/types.py index f1f24e1670..7dfe12629e 100644 --- a/ironic/api/controllers/v1/types.py +++ b/ironic/api/controllers/v1/types.py @@ -328,3 +328,33 @@ class LocalLinkConnectionType(wtypes.UserType): return LocalLinkConnectionType.validate(value) locallinkconnectiontype = LocalLinkConnectionType() + + +class VifType(JsonType): + + basetype = wtypes.text + name = 'viftype' + + mandatory_fields = {'id'} + + @staticmethod + def validate(value): + super(VifType, VifType).validate(value) + keys = set(value) + # Check all mandatory fields are present + missing = VifType.mandatory_fields - keys + if missing: + msg = _('Missing mandatory keys: %s') % ', '.join(list(missing)) + raise exception.Invalid(msg) + UuidOrNameType.validate(value['id']) + + return value + + @staticmethod + def frombasetype(value): + if value is None: + return None + return VifType.validate(value) + + +viftype = VifType() diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index b031efac61..3e28705a93 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -471,6 +471,16 @@ def allow_portgroup_mode_properties(): versions.MINOR_26_PORTGROUP_MODE_PROPERTIES) +def allow_vifs_subcontroller(): + """Check if node/vifs can be used. + + Version 1.28 of the API added support for VIFs to be + attached to Nodes. + """ + return (pecan.request.version.minor >= + versions.MINOR_28_VIFS_SUBCONTROLLER) + + 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 65997e3d8c..e889708721 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -58,6 +58,7 @@ BASE_VERSION = 1 # v1.25: Add possibility to unset chassis_uuid from node. # v1.26: Add portgroup.mode and portgroup.properties. # v1.27: Add soft reboot, soft power off and timeout. +# v1.28: Add vifs subcontroller to node MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -87,11 +88,12 @@ MINOR_24_PORTGROUPS_SUBCONTROLLERS = 24 MINOR_25_UNSET_CHASSIS_UUID = 25 MINOR_26_PORTGROUP_MODE_PROPERTIES = 26 MINOR_27_SOFT_POWER_OFF = 27 +MINOR_28_VIFS_SUBCONTROLLER = 28 # When adding another version, update MINOR_MAX_VERSION and also update # doc/source/dev/webapi-version-history.rst with a detailed explanation of # what the version has changed. -MINOR_MAX_VERSION = MINOR_27_SOFT_POWER_OFF +MINOR_MAX_VERSION = MINOR_28_VIFS_SUBCONTROLLER # String representations of the minor and maximum versions MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/policy.py b/ironic/common/policy.py index fb73be2724..c7a30174f5 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -118,6 +118,15 @@ node_policies = [ policy.RuleDefault('baremetal:node:set_console_state', 'rule:is_admin', description='Change Node console status'), + policy.RuleDefault('baremetal:node:vif:list', + 'rule:is_admin', + description='List VIFs attached to node'), + policy.RuleDefault('baremetal:node:vif:attach', + 'rule:is_admin', + description='Attach a VIF to a node'), + policy.RuleDefault('baremetal:node:vif:detach', + 'rule:is_admin', + description='Detach a VIF from a node'), ] port_policies = [ diff --git a/ironic/common/utils.py b/ironic/common/utils.py index cbd9b1ae0f..8b221cc56b 100644 --- a/ironic/common/utils.py +++ b/ironic/common/utils.py @@ -42,6 +42,8 @@ from ironic.conf import CONF LOG = logging.getLogger(__name__) +warn_deprecated_extra_vif_port_id = False + def _get_root_helper(): # NOTE(jlvillal): This function has been moved to ironic-lib. And is @@ -536,3 +538,13 @@ def render_template(template, params, is_file=True): env = jinja2.Environment(loader=loader) tmpl = env.get_template(tmpl_name) return tmpl.render(params) + + +def warn_about_deprecated_extra_vif_port_id(): + global warn_deprecated_extra_vif_port_id + if not warn_deprecated_extra_vif_port_id: + warn_deprecated_extra_vif_port_id = True + LOG.warning(_LW("Attaching VIF to a port via " + "extra['vif_port_id'] is deprecated and will not " + "be supported in Pike release. API endpoint " + "v1/nodes//vifs should be used instead.")) diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py index 23c07e353b..150f9b4921 100644 --- a/ironic/drivers/modules/network/common.py +++ b/ironic/drivers/modules/network/common.py @@ -21,6 +21,7 @@ from ironic.common import dhcp_factory from ironic.common import exception from ironic.common.i18n import _, _LW from ironic.common import neutron +from ironic.common import utils from ironic import objects CONF = cfg.CONF @@ -57,6 +58,10 @@ class VIFPortIDMixin(object): if 'extra' in port_obj.obj_what_changed(): original_port = objects.Port.get_by_id(context, port_obj.id) updated_client_id = port_obj.extra.get('client-id') + if (port_obj.extra.get('vif_port_id') and + (port_obj.extra['vif_port_id'] != + original_port.extra.get('vif_port_id'))): + utils.warn_about_deprecated_extra_vif_port_id() if (original_port.extra.get('client-id') != updated_client_id): # DHCP Option with opt_value=None will remove it diff --git a/ironic/tests/unit/api/v1/test_nodes.py b/ironic/tests/unit/api/v1/test_nodes.py index b7c543ea55..6fe96c09d8 100644 --- a/ironic/tests/unit/api/v1/test_nodes.py +++ b/ironic/tests/unit/api/v1/test_nodes.py @@ -3207,3 +3207,227 @@ class TestCheckCleanSteps(base.TestCase): step2 = {"step": "configure raid", "interface": "raid"} api_node._check_clean_steps([step1, step2]) + + +class TestAttachDetachVif(test_api_base.BaseApiTest): + + def setUp(self): + super(TestAttachDetachVif, self).setUp() + self.vif_version = "1.28" + self.node = obj_utils.create_test_node( + self.context, + provision_state=states.AVAILABLE, name='node-39') + p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for') + self.mock_gtf = p.start() + self.mock_gtf.return_value = 'test-topic' + self.addCleanup(p.stop) + + @mock.patch.object(objects.Node, 'get_by_uuid') + def test_vif_subcontroller_old_version(self, mock_get): + mock_get.return_value = self.node + ret = self.get_json('/nodes/%s/vifs' % self.node.uuid, + headers={api_base.Version.string: "1.26"}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, ret.status_code) + + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(rpcapi.ConductorAPI, 'vif_list') + def test_vif_list(self, mock_list, mock_get): + mock_get.return_value = self.node + self.get_json('/nodes/%s/vifs' % self.node.uuid, + headers={api_base.Version.string: + self.vif_version}) + + mock_get.assert_called_once_with(mock.ANY, self.node.uuid) + mock_list.assert_called_once_with(mock.ANY, self.node.uuid, + topic='test-topic') + + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(rpcapi.ConductorAPI, 'vif_attach') + def test_vif_attach(self, mock_attach, mock_get): + vif_id = uuidutils.generate_uuid() + request_body = { + 'id': vif_id + } + + mock_get.return_value = self.node + + ret = self.post_json('/nodes/%s/vifs' % self.node.uuid, + request_body, + headers={api_base.Version.string: + self.vif_version}) + + self.assertEqual(http_client.NO_CONTENT, ret.status_code) + mock_get.assert_called_once_with(mock.ANY, self.node.uuid) + mock_attach.assert_called_once_with(mock.ANY, self.node.uuid, + vif_info=request_body, + topic='test-topic') + + @mock.patch.object(objects.Node, 'get_by_name') + @mock.patch.object(rpcapi.ConductorAPI, 'vif_attach') + def test_vif_attach_by_node_name(self, mock_attach, mock_get): + vif_id = uuidutils.generate_uuid() + request_body = { + 'id': vif_id + } + + mock_get.return_value = self.node + + ret = self.post_json('/nodes/%s/vifs' % self.node.name, + request_body, + headers={api_base.Version.string: + self.vif_version}) + + self.assertEqual(http_client.NO_CONTENT, ret.status_code) + mock_get.assert_called_once_with(mock.ANY, self.node.name) + mock_attach.assert_called_once_with(mock.ANY, self.node.uuid, + vif_info=request_body, + topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'vif_attach') + def test_vif_attach_node_not_found(self, mock_attach): + vif_id = uuidutils.generate_uuid() + request_body = { + 'id': vif_id + } + + ret = self.post_json('/nodes/doesntexist/vifs', + request_body, expect_errors=True, + headers={api_base.Version.string: + self.vif_version}) + + self.assertEqual(http_client.NOT_FOUND, ret.status_code) + self.assertTrue(ret.json['error_message']) + self.assertFalse(mock_attach.called) + + @mock.patch.object(objects.Node, 'get_by_name') + @mock.patch.object(rpcapi.ConductorAPI, 'vif_attach') + def test_vif_attach_conductor_unavailable(self, mock_attach, mock_get): + vif_id = uuidutils.generate_uuid() + request_body = { + 'id': vif_id + } + mock_get.return_value = self.node + self.mock_gtf.side_effect = exception.NoValidHost('boom') + ret = self.post_json('/nodes/%s/vifs' % self.node.name, + request_body, expect_errors=True, + headers={api_base.Version.string: + self.vif_version}) + + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertTrue(ret.json['error_message']) + self.assertFalse(mock_attach.called) + + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(rpcapi.ConductorAPI, 'vif_attach') + def test_vif_attach_no_vif_id(self, mock_attach, mock_get): + vif_id = uuidutils.generate_uuid() + request_body = { + 'bad_id': vif_id + } + + mock_get.return_value = self.node + + ret = self.post_json('/nodes/%s/vifs' % self.node.uuid, + request_body, expect_errors=True, + headers={api_base.Version.string: + self.vif_version}) + + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertTrue(ret.json['error_message']) + + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(rpcapi.ConductorAPI, 'vif_attach') + def test_vif_attach_invalid_vif_id(self, mock_attach, mock_get): + request_body = { + 'id': "invalid%id^" + } + + mock_get.return_value = self.node + + ret = self.post_json('/nodes/%s/vifs' % self.node.uuid, + request_body, expect_errors=True, + headers={api_base.Version.string: + self.vif_version}) + + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertTrue(ret.json['error_message']) + + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(rpcapi.ConductorAPI, 'vif_attach') + def test_vif_attach_node_locked(self, mock_attach, mock_get): + vif_id = uuidutils.generate_uuid() + request_body = { + 'id': vif_id + } + + mock_get.return_value = self.node + mock_attach.side_effect = exception.NodeLocked(node='', host='') + + ret = self.post_json('/nodes/%s/vifs' % self.node.uuid, + request_body, expect_errors=True, + headers={api_base.Version.string: + self.vif_version}) + + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertTrue(ret.json['error_message']) + + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(rpcapi.ConductorAPI, 'vif_detach') + def test_vif_detach(self, mock_detach, mock_get): + vif_id = uuidutils.generate_uuid() + + mock_get.return_value = self.node + + ret = self.delete('/nodes/%s/vifs/%s' % (self.node.uuid, vif_id), + headers={api_base.Version.string: + self.vif_version}) + + self.assertEqual(http_client.NO_CONTENT, ret.status_code) + mock_get.assert_called_once_with(mock.ANY, self.node.uuid) + mock_detach.assert_called_once_with(mock.ANY, self.node.uuid, + vif_id=vif_id, + topic='test-topic') + + @mock.patch.object(objects.Node, 'get_by_name') + @mock.patch.object(rpcapi.ConductorAPI, 'vif_detach') + def test_vif_detach_by_node_name(self, mock_detach, mock_get): + vif_id = uuidutils.generate_uuid() + + mock_get.return_value = self.node + + ret = self.delete('/nodes/%s/vifs/%s' % (self.node.name, vif_id), + headers={api_base.Version.string: self.vif_version}) + + self.assertEqual(http_client.NO_CONTENT, ret.status_code) + mock_get.assert_called_once_with(mock.ANY, self.node.name) + mock_detach.assert_called_once_with(mock.ANY, self.node.uuid, + vif_id=vif_id, + topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'vif_detach') + def test_vif_detach_node_not_found(self, mock_detach): + vif_id = uuidutils.generate_uuid() + + ret = self.delete('/nodes/doesntexist/vifs/%s' % vif_id, + headers={api_base.Version.string: self.vif_version}, + expect_errors=True) + + self.assertEqual(http_client.NOT_FOUND, ret.status_code) + self.assertTrue(ret.json['error_message']) + self.assertFalse(mock_detach.called) + + @mock.patch.object(objects.Node, 'get_by_uuid') + @mock.patch.object(rpcapi.ConductorAPI, 'vif_detach') + def test_vif_detach_node_locked(self, mock_detach, mock_get): + vif_id = uuidutils.generate_uuid() + + mock_get.return_value = self.node + mock_detach.side_effect = exception.NodeLocked(node='', host='') + + ret = self.delete('/nodes/%s/vifs/%s' % (self.node.uuid, vif_id), + headers={api_base.Version.string: self.vif_version}, + expect_errors=True) + + self.assertEqual(http_client.CONFLICT, ret.status_code) + self.assertTrue(ret.json['error_message']) diff --git a/ironic/tests/unit/api/v1/test_ports.py b/ironic/tests/unit/api/v1/test_ports.py index 39d2a0a935..b6f1bc36e0 100644 --- a/ironic/tests/unit/api/v1/test_ports.py +++ b/ironic/tests/unit/api/v1/test_ports.py @@ -34,6 +34,7 @@ from ironic.api.controllers.v1 import port as api_port from ironic.api.controllers.v1 import utils as api_utils from ironic.api.controllers.v1 import versions from ironic.common import exception +from ironic.common import utils as common_utils from ironic.conductor import rpcapi from ironic import objects from ironic.objects import fields as obj_fields @@ -956,9 +957,11 @@ class TestPost(test_api_base.BaseApiTest): self.headers = {api_base.Version.string: str( versions.MAX_VERSION_STRING)} + @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', + autospec=True) @mock.patch.object(notification_utils, '_emit_api_notification') @mock.patch.object(timeutils, 'utcnow') - def test_create_port(self, mock_utcnow, mock_notify): + def test_create_port(self, mock_utcnow, mock_notify, mock_warn): pdict = post_get_test_port() test_time = datetime.datetime(2000, 1, 1, 0, 0) mock_utcnow.return_value = test_time @@ -984,6 +987,7 @@ class TestPost(test_api_base.BaseApiTest): obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.END, node_uuid=self.node.uuid)]) + self.assertEqual(0, mock_warn.call_count) def test_create_port_min_api_version(self): pdict = post_get_test_port( @@ -1256,6 +1260,16 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.FORBIDDEN, response.status_int) + @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', + autospec=True) + def test_create_port_with_extra_vif_port_id_deprecated(self, mock_warn): + pdict = post_get_test_port(pxe_enabled=False, + extra={'vif_port_id': 'foo'}) + response = self.post_json('/ports', pdict, headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.CREATED, response.status_int) + self.assertEqual(1, mock_warn.call_count) + def _test_create_port(self, has_vif=False, in_portgroup=False, pxe_enabled=True, standalone_ports=True, http_status=http_client.CREATED): diff --git a/ironic/tests/unit/api/v1/test_types.py b/ironic/tests/unit/api/v1/test_types.py index 2da257527c..eadb67ecb6 100644 --- a/ironic/tests/unit/api/v1/test_types.py +++ b/ironic/tests/unit/api/v1/test_types.py @@ -338,3 +338,28 @@ class TestLocalLinkConnectionType(base.TestCase): v = types.locallinkconnectiontype value = {} self.assertItemsEqual(value, v.validate(value)) + + +@mock.patch("pecan.request", mock.Mock(version=mock.Mock(minor=10))) +class TestVifType(base.TestCase): + + def test_vif_type(self): + v = types.viftype + value = {'id': 'foo'} + self.assertItemsEqual(value, v.validate(value)) + + def test_vif_type_missing_mandatory_key(self): + v = types.viftype + value = {'foo': 'bar'} + self.assertRaisesRegex(exception.Invalid, 'Missing mandatory', + v.validate, value) + + def test_vif_type_optional_key(self): + v = types.viftype + value = {'id': 'foo', 'misc': 'something'} + self.assertItemsEqual(value, v.frombasetype(value)) + + def test_vif_type_bad_id(self): + v = types.viftype + self.assertRaises(exception.InvalidUuidOrName, + v.frombasetype, {'id': 5678}) diff --git a/ironic/tests/unit/common/test_utils.py b/ironic/tests/unit/common/test_utils.py index 2c6068af37..58fd9bd933 100644 --- a/ironic/tests/unit/common/test_utils.py +++ b/ironic/tests/unit/common/test_utils.py @@ -427,6 +427,16 @@ class GenericUtilsTestCase(base.TestCase): utils.is_valid_no_proxy(no_proxy), msg="'no_proxy' value should be invalid: {}".format(no_proxy)) + @mock.patch.object(utils, 'LOG', autospec=True) + def test_warn_about_deprecated_extra_vif_port_id(self, mock_log): + # Set variable to default value + utils.warn_deprecated_extra_vif_port_id = False + utils.warn_about_deprecated_extra_vif_port_id() + utils.warn_about_deprecated_extra_vif_port_id() + self.assertEqual(1, mock_log.warning.call_count) + self.assertIn("extra['vif_port_id'] is deprecated and will not", + mock_log.warning.call_args[0][0]) + class TempFilesTestCase(base.TestCase): diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py index 4f320f0559..b4bc6cfb98 100644 --- a/ironic/tests/unit/drivers/modules/network/test_common.py +++ b/ironic/tests/unit/drivers/modules/network/test_common.py @@ -16,6 +16,7 @@ from oslo_utils import uuidutils from ironic.common import exception from ironic.common import neutron as neutron_common +from ironic.common import utils as common_utils from ironic.conductor import task_manager from ironic.drivers.modules.network import common from ironic.tests.unit.conductor import mgr_utils @@ -199,8 +200,10 @@ class TestVifPortIDMixin(db_base.DbTestCase): vif = self.interface.get_current_vif(task, self.port) self.assertIsNone(vif) + @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', + autospec=True) @mock.patch.object(neutron_common, 'update_port_address', autospec=True) - def test_port_changed_address(self, mac_update_mock): + def test_port_changed_address(self, mac_update_mock, mock_warn): new_address = '11:22:33:44:55:bb' self.port.address = new_address with task_manager.acquire(self.context, self.node.id) as task: @@ -208,6 +211,7 @@ class TestVifPortIDMixin(db_base.DbTestCase): mac_update_mock.assert_called_once_with( self.port.extra['vif_port_id'], new_address, token=task.context.auth_token) + self.assertFalse(mock_warn.called) @mock.patch.object(neutron_common, 'update_port_address', autospec=True) def test_port_changed_address_VIF_MAC_update_fail(self, mac_update_mock): @@ -242,13 +246,43 @@ class TestVifPortIDMixin(db_base.DbTestCase): dhcp_update_mock.assert_called_once_with( 'fake-id', expected_dhcp_opts, token=self.context.auth_token) + @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', + autospec=True) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') - def test_port_changed_vif(self, dhcp_update_mock): + def test_port_changed_vif(self, dhcp_update_mock, mock_warn): expected_extra = {'vif_port_id': 'new_ake-id', 'client-id': 'fake1'} self.port.extra = expected_extra with task_manager.acquire(self.context, self.node.id) as task: self.interface.port_changed(task, self.port) self.assertFalse(dhcp_update_mock.called) + self.assertEqual(1, mock_warn.call_count) + + @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', + autospec=True) + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') + def test_port_changed_extra_add_new_key(self, dhcp_update_mock, mock_warn): + self.port.extra = {'vif_port_id': 'fake-id'} + self.port.save() + expected_extra = self.port.extra + expected_extra['foo'] = 'bar' + self.port.extra = expected_extra + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.port_changed(task, self.port) + self.assertFalse(dhcp_update_mock.called) + self.assertEqual(0, mock_warn.call_count) + + @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', + autospec=True) + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') + def test_port_changed_extra_no_deprecation_if_removing_vif( + self, dhcp_update_mock, mock_warn): + self.port.extra = {'vif_port_id': 'foo'} + self.port.save() + self.port.extra = {'foo': 'bar'} + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.port_changed(task, self.port) + self.assertFalse(dhcp_update_mock.called) + self.assertEqual(0, mock_warn.call_count) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') def test_port_changed_client_id_fail(self, dhcp_update_mock): diff --git a/ironic_tempest_plugin/services/baremetal/base.py b/ironic_tempest_plugin/services/baremetal/base.py index 67e4d08cd0..30589e13c2 100644 --- a/ironic_tempest_plugin/services/baremetal/base.py +++ b/ironic_tempest_plugin/services/baremetal/base.py @@ -115,10 +115,13 @@ class BaremetalClient(rest_client.RestClient): return patch - def _list_request(self, resource, permanent=False, **kwargs): + def _list_request(self, resource, permanent=False, headers=None, + extra_headers=False, **kwargs): """Get the list of objects of the specified type. :param resource: The name of the REST resource, e.g., 'nodes'. + :param headers: List of headers to use in request. + :param extra_headers: Specify whether to use headers. :param **kwargs: Parameters for the request. :returns: A tuple with the server response and deserialized JSON list of objects @@ -128,7 +131,8 @@ class BaremetalClient(rest_client.RestClient): if kwargs: uri += "?%s" % urllib.urlencode(kwargs) - resp, body = self.get(uri) + resp, body = self.get(uri, headers=headers, + extra_headers=extra_headers) self.expected_success(200, resp.status) return resp, self.deserialize(body) @@ -167,6 +171,25 @@ class BaremetalClient(rest_client.RestClient): return resp, self.deserialize(body) + def _create_request_no_response_body(self, resource, object_dict): + """Create an object of the specified type. + + Do not expect any body in the response. + + :param resource: The name of the REST resource, e.g., 'nodes'. + :param object_dict: A Python dict that represents an object of the + specified type. + :returns: The server response. + """ + + body = self.serialize(object_dict) + uri = self._get_uri(resource) + + resp, body = self.post(uri, body=body) + self.expected_success(204, resp.status) + + return resp + def _delete_request(self, resource, uuid): """Delete specified object. diff --git a/ironic_tempest_plugin/services/baremetal/v1/json/baremetal_client.py b/ironic_tempest_plugin/services/baremetal/v1/json/baremetal_client.py index 49049d2734..cf1abad9c7 100644 --- a/ironic_tempest_plugin/services/baremetal/v1/json/baremetal_client.py +++ b/ironic_tempest_plugin/services/baremetal/v1/json/baremetal_client.py @@ -372,3 +372,43 @@ class BaremetalClient(base.BaremetalClient): enabled) self.expected_success(202, resp.status) return resp, body + + @base.handle_errors + def vif_list(self, node_uuid, api_version=None): + """Get list of attached VIFs. + + :param node_uuid: Unique identifier of the node in UUID format. + :param api_version: Ironic API version to use. + """ + extra_headers = False + headers = None + if api_version is not None: + extra_headers = True + headers = {'x-openstack-ironic-api-version': api_version} + return self._list_request('nodes/%s/vifs' % node_uuid, + headers=headers, + extra_headers=extra_headers) + + @base.handle_errors + def vif_attach(self, node_uuid, vif_id): + """Attach a VIF to a node + + :param node_uuid: Unique identifier of the node in UUID format. + :param vif_id: An ID representing the VIF + """ + vif = {'id': vif_id} + resp = self._create_request_no_response_body( + 'nodes/%s/vifs' % node_uuid, vif) + + return resp + + @base.handle_errors + def vif_detach(self, node_uuid, vif_id): + """Detach a VIF from a node + + :param node_uuid: Unique identifier of the node in UUID format. + :param vif_id: An ID representing the VIF + """ + resp, body = self._delete_request('nodes/%s/vifs' % node_uuid, vif_id) + self.expected_success(204, resp.status) + return resp, body diff --git a/ironic_tempest_plugin/tests/api/admin/test_nodes.py b/ironic_tempest_plugin/tests/api/admin/test_nodes.py index fe95c02574..e7cfa8b74a 100644 --- a/ironic_tempest_plugin/tests/api/admin/test_nodes.py +++ b/ironic_tempest_plugin/tests/api/admin/test_nodes.py @@ -16,6 +16,7 @@ from tempest.lib import exceptions as lib_exc from tempest import test from ironic_tempest_plugin.common import waiters +from ironic_tempest_plugin.tests.api.admin import api_microversion_fixture from ironic_tempest_plugin.tests.api.admin import base @@ -166,3 +167,33 @@ class TestNodes(base.BaseBaremetalTest): _, body = self.client.show_node_by_instance_uuid(instance_uuid) self.assertEqual(1, len(body['nodes'])) self.assertIn(self.node['uuid'], [n['uuid'] for n in body['nodes']]) + + @test.idempotent_id('a3d319d0-cacb-4e55-a3dc-3fa8b74880f1') + def test_vifs(self): + self.useFixture( + api_microversion_fixture.APIMicroversionFixture('1.28')) + _, self.port = self.create_port(self.node['uuid'], + data_utils.rand_mac_address()) + self.client.vif_attach(self.node['uuid'], 'test-vif') + _, body = self.client.vif_list(self.node['uuid']) + self.assertEqual(body, {'vifs': [{'id': 'test-vif'}]}) + self.client.vif_detach(self.node['uuid'], 'test-vif') + + @test.idempotent_id('a3d319d0-cacb-4e55-a3dc-3fa8b74880f2') + def test_vif_already_set_on_extra(self): + self.useFixture( + api_microversion_fixture.APIMicroversionFixture('1.28')) + _, self.port = self.create_port(self.node['uuid'], + data_utils.rand_mac_address()) + patch = [{'path': '/extra/vif_port_id', + 'op': 'add', + 'value': 'test-vif'}] + self.client.update_port(self.port['uuid'], patch) + + _, body = self.client.vif_list(self.node['uuid']) + self.assertEqual(body, {'vifs': [{'id': 'test-vif'}]}) + + self.assertRaises(lib_exc.Conflict, self.client.vif_attach, + self.node['uuid'], 'test-vif') + + self.client.vif_detach(self.node['uuid'], 'test-vif') diff --git a/ironic_tempest_plugin/tests/scenario/baremetal_manager.py b/ironic_tempest_plugin/tests/scenario/baremetal_manager.py index 286750ded1..5c805a957b 100644 --- a/ironic_tempest_plugin/tests/scenario/baremetal_manager.py +++ b/ironic_tempest_plugin/tests/scenario/baremetal_manager.py @@ -134,6 +134,11 @@ class BaremetalScenarioTest(manager.ScenarioTest): ports.append(p) return ports + def get_node_vifs(self, node_uuid, api_version='1.28'): + _, body = self.baremetal_client.vif_list(node_uuid, + api_version=api_version) + return body['vifs'] + def add_keypair(self): self.keypair = self.create_keypair() diff --git a/ironic_tempest_plugin/tests/scenario/test_baremetal_basic_ops.py b/ironic_tempest_plugin/tests/scenario/test_baremetal_basic_ops.py index 6d07399d96..1a918790b7 100644 --- a/ironic_tempest_plugin/tests/scenario/test_baremetal_basic_ops.py +++ b/ironic_tempest_plugin/tests/scenario/test_baremetal_basic_ops.py @@ -97,12 +97,16 @@ class BaremetalBasicOps(baremetal_manager.BaremetalScenarioTest): return int(ephemeral) def validate_ports(self): - for port in self.get_ports(self.node['uuid']): - n_port_id = port['extra']['vif_port_id'] + node_uuid = self.node['uuid'] + vifs = self.get_node_vifs(node_uuid) + ir_ports = self.get_ports(node_uuid) + ir_ports_addresses = [x['address'] for x in ir_ports] + for vif in vifs: + n_port_id = vif['id'] body = self.ports_client.show_port(n_port_id) n_port = body['port'] self.assertEqual(n_port['device_id'], self.instance['id']) - self.assertEqual(n_port['mac_address'], port['address']) + self.assertIn(n_port['mac_address'], ir_ports_addresses) @test.idempotent_id('549173a5-38ec-42bb-b0e2-c8b9f4a08943') @test.services('compute', 'image', 'network') diff --git a/releasenotes/notes/add-vif-attach-detach-support-99eca43eea6e5a30.yaml b/releasenotes/notes/add-vif-attach-detach-support-99eca43eea6e5a30.yaml new file mode 100644 index 0000000000..ac3f2efe95 --- /dev/null +++ b/releasenotes/notes/add-vif-attach-detach-support-99eca43eea6e5a30.yaml @@ -0,0 +1,8 @@ +--- +features: + - Adds support of attaching/detaching network VIFs + to ironic ports by using ``v1/nodes//vifs`` + API endpoint that was added in API version 1.28. +deprecations: + - Using port.extra['vif_port_id'] for attaching/detaching + VIFs to ports is deprecated and will be removed in Pike release.