Merge "Allow node vif attach to specify port_uuid or portgroup_uuid"

This commit is contained in:
Zuul 2020-06-23 10:29:37 +00:00 committed by Gerrit Code Review
commit 305532e259
7 changed files with 161 additions and 26 deletions

View File

@ -1728,6 +1728,10 @@ class NodeVIFController(rest.RestController):
for that VIF.
"""
rpc_node, topic = self._get_node_and_topic('baremetal:node:vif:attach')
if api.request.version.minor >= versions.MINOR_67_NODE_VIF_ATTACH_PORT:
if 'port_uuid' in vif and 'portgroup_uuid' in vif:
msg = _("Cannot specify both port_uuid and portgroup_uuid")
raise exception.Invalid(msg)
api.request.rpcapi.vif_attach(api.request.context, rpc_node.uuid,
vif_info=vif, topic=topic)

View File

@ -104,6 +104,7 @@ BASE_VERSION = 1
# v1.64: Add network_type to port.local_link_connection
# v1.65: Add lessee to the node object.
# v1.66: Add support for node network_data field.
# v1.67: Add support for port_uuid/portgroup_uuid in node vif_attach
MINOR_0_JUNO = 0
MINOR_1_INITIAL_VERSION = 1
@ -172,6 +173,7 @@ MINOR_63_INDICATORS = 63
MINOR_64_LOCAL_LINK_CONNECTION_NETWORK_TYPE = 64
MINOR_65_NODE_LESSEE = 65
MINOR_66_NODE_NETWORK_DATA = 66
MINOR_67_NODE_VIF_ATTACH_PORT = 67
# When adding another version, update:
# - MINOR_MAX_VERSION
@ -179,7 +181,7 @@ MINOR_66_NODE_NETWORK_DATA = 66
# explanation of what changed in the new version
# - common/release_mappings.py, RELEASE_MAPPING['master']['api']
MINOR_MAX_VERSION = MINOR_66_NODE_NETWORK_DATA
MINOR_MAX_VERSION = MINOR_67_NODE_VIF_ATTACH_PORT
# String representations of the minor and maximum versions
_MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)

View File

@ -231,7 +231,7 @@ RELEASE_MAPPING = {
}
},
'master': {
'api': '1.66',
'api': '1.67',
'rpc': '1.50',
'objects': {
'Allocation': ['1.1'],

View File

@ -83,7 +83,7 @@ def _is_port_physnet_allowed(port, physnets):
or port.physical_network in physnets)
def _get_free_portgroups_and_ports(task, vif_id, physnets):
def _get_free_portgroups_and_ports(task, vif_id, physnets, vif_info={}):
"""Get free portgroups and ports.
It only returns ports or portgroups that can be used for attachment of
@ -95,6 +95,8 @@ def _get_free_portgroups_and_ports(task, vif_id, physnets):
attached. This is governed by the segments of the VIF's network. An
empty set indicates that the ports' physical networks should be
ignored.
:param vif_info: dict that may contain extra information, such as
port_uuid
:returns: list of free ports and portgroups.
:raises: VifAlreadyAttached, if vif_id is attached to any of the
node's ports or portgroups.
@ -109,9 +111,18 @@ def _get_free_portgroups_and_ports(task, vif_id, physnets):
# at least one port with vif already attached to it
non_usable_portgroups = set()
port_uuid = None
portgroup_uuid = None
if 'port_uuid' in vif_info:
port_uuid = vif_info['port_uuid']
elif 'portgroup_uuid' in vif_info:
portgroup_uuid = vif_info['portgroup_uuid']
for p in task.ports:
# If port_uuid is specified in vif_info, check id
# Validate that port has needed information
if not neutron.validate_port_info(task.node, p):
if ((port_uuid and port_uuid != p.uuid)
or not neutron.validate_port_info(task.node, p)):
continue
if _vif_attached(p, vif_id):
# Consider such portgroup unusable. The fact that we can have None
@ -120,27 +131,30 @@ def _get_free_portgroups_and_ports(task, vif_id, physnets):
continue
if not _is_port_physnet_allowed(p, physnets):
continue
if p.portgroup_id is None:
# ports without portgroup_id are always considered candidates
if p.portgroup_id is None and not portgroup_uuid:
free_port_like_objs.append(p)
else:
ports_by_portgroup[p.portgroup_id].append(p)
for pg in task.portgroups:
if _vif_attached(pg, vif_id):
continue
if pg.id in non_usable_portgroups:
# This portgroup has vifs attached to its ports, consider its
# ports instead to avoid collisions
free_port_like_objs.extend(ports_by_portgroup[pg.id])
# Also ignore empty portgroups
elif ports_by_portgroup[pg.id]:
free_port_like_objs.append(pg)
if not port_uuid:
for pg in task.portgroups:
# if portgroup_uuid is specified in vif_info, check id
if ((portgroup_uuid and portgroup_uuid != pg.uuid)
or _vif_attached(pg, vif_id)):
continue
if pg.id in non_usable_portgroups:
# This portgroup has vifs attached to its ports, consider its
# ports instead to avoid collisions
if not portgroup_uuid:
free_port_like_objs.extend(ports_by_portgroup[pg.id])
# Also ignore empty portgroups
elif ports_by_portgroup[pg.id]:
free_port_like_objs.append(pg)
return free_port_like_objs
def get_free_port_like_object(task, vif_id, physnets):
def get_free_port_like_object(task, vif_id, physnets, vif_info={}):
"""Find free port-like object (portgroup or port) VIF will be attached to.
Ensures that the VIF is not already attached to this node. When selecting
@ -160,6 +174,8 @@ def get_free_port_like_object(task, vif_id, physnets):
attached. This is governed by the segments of the VIF's network. An
empty set indicates that the ports' physical networks should be
ignored.
:param vif_info: dict that may contain extra information, such as
port_uuid
:raises: VifAlreadyAttached, if VIF is already attached to the node.
:raises: NoFreePhysicalPorts, if there is no port-like object VIF can be
attached to.
@ -167,8 +183,8 @@ def get_free_port_like_object(task, vif_id, physnets):
has ports which are not all assigned the same physical network.
:returns: port-like object VIF will be attached to.
"""
free_port_like_objs = _get_free_portgroups_and_ports(task, vif_id,
physnets)
free_port_like_objs = _get_free_portgroups_and_ports(
task, vif_id, physnets, vif_info)
if not free_port_like_objs:
raise exception.NoFreePhysicalPorts(vif=vif_id)
@ -576,7 +592,8 @@ class NeutronVIFPortIDMixin(VIFPortIDMixin):
raise exception.VifInvalidForAttach(
node=task.node.uuid, vif=vif_id, reason=reason)
port_like_obj = get_free_port_like_object(task, vif_id, physnets)
port_like_obj = get_free_port_like_object(
task, vif_id, physnets, vif_info)
# Address is optional for portgroups
if port_like_obj.address:

View File

@ -6176,6 +6176,51 @@ class TestAttachDetachVif(test_api_base.BaseApiTest):
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_attach')
def test_vif_attach_port_uuid_and_portgroup_uuid(self, mock_attach,
mock_get):
vif_id = uuidutils.generate_uuid()
request_body = {
'id': vif_id,
'port_uuid': 'port-uuid',
'portgroup_uuid': 'portgroup-uuid'
}
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:
"1.67"})
self.assertEqual(http_client.BAD_REQUEST, ret.status_int)
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_port_uuid_and_portgroup_uuid_old(self, mock_attach,
mock_get):
vif_id = uuidutils.generate_uuid()
request_body = {
'id': vif_id,
'port_uuid': 'port-uuid',
'portgroup_uuid': 'portgroup-uuid'
}
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_uuid')
@mock.patch.object(rpcapi.ConductorAPI, 'vif_detach')
def test_vif_detach(self, mock_detach, mock_get):

View File

@ -209,6 +209,59 @@ class TestCommonFunctions(db_base.DbTestCase):
self.assertCountEqual(
[self.port.uuid], [p.uuid for p in free_port_like_objs])
def test__get_free_portgroups_and_ports_port_uuid(self):
self.node.network_interface = 'flat'
self.node.save()
pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
set_physnets=False)
with task_manager.acquire(self.context, self.node.id) as task:
free_port_like_objs = (
common._get_free_portgroups_and_ports(
task, self.vif_id, {}, {'port_uuid': self.port.uuid}))
self.assertItemsEqual(
[self.port.uuid],
[p.uuid for p in free_port_like_objs])
def test__get_free_portgroups_and_ports_portgroup_uuid(self):
self.node.network_interface = 'flat'
self.node.save()
pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
set_physnets=False)
with task_manager.acquire(self.context, self.node.id) as task:
free_port_like_objs = (
common._get_free_portgroups_and_ports(
task, self.vif_id, {}, {'portgroup_uuid': pg1.uuid}))
self.assertItemsEqual(
[pg1.uuid],
[p.uuid for p in free_port_like_objs])
def test__get_free_portgroups_and_ports_portgroup_uuid_attached_vifs(self):
self.node.network_interface = 'flat'
self.node.save()
pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
set_physnets=False)
with task_manager.acquire(self.context, self.node.id) as task:
free_port_like_objs = (
common._get_free_portgroups_and_ports(
task, self.vif_id, {}, {'portgroup_uuid': pg2.uuid}))
self.assertItemsEqual(
[],
[p.uuid for p in free_port_like_objs])
def test__get_free_portgroups_and_ports_no_matching_uuid(self):
self.node.network_interface = 'flat'
self.node.save()
pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
set_physnets=False)
with task_manager.acquire(self.context, self.node.id) as task:
free_port_like_objs = (
common._get_free_portgroups_and_ports(
task, self.vif_id, {},
{'port_uuid': uuidutils.generate_uuid()}))
self.assertItemsEqual(
[],
[p.uuid for p in free_port_like_objs])
@mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
return_value=True)
def test_get_free_port_like_object_ports(self, vpi_mock):
@ -726,7 +779,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
mock_upa.assert_called_once_with(
"fake_vif_id", self.port.address, context=task.context)
self.assertFalse(mock_gpbpi.called)
mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
mock_gfp.assert_called_once_with(task, 'fake_vif_id', set(),
{'id': 'fake_vif_id'})
mock_save.assert_called_once_with(self.port, "fake_vif_id")
@mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj')
@ -742,7 +796,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
with task_manager.acquire(self.context, self.node.id) as task:
self.assertRaises(exception.NoFreePhysicalPorts,
self.interface.vif_attach, task, vif)
mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
mock_gfp.assert_called_once_with(task, 'fake_vif_id', set(),
{'id': 'fake_vif_id'})
self.assertFalse(mock_save.called)
@mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj')
@ -765,7 +820,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
"fake_vif_id", self.port.address, context=task.context)
mock_gpbpi.assert_called_once_with(mock_client.return_value,
'fake_vif_id')
mock_gfp.assert_called_once_with(task, 'fake_vif_id', {'physnet1'})
mock_gfp.assert_called_once_with(task, 'fake_vif_id', {'physnet1'},
{'id': 'fake_vif_id'})
mock_save.assert_called_once_with(self.port, "fake_vif_id")
@mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj')
@ -787,7 +843,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
mock_upa.assert_called_once_with(
"fake_vif_id", self.port.address, context=task.context)
self.assertFalse(mock_gpbpi.called)
mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
mock_gfp.assert_called_once_with(task, 'fake_vif_id', set(),
{'id': 'fake_vif_id'})
mock_save.assert_called_once_with(self.port, "fake_vif_id")
mock_plug.assert_called_once_with(task, self.port, mock.ANY)
@ -813,7 +870,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
mock_upa.assert_called_once_with(
"fake_vif_id", self.port.address, context=task.context)
self.assertFalse(mock_gpbpi.called)
mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
mock_gfp.assert_called_once_with(task, 'fake_vif_id', set(),
{'id': 'fake_vif_id'})
mock_save.assert_called_once_with(self.port, "fake_vif_id")
mock_plug.assert_called_once_with(task, self.port, mock.ANY)
@ -833,7 +891,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase):
self.interface.vif_attach(task, vif)
mock_client.assert_called_once_with(context=task.context)
self.assertFalse(mock_gpbpi.called)
mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
mock_gfp.assert_called_once_with(task, 'fake_vif_id', set(),
{'id': 'fake_vif_id'})
self.assertFalse(mock_client.return_value.show_port.called)
self.assertFalse(mock_upa.called)
mock_save.assert_called_once_with(pg, "fake_vif_id")

View File

@ -0,0 +1,8 @@
---
features:
- |
Adds the ability for Ironic to attach a node to a specific port or portgroup.
This is accomplished by having the node vif_attach API accept a port_uuid or
portgroup_uuid key within vif_info. If one is specified, then Ironic will
attempt to attach to the specified port/portgroup. Specifying both returns
an error.