openstack
/
openstacksdk
Code Issues Proposed changes

cloud: Do not reference other cloud mixin's methods 

We are using a mixin-like pattern to construct the cloud layer of our
API. However, this is not a true mixin pattern since each of the
"mixins" have dependencies on each other and to be honest, it's really
only there to allow us to split up the otherwise giant 'Connection'
object. Unfortunately this pattern doesn't really work with type
checkers or language servers since there's no obvious way to identify
where the additional resources are coming from until they're combined in
the 'Connection' class. Fixing this will require multiple steps, but the
first it to remove all dependencies between the various service mixins.
As such, rather than depending on e.g. the 'get_volume' cloud method in
various compute-related cloud methods, we use the 'find_volume' proxy
method. A later change can then add these various proxy attributes,
exposing them.

Change-Id: I10d3782899ac519f715d771d83303990a8289f04
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane 2023-08-17 12:26:33 +01:00
parent 763e09a4b1
commit a8adbadf0c
12 changed files with 143 additions and 190 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
openstack/cloud/_baremetal.py
View File

@ -427,7 +427,7 @@ class BaremetalCloudMixin:
:return: Nothing.
"""
machine = self.get_machine(name_or_id)
port = self.get_port(port_name_or_id)
port = self.network.find_port(port_name_or_id)
self.baremetal.attach_vif_to_node(machine, port['id'])
def detach_port_from_machine(self, name_or_id, port_name_or_id):
@ -439,7 +439,7 @@ class BaremetalCloudMixin:
:return: Nothing.
"""
machine = self.get_machine(name_or_id)
port = self.get_port(port_name_or_id)
port = self.network.find_port(port_name_or_id)
self.baremetal.detach_vif_from_node(machine, port['id'])
def list_ports_attached_to_machine(self, name_or_id):
@ -451,7 +451,7 @@ class BaremetalCloudMixin:
"""
machine = self.get_machine(name_or_id)
vif_ids = self.baremetal.list_node_vifs(machine)
return [self.get_port(vif) for vif in vif_ids]
return [self.network.find_port(vif) for vif in vif_ids]
def validate_machine(self, name_or_id, for_deploy=True):
"""Validate parameters of the machine.

4
openstack/cloud/_block_storage.py
View File

@ -144,7 +144,7 @@ class BlockStorageCloudMixin:
wait = True
if image:
image_obj = self.get_image(image)
image_obj = self.image.find_image(image)
if not image_obj:
raise exceptions.SDKException(
f"Image {image} was requested as the basis for a new "
@ -273,7 +273,7 @@ class BlockStorageCloudMixin:
"""
params = {}
if name_or_id:
project = self.get_project(name_or_id)
project = self.identity.find_project(name_or_id)
if not project:
raise exceptions.SDKException("project does not exist")
params['project'] = project

31
openstack/cloud/_compute.py
View File

@ -390,7 +390,7 @@ class ComputeCloudMixin:
"""
params = {}
if name_or_id:
project = self.get_project(name_or_id)
project = self.identity.find_project(name_or_id)
if not project:
raise exceptions.SDKException(
f"Project {name_or_id} was requested but was not found "
@ -851,7 +851,7 @@ class ComputeCloudMixin:
kwargs[desired] = value
if group:
group_obj = self.get_server_group(group)
group_obj = self.compute.find_server_group(group)
if not group_obj:
raise exceptions.SDKException(
"Server Group {group} was requested but was not found"
@ -882,7 +882,7 @@ class ComputeCloudMixin:
if isinstance(net_name, dict) and 'id' in net_name:
network_obj = net_name
else:
network_obj = self.get_network(name_or_id=net_name)
network_obj = self.network.find_network(net_name)
if not network_obj:
raise exceptions.SDKException(
'Network {network} is not a valid network in'
@ -910,7 +910,7 @@ class ComputeCloudMixin:
nic.pop('net-name', None)
elif 'net-name' in nic:
net_name = nic.pop('net-name')
nic_net = self.get_network(net_name)
nic_net = self.network.find_network(net_name)
if not nic_net:
raise exceptions.SDKException(
"Requested network {net} could not be found.".format(
@ -950,10 +950,21 @@ class ComputeCloudMixin:
kwargs['networks'] = 'auto'
if image:
# TODO(stephenfin): Drop support for dicts: we should only accept
# strings or Image objects
if isinstance(image, dict):
kwargs['imageRef'] = image['id']
else:
kwargs['imageRef'] = self.get_image(image).id
image_obj = self.image.find_image(image)
if not image_obj:
raise exc.OpenStackCloudException(
f"Image {image} was requested but was not found "
f"on the cloud"
)
kwargs['imageRef'] = image_obj.id
# TODO(stephenfin): Drop support for dicts: we should only accept
# strings or Image objects
if isinstance(flavor, dict):
kwargs['flavorRef'] = flavor['id']
else:
@ -1029,7 +1040,7 @@ class ComputeCloudMixin:
# If we have boot_from_volume but no root volume, then we're
# booting an image from volume
if boot_volume:
volume = self.get_volume(boot_volume)
volume = self.block_storage.find_volume(boot_volume)
if not volume:
raise exceptions.SDKException(
f"Volume {volume} was requested but was not found "
@ -1045,10 +1056,12 @@ class ComputeCloudMixin:
kwargs['block_device_mapping_v2'].append(block_mapping)
kwargs['imageRef'] = ''
elif boot_from_volume:
# TODO(stephenfin): Drop support for dicts: we should only accept
# strings or Image objects
if isinstance(image, dict):
image_obj = image
else:
image_obj = self.get_image(image)
image_obj = self.image.find_image(image)
if not image_obj:
raise exceptions.SDKException(
f"Image {image} was requested but was not found "
@ -1077,7 +1090,7 @@ class ComputeCloudMixin:
}
kwargs['block_device_mapping_v2'].append(block_mapping)
for volume in volumes:
volume_obj = self.get_volume(volume)
volume_obj = self.block_storage.find_volume(volume)
if not volume_obj:
raise exceptions.SDKException(
f"Volume {volume} was requested but was not found "
@ -1810,7 +1823,7 @@ class ComputeCloudMixin:
if isinstance(end, str):
end = parse_date(end)
proj = self.get_project(name_or_id)
proj = self.identity.find_project(name_or_id)
if not proj:
raise exceptions.SDKException(
f"Project {name_or_id} was requested but was not found "

16
openstack/cloud/_floating_ip.py
View File

@ -297,13 +297,14 @@ class FloatingIPCloudMixin:
def _find_floating_network_by_router(self):
"""Find the network providing floating ips by looking at routers."""
for router in self.list_routers():
for router in self.network.routers():
if router['admin_state_up']:
network_id = router.get('external_gateway_info', {}).get(
'network_id'
)
if network_id:
return network_id
self._floating_network_by_router = network_id
return self._floating_network_by_router
def available_floating_ip(self, network=None, server=None):
"""Get a floating IP from a network or a pool.
@ -679,7 +680,7 @@ class FloatingIPCloudMixin:
# old to check whether it belongs to us now, thus refresh
# the server data and try again. There are some clouds, which
# explicitely forbids FIP assign call if it is already assigned.
server = self.get_server_by_id(server['id'])
server = self.compute.get_server(server['id'])
ext_ip = meta.get_server_ip(
server, ext_tag='floating', public=True
)
@ -718,7 +719,7 @@ class FloatingIPCloudMixin:
"Timeout waiting for the floating IP to be attached.",
wait=min(5, timeout),
):
server = self.get_server_by_id(server_id)
server = self.compute.get_server(server_id)
ext_ip = meta.get_server_ip(
server, ext_tag='floating', public=True
)
@ -879,7 +880,7 @@ class FloatingIPCloudMixin:
timeout=timeout,
)
timeout = timeout - (time.time() - start_time)
server = self.get_server(server.id)
server = self.compute.get_server(server.id)
# We run attach as a second call rather than in the create call
# because there are code flows where we will not have an attached
@ -1154,8 +1155,7 @@ class FloatingIPCloudMixin:
:param fixed_address: Fixed ip address of the port
:param nat_destination: Name or ID of the network of the port.
"""
port_filter = {'device_id': server['id']}
ports = self.search_ports(filters=port_filter)
ports = list(self.network.ports(device_id=server['id']))
if not ports:
return (None, None)
@ -1163,7 +1163,7 @@ class FloatingIPCloudMixin:
if not fixed_address:
if len(ports) > 1:
if nat_destination:
nat_network = self.get_network(nat_destination)
nat_network = self.network.find_network(nat_destination)
if not nat_network:
raise exceptions.SDKException(
'NAT Destination {nat_destination} was configured'

7
openstack/cloud/_image.py
View File

@ -202,11 +202,14 @@ class ImageCloudMixin:
self.image._IMAGE_OBJECT_KEY in image.properties
or self.image._SHADE_IMAGE_OBJECT_KEY in image.properties
):
(container, objname) = image.properties.get(
container, objname = image.properties.get(
self.image._IMAGE_OBJECT_KEY,
image.properties.get(self.image._SHADE_IMAGE_OBJECT_KEY),
).split('/', 1)
self.delete_object(container=container, name=objname)
self.object_store.delete_object(
objname,
container=container,
)
if wait:
for count in utils.iterate_timeout(

12
openstack/cloud/_network.py
View File

@ -649,8 +649,7 @@ class NetworkCloudMixin:
:raises: :class:`~openstack.exceptions.SDKException` if the resource to
set the quota does not exist.
"""
proj = self.get_project(name_or_id)
proj = self.identity.find_project(name_or_id)
if not proj:
raise exceptions.SDKException(
f"Project {name_or_id} was requested by was not found "
@ -669,7 +668,12 @@ class NetworkCloudMixin:
:raises: :class:`~openstack.exceptions.SDKException` if it's not a
valid project
"""
proj = self.identity.find_project(name_or_id, ignore_missing=False)
proj = self.identity.find_project(name_or_id)
if not proj:
raise exc.OpenStackCloudException(
f"Project {name_or_id} was requested by was not found "
f"on the cloud"
)
return self.network.get_quota(proj.id, details)
def get_network_extensions(self):
@ -688,7 +692,7 @@ class NetworkCloudMixin:
:raises: :class:`~openstack.exceptions.SDKException` if it's not a
valid project or the network client call failed
"""
proj = self.get_project(name_or_id)
proj = self.identity.find_project(name_or_id)
if not proj:
raise exceptions.SDKException(
f"Project {name_or_id} was requested by was not found "

10
openstack/cloud/_network_common.py
View File

@ -80,7 +80,10 @@ class NetworkCommonCloudMixin:
# this search_networks can just totally fail. If it does
# though, that's fine, clearly the neutron introspection is
# not going to work.
all_networks = self.list_networks()
if self.has_service('network'):
all_networks = list(self.network.networks())
else:
all_networks = []
except exceptions.SDKException:
self._network_list_stamp = True
return
@ -179,7 +182,10 @@ class NetworkCommonCloudMixin:
# it out.
if all_subnets is None:
try:
all_subnets = self.list_subnets()
if self.has_service('network'):
all_subnets = list(self.network.subnets())
else:
all_subnets = []
except exceptions.SDKException:
# Thanks Rackspace broken neutron
all_subnets = []

54
openstack/tests/unit/cloud/test_create_server.py
View File

@ -21,7 +21,6 @@ import base64
from unittest import mock
import uuid
from openstack.cloud import meta
from openstack.compute.v2 import server
from openstack import connection
from openstack import exceptions
@ -721,7 +720,7 @@ class TestCreateServer(base.TestCase):
networks='auto',
imageRef='image-id',
flavorRef='flavor-id',
**fake_server
**fake_server,
)
mock_wait.assert_called_once_with(
srv,
@ -1201,7 +1200,7 @@ class TestCreateServer(base.TestCase):
[
dict(
method='GET',
uri='https://image.example.com/v2/images',
uri=f'https://image.example.com/v2/images/{image_id}',
json=fake_image_search_return,
),
self.get_nova_discovery_mock_dict(),
@ -1320,14 +1319,7 @@ class TestCreateServer(base.TestCase):
def test_create_boot_attach_volume(self):
build_server = fakes.make_fake_server('1234', '', 'BUILD')
active_server = fakes.make_fake_server('1234', '', 'BUILD')
vol = {
'id': 'volume001',
'status': 'available',
'name': '',
'attachments': [],
}
volume = meta.obj_to_munch(fakes.FakeVolume(**vol))
volume_id = '20e82d93-14fa-475b-bfcc-f5e6246dd194'
self.register_uris(
[
@ -1339,6 +1331,24 @@ class TestCreateServer(base.TestCase):
json={'networks': []},
),
self.get_nova_discovery_mock_dict(),
self.get_cinder_discovery_mock_dict(),
dict(
method='GET',
uri=self.get_mock_url(
'volumev3', 'public', append=['volumes', volume_id]
),
json={
'volume': {
'id': volume_id,
'status': 'available',
'size': 1,
'availability_zone': 'cinder',
'name': '',
'description': None,
'volume_type': 'lvmdriver-1',
}
},
),
dict(
method='POST',
uri=self.get_mock_url(
@ -1365,7 +1375,7 @@ class TestCreateServer(base.TestCase):
'delete_on_termination': False,
'destination_type': 'volume',
'source_type': 'volume',
'uuid': 'volume001',
'uuid': volume_id,
},
],
'name': 'server-name',
@ -1389,7 +1399,7 @@ class TestCreateServer(base.TestCase):
image=dict(id='image-id'),
flavor=dict(id='flavor-id'),
boot_from_volume=False,
volumes=[volume],
volumes=[volume_id],
wait=False,
)
@ -1552,7 +1562,9 @@ class TestCreateServer(base.TestCase):
dict(
method='GET',
uri=self.get_mock_url(
'compute', 'public', append=['os-server-groups']
'compute',
'public',
append=['os-server-groups', group_id],
),
json={'server_groups': [fake_group]},
),
@ -1598,7 +1610,7 @@ class TestCreateServer(base.TestCase):
image=dict(id='image-id'),
flavor=dict(id='flavor-id'),
scheduler_hints=dict(scheduler_hints),
group=group_name,
group=group_id,
wait=False,
)
@ -1610,11 +1622,11 @@ class TestCreateServer(base.TestCase):
param
"""
group_id_scheduler_hints = uuid.uuid4().hex
group_id_param = uuid.uuid4().hex
group_id = uuid.uuid4().hex
group_name = self.getUniqueString('server-group')
policies = ['affinity']
fake_group = fakes.make_fake_server_group(
group_id_param, group_name, policies
group_id, group_name, policies
)
# The scheduler hints we pass in that are expected to be ignored in
@ -1625,7 +1637,7 @@ class TestCreateServer(base.TestCase):
# The scheduler hints we expect to be in POST request
group_scheduler_hints = {
'group': group_id_param,
'group': group_id,
}
fake_server = fakes.make_fake_server('1234', '', 'BUILD')
@ -1637,7 +1649,9 @@ class TestCreateServer(base.TestCase):
dict(
method='GET',
uri=self.get_mock_url(
'compute', 'public', append=['os-server-groups']
'compute',
'public',
append=['os-server-groups', group_id],
),
json={'server_groups': [fake_group]},
),
@ -1683,7 +1697,7 @@ class TestCreateServer(base.TestCase):
image=dict(id='image-id'),
flavor=dict(id='flavor-id'),
scheduler_hints=dict(scheduler_hints),
group=group_name,
group=group_id,
wait=False,
)

185
openstack/tests/unit/cloud/test_floating_ip_neutron.py
View File

@ -500,6 +500,7 @@ class TestFloatingIP(base.TestCase):
self.assert_calls()
def test_auto_ip_pool_no_reuse(self):
server_id = 'f80e3ad0-e13e-41d4-8e9c-be79bccdb8f7'
# payloads taken from citycloud
self.register_uris(
[
@ -540,8 +541,7 @@ class TestFloatingIP(base.TestCase):
),
dict(
method='GET',
uri='https://network.example.com/v2.0/ports'
'?device_id=f80e3ad0-e13e-41d4-8e9c-be79bccdb8f7',
uri=f'https://network.example.com/v2.0/ports?device_id={server_id}',
json={
"ports": [
{
@ -569,7 +569,7 @@ class TestFloatingIP(base.TestCase):
"security_groups": [
"9fb5ba44-5c46-4357-8e60-8b55526cab54"
],
"device_id": "f80e3ad0-e13e-41d4-8e9c-be79bccdb8f7", # noqa: E501
"device_id": server_id, # noqa: E501
}
]
},
@ -605,141 +605,54 @@ class TestFloatingIP(base.TestCase):
self.get_nova_discovery_mock_dict(),
dict(
method='GET',
uri='{endpoint}/servers/detail'.format(
endpoint=fakes.COMPUTE_ENDPOINT
),
uri=f'https://compute.example.com/v2.1/servers/{server_id}',
json={
"servers": [
{
"status": "ACTIVE",
"updated": "2017-02-06T20:59:49Z",
"addresses": {
"private": [
{
"OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:e8:7f:03", # noqa: E501
"version": 4,
"addr": "10.4.0.16",
"OS-EXT-IPS:type": "fixed",
},
{
"OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:e8:7f:03", # noqa: E501
"version": 4,
"addr": "89.40.216.153",
"OS-EXT-IPS:type": "floating",
},
]
},
"key_name": None,
"image": {
"id": "95e4c449-8abf-486e-97d9-dc3f82417d2d" # noqa: E501
},
"OS-EXT-STS:task_state": None,
"OS-EXT-STS:vm_state": "active",
"OS-SRV-USG:launched_at": "2017-02-06T20:59:48.000000", # noqa: E501
"flavor": {
"id": "2186bd79-a05e-4953-9dde-ddefb63c88d4" # noqa: E501
},
"id": "f80e3ad0-e13e-41d4-8e9c-be79bccdb8f7",
"security_groups": [{"name": "default"}],
"OS-SRV-USG:terminated_at": None,
"OS-EXT-AZ:availability_zone": "nova",
"user_id": "c17534835f8f42bf98fc367e0bf35e09",
"name": "testmt",
"created": "2017-02-06T20:59:44Z",
"tenant_id": "65222a4d09ea4c68934fa1028c77f394", # noqa: E501
"OS-DCF:diskConfig": "MANUAL",
"os-extended-volumes:volumes_attached": [],
"accessIPv4": "",
"accessIPv6": "",
"progress": 0,
"OS-EXT-STS:power_state": 1,
"config_drive": "",
"metadata": {},
}
]
},
),
dict(
method='GET',
uri='https://network.example.com/v2.0/networks',
json={
"networks": [
{
"status": "ACTIVE",
"subnets": [
"df3e17fa-a4b2-47ae-9015-bc93eb076ba2",
"6b0c3dc9-b0b8-4d87-976a-7f2ebf13e7ec",
"fc541f48-fc7f-48c0-a063-18de6ee7bdd7",
],
"availability_zone_hints": [],
"availability_zones": ["nova"],
"name": "ext-net",
"admin_state_up": True,
"tenant_id": "a564613210ee43708b8a7fc6274ebd63", # noqa: E501
"tags": [],
"ipv6_address_scope": "9f03124f-89af-483a-b6fd-10f08079db4d", # noqa: E501
"mtu": 0,
"is_default": False,
"router:external": True,
"ipv4_address_scope": None,
"shared": False,
"id": "0232c17f-2096-49bc-b205-d3dcd9a30ebf",
"description": None,
"server": {
"status": "ACTIVE",
"updated": "2017-02-06T20:59:49Z",
"addresses": {
"private": [
{
"OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:e8:7f:03", # noqa: E501
"version": 4,
"addr": "10.4.0.16",
"OS-EXT-IPS:type": "fixed",
},
{
"OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:e8:7f:03", # noqa: E501
"version": 4,
"addr": "89.40.216.153",
"OS-EXT-IPS:type": "floating",
},
]
},
{
"status": "ACTIVE",
"subnets": [
"f0ad1df5-53ee-473f-b86b-3604ea5591e9"
],
"availability_zone_hints": [],
"availability_zones": ["nova"],
"name": "private",
"admin_state_up": True,
"tenant_id": "65222a4d09ea4c68934fa1028c77f394", # noqa: E501
"created_at": "2016-10-22T13:46:26",
"tags": [],
"updated_at": "2016-10-22T13:46:26",
"ipv6_address_scope": None,
"router:external": False,
"ipv4_address_scope": None,
"shared": False,
"mtu": 1450,
"id": "2c9adcb5-c123-4c5a-a2ba-1ad4c4e1481f",
"description": "",
"key_name": None,
"image": {
"id": "95e4c449-8abf-486e-97d9-dc3f82417d2d" # noqa: E501
},
]
},
),
dict(
method='GET',
uri='https://network.example.com/v2.0/subnets',
json={
"subnets": [
{
"description": "",
"enable_dhcp": True,
"network_id": "2c9adcb5-c123-4c5a-a2ba-1ad4c4e1481f", # noqa: E501
"tenant_id": "65222a4d09ea4c68934fa1028c77f394", # noqa: E501
"created_at": "2016-10-22T13:46:26",
"dns_nameservers": [
"89.36.90.101",
"89.36.90.102",
],
"updated_at": "2016-10-22T13:46:26",
"gateway_ip": "10.4.0.1",
"ipv6_ra_mode": None,
"allocation_pools": [
{"start": "10.4.0.2", "end": "10.4.0.200"}
],
"host_routes": [],
"ip_version": 4,
"ipv6_address_mode": None,
"cidr": "10.4.0.0/24",
"id": "f0ad1df5-53ee-473f-b86b-3604ea5591e9",
"subnetpool_id": None,
"name": "private-subnet-ipv4",
}
]
"OS-EXT-STS:task_state": None,
"OS-EXT-STS:vm_state": "active",
"OS-SRV-USG:launched_at": "2017-02-06T20:59:48.000000", # noqa: E501
"flavor": {
"id": "2186bd79-a05e-4953-9dde-ddefb63c88d4" # noqa: E501
},
"id": server_id,
"security_groups": [{"name": "default"}],
"OS-SRV-USG:terminated_at": None,
"OS-EXT-AZ:availability_zone": "nova",
"user_id": "c17534835f8f42bf98fc367e0bf35e09",
"name": "testmt",
"created": "2017-02-06T20:59:44Z",
"tenant_id": "65222a4d09ea4c68934fa1028c77f394", # noqa: E501
"OS-DCF:diskConfig": "MANUAL",
"os-extended-volumes:volumes_attached": [],
"accessIPv4": "",
"accessIPv6": "",
"progress": 0,
"OS-EXT-STS:power_state": 1,
"config_drive": "",
"metadata": {},
}
},
),
]
@ -747,7 +660,7 @@ class TestFloatingIP(base.TestCase):
self.cloud.add_ips_to_server(
utils.Munch(
id='f80e3ad0-e13e-41d4-8e9c-be79bccdb8f7',
id=server_id,
addresses={
"private": [
{

2
openstack/tests/unit/cloud/test_limits.py
View File

@ -58,7 +58,7 @@ class TestLimits(base.TestCase):
def test_other_get_compute_limits(self):
project = self.mock_for_keystone_projects(
project_count=1, list_get=True
project_count=1, id_get=True
)[0]
self.register_uris(
[

4
openstack/tests/unit/cloud/test_quotas.py
View File

@ -258,7 +258,7 @@ class TestQuotas(base.TestCase):
def test_neutron_update_quotas(self):
project = self.mock_for_keystone_projects(
project_count=1, list_get=True
project_count=1, id_get=True
)[0]
self.register_uris(
[
@ -362,7 +362,7 @@ class TestQuotas(base.TestCase):
def test_neutron_delete_quotas(self):
project = self.mock_for_keystone_projects(
project_count=1, list_get=True
project_count=1, id_get=True
)[0]
self.register_uris(
[

2
openstack/tests/unit/cloud/test_usage.py
View File

@ -20,7 +20,7 @@ from openstack.tests.unit import base
class TestUsage(base.TestCase):
def test_get_usage(self):
project = self.mock_for_keystone_projects(
project_count=1, list_get=True
project_count=1, id_get=True
)[0]
start = end = datetime.datetime.now()