Add portgroups to support LAG interfaces - DB

Ironic should be able to provide the requisite connectivity
information to the Neutron ML2 plugin to allow drivers to
provision the top-of-rack switch for the bare metal server.
The addition of portgroups in Ironic allows the concept of
link aggregation to be handled in Ironic in order to provide
support for cases where multiple interfaces on the bare metal
server connect to switch ports of a single LAG.

This commit includes changes to:
- the DB models (extension of port model and addition of portgroup
  model)
- the DB tests

Partial-bug: #1526403
DocImpact
Co-Authored-By: Laura Moore (laura.moore@sap.com)
Co-Authored-By: Jenny Moorehead (jenny.moorehead@sap.com)
Co-Authored-By: Will Stevenson (will.stevenson@sap.com)

Change-Id: Ic028c316b0670c2d18de89111e83069f3441f476

ironic/common/exception.py

@@ -161,6 +161,18 @@ class PortAlreadyExists(Conflict):
     _msg_fmt = _("A port with UUID %(uuid)s already exists.")
 
 
+class PortgroupAlreadyExists(Conflict):
+    _msg_fmt = _("A portgroup with UUID %(uuid)s already exists.")
+
+
+class PortgroupDuplicateName(Conflict):
+    _msg_fmt = _("A portgroup with name %(name)s already exists.")
+
+
+class PortgroupMACAlreadyExists(Conflict):
+    _msg_fmt = _("A portgroup with MAC address %(mac)s already exists.")
+
+
 class InstanceAssociated(Conflict):
     _msg_fmt = _("Instance %(instance_uuid)s is already associated with a "
                  "node, it cannot be associated with this other node %(node)s")
@@ -255,6 +267,15 @@ class NodeNotFound(NotFound):
     _msg_fmt = _("Node %(node)s could not be found.")
 
 
+class PortgroupNotFound(NotFound):
+    _msg_fmt = _("Portgroup %(portgroup)s could not be found.")
+
+
+class PortgroupNotEmpty(Invalid):
+    _msg_fmt = _("Cannot complete the requested action because portgroup "
+                 "%(portgroup)s contains ports.")
+
+
 class NodeAssociated(InvalidState):
     _msg_fmt = _("Node %(node)s is associated with instance %(instance)s.")
 

ironic/db/api.py

@@ -260,6 +260,21 @@ class Connection(object):
         :returns: A list of ports.
         """
 
+    @abc.abstractmethod
+    def get_ports_by_portgroup_id(self, portgroup_id, limit=None, marker=None,
+                                  sort_key=None, sort_dir=None):
+        """List all the ports for a given portgroup.
+
+        :param portgroup_id: The integer portgroup ID.
+        :param limit: Maximum number of ports to return.
+        :param marker: The last item of the previous page; we return the next
+                       result set.
+        :param sort_key: Attribute by which results should be sorted
+        :param sort_dir: Direction in which results should be sorted
+                         (asc, desc)
+        :returns: A list of ports.
+        """
+
     @abc.abstractmethod
     def create_port(self, values):
         """Create a new port.
@@ -283,6 +298,120 @@ class Connection(object):
         :param port_id: The id or MAC of a port.
         """
 
+    @abc.abstractmethod
+    def get_portgroup_by_id(self, portgroup_id):
+        """Return a network portgroup representation.
+
+        :param portgroup_id: The id of a portgroup.
+        :returns: A portgroup.
+        :raises: PortgroupNotFound
+        """
+
+    @abc.abstractmethod
+    def get_portgroup_by_uuid(self, portgroup_uuid):
+        """Return a network portgroup representation.
+
+        :param portgroup_uuid: The uuid of a portgroup.
+        :returns: A portgroup.
+        :raises: PortgroupNotFound
+        """
+
+    @abc.abstractmethod
+    def get_portgroup_by_address(self, address):
+        """Return a network portgroup representation.
+
+        :param address: The MAC address of a portgroup.
+        :returns: A portgroup.
+        :raises: PortgroupNotFound
+        """
+
+    @abc.abstractmethod
+    def get_portgroup_by_name(self, name):
+        """Return a network portgroup representation.
+
+        :param name: The logical name of a portgroup.
+        :returns: A portgroup.
+        :raises: PortgroupNotFound
+        """
+
+    @abc.abstractmethod
+    def get_portgroup_list(self, limit=None, marker=None,
+                           sort_key=None, sort_dir=None):
+        """Return a list of portgroups.
+
+        :param limit: Maximum number of portgroups to return.
+        :param marker: The last item of the previous page; we return the next
+                       result set.
+        :param sort_key: Attribute by which results should be sorted.
+        :param sort_dir: Direction in which results should be sorted.
+                         (asc, desc)
+        :returns: A list of portgroups.
+        """
+
+    @abc.abstractmethod
+    def get_portgroups_by_node_id(self, node_id, limit=None, marker=None,
+                                  sort_key=None, sort_dir=None):
+        """List all the portgroups for a given node.
+
+        :param node_id: The integer node ID.
+        :param limit: Maximum number of portgroups to return.
+        :param marker: The last item of the previous page; we return the next
+                       result set.
+        :param sort_key: Attribute by which results should be sorted
+        :param sort_dir: Direction in which results should be sorted
+                         (asc, desc)
+        :returns: A list of portgroups.
+        """
+
+    @abc.abstractmethod
+    def create_portgroup(self, values):
+        """Create a new portgroup.
+
+        :param values: Dict of values with the the following keys:
+                       'id'
+                       'uuid'
+                       'name'
+                       'node_id'
+                       'address'
+                       'extra'
+                       'created_at'
+                       'updated_at'
+        :returns: A portgroup
+        :raises: PortgroupDuplicateName
+        :raises: PortgroupMACAlreadyExists
+        :raises: PortgroupAlreadyExists
+        """
+
+    @abc.abstractmethod
+    def update_portgroup(self, portgroup_id, values):
+        """Update properties of a portgroup.
+
+        :param portgroup_id: The UUID or MAC of a portgroup.
+        :param values: Dict of values to update.
+                       May contain the following keys:
+                       'uuid'
+                       'name'
+                       'node_id'
+                       'address'
+                       'extra'
+                       'created_at'
+                       'updated_at'
+        :returns: A portgroup.
+        :raises: InvalidParameterValue
+        :raises: PortgroupNotFound
+        :raises: PortgroupDuplicateName
+        :raises: PortgroupMACAlreadyExists
+        """
+
+    @abc.abstractmethod
+    def destroy_portgroup(self, portgroup_id):
+        """Destroy a portgroup.
+
+        :param portgroup_id: The UUID or MAC of a portgroup.
+        :raises: PortgroupNotEmpty
+        :raises: PortgroupNotFound
+        """
+
     @abc.abstractmethod
     def create_chassis(self, values):
         """Create a new chassis.

ironic/db/sqlalchemy/alembic/versions/5ea1b0d310e_added_port_group_table_and_altered_ports.py

@@ -0,0 +1,54 @@
+#    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.
+
+"""Added portgroups table and altered ports
+
+Revision ID: 5ea1b0d310e
+Revises: 48d6c242bb9b
+Create Date: 2015-06-30 14:14:26.972368
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '5ea1b0d310e'
+down_revision = '48d6c242bb9b'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    op.create_table('portgroups',
+                    sa.Column('created_at', sa.DateTime(), nullable=True),
+                    sa.Column('updated_at', sa.DateTime(), nullable=True),
+                    sa.Column('id', sa.Integer(), nullable=False),
+                    sa.Column('uuid', sa.String(length=36), nullable=True),
+                    sa.Column('name', sa.String(length=255), nullable=True),
+                    sa.Column('node_id', sa.Integer(), nullable=True),
+                    sa.Column('address', sa.String(length=18), nullable=True),
+                    sa.Column('extra', sa.Text(), nullable=True),
+                    sa.ForeignKeyConstraint(['node_id'], ['nodes.id'], ),
+                    sa.PrimaryKeyConstraint('id'),
+                    sa.UniqueConstraint('uuid', name='uniq_portgroups0uuid'),
+                    sa.UniqueConstraint('address',
+                                        name='uniq_portgroups0address'),
+                    sa.UniqueConstraint('name', name='uniq_portgroups0name'),
+                    mysql_ENGINE='InnoDB',
+                    mysql_DEFAULT_CHARSET='UTF8')
+    op.add_column(u'ports', sa.Column('local_link_connection', sa.Text(),
+                                      nullable=True))
+    op.add_column(u'ports', sa.Column('portgroup_id', sa.Integer(),
+                                      nullable=True))
+    op.add_column(u'ports', sa.Column('pxe_enabled', sa.Boolean(),
+                                      default=True))
+    op.create_foreign_key('fk_portgroups_ports', 'ports', 'portgroups',
+                          ['portgroup_id'], ['id'])

ironic/db/sqlalchemy/api.py

@@ -117,6 +117,40 @@ def add_port_filter_by_node(query, value):
         return query.filter(models.Node.uuid == value)
 
 
+def add_portgroup_filter(query, value):
+    """Adds a portgroup-specific filter to a query.
+
+    Filters results by address, if supplied value is a valid MAC
+    address. Otherwise attempts to filter results by identity.
+
+    :param query: Initial query to add filter to.
+    :param value: Value for filtering results by.
+    :return: Modified query.
+    """
+    if utils.is_valid_mac(value):
+        return query.filter_by(address=value)
+    else:
+        return add_identity_filter(query, value)
+
+
+def add_portgroup_filter_by_node(query, value):
+    if strutils.is_int_like(value):
+        return query.filter_by(node_id=value)
+    else:
+        query = query.join(models.Node,
+                           models.Portgroup.node_id == models.Node.id)
+        return query.filter(models.Node.uuid == value)
+
+
+def add_port_filter_by_portgroup(query, value):
+    if strutils.is_int_like(value):
+        return query.filter_by(portgroup_id=value)
+    else:
+        query = query.join(models.Portgroup,
+                           models.Port.portgroup_id == models.Portgroup.id)
+        return query.filter(models.Portgroup.uuid == value)
+
+
 def add_node_filter_by_chassis(query, value):
     if strutils.is_int_like(value):
         return query.filter_by(chassis_id=value)
@@ -325,6 +359,11 @@ class Connection(api.Connection):
             port_query = add_port_filter_by_node(port_query, node_id)
             port_query.delete()
 
+            portgroup_query = model_query(models.Portgroup)
+            portgroup_query = add_portgroup_filter_by_node(portgroup_query,
+                                                           node_id)
+            portgroup_query.delete()
+
             query.delete()
 
     def update_node(self, node_id, values):
@@ -410,6 +449,13 @@ class Connection(api.Connection):
         return _paginate_query(models.Port, limit, marker,
                                sort_key, sort_dir, query)
 
+    def get_ports_by_portgroup_id(self, portgroup_id, limit=None, marker=None,
+                                  sort_key=None, sort_dir=None):
+        query = model_query(models.Port)
+        query = query.filter_by(portgroup_id=portgroup_id)
+        return _paginate_query(models.Port, limit, marker,
+                               sort_key, sort_dir, query)
+
     def create_port(self, values):
         if not values.get('uuid'):
             values['uuid'] = uuidutils.generate_uuid()
@@ -453,6 +499,109 @@ class Connection(api.Connection):
             if count == 0:
                 raise exception.PortNotFound(port=port_id)
 
+    def get_portgroup_by_id(self, portgroup_id):
+        query = model_query(models.Portgroup).filter_by(id=portgroup_id)
+        try:
+            return query.one()
+        except NoResultFound:
+            raise exception.PortgroupNotFound(portgroup=portgroup_id)
+
+    def get_portgroup_by_uuid(self, portgroup_uuid):
+        query = model_query(models.Portgroup).filter_by(uuid=portgroup_uuid)
+        try:
+            return query.one()
+        except NoResultFound:
+            raise exception.PortgroupNotFound(portgroup=portgroup_uuid)
+
+    def get_portgroup_by_address(self, address):
+        query = model_query(models.Portgroup).filter_by(address=address)
+        try:
+            return query.one()
+        except NoResultFound:
+            raise exception.PortgroupNotFound(portgroup=address)
+
+    def get_portgroup_by_name(self, name):
+        query = model_query(models.Portgroup).filter_by(name=name)
+        try:
+            return query.one()
+        except NoResultFound:
+            raise exception.PortgroupNotFound(portgroup=name)
+
+    def get_portgroup_list(self, limit=None, marker=None,
+                           sort_key=None, sort_dir=None):
+        return _paginate_query(models.Portgroup, limit, marker,
+                               sort_key, sort_dir)
+
+    def get_portgroups_by_node_id(self, node_id, limit=None, marker=None,
+                                  sort_key=None, sort_dir=None):
+        query = model_query(models.Portgroup)
+        query = query.filter_by(node_id=node_id)
+        return _paginate_query(models.Portgroup, limit, marker,
+                               sort_key, sort_dir, query)
+
+    def create_portgroup(self, values):
+        if not values.get('uuid'):
+            values['uuid'] = uuidutils.generate_uuid()
+
+        portgroup = models.Portgroup()
+        portgroup.update(values)
+        with _session_for_write() as session:
+            try:
+                session.add(portgroup)
+                session.flush()
+            except db_exc.DBDuplicateEntry as exc:
+                if 'name' in exc.columns:
+                    raise exception.PortgroupDuplicateName(name=values['name'])
+                elif 'address' in exc.columns:
+                    raise exception.PortgroupMACAlreadyExists(
+                        mac=values['address'])
+                raise exception.PortgroupAlreadyExists(uuid=values['uuid'])
+            return portgroup
+
+    def update_portgroup(self, portgroup_id, values):
+        if 'uuid' in values:
+            msg = _("Cannot overwrite UUID for an existing portgroup.")
+            raise exception.InvalidParameterValue(err=msg)
+
+        with _session_for_write() as session:
+            try:
+                query = model_query(models.Portgroup)
+                query = add_portgroup_filter(query, portgroup_id)
+                ref = query.one()
+                ref.update(values)
+                session.flush()
+            except NoResultFound:
+                raise exception.PortgroupNotFound(portgroup=portgroup_id)
+            except db_exc.DBDuplicateEntry as exc:
+                if 'name' in exc.columns:
+                    raise exception.PortgroupDuplicateName(name=values['name'])
+                elif 'address' in exc.columns:
+                    raise exception.PortgroupMACAlreadyExists(
+                        mac=values['address'])
+                else:
+                    raise exc
+            return ref
+
+    def destroy_portgroup(self, portgroup_id):
+        def portgroup_not_empty(session):
+            """Checks whether the portgroup does not have ports."""
+
+            query = model_query(models.Port)
+            query = add_port_filter_by_portgroup(query, portgroup_id)
+
+            return query.count() != 0
+
+        with _session_for_write() as session:
+            if portgroup_not_empty(session):
+                raise exception.PortgroupNotEmpty(portgroup=portgroup_id)
+
+            query = model_query(models.Portgroup, session=session)
+            query = add_identity_filter(query, portgroup_id)
+
+            count = query.delete()
+            if count == 0:
+                raise exception.PortgroupNotFound(portgroup=portgroup_id)
+
     def get_chassis_by_id(self, chassis_id):
         query = model_query(models.Chassis).filter_by(id=chassis_id)
         try:

ironic/db/sqlalchemy/models.py

@@ -165,6 +165,26 @@ class Port(Base):
     address = Column(String(18))
     node_id = Column(Integer, ForeignKey('nodes.id'), nullable=True)
     extra = Column(db_types.JsonEncodedDict)
+    local_link_connection = Column(db_types.JsonEncodedDict)
+    portgroup_id = Column(Integer, ForeignKey('portgroups.id'), nullable=True)
+    pxe_enabled = Column(Boolean, default=True)
+
+
+class Portgroup(Base):
+    """Represents a group of network ports of a bare metal node."""
+
+    __tablename__ = 'portgroups'
+    __table_args__ = (
+        schema.UniqueConstraint('uuid', name='uniq_portgroups0uuid'),
+        schema.UniqueConstraint('address', name='uniq_portgroups0address'),
+        schema.UniqueConstraint('name', name='uniq_portgroups0name'),
+        table_args())
+    id = Column(Integer, primary_key=True)
+    uuid = Column(String(36))
+    name = Column(String(255), nullable=True)
+    node_id = Column(Integer, ForeignKey('nodes.id'), nullable=True)
+    address = Column(String(18))
+    extra = Column(db_types.JsonEncodedDict)
 
 
 class NodeTag(Base):

ironic/tests/unit/api/utils.py

@@ -103,6 +103,11 @@ def port_post_data(**kw):
     port = utils.get_test_port(**kw)
     # node_id is not part of the API object
     port.pop('node_id')
+    # TODO(vsaienko): remove when API part is added
+    port.pop('local_link_connection')
+    port.pop('pxe_enabled')
+    # portgroup_id is not part of the API object
+    port.pop('portgroup_id')
     internal = port_controller.PortPatchType.internal_attrs()
     return remove_internal(port, internal)
 

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

@@ -407,6 +407,43 @@ class MigrationCheckersMixin(object):
         tag = node_tags.select(node_tags.c.node_id == '123').execute().first()
         self.assertEqual('tag1', tag['tag'])
 
+    def _check_5ea1b0d310e(self, engine, data):
+        portgroup = db_utils.get_table(engine, 'portgroups')
+        col_names = [column.name for column in portgroup.c]
+        expected_names = ['created_at', 'updated_at', 'id', 'uuid', 'name',
+                          'node_id', 'address', 'extra']
+        self.assertEqual(sorted(expected_names), sorted(col_names))
+
+        self.assertIsInstance(portgroup.c.created_at.type,
+                              sqlalchemy.types.DateTime)
+        self.assertIsInstance(portgroup.c.updated_at.type,
+                              sqlalchemy.types.DateTime)
+        self.assertIsInstance(portgroup.c.id.type,
+                              sqlalchemy.types.Integer)
+        self.assertIsInstance(portgroup.c.uuid.type,
+                              sqlalchemy.types.String)
+        self.assertIsInstance(portgroup.c.name.type,
+                              sqlalchemy.types.String)
+        self.assertIsInstance(portgroup.c.node_id.type,
+                              sqlalchemy.types.Integer)
+        self.assertIsInstance(portgroup.c.address.type,
+                              sqlalchemy.types.String)
+        self.assertIsInstance(portgroup.c.extra.type,
+                              sqlalchemy.types.TEXT)
+
+        ports = db_utils.get_table(engine, 'ports')
+        col_names = [column.name for column in ports.c]
+        self.assertIn('pxe_enabled', col_names)
+        self.assertIn('portgroup_id', col_names)
+        self.assertIn('local_link_connection', col_names)
+        self.assertIsInstance(ports.c.portgroup_id.type,
+                              sqlalchemy.types.Integer)
+        # in some backends bool type is integer
+        self.assertTrue(isinstance(ports.c.pxe_enabled.type,
+                                   sqlalchemy.types.Boolean) or
+                        isinstance(ports.c.pxe_enabled.type,
+                                   sqlalchemy.types.Integer))
+
     def test_upgrade_and_version(self):
         with patch_with_engine(self.engine):
             self.migration_api.upgrade('head')

ironic/tests/unit/db/test_portgroups.py

@@ -0,0 +1,202 @@
+#    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.
+
+"""Tests for manipulating portgroups via the DB API"""
+
+from oslo_utils import uuidutils
+import six
+
+from ironic.common import exception
+from ironic.tests.unit.db import base
+from ironic.tests.unit.db import utils as db_utils
+
+
+class DbportgroupTestCase(base.DbTestCase):
+
+    def setUp(self):
+        # This method creates a portgroup for every test and
+        # replaces a test for creating a portgroup.
+        super(DbportgroupTestCase, self).setUp()
+        self.node = db_utils.create_test_node()
+        self.portgroup = db_utils.create_test_portgroup(node_id=self.node.id)
+
+    def _create_test_portgroup_range(self, count):
+        """Create the specified number of test portgroup entries in DB
+
+        It uses create_test_portgroup method. And returns List of Portgroup
+        DB objects.
+
+        :param count: Specifies the number of portgroups to be created
+        :returns: List of Portgroup DB objects
+
+        """
+        uuids = []
+        for i in range(1, count):
+            portgroup = db_utils.create_test_portgroup(
+                uuid=uuidutils.generate_uuid(),
+                name='portgroup' + str(i),
+                address='52:54:00:cf:2d:4%s' % i)
+            uuids.append(six.text_type(portgroup.uuid))
+
+        return uuids
+
+    def test_get_portgroup_by_id(self):
+        res = self.dbapi.get_portgroup_by_id(self.portgroup.id)
+        self.assertEqual(self.portgroup.address, res.address)
+
+    def test_get_portgroup_by_id_that_does_not_exist(self):
+        self.assertRaises(exception.PortgroupNotFound,
+                          self.dbapi.get_portgroup_by_id, 99)
+
+    def test_get_portgroup_by_uuid(self):
+        res = self.dbapi.get_portgroup_by_uuid(self.portgroup.uuid)
+        self.assertEqual(self.portgroup.id, res.id)
+
+    def test_get_portgroup_by_uuid_that_does_not_exist(self):
+        self.assertRaises(exception.PortgroupNotFound,
+                          self.dbapi.get_portgroup_by_uuid,
+                          'EEEEEEEE-EEEE-EEEE-EEEE-EEEEEEEEEEEE')
+
+    def test_get_portgroup_by_address(self):
+        res = self.dbapi.get_portgroup_by_address(self.portgroup.address)
+        self.assertEqual(self.portgroup.id, res.id)
+
+    def test_get_portgroup_by_address_that_does_not_exist(self):
+        self.assertRaises(exception.PortgroupNotFound,
+                          self.dbapi.get_portgroup_by_address,
+                          '31:31:31:31:31:31')
+
+    def test_get_portgroup_by_name(self):
+        res = self.dbapi.get_portgroup_by_name(self.portgroup.name)
+        self.assertEqual(self.portgroup.id, res.id)
+
+    def test_get_portgroup_by_name_that_does_not_exist(self):
+        self.assertRaises(exception.PortgroupNotFound,
+                          self.dbapi.get_portgroup_by_name, 'testfail')
+
+    def test_get_portgroup_list(self):
+        uuids = self._create_test_portgroup_range(6)
+
+        # Also add the uuid for the portgroup created in setUp()
+        uuids.append(six.text_type(self.portgroup.uuid))
+        res = self.dbapi.get_portgroup_list()
+        res_uuids = [r.uuid for r in res]
+        six.assertCountEqual(self, uuids, res_uuids)
+
+    def test_get_portgroup_list_sorted(self):
+        uuids = self._create_test_portgroup_range(6)
+
+        # Also add the uuid for the portgroup created in setUp()
+        uuids.append(six.text_type(self.portgroup.uuid))
+        res = self.dbapi.get_portgroup_list(sort_key='uuid')
+        res_uuids = [r.uuid for r in res]
+        self.assertEqual(sorted(uuids), res_uuids)
+
+        self.assertRaises(exception.InvalidParameterValue,
+                          self.dbapi.get_portgroup_list, sort_key='foo')
+
+    def test_get_portgroups_by_node_id(self):
+        res = self.dbapi.get_portgroups_by_node_id(self.node.id)
+        self.assertEqual(self.portgroup.address, res[0].address)
+
+    def test_get_portgroups_by_node_id_that_does_not_exist(self):
+        self.assertEqual([], self.dbapi.get_portgroups_by_node_id(99))
+
+    def test_destroy_portgroup(self):
+        self.dbapi.destroy_portgroup(self.portgroup.id)
+        self.assertRaises(exception.PortgroupNotFound,
+                          self.dbapi.get_portgroup_by_id, self.portgroup.id)
+
+    def test_destroy_portgroup_that_does_not_exist(self):
+        self.assertRaises(exception.PortgroupNotFound,
+                          self.dbapi.destroy_portgroup, 99)
+
+    def test_destroy_portgroup_uuid(self):
+        self.dbapi.destroy_portgroup(self.portgroup.uuid)
+
+    def test_destroy_portgroup_not_empty(self):
+        self.port = db_utils.create_test_port(node_id=self.node.id,
+                                              portgroup_id=self.portgroup.id)
+        self.assertRaises(exception.PortgroupNotEmpty,
+                          self.dbapi.destroy_portgroup, self.portgroup.id)
+
+    def test_update_portgroup(self):
+        old_address = self.portgroup.address
+        new_address = 'ff:ee:dd:cc:bb:aa'
+        self.assertNotEqual(old_address, new_address)
+        old_name = self.portgroup.name
+        new_name = 'newname'
+        self.assertNotEqual(old_name, new_name)
+        res = self.dbapi.update_portgroup(self.portgroup.id,
+                                          {'address': new_address,
+                                           'name': new_name})
+        self.assertEqual(new_address, res.address)
+        self.assertEqual(new_name, res.name)
+
+    def test_update_portgroup_uuid(self):
+        self.assertRaises(exception.InvalidParameterValue,
+                          self.dbapi.update_portgroup, self.portgroup.id,
+                          {'uuid': ''})
+
+    def test_update_portgroup_not_found(self):
+        id_2 = 99
+        self.assertNotEqual(self.portgroup.id, id_2)
+        address2 = 'aa:bb:cc:11:22:33'
+        self.assertRaises(exception.PortgroupNotFound,
+                          self.dbapi.update_portgroup, id_2,
+                          {'address': address2})
+
+    def test_update_portgroup_duplicated_address(self):
+        address1 = self.portgroup.address
+        address2 = 'aa:bb:cc:11:22:33'
+        portgroup2 = db_utils.create_test_portgroup(
+            uuid=uuidutils.generate_uuid(),
+            node_id=self.node.id,
+            name=str(uuidutils.generate_uuid()),
+            address=address2)
+        self.assertRaises(exception.PortgroupMACAlreadyExists,
+                          self.dbapi.update_portgroup, portgroup2.id,
+                          {'address': address1})
+
+    def test_update_portgroup_duplicated_name(self):
+        name1 = self.portgroup.name
+        portgroup2 = db_utils.create_test_portgroup(
+            uuid=uuidutils.generate_uuid(),
+            node_id=self.node.id,
+            name='name2', address='aa:bb:cc:11:22:55')
+        self.assertRaises(exception.PortgroupDuplicateName,
+                          self.dbapi.update_portgroup, portgroup2.id,
+                          {'name': name1})
+
+    def test_create_portgroup_duplicated_name(self):
+        self.assertRaises(exception.PortgroupDuplicateName,
+                          db_utils.create_test_portgroup,
+                          uuid=uuidutils.generate_uuid(),
+                          node_id=self.node.id,
+                          name=self.portgroup.name,
+                          address='aa:bb:cc:11:22:55')
+
+    def test_create_portgroup_duplicated_address(self):
+        self.assertRaises(exception.PortgroupMACAlreadyExists,
+                          db_utils.create_test_portgroup,
+                          uuid=uuidutils.generate_uuid(),
+                          node_id=self.node.id,
+                          name=str(uuidutils.generate_uuid()),
+                          address=self.portgroup.address)
+
+    def test_create_portgroup_duplicated_uuid(self):
+        self.assertRaises(exception.PortgroupAlreadyExists,
+                          db_utils.create_test_portgroup,
+                          uuid=self.portgroup.uuid,
+                          node_id=self.node.id,
+                          name=str(uuidutils.generate_uuid()),
+                          address='aa:bb:cc:33:11:22')

ironic/tests/unit/db/test_ports.py

@@ -30,7 +30,9 @@ class DbPortTestCase(base.DbTestCase):
         # replaces a test for creating a port.
         super(DbPortTestCase, self).setUp()
         self.node = db_utils.create_test_node()
-        self.port = db_utils.create_test_port(node_id=self.node.id)
+        self.portgroup = db_utils.create_test_portgroup(node_id=self.node.id)
+        self.port = db_utils.create_test_port(node_id=self.node.id,
+                                              portgroup_id=self.portgroup.id)
 
     def test_get_port_by_id(self):
         res = self.dbapi.get_port_by_id(self.port.id)
@@ -78,6 +80,13 @@ class DbPortTestCase(base.DbTestCase):
     def test_get_ports_by_node_id_that_does_not_exist(self):
         self.assertEqual([], self.dbapi.get_ports_by_node_id(99))
 
+    def test_get_ports_by_portgroup_id(self):
+        res = self.dbapi.get_ports_by_portgroup_id(self.portgroup.id)
+        self.assertEqual(self.port.address, res[0].address)
+
+    def test_get_ports_by_portgroup_id_that_does_not_exist(self):
+        self.assertEqual([], self.dbapi.get_ports_by_portgroup_id(99))
+
     def test_destroy_port(self):
         self.dbapi.destroy_port(self.port.id)
         self.assertRaises(exception.PortNotFound,

ironic/tests/unit/db/utils.py

@@ -253,6 +253,12 @@ def get_test_port(**kw):
         'extra': kw.get('extra', {}),
         'created_at': kw.get('created_at'),
         'updated_at': kw.get('updated_at'),
+        'local_link_connection': kw.get('local_link_connection',
+                                        {'switch_id': '0a:1b:2c:3d:4e:5f',
+                                         'port_id': 'Ethernet3/1',
+                                         'switch_info': 'switch1'}),
+        'portgroup_id': kw.get('portgroup_id'),
+        'pxe_enabled': kw.get('pxe_enabled', True),
     }
 
 
@@ -344,3 +350,33 @@ def get_test_oneview_driver_info():
     return {
         'server_hardware_uri': 'fake_sh_uri',
     }
+
+
+def get_test_portgroup(**kw):
+    return {
+        'id': kw.get('id', 654),
+        'uuid': kw.get('uuid', '6eb02b44-18a3-4659-8c0b-8d2802581ae4'),
+        'name': kw.get('name', 'fooname'),
+        'node_id': kw.get('node_id', 123),
+        'address': kw.get('address', '52:54:00:cf:2d:31'),
+        'extra': kw.get('extra', {}),
+        'created_at': kw.get('created_at'),
+        'updated_at': kw.get('updated_at'),
+    }
+
+
+def create_test_portgroup(**kw):
+    """Create test portgroup entry in DB and return Portgroup DB object.
+
+    Function to be used to create test Portgroup objects in the database.
+
+    :param kw: kwargs with overriding values for port's attributes.
+    :returns: Test Portgroup DB object.
+
+    """
+    portgroup = get_test_portgroup(**kw)
+    # Let DB generate ID if it isn't specified explicitly
+    if 'id' not in kw:
+        del portgroup['id']
+    dbapi = db_api.get_instance()
+    return dbapi.create_portgroup(portgroup)