Merge "Support port name"

12 files changed, 199 insertions, 14 deletions

diff --git a/ironic/common/exception.py b/ironic/common/exception.py
index ba636e154..6f9a7d331 100644
--- a/ironic/common/exception.py
+++ b/ironic/common/exception.py
@@ -77,6 +77,10 @@ class PortAlreadyExists(Conflict):
     _msg_fmt = _("A port with UUID %(uuid)s already exists.")
 
 
+class PortDuplicateName(Conflict):
+    _msg_fmt = _("A port with name %(name)s already exists.")
+
+
 class PortgroupAlreadyExists(Conflict):
     _msg_fmt = _("A portgroup with UUID %(uuid)s already exists.")
 
diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py
index c6ff25e6d..0e3d18992 100644
--- a/ironic/common/release_mappings.py
+++ b/ironic/common/release_mappings.py
@@ -293,7 +293,7 @@ RELEASE_MAPPING = {
             'Chassis': ['1.3'],
             'Deployment': ['1.0'],
             'DeployTemplate': ['1.1'],
-            'Port': ['1.9'],
+            'Port': ['1.10'],
             'Portgroup': ['1.4'],
             'Trait': ['1.0'],
             'TraitList': ['1.0'],
diff --git a/ironic/db/api.py b/ironic/db/api.py
index 48228773f..bee805f5c 100644
--- a/ironic/db/api.py
+++ b/ironic/db/api.py
@@ -252,6 +252,14 @@ class Connection(object, metaclass=abc.ABCMeta):
         """
 
     @abc.abstractmethod
+    def get_port_by_name(self, port_name):
+        """Return a network port representation.
+
+        :param port_name: The name of a port.
+        :returns: A port.
+        """
+
+    @abc.abstractmethod
     def get_port_list(self, limit=None, marker=None,
                       sort_key=None, sort_dir=None):
         """Return a list of ports.
diff --git a/ironic/db/sqlalchemy/alembic/versions/c0455649680c_port_name.py b/ironic/db/sqlalchemy/alembic/versions/c0455649680c_port_name.py
new file mode 100644
index 000000000..80023664b
--- /dev/null
+++ b/ironic/db/sqlalchemy/alembic/versions/c0455649680c_port_name.py
@@ -0,0 +1,33 @@
+#    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.
+
+"""port-name
+
+Revision ID: c0455649680c
+Revises: cf1a80fdb352
+Create Date: 2020-11-27 20:12:24.752897
+
+"""
+
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'c0455649680c'
+down_revision = 'cf1a80fdb352'
+
+
+def upgrade():
+    op.add_column('ports', sa.Column('name', sa.String(length=255),
+                                     nullable=True))
+    op.create_unique_constraint('uniq_ports0name', 'ports', ['name'])
diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py
index 7c478b009..7891122ce 100644
--- a/ironic/db/sqlalchemy/api.py
+++ b/ironic/db/sqlalchemy/api.py
@@ -705,6 +705,13 @@ class Connection(api.Connection):
         except NoResultFound:
             raise exception.PortNotFound(port=address)
 
+    def get_port_by_name(self, port_name):
+        query = model_query(models.Port).filter_by(name=port_name)
+        try:
+            return query.one()
+        except NoResultFound:
+            raise exception.PortNotFound(port=port_name)
+
     def get_port_list(self, limit=None, marker=None,
                       sort_key=None, sort_dir=None, owner=None,
                       project=None):
@@ -773,8 +780,11 @@ class Connection(api.Connection):
                 session.flush()
         except NoResultFound:
             raise exception.PortNotFound(port=port_id)
-        except db_exc.DBDuplicateEntry:
-            raise exception.MACAlreadyExists(mac=values['address'])
+        except db_exc.DBDuplicateEntry as exc:
+            if 'name' in exc.columns:
+                raise exception.PortDuplicateName(name=values['name'])
+            else:
+                raise exception.MACAlreadyExists(mac=values['address'])
         return ref
 
     @oslo_db_api.retry_on_deadlock
diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py
index 68b366f21..2072153e2 100644
--- a/ironic/db/sqlalchemy/models.py
+++ b/ironic/db/sqlalchemy/models.py
@@ -210,6 +210,7 @@ class Port(Base):
     __table_args__ = (
         schema.UniqueConstraint('address', name='uniq_ports0address'),
         schema.UniqueConstraint('uuid', name='uniq_ports0uuid'),
+        schema.UniqueConstraint('name', name='uniq_ports0name'),
         table_args())
     id = Column(Integer, primary_key=True)
     uuid = Column(String(36))
@@ -222,6 +223,7 @@ class Port(Base):
     internal_info = Column(db_types.JsonEncodedDict)
     physical_network = Column(String(64), nullable=True)
     is_smartnic = Column(Boolean, nullable=True, default=False)
+    name = Column(String(255), nullable=True)
 
 
 class Portgroup(Base):
diff --git a/ironic/objects/port.py b/ironic/objects/port.py
index 85690b162..b4d0e78eb 100644
--- a/ironic/objects/port.py
+++ b/ironic/objects/port.py
@@ -20,6 +20,7 @@ from oslo_utils import versionutils
 from oslo_versionedobjects import base as object_base
 
 from ironic.common import exception
+from ironic.common import utils
 from ironic.db import api as dbapi
 from ironic.objects import base
 from ironic.objects import fields as object_fields
@@ -42,7 +43,8 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
     #              internal_info['tenant_vif_port_id'] (not an explicit db
     #              change)
     # Version 1.9: Add support for Smart NIC port
-    VERSION = '1.9'
+    # Version 1.10: Add name field
+    VERSION = '1.10'
 
     dbapi = dbapi.get_instance()
 
@@ -60,8 +62,26 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
         'physical_network': object_fields.StringField(nullable=True),
         'is_smartnic': object_fields.BooleanField(nullable=True,
                                                   default=False),
+        'name': object_fields.StringField(nullable=True),
     }
 
+    def _convert_name_field(self, target_version,
+                            remove_unavailable_fields=True):
+        name_is_set = self.obj_attr_is_set('name')
+        if target_version >= (1, 10):
+            # Target version supports name. Set it to its default
+            # value if it is not set.
+            if not name_is_set:
+                self.name = None
+        elif name_is_set:
+            # Target version does not support name, and it is set.
+            if remove_unavailable_fields:
+                # (De)serialising: remove unavailable fields.
+                delattr(self, 'name')
+            elif self.name is not None:
+                # DB: set unavailable fields to their default.
+                self.name = None
+
     def _convert_to_version(self, target_version,
                             remove_unavailable_fields=True):
         """Convert to the target version.
@@ -82,6 +102,9 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
         Version 1.9: remove is_smartnic field for unsupported versions if
             remove_unavailable_fields is True.
 
+        Version 1.10: remove name field for unsupported versions if
+            remove_unavailable_fields is True.
+
         :param target_version: the desired version of the object
         :param remove_unavailable_fields: True to remove fields that are
             unavailable in the target version; set this to True when
@@ -134,6 +157,9 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
                 # DB: set unavailable fields to their default.
                 self.is_smartnic = False
 
+        # Convert the name field.
+        self._convert_name_field(target_version, remove_unavailable_fields)
+
     # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable
     # methods can be used in the future to replace current explicit RPC calls.
     # Implications of calling new remote procedures should be thought through.
@@ -142,11 +168,11 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
     def get(cls, context, port_id):
         """Find a port.
 
-        Find a port based on its id or uuid or MAC address and return a Port
-        object.
+        Find a port based on its id or uuid or name or MAC address and return
+        a Port object.
 
         :param context: Security context
-        :param port_id: the id *or* uuid *or* MAC address of a port.
+        :param port_id: the id *or* uuid *or* name *or* MAC address of a port.
         :returns: a :class:`Port` object.
         :raises: InvalidIdentity
 
@@ -157,6 +183,8 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
             return cls.get_by_uuid(context, port_id)
         elif netutils.is_valid_mac(port_id):
             return cls.get_by_address(context, port_id)
+        elif utils.is_valid_logical_name(port_id):
+            return cls.get_by_name(context, port_id)
         else:
             raise exception.InvalidIdentity(identity=port_id)
 
@@ -226,6 +254,25 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
     # Implications of calling new remote procedures should be thought through.
     # @object_base.remotable_classmethod
     @classmethod
+    def get_by_name(cls, context, name):
+        """Find a port based on name and return a :class:`Port` object.
+
+        :param cls: the :class:`Port`
+        :param context: Security context
+        :param name: the name of a port.
+        :returns: a :class:`Port` object.
+        :raises: PortNotFound
+
+        """
+        db_port = cls.dbapi.get_port_by_name(name)
+        port = cls._from_db_object(context, cls(), db_port)
+        return port
+
+    # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable
+    # methods can be used in the future to replace current explicit RPC calls.
+    # Implications of calling new remote procedures should be thought through.
+    # @object_base.remotable_classmethod
+    @classmethod
     def list(cls, context, limit=None, marker=None,
              sort_key=None, sort_dir=None, owner=None, project=None):
         """Return a list of Port objects.
@@ -435,7 +482,8 @@ class PortCRUDPayload(notification.NotificationPayloadBase):
     # Version 1.1: Add "portgroup_uuid" field
     # Version 1.2: Add "physical_network" field
     # Version 1.3: Add "is_smartnic" field
-    VERSION = '1.3'
+    # Version 1.4: Add "name" field
+    VERSION = '1.4'
 
     SCHEMA = {
         'address': ('port', 'address'),
@@ -447,6 +495,7 @@ class PortCRUDPayload(notification.NotificationPayloadBase):
         'updated_at': ('port', 'updated_at'),
         'uuid': ('port', 'uuid'),
         'is_smartnic': ('port', 'is_smartnic'),
+        'name': ('port', 'name'),
     }
 
     fields = {
@@ -463,6 +512,7 @@ class PortCRUDPayload(notification.NotificationPayloadBase):
         'uuid': object_fields.UUIDField(),
         'is_smartnic': object_fields.BooleanField(nullable=True,
                                                   default=False),
+        'name': object_fields.StringField(nullable=True),
     }
 
     def __init__(self, port, node_uuid, portgroup_uuid):
diff --git a/ironic/tests/unit/db/sqlalchemy/test_migrations.py b/ironic/tests/unit/db/sqlalchemy/test_migrations.py
index 39293c6ac..7a2641323 100644
--- a/ironic/tests/unit/db/sqlalchemy/test_migrations.py
+++ b/ironic/tests/unit/db/sqlalchemy/test_migrations.py
@@ -1002,6 +1002,11 @@ class MigrationCheckersMixin(object):
         col_names = [column.name for column in nodes.c]
         self.assertIn('lessee', col_names)
 
+    def _check_c0455649680c(self, engine, data):
+        ports = db_utils.get_table(engine, 'ports')
+        col_names = [column.name for column in ports.c]
+        self.assertIn('name', col_names)
+
     def test_upgrade_and_version(self):
         with patch_with_engine(self.engine):
             self.migration_api.upgrade('head')
diff --git a/ironic/tests/unit/db/test_ports.py b/ironic/tests/unit/db/test_ports.py
index d2434d603..18b8a9032 100644
--- a/ironic/tests/unit/db/test_ports.py
+++ b/ironic/tests/unit/db/test_ports.py
@@ -32,7 +32,8 @@ class DbPortTestCase(base.DbTestCase):
                                               lessee='54321')
         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)
+                                              portgroup_id=self.portgroup.id,
+                                              name='port-name')
 
     def test_get_port_by_id(self):
         res = self.dbapi.get_port_by_id(self.port.id)
@@ -68,6 +69,10 @@ class DbPortTestCase(base.DbTestCase):
                           self.port.address,
                           project='55555')
 
+    def test_get_port_by_name(self):
+        res = self.dbapi.get_port_by_name(self.port.name)
+        self.assertEqual(self.port.id, res.id)
+
     def test_get_port_list(self):
         uuids = []
         for i in range(1, 6):
diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py
index 96254889d..436849054 100644
--- a/ironic/tests/unit/db/utils.py
+++ b/ironic/tests/unit/db/utils.py
@@ -277,6 +277,7 @@ def get_test_port(**kw):
         'internal_info': kw.get('internal_info', {"bar": "buzz"}),
         'physical_network': kw.get('physical_network'),
         'is_smartnic': kw.get('is_smartnic', False),
+        'name': kw.get('name'),
     }
 
 
diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py
index 3852a9abd..7b7788933 100644
--- a/ironic/tests/unit/objects/test_objects.py
+++ b/ironic/tests/unit/objects/test_objects.py
@@ -679,7 +679,7 @@ expected_object_fingerprints = {
     'Node': '1.35-aee8ecf5c4d0ed590eb484762aee7fca',
     'MyObj': '1.5-9459d30d6954bffc7a9afd347a807ca6',
     'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905',
-    'Port': '1.9-0cb9202a4ec442e8c0d87a324155eaaf',
+    'Port': '1.10-67381b065c597c8d3a13c5dbc6243c33',
     'Portgroup': '1.4-71923a81a86743b313b190f5c675e258',
     'Conductor': '1.3-d3f53e853b4d58cae5bfbd9a8341af4a',
     'EventType': '1.1-aa2ba1afd38553e3880c267404e8d370',
@@ -700,7 +700,7 @@ expected_object_fingerprints = {
     'NodeCRUDNotification': '1.0-59acc533c11d306f149846f922739c15',
     'NodeCRUDPayload': '1.13-8f673253ff8d7389897a6a80d224ac33',
     'PortCRUDNotification': '1.0-59acc533c11d306f149846f922739c15',
-    'PortCRUDPayload': '1.3-21235916ed54a91b2a122f59571194e7',
+    'PortCRUDPayload': '1.4-9411a1701077ae9dc0aea27d6bf586fc',
     'NodeMaintenanceNotification': '1.0-59acc533c11d306f149846f922739c15',
     'NodeConsoleNotification': '1.0-59acc533c11d306f149846f922739c15',
     'PortgroupCRUDNotification': '1.0-59acc533c11d306f149846f922739c15',
diff --git a/ironic/tests/unit/objects/test_port.py b/ironic/tests/unit/objects/test_port.py
index 43c58876e..43459c958 100644
--- a/ironic/tests/unit/objects/test_port.py
+++ b/ironic/tests/unit/objects/test_port.py
@@ -34,7 +34,7 @@ class TestPortObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn):
 
     def setUp(self):
         super(TestPortObject, self).setUp()
-        self.fake_port = db_utils.get_test_port()
+        self.fake_port = db_utils.get_test_port(name='port-name')
 
     def test_get_by_id(self):
         port_id = self.fake_port['id']
@@ -69,9 +69,20 @@ class TestPortObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn):
             mock_get_port.assert_called_once_with(address, project=None)
             self.assertEqual(self.context, port._context)
 
-    def test_get_bad_id_and_uuid_and_address(self):
+    def test_get_by_name(self):
+        name = self.fake_port['name']
+        with mock.patch.object(self.dbapi, 'get_port_by_name',
+                               autospec=True) as mock_get_port:
+            mock_get_port.return_value = self.fake_port
+
+            port = objects.Port.get(self.context, name)
+
+            mock_get_port.assert_called_once_with(name)
+            self.assertEqual(self.context, port._context)
+
+    def test_get_bad_id_and_uuid_and_name_and_address(self):
         self.assertRaises(exception.InvalidIdentity,
-                          objects.Port.get, self.context, 'not-a-uuid')
+                          objects.Port.get, self.context, '#not-valid')
 
     def test_create(self):
         port = objects.Port(self.context, **self.fake_port)
@@ -349,3 +360,59 @@ class TestConvertToVersion(db_base.DbTestCase):
         port._convert_to_version("1.8", False)
         self.assertFalse(port.is_smartnic)
         self.assertNotIn('is_smartnic', port.obj_get_changes())
+
+    def test_name_supported_missing(self):
+        # name not set, should be set to default.
+        port = objects.Port(self.context, **self.fake_port)
+        delattr(port, 'name')
+        port.obj_reset_changes()
+        port._convert_to_version("1.10")
+        self.assertIsNone(port.name)
+        self.assertIn('name', port.obj_get_changes())
+        self.assertIsNone(port.obj_get_changes()['name'])
+
+    def test_name_supported_set(self):
+        # Physical network set, no change required.
+        port = objects.Port(self.context, **self.fake_port)
+        port.name = 'meow'
+        port.obj_reset_changes()
+        port._convert_to_version("1.10")
+        self.assertEqual('meow', port.name)
+        self.assertNotIn('name', port.obj_get_changes())
+
+    def test_name_unsupported_missing(self):
+        # name not set, no change required.
+        port = objects.Port(self.context, **self.fake_port)
+        delattr(port, 'name')
+        port.obj_reset_changes()
+        port._convert_to_version("1.9")
+        self.assertNotIn('name', port)
+        self.assertNotIn('name', port.obj_get_changes())
+
+    def test_name_unsupported_set_remove(self):
+        # name set, should be removed.
+        port = objects.Port(self.context, **self.fake_port)
+        port.name = 'meow'
+        port.obj_reset_changes()
+        port._convert_to_version("1.9")
+        self.assertNotIn('name', port)
+        self.assertNotIn('name', port.obj_get_changes())
+
+    def test_name_unsupported_set_no_remove_non_default(self):
+        # name set, should be set to default.
+        port = objects.Port(self.context, **self.fake_port)
+        port.name = 'meow'
+        port.obj_reset_changes()
+        port._convert_to_version("1.9", False)
+        self.assertIsNone(port.name)
+        self.assertIn('name', port.obj_get_changes())
+        self.assertIsNone(port.obj_get_changes()['name'])
+
+    def test_name_unsupported_set_no_remove_default(self):
+        # name set, no change required.
+        port = objects.Port(self.context, **self.fake_port)
+        port.name = None
+        port.obj_reset_changes()
+        port._convert_to_version("1.9", False)
+        self.assertIsNone(port.name)
+        self.assertNotIn('name', port.obj_get_changes())