Integrate portgroups with ports to support LAG

This patch adds portgroups subcontroller. The API version
has been bumped to 1.24. New endpoints were added:

  * '/v1/nodes/<node>/portgroups'
  * '/v1/portgroups/<pg>/ports'

Starting with this API version, ports have a new 'portgroup_uuid'
field that contains UUID of a portgroup this port belongs to.

Partial-bug: #1618754

DocImpact
Co-Authored-By: Jenny Moorehead <jenny.moorehead@sap.com>
Co-Authored-By: Will Stevenson <will.stevenson@sap.com>
Co-Authored-By: Vasyl Saienko <vsaienko@mirantis.com>
Co-Authored-By: Vladyslav Drok <vdrok@mirantis.com>
Co-Authored-By: Zhenguo Niu <Niu.ZGlinux@gmail.com>
Co-Authored-By: Michael Turek <mjturek@linux.vnet.ibm.com>

Change-Id: I597ae1a3a969ee9fb4df57e444c606c77c5c093c

doc/source/dev/webapi-version-history.rst

@@ -2,6 +2,11 @@
 REST API Version History
 ========================
 
+**1.24**
+
+    Added new endpoints '/v1/nodes/<node>/portgroups' and '/v1/portgroups/<portgroup>/ports'.
+    Added new field ``port.portgroup_uuid``.
+
 **1.23**
 
     Added '/v1/portgroups/ endpoint.

ironic/api/controllers/v1/node.py

@@ -30,6 +30,7 @@ from ironic.api.controllers import base
 from ironic.api.controllers import link
 from ironic.api.controllers.v1 import collection
 from ironic.api.controllers.v1 import port
+from ironic.api.controllers.v1 import portgroup
 from ironic.api.controllers.v1 import types
 from ironic.api.controllers.v1 import utils as api_utils
 from ironic.api.controllers.v1 import versions
@@ -746,6 +747,9 @@ class Node(base.APIBase):
     ports = wsme.wsattr([link.Link], readonly=True)
     """Links to the collection of ports on this node"""
 
+    portgroups = wsme.wsattr([link.Link], readonly=True)
+    """Links to the collection of portgroups on this node"""
+
     states = wsme.wsattr([link.Link], readonly=True)
     """Links to endpoint for retrieving and setting node states"""
 
@@ -775,7 +779,8 @@ class Node(base.APIBase):
         setattr(self, 'chassis_uuid', kwargs.get('chassis_id', wtypes.Unset))
 
     @staticmethod
-    def _convert_with_links(node, url, fields=None, show_states_links=True):
+    def _convert_with_links(node, url, fields=None, show_states_links=True,
+                            show_portgroups=True):
         # NOTE(lucasagomes): Since we are able to return a specified set of
         # fields the "uuid" can be unset, so we need to save it in another
         # variable to use when building the links
@@ -795,6 +800,13 @@ class Node(base.APIBase):
                                link.Link.make_link('bookmark', url, 'nodes',
                                                    node_uuid + "/states",
                                                    bookmark=True)]
+            if show_portgroups:
+                node.portgroups = [
+                    link.Link.make_link('self', url, 'nodes',
+                                        node_uuid + "/portgroups"),
+                    link.Link.make_link('bookmark', url, 'nodes',
+                                        node_uuid + "/portgroups",
+                                        bookmark=True)]
 
         # NOTE(lucasagomes): The numeric ID should not be exposed to
         #                    the user, it's internal only.
@@ -841,9 +853,11 @@ class Node(base.APIBase):
         hide_fields_in_newer_versions(node)
         show_states_links = (
             api_utils.allow_links_node_states_and_driver_properties())
+        show_portgroups = api_utils.allow_portgroups_subcontrollers()
         return cls._convert_with_links(node, pecan.request.public_url,
                                        fields=fields,
-                                       show_states_links=show_states_links)
+                                       show_states_links=show_states_links,
+                                       show_portgroups=show_portgroups)
 
     @classmethod
     def sample(cls, expand=True):
@@ -1059,7 +1073,8 @@ class NodesController(rest.RestController):
                              'clean_step', 'raid_config', 'target_raid_config']
 
     _subcontroller_map = {
-        'ports': port.PortsController
+        'ports': port.PortsController,
+        'portgroups': portgroup.PortgroupsController,
     }
 
     @pecan.expose()
@@ -1071,6 +1086,9 @@ class NodesController(rest.RestController):
         if remainder:
             subcontroller = self._subcontroller_map.get(remainder[0])
             if subcontroller:
+                if (remainder[0] == 'portgroups' and
+                        not api_utils.allow_portgroups_subcontrollers()):
+                    pecan.abort(http_client.NOT_FOUND)
                 return subcontroller(node_ident=ident), remainder[1:]
 
     def _get_nodes_collection(self, chassis_uuid, instance_uuid, associated,

ironic/api/controllers/v1/port.py

@@ -49,6 +49,9 @@ def hide_fields_in_newer_versions(obj):
     if not api_utils.allow_port_advanced_net_fields():
         obj.pxe_enabled = wsme.Unset
         obj.local_link_connection = wsme.Unset
+    # if requested version is < 1.24, hide portgroup_uuid field
+    if not api_utils.allow_portgroups_subcontrollers():
+        obj.portgroup_uuid = wsme.Unset
 
 
 class Port(base.APIBase):
@@ -59,6 +62,7 @@ class Port(base.APIBase):
     """
 
     _node_uuid = None
+    _portgroup_uuid = None
 
     def _get_node_uuid(self):
         return self._node_uuid
@@ -83,6 +87,36 @@ class Port(base.APIBase):
         elif value == wtypes.Unset:
             self._node_uuid = wtypes.Unset
 
+    def _get_portgroup_uuid(self):
+        return self._portgroup_uuid
+
+    def _set_portgroup_uuid(self, value):
+        if value and self._portgroup_uuid != value:
+            if not api_utils.allow_portgroups_subcontrollers():
+                self._portgroup_uuid = wtypes.Unset
+                return
+            try:
+                portgroup = objects.Portgroup.get(pecan.request.context, value)
+                if portgroup.node_id != self.node_id:
+                    raise exception.BadRequest(_('Port can not be added to a '
+                                                 'portgroup belonging to a '
+                                                 'different node.'))
+                self._portgroup_uuid = portgroup.uuid
+                # NOTE(lucasagomes): Create the portgroup_id attribute
+                #                    on-the-fly to satisfy the api ->
+                #                    rpc object conversion.
+                self.portgroup_id = portgroup.id
+            except exception.PortgroupNotFound as e:
+                # Change error code because 404 (NotFound) is inappropriate
+                # response for a POST request to create a Port
+                e.code = http_client.BAD_REQUEST  # BadRequest
+                raise e
+        elif value == wtypes.Unset:
+            self._portgroup_uuid = wtypes.Unset
+        elif value is None and api_utils.allow_portgroups_subcontrollers():
+            # This is to output portgroup_uuid field if API version allows this
+            self._portgroup_uuid = None
+
     uuid = types.uuid
     """Unique UUID for this port"""
 
@@ -99,6 +133,10 @@ class Port(base.APIBase):
                                 mandatory=True)
     """The UUID of the node this port belongs to"""
 
+    portgroup_uuid = wsme.wsproperty(types.uuid, _get_portgroup_uuid,
+                                     _set_portgroup_uuid, mandatory=False)
+    """The UUID of the portgroup this port belongs to"""
+
     pxe_enabled = types.boolean
     """Indicates whether pxe is enabled or disabled on the node."""
 
@@ -114,6 +152,9 @@ class Port(base.APIBase):
         # NOTE(lucasagomes): node_uuid is not part of objects.Port.fields
         #                    because it's an API-only attribute
         fields.append('node_uuid')
+        # NOTE: portgroup_uuid is not part of objects.Port.fields
+        #                    because it's an API-only attribute
+        fields.append('portgroup_uuid')
         for field in fields:
             # Add fields we expose.
             if hasattr(self, field):
@@ -127,6 +168,14 @@ class Port(base.APIBase):
         self.fields.append('node_id')
         setattr(self, 'node_uuid', kwargs.get('node_id', wtypes.Unset))
 
+        # NOTE: portgroup_id is an attribute created on-the-fly
+        # by _set_portgroup_uuid(), it needs to be present in the fields so
+        # that as_dict() will contain portgroup_id field when converting it
+        # before saving it in the database.
+        self.fields.append('portgroup_id')
+        setattr(self, 'portgroup_uuid', kwargs.get('portgroup_id',
+                                                   wtypes.Unset))
+
     @staticmethod
     def _convert_with_links(port, url, fields=None):
         # NOTE(lucasagomes): Since we are able to return a specified set of
@@ -139,6 +188,9 @@ class Port(base.APIBase):
         # never expose the node_id attribute
         port.node_id = wtypes.Unset
 
+        # never expose the portgroup_id attribute
+        port.portgroup_id = wtypes.Unset
+
         port.links = [link.Link.make_link('self', url,
                                           'ports', port_uuid),
                       link.Link.make_link('bookmark', url,
@@ -174,6 +226,7 @@ class Port(base.APIBase):
         # NOTE(lucasagomes): node_uuid getter() method look at the
         # _node_uuid variable
         sample._node_uuid = '7ae81bb3-dec3-4289-8d6c-da80bd8001ae'
+        sample._portgroup_uuid = '037d9a52-af89-4560-b5a3-a33283295ba2'
         fields = None if expand else _DEFAULT_RETURN_FIELDS
         return cls._convert_with_links(sample, 'http://localhost:6385',
                                        fields=fields)
@@ -223,13 +276,14 @@ class PortsController(rest.RestController):
 
     advanced_net_fields = ['pxe_enabled', 'local_link_connection']
 
-    def __init__(self, node_ident=None):
+    def __init__(self, node_ident=None, portgroup_ident=None):
         super(PortsController, self).__init__()
         self.parent_node_ident = node_ident
+        self.parent_portgroup_ident = portgroup_ident
 
-    def _get_ports_collection(self, node_ident, address, marker, limit,
+    def _get_ports_collection(self, node_ident, address, portgroup_ident,
-                              sort_key, sort_dir, resource_url=None,
+                              marker, limit, sort_key, sort_dir,
-                              fields=None):
+                              resource_url=None, fields=None):
 
         limit = api_utils.validate_limit(limit)
         sort_dir = api_utils.validate_sort_dir(sort_dir)
@@ -245,7 +299,23 @@ class PortsController(rest.RestController):
                   "sorting") % {'key': sort_key})
 
         node_ident = self.parent_node_ident or node_ident
-        if node_ident:
+        portgroup_ident = self.parent_portgroup_ident or portgroup_ident
+
+        if node_ident and portgroup_ident:
+            raise exception.OperationNotPermitted()
+
+        if portgroup_ident:
+            # FIXME: Since all we need is the portgroup ID, we can
+            #                 make this more efficient by only querying
+            #                 for that column. This will get cleaned up
+            #                 as we move to the object interface.
+            portgroup = api_utils.get_rpc_portgroup(portgroup_ident)
+            ports = objects.Port.list_by_portgroup_id(pecan.request.context,
+                                                      portgroup.id, limit,
+                                                      marker_obj,
+                                                      sort_key=sort_key,
+                                                      sort_dir=sort_dir)
+        elif node_ident:
             # FIXME(comstud): Since all we need is the node ID, we can
             #                 make this more efficient by only querying
             #                 for that column. This will get cleaned up
@@ -285,9 +355,10 @@ class PortsController(rest.RestController):
     @METRICS.timer('PortsController.get_all')
     @expose.expose(PortCollection, types.uuid_or_name, types.uuid,
                    types.macaddress, types.uuid, int, wtypes.text,
-                   wtypes.text, types.listtype)
+                   wtypes.text, types.listtype, types.uuid_or_name)
     def get_all(self, node=None, node_uuid=None, address=None, marker=None,
-                limit=None, sort_key='id', sort_dir='asc', fields=None):
+                limit=None, sort_key='id', sort_dir='asc', fields=None,
+                portgroup=None):
         """Retrieve a list of ports.
 
         Note that the 'node_uuid' interface is deprecated in favour
@@ -308,14 +379,23 @@ class PortsController(rest.RestController):
         :param sort_dir: direction to sort. "asc" or "desc". Default: asc.
         :param fields: Optional, a list with a specified set of fields
             of the resource to be returned.
-        :raises: NotAcceptable
+        :param portgroup: UUID or name of a portgroup, to get only ports
+                                   for that portgroup.
+        :raises: NotAcceptable, HTTPNotFound
         """
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:port:get', cdict, cdict)
 
         api_utils.check_allow_specify_fields(fields)
-        if (fields and not api_utils.allow_port_advanced_net_fields() and
+        if fields:
-                set(fields).intersection(self.advanced_net_fields)):
+            if (not api_utils.allow_port_advanced_net_fields() and
+                    set(fields).intersection(self.advanced_net_fields)):
+                raise exception.NotAcceptable()
+            if ('portgroup_uuid' in fields and not
+                    api_utils.allow_portgroups_subcontrollers()):
+                    raise exception.NotAcceptable()
+
+        if portgroup and not api_utils.allow_portgroups_subcontrollers():
             raise exception.NotAcceptable()
 
         if fields is None:
@@ -329,16 +409,16 @@ class PortsController(rest.RestController):
                 not uuidutils.is_uuid_like(node)):
                 raise exception.NotAcceptable()
 
-        return self._get_ports_collection(node_uuid or node, address, marker,
+        return self._get_ports_collection(node_uuid or node, address,
-                                          limit, sort_key, sort_dir,
+                                          portgroup, marker, limit, sort_key,
-                                          fields=fields)
+                                          sort_dir, fields=fields)
 
     @METRICS.timer('PortsController.detail')
     @expose.expose(PortCollection, types.uuid_or_name, types.uuid,
                    types.macaddress, types.uuid, int, wtypes.text,
-                   wtypes.text)
+                   wtypes.text, types.uuid_or_name)
     def detail(self, node=None, node_uuid=None, address=None, marker=None,
-               limit=None, sort_key='id', sort_dir='asc'):
+               limit=None, sort_key='id', sort_dir='asc', portgroup=None):
         """Retrieve a list of ports with detail.
 
         Note that the 'node_uuid' interface is deprecated in favour
@@ -350,6 +430,8 @@ class PortsController(rest.RestController):
                           node.
         :param address: MAC address of a port, to get the port which has
                         this MAC address.
+        :param portgroup: UUID or name of a portgroup, to get only ports
+                           for that portgroup.
         :param marker: pagination marker for large data sets.
         :param limit: maximum number of resources to return in a single result.
                       This value cannot be larger than the value of max_limit
@@ -362,6 +444,9 @@ class PortsController(rest.RestController):
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:port:get', cdict, cdict)
 
+        if portgroup and not api_utils.allow_portgroups_subcontrollers():
+            raise exception.NotAcceptable()
+
         if not node_uuid and node:
             # We're invoking this interface using positional notation, or
             # explicitly using 'node'.  Try and determine which one.
@@ -376,9 +461,9 @@ class PortsController(rest.RestController):
             raise exception.HTTPNotFound()
 
         resource_url = '/'.join(['ports', 'detail'])
-        return self._get_ports_collection(node_uuid or node, address, marker,
+        return self._get_ports_collection(node_uuid or node, address,
-                                          limit, sort_key, sort_dir,
+                                          portgroup, marker, limit, sort_key,
-                                          resource_url)
+                                          sort_dir, resource_url)
 
     @METRICS.timer('PortsController.get_one')
     @expose.expose(Port, types.uuid, types.listtype)
@@ -388,12 +473,12 @@ class PortsController(rest.RestController):
         :param port_uuid: UUID of a port.
         :param fields: Optional, a list with a specified set of fields
             of the resource to be returned.
-        :raises: NotAcceptable
+        :raises: NotAcceptable, HTTPNotFound
         """
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:port:get', cdict, cdict)
 
-        if self.parent_node_ident:
+        if self.parent_node_ident or self.parent_portgroup_ident:
             raise exception.OperationNotPermitted()
 
         api_utils.check_allow_specify_fields(fields)
@@ -407,18 +492,21 @@ class PortsController(rest.RestController):
         """Create a new port.
 
         :param port: a port within the request body.
-        :raises: NotAcceptable
+        :raises: NotAcceptable, HTTPNotFound
         """
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:port:create', cdict, cdict)
 
-        if self.parent_node_ident:
+        if self.parent_node_ident or self.parent_portgroup_ident:
             raise exception.OperationNotPermitted()
 
         pdict = port.as_dict()
-        if not api_utils.allow_port_advanced_net_fields():
+        if (not api_utils.allow_port_advanced_net_fields() and
-            if set(pdict).intersection(self.advanced_net_fields):
+                set(pdict).intersection(self.advanced_net_fields)):
-                raise exception.NotAcceptable()
+            raise exception.NotAcceptable()
+        if (not api_utils.allow_portgroups_subcontrollers() and
+            'portgroup_uuid' in pdict):
+            raise exception.NotAcceptable()
 
         new_port = objects.Port(pecan.request.context,
                                 **pdict)
@@ -436,19 +524,26 @@ class PortsController(rest.RestController):
 
         :param port_uuid: UUID of a port.
         :param patch: a json PATCH document to apply to this port.
-        :raises: NotAcceptable
+        :raises: NotAcceptable, HTTPNotFound
         """
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:port:update', cdict, cdict)
 
-        if self.parent_node_ident:
+        if self.parent_node_ident or self.parent_portgroup_ident:
             raise exception.OperationNotPermitted()
-        if not api_utils.allow_port_advanced_net_fields():
+
-            for field in self.advanced_net_fields:
+        fields_to_check = set()
-                field_path = '/%s' % field
+        for field in self.advanced_net_fields + ['portgroup_uuid']:
-                if (api_utils.get_patch_values(patch, field_path) or
+            field_path = '/%s' % field
-                        api_utils.is_path_removed(patch, field_path)):
+            if (api_utils.get_patch_values(patch, field_path) or
-                    raise exception.NotAcceptable()
+                    api_utils.is_path_removed(patch, field_path)):
+                fields_to_check.add(field)
+        if (fields_to_check.intersection(self.advanced_net_fields) and
+                not api_utils.allow_port_advanced_net_fields()):
+            raise exception.NotAcceptable()
+        if ('portgroup_uuid' in fields_to_check and
+                not api_utils.allow_portgroups_subcontrollers()):
+            raise exception.NotAcceptable()
 
         rpc_port = objects.Port.get_by_uuid(pecan.request.context, port_uuid)
         try:
@@ -458,10 +553,18 @@ class PortsController(rest.RestController):
             #    not present in the API object
             # 2) Add node_uuid
             port_dict['node_uuid'] = port_dict.pop('node_id', None)
+            # NOTE(vsaienko):
+            # 1) Remove portgroup_id because it's an internal value and
+            #    not present in the API object
+            # 2) Add portgroup_uuid
+            port_dict['portgroup_uuid'] = port_dict.pop('portgroup_id', None)
             port = Port(**api_utils.apply_jsonpatch(port_dict, patch))
         except api_utils.JSONPATCH_EXCEPTIONS as e:
             raise exception.PatchError(patch=patch, reason=e)
 
+        if api_utils.is_path_removed(patch, '/portgroup_uuid'):
+            rpc_port.portgroup_id = None
+
         # Update only the fields that have changed
         for field in objects.Port.fields:
             try:
@@ -489,12 +592,14 @@ class PortsController(rest.RestController):
         """Delete a port.
 
         :param port_uuid: UUID of a port.
+        :raises OperationNotPermitted, HTTPNotFound
         """
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:port:delete', cdict, cdict)
 
-        if self.parent_node_ident:
+        if self.parent_node_ident or self.parent_portgroup_ident:
             raise exception.OperationNotPermitted()
+
         rpc_port = objects.Port.get_by_uuid(pecan.request.context,
                                             port_uuid)
         rpc_node = objects.Node.get_by_id(pecan.request.context,

ironic/api/controllers/v1/portgroup.py

@@ -21,6 +21,7 @@ from wsme import types as wtypes
 from ironic.api.controllers import base
 from ironic.api.controllers import link
 from ironic.api.controllers.v1 import collection
+from ironic.api.controllers.v1 import port
 from ironic.api.controllers.v1 import types
 from ironic.api.controllers.v1 import utils as api_utils
 from ironic.api import expose
@@ -93,6 +94,9 @@ class Portgroup(base.APIBase):
     """Indicates whether ports of this portgroup may be used as
        single NIC ports"""
 
+    ports = wsme.wsattr([link.Link], readonly=True)
+    """Links to the collection of ports of this portgroup"""
+
     def __init__(self, **kwargs):
         self.fields = []
         fields = list(objects.Portgroup.fields)
@@ -216,6 +220,30 @@ class PortgroupsController(pecan.rest.RestController):
 
     invalid_sort_key_list = ['extra', 'internal_info']
 
+    _subcontroller_map = {
+        'ports': port.PortsController,
+    }
+
+    @pecan.expose()
+    def _lookup(self, ident, subres, *remainder):
+        if not api_utils.allow_portgroups():
+            pecan.abort(http_client.NOT_FOUND)
+        try:
+            ident = types.uuid_or_name.validate(ident)
+        except exception.InvalidUuidOrName as e:
+            pecan.abort(http_client.BAD_REQUEST, e.args[0])
+        subcontroller = self._subcontroller_map.get(subres)
+        if subcontroller:
+            if api_utils.allow_portgroups_subcontrollers():
+                return subcontroller(
+                    portgroup_ident=ident,
+                    node_ident=self.parent_node_ident), remainder
+            pecan.abort(http_client.NOT_FOUND)
+
+    def __init__(self, node_ident=None):
+        super(PortgroupsController, self).__init__()
+        self.parent_node_ident = node_ident
+
     def _get_portgroups_collection(self, node_ident, address,
                                    marker, limit, sort_key, sort_dir,
                                    resource_url=None, fields=None):
@@ -244,6 +272,8 @@ class PortgroupsController(pecan.rest.RestController):
                 _("The sort_key value %(key)s is an invalid field for "
                   "sorting") % {'key': sort_key})
 
+        node_ident = self.parent_node_ident or node_ident
+
         if node_ident:
             # FIXME: Since all we need is the node ID, we can
             #        make this more efficient by only querying
@@ -367,6 +397,9 @@ class PortgroupsController(pecan.rest.RestController):
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:portgroup:get', cdict, cdict)
 
+        if self.parent_node_ident:
+            raise exception.OperationNotPermitted()
+
         rpc_portgroup = api_utils.get_rpc_portgroup(portgroup_ident)
         return Portgroup.convert_with_links(rpc_portgroup, fields=fields)
 
@@ -383,6 +416,9 @@ class PortgroupsController(pecan.rest.RestController):
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:portgroup:create', cdict, cdict)
 
+        if self.parent_node_ident:
+            raise exception.OperationNotPermitted()
+
         if (portgroup.name and
                 not api_utils.is_valid_logical_name(portgroup.name)):
             error_msg = _("Cannot create portgroup with invalid name "
@@ -413,6 +449,9 @@ class PortgroupsController(pecan.rest.RestController):
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:portgroup:update', cdict, cdict)
 
+        if self.parent_node_ident:
+            raise exception.OperationNotPermitted()
+
         rpc_portgroup = api_utils.get_rpc_portgroup(portgroup_ident)
 
         names = api_utils.get_patch_values(patch, '/name')
@@ -473,6 +512,9 @@ class PortgroupsController(pecan.rest.RestController):
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:portgroup:delete', cdict, cdict)
 
+        if self.parent_node_ident:
+            raise exception.OperationNotPermitted()
+
         rpc_portgroup = api_utils.get_rpc_portgroup(portgroup_ident)
         rpc_node = objects.Node.get_by_id(pecan.request.context,
                                           rpc_portgroup.node_id)

ironic/api/controllers/v1/utils.py

@@ -422,6 +422,16 @@ def allow_portgroups():
             versions.MINOR_23_PORTGROUPS)
 
 
+def allow_portgroups_subcontrollers():
+    """Check if portgroups can be used as subcontrollers.
+
+    Version 1.24 of the API added support for Portgroups as
+    subcontrollers
+    """
+    return (pecan.request.version.minor >=
+            versions.MINOR_24_PORTGROUPS_SUBCONTROLLERS)
+
+
 def get_controller_reserved_names(cls):
     """Get reserved names for a given controller.
 

ironic/api/controllers/v1/versions.py

@@ -53,6 +53,8 @@ BASE_VERSION = 1
 # v1.21: Add node.resource_class
 # v1.22: Ramdisk lookup and heartbeat endpoints.
 # v1.23: Add portgroup support.
+# v1.24: Add subcontrollers: node.portgroup, portgroup.ports.
+#        Add port.portgroup_uuid field.
 
 MINOR_0_JUNO = 0
 MINOR_1_INITIAL_VERSION = 1
@@ -78,11 +80,12 @@ MINOR_20_NETWORK_INTERFACE = 20
 MINOR_21_RESOURCE_CLASS = 21
 MINOR_22_LOOKUP_HEARTBEAT = 22
 MINOR_23_PORTGROUPS = 23
+MINOR_24_PORTGROUPS_SUBCONTROLLERS = 24
 
 # When adding another version, update MINOR_MAX_VERSION and also update
 # doc/source/dev/webapi-version-history.rst with a detailed explanation of
 # what the version has changed.
-MINOR_MAX_VERSION = MINOR_23_PORTGROUPS
+MINOR_MAX_VERSION = MINOR_24_PORTGROUPS_SUBCONTROLLERS
 
 # String representations of the minor and maximum versions
 MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)

ironic/tests/unit/api/base.py

@@ -223,7 +223,7 @@ class BaseApiTest(base.DbTestCase):
         print('GOT:%s' % response)
         return response
 
-    def validate_link(self, link, bookmark=False):
+    def validate_link(self, link, bookmark=False, headers=None):
         """Checks if the given link can get correct data."""
         # removes the scheme and net location parts of the link
         url_parts = list(urlparse.urlparse(link))
@@ -235,7 +235,7 @@ class BaseApiTest(base.DbTestCase):
 
         full_path = urlparse.urlunparse(url_parts)
         try:
-            self.get_json(full_path, path_prefix='')
+            self.get_json(full_path, path_prefix='', headers=headers)
             return True
         except Exception:
             return False

ironic/tests/unit/api/v1/test_nodes.py

@@ -459,6 +459,47 @@ class TestListNodes(test_api_base.BaseApiTest):
         data = self.get_json('/nodes/%s' % node.uuid)
         self.assertIn('ports', data)
 
+    def test_portgroups_subresource(self):
+        node = obj_utils.create_test_node(self.context)
+        headers = {'X-OpenStack-Ironic-API-Version': '1.24'}
+        for id_ in range(2):
+            obj_utils.create_test_portgroup(self.context, node_id=node.id,
+                                            name="pg-%s" % id_,
+                                            uuid=uuidutils.generate_uuid(),
+                                            address='52:54:00:cf:2d:3%s' % id_)
+
+        data = self.get_json('/nodes/%s/portgroups' % node.uuid,
+                             headers=headers)
+        self.assertEqual(2, len(data['portgroups']))
+        self.assertNotIn('next', data)
+
+        # Test collection pagination
+        data = self.get_json('/nodes/%s/portgroups?limit=1' % node.uuid,
+                             headers=headers)
+        self.assertEqual(1, len(data['portgroups']))
+        self.assertIn('next', data)
+
+    def test_portgroups_subresource_link(self):
+        node = obj_utils.create_test_node(self.context)
+        data = self.get_json(
+            '/nodes/%s' % node.uuid,
+            headers={'X-OpenStack-Ironic-API-Version': '1.24'})
+        self.assertIn('portgroups', data.keys())
+
+    def test_portgroups_subresource_link_hidden_for_older_versions(self):
+        node = obj_utils.create_test_node(self.context)
+        data = self.get_json(
+            '/nodes/%s' % node.uuid,
+            headers={'X-OpenStack-Ironic-API-Version': '1.20'})
+        self.assertNotIn('portgroups', data.keys())
+
+    def test_portgroups_subresource_old_api_version(self):
+        node = obj_utils.create_test_node(self.context)
+        response = self.get_json(
+            '/nodes/%s/portgroups' % node.uuid, expect_errors=True,
+            headers={'X-OpenStack-Ironic-API-Version': '1.23'})
+        self.assertEqual(http_client.NOT_FOUND, response.status_int)
+
     def test_ports_subresource(self):
         node = obj_utils.create_test_node(self.context)
 
@@ -497,6 +538,15 @@ class TestListNodes(test_api_base.BaseApiTest):
         self.assertIn('Expected a logical name or UUID',
                       response.json['error_message'])
 
+    def test_ports_subresource_via_portgroups_subres_not_allowed(self):
+        node = obj_utils.create_test_node(self.context)
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=node.id)
+        response = self.get_json('/nodes/%s/portgroups/%s/ports' % (
+            node.uuid, pg.uuid), expect_errors=True,
+            headers={api_base.Version.string: '1.24'})
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
     @mock.patch.object(timeutils, 'utcnow')
     def _test_node_states(self, mock_utcnow, api_version=None):
         fake_state = 'fake-state'
@@ -1210,6 +1260,15 @@ class TestPatch(test_api_base.BaseApiTest):
               'op': 'add'}], expect_errors=True)
         self.assertEqual(http_client.FORBIDDEN, response.status_int)
 
+    def test_patch_portgroups_subresource(self):
+        response = self.patch_json(
+            '/nodes/%s/portgroups/9bb50f13-0b8d-4ade-ad2d-d91fefdef9cc' %
+            self.node.uuid,
+            [{'path': '/extra/foo', 'value': 'bar',
+              'op': 'add'}], expect_errors=True,
+            headers={'X-OpenStack-Ironic-API-Version': '1.24'})
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
     def test_remove_uuid(self):
         response = self.patch_json('/nodes/%s' % self.node.uuid,
                                    [{'path': '/uuid', 'op': 'remove'}],
@@ -1929,6 +1988,15 @@ class TestPost(test_api_base.BaseApiTest):
                                   expect_errors=True)
         self.assertEqual(http_client.FORBIDDEN, response.status_int)
 
+    def test_post_portgroups_subresource(self):
+        node = obj_utils.create_test_node(self.context)
+        pgdict = test_api_utils.portgroup_post_data(node_id=None)
+        pgdict['node_uuid'] = node.uuid
+        response = self.post_json(
+            '/nodes/%s/portgroups' % node.uuid, pgdict, expect_errors=True,
+            headers={'X-OpenStack-Ironic-API-Version': '1.24'})
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
     def test_create_node_no_mandatory_field_driver(self):
         ndict = test_api_utils.post_get_test_node()
         del ndict['driver']
@@ -2134,6 +2202,16 @@ class TestDelete(test_api_base.BaseApiTest):
             expect_errors=True)
         self.assertEqual(http_client.FORBIDDEN, response.status_int)
 
+    def test_delete_portgroup_subresource(self):
+        node = obj_utils.create_test_node(self.context)
+        pg = obj_utils.create_test_portgroup(self.context, node_id=node.id)
+        response = self.delete(
+            '/nodes/%(node_uuid)s/portgroups/%(pg_uuid)s' %
+            {'node_uuid': node.uuid, 'pg_uuid': pg.uuid},
+            expect_errors=True,
+            headers={'X-OpenStack-Ironic-API-Version': '1.24'})
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
     @mock.patch.object(rpcapi.ConductorAPI, 'destroy_node')
     def test_delete_associated(self, mock_dn):
         node = obj_utils.create_test_node(

ironic/tests/unit/api/v1/test_portgroups.py

@@ -171,6 +171,21 @@ class TestListPortgroups(test_api_base.BaseApiTest):
         uuids = [n['uuid'] for n in data['portgroups']]
         six.assertCountEqual(self, portgroups, uuids)
 
+    def test_links(self):
+        uuid = uuidutils.generate_uuid()
+        obj_utils.create_test_portgroup(self.context,
+                                        uuid=uuid,
+                                        node_id=self.node.id)
+        data = self.get_json('/portgroups/%s' % uuid, headers=self.headers)
+        self.assertIn('links', data)
+        self.assertIn('ports', data)
+        self.assertEqual(2, len(data['links']))
+        self.assertIn(uuid, data['links'][0]['href'])
+        for l in data['links']:
+            bookmark = l['rel'] == 'bookmark'
+            self.assertTrue(self.validate_link(l['href'], bookmark=bookmark,
+                            headers=self.headers))
+
     def test_collection_links(self):
         portgroups = []
         for id_ in range(5):
@@ -204,6 +219,47 @@ class TestListPortgroups(test_api_base.BaseApiTest):
         next_marker = data['portgroups'][-1]['uuid']
         self.assertIn(next_marker, data['next'])
 
+    def test_ports_subresource(self):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             uuid=uuidutils.generate_uuid(),
+                                             node_id=self.node.id)
+
+        for id_ in range(2):
+            obj_utils.create_test_port(self.context, node_id=self.node.id,
+                                       uuid=uuidutils.generate_uuid(),
+                                       portgroup_id=pg.id,
+                                       address='52:54:00:cf:2d:3%s' % id_)
+
+        data = self.get_json('/portgroups/%s/ports' % pg.uuid,
+                             headers=self.headers)
+        self.assertEqual(2, len(data['ports']))
+        self.assertNotIn('next', data.keys())
+
+        data = self.get_json('/portgroups/%s/ports/detail' % pg.uuid,
+                             headers=self.headers)
+        self.assertEqual(2, len(data['ports']))
+        self.assertNotIn('next', data.keys())
+
+        # Test collection pagination
+        data = self.get_json('/portgroups/%s/ports?limit=1' % pg.uuid,
+                             headers=self.headers)
+        self.assertEqual(1, len(data['ports']))
+        self.assertIn('next', data.keys())
+
+        # Test get one old api version, /portgroups controller not allowed
+        response = self.get_json('/portgroups/%s/ports/%s' % (
+            pg.uuid, uuidutils.generate_uuid()),
+            headers={api_base.Version.string: str(api_v1.MIN_VER)},
+            expect_errors=True)
+        self.assertEqual(http_client.NOT_FOUND, response.status_int)
+
+        # Test get one not allowed to access to /portgroups/<uuid>/ports/<uuid>
+        response = self.get_json(
+            '/portgroups/%s/ports/%s' % (pg.uuid, uuidutils.generate_uuid()),
+            headers={api_base.Version.string: str(api_v1.MAX_VER)},
+            expect_errors=True)
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
     def test_ports_subresource_no_portgroups_allowed(self):
         pg = obj_utils.create_test_portgroup(self.context,
                                              uuid=uuidutils.generate_uuid(),
@@ -220,11 +276,32 @@ class TestListPortgroups(test_api_base.BaseApiTest):
         self.assertEqual(http_client.NOT_FOUND, response.status_int)
         self.assertEqual('application/json', response.content_type)
 
+    def test_get_all_ports_by_portgroup_uuid(self):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id)
+        port = obj_utils.create_test_port(self.context, node_id=self.node.id,
+                                          portgroup_id=pg.id)
+        data = self.get_json('/portgroups/%s/ports' % pg.uuid,
+                             headers={api_base.Version.string: '1.24'})
+        self.assertEqual(port.uuid, data['ports'][0]['uuid'])
+
+    def test_ports_subresource_not_allowed(self):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id)
+        response = self.get_json('/portgroups/%s/ports' % pg.uuid,
+                                 expect_errors=True,
+                                 headers={api_base.Version.string: '1.23'})
+        self.assertEqual(http_client.NOT_FOUND, response.status_int)
+        self.assertIn('The resource could not be found.',
+                      response.json['error_message'])
+
     def test_ports_subresource_portgroup_not_found(self):
         non_existent_uuid = 'eeeeeeee-cccc-aaaa-bbbb-cccccccccccc'
         response = self.get_json('/portgroups/%s/ports' % non_existent_uuid,
                                  expect_errors=True, headers=self.headers)
         self.assertEqual(http_client.NOT_FOUND, response.status_int)
+        self.assertIn('Portgroup %s could not be found.' % non_existent_uuid,
+                      response.json['error_message'])
 
     def test_portgroup_by_address(self):
         address_template = "aa:bb:cc:dd:ee:f%d"
@@ -753,6 +830,29 @@ class TestPost(test_api_base.BaseApiTest):
         self.assertEqual(urlparse.urlparse(response.location).path,
                          expected_location)
 
+    @mock.patch.object(timeutils, 'utcnow', autospec=True)
+    def test_create_portgroup_v123(self, mock_utcnow):
+        pdict = apiutils.post_get_test_portgroup()
+        test_time = datetime.datetime(2000, 1, 1, 0, 0)
+        mock_utcnow.return_value = test_time
+        headers = {api_base.Version.string: "1.23"}
+        response = self.post_json('/portgroups', pdict,
+                                  headers=headers)
+        self.assertEqual(http_client.CREATED, response.status_int)
+        result = self.get_json('/portgroups/%s' % pdict['uuid'],
+                               headers=headers)
+        self.assertEqual(pdict['uuid'], result['uuid'])
+        self.assertEqual(pdict['node_uuid'], result['node_uuid'])
+        self.assertFalse(result['updated_at'])
+        return_created_at = timeutils.parse_isotime(
+            result['created_at']).replace(tzinfo=None)
+        self.assertEqual(test_time, return_created_at)
+        # Check location header
+        self.assertIsNotNone(response.location)
+        expected_location = '/v1/portgroups/%s' % pdict['uuid']
+        self.assertEqual(urlparse.urlparse(response.location).path,
+                         expected_location)
+
     def test_create_portgroup_invalid_api_version(self):
         pdict = apiutils.post_get_test_portgroup()
         response = self.post_json(

ironic/tests/unit/api/v1/test_ports.py

@@ -42,18 +42,23 @@ from ironic.tests.unit.objects import utils as obj_utils
 
 
 # NOTE(lucasagomes): When creating a port via API (POST)
-#                    we have to use node_uuid
+#                    we have to use node_uuid and portgroup_uuid
 def post_get_test_port(**kw):
     port = apiutils.port_post_data(**kw)
     node = dbutils.get_test_node()
+    portgroup = dbutils.get_test_portgroup()
     port['node_uuid'] = kw.get('node_uuid', node['uuid'])
+    port['portgroup_uuid'] = kw.get('portgroup_uuid', portgroup['uuid'])
     return port
 
 
 class TestPortObject(base.TestCase):
 
-    def test_port_init(self):
+    @mock.patch("pecan.request")
-        port_dict = apiutils.port_post_data(node_id=None)
+    def test_port_init(self, mock_pecan_req):
+        mock_pecan_req.version.minor = 1
+        port_dict = apiutils.port_post_data(node_id=None,
+                                            portgroup_uuid=None)
         del port_dict['extra']
         port = api_port.Port(**port_dict)
         self.assertEqual(wtypes.Unset, port.extra)
@@ -84,8 +89,24 @@ class TestListPorts(test_api_base.BaseApiTest):
         self.assertEqual(port.uuid, data['uuid'])
         self.assertIn('extra', data)
         self.assertIn('node_uuid', data)
-        # never expose the node_id
+        # never expose the node_id, port_id, portgroup_id
         self.assertNotIn('node_id', data)
+        self.assertNotIn('port_id', data)
+        self.assertNotIn('portgroup_id', data)
+        self.assertNotIn('portgroup_uuid', data)
+
+    def test_get_one_portgroup_is_none(self):
+        port = obj_utils.create_test_port(self.context, node_id=self.node.id)
+        data = self.get_json('/ports/%s' % port.uuid,
+                             headers={api_base.Version.string: '1.24'})
+        self.assertEqual(port.uuid, data['uuid'])
+        self.assertIn('extra', data)
+        self.assertIn('node_uuid', data)
+        # never expose the node_id, port_id, portgroup_id
+        self.assertNotIn('node_id', data)
+        self.assertNotIn('port_id', data)
+        self.assertNotIn('portgroup_id', data)
+        self.assertIn('portgroup_uuid', data)
 
     def test_get_one_custom_fields(self):
         port = obj_utils.create_test_port(self.context, node_id=self.node.id)
@@ -146,7 +167,14 @@ class TestListPorts(test_api_base.BaseApiTest):
         self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
 
     def test_detail(self):
-        port = obj_utils.create_test_port(self.context, node_id=self.node.id)
+        llc = {'switch_info': 'switch', 'switch_id': 'aa:bb:cc:dd:ee:ff',
+               'port_id': 'Gig0/1'}
+        portgroup = obj_utils.create_test_portgroup(self.context,
+                                                    node_id=self.node.id)
+        port = obj_utils.create_test_port(self.context, node_id=self.node.id,
+                                          portgroup_id=portgroup.id,
+                                          pxe_enabled=False,
+                                          local_link_connection=llc)
         data = self.get_json(
             '/ports/detail',
             headers={api_base.Version.string: str(api_v1.MAX_VER)}
@@ -157,8 +185,10 @@ class TestListPorts(test_api_base.BaseApiTest):
         self.assertIn('node_uuid', data['ports'][0])
         self.assertIn('pxe_enabled', data['ports'][0])
         self.assertIn('local_link_connection', data['ports'][0])
-        # never expose the node_id
+        self.assertIn('portgroup_uuid', data['ports'][0])
+        # never expose the node_id and portgroup_id
         self.assertNotIn('node_id', data['ports'][0])
+        self.assertNotIn('portgroup_id', data['ports'][0])
 
     def test_detail_against_single(self):
         port = obj_utils.create_test_port(self.context, node_id=self.node.id)
@@ -353,6 +383,50 @@ class TestListPorts(test_api_base.BaseApiTest):
         self.assertEqual(0, mock_get_rpc_node.call_count)
         self.assertEqual(http_client.NOT_ACCEPTABLE, data.status_int)
 
+    def test_get_all_by_portgroup_uuid(self):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id)
+        port = obj_utils.create_test_port(self.context, node_id=self.node.id,
+                                          portgroup_id=pg.id)
+        data = self.get_json('/ports/detail?portgroup=%s' % pg.uuid,
+                             headers={api_base.Version.string: '1.24'})
+        self.assertEqual(port.uuid, data['ports'][0]['uuid'])
+        self.assertEqual(pg.uuid,
+                         data['ports'][0]['portgroup_uuid'])
+
+    def test_get_all_by_portgroup_uuid_older_api_version(self):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id)
+        response = self.get_json(
+            '/ports/detail?portgroup=%s' % pg.uuid,
+            headers={api_base.Version.string: '1.14'},
+            expect_errors=True
+        )
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
+
+    def test_get_all_by_portgroup_name(self):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id)
+        port = obj_utils.create_test_port(self.context, node_id=self.node.id,
+                                          portgroup_id=pg.id)
+        data = self.get_json('/ports/detail?portgroup=%s' % pg.name,
+                             headers={api_base.Version.string: '1.24'})
+        self.assertEqual(port.uuid, data['ports'][0]['uuid'])
+        self.assertEqual(pg.uuid,
+                         data['ports'][0]['portgroup_uuid'])
+        self.assertEqual(1, len(data['ports']))
+
+    def test_get_all_by_portgroup_uuid_and_node_uuid(self):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id)
+        response = self.get_json(
+            '/ports/detail?portgroup=%s&node=%s' % (pg.uuid, self.node.uuid),
+            headers={api_base.Version.string: '1.24'},
+            expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
     @mock.patch.object(api_port.PortsController, '_get_ports_collection')
     def test_detail_with_incorrect_api_usage(self, mock_gpc):
         # GET /v1/ports/detail specifying node and node_uuid.  In this case
@@ -361,7 +435,22 @@ class TestListPorts(test_api_base.BaseApiTest):
                       ('test-node', self.node.uuid))
         mock_gpc.assert_called_once_with(self.node.uuid, mock.ANY, mock.ANY,
                                          mock.ANY, mock.ANY, mock.ANY,
-                                         mock.ANY)
+                                         mock.ANY, mock.ANY)
+
+    def test_portgroups_subresource_node_not_found(self):
+        non_existent_uuid = 'eeeeeeee-cccc-aaaa-bbbb-cccccccccccc'
+        response = self.get_json('/portgroups/%s/ports' % non_existent_uuid,
+                                 expect_errors=True)
+        self.assertEqual(http_client.NOT_FOUND, response.status_int)
+
+    def test_portgroups_subresource_invalid_ident(self):
+        invalid_ident = '123 123'
+        response = self.get_json('/portgroups/%s/ports' % invalid_ident,
+                                 headers={api_base.Version.string: '1.24'},
+                                 expect_errors=True)
+        self.assertEqual(http_client.BAD_REQUEST, response.status_int)
+        self.assertIn('Expected a logical name or UUID',
+                      response.json['error_message'])
 
 
 @mock.patch.object(rpcapi.ConductorAPI, 'update_port')
@@ -498,6 +587,96 @@ class TestPatch(test_api_base.BaseApiTest):
         self.assertTrue(response.json['error_message'])
         self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code)
 
+    def test_add_portgroup_uuid(self, mock_upd):
+        mock_upd.return_value = self.port
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id,
+                                             uuid=uuidutils.generate_uuid(),
+                                             address='bb:bb:bb:bb:bb:bb',
+                                             name='bar')
+        headers = {api_base.Version.string: '1.24'}
+        response = self.patch_json('/ports/%s' % self.port.uuid,
+                                   [{'path':
+                                     '/portgroup_uuid',
+                                     'value': pg.uuid,
+                                     'op': 'add'}],
+                                   headers=headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.OK, response.status_code)
+
+    def test_replace_portgroup_uuid(self, mock_upd):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id,
+                                             uuid=uuidutils.generate_uuid(),
+                                             address='bb:bb:bb:bb:bb:bb',
+                                             name='bar')
+        mock_upd.return_value = self.port
+        headers = {api_base.Version.string: '1.24'}
+        response = self.patch_json('/ports/%s' % self.port.uuid,
+                                   [{'path': '/portgroup_uuid',
+                                     'value': pg.uuid,
+                                     'op': 'replace'}],
+                                   headers=headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.OK, response.status_code)
+
+    def test_replace_portgroup_uuid_remove(self, mock_upd):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id,
+                                             uuid=uuidutils.generate_uuid(),
+                                             address='bb:bb:bb:bb:bb:bb',
+                                             name='bar')
+        mock_upd.return_value = self.port
+        headers = {api_base.Version.string: '1.24'}
+        response = self.patch_json('/ports/%s' % self.port.uuid,
+                                   [{'path': '/portgroup_uuid',
+                                     'value': pg.uuid,
+                                     'op': 'remove'}],
+                                   headers=headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertIsNone(mock_upd.call_args[0][1].portgroup_id)
+
+    def test_replace_portgroup_uuid_remove_add(self, mock_upd):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id,
+                                             uuid=uuidutils.generate_uuid(),
+                                             address='bb:bb:bb:bb:bb:bb',
+                                             name='bar')
+        pg1 = obj_utils.create_test_portgroup(self.context,
+                                              node_id=self.node.id,
+                                              uuid=uuidutils.generate_uuid(),
+                                              address='bb:bb:bb:bb:bb:b1',
+                                              name='bbb')
+        mock_upd.return_value = self.port
+        headers = {api_base.Version.string: '1.24'}
+        response = self.patch_json('/ports/%s' % self.port.uuid,
+                                   [{'path': '/portgroup_uuid',
+                                     'value': pg.uuid,
+                                     'op': 'remove'},
+                                    {'path': '/portgroup_uuid',
+                                     'value': pg1.uuid,
+                                     'op': 'add'}],
+                                   headers=headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertTrue(pg1.id, mock_upd.call_args[0][1].portgroup_id)
+
+    def test_replace_portgroup_uuid_old_api(self, mock_upd):
+        pg = obj_utils.create_test_portgroup(self.context,
+                                             node_id=self.node.id,
+                                             uuid=uuidutils.generate_uuid(),
+                                             address='bb:bb:bb:bb:bb:bb',
+                                             name='bar')
+        mock_upd.return_value = self.port
+        headers = {api_base.Version.string: '1.15'}
+        response = self.patch_json('/ports/%s' % self.port.uuid,
+                                   [{'path': '/portgroup_uuid',
+                                     'value': pg.uuid,
+                                     'op': 'replace'}],
+                                   headers=headers,
+                                   expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code)
+
     def test_add_node_uuid(self, mock_upd):
         mock_upd.return_value = self.port
         response = self.patch_json('/ports/%s' % self.port.uuid,
@@ -729,12 +908,30 @@ class TestPatch(test_api_base.BaseApiTest):
         self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
         self.assertFalse(mock_upd.called)
 
+    def test_portgroups_subresource_patch(self, mock_upd):
+        portgroup = obj_utils.create_test_portgroup(self.context,
+                                                    node_id=self.node.id)
+        port = obj_utils.create_test_port(self.context, node_id=self.node.id,
+                                          uuid=uuidutils.generate_uuid(),
+                                          portgroup_id=portgroup.id,
+                                          address='52:55:00:cf:2d:31')
+        headers = {api_base.Version.string: '1.24'}
+        response = self.patch_json(
+            '/portgroups/%(portgroup)s/ports/%(port)s' %
+            {'portgroup': portgroup.uuid, 'port': port.uuid},
+            [{'path': '/address', 'value': '00:00:00:00:00:00',
+              'op': 'replace'}], headers=headers, expect_errors=True)
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+        self.assertEqual('application/json', response.content_type)
+
 
 class TestPost(test_api_base.BaseApiTest):
 
     def setUp(self):
         super(TestPost, 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)
         self.headers = {api_base.Version.string: str(
             versions.MAX_VERSION_STRING)}
 
@@ -853,6 +1050,53 @@ class TestPost(test_api_base.BaseApiTest):
         self.assertEqual(http_client.BAD_REQUEST, response.status_int)
         self.assertTrue(response.json['error_message'])
 
+    def test_create_port_portgroup_uuid_not_found(self):
+        pdict = post_get_test_port(
+            portgroup_uuid='1a1a1a1a-2b2b-3c3c-4d4d-5e5e5e5e5e5e')
+        response = self.post_json('/ports', pdict, expect_errors=True,
+                                  headers=self.headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.BAD_REQUEST, response.status_int)
+        self.assertTrue(response.json['error_message'])
+
+    def test_create_port_portgroup_uuid_not_found_old_api_version(self):
+        pdict = post_get_test_port(
+            portgroup_uuid='1a1a1a1a-2b2b-3c3c-4d4d-5e5e5e5e5e5e')
+        response = self.post_json('/ports', pdict, expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
+        self.assertTrue(response.json['error_message'])
+
+    def test_create_port_portgroup(self):
+        pdict = post_get_test_port(
+            portgroup_uuid=self.portgroup.uuid,
+            node_uuid=self.node.uuid)
+
+        response = self.post_json('/ports', pdict, headers=self.headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.CREATED, response.status_int)
+
+    def test_create_port_portgroup_different_nodes(self):
+        pdict = post_get_test_port(
+            portgroup_uuid=self.portgroup.uuid,
+            node_uuid=uuidutils.generate_uuid())
+
+        response = self.post_json('/ports', pdict, headers=self.headers,
+                                  expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.BAD_REQUEST, response.status_int)
+
+    def test_create_port_portgroup_old_api_version(self):
+        pdict = post_get_test_port(
+            portgroup_uuid=self.portgroup.uuid,
+            node_uuid=self.node.uuid
+        )
+        headers = {api_base.Version.string: '1.15'}
+        response = self.post_json('/ports', pdict, expect_errors=True,
+                                  headers=headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
+
     def test_create_port_address_already_exist(self):
         address = 'AA:AA:AA:11:22:33'
         pdict = post_get_test_port(address=address)
@@ -936,11 +1180,20 @@ class TestPost(test_api_base.BaseApiTest):
         headers = {api_base.Version.string: '1.14'}
         pdict = post_get_test_port(pxe_enabled=False)
         del pdict['local_link_connection']
+        del pdict['portgroup_uuid']
         response = self.post_json('/ports', pdict, headers=headers,
                                   expect_errors=True)
         self.assertEqual('application/json', response.content_type)
         self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
 
+    def test_portgroups_subresource_post(self):
+        headers = {api_base.Version.string: '1.24'}
+        pdict = post_get_test_port()
+        response = self.post_json('/portgroups/%s/ports' % self.portgroup.uuid,
+                                  pdict, headers=headers, expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
 
 @mock.patch.object(rpcapi.ConductorAPI, 'destroy_port')
 class TestDelete(test_api_base.BaseApiTest):
@@ -975,3 +1228,18 @@ class TestDelete(test_api_base.BaseApiTest):
         self.assertEqual(http_client.CONFLICT, ret.status_code)
         self.assertTrue(ret.json['error_message'])
         self.assertTrue(mock_dpt.called)
+
+    def test_portgroups_subresource_delete(self, mock_dpt):
+        portgroup = obj_utils.create_test_portgroup(self.context,
+                                                    node_id=self.node.id)
+        port = obj_utils.create_test_port(self.context, node_id=self.node.id,
+                                          uuid=uuidutils.generate_uuid(),
+                                          portgroup_id=portgroup.id,
+                                          address='52:55:00:cf:2d:31')
+        headers = {api_base.Version.string: '1.24'}
+        response = self.delete(
+            '/portgroups/%(portgroup)s/ports/%(port)s' %
+            {'portgroup': portgroup.uuid, 'port': port.uuid},
+            headers=headers, expect_errors=True)
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+        self.assertEqual('application/json', response.content_type)

releasenotes/notes/add-portgroups-subcontroller-9039f59bcf48b3e0.yaml

@@ -0,0 +1,10 @@
+---
+features:
+  - |
+    Adds, starting with REST API version 1.24:
+
+    * the new endpoint `v1/nodes/<node>/portgroups`;
+    * the new endpoint `v1/portgroups/<portgroup>/ports`;
+    * the new field `portgroup_uuid` to a port. This is the UUID
+      of a port group that this port belongs to, or None if it doesn't
+      belong to any port group.