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.