From 346756097714b93d389c52a2c280cbe766983ed8 Mon Sep 17 00:00:00 2001 From: Salman Hajizada Date: Wed, 12 Jun 2024 22:30:24 +0300 Subject: [PATCH] baremetal: Enhance VIF attachment with port and portgroup UUIDs This change extends the `attach_vif` and `attach_vif_to_node` methods to accept optional parameters for VIF port UUID and VIF portgroup UUID. This enhancement allows for more flexible VIF attachment scenarios while ensuring that only one of these parameters can be set at a time. - Added parameters `vif_port_uuid` and `vif_portgroup_uuid` to the `attach_vif` method in the `Node` class. - Updated the `attach_vif_to_node` method in the `Proxy` class to pass these parameters to the `attach_vif` method. - Included a check to ensure only one of `vif_port_uuid` and `vif_portgroup_uuid` can be set at a time in both methods. - Modified the request body in the `attach_vif` method to include these parameters if provided. Change-Id: I4b8487b45ae04f387c2f02a9505916072edc96aa --- openstack/baremetal/v1/_common.py | 3 + openstack/baremetal/v1/_proxy.py | 32 ++++++-- openstack/baremetal/v1/node.py | 37 +++++++++- .../tests/unit/baremetal/v1/test_node.py | 74 +++++++++++++++++-- ...-vif-optional-params-abb755b74f076eb2.yaml | 6 ++ 5 files changed, 137 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/add-vif-optional-params-abb755b74f076eb2.yaml diff --git a/openstack/baremetal/v1/_common.py b/openstack/baremetal/v1/_common.py index 0ee266d14..3eb3c4334 100644 --- a/openstack/baremetal/v1/_common.py +++ b/openstack/baremetal/v1/_common.py @@ -70,6 +70,9 @@ STATE_VERSIONS = { VIF_VERSION = '1.28' """API version in which the VIF operations were introduced.""" +VIF_OPTIONAL_PARAMS_VERSION = '1.67' +"""API version in which the VIF optional parameters were introduced.""" + INJECT_NMI_VERSION = '1.29' """API vresion in which support for injecting NMI was introduced.""" diff --git a/openstack/baremetal/v1/_proxy.py b/openstack/baremetal/v1/_proxy.py index 71699fb5d..3607a9263 100644 --- a/openstack/baremetal/v1/_proxy.py +++ b/openstack/baremetal/v1/_proxy.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +import typing as ty + from openstack.baremetal.v1 import _common from openstack.baremetal.v1 import allocation as _allocation from openstack.baremetal.v1 import chassis as _chassis @@ -64,7 +66,7 @@ class Proxy(proxy.Proxy): error_message="No {resource_type} found for {value}".format( resource_type=resource_type.__name__, value=value ), - **kwargs + **kwargs, ) def chassis(self, details=False, **query): @@ -964,7 +966,15 @@ class Proxy(proxy.Proxy): _portgroup.PortGroup, port_group, ignore_missing=ignore_missing ) - def attach_vif_to_node(self, node, vif_id, retry_on_conflict=True): + def attach_vif_to_node( + self, + node: ty.Union[_node.Node, str], + vif_id: str, + retry_on_conflict: bool = True, + *, + port_id: ty.Optional[str] = None, + port_group_id: ty.Optional[str] = None, + ) -> None: """Attach a VIF to the node. The exact form of the VIF ID depends on the network interface used by @@ -974,17 +984,29 @@ class Proxy(proxy.Proxy): :param node: The value can be either the name or ID of a node or a :class:`~openstack.baremetal.v1.node.Node` instance. - :param string vif_id: Backend-specific VIF ID. + :param vif_id: Backend-specific VIF ID. :param retry_on_conflict: Whether to retry HTTP CONFLICT errors. This can happen when either the VIF is already used on a node or the node is locked. Since the latter happens more often, the default value is True. - :return: ``None`` + :param port_id: The UUID of the port to attach the VIF to. Only one of + port_id or port_group_id can be provided. + :param port_group_id: The UUID of the portgroup to attach to. Only one + of port_group_id or port_id can be provided. + :return: None :raises: :exc:`~openstack.exceptions.NotSupported` if the server does not support the VIF API. + :raises: :exc:`~openstack.exceptions.InvalidRequest` if both port_id + and port_group_id are provided. """ res = self._get_resource(_node.Node, node) - res.attach_vif(self, vif_id, retry_on_conflict=retry_on_conflict) + res.attach_vif( + self, + vif_id=vif_id, + retry_on_conflict=retry_on_conflict, + port_id=port_id, + port_group_id=port_group_id, + ) def detach_vif_from_node(self, node, vif_id, ignore_missing=True): """Detach a VIF from the node. diff --git a/openstack/baremetal/v1/node.py b/openstack/baremetal/v1/node.py index 2832c93c0..1281f79d4 100644 --- a/openstack/baremetal/v1/node.py +++ b/openstack/baremetal/v1/node.py @@ -12,6 +12,7 @@ import collections import enum +import typing as ty from openstack.baremetal.v1 import _common from openstack import exceptions @@ -790,7 +791,15 @@ class Node(_common.Resource): if wait: self.wait_for_power_state(session, expected, timeout=timeout) - def attach_vif(self, session, vif_id, retry_on_conflict=True): + def attach_vif( + self, + session, + vif_id: str, + retry_on_conflict: bool = True, + *, + port_id: ty.Optional[str] = None, + port_group_id: ty.Optional[str] = None, + ) -> None: """Attach a VIF to the node. The exact form of the VIF ID depends on the network interface used by @@ -800,26 +809,46 @@ class Node(_common.Resource): :param session: The session to use for making this request. :type session: :class:`~keystoneauth1.adapter.Adapter` - :param string vif_id: Backend-specific VIF ID. + :param vif_id: Backend-specific VIF ID. :param retry_on_conflict: Whether to retry HTTP CONFLICT errors. This can happen when either the VIF is already used on a node or the node is locked. Since the latter happens more often, the default value is True. - :return: ``None`` + :param port_id: The UUID of the port to attach the VIF to. Only one of + port_id or port_group_id can be provided. + :param port_group_id: The UUID of the portgroup to attach to. Only one + of port_group_id or port_id can be provided. + :return: None :raises: :exc:`~openstack.exceptions.NotSupported` if the server does not support the VIF API. + :raises: :exc:`~openstack.exceptions.InvalidRequest` if both port_id + and port_group_id are provided. """ + if port_id and port_group_id: + msg = ( + 'Only one of vif_port_id and vif_portgroup_id can be provided' + ) + raise exceptions.InvalidRequest(msg) + session = self._get_session(session) + if port_id or port_group_id: + required_version = _common.VIF_OPTIONAL_PARAMS_VERSION + else: + required_version = _common.VIF_VERSION version = self._assert_microversion_for( session, 'commit', - _common.VIF_VERSION, + required_version, error_message=("Cannot use VIF attachment API"), ) request = self._prepare_request(requires_id=True) request.url = utils.urljoin(request.url, 'vifs') body = {'id': vif_id} + if port_id: + body['port_uuid'] = port_id + elif port_group_id: + body['portgroup_uuid'] = port_group_id retriable_status_codes = _common.RETRIABLE_STATUS_CODES if not retry_on_conflict: retriable_status_codes = list(set(retriable_status_codes) - {409}) diff --git a/openstack/tests/unit/baremetal/v1/test_node.py b/openstack/tests/unit/baremetal/v1/test_node.py index 891dafd7e..5e8af118e 100644 --- a/openstack/tests/unit/baremetal/v1/test_node.py +++ b/openstack/tests/unit/baremetal/v1/test_node.py @@ -558,12 +558,14 @@ class TestNodeVif(base.TestCase): def setUp(self): super().setUp() self.session = mock.Mock(spec=adapter.Adapter) - self.session.default_microversion = '1.28' + self.session.default_microversion = '1.67' self.session.log = mock.Mock() self.node = node.Node( id='c29db401-b6a7-4530-af8e-20a720dee946', driver=FAKE['driver'] ) self.vif_id = '714bdf6d-2386-4b5e-bd0d-bc036f04b1ef' + self.vif_port_uuid = 'port-uuid' + self.vif_portgroup_uuid = 'portgroup-uuid' def test_attach_vif(self): self.assertIsNone(self.node.attach_vif(self.session, self.vif_id)) @@ -571,7 +573,7 @@ class TestNodeVif(base.TestCase): 'nodes/%s/vifs' % self.node.id, json={'id': self.vif_id}, headers=mock.ANY, - microversion='1.28', + microversion='1.67', retriable_status_codes=[409, 503], ) @@ -585,16 +587,59 @@ class TestNodeVif(base.TestCase): 'nodes/%s/vifs' % self.node.id, json={'id': self.vif_id}, headers=mock.ANY, - microversion='1.28', + microversion='1.67', retriable_status_codes=[503], ) + def test_attach_vif_with_port_uuid(self): + self.assertIsNone( + self.node.attach_vif( + self.session, self.vif_id, port_id=self.vif_port_uuid + ) + ) + self.session.post.assert_called_once_with( + 'nodes/%s/vifs' % self.node.id, + json={'id': self.vif_id, 'port_uuid': self.vif_port_uuid}, + headers=mock.ANY, + microversion='1.67', + retriable_status_codes=[409, 503], + ) + + def test_attach_vif_with_portgroup_uuid(self): + self.assertIsNone( + self.node.attach_vif( + self.session, + self.vif_id, + port_group_id=self.vif_portgroup_uuid, + ) + ) + self.session.post.assert_called_once_with( + 'nodes/%s/vifs' % self.node.id, + json={ + 'id': self.vif_id, + 'portgroup_uuid': self.vif_portgroup_uuid, + }, + headers=mock.ANY, + microversion='1.67', + retriable_status_codes=[409, 503], + ) + + def test_attach_vif_with_port_uuid_and_portgroup_uuid(self): + self.assertRaises( + exceptions.InvalidRequest, + self.node.attach_vif, + self.session, + self.vif_id, + port_id=self.vif_port_uuid, + port_group_id=self.vif_portgroup_uuid, + ) + def test_detach_vif_existing(self): self.assertTrue(self.node.detach_vif(self.session, self.vif_id)) self.session.delete.assert_called_once_with( f'nodes/{self.node.id}/vifs/{self.vif_id}', headers=mock.ANY, - microversion='1.28', + microversion='1.67', retriable_status_codes=_common.RETRIABLE_STATUS_CODES, ) @@ -604,7 +649,7 @@ class TestNodeVif(base.TestCase): self.session.delete.assert_called_once_with( f'nodes/{self.node.id}/vifs/{self.vif_id}', headers=mock.ANY, - microversion='1.28', + microversion='1.67', retriable_status_codes=_common.RETRIABLE_STATUS_CODES, ) @@ -620,7 +665,7 @@ class TestNodeVif(base.TestCase): self.session.get.assert_called_once_with( 'nodes/%s/vifs' % self.node.id, headers=mock.ANY, - microversion='1.28', + microversion='1.67', ) def test_incompatible_microversion(self): @@ -641,6 +686,23 @@ class TestNodeVif(base.TestCase): exceptions.NotSupported, self.node.list_vifs, self.session ) + def test_incompatible_microversion_optional_params(self): + self.session.default_microversion = '1.28' + self.assertRaises( + exceptions.NotSupported, + self.node.attach_vif, + self.session, + self.vif_id, + port_id=self.vif_port_uuid, + ) + self.assertRaises( + exceptions.NotSupported, + self.node.attach_vif, + self.session, + self.vif_id, + port_group_id=self.vif_portgroup_uuid, + ) + @mock.patch.object(exceptions, 'raise_from_response', mock.Mock()) @mock.patch.object(node.Node, '_get_session', lambda self, x: x) diff --git a/releasenotes/notes/add-vif-optional-params-abb755b74f076eb2.yaml b/releasenotes/notes/add-vif-optional-params-abb755b74f076eb2.yaml new file mode 100644 index 000000000..924436dea --- /dev/null +++ b/releasenotes/notes/add-vif-optional-params-abb755b74f076eb2.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Extend the ``attach_vif`` and ``attach_vif_to_node`` methods of the + baremetal proxy to to accept optional parameters for VIF port UUID and + VIF portgroup UUID.