Merge "baremetal: Enhance VIF attachment with port and portgroup UUIDs"

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."""
 

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.

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})

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)

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.