Add portgroup configuration fields

This change adds mode and properties fields into portgroup object
and adds a new API microversion to work with them.

It also makes portgroups' address field optional for all API
microversions.

Partial-Bug: #1618754
Change-Id: Id8c62fa56908040b0df16cc54c122ce2473a4587

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

@@ -2,6 +2,10 @@
 REST API Version History
 ========================
 
+**1.26** (Ocata)
+
+    Add portgroup ``mode`` and ``properties`` fields.
+
 **1.25** (Ocata)
 
     Add possibility to unset chassis_uuid from a node.

etc/ironic/ironic.conf.sample

@@ -123,6 +123,12 @@
 # value)
 #state_path = $pybasedir
 
+# Default mode for portgroups. Allowed values can be found in
+# the linux kernel documentation on bonding:
+# https://www.kernel.org/doc/Documentation/networking/bonding.txt.
+# (string value)
+#default_portgroup_mode = active-backup
+
 # Name of this node. This can be an opaque identifier. It is
 # not necessarily a hostname, FQDN, or IP address. However,
 # the node name must be valid within an AMQP key, and if using
@@ -2300,6 +2306,11 @@
 # Minimum value: 5
 #default_notify_timeout = 30
 
+# The duration to schedule a purge of idle sender links.
+# Detach link after expiry. (integer value)
+# Minimum value: 1
+#default_sender_link_timeout = 600
+
 # Indicates the addressing mode used by the driver.
 # Permitted values:
 # 'legacy'   - use legacy non-routable addressing

ironic/api/controllers/v1/portgroup.py

@@ -71,7 +71,7 @@ class Portgroup(base.APIBase):
     uuid = types.uuid
     """Unique UUID for this portgroup"""
 
-    address = wsme.wsattr(types.macaddress, mandatory=True)
+    address = wsme.wsattr(types.macaddress)
     """MAC Address for this portgroup"""
 
     extra = {wtypes.text: types.jsontype}
@@ -94,6 +94,14 @@ class Portgroup(base.APIBase):
     """Indicates whether ports of this portgroup may be used as
        single NIC ports"""
 
+    mode = wsme.wsattr(wtypes.text)
+    """The mode for this portgroup. See linux bonding
+    documentation for details:
+    https://www.kernel.org/doc/Documentation/networking/bonding.txt"""
+
+    properties = {wtypes.text: types.jsontype}
+    """This portgroup's properties"""
+
     ports = wsme.wsattr([link.Link], readonly=True)
     """Links to the collection of ports of this portgroup"""
 
@@ -165,6 +173,8 @@ class Portgroup(base.APIBase):
                      extra={'foo': 'bar'},
                      internal_info={'baz': 'boo'},
                      standalone_ports_supported=True,
+                     mode='active-backup',
+                     properties={},
                      created_at=datetime.datetime(2000, 1, 1, 12, 0, 0),
                      updated_at=datetime.datetime(2000, 1, 1, 12, 0, 0))
         # NOTE(lucasagomes): node_uuid getter() method look at the
@@ -218,7 +228,7 @@ class PortgroupsController(pecan.rest.RestController):
         'detail': ['GET'],
     }
 
-    invalid_sort_key_list = ['extra', 'internal_info']
+    invalid_sort_key_list = ['extra', 'internal_info', 'properties']
 
     _subcontroller_map = {
         'ports': port.PortsController,
@@ -339,6 +349,8 @@ class PortgroupsController(pecan.rest.RestController):
         cdict = pecan.request.context.to_dict()
         policy.authorize('baremetal:portgroup:get', cdict, cdict)
 
+        api_utils.check_allowed_portgroup_fields(fields)
+
         if fields is None:
             fields = _DEFAULT_RETURN_FIELDS
 
@@ -400,6 +412,8 @@ class PortgroupsController(pecan.rest.RestController):
         if self.parent_node_ident:
             raise exception.OperationNotPermitted()
 
+        api_utils.check_allowed_portgroup_fields(fields)
+
         rpc_portgroup = api_utils.get_rpc_portgroup(portgroup_ident)
         return Portgroup.convert_with_links(rpc_portgroup, fields=fields)
 
@@ -419,6 +433,11 @@ class PortgroupsController(pecan.rest.RestController):
         if self.parent_node_ident:
             raise exception.OperationNotPermitted()
 
+        if (not api_utils.allow_portgroup_mode_properties() and
+                (portgroup.mode is not wtypes.Unset or
+                 portgroup.properties is not wtypes.Unset)):
+            raise exception.NotAcceptable()
+
         if (portgroup.name and
                 not api_utils.is_valid_logical_name(portgroup.name)):
             error_msg = _("Cannot create portgroup with invalid name "
@@ -452,6 +471,11 @@ class PortgroupsController(pecan.rest.RestController):
         if self.parent_node_ident:
             raise exception.OperationNotPermitted()
 
+        if (not api_utils.allow_portgroup_mode_properties() and
+                (api_utils.is_path_updated(patch, '/mode') or
+                 api_utils.is_path_updated(patch, '/properties'))):
+            raise exception.NotAcceptable()
+
         rpc_portgroup = api_utils.get_rpc_portgroup(portgroup_ident)
 
         names = api_utils.get_patch_values(patch, '/name')

ironic/api/controllers/v1/utils.py

@@ -113,6 +113,18 @@ def is_path_removed(patch, path):
             return True
 
 
+def is_path_updated(patch, path):
+    """Returns whether the patch includes operation on path (or its subpath).
+
+    :param patch: HTTP PATCH request body.
+    :param path: the path to check.
+    :returns: True if path or subpath being patched, False otherwise.
+    """
+    path = path.rstrip('/')
+    for p in patch:
+        return p['path'] == path or p['path'].startswith(path + '/')
+
+
 def allow_node_logical_names():
     # v1.5 added logical name aliases
     return pecan.request.version.minor >= versions.MINOR_5_NODE_NAME
@@ -276,6 +288,19 @@ def check_allowed_fields(fields):
         raise exception.NotAcceptable()
 
 
+def check_allowed_portgroup_fields(fields):
+    """Check if fetching a particular field of a portgroup is allowed.
+
+    This method checks if the required version is being requested for fields
+    that are only allowed to be fetched in a particular API version.
+    """
+    if fields is None:
+        return
+    if (('mode' in fields or 'properties' in fields) and
+            not allow_portgroup_mode_properties()):
+        raise exception.NotAcceptable()
+
+
 def check_allow_management_verbs(verb):
     min_version = MIN_VERB_VERSIONS.get(verb)
     if min_version is not None and pecan.request.version.minor < min_version:
@@ -427,6 +452,16 @@ def allow_remove_chassis_uuid():
             versions.MINOR_25_UNSET_CHASSIS_UUID)
 
 
+def allow_portgroup_mode_properties():
+    """Check if mode and properties can be added to/queried from a portgroup.
+
+    Version 1.26 of the API added mode and properties fields to portgroup
+    object.
+    """
+    return (pecan.request.version.minor >=
+            versions.MINOR_26_PORTGROUP_MODE_PROPERTIES)
+
+
 def get_controller_reserved_names(cls):
     """Get reserved names for a given controller.
 

ironic/api/controllers/v1/versions.py

@@ -56,6 +56,7 @@ BASE_VERSION = 1
 # v1.24: Add subcontrollers: node.portgroup, portgroup.ports.
 #        Add port.portgroup_uuid field.
 # v1.25: Add possibility to unset chassis_uuid from node.
+# v1.26: Add portgroup.mode and portgroup.properties.
 
 MINOR_0_JUNO = 0
 MINOR_1_INITIAL_VERSION = 1
@@ -83,11 +84,12 @@ MINOR_22_LOOKUP_HEARTBEAT = 22
 MINOR_23_PORTGROUPS = 23
 MINOR_24_PORTGROUPS_SUBCONTROLLERS = 24
 MINOR_25_UNSET_CHASSIS_UUID = 25
+MINOR_26_PORTGROUP_MODE_PROPERTIES = 26
 
 # 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_25_UNSET_CHASSIS_UUID
+MINOR_MAX_VERSION = MINOR_26_PORTGROUP_MODE_PROPERTIES
 
 # String representations of the minor and maximum versions
 MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)

ironic/conf/default.py

@@ -177,6 +177,16 @@ path_opts = [
                help=_("Top-level directory for maintaining ironic's state.")),
 ]
 
+portgroup_opts = [
+    cfg.StrOpt(
+        'default_portgroup_mode', default='active-backup',
+        help=_(
+            'Default mode for portgroups. Allowed values can be found in the '
+            'linux kernel documentation on bonding: '
+            'https://www.kernel.org/doc/Documentation/networking/bonding.txt.')
+    ),
+]
+
 service_opts = [
     cfg.StrOpt('host',
                default=socket.getfqdn(),
@@ -211,5 +221,6 @@ def register_opts(conf):
     conf.register_opts(netconf_opts)
     conf.register_opts(notification_opts)
     conf.register_opts(path_opts)
+    conf.register_opts(portgroup_opts)
     conf.register_opts(service_opts)
     conf.register_opts(utils_opts)

ironic/conf/opts.py

@@ -24,6 +24,7 @@ _default_opt_lists = [
     ironic.conf.default.netconf_opts,
     ironic.conf.default.notification_opts,
     ironic.conf.default.path_opts,
+    ironic.conf.default.portgroup_opts,
     ironic.conf.default.service_opts,
     ironic.conf.default.utils_opts,
 ]

ironic/db/sqlalchemy/alembic/versions/493d8f27f235_add_portgroup_configuration_fields.py

@@ -0,0 +1,40 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""add portgroup configuration fields
+
+Revision ID: 493d8f27f235
+Revises: 60cf717201bc
+Create Date: 2016-11-15 18:09:31.362613
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '493d8f27f235'
+down_revision = '1a59178ebdf6'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy import sql
+
+from ironic.conf import CONF
+
+
+def upgrade():
+    op.add_column('portgroups', sa.Column('properties', sa.Text(),
+                                          nullable=True))
+    op.add_column('portgroups', sa.Column('mode', sa.String(255)))
+
+    portgroups = sql.table('portgroups',
+                           sql.column('mode', sa.String(255)))
+    op.execute(
+        portgroups.update().values({'mode': CONF.default_portgroup_mode}))

ironic/db/sqlalchemy/api.py

@@ -568,6 +568,8 @@ class Connection(api.Connection):
     def create_portgroup(self, values):
         if not values.get('uuid'):
             values['uuid'] = uuidutils.generate_uuid()
+        if not values.get('mode'):
+            values['mode'] = CONF.default_portgroup_mode
 
         portgroup = models.Portgroup()
         portgroup.update(values)

ironic/db/sqlalchemy/models.py

@@ -191,6 +191,8 @@ class Portgroup(Base):
     extra = Column(db_types.JsonEncodedDict)
     internal_info = Column(db_types.JsonEncodedDict)
     standalone_ports_supported = Column(Boolean, default=True)
+    mode = Column(String(255))
+    properties = Column(db_types.JsonEncodedDict)
 
 
 class NodeTag(Base):

ironic/objects/portgroup.py

@@ -30,7 +30,8 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat):
     # Version 1.0: Initial version
     # Version 1.1: Add internal_info field
     # Version 1.2: Add standalone_ports_supported field
-    VERSION = '1.2'
+    # Version 1.3: Add mode and properties fields
+    VERSION = '1.3'
 
     dbapi = dbapi.get_instance()
 
@@ -43,6 +44,8 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat):
         'extra': object_fields.FlexibleDictField(nullable=True),
         'internal_info': object_fields.FlexibleDictField(nullable=True),
         'standalone_ports_supported': object_fields.BooleanField(),
+        'mode': object_fields.StringField(nullable=True),
+        'properties': object_fields.FlexibleDictField(nullable=True),
     }
 
     # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable

ironic/tests/unit/api/utils.py

@@ -139,7 +139,18 @@ def post_get_test_node(**kw):
 def portgroup_post_data(**kw):
     """Return a Portgroup object without internal attributes."""
     portgroup = utils.get_test_portgroup(**kw)
+
+    # node_id is not a part of the API object
     portgroup.pop('node_id')
+
+    # NOTE(jroll): pop out fields that were introduced in later API versions,
+    # unless explicitly requested. Otherwise, these will cause tests using
+    # older API versions to fail.
+    new_api_ver_arguments = ['mode', 'properties']
+    for arg in new_api_ver_arguments:
+        if arg not in kw:
+            portgroup.pop(arg)
+
     internal = portgroup_controller.PortgroupPatchType.internal_attrs()
     return remove_internal(portgroup, internal)
 

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

@@ -92,6 +92,17 @@ class TestListPortgroups(test_api_base.BaseApiTest):
         # We always append "links"
         self.assertItemsEqual(['address', 'extra', 'links'], data)
 
+    def test_get_one_mode_field_lower_api_version(self):
+        portgroup = obj_utils.create_test_portgroup(self.context,
+                                                    node_id=self.node.id)
+        headers = {api_base.Version.string: '1.25'}
+        fields = 'address,mode'
+        response = self.get_json(
+            '/portgroups/%s?fields=%s' % (portgroup.uuid, fields),
+            headers=headers, expect_errors=True)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
+        self.assertEqual('application/json', response.content_type)
+
     def test_get_collection_custom_fields(self):
         fields = 'uuid,extra'
         for i in range(3):
@@ -111,6 +122,16 @@ class TestListPortgroups(test_api_base.BaseApiTest):
             # We always append "links"
             self.assertItemsEqual(['uuid', 'extra', 'links'], portgroup)
 
+    def test_get_collection_properties_field_lower_api_version(self):
+        obj_utils.create_test_portgroup(self.context, node_id=self.node.id)
+        headers = {api_base.Version.string: '1.25'}
+        fields = 'address,properties'
+        response = self.get_json(
+            '/portgroups/?fields=%s' % fields,
+            headers=headers, expect_errors=True)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
+        self.assertEqual('application/json', response.content_type)
+
     def test_get_custom_fields_invalid_fields(self):
         portgroup = obj_utils.create_test_portgroup(self.context,
                                                     node_id=self.node.id)
@@ -358,7 +379,7 @@ class TestListPortgroups(test_api_base.BaseApiTest):
         self.assertEqual(sorted(portgroups), uuids)
 
     def test_sort_key_invalid(self):
-        invalid_keys_list = ['foo', 'extra']
+        invalid_keys_list = ['foo', 'extra', 'internal_info', 'properties']
         for invalid_key in invalid_keys_list:
             response = self.get_json('/portgroups?sort_key=%s' % invalid_key,
                                      expect_errors=True, headers=self.headers)
@@ -669,16 +690,17 @@ class TestPatch(test_api_base.BaseApiTest):
         self.assertTrue(response.json['error_message'])
         self.assertFalse(mock_upd.called)
 
-    def test_remove_mandatory_field(self, mock_upd):
+    def test_remove_address(self, mock_upd):
+        mock_upd.return_value = self.portgroup
+        mock_upd.return_value.address = None
         response = self.patch_json('/portgroups/%s' % self.portgroup.uuid,
                                    [{'path': '/address',
                                      'op': 'remove'}],
-                                   expect_errors=True,
                                    headers=self.headers)
         self.assertEqual('application/json', response.content_type)
-        self.assertEqual(http_client.BAD_REQUEST, response.status_code)
+        self.assertEqual(http_client.OK, response.status_code)
-        self.assertTrue(response.json['error_message'])
+        self.assertIsNone(response.json['address'])
-        self.assertFalse(mock_upd.called)
+        self.assertTrue(mock_upd.called)
 
     def test_add_root(self, mock_upd):
         address = 'aa:bb:cc:dd:ee:ff'
@@ -801,6 +823,39 @@ class TestPatch(test_api_base.BaseApiTest):
         self.assertTrue(response.json['error_message'])
         self.assertFalse(mock_upd.called)
 
+    def test_update_portgroup_mode_properties(self, mock_upd):
+        mock_upd.return_value = self.portgroup
+        mock_upd.return_value.mode = '802.3ad'
+        mock_upd.return_value.properties = {'bond_param': '100'}
+        response = self.patch_json('/portgroups/%s' % self.portgroup.uuid,
+                                   [{'path': '/mode',
+                                     'value': '802.3ad',
+                                     'op': 'add'},
+                                    {'path': '/properties/bond_param',
+                                     'value': '100',
+                                     'op': 'add'}],
+                                   headers=self.headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.OK, response.status_code)
+        self.assertEqual('802.3ad', response.json['mode'])
+        self.assertEqual({'bond_param': '100'}, response.json['properties'])
+
+    def _test_update_portgroup_mode_properties_bad_api_version(self, patch,
+                                                               mock_upd):
+        response = self.patch_json('/portgroups/%s' % self.portgroup.uuid,
+                                   patch, expect_errors=True,
+                                   headers={api_base.Version.string: '1.25'})
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
+        self.assertTrue(response.json['error_message'])
+        self.assertFalse(mock_upd.called)
+
+    def test_update_portgroup_mode_properties_bad_api_version(self, mock_upd):
+        self._test_update_portgroup_mode_properties_bad_api_version(
+            [{'path': '/mode', 'op': 'remove'}], mock_upd)
+        self._test_update_portgroup_mode_properties_bad_api_version(
+            [{'path': '/properties/abc', 'op': 'add', 'value': 123}], mock_upd)
+
 
 class TestPost(test_api_base.BaseApiTest):
     headers = {api_base.Version.string: str(api_v1.MAX_VER)}
@@ -890,14 +945,13 @@ class TestPost(test_api_base.BaseApiTest):
                                headers=self.headers)
         self.assertEqual(pdict['extra'], result['extra'])
 
-    def test_create_portgroup_no_mandatory_field_address(self):
+    def test_create_portgroup_no_address(self):
         pdict = apiutils.post_get_test_portgroup()
         del pdict['address']
-        response = self.post_json('/portgroups', pdict, expect_errors=True,
+        self.post_json('/portgroups', pdict, headers=self.headers)
-                                  headers=self.headers)
+        result = self.get_json('/portgroups/%s' % pdict['uuid'],
-        self.assertEqual(http_client.BAD_REQUEST, response.status_int)
+                               headers=self.headers)
-        self.assertEqual('application/json', response.content_type)
+        self.assertIsNone(result['address'])
-        self.assertTrue(response.json['error_message'])
 
     def test_create_portgroup_no_mandatory_field_node_uuid(self):
         pdict = apiutils.post_get_test_portgroup()
@@ -999,6 +1053,32 @@ class TestPost(test_api_base.BaseApiTest):
         self.assertEqual('application/json', response.content_type)
         self.assertTrue(response.json['error_message'])
 
+    def test_create_portgroup_mode_old_api_version(self):
+        for kwarg in [{'mode': '802.3ad'}, {'properties': {'bond_prop': 123}}]:
+            pdict = apiutils.post_get_test_portgroup(**kwarg)
+            response = self.post_json(
+                '/portgroups', pdict, expect_errors=True,
+                headers={api_base.Version.string: '1.25'})
+            self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int)
+            self.assertEqual('application/json', response.content_type)
+            self.assertTrue(response.json['error_message'])
+
+    def test_create_portgroup_mode_properties(self):
+        mode = '802.3ad'
+        props = {'bond_prop': 123}
+        pdict = apiutils.post_get_test_portgroup(mode=mode, properties=props)
+        self.post_json('/portgroups', pdict,
+                       headers={api_base.Version.string: '1.26'})
+        portgroup = self.dbapi.get_portgroup_by_uuid(pdict['uuid'])
+        self.assertEqual((mode, props), (portgroup.mode, portgroup.properties))
+
+    def test_create_portgroup_default_mode(self):
+        pdict = apiutils.post_get_test_portgroup()
+        self.post_json('/portgroups', pdict,
+                       headers={api_base.Version.string: '1.26'})
+        portgroup = self.dbapi.get_portgroup_by_uuid(pdict['uuid'])
+        self.assertEqual('active-backup', portgroup.mode)
+
 
 @mock.patch.object(rpcapi.ConductorAPI, 'destroy_portgroup')
 class TestDelete(test_api_base.BaseApiTest):

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

@@ -107,6 +107,25 @@ class TestApiUtils(base.TestCase):
         value = utils.is_path_removed(patch, path)
         self.assertFalse(value)
 
+    def test_is_path_updated_success(self):
+        patch = [{'path': '/name', 'op': 'remove'}]
+        path = '/name'
+        value = utils.is_path_updated(patch, path)
+        self.assertTrue(value)
+
+    def test_is_path_updated_subpath_success(self):
+        patch = [{'path': '/properties/switch_id', 'op': 'add', 'value': 'id'}]
+        path = '/properties'
+        value = utils.is_path_updated(patch, path)
+        self.assertTrue(value)
+
+    def test_is_path_updated_similar_subpath(self):
+        patch = [{'path': '/properties2/switch_id',
+                  'op': 'replace', 'value': 'spam'}]
+        path = '/properties'
+        value = utils.is_path_updated(patch, path)
+        self.assertFalse(value)
+
     def test_check_for_invalid_fields(self):
         requested = ['field_1', 'field_3']
         supported = ['field_1', 'field_2', 'field_3']
@@ -158,6 +177,28 @@ class TestApiUtils(base.TestCase):
             utils.check_allowed_fields,
             ['resource_class'])
 
+    @mock.patch.object(pecan, 'request', spec_set=['version'])
+    def test_check_allowed_portgroup_fields_mode_properties(self,
+                                                            mock_request):
+        mock_request.version.minor = 26
+        self.assertIsNone(
+            utils.check_allowed_portgroup_fields(['mode']))
+        self.assertIsNone(
+            utils.check_allowed_portgroup_fields(['properties']))
+
+    @mock.patch.object(pecan, 'request', spec_set=['version'])
+    def test_check_allowed_portgroup_fields_mode_properties_fail(self,
+                                                                 mock_request):
+        mock_request.version.minor = 25
+        self.assertRaises(
+            exception.NotAcceptable,
+            utils.check_allowed_portgroup_fields,
+            ['mode'])
+        self.assertRaises(
+            exception.NotAcceptable,
+            utils.check_allowed_portgroup_fields,
+            ['properties'])
+
     @mock.patch.object(pecan, 'request', spec_set=['version'])
     def test_check_allow_specify_driver(self, mock_request):
         mock_request.version.minor = 16
@@ -313,6 +354,13 @@ class TestApiUtils(base.TestCase):
         mock_request.version.minor = 24
         self.assertFalse(utils.allow_remove_chassis_uuid())
 
+    @mock.patch.object(pecan, 'request', spec_set=['version'])
+    def test_allow_portgroup_mode_properties(self, mock_request):
+        mock_request.version.minor = 26
+        self.assertTrue(utils.allow_portgroup_mode_properties())
+        mock_request.version.minor = 25
+        self.assertFalse(utils.allow_portgroup_mode_properties())
+
 
 class TestNodeIdent(base.TestCase):
 

ironic/tests/unit/db/sqlalchemy/test_migrations.py

@@ -51,6 +51,7 @@ import sqlalchemy
 import sqlalchemy.exc
 
 from ironic.common.i18n import _LE
+from ironic.conf import CONF
 from ironic.db.sqlalchemy import migration
 from ironic.db.sqlalchemy import models
 from ironic.tests import base
@@ -601,6 +602,27 @@ class MigrationCheckersMixin(object):
         self.assertIsInstance(targets.c.volume_id.type,
                               sqlalchemy.types.String)
 
+    def _pre_upgrade_493d8f27f235(self, engine):
+        portgroups = db_utils.get_table(engine, 'portgroups')
+        data = [{'uuid': uuidutils.generate_uuid()},
+                {'uuid': uuidutils.generate_uuid()}]
+        portgroups.insert().values(data).execute()
+        return data
+
+    def _check_493d8f27f235(self, engine, data):
+        portgroups = db_utils.get_table(engine, 'portgroups')
+        col_names = [column.name for column in portgroups.c]
+        self.assertIn('properties', col_names)
+        self.assertIsInstance(portgroups.c.properties.type,
+                              sqlalchemy.types.TEXT)
+        self.assertIn('mode', col_names)
+        self.assertIsInstance(portgroups.c.mode.type,
+                              sqlalchemy.types.String)
+
+        result = engine.execute(portgroups.select())
+        for row in result:
+            self.assertEqual(CONF.default_portgroup_mode, row['mode'])
+
     def test_upgrade_and_version(self):
         with patch_with_engine(self.engine):
             self.migration_api.upgrade('head')

ironic/tests/unit/db/test_portgroups.py

@@ -200,3 +200,14 @@ class DbportgroupTestCase(base.DbTestCase):
                           node_id=self.node.id,
                           name=str(uuidutils.generate_uuid()),
                           address='aa:bb:cc:33:11:22')
+
+    def test_create_portgroup_no_mode(self):
+        self.config(default_portgroup_mode='802.3ad')
+        name = uuidutils.generate_uuid()
+        db_utils.create_test_portgroup(uuid=uuidutils.generate_uuid(),
+                                       node_id=self.node.id, name=name,
+                                       address='aa:bb:cc:dd:ee:ff')
+        res = self.dbapi.get_portgroup_by_id(self.portgroup.id)
+        self.assertEqual('active-backup', res.mode)
+        res = self.dbapi.get_portgroup_by_name(name)
+        self.assertEqual('802.3ad', res.mode)

ironic/tests/unit/db/utils.py

@@ -465,6 +465,8 @@ def get_test_portgroup(**kw):
         'internal_info': kw.get('internal_info', {"bar": "buzz"}),
         'standalone_ports_supported': kw.get('standalone_ports_supported',
                                              True),
+        'mode': kw.get('mode'),
+        'properties': kw.get('properties', {}),
     }
 
 

ironic/tests/unit/objects/test_objects.py

@@ -408,7 +408,7 @@ expected_object_fingerprints = {
     'MyObj': '1.5-4f5efe8f0fcaf182bbe1c7fe3ba858db',
     'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905',
     'Port': '1.6-609504503d68982a10f495659990084b',
-    'Portgroup': '1.2-37b374b19bfd25db7e86aebc364e611e',
+    'Portgroup': '1.3-71923a81a86743b313b190f5c675e258',
     'Conductor': '1.1-5091f249719d4a465062a1b3dc7f860d',
     'EventType': '1.1-aa2ba1afd38553e3880c267404e8d370',
     'NotificationPublisher': '1.0-51a09397d6c0687771fb5be9a999605d',

releasenotes/notes/add-portgroup-config-fields-cd21e35e9c210733.yaml

@@ -0,0 +1,12 @@
+---
+features:
+  - Adds ``mode`` and ``properties`` fields in the portgroup object. Both of
+    them are optional and can be set from the API. They are available starting
+    with API microversion 1.26. If the ``mode`` field of a portgroup is not
+    specified in a POST request, its value will be set to the value of the
+    configuration option ``[DEFAULT]default_portgroup_mode``. The configuration
+    option ``[DEFAULT]default_portgroup_mode`` has a value of ``active-backup``
+    by default.
+fixes:
+  - |
+    ``address`` field of a portgroup is optional for all API microversions.