diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index d70c13f27e..2c9c3dab73 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -163,22 +163,13 @@ def port_sanitize(port, fields=None): def list_convert_with_links(rpc_ports, limit, url, fields=None, **kwargs): ports = [] for rpc_port in rpc_ports: - try: - port = convert_with_links(rpc_port, fields=fields, - sanitize=False) - # NOTE(dtantsur): node was deleted after we fetched the port - # list, meaning that the port was also deleted. Skip it. - if port['node_uuid'] is None: - continue - except exception.PortgroupNotFound: - # NOTE(dtantsur): port group was deleted after we fetched the - # port list, it may mean that the port was deleted too, but - # we don't know it. Pretend that the port group was removed. - LOG.debug('Removing port group UUID from port %s as the port ' - 'group was deleted', rpc_port.uuid) - rpc_port.portgroup_id = None - port = convert_with_links(rpc_port, fields=fields, - sanitize=False) + port = convert_with_links(rpc_port, fields=fields, + sanitize=False) + # NOTE(dtantsur): node was deleted after we fetched the port + # list, meaning that the port was also deleted. Skip it. + if port['node_uuid'] is None: + continue + ports.append(port) return collection.list_convert_with_links( items=ports, diff --git a/ironic/api/controllers/v1/portgroup.py b/ironic/api/controllers/v1/portgroup.py index 6c68e07ba3..91740d3c74 100644 --- a/ironic/api/controllers/v1/portgroup.py +++ b/ironic/api/controllers/v1/portgroup.py @@ -90,10 +90,10 @@ def convert_with_links(rpc_portgroup, fields=None, sanitize=True): 'mode', 'name', 'properties', - 'standalone_ports_supported' + 'standalone_ports_supported', + 'node_uuid' ) ) - api_utils.populate_node_uuid(rpc_portgroup, portgroup) url = api.request.public_url portgroup['ports'] = [ link.make_link('self', url, 'portgroups', diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 76c40fc2fd..993ac2b77e 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -524,7 +524,7 @@ RELEASE_MAPPING = { 'Deployment': ['1.0'], 'DeployTemplate': ['1.1'], 'Port': ['1.11'], - 'Portgroup': ['1.4'], + 'Portgroup': ['1.5'], 'Trait': ['1.0'], 'TraitList': ['1.0'], 'VolumeConnector': ['1.0'], diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index 6fc8e21ab1..0dbfabddd7 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -293,6 +293,15 @@ class Portgroup(Base): mode = Column(String(255)) properties = Column(db_types.JsonEncodedDict) + _node_uuid = orm.relationship( + "Node", + viewonly=True, + primaryjoin="(Node.id == Portgroup.node_id)", + lazy="selectin", + ) + node_uuid = association_proxy( + "_node_uuid", "uuid", creator=lambda _i: Node(uuid=_i)) + class NodeTag(Base): """Represents a tag of a bare metal node.""" diff --git a/ironic/objects/portgroup.py b/ironic/objects/portgroup.py index 8628df731e..ef21a5f90e 100644 --- a/ironic/objects/portgroup.py +++ b/ironic/objects/portgroup.py @@ -36,7 +36,8 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): # Version 1.4: Migrate/copy extra['vif_port_id'] to # internal_info['tenant_vif_port_id'] (not an explicit db # change) - VERSION = '1.4' + # Version 1.5: Add node_uuid field + VERSION = '1.5' dbapi = dbapi.get_instance() @@ -45,6 +46,7 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): 'uuid': object_fields.UUIDField(nullable=True), 'name': object_fields.StringField(nullable=True), 'node_id': object_fields.IntegerField(nullable=True), + 'node_uuid': object_fields.UUIDField(nullable=True), 'address': object_fields.MACAddressField(nullable=True), 'extra': object_fields.FlexibleDictField(nullable=True), 'internal_info': object_fields.FlexibleDictField(nullable=True), @@ -261,6 +263,10 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): """ values = self.do_version_changes_for_db() db_portgroup = self.dbapi.create_portgroup(values) + # NOTE(hjensas): To avoid lazy load issue (DetachedInstanceError) in + # sqlalchemy, get new port the port from the DB to ensure the node_uuid + # via association_proxy relationship is loaded. + db_portgroup = self.dbapi.get_portgroup_by_id(db_portgroup['id']) self._from_db_object(self._context, self, db_portgroup) # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable diff --git a/ironic/tests/unit/api/controllers/v1/test_port.py b/ironic/tests/unit/api/controllers/v1/test_port.py index 7092a4012d..7b9e0e9d1d 100644 --- a/ironic/tests/unit/api/controllers/v1/test_port.py +++ b/ironic/tests/unit/api/controllers/v1/test_port.py @@ -15,7 +15,6 @@ Tests for the API /ports/ methods. import datetime from http import client as http_client -import types from unittest import mock from urllib import parse as urlparse @@ -236,24 +235,6 @@ class TestListPorts(test_api_base.BaseApiTest): # never expose the node_id self.assertNotIn('node_id', data['ports'][0]) - # NOTE(jlvillal): autospec=True doesn't work on staticmethods: - # https://bugs.python.org/issue23078 - @mock.patch.object(objects.Portgroup, 'get', spec_set=types.FunctionType) - def test_list_with_deleted_port_group(self, mock_get_pg): - # check that we don't end up with HTTP 400 when port group deletion - # races with listing ports - see https://launchpad.net/bugs/1748893 - 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) - mock_get_pg.side_effect = exception.PortgroupNotFound('boom') - data = self.get_json( - '/ports/detail', - headers={api_base.Version.string: str(api_v1.max_version())} - ) - self.assertEqual(port.uuid, data['ports'][0]["uuid"]) - self.assertIsNone(data['ports'][0]["portgroup_uuid"]) - @mock.patch.object(policy, 'authorize', spec=True) def test_list_non_admin_forbidden(self, mock_authorize): def mock_authorize_function(rule, target, creds): diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index 87ba4b5147..f75eea06d9 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -448,6 +448,8 @@ def get_test_portgroup(**kw): 'uuid': kw.get('uuid', '6eb02b44-18a3-4659-8c0b-8d2802581ae4'), 'name': kw.get('name', 'fooname'), 'node_id': kw.get('node_id', 123), + 'node_uuid': kw.get('node_uuid', + '40481b96-306b-4a33-901f-795a3dc2f397'), 'address': kw.get('address', '52:54:00:cf:2d:31'), 'extra': kw.get('extra', {}), 'created_at': kw.get('created_at'), diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 017a0d2103..9b4adfcfe7 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -680,7 +680,7 @@ expected_object_fingerprints = { 'MyObj': '1.5-9459d30d6954bffc7a9afd347a807ca6', 'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905', 'Port': '1.11-97bf15b61224f26c65e90f007d78bfd2', - 'Portgroup': '1.4-71923a81a86743b313b190f5c675e258', + 'Portgroup': '1.5-df4dc15967f67114d51176a98a901a83', 'Conductor': '1.3-d3f53e853b4d58cae5bfbd9a8341af4a', 'EventType': '1.1-aa2ba1afd38553e3880c267404e8d370', 'NotificationPublisher': '1.0-51a09397d6c0687771fb5be9a999605d', diff --git a/ironic/tests/unit/objects/test_portgroup.py b/ironic/tests/unit/objects/test_portgroup.py index 81b68437b7..7e844dac74 100644 --- a/ironic/tests/unit/objects/test_portgroup.py +++ b/ironic/tests/unit/objects/test_portgroup.py @@ -80,13 +80,18 @@ class TestPortgroupObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn): def test_create(self): portgroup = objects.Portgroup(self.context, **self.fake_portgroup) with mock.patch.object(self.dbapi, 'create_portgroup', - autospec=True) as mock_create_portgroup: - mock_create_portgroup.return_value = db_utils.get_test_portgroup() + autospec=True) as mock_create_pg: + with mock.patch.object(self.dbapi, 'get_portgroup_by_id', + autospec=True) as mock_get_pg: + test_pg = db_utils.get_test_portgroup() + mock_create_pg.return_value = test_pg + mock_get_pg.return_value = test_pg + mock_create_pg.return_value = db_utils.get_test_portgroup() - portgroup.create() + portgroup.create() - args, _kwargs = mock_create_portgroup.call_args - self.assertEqual(objects.Portgroup.VERSION, args[0]['version']) + args, _kwargs = mock_create_pg.call_args + self.assertEqual(objects.Portgroup.VERSION, args[0]['version']) def test_save(self): uuid = self.fake_portgroup['uuid']