diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index f0f6919696..9969ced8ce 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1729,6 +1729,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) diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 2e48dcd754..af65c183dd 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -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) diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 8b47b36792..c0fc095a71 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -231,7 +231,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.66', + 'api': '1.67', 'rpc': '1.50', 'objects': { 'Allocation': ['1.1'], diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py index 736249b69c..fcd07bdff8 100644 --- a/ironic/drivers/modules/network/common.py +++ b/ironic/drivers/modules/network/common.py @@ -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) @@ -552,7 +568,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: diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 99f3cf34f4..e6c59c060f 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -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): diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py index d2d4b6bb3d..7c0cde9d1a 100644 --- a/ironic/tests/unit/drivers/modules/network/test_common.py +++ b/ironic/tests/unit/drivers/modules/network/test_common.py @@ -207,6 +207,59 @@ class TestCommonFunctions(db_base.DbTestCase): self.assertItemsEqual( [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): @@ -712,7 +765,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') @@ -728,7 +782,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') @@ -751,7 +806,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') @@ -773,7 +829,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) @@ -799,7 +856,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) @@ -819,7 +877,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") diff --git a/releasenotes/notes/vif-port-attach-17a9993bf5c21d69.yaml b/releasenotes/notes/vif-port-attach-17a9993bf5c21d69.yaml new file mode 100644 index 0000000000..f5b791c0c1 --- /dev/null +++ b/releasenotes/notes/vif-port-attach-17a9993bf5c21d69.yaml @@ -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.