openstack/ironic
Code Issues Proposed changes

Use association_proxy for port groups node_uuid

Browse Source 
This change adds 'node_uuid' to:
  ironic.objects.portgroup.Portgroup

'node_uuid' is a relationship using association_proxy in
models.Portgroup. Using the association_proxy removes the
need to do the node lookup to populate node uuid for
portgroups in the api controller.

NOTE:
 On portgroup create a read is added to read the port from
 the database, this ensures node_uuid is loaded and solves
 the DetachedInstanceError which is otherwise raised.

The test test_list_with_deleted_port_group was deleted, if
the portgroup does not exist porgroup_uuid on the port will
be None, no need for extra handling of that case.

Bumps Portgroup object version to 1.5

Change-Id: I4317d034b6661da4248935cb0b9cb095982cc052
This commit is contained in:
Harald Jensås 2022-11-16 20:39:11 +01:00
parent 8b00932e48
commit 1f8a0a21de
9 changed files with 39 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
ironic/api/controllers/v1/port.py
View File

@ -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,

4
ironic/api/controllers/v1/portgroup.py
View File

@ -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',

2
ironic/common/release_mappings.py
View File

@ -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'],

9
ironic/db/sqlalchemy/models.py
View File

@ -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."""

8
ironic/objects/portgroup.py
View File

@ -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

19
ironic/tests/unit/api/controllers/v1/test_port.py
View File

@ -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):

2
ironic/tests/unit/db/utils.py
View File

@ -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'),

2
ironic/tests/unit/objects/test_objects.py
View File

@ -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',

15
ironic/tests/unit/objects/test_portgroup.py
View File

@ -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']