openstack/openstacksdk
Code Issues Proposed changes

Switch flavor ops in the cloud layer to proxy

Browse Source 
Since very long time we want to switch cloud layer functions to use
underneath the proxy layer to reduce maintenance complexity. For flavors
we even did some tries in the past. This time it is meant even more
seriosly. Since flavors listing is a subject of caching, but
dogpile/pickle does not support caching of the complex objects returned
by funciton we convert and return flavors to Munch objects (we anyway
wanted to have it this way).

Change-Id: I0353bb8d1be69e18dd31f0abedf25818b42c14ce
This commit is contained in:
Artem Goncharov 2020-11-03 13:18:14 +01:00
parent 256e25e321
commit 292c917949
9 changed files with 173 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

159
openstack/cloud/_compute.py
View File

@ -159,29 +159,13 @@ class ComputeCloudMixin(_normalize.Normalizer):
:returns: A list of flavor ``munch.Munch``.
"""
data = proxy._json_response(
self.compute.get(
'/flavors/detail', params=dict(is_public='None')),
error_message="Error fetching flavor list")
flavors = self._normalize_flavors(
self._get_and_munchify('flavors', data))
data = self.compute.flavors(details=True)
flavors = []
for flavor in flavors:
for flavor in data:
if not flavor.extra_specs and get_extra:
endpoint = "/flavors/{id}/os-extra_specs".format(
id=flavor.id)
try:
data = proxy._json_response(
self.compute.get(endpoint),
error_message="Error fetching flavor extra specs")
flavor.extra_specs = self._get_and_munchify(
'extra_specs', data)
except exc.OpenStackCloudHTTPError as e:
flavor.extra_specs = {}
self.log.debug(
'Fetching extra specs for flavor failed:'
' %(msg)s', {'msg': str(e)})
flavor.fetch_extra_specs(self.compute)
flavors.append(flavor._to_munch(original_names=False))
return flavors
def list_server_security_groups(self, server):
@ -441,9 +425,12 @@ class ComputeCloudMixin(_normalize.Normalizer):
found.
"""
search_func = functools.partial(
self.search_flavors, get_extra=get_extra)
return _utils._get_entity(self, search_func, name_or_id, filters)
if not filters:
filters = {}
flavor = self.compute.find_flavor(
name_or_id, get_extra_specs=get_extra, **filters)
if flavor:
return flavor._to_munch(original_names=False)
def get_flavor_by_id(self, id, get_extra=False):
""" Get a flavor by ID
@ -454,29 +441,8 @@ class ComputeCloudMixin(_normalize.Normalizer):
specs.
:returns: A flavor ``munch.Munch``.
"""
data = proxy._json_response(
self.compute.get('/flavors/{id}'.format(id=id)),
error_message="Error getting flavor with ID {id}".format(id=id)
)
flavor = self._normalize_flavor(
self._get_and_munchify('flavor', data))
if not flavor.extra_specs and get_extra:
endpoint = "/flavors/{id}/os-extra_specs".format(
id=flavor.id)
try:
data = proxy._json_response(
self.compute.get(endpoint),
error_message="Error fetching flavor extra specs")
flavor.extra_specs = self._get_and_munchify(
'extra_specs', data)
except exc.OpenStackCloudHTTPError as e:
flavor.extra_specs = {}
self.log.debug(
'Fetching extra specs for flavor failed:'
' %(msg)s', {'msg': str(e)})
return flavor
flavor = self.compute.get_flavor(id, get_extra_specs=get_extra)
return flavor._to_munch(original_names=False)
def get_server_console(self, server, length=None):
"""Get the console log for a server.
@ -1412,27 +1378,23 @@ class ComputeCloudMixin(_normalize.Normalizer):
:raises: OpenStackCloudException on operation error.
"""
with _utils.shade_exceptions("Failed to create flavor {name}".format(
name=name)):
payload = {
'disk': disk,
'OS-FLV-EXT-DATA:ephemeral': ephemeral,
'id': flavorid,
'os-flavor-access:is_public': is_public,
'name': name,
'ram': ram,
'rxtx_factor': rxtx_factor,
'swap': swap,
'vcpus': vcpus,
}
if flavorid == 'auto':
payload['id'] = None
data = proxy._json_response(self.compute.post(
'/flavors',
json=dict(flavor=payload)))
attrs = {
'disk': disk,
'ephemeral': ephemeral,
'id': flavorid,
'is_public': is_public,
'name': name,
'ram': ram,
'rxtx_factor': rxtx_factor,
'swap': swap,
'vcpus': vcpus,
}
if flavorid == 'auto':
attrs['id'] = None
return self._normalize_flavor(
self._get_and_munchify('flavor', data))
flavor = self.compute.create_flavor(**attrs)
return flavor._to_munch(original_names=False)
def delete_flavor(self, name_or_id):
"""Delete a flavor
@ -1443,19 +1405,17 @@ class ComputeCloudMixin(_normalize.Normalizer):
:raises: OpenStackCloudException on operation error.
"""
flavor = self.get_flavor(name_or_id, get_extra=False)
if flavor is None:
self.log.debug(
"Flavor %s not found for deleting", name_or_id)
return False
proxy._json_response(
self.compute.delete(
'/flavors/{id}'.format(id=flavor['id'])),
error_message="Unable to delete flavor {name}".format(
name=name_or_id))
return True
try:
flavor = self.compute.find_flavor(name_or_id)
if not flavor:
self.log.debug(
"Flavor %s not found for deleting", name_or_id)
return False
self.compute.delete_flavor(flavor)
return True
except exceptions.SDKException:
raise exceptions.OpenStackCloudException(
"Unable to delete flavor {name}".format(name=name_or_id))
def set_flavor_specs(self, flavor_id, extra_specs):
"""Add extra specs to a flavor
@ -1466,11 +1426,7 @@ class ComputeCloudMixin(_normalize.Normalizer):
:raises: OpenStackCloudException on operation error.
:raises: OpenStackCloudResourceNotFound if flavor ID is not found.
"""
proxy._json_response(
self.compute.post(
"/flavors/{id}/os-extra_specs".format(id=flavor_id),
json=dict(extra_specs=extra_specs)),
error_message="Unable to set flavor specs")
self.compute.create_flavor_extra_specs(flavor_id, extra_specs)
def unset_flavor_specs(self, flavor_id, keys):
"""Delete extra specs from a flavor
@ -1482,24 +1438,7 @@ class ComputeCloudMixin(_normalize.Normalizer):
:raises: OpenStackCloudResourceNotFound if flavor ID is not found.
"""
for key in keys:
proxy._json_response(
self.compute.delete(
"/flavors/{id}/os-extra_specs/{key}".format(
id=flavor_id, key=key)),
error_message="Unable to delete flavor spec {0}".format(key))
def _mod_flavor_access(self, action, flavor_id, project_id):
"""Common method for adding and removing flavor access
"""
with _utils.shade_exceptions("Error trying to {action} access from "
"flavor ID {flavor}".format(
action=action, flavor=flavor_id)):
endpoint = '/flavors/{id}/action'.format(id=flavor_id)
access = {'tenant': project_id}
access_key = '{action}TenantAccess'.format(action=action)
proxy._json_response(
self.compute.post(endpoint, json={access_key: access}))
self.compute.delete_flavor_extra_specs_property(flavor_id, key)
def add_flavor_access(self, flavor_id, project_id):
"""Grant access to a private flavor for a project/tenant.
@ -1509,7 +1448,7 @@ class ComputeCloudMixin(_normalize.Normalizer):
:raises: OpenStackCloudException on operation error.
"""
self._mod_flavor_access('add', flavor_id, project_id)
self.compute.flavor_add_tenant_access(flavor_id, project_id)
def remove_flavor_access(self, flavor_id, project_id):
"""Revoke access from a private flavor for a project/tenant.
@ -1519,7 +1458,7 @@ class ComputeCloudMixin(_normalize.Normalizer):
:raises: OpenStackCloudException on operation error.
"""
self._mod_flavor_access('remove', flavor_id, project_id)
self.compute.flavor_remove_tenant_access(flavor_id, project_id)
def list_flavor_access(self, flavor_id):
"""List access from a private flavor for a project/tenant.
@ -1530,14 +1469,8 @@ class ComputeCloudMixin(_normalize.Normalizer):
:raises: OpenStackCloudException on operation error.
"""
data = proxy._json_response(
self.compute.get(
'/flavors/{id}/os-flavor-access'.format(id=flavor_id)),
error_message=(
"Error trying to list access from flavorID {flavor}".format(
flavor=flavor_id)))
return _utils.normalize_flavor_accesses(
self._get_and_munchify('flavor_access', data))
access = self.compute.get_flavor_access(flavor_id)
return _utils.normalize_flavor_accesses(access)
def list_hypervisors(self, filters={}):
"""List all hypervisors

20
openstack/compute/v2/_proxy.py
View File

@ -62,7 +62,7 @@ class Proxy(proxy.Proxy):
# ========== Flavors ==========
def find_flavor(self, name_or_id, ignore_missing=True,
get_extra_specs=False):
get_extra_specs=False, **query):
"""Find a single flavor
:param name_or_id: The name or ID of a flavor.
@ -73,10 +73,14 @@ class Proxy(proxy.Proxy):
:param bool get_extra_specs: When set to ``True`` and extra_specs not
present in the response will invoke additional API call to fetch
extra_specs.
:param kwargs query: Optional query parameters to be sent to limit
the flavors being returned.
:returns: One :class:`~openstack.compute.v2.flavor.Flavor` or None
"""
flavor = self._find(_flavor.Flavor, name_or_id,
ignore_missing=ignore_missing)
flavor = self._find(
_flavor.Flavor, name_or_id, ignore_missing=ignore_missing, **query)
if flavor and get_extra_specs and not flavor.extra_specs:
flavor = flavor.fetch_extra_specs(self)
return flavor
@ -126,19 +130,25 @@ class Proxy(proxy.Proxy):
flavor = flavor.fetch_extra_specs(self)
return flavor
def flavors(self, details=True, **query):
def flavors(self, details=True, get_extra_specs=False, **query):
"""Return a generator of flavors
:param bool details: When ``True``, returns
:class:`~openstack.compute.v2.flavor.Flavor` objects,
with additional attributes filled.
:param bool get_extra_specs: When set to ``True`` and extra_specs not
present in the response will invoke additional API call to fetch
extra_specs.
:param kwargs query: Optional query parameters to be sent to limit
the flavors being returned.
:returns: A generator of flavor objects
"""
base_path = '/flavors/detail' if details else '/flavors'
return self._list(_flavor.Flavor, base_path=base_path, **query)
for flv in self._list(_flavor.Flavor, base_path=base_path, **query):
if get_extra_specs and not flv.extra_specs:
flv = flv.fetch_extra_specs(self)
yield flv
def flavor_add_tenant_access(self, flavor, tenant):
"""Adds tenant/project access to flavor.

2
openstack/compute/v2/flavor.py
View File

@ -62,7 +62,7 @@ class Flavor(resource.Resource):
# TODO(mordred) extra_specs can historically also come from
# OS-FLV-WITH-EXT-SPECS:extra_specs. Do we care?
#: A dictionary of the flavor's extra-specs key-and-value pairs.
extra_specs = resource.Body('extra_specs', type=dict)
extra_specs = resource.Body('extra_specs', type=dict, default={})
@classmethod
def list(cls, session, paginated=True, base_path='/flavors/detail',

2
openstack/tests/functional/cloud/test_flavor.py
View File

@ -66,10 +66,8 @@ class TestFlavor(base.BaseFunctionalTest):
# We should also always have ephemeral and public attributes
self.assertIn('ephemeral', flavor)
self.assertIn('OS-FLV-EXT-DATA:ephemeral', flavor)
self.assertEqual(5, flavor['ephemeral'])
self.assertIn('is_public', flavor)
self.assertIn('os-flavor-access:is_public', flavor)
self.assertTrue(flavor['is_public'])
for key in flavor_kwargs.keys():

16
openstack/tests/unit/cloud/test_caching.py
View File

@ -18,6 +18,7 @@ from testscenarios import load_tests_apply_scenarios as load_tests # noqa
import openstack
import openstack.cloud
from openstack.cloud import meta
from openstack.compute.v2 import flavor as _flavor
from openstack import exceptions
from openstack.tests import fakes
from openstack.tests.unit import base
@ -436,10 +437,16 @@ class TestMemoryCache(base.TestCase):
endpoint=fakes.COMPUTE_ENDPOINT)
uris_to_mock = [
dict(method='GET', uri=mock_uri, json={'flavors': []}),
dict(method='GET', uri=mock_uri,
validate=dict(
headers={'OpenStack-API-Version': 'compute 2.53'}),
json={'flavors': []}),
dict(method='GET', uri=mock_uri,
validate=dict(
headers={'OpenStack-API-Version': 'compute 2.53'}),
json={'flavors': fakes.FAKE_FLAVOR_LIST})
]
self.use_compute_discovery()
self.register_uris(uris_to_mock)
@ -447,8 +454,11 @@ class TestMemoryCache(base.TestCase):
self.assertEqual([], self.cloud.list_flavors())
fake_flavor_dicts = self.cloud._normalize_flavors(
fakes.FAKE_FLAVOR_LIST)
fake_flavor_dicts = [
_flavor.Flavor(connection=self.cloud, **f)
for f in fakes.FAKE_FLAVOR_LIST
]
self.cloud.list_flavors.invalidate(self.cloud)
self.assertEqual(fake_flavor_dicts, self.cloud.list_flavors())

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

@ -791,11 +791,12 @@ class TestCreateServer(base.TestCase):
dict(method='GET',
uri='https://image.example.com/v2/images',
json=fake_image_search_return),
self.get_nova_discovery_mock_dict(),
dict(method='GET',
uri=self.get_mock_url(
'compute', 'public', append=['flavors', 'detail'],
qs_elements=['is_public=None']),
json={'flavors': fakes.FAKE_FLAVOR_LIST}),
'compute', 'public', append=['flavors', 'vanilla'],
qs_elements=[]),
json=fakes.FAKE_FLAVOR),
dict(method='POST',
uri=self.get_mock_url(
'compute', 'public', append=['servers']),

43
openstack/tests/unit/cloud/test_flavors.py
View File

@ -18,8 +18,13 @@ from openstack.tests.unit import base
class TestFlavors(base.TestCase):
def setUp(self):
super(TestFlavors, self).setUp()
# self.use_compute_discovery()
def test_create_flavor(self):
self.use_compute_discovery()
self.register_uris([
dict(method='POST',
uri='{endpoint}/flavors'.format(
@ -44,11 +49,12 @@ class TestFlavors(base.TestCase):
self.assert_calls()
def test_delete_flavor(self):
self.use_compute_discovery()
self.register_uris([
dict(method='GET',
uri='{endpoint}/flavors/detail?is_public=None'.format(
uri='{endpoint}/flavors/vanilla'.format(
endpoint=fakes.COMPUTE_ENDPOINT),
json={'flavors': fakes.FAKE_FLAVOR_LIST}),
json=fakes.FAKE_FLAVOR),
dict(method='DELETE',
uri='{endpoint}/flavors/{id}'.format(
endpoint=fakes.COMPUTE_ENDPOINT, id=fakes.FLAVOR_ID))])
@ -57,7 +63,12 @@ class TestFlavors(base.TestCase):
self.assert_calls()
def test_delete_flavor_not_found(self):
self.use_compute_discovery()
self.register_uris([
dict(method='GET',
uri='{endpoint}/flavors/invalid'.format(
endpoint=fakes.COMPUTE_ENDPOINT),
status_code=404),
dict(method='GET',
uri='{endpoint}/flavors/detail?is_public=None'.format(
endpoint=fakes.COMPUTE_ENDPOINT),
@ -68,7 +79,12 @@ class TestFlavors(base.TestCase):
self.assert_calls()
def test_delete_flavor_exception(self):
self.use_compute_discovery()
self.register_uris([
dict(method='GET',
uri='{endpoint}/flavors/vanilla'.format(
endpoint=fakes.COMPUTE_ENDPOINT),
json=fakes.FAKE_FLAVOR),
dict(method='GET',
uri='{endpoint}/flavors/detail?is_public=None'.format(
endpoint=fakes.COMPUTE_ENDPOINT),
@ -82,6 +98,7 @@ class TestFlavors(base.TestCase):
self.cloud.delete_flavor, 'vanilla')
def test_list_flavors(self):
self.use_compute_discovery()
uris_to_mock = [
dict(method='GET',
uri='{endpoint}/flavors/detail?is_public=None'.format(
@ -106,6 +123,7 @@ class TestFlavors(base.TestCase):
self.assert_calls()
def test_list_flavors_with_extra(self):
self.use_compute_discovery()
uris_to_mock = [
dict(method='GET',
uri='{endpoint}/flavors/detail?is_public=None'.format(
@ -136,6 +154,7 @@ class TestFlavors(base.TestCase):
self.assert_calls()
def test_get_flavor_by_ram(self):
self.use_compute_discovery()
uris_to_mock = [
dict(method='GET',
uri='{endpoint}/flavors/detail?is_public=None'.format(
@ -154,6 +173,7 @@ class TestFlavors(base.TestCase):
self.assertEqual(fakes.STRAWBERRY_FLAVOR_ID, flavor['id'])
def test_get_flavor_by_ram_and_include(self):
self.use_compute_discovery()
uris_to_mock = [
dict(method='GET',
uri='{endpoint}/flavors/detail?is_public=None'.format(
@ -171,6 +191,7 @@ class TestFlavors(base.TestCase):
self.assertEqual(fakes.STRAWBERRY_FLAVOR_ID, flavor['id'])
def test_get_flavor_by_ram_not_found(self):
self.use_compute_discovery()
self.register_uris([
dict(method='GET',
uri='{endpoint}/flavors/detail?is_public=None'.format(
@ -182,19 +203,19 @@ class TestFlavors(base.TestCase):
ram=100)
def test_get_flavor_string_and_int(self):
flavor_list_uri = '{endpoint}/flavors/detail?is_public=None'.format(
endpoint=fakes.COMPUTE_ENDPOINT)
self.use_compute_discovery()
flavor_resource_uri = '{endpoint}/flavors/1/os-extra_specs'.format(
endpoint=fakes.COMPUTE_ENDPOINT)
flavor_list_json = {'flavors': [fakes.make_fake_flavor(
'1', 'vanilla')]}
flavor = fakes.make_fake_flavor('1', 'vanilla')
flavor_json = {'extra_specs': {}}
self.register_uris([
dict(method='GET', uri=flavor_list_uri, json=flavor_list_json),
dict(method='GET',
uri='{endpoint}/flavors/1'.format(
endpoint=fakes.COMPUTE_ENDPOINT),
json=flavor),
dict(method='GET', uri=flavor_resource_uri, json=flavor_json),
dict(method='GET', uri=flavor_list_uri, json=flavor_list_json),
dict(method='GET', uri=flavor_resource_uri, json=flavor_json)])
])
flavor1 = self.cloud.get_flavor('1')
self.assertEqual('1', flavor1['id'])
@ -202,6 +223,7 @@ class TestFlavors(base.TestCase):
self.assertEqual('1', flavor2['id'])
def test_set_flavor_specs(self):
self.use_compute_discovery()
extra_specs = dict(key1='value1')
self.register_uris([
dict(method='POST',
@ -213,6 +235,7 @@ class TestFlavors(base.TestCase):
self.assert_calls()
def test_unset_flavor_specs(self):
self.use_compute_discovery()
keys = ['key1', 'key2']
self.register_uris([
dict(method='DELETE',
@ -262,6 +285,7 @@ class TestFlavors(base.TestCase):
self.assert_calls()
def test_get_flavor_by_id(self):
self.use_compute_discovery()
flavor_uri = '{endpoint}/flavors/1'.format(
endpoint=fakes.COMPUTE_ENDPOINT)
flavor_json = {'flavor': fakes.make_fake_flavor('1', 'vanilla')}
@ -278,6 +302,7 @@ class TestFlavors(base.TestCase):
self.assertEqual({}, flavor2.extra_specs)
def test_get_flavor_with_extra_specs(self):
self.use_compute_discovery()
flavor_uri = '{endpoint}/flavors/1'.format(
endpoint=fakes.COMPUTE_ENDPOINT)
flavor_extra_uri = '{endpoint}/flavors/1/os-extra_specs'.format(

66
openstack/tests/unit/compute/v2/test_proxy.py
View File

@ -47,6 +47,13 @@ class TestFlavor(TestComputeProxy):
def test_flavor_find(self):
self.verify_find(self.proxy.find_flavor, flavor.Flavor)
def test_flavor_find_query(self):
self.verify_find(
self.proxy.find_flavor, flavor.Flavor,
method_kwargs={"a": "b"},
expected_kwargs={"a": "b", "ignore_missing": False}
)
def test_flavor_find_fetch_extra(self):
"""fetch extra_specs is triggered"""
with mock.patch(
@ -129,17 +136,56 @@ class TestFlavor(TestComputeProxy):
)
mocked.assert_not_called()
def test_flavors_detailed(self):
self.verify_list(self.proxy.flavors, flavor.FlavorDetail,
method_kwargs={"details": True, "query": 1},
expected_kwargs={"query": 1,
"base_path": "/flavors/detail"})
@mock.patch("openstack.proxy.Proxy._list", auto_spec=True)
@mock.patch("openstack.compute.v2.flavor.Flavor.fetch_extra_specs",
auto_spec=True)
def test_flavors_detailed(self, fetch_mock, list_mock):
res = self.proxy.flavors(details=True)
for r in res:
self.assertIsNotNone(r)
fetch_mock.assert_not_called()
list_mock.assert_called_with(
flavor.Flavor,
base_path="/flavors/detail"
)
def test_flavors_not_detailed(self):
self.verify_list(self.proxy.flavors, flavor.Flavor,
method_kwargs={"details": False, "query": 1},
expected_kwargs={"query": 1,
"base_path": "/flavors"})
@mock.patch("openstack.proxy.Proxy._list", auto_spec=True)
@mock.patch("openstack.compute.v2.flavor.Flavor.fetch_extra_specs",
auto_spec=True)
def test_flavors_not_detailed(self, fetch_mock, list_mock):
res = self.proxy.flavors(details=False)
for r in res:
self.assertIsNotNone(r)
fetch_mock.assert_not_called()
list_mock.assert_called_with(
flavor.Flavor,
base_path="/flavors"
)
@mock.patch("openstack.proxy.Proxy._list", auto_spec=True)
@mock.patch("openstack.compute.v2.flavor.Flavor.fetch_extra_specs",
auto_spec=True)
def test_flavors_query(self, fetch_mock, list_mock):
res = self.proxy.flavors(details=False, get_extra_specs=True, a="b")
for r in res:
fetch_mock.assert_called_with(self.proxy)
list_mock.assert_called_with(
flavor.Flavor,
base_path="/flavors",
a="b"
)
@mock.patch("openstack.proxy.Proxy._list", auto_spec=True)
@mock.patch("openstack.compute.v2.flavor.Flavor.fetch_extra_specs",
auto_spec=True)
def test_flavors_get_extra(self, fetch_mock, list_mock):
res = self.proxy.flavors(details=False, get_extra_specs=True)
for r in res:
fetch_mock.assert_called_with(self.proxy)
list_mock.assert_called_with(
flavor.Flavor,
base_path="/flavors"
)
def test_flavor_get_access(self):
self._verify("openstack.compute.v2.flavor.Flavor.get_access",

4
releasenotes/notes/flavor-cloud-layer-0b4d130ac1c5e7c4.yaml Normal file
View File

@ -0,0 +1,4 @@
---
other:
- Flavor operations of the cloud layer are switched to the rely on
the proxy layer