From f4ff15938cc2ee1103602d865a533efdd6e77b13 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Wed, 16 Dec 2020 15:06:55 -0800 Subject: [PATCH] Remove extra/vif_port_id Removes API translation layer into port/portgroup internal_info Removes internal logic to support use of VIFs stored in the extra field, which effectively means all vif binding must now utilize vif attachment and detachment. This is a change that we likely should have pushed forward a couple years back, but I didn't want to break compatability with very old of ironic or standalone users which were deploying instances using very old instructions. That being said, it is necessary to remove the legacy vif support so new access controls can properly wrap vif API endpoints. Depends-On: https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/769204 Change-Id: I588b3a64475656542389ff83786189e2dc16d35c --- ironic/api/controllers/v1/port.py | 17 +- ironic/api/controllers/v1/portgroup.py | 12 +- ironic/api/controllers/v1/utils.py | 83 ------- ironic/common/utils.py | 13 - ironic/drivers/modules/network/common.py | 12 +- ironic/drivers/modules/network/flat.py | 3 +- ironic/drivers/modules/network/neutron.py | 2 +- .../unit/api/controllers/v1/test_port.py | 223 +----------------- .../unit/api/controllers/v1/test_portgroup.py | 172 +------------- ironic/tests/unit/common/test_network.py | 58 +---- ironic/tests/unit/common/test_utils.py | 10 - ironic/tests/unit/dhcp/test_neutron.py | 26 +- .../drivers/modules/network/test_common.py | 150 +++++------- .../unit/drivers/modules/network/test_flat.py | 23 +- .../drivers/modules/network/test_neutron.py | 84 ++++--- .../unit/drivers/modules/test_deploy_utils.py | 10 - ...ve-extra-vif-port-id-ea4e59176c2065fd.yaml | 8 + 17 files changed, 158 insertions(+), 748 deletions(-) create mode 100644 releasenotes/notes/remove-extra-vif-port-id-ea4e59176c2065fd.yaml diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index 4c58e0110a..eacbdf5dbe 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -import copy from http import client as http_client from ironic_lib import metrics_utils @@ -58,10 +57,7 @@ PORT_SCHEMA = { } -PORT_PATCH_SCHEMA = copy.deepcopy(PORT_SCHEMA) -# patching /extra/vif_port_id has the side-effect of modifying -# internal_info values, so include it in the patch schema -PORT_PATCH_SCHEMA['properties']['internal_info'] = {'type': ['null', 'object']} +PORT_PATCH_SCHEMA = PORT_SCHEMA PATCH_ALLOWED_FIELDS = [ 'address', @@ -576,14 +572,12 @@ class PortsController(rest.RestController): raise exception.Invalid('A non-empty value is required when ' 'setting physical_network') - vif = api_utils.handle_post_port_like_extra_vif(port) - - if (portgroup and (port.get('pxe_enabled') or vif)): + if (portgroup and (port.get('pxe_enabled'))): if not portgroup.standalone_ports_supported: msg = _("Port group %s doesn't support standalone ports. " "This port cannot be created as a member of that " - "port group because either 'extra/vif_port_id' " - "was specified or 'pxe_enabled' was set to True.") + "portgroup as the port's 'pxe_enabled' field was " + "set to True.") raise exception.Conflict( msg % portgroup.uuid) @@ -659,9 +653,6 @@ class PortsController(rest.RestController): port_dict = api_utils.apply_jsonpatch(port_dict, patch) - api_utils.handle_patch_port_like_extra_vif( - rpc_port, port_dict['internal_info'], patch) - try: if api_utils.is_path_updated(patch, '/portgroup_uuid'): if port_dict.get('portgroup_uuid'): diff --git a/ironic/api/controllers/v1/portgroup.py b/ironic/api/controllers/v1/portgroup.py index 6c9cc9303f..9a2c2dc053 100644 --- a/ironic/api/controllers/v1/portgroup.py +++ b/ironic/api/controllers/v1/portgroup.py @@ -10,7 +10,6 @@ # License for the specific language governing permissions and limitations # under the License. -import copy from http import client as http_client from ironic_lib import metrics_utils @@ -50,11 +49,7 @@ PORTGROUP_SCHEMA = { 'additionalProperties': False, } -PORTGROUP_PATCH_SCHEMA = copy.deepcopy(PORTGROUP_SCHEMA) -# patching /extra/vif_port_id has the side-effect of modifying -# internal_info values, so include it in the patch schema -PORTGROUP_PATCH_SCHEMA['properties']['internal_info'] = { - 'type': ['null', 'object']} +PORTGROUP_PATCH_SCHEMA = PORTGROUP_SCHEMA PORTGROUP_VALIDATOR_EXTRA = args.dict_valid( address=args.mac_address, @@ -417,8 +412,6 @@ class PortgroupsController(pecan.rest.RestController): raise exception.ClientSideError( error_msg, status_code=http_client.BAD_REQUEST) - api_utils.handle_post_port_like_extra_vif(portgroup) - # NOTE(yuriyz): UUID is mandatory for notifications payload if not portgroup.get('uuid'): portgroup['uuid'] = uuidutils.generate_uuid() @@ -501,9 +494,6 @@ class PortgroupsController(pecan.rest.RestController): e.code = http_client.BAD_REQUEST # BadRequest raise - api_utils.handle_patch_port_like_extra_vif( - rpc_portgroup, portgroup_dict.get('internal_info'), patch) - api_utils.patched_validate_with_schema( portgroup_dict, PORTGROUP_PATCH_SCHEMA, PORTGROUP_PATCH_VALIDATOR) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 7d25670c2c..0e4b77ec2e 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -1310,89 +1310,6 @@ def allow_inspect_abort(): return api.request.version.minor >= versions.MINOR_41_INSPECTION_ABORT -def handle_post_port_like_extra_vif(p_dict): - """Handle a Post request that sets .extra['vif_port_id']. - - This handles attach of VIFs via specifying the VIF port ID - in a port or port group's extra['vif_port_id'] field. - - :param p_dict: a dictionary with field names/values for the port or - port group - :return: VIF or None - """ - vif = p_dict.get('extra', {}).get('vif_port_id') - if vif: - # TODO(rloo): in Stein cycle: if API version >= 1.28, remove - # warning and support for extra[]; else (< 1.28) - # still support it; continue copying to internal_info - # (see bug 1722850). i.e., change the 7 lines of code - # below to something like: - # if not api_utils.allow_vifs_subcontroller(): - # internal_info = {'tenant_vif_port_id': vif} - # pg_dict['internal_info'] = internal_info - if allow_vifs_subcontroller(): - utils.warn_about_deprecated_extra_vif_port_id() - # NOTE(rloo): this value should really be in .internal_info[..] - # which is what would happen if they had used the - # POST /v1/nodes//vifs API. - internal_info = {'tenant_vif_port_id': vif} - p_dict['internal_info'] = internal_info - return vif - - -def handle_patch_port_like_extra_vif(rpc_object, internal_info, patch): - """Handle a Patch request that modifies .extra['vif_port_id']. - - This handles attach/detach of VIFs via the VIF port ID - in a port or port group's extra['vif_port_id'] field. - - :param rpc_object: a Port or Portgroup RPC object - :param internal_info: Dict of port or portgroup internal info - :param patch: the JSON patch in the API request - """ - vif_list = get_patch_values(patch, '/extra/vif_port_id') - vif = None - if vif_list: - # if specified more than once, use the last value - vif = vif_list[-1] - - # TODO(rloo): in Stein cycle: if API version >= 1.28, remove this - # warning and don't copy to internal_info; else (<1.28) still - # support it; continue copying to internal_info (see bug 1722850). - # i.e., change the 8 lines of code below to something like: - # if not allow_vifs_subcontroller(): - # int_info = rpc_object.internal_info.get('tenant_vif_port_id') - # if (not int_info or - # int_info == rpc_object.extra.get('vif_port_id')): - # internal_info['tenant_vif_port_id'] = vif - if allow_vifs_subcontroller(): - utils.warn_about_deprecated_extra_vif_port_id() - # NOTE(rloo): if the user isn't also using the REST API - # 'POST nodes//vifs', we are safe to copy the - # .extra[] value to the .internal_info location - int_info = rpc_object.internal_info.get('tenant_vif_port_id') - if (not int_info or int_info == rpc_object.extra.get('vif_port_id')): - internal_info['tenant_vif_port_id'] = vif - - elif is_path_removed(patch, '/extra/vif_port_id'): - # TODO(rloo): in Stein cycle: if API version >= 1.28, remove this - # warning and don't remove from internal_info; else (<1.28) still - # support it; remove from internal_info (see bug 1722850). - # i.e., change the 8 lines of code below to something like: - # if not allow_vifs_subcontroller(): - # int_info = rpc_object.internal_info.get('tenant_vif...') - # if (int_info and int_info==rpc_object.extra.get('vif_port_id')): - # internal_info['tenant_vif_port_id'] = None - if allow_vifs_subcontroller(): - utils.warn_about_deprecated_extra_vif_port_id() - # NOTE(rloo): if the user isn't also using the REST API - # 'POST nodes//vifs', we are safe to remove the - # .extra[] value from the .internal_info location - int_info = rpc_object.internal_info.get('tenant_vif_port_id') - if (int_info and int_info == rpc_object.extra.get('vif_port_id')): - internal_info.pop('tenant_vif_port_id') - - def allow_detail_query(): """Check if passing a detail=True query string is allowed. diff --git a/ironic/common/utils.py b/ironic/common/utils.py index cd323af364..ce482f2cc6 100644 --- a/ironic/common/utils.py +++ b/ironic/common/utils.py @@ -54,8 +54,6 @@ TZ_RE = r'((?P[+-])(?P\d{2}):(?P\d{2}))' + \ DATETIME_RE = re.compile( '%sT%s(%s)?' % (DATE_RE, TIME_RE, TZ_RE)) -warn_deprecated_extra_vif_port_id = False - def _get_root_helper(): # NOTE(jlvillal): This function has been moved to ironic-lib. And is @@ -494,17 +492,6 @@ def render_template(template, params, is_file=True): return tmpl.render(params, enumerate=enumerate) -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("Starting with API version 1.28, attaching/detaching VIF " - "to/from a port or port group via extra['vif_port_id'] is " - "deprecated and will not be supported starting in the " - "Stein release. API endpoint v1/nodes//vifs should " - "be used instead.") - - def parse_instance_info_capabilities(node): """Parse the instance_info capabilities. diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py index f10089d265..be15a2421b 100644 --- a/ironic/drivers/modules/network/common.py +++ b/ironic/drivers/modules/network/common.py @@ -46,8 +46,7 @@ def _vif_attached(port_like_obj, vif_id): False otherwise. :raises: VifAlreadyAttached, if vif_id is attached to port_like_obj. """ - attached_vif_id = port_like_obj.internal_info.get( - TENANT_VIF_KEY, port_like_obj.extra.get('vif_port_id')) + attached_vif_id = port_like_obj.internal_info.get(TENANT_VIF_KEY) if attached_vif_id == vif_id: raise exception.VifAlreadyAttached( object_type=port_like_obj.__class__.__name__, @@ -233,9 +232,7 @@ def plug_port_to_tenant_network(task, port_like_obj, client=None): local_group_info = None client_id_opt = None - vif_id = ( - port_like_obj.internal_info.get(TENANT_VIF_KEY) - or port_like_obj.extra.get('vif_port_id')) + vif_id = port_like_obj.internal_info.get(TENANT_VIF_KEY) if not vif_id: obj_name = port_like_obj.__class__.__name__.lower() @@ -365,10 +362,7 @@ class VIFPortIDMixin(object): :param port_like_obj: A port or portgroup to check. :returns: The ID of the attached VIF, or None. """ - # FIXME(sambetts) Remove this when we no longer support a nova - # driver that uses port.extra - return (port_like_obj.internal_info.get(TENANT_VIF_KEY) - or port_like_obj.extra.get('vif_port_id')) + return port_like_obj.internal_info.get(TENANT_VIF_KEY) def vif_list(self, task): """List attached VIF IDs for a node diff --git a/ironic/drivers/modules/network/flat.py b/ironic/drivers/modules/network/flat.py index 7e54bd9826..a069878176 100644 --- a/ironic/drivers/modules/network/flat.py +++ b/ironic/drivers/modules/network/flat.py @@ -59,7 +59,6 @@ class FlatNetwork(common.NeutronVIFPortIDMixin, for port_like_obj in task.ports + task.portgroups: vif_port_id = ( port_like_obj.internal_info.get(common.TENANT_VIF_KEY) - or port_like_obj.extra.get('vif_port_id') ) if not vif_port_id: continue @@ -88,7 +87,7 @@ class FlatNetwork(common.NeutronVIFPortIDMixin, for port_like_obj in ports + portgroups: vif_port_id = ( port_like_obj.internal_info.get(common.TENANT_VIF_KEY) - or port_like_obj.extra.get('vif_port_id')) + ) if not vif_port_id: continue neutron.unbind_neutron_port(vif_port_id, context=task.context) diff --git a/ironic/drivers/modules/network/neutron.py b/ironic/drivers/modules/network/neutron.py index 68520cbf16..3e4dcbfd65 100644 --- a/ironic/drivers/modules/network/neutron.py +++ b/ironic/drivers/modules/network/neutron.py @@ -225,7 +225,7 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, for port_like_obj in ports + portgroups: vif_port_id = ( port_like_obj.internal_info.get(common.TENANT_VIF_KEY) - or port_like_obj.extra.get('vif_port_id')) + ) if not vif_port_id: continue diff --git a/ironic/tests/unit/api/controllers/v1/test_port.py b/ironic/tests/unit/api/controllers/v1/test_port.py index f53782237d..1f3322accf 100644 --- a/ironic/tests/unit/api/controllers/v1/test_port.py +++ b/ironic/tests/unit/api/controllers/v1/test_port.py @@ -34,7 +34,6 @@ from ironic.api.controllers.v1 import versions from ironic.common import exception from ironic.common import policy from ironic.common import states -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 @@ -1804,155 +1803,6 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(http_client.FORBIDDEN, response.status_int) self.assertEqual('application/json', response.content_type) - def _test_add_extra_vif_port_id(self, port, headers, mock_warn, mock_upd): - response = self.patch_json( - '/ports/%s' % port.uuid, - [{'path': '/extra/vif_port_id', 'value': 'foo', 'op': 'add'}, - {'path': '/extra/vif_port_id', 'value': 'bar', 'op': 'add'}], - headers=headers) - self.assertEqual('application/json', response.content_type) - self.assertEqual(http_client.OK, response.status_code) - self.assertEqual({'vif_port_id': 'bar'}, - response.json['extra']) - self.assertTrue(mock_upd.called) - return response - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_add_extra_vif_port_id(self, mock_warn, mock_upd): - port = obj_utils.create_test_port(self.context, node_id=self.node.id, - uuid=uuidutils.generate_uuid(), - address='52:55:00:cf:2d:31') - expected_intern_info = port.internal_info - expected_intern_info.update({'tenant_vif_port_id': 'bar'}) - headers = {api_base.Version.string: '1.27'} - response = self._test_add_extra_vif_port_id(port, headers, - mock_warn, mock_upd) - self.assertEqual(expected_intern_info, response.json['internal_info']) - self.assertFalse(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_add_extra_vif_port_id_no_internal(self, mock_warn, mock_upd): - port = obj_utils.create_test_port(self.context, node_id=self.node.id, - uuid=uuidutils.generate_uuid(), - address='52:55:00:cf:2d:31') - expected_intern_info = port.internal_info - expected_intern_info.update({'tenant_vif_port_id': 'bar'}) - headers = {api_base.Version.string: '1.27'} - response = self._test_add_extra_vif_port_id(port, headers, - mock_warn, mock_upd) - self.assertEqual(expected_intern_info, response.json['internal_info']) - self.assertFalse(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_add_extra_vif_port_id_deprecated(self, mock_warn, mock_upd): - port = obj_utils.create_test_port(self.context, node_id=self.node.id, - uuid=uuidutils.generate_uuid(), - address='52:55:00:cf:2d:31') - expected_intern_info = port.internal_info - expected_intern_info.update({'tenant_vif_port_id': 'bar'}) - headers = {api_base.Version.string: '1.34'} - response = self._test_add_extra_vif_port_id(port, headers, - mock_warn, mock_upd) - self.assertEqual(expected_intern_info, response.json['internal_info']) - self.assertTrue(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_replace_extra_vif_port_id(self, mock_warn, mock_upd): - extra = {'vif_port_id': 'original'} - internal_info = {'tenant_vif_port_id': 'original'} - port = obj_utils.create_test_port(self.context, node_id=self.node.id, - uuid=uuidutils.generate_uuid(), - address='52:55:00:cf:2d:31', - extra=extra, - internal_info=internal_info) - expected_intern_info = port.internal_info - expected_intern_info.update({'tenant_vif_port_id': 'bar'}) - headers = {api_base.Version.string: '1.27'} - response = self._test_add_extra_vif_port_id(port, headers, - mock_warn, mock_upd) - self.assertEqual(expected_intern_info, response.json['internal_info']) - self.assertFalse(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_add_extra_vif_port_id_diff_internal(self, mock_warn, mock_upd): - internal_info = {'tenant_vif_port_id': 'original'} - port = obj_utils.create_test_port(self.context, node_id=self.node.id, - uuid=uuidutils.generate_uuid(), - address='52:55:00:cf:2d:31', - internal_info=internal_info) - headers = {api_base.Version.string: '1.27'} - response = self._test_add_extra_vif_port_id(port, headers, - mock_warn, mock_upd) - self.assertEqual(internal_info, response.json['internal_info']) - self.assertFalse(mock_warn.called) - - def _test_remove_extra_vif_port_id(self, port, headers, mock_warn, - mock_upd): - response = self.patch_json( - '/ports/%s' % port.uuid, - [{'path': '/extra/vif_port_id', 'op': 'remove'}], - headers=headers) - self.assertEqual('application/json', response.content_type) - self.assertEqual(http_client.OK, response.status_code) - self.assertEqual({}, response.json['extra']) - self.assertTrue(mock_upd.called) - return response - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_remove_extra_vif_port_id(self, mock_warn, mock_upd): - internal_info = {'tenant_vif_port_id': 'bar'} - extra = {'vif_port_id': 'bar'} - port = obj_utils.create_test_port(self.context, node_id=self.node.id, - uuid=uuidutils.generate_uuid(), - address='52:55:00:cf:2d:31', - internal_info=internal_info, - extra=extra) - headers = {api_base.Version.string: '1.27'} - response = self._test_remove_extra_vif_port_id(port, headers, - mock_warn, mock_upd) - self.assertEqual({}, response.json['internal_info']) - self.assertFalse(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_remove_extra_vif_port_id_not_same(self, mock_warn, mock_upd): - # .internal_info['tenant_vif_port_id'] != .extra['vif_port_id'] - internal_info = {'tenant_vif_port_id': 'bar'} - extra = {'vif_port_id': 'foo'} - port = obj_utils.create_test_port(self.context, node_id=self.node.id, - uuid=uuidutils.generate_uuid(), - address='52:55:00:cf:2d:31', - internal_info=internal_info, - extra=extra) - headers = {api_base.Version.string: '1.28'} - response = self._test_remove_extra_vif_port_id(port, headers, - mock_warn, mock_upd) - self.assertEqual(internal_info, response.json['internal_info']) - self.assertTrue(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_remove_extra_vif_port_id_not_internal(self, mock_warn, mock_upd): - # no .internal_info['tenant_vif_port_id'] - internal_info = {'foo': 'bar'} - extra = {'vif_port_id': 'bar'} - port = obj_utils.create_test_port(self.context, node_id=self.node.id, - uuid=uuidutils.generate_uuid(), - address='52:55:00:cf:2d:31', - internal_info=internal_info, - extra=extra) - headers = {api_base.Version.string: '1.28'} - response = self._test_remove_extra_vif_port_id(port, headers, - mock_warn, mock_upd) - self.assertEqual(internal_info, response.json['internal_info']) - self.assertTrue(mock_warn.called) - def test_update_in_inspecting_not_allowed(self, mock_upd): self.node.provision_state = states.INSPECTING self.node.save() @@ -1997,13 +1847,10 @@ class TestPost(test_api_base.BaseApiTest): self.mock_gtf.return_value = 'test-topic' self.addCleanup(p.stop) - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) @mock.patch.object(notification_utils, '_emit_api_notification', autospec=True) @mock.patch.object(timeutils, 'utcnow', autospec=True) - def test_create_port(self, mock_utcnow, mock_notify, mock_warn, - mock_create): + def test_create_port(self, mock_utcnow, mock_notify, mock_create): pdict = post_get_test_port() test_time = datetime.datetime(2000, 1, 1, 0, 0) mock_utcnow.return_value = test_time @@ -2033,7 +1880,6 @@ class TestPost(test_api_base.BaseApiTest): obj_fields.NotificationStatus.END, node_uuid=self.node.uuid, portgroup_uuid=self.portgroup.uuid)]) - self.assertEqual(0, mock_warn.call_count) def test_create_port_min_api_version(self, mock_create): pdict = post_get_test_port( @@ -2424,43 +2270,10 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual(http_client.FORBIDDEN, response.status_int) self.assertFalse(mock_create.called) - def _test_create_port_with_extra_vif_port_id(self, headers, mock_warn, - mock_create): - pdict = post_get_test_port(pxe_enabled=False, - extra={'vif_port_id': 'foo'}) - pdict.pop('physical_network') - pdict.pop('is_smartnic') - response = self.post_json('/ports', pdict, headers=headers) - self.assertEqual('application/json', response.content_type) - self.assertEqual(http_client.CREATED, response.status_int) - self.assertEqual({'vif_port_id': 'foo'}, response.json['extra']) - self.assertEqual({'tenant_vif_port_id': 'foo'}, - response.json['internal_info']) - mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, - 'test-topic') - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_create_port_with_extra_vif_port_id(self, mock_warn, mock_create): - headers = {api_base.Version.string: '1.27'} - self._test_create_port_with_extra_vif_port_id(headers, mock_warn, - mock_create) - self.assertFalse(mock_warn.called) - - @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, - mock_create): - self._test_create_port_with_extra_vif_port_id(self.headers, mock_warn, - mock_create) - self.assertTrue(mock_warn.called) - - def _test_create_port(self, mock_create, has_vif=False, in_portgroup=False, + def _test_create_port(self, mock_create, in_portgroup=False, pxe_enabled=True, standalone_ports=True, http_status=http_client.CREATED): extra = {} - if has_vif: - extra = {'vif_port_id': uuidutils.generate_uuid()} pdict = post_get_test_port( node_uuid=self.node.uuid, pxe_enabled=pxe_enabled, @@ -2483,89 +2296,79 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual(expected_portgroup_uuid, response.json['portgroup_uuid']) self.assertEqual(extra, response.json['extra']) - if has_vif: - expected = {'tenant_vif_port_id': extra['vif_port_id']} - self.assertEqual(expected, response.json['internal_info']) mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, 'test-topic') else: self.assertFalse(mock_create.called) def test_create_port_novif_pxe_noportgroup(self, mock_create): - self._test_create_port(mock_create, has_vif=False, in_portgroup=False, + self._test_create_port(mock_create, in_portgroup=False, pxe_enabled=True, http_status=http_client.CREATED) def test_create_port_novif_nopxe_noportgroup(self, mock_create): - self._test_create_port(mock_create, has_vif=False, in_portgroup=False, + self._test_create_port(mock_create, in_portgroup=False, pxe_enabled=False, http_status=http_client.CREATED) def test_create_port_vif_pxe_noportgroup(self, mock_create): - self._test_create_port(mock_create, has_vif=True, in_portgroup=False, + self._test_create_port(mock_create, in_portgroup=False, pxe_enabled=True, http_status=http_client.CREATED) def test_create_port_vif_nopxe_noportgroup(self, mock_create): - self._test_create_port(mock_create, has_vif=True, in_portgroup=False, + self._test_create_port(mock_create, in_portgroup=False, pxe_enabled=False, http_status=http_client.CREATED) def test_create_port_novif_pxe_portgroup_standalone_ports(self, mock_create): - self._test_create_port(mock_create, has_vif=False, in_portgroup=True, + self._test_create_port(mock_create, in_portgroup=True, pxe_enabled=True, standalone_ports=True, http_status=http_client.CREATED) def test_create_port_novif_pxe_portgroup_nostandalone_ports(self, mock_create): - self._test_create_port(mock_create, has_vif=False, in_portgroup=True, + self._test_create_port(mock_create, in_portgroup=True, pxe_enabled=True, standalone_ports=False, http_status=http_client.CONFLICT) def test_create_port_novif_nopxe_portgroup_standalone_ports(self, mock_create): - self._test_create_port(mock_create, has_vif=False, in_portgroup=True, + self._test_create_port(mock_create, in_portgroup=True, pxe_enabled=False, standalone_ports=True, http_status=http_client.CREATED) def test_create_port_novif_nopxe_portgroup_nostandalone_ports(self, mock_create): - self._test_create_port(mock_create, has_vif=False, in_portgroup=True, + self._test_create_port(mock_create, in_portgroup=True, pxe_enabled=False, standalone_ports=False, http_status=http_client.CREATED) def test_create_port_vif_pxe_portgroup_standalone_ports(self, mock_create): - self._test_create_port(mock_create, has_vif=True, in_portgroup=True, + self._test_create_port(mock_create, in_portgroup=True, pxe_enabled=True, standalone_ports=True, http_status=http_client.CREATED) def test_create_port_vif_pxe_portgroup_nostandalone_ports(self, mock_create): - self._test_create_port(mock_create, has_vif=True, in_portgroup=True, + self._test_create_port(mock_create, in_portgroup=True, pxe_enabled=True, standalone_ports=False, http_status=http_client.CONFLICT) def test_create_port_vif_nopxe_portgroup_standalone_ports(self, mock_create): - self._test_create_port(mock_create, has_vif=True, in_portgroup=True, + self._test_create_port(mock_create, in_portgroup=True, pxe_enabled=False, standalone_ports=True, http_status=http_client.CREATED) - def test_create_port_vif_nopxe_portgroup_nostandalone_ports(self, - mock_create): - self._test_create_port(mock_create, has_vif=True, in_portgroup=True, - pxe_enabled=False, - standalone_ports=False, - http_status=http_client.CONFLICT) - def test_create_port_invalid_physnet_non_text(self, mock_create): physnet = 1234 pdict = post_get_test_port(physical_network=physnet) diff --git a/ironic/tests/unit/api/controllers/v1/test_portgroup.py b/ironic/tests/unit/api/controllers/v1/test_portgroup.py index 334a643001..394e502ef6 100644 --- a/ironic/tests/unit/api/controllers/v1/test_portgroup.py +++ b/ironic/tests/unit/api/controllers/v1/test_portgroup.py @@ -29,7 +29,6 @@ from ironic.api.controllers.v1 import notification_utils from ironic.api.controllers.v1 import utils as api_utils from ironic.common import exception from ironic.common import states -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 @@ -1087,137 +1086,6 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(address.lower(), kargs.address) -@mock.patch.object(rpcapi.ConductorAPI, 'update_portgroup', autospec=True, - side_effect=_rpcapi_update_portgroup) -class TestPatchExtraVifPortId(test_api_base.BaseApiTest): - headers = {api_base.Version.string: str(api_v1.max_version())} - - def setUp(self): - super(TestPatchExtraVifPortId, self).setUp() - self.node = obj_utils.create_test_node(self.context) - self.portgroup = obj_utils.create_test_portgroup(self.context, - node_id=self.node.id) - p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for', - autospec=True) - self.mock_gtf = p.start() - self.mock_gtf.return_value = 'test-topic' - self.addCleanup(p.stop) - - def _test_add_extra_vif_port_id(self, headers, mock_warn, mock_upd): - extra = {'vif_port_id': 'bar'} - response = self.patch_json( - '/portgroups/%s' % self.portgroup.uuid, - [{'path': '/extra/vif_port_id', 'value': 'foo', 'op': 'add'}, - {'path': '/extra/vif_port_id', 'value': 'bar', 'op': 'add'}], - headers=headers) - self.assertEqual('application/json', response.content_type) - self.assertEqual(http_client.OK, response.status_code) - self.assertEqual(extra, response.json['extra']) - return response - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_add_extra_vif_port_id(self, mock_warn, mock_upd): - expected_intern_info = self.portgroup.internal_info - expected_intern_info.update({'tenant_vif_port_id': 'bar'}) - headers = {api_base.Version.string: '1.27'} - response = self._test_add_extra_vif_port_id(headers, mock_warn, - mock_upd) - self.assertEqual(expected_intern_info, response.json['internal_info']) - self.assertFalse(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_add_extra_vif_port_id_deprecated(self, mock_warn, mock_upd): - expected_intern_info = self.portgroup.internal_info - expected_intern_info.update({'tenant_vif_port_id': 'bar'}) - response = self._test_add_extra_vif_port_id(self.headers, mock_warn, - mock_upd) - self.assertEqual(expected_intern_info, response.json['internal_info']) - self.assertTrue(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_replace_extra_vif_port_id(self, mock_warn, mock_upd): - self.portgroup.extra = {'vif_port_id': 'original'} - self.portgroup.internal_info = {'tenant_vif_port_id': 'original'} - self.portgroup.save() - expected_intern_info = self.portgroup.internal_info - expected_intern_info.update({'tenant_vif_port_id': 'bar'}) - headers = {api_base.Version.string: '1.27'} - response = self._test_add_extra_vif_port_id(headers, mock_warn, - mock_upd) - self.assertEqual(expected_intern_info, response.json['internal_info']) - self.assertFalse(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_add_extra_vif_port_id_diff_internal(self, mock_warn, mock_upd): - internal_info = {'tenant_vif_port_id': 'original'} - self.portgroup.internal_info = internal_info - self.portgroup.save() - headers = {api_base.Version.string: '1.27'} - response = self._test_add_extra_vif_port_id(headers, mock_warn, - mock_upd) - # not changed - self.assertEqual(internal_info, response.json['internal_info']) - self.assertFalse(mock_warn.called) - - def _test_remove_extra_vif_port_id(self, headers, mock_warn, mock_upd): - response = self.patch_json( - '/portgroups/%s' % self.portgroup.uuid, - [{'path': '/extra/vif_port_id', 'op': 'remove'}], - headers=headers) - self.assertEqual('application/json', response.content_type) - self.assertEqual(http_client.OK, response.status_code) - self.assertEqual({}, response.json['extra']) - self.assertTrue(mock_upd.called) - return response - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_remove_extra_vif_port_id(self, mock_warn, mock_upd): - self.portgroup.extra = {'vif_port_id': 'bar'} - orig_info = self.portgroup.internal_info.copy() - intern_info = self.portgroup.internal_info - intern_info.update({'tenant_vif_port_id': 'bar'}) - self.portgroup.internal_info = intern_info - self.portgroup.save() - headers = {api_base.Version.string: '1.27'} - response = self._test_remove_extra_vif_port_id(headers, mock_warn, - mock_upd) - self.assertEqual(orig_info, response.json['internal_info']) - self.assertFalse(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_remove_extra_vif_port_id_not_same(self, mock_warn, mock_upd): - # .internal_info['tenant_vif_port_id'] != .extra['vif_port_id'] - self.portgroup.extra = {'vif_port_id': 'foo'} - intern_info = self.portgroup.internal_info - intern_info.update({'tenant_vif_port_id': 'bar'}) - self.portgroup.internal_info = intern_info - self.portgroup.save() - headers = {api_base.Version.string: '1.28'} - response = self._test_remove_extra_vif_port_id(headers, mock_warn, - mock_upd) - self.assertEqual(intern_info, response.json['internal_info']) - self.assertTrue(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_remove_extra_vif_port_id_not_internal(self, mock_warn, mock_upd): - # no portgroup.internal_info['tenant_vif_port_id'] - self.portgroup.extra = {'vif_port_id': 'foo'} - self.portgroup.save() - intern_info = self.portgroup.internal_info - headers = {api_base.Version.string: '1.28'} - response = self._test_remove_extra_vif_port_id(headers, mock_warn, - mock_upd) - self.assertEqual(intern_info, response.json['internal_info']) - self.assertTrue(mock_warn.called) - - class TestPost(test_api_base.BaseApiTest): headers = {api_base.Version.string: str(api_v1.max_version())} @@ -1227,10 +1095,8 @@ class TestPost(test_api_base.BaseApiTest): @mock.patch.object(notification_utils, '_emit_api_notification', autospec=True) - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) @mock.patch.object(timeutils, 'utcnow', autospec=True) - def test_create_portgroup(self, mock_utcnow, mock_warn, mock_notify): + def test_create_portgroup(self, mock_utcnow, mock_notify): pdict = apiutils.post_get_test_portgroup() test_time = datetime.datetime(2000, 1, 1, 0, 0) mock_utcnow.return_value = test_time @@ -1249,7 +1115,6 @@ class TestPost(test_api_base.BaseApiTest): expected_location = '/v1/portgroups/%s' % pdict['uuid'] self.assertEqual(urlparse.urlparse(response.location).path, expected_location) - self.assertEqual(0, mock_warn.call_count) mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'create', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, @@ -1340,41 +1205,6 @@ class TestPost(test_api_base.BaseApiTest): headers=self.headers) self.assertEqual(pdict['extra'], result['extra']) - def _test_create_portgroup_with_extra_vif_port_id(self, headers, - mock_warn): - pgdict = apiutils.post_get_test_portgroup(extra={'vif_port_id': 'foo'}) - response = self.post_json('/portgroups', pgdict, headers=headers) - self.assertEqual('application/json', response.content_type) - self.assertEqual(http_client.CREATED, response.status_int) - self.assertEqual({'vif_port_id': 'foo'}, response.json['extra']) - self.assertEqual({'tenant_vif_port_id': 'foo'}, - response.json['internal_info']) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_create_portgroup_with_extra_vif_port_id(self, mock_warn): - headers = {api_base.Version.string: '1.27'} - self._test_create_portgroup_with_extra_vif_port_id(headers, mock_warn) - self.assertFalse(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_create_portgroup_with_extra_vif_port_id_deprecated( - self, mock_warn): - self._test_create_portgroup_with_extra_vif_port_id( - self.headers, mock_warn) - self.assertTrue(mock_warn.called) - - @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', - autospec=True) - def test_create_portgroup_with_no_extra(self, mock_warn): - pgdict = apiutils.post_get_test_portgroup() - del pgdict['extra'] - response = self.post_json('/portgroups', pgdict, headers=self.headers) - self.assertEqual('application/json', response.content_type) - self.assertEqual(http_client.CREATED, response.status_int) - self.assertEqual(0, mock_warn.call_count) - def test_create_portgroup_no_address(self): pdict = apiutils.post_get_test_portgroup() del pdict['address'] diff --git a/ironic/tests/unit/common/test_network.py b/ironic/tests/unit/common/test_network.py index f0258558c4..0afbe35739 100644 --- a/ironic/tests/unit/common/test_network.py +++ b/ironic/tests/unit/common/test_network.py @@ -41,11 +41,8 @@ class TestNetwork(db_base.DbTestCase): result = network.get_node_vif_ids(task) self.assertEqual(expected, result) - def _test_get_node_vif_ids_one_port(self, key): - if key == "extra": - kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} - else: - kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} + def test_get_node_vif_ids_one_port_int_info(self): + kwargs1 = {'internal_info': {'tenant_vif_port_id': 'test-vif-A'}} port1 = db_utils.create_test_port(node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', uuid=uuidutils.generate_uuid(), @@ -56,17 +53,8 @@ class TestNetwork(db_base.DbTestCase): result = network.get_node_vif_ids(task) self.assertEqual(expected, result) - def test_get_node_vif_ids_one_port_extra(self): - self._test_get_node_vif_ids_one_port("extra") - - def test_get_node_vif_ids_one_port_int_info(self): - self._test_get_node_vif_ids_one_port("internal_info") - - def _test_get_node_vif_ids_one_portgroup(self, key): - if key == "extra": - kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} - else: - kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} + def test_get_node_vif_ids_one_portgroup_int_info(self): + kwargs1 = {'internal_info': {'tenant_vif_port_id': 'test-vif-A'}} pg1 = db_utils.create_test_portgroup( node_id=self.node.id, **kwargs1) @@ -76,19 +64,9 @@ class TestNetwork(db_base.DbTestCase): result = network.get_node_vif_ids(task) self.assertEqual(expected, result) - def test_get_node_vif_ids_one_portgroup_extra(self): - self._test_get_node_vif_ids_one_portgroup("extra") - - def test_get_node_vif_ids_one_portgroup_int_info(self): - self._test_get_node_vif_ids_one_portgroup("internal_info") - - def _test_get_node_vif_ids_two_ports(self, key): - if key == "extra": - kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} - kwargs2 = {key: {'vif_port_id': 'test-vif-B'}} - else: - kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} - kwargs2 = {key: {'tenant_vif_port_id': 'test-vif-B'}} + def test_get_node_vif_ids_two_ports_int_info(self): + kwargs1 = {'internal_info': {'tenant_vif_port_id': 'test-vif-A'}} + kwargs2 = {'internal_info': {'tenant_vif_port_id': 'test-vif-B'}} port1 = db_utils.create_test_port(node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', uuid=uuidutils.generate_uuid(), @@ -104,19 +82,9 @@ class TestNetwork(db_base.DbTestCase): result = network.get_node_vif_ids(task) self.assertEqual(expected, result) - def test_get_node_vif_ids_two_ports_extra(self): - self._test_get_node_vif_ids_two_ports('extra') - - def test_get_node_vif_ids_two_ports_int_info(self): - self._test_get_node_vif_ids_two_ports('internal_info') - - def _test_get_node_vif_ids_two_portgroups(self, key): - if key == "extra": - kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} - kwargs2 = {key: {'vif_port_id': 'test-vif-B'}} - else: - kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} - kwargs2 = {key: {'tenant_vif_port_id': 'test-vif-B'}} + def test_get_node_vif_ids_two_portgroups_int_info(self): + kwargs1 = {'internal_info': {'tenant_vif_port_id': 'test-vif-A'}} + kwargs2 = {'internal_info': {'tenant_vif_port_id': 'test-vif-B'}} pg1 = db_utils.create_test_portgroup( node_id=self.node.id, **kwargs1) pg2 = db_utils.create_test_portgroup( @@ -131,12 +99,6 @@ class TestNetwork(db_base.DbTestCase): result = network.get_node_vif_ids(task) self.assertEqual(expected, result) - def test_get_node_vif_ids_two_portgroups_extra(self): - self._test_get_node_vif_ids_two_portgroups('extra') - - def test_get_node_vif_ids_two_portgroups_int_info(self): - self._test_get_node_vif_ids_two_portgroups('internal_info') - def _test_get_node_vif_ids_multitenancy(self, int_info_key): port = db_utils.create_test_port( node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', diff --git a/ironic/tests/unit/common/test_utils.py b/ironic/tests/unit/common/test_utils.py index 9bfcddb863..a1b73c033b 100644 --- a/ironic/tests/unit/common/test_utils.py +++ b/ironic/tests/unit/common/test_utils.py @@ -306,16 +306,6 @@ 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/dhcp/test_neutron.py b/ironic/tests/unit/dhcp/test_neutron.py index 97c552fbe8..8d87345d28 100644 --- a/ironic/tests/unit/dhcp/test_neutron.py +++ b/ironic/tests/unit/dhcp/test_neutron.py @@ -477,11 +477,8 @@ class TestNeutron(db_base.DbTestCase): @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address', autospec=True) - def _test__get_ip_addresses_ports(self, key, mock_gfia): - if key == "extra": - kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} - else: - kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} + def test__get_ip_addresses_ports_int_info(self, mock_gfia): + kwargs1 = {'internal_info': {'tenant_vif_port_id': 'test-vif-A'}} ip_address = '10.10.0.1' expected = [ip_address] port = object_utils.create_test_port(self.context, @@ -496,19 +493,10 @@ class TestNeutron(db_base.DbTestCase): mock.sentinel.client) self.assertEqual(expected, result) - def test__get_ip_addresses_ports_extra(self): - self._test__get_ip_addresses_ports('extra') - - def test__get_ip_addresses_ports_int_info(self): - self._test__get_ip_addresses_ports('internal_info') - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address', autospec=True) - def _test__get_ip_addresses_portgroup(self, key, mock_gfia): - if key == "extra": - kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} - else: - kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} + def test__get_ip_addresses_portgroup_int_info(self, mock_gfia): + kwargs1 = {'internal_info': {'tenant_vif_port_id': 'test-vif-A'}} ip_address = '10.10.0.1' expected = [ip_address] pg = object_utils.create_test_portgroup( @@ -521,12 +509,6 @@ class TestNeutron(db_base.DbTestCase): result = api._get_ip_addresses(task, [pg], mock.sentinel.client) self.assertEqual(expected, result) - def test__get_ip_addresses_portgroup_extra(self): - self._test__get_ip_addresses_portgroup('extra') - - def test__get_ip_addresses_portgroup_int_info(self): - self._test__get_ip_addresses_portgroup('internal_info') - @mock.patch('ironic.common.neutron.get_client', autospec=True) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_port_ip_address', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py index 9e8ac2b414..555024216d 100644 --- a/ironic/tests/unit/drivers/modules/network/test_common.py +++ b/ironic/tests/unit/drivers/modules/network/test_common.py @@ -70,7 +70,7 @@ class TestCommonFunctions(db_base.DbTestCase): self.context, node_id=self.node.id, address='52:54:00:cf:2d:04', physical_network=physical_network, - extra={'vif_port_id': 'some-vif'}, + internal_info={'tenant_vif_port_id': 'some-vif'}, uuid=uuidutils.generate_uuid(), portgroup_id=pg2.id)) # This portgroup has 'some-vif-2' attached to it and contains one port, # so neither portgroup nor port can be considered free. The ports are @@ -368,7 +368,7 @@ class TestCommonFunctions(db_base.DbTestCase): obj_utils.create_test_port( self.context, node_id=self.node.id, address='52:54:00:cf:2d:01', uuid=uuidutils.generate_uuid(), portgroup_id=pg.id, - extra={'vif_port_id': 'some-vif'}) + internal_info={'tenant_vif_port_id': 'some-vif'}) free_port = obj_utils.create_test_port( self.context, node_id=self.node.id, address='52:54:00:cf:2d:02', uuid=uuidutils.generate_uuid(), portgroup_id=pg.id) @@ -394,23 +394,6 @@ class TestCommonFunctions(db_base.DbTestCase): common.get_free_port_like_object, task, self.vif_id, {'anyphysnet'}) - @mock.patch.object(neutron_common, 'validate_port_info', autospec=True, - return_value=True) - def test_get_free_port_like_object_vif_attached_to_portgroup_extra( - self, vpi_mock): - pg = obj_utils.create_test_portgroup( - self.context, node_id=self.node.id, - extra={'vif_port_id': self.vif_id}) - obj_utils.create_test_port( - self.context, node_id=self.node.id, address='52:54:00:cf:2d:01', - uuid=uuidutils.generate_uuid(), portgroup_id=pg.id) - with task_manager.acquire(self.context, self.node.id) as task: - self.assertRaisesRegex( - exception.VifAlreadyAttached, - r"already attached to Ironic Portgroup", - common.get_free_port_like_object, - task, self.vif_id, {'anyphysnet'}) - @mock.patch.object(neutron_common, 'validate_port_info', autospec=True, return_value=True) def test_get_free_port_like_object_vif_attached_to_port(self, vpi_mock): @@ -423,23 +406,10 @@ class TestCommonFunctions(db_base.DbTestCase): common.get_free_port_like_object, task, self.vif_id, {'anyphysnet'}) - @mock.patch.object(neutron_common, 'validate_port_info', autospec=True, - return_value=True) - def test_get_free_port_like_object_vif_attached_to_port_extra( - self, vpi_mock): - self.port.extra = {'vif_port_id': self.vif_id} - self.port.save() - with task_manager.acquire(self.context, self.node.id) as task: - self.assertRaisesRegex( - exception.VifAlreadyAttached, - r"already attached to Ironic Port\b", - common.get_free_port_like_object, - task, self.vif_id, {'anyphysnet'}) - @mock.patch.object(neutron_common, 'validate_port_info', autospec=True, return_value=True) def test_get_free_port_like_object_nothing_free(self, vpi_mock): - self.port.extra = {'vif_port_id': 'another-vif'} + self.port.internal_info = {'tenant_vif_port_id': 'another-vif'} self.port.save() with task_manager.acquire(self.context, self.node.id) as task: self.assertRaises(exception.NoFreePhysicalPorts, @@ -523,8 +493,8 @@ class TestVifPortIDMixin(db_base.DbTestCase): self.port = obj_utils.create_test_port( self.context, node_id=self.node.id, address='52:54:00:cf:2d:32', - extra={'vif_port_id': uuidutils.generate_uuid(), - 'client-id': 'fake1'}) + internal_info={'tenant_vif_port_id': uuidutils.generate_uuid()}, + extra={'client-id': 'fake1'}) network_data_file = os.path.join( os.path.dirname(__file__), 'json_samples', 'network_data.json') with open(network_data_file, 'rb') as fl: @@ -564,7 +534,10 @@ class TestVifPortIDMixin(db_base.DbTestCase): def test__clear_vif_from_port_like_obj_in_internal_info_port(self): self.port.internal_info = { - common.TENANT_VIF_KEY: self.port.extra['vif_port_id']} + common.TENANT_VIF_KEY: self.port.internal_info[ + 'tenant_vif_port_id' + ] + } self.port.extra = {} self.port.save() @@ -601,14 +574,8 @@ class TestVifPortIDMixin(db_base.DbTestCase): self.assertNotIn('vif_port_id', pg.extra) self.assertNotIn(common.TENANT_VIF_KEY, pg.internal_info) - def test__get_port_like_obj_by_vif_id_in_extra(self): - vif_id = self.port.extra["vif_port_id"] - with task_manager.acquire(self.context, self.node.id) as task: - result = self.interface._get_port_like_obj_by_vif_id(task, vif_id) - self.assertEqual(self.port.id, result.id) - def test__get_port_like_obj_by_vif_id_in_internal_info(self): - vif_id = self.port.extra["vif_port_id"] + vif_id = self.port.internal_info["tenant_vif_port_id"] self.port.internal_info = {common.TENANT_VIF_KEY: vif_id} self.port.extra = {} self.port.save() @@ -617,8 +584,8 @@ class TestVifPortIDMixin(db_base.DbTestCase): self.assertEqual(self.port.id, result.id) def test__get_port_like_obj_by_vif_id_not_attached(self): - vif_id = self.port.extra["vif_port_id"] - self.port.extra = {} + vif_id = self.port.internal_info["tenant_vif_port_id"] + self.port.internal_info = {} self.port.save() with task_manager.acquire(self.context, self.node.id) as task: self.assertRaisesRegex(exception.VifNotAttached, @@ -626,13 +593,8 @@ class TestVifPortIDMixin(db_base.DbTestCase): self.interface._get_port_like_obj_by_vif_id, task, vif_id) - def test__get_vif_id_by_port_like_obj_in_extra(self): - vif_id = self.port.extra["vif_port_id"] - result = self.interface._get_vif_id_by_port_like_obj(self.port) - self.assertEqual(vif_id, result) - def test__get_vif_id_by_port_like_obj_in_internal_info(self): - vif_id = self.port.extra["vif_port_id"] + vif_id = self.port.internal_info["tenant_vif_port_id"] self.port.internal_info = {common.TENANT_VIF_KEY: vif_id} self.port.extra = {} self.port.save() @@ -640,14 +602,14 @@ class TestVifPortIDMixin(db_base.DbTestCase): self.assertEqual(vif_id, result) def test__get_vif_id_by_port_like_obj_not_attached(self): - self.port.extra = {} + self.port.internal_info = {} self.port.save() result = self.interface._get_vif_id_by_port_like_obj(self.port) self.assertIsNone(result) - def test_vif_list_extra(self): + def test_vif_list_port_and_portgroup(self): vif_id = uuidutils.generate_uuid() - self.port.extra = {'vif_port_id': vif_id} + self.port.internal_info = {'tenant_vif_port_id': vif_id} self.port.save() pg_vif_id = uuidutils.generate_uuid() portgroup = obj_utils.create_test_portgroup( @@ -678,6 +640,7 @@ class TestVifPortIDMixin(db_base.DbTestCase): self.assertCountEqual([{'id': pg_vif_id}, {'id': vif_id}], vifs) def test_vif_list_extra_and_internal_priority(self): + # TODO(TheJulia): Remove in Xena? vif_id = uuidutils.generate_uuid() vif_id2 = uuidutils.generate_uuid() self.port.extra = {'vif_port_id': vif_id2} @@ -687,14 +650,6 @@ class TestVifPortIDMixin(db_base.DbTestCase): vifs = self.interface.vif_list(task) self.assertEqual([{'id': vif_id}], vifs) - def test_get_current_vif_extra_vif_port_id(self): - extra = {'vif_port_id': 'foo'} - self.port.extra = extra - self.port.save() - with task_manager.acquire(self.context, self.node.id) as task: - vif = self.interface.get_current_vif(task, self.port) - self.assertEqual('foo', vif) - def test_get_current_vif_internal_info_cleaning(self): internal_info = {'cleaning_vif_port_id': 'foo', 'tenant_vif_port_id': 'bar'} @@ -750,8 +705,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): self.port = obj_utils.create_test_port( self.context, node_id=self.node.id, address='52:54:00:cf:2d:32', - extra={'vif_port_id': uuidutils.generate_uuid(), - 'client-id': 'fake1'}) + internal_info={'tenant_vif_port_id': uuidutils.generate_uuid()}, + extra={'client-id': 'fake1'}) self.neutron_port = {'id': '132f871f-eaec-4fed-9475-0d54465e0f00', 'mac_address': '52:54:00:cf:2d:32'} @@ -903,7 +858,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): autospec=True) def test_vif_attach_update_port_exception(self, mock_gpbpi, mock_upa, mock_client, mock_save): - self.port.extra = {} + self.port.internal_info = {} self.port.physical_network = 'physnet1' self.port.save() vif = {'id': "fake_vif_id"} @@ -1075,7 +1030,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.port_changed(task, self.port) mac_update_mock.assert_called_once_with( - self.port.extra['vif_port_id'], + self.port.internal_info['tenant_vif_port_id'], new_address, context=task.context) @@ -1090,12 +1045,12 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): self.interface.port_changed, task, self.port) mac_update_mock.assert_called_once_with( - self.port.extra['vif_port_id'], new_address, + self.port.internal_info['tenant_vif_port_id'], new_address, context=task.context) @mock.patch.object(neutron_common, 'update_port_address', autospec=True) def test_port_changed_address_no_vif_id(self, mac_update_mock): - self.port.extra = {} + self.port.internal_info = {} self.port.save() self.port.address = '11:22:33:44:55:bb' with task_manager.acquire(self.context, self.node.id) as task: @@ -1105,9 +1060,11 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts', autospec=True) def test_port_changed_client_id(self, dhcp_update_mock): - expected_extra = {'vif_port_id': 'fake-id', 'client-id': 'fake2'} + expected_ii = {'tenant_vif_port_id': 'fake-id'} + expected_extra = {'client-id': 'fake2'} expected_dhcp_opts = [{'opt_name': '61', 'opt_value': 'fake2'}] self.port.extra = expected_extra + self.port.internal_info = expected_ii with task_manager.acquire(self.context, self.node.id) as task: self.interface.port_changed(task, self.port) dhcp_update_mock.assert_called_once_with( @@ -1116,7 +1073,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts', autospec=True) def test_port_changed_extra_add_new_key(self, dhcp_update_mock): - self.port.extra = {'vif_port_id': 'fake-id'} + self.port.internal_info = {'tenant_vif_port_id': 'fake-id'} self.port.save() expected_extra = self.port.extra expected_extra['foo'] = 'bar' @@ -1128,7 +1085,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts', autospec=True) def test_port_changed_client_id_fail(self, dhcp_update_mock): - self.port.extra = {'vif_port_id': 'fake-id', 'client-id': 'fake2'} + self.port.internal_info = {'tenant_vif_port_id': 'fake-id'} + self.port.extra = {'client-id': 'fake2'} dhcp_update_mock.side_effect = ( exception.FailedToUpdateDHCPOptOnPort(port_id=self.port.uuid)) with task_manager.acquire(self.context, self.node.id) as task: @@ -1139,6 +1097,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts', autospec=True) def test_port_changed_client_id_no_vif_id(self, dhcp_update_mock): + self.port.internal_info = {} self.port.extra = {'client-id': 'fake1'} self.port.save() self.port.extra = {'client-id': 'fake2'} @@ -1153,11 +1112,12 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): self.context, node_id=self.node.id, standalone_ports_supported=False) - port = obj_utils.create_test_port(self.context, node_id=self.node.id, - uuid=uuidutils.generate_uuid(), - address="aa:bb:cc:dd:ee:01", - extra={'vif_port_id': 'blah'}, - pxe_enabled=False) + port = obj_utils.create_test_port( + self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address="aa:bb:cc:dd:ee:01", + internal_info={'tenant_vif_port_id': 'blah'}, + pxe_enabled=False) port.portgroup_id = pg.id with task_manager.acquire(self.context, self.node.id) as task: @@ -1173,12 +1133,12 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): self.context, node_id=self.node.id, standalone_ports_supported=standalone_ports) - extra_vif = {'vif_port_id': uuidutils.generate_uuid()} + extra_vif = {'tenant_vif_port_id': uuidutils.generate_uuid()} if has_vif: - extra = extra_vif + internal_info = extra_vif opposite_extra = {} else: - extra = {} + internal_info = {} opposite_extra = extra_vif opposite_pxe_enabled = not pxe_enabled @@ -1193,7 +1153,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): p1 = obj_utils.create_test_port(self.context, node_id=self.node.id, uuid=uuidutils.generate_uuid(), address="aa:bb:cc:dd:ee:01", - extra=extra, + internal_info=internal_info, pxe_enabled=pxe_enabled) p1.portgroup_id = pg_id ports.append(p1) @@ -1202,9 +1162,9 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): p2 = obj_utils.create_test_port(self.context, node_id=self.node.id, uuid=uuidutils.generate_uuid(), address="aa:bb:cc:dd:ee:02", - extra=opposite_extra, + internal_info=opposite_extra, pxe_enabled=opposite_pxe_enabled) - p2.extra = extra + p2.internal_info = internal_info p2.pxe_enabled = pxe_enabled p2.portgroup_id = pg_id ports.append(p2) @@ -1213,7 +1173,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): p3 = obj_utils.create_test_port(self.context, node_id=self.node.id, uuid=uuidutils.generate_uuid(), address="aa:bb:cc:dd:ee:03", - extra=extra, + internal_info=internal_info, pxe_enabled=opposite_pxe_enabled) p3.pxe_enabled = pxe_enabled p3.portgroup_id = pg_id @@ -1224,8 +1184,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): uuid=uuidutils.generate_uuid(), address="aa:bb:cc:dd:ee:04", pxe_enabled=pxe_enabled, - extra=opposite_extra) - p4.extra = extra + internal_info=opposite_extra) + p4.internal_info = internal_info p4.portgroup_id = pg_id ports.append(p4) @@ -1302,7 +1262,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): def test_update_portgroup_address(self, mac_update_mock): pg = obj_utils.create_test_portgroup( self.context, node_id=self.node.id, - extra={'vif_port_id': 'fake-id'}) + internal_info={'tenant_vif_port_id': 'fake-id'}) new_address = '11:22:33:44:55:bb' pg.address = new_address with task_manager.acquire(self.context, self.node.id) as task: @@ -1314,7 +1274,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): def test_update_portgroup_remove_address(self, mac_update_mock): pg = obj_utils.create_test_portgroup( self.context, node_id=self.node.id, - extra={'vif_port_id': 'fake-id'}) + internal_info={'tenant_vif_port_id': 'fake-id'}) pg.address = None with task_manager.acquire(self.context, self.node.id) as task: self.interface.portgroup_changed(task, pg) @@ -1324,7 +1284,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): def test_update_portgroup_address_fail(self, mac_update_mock): pg = obj_utils.create_test_portgroup( self.context, node_id=self.node.id, - extra={'vif_port_id': 'fake-id'}) + internal_info={'tenant_vif_port_id': 'fake-id'}) new_address = '11:22:33:44:55:bb' pg.address = new_address mac_update_mock.side_effect = ( @@ -1352,9 +1312,10 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): self, mac_update_mock): pg = obj_utils.create_test_portgroup( self.context, node_id=self.node.id) - extra = {'vif_port_id': 'foo'} + internal_info = {'tenant_vif_port_id': 'foo'} obj_utils.create_test_port( - self.context, node_id=self.node.id, extra=extra, + self.context, node_id=self.node.id, + internal_info=internal_info, pxe_enabled=True, portgroup_id=pg.id, address="aa:bb:cc:dd:ee:01", uuid=uuidutils.generate_uuid()) @@ -1378,12 +1339,15 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): standalone_ports_supported=old_standalone_ports_supported) if with_ports: - extra = {} + internal_info = {} if has_vif: - extra = {'vif_port_id': uuidutils.generate_uuid()} + internal_info = { + 'tenant_vif_port_id': uuidutils.generate_uuid() + } obj_utils.create_test_port( - self.context, node_id=self.node.id, extra=extra, + self.context, node_id=self.node.id, + internal_info=internal_info, pxe_enabled=pxe_enabled, portgroup_id=pg.id, address="aa:bb:cc:dd:ee:01", uuid=uuidutils.generate_uuid()) diff --git a/ironic/tests/unit/drivers/modules/network/test_flat.py b/ironic/tests/unit/drivers/modules/network/test_flat.py index f76f2a4463..97e691f493 100644 --- a/ironic/tests/unit/drivers/modules/network/test_flat.py +++ b/ironic/tests/unit/drivers/modules/network/test_flat.py @@ -174,9 +174,10 @@ class TestFlatInterface(db_base.DbTestCase): @mock.patch.object(neutron, 'update_neutron_port', autospec=True) def test__bind_flat_ports_set_binding_host_id(self, update_mock): - extra = {'vif_port_id': 'foo'} + internal_info = {'tenant_vif_port_id': 'foo'} utils.create_test_port(self.context, node_id=self.node.id, - address='52:54:00:cf:2d:33', extra=extra, + address='52:54:00:cf:2d:33', + internal_info=internal_info, uuid=uuidutils.generate_uuid()) exp_body = {'binding:host_id': self.node.uuid, 'binding:vnic_type': neutron.VNIC_BAREMETAL, @@ -193,7 +194,8 @@ class TestFlatInterface(db_base.DbTestCase): uuid=uuidutils.generate_uuid()) utils.create_test_port( self.context, node_id=self.node.id, address='52:54:00:cf:2d:33', - extra={'vif_port_id': 'bar'}, uuid=uuidutils.generate_uuid()) + internal_info={'tenant_vif_port_id': 'bar'}, + uuid=uuidutils.generate_uuid()) exp_body1 = {'binding:host_id': self.node.uuid, 'binding:vnic_type': neutron.VNIC_BAREMETAL, 'mac_address': '52:54:00:cf:2d:33'} @@ -208,9 +210,10 @@ class TestFlatInterface(db_base.DbTestCase): @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True) def test__unbind_flat_ports(self, unbind_neutron_port_mock): - extra = {'vif_port_id': 'foo'} + internal_info = {'tenant_vif_port_id': 'foo'} utils.create_test_port(self.context, node_id=self.node.id, - address='52:54:00:cf:2d:33', extra=extra, + address='52:54:00:cf:2d:33', + internal_info=internal_info, uuid=uuidutils.generate_uuid()) with task_manager.acquire(self.context, self.node.id) as task: self.interface._unbind_flat_ports(task) @@ -223,9 +226,10 @@ class TestFlatInterface(db_base.DbTestCase): utils.create_test_portgroup(self.context, node_id=self.node.id, internal_info=internal_info, uuid=uuidutils.generate_uuid()) - extra = {'vif_port_id': 'bar'} + internal_info = {'tenant_vif_port_id': 'bar'} utils.create_test_port(self.context, node_id=self.node.id, - address='52:54:00:cf:2d:33', extra=extra, + address='52:54:00:cf:2d:33', + internal_info=internal_info, uuid=uuidutils.generate_uuid()) with task_manager.acquire(self.context, self.node.id) as task: self.interface._unbind_flat_ports(task) @@ -236,9 +240,10 @@ class TestFlatInterface(db_base.DbTestCase): @mock.patch.object(neutron, 'update_neutron_port', autospec=True) def test__bind_flat_ports_set_binding_host_id_raise(self, update_mock): update_mock.side_effect = openstack_exc.OpenStackCloudException() - extra = {'vif_port_id': 'foo'} + internal_info = {'tenant_vif_port_id': 'foo'} utils.create_test_port(self.context, node_id=self.node.id, - address='52:54:00:cf:2d:33', extra=extra, + address='52:54:00:cf:2d:33', + internal_info=internal_info, uuid=uuidutils.generate_uuid()) with task_manager.acquire(self.context, self.node.id) as task: self.assertRaises(exception.NetworkError, diff --git a/ironic/tests/unit/drivers/modules/network/test_neutron.py b/ironic/tests/unit/drivers/modules/network/test_neutron.py index 895c8ec5aa..0b70446cec 100644 --- a/ironic/tests/unit/drivers/modules/network/test_neutron.py +++ b/ironic/tests/unit/drivers/modules/network/test_neutron.py @@ -53,7 +53,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.port = utils.create_test_port( self.context, node_id=self.node.id, address='52:54:00:cf:2d:32', - extra={'vif_port_id': uuidutils.generate_uuid()}) + internal_info={'tenant_vif_port_id': uuidutils.generate_uuid()}) self.neutron_port = stubs.FakeNeutronPort( id='132f871f-eaec-4fed-9475-0d54465e0f00', mac_address='52:54:00:cf:2d:32') @@ -425,7 +425,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.context, node_id=self.node.id, address='52:54:00:cf:2d:33', uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': uuidutils.generate_uuid()}) + internal_info={'tenant_vif_port_id': uuidutils.generate_uuid()}) neutron_other_port = {'id': uuidutils.generate_uuid(), 'mac_address': '52:54:00:cf:2d:33'} add_ports_mock.return_value = { @@ -456,7 +456,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.context, node_id=self.node.id, address='52:54:00:cf:2d:33', uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': uuidutils.generate_uuid()}) + internal_info={'tenant_vif_port_id': uuidutils.generate_uuid()}) neutron_other_port = {'id': uuidutils.generate_uuid(), 'mac_address': '52:54:00:cf:2d:33'} add_ports_mock.return_value = { @@ -514,7 +514,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.context, node_id=self.node.id, address='52:54:00:cf:2d:33', uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': uuidutils.generate_uuid()}) + internal_info={'tenant_vif_port_id': uuidutils.generate_uuid()}) other_port.internal_info = {'rescuing_vif_port_id': 'vif-port-id'} other_port.save() with task_manager.acquire(self.context, self.node.id) as task: @@ -533,7 +533,8 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.unconfigure_tenant_networks(task) mock_unbind_port.assert_called_once_with( - self.port.extra['vif_port_id'], context=task.context, + self.port.internal_info['tenant_vif_port_id'], + context=task.context, reset_mac=True) @mock.patch.object(neutron_common, 'get_client', autospec=True) @@ -551,7 +552,8 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.unconfigure_tenant_networks(task) mock_unbind_port.assert_called_once_with( - self.port.extra['vif_port_id'], context=task.context, + self.port.internal_info['tenant_vif_port_id'], + context=task.context, reset_mac=True) wait_agent_mock.assert_called_once_with(nclient, 'hostname') @@ -563,7 +565,8 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.unconfigure_tenant_networks(task) mock_unbind_port.assert_has_calls([ - mock.call(self.port.extra['vif_port_id'], context=task.context, + mock.call(self.port.internal_info['tenant_vif_port_id'], + context=task.context, reset_mac=True), mock.call(pg.internal_info['tenant_vif_port_id'], context=task.context, reset_mac=True)]) @@ -576,7 +579,8 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.unconfigure_tenant_networks(task) mock_unbind_port.assert_has_calls([ - mock.call(self.port.extra['vif_port_id'], context=task.context, + mock.call(self.port.internal_info['tenant_vif_port_id'], + context=task.context, reset_mac=True), mock.call(pg.internal_info['tenant_vif_port_id'], context=task.context, reset_mac=False)]) @@ -592,7 +596,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): @mock.patch.object(neutron_common, 'get_client', autospec=True) @mock.patch.object(neutron, 'LOG', autospec=True) def test_configure_tenant_networks_no_vif_id(self, log_mock, client_mock): - self.port.extra = {} + self.port.internal_info = {} self.port.save() upd_mock = mock.Mock() client_mock.return_value.update_port = upd_mock @@ -624,9 +628,10 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.configure_tenant_networks(task) client_mock.assert_called_once_with(context=task.context) - update_mock.assert_called_once_with(self.context, - self.port.extra['vif_port_id'], - expected_attrs) + update_mock.assert_called_once_with( + self.context, + self.port.internal_info['tenant_vif_port_id'], + expected_attrs) @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True) @mock.patch.object(neutron_common, 'update_neutron_port', autospec=True) @@ -647,16 +652,18 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): @mock.patch.object(neutron_common, 'get_client', autospec=True) def _test_configure_tenant_networks(self, client_mock, update_mock, wait_agent_mock, - is_client_id=False, - vif_int_info=False): - if vif_int_info: - kwargs = {'internal_info': { + is_client_id=False): + # NOTE(TheJulia): Until we have a replacement for infiniband client-id + # storage, extra has to stay put. On a plus side, this would be + # pointless/difficult to abuse other than just break dhcp for the node. + extra = {} + tenant_vif = self.port.internal_info['tenant_vif_port_id'] + kwargs = { + 'internal_info': { 'tenant_vif_port_id': uuidutils.generate_uuid()}} - self.port.internal_info = { - 'tenant_vif_port_id': self.port.extra['vif_port_id']} - self.port.extra = {} - else: - kwargs = {'extra': {'vif_port_id': uuidutils.generate_uuid()}} + self.port.internal_info = { + 'tenant_vif_port_id': tenant_vif} + self.port.extra = {} second_port = utils.create_test_port( self.context, node_id=self.node.id, address='52:54:00:cf:2d:33', uuid=uuidutils.generate_uuid(), @@ -669,7 +676,6 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): client_ids = (CLIENT_ID1, CLIENT_ID2) ports = (self.port, second_port) for port, client_id in zip(ports, client_ids): - extra = port.extra extra['client-id'] = client_id port.extra = extra port.save() @@ -694,31 +700,19 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.configure_tenant_networks(task) client_mock.assert_called_once_with(context=task.context) - if vif_int_info: - portid1 = self.port.internal_info['tenant_vif_port_id'] - portid2 = second_port.internal_info['tenant_vif_port_id'] - else: - portid1 = self.port.extra['vif_port_id'] - portid2 = second_port.extra['vif_port_id'] + portid1 = self.port.internal_info['tenant_vif_port_id'] + portid2 = second_port.internal_info['tenant_vif_port_id'] update_mock.assert_has_calls( [mock.call(self.context, portid1, port1_attrs), mock.call(self.context, portid2, port2_attrs)], any_order=True ) - def test_configure_tenant_networks_vif_extra(self): + def test_configure_tenant_networks(self): self.node.instance_uuid = uuidutils.generate_uuid() self.node.save() self._test_configure_tenant_networks() - def test_configure_tenant_networks_vif_int_info(self): - self.node.instance_uuid = uuidutils.generate_uuid() - self.node.save() - self._test_configure_tenant_networks(vif_int_info=True) - - def test_configure_tenant_networks_no_instance_uuid(self): - self._test_configure_tenant_networks() - def test_configure_tenant_networks_with_client_id(self): self.node.instance_uuid = uuidutils.generate_uuid() self.node.save() @@ -735,7 +729,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): port_data_mock): pg = utils.create_test_portgroup( self.context, node_id=self.node.id, address='ff:54:00:cf:2d:32', - extra={'vif_port_id': uuidutils.generate_uuid()}) + internal_info={'tenant_vif_port_id': uuidutils.generate_uuid()}) port1 = utils.create_test_port( self.context, node_id=self.node.id, address='ff:54:00:cf:2d:33', uuid=uuidutils.generate_uuid(), @@ -777,9 +771,11 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): client_mock.assert_called_once_with(context=task.context) glgi_mock.assert_called_once_with(task, pg) update_mock.assert_has_calls( - [mock.call(self.context, self.port.extra['vif_port_id'], + [mock.call(self.context, + self.port.internal_info['tenant_vif_port_id'], call1_attrs), - mock.call(self.context, pg.extra['vif_port_id'], + mock.call(self.context, + pg.internal_info['tenant_vif_port_id'], call2_attrs)] ) @@ -794,7 +790,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): port_data_mock): pg = utils.create_test_portgroup( self.context, node_id=self.node.id, address=None, - extra={'vif_port_id': uuidutils.generate_uuid()}) + internal_info={'tenant_vif_port_id': uuidutils.generate_uuid()}) port1 = utils.create_test_port( self.context, node_id=self.node.id, address='ff:54:00:cf:2d:33', uuid=uuidutils.generate_uuid(), @@ -835,9 +831,11 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): client_mock.assert_called_once_with(context=task.context) glgi_mock.assert_called_once_with(task, pg) update_mock.assert_has_calls( - [mock.call(self.context, self.port.extra['vif_port_id'], + [mock.call(self.context, + self.port.internal_info['tenant_vif_port_id'], call1_attrs), - mock.call(self.context, pg.extra['vif_port_id'], + mock.call(self.context, + pg.internal_info['tenant_vif_port_id'], call2_attrs)] ) diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 03eba20114..f35ce15bc5 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -875,16 +875,6 @@ class GetSingleNicTestCase(db_base.DbTestCase): address = utils.get_single_nic_with_vif_port_id(task) self.assertEqual('aa:bb:cc:dd:ee:ff', address) - def test_get_single_nic_with_vif_port_id_extra(self): - obj_utils.create_test_port( - self.context, node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', - uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': 'test-vif-A'}) - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - address = utils.get_single_nic_with_vif_port_id(task) - self.assertEqual('aa:bb:cc:dd:ee:ff', address) - def test_get_single_nic_with_cleaning_vif_port_id(self): obj_utils.create_test_port( self.context, node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', diff --git a/releasenotes/notes/remove-extra-vif-port-id-ea4e59176c2065fd.yaml b/releasenotes/notes/remove-extra-vif-port-id-ea4e59176c2065fd.yaml new file mode 100644 index 0000000000..c16d57a5cd --- /dev/null +++ b/releasenotes/notes/remove-extra-vif-port-id-ea4e59176c2065fd.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + The functionality of using a port.extra ``vif_port_id`` + value to signal and control a VIF attachment has been removed + to support changing the permission model and access control policy. + Use of ``vif_port_id`` outside of the VIF attachment/detachment workflow + has been deprecated since the Ocata development cycle.