openstack/python-cinderclient
Code Issues Proposed changes

Remove v2 support from the shell

Browse Source 
Also removes the v2 support from the generic client and restores a
skipped test.

Additionally, the cinderclient.tests.v2.test_availablity_zone module
depends on the v2.shell class, so move that module to v3, update the
v3 AvailablityZone class, and make appropriate adjustments to the
tests and test fixtures.

Change-Id: I7a3cca15f5944141d510a75af6684221c297963b
This commit is contained in:
Brian Rosmaita 2021-05-17 18:34:11 -04:00
parent f54b873ca3
commit 3502a5591a
13 changed files with 188 additions and 1454 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
cinderclient/api_versions.py
View File

@ -13,8 +13,6 @@
import functools
import logging
import os
import pkgutil
import re
from oslo_utils import strutils
@ -26,9 +24,8 @@ from cinderclient import utils
LOG = logging.getLogger(__name__)
# key is a deprecated version and value is an alternative version.
DEPRECATED_VERSIONS = {"2": "3"}
DEPRECATED_VERSION = "2.0"
# key is unsupported version, value is appropriate supported alternative
REPLACEMENT_VERSIONS = {"1": "3", "2": "3"}
MAX_VERSION = "3.64"
MIN_VERSION = "3.0"
@ -190,14 +187,12 @@ class VersionedMethod(object):
def get_available_major_versions():
# NOTE(andreykurilin): available clients version should not be
# hardcoded, so let's discover them.
matcher = re.compile(r"v[0-9]*$")
submodules = pkgutil.iter_modules([os.path.dirname(__file__)])
available_versions = [name[1:] for loader, name, ispkg in submodules
if matcher.search(name)]
return available_versions
# NOTE: the discovery code previously here assumed that if a v2
# module exists, it must contain a client. This will be False
# during the transition period when the v2 client is removed but
# we are still using other classes in that module. Right now there's
# only one client version available, so we simply hard-code it.
return ['3']
def check_major_version(api_version):
@ -224,11 +219,11 @@ def check_major_version(api_version):
def get_api_version(version_string):
"""Returns checked APIVersion object"""
version_string = str(version_string)
if version_string in DEPRECATED_VERSIONS:
LOG.warning("Version %(deprecated_version)s is deprecated, use "
"alternative version %(alternative)s instead.",
{"deprecated_version": version_string,
"alternative": DEPRECATED_VERSIONS[version_string]})
if version_string in REPLACEMENT_VERSIONS:
LOG.warning("Version %(old)s is not supported, use "
"supported version %(now)s instead.",
{"old": version_string,
"now": REPLACEMENT_VERSIONS[version_string]})
if strutils.is_int_like(version_string):
version_string = "%s.0" % version_string
@ -248,11 +243,20 @@ def _get_server_version_range(client):
client.version)
if not versions:
return APIVersion(), APIVersion()
msg = _("Server does not support microversions. You cannot use this "
"version of the cinderclient with the requested server. "
"Try using a cinderclient version less than 8.0.0.")
raise exceptions.UnsupportedVersion(msg)
for version in versions:
if '3.' in version.version:
return APIVersion(version.min_version), APIVersion(version.version)
# if we're still here, there's nothing we understand in the versions
msg = _("You cannot use this version of the cinderclient with the "
"requested server.")
raise exceptions.UnsupportedVersion(msg)
def get_highest_version(client):
"""Queries the server version info and returns highest supported
@ -278,12 +282,6 @@ def discover_version(client, requested_version):
server_start_version, server_end_version = _get_server_version_range(
client)
if not server_start_version and not server_end_version:
msg = ("Server does not support microversions. Changing server "
"version to %(min_version)s.")
LOG.debug(msg, {"min_version": DEPRECATED_VERSION})
return APIVersion(DEPRECATED_VERSION)
_validate_server_version(server_start_version, server_end_version)
# get the highest version the server can handle relative to the

44
cinderclient/client.py
View File

@ -57,16 +57,14 @@ except Exception:
pass
_VALID_VERSIONS = ['v2', 'v3']
_VALID_VERSIONS = ['v3']
V3_SERVICE_TYPE = 'volumev3'
V2_SERVICE_TYPE = 'volumev2'
SERVICE_TYPES = {'2': V2_SERVICE_TYPE,
'3': V3_SERVICE_TYPE}
SERVICE_TYPES = {'3': V3_SERVICE_TYPE}
REQ_ID_HEADER = 'X-OpenStack-Request-ID'
# tell keystoneclient that we can ignore the /v1|v2/{project_id} component of
# the service catalog when doing discovery lookups
for svc in ('volume', 'volumev2', 'volumev3'):
for svc in ('volume', 'volumev3'):
discover.add_catalog_discover_hack(svc, re.compile(r'/v[12]/\w+/?$'), '/')
@ -85,6 +83,8 @@ def get_server_version(url, insecure=False, cacert=None, cert=None):
:returns: APIVersion object for min and max version supported by
the server
"""
# NOTE: we (the client) don't support v2 anymore, but this function
# is checking the server version
min_version = "2.0"
current_version = "2.0"
@ -128,22 +128,37 @@ def get_server_version(url, insecure=False, cacert=None, cert=None):
current_version = version['version']
break
else:
# Set the values, but don't break out the loop here in case v3
# comes later
min_version = '2.0'
current_version = '2.0'
# keep looking in case this cloud is running v2 and
# we haven't seen v3 yet
continue
except exceptions.ClientException as e:
# NOTE: logging the warning but returning the lowest server API version
# supported in this OpenStack release is the legacy behavior, so that's
# what we do here
min_version = '3.0'
current_version = '3.0'
logger.warning("Error in server version query:%s\n"
"Returning APIVersion 2.0", str(e.message))
"Returning APIVersion 3.0", str(e.message))
return (api_versions.APIVersion(min_version),
api_versions.APIVersion(current_version))
def get_highest_client_server_version(url, insecure=False,
cacert=None, cert=None):
"""Returns highest supported version by client and server as a string."""
"""Returns highest supported version by client and server as a string.
:raises: UnsupportedVersion if the maximum supported by the server
is less than the minimum supported by the client
"""
min_server, max_server = get_server_version(url, insecure, cacert, cert)
max_client = api_versions.APIVersion(api_versions.MAX_VERSION)
min_client = api_versions.APIVersion(api_versions.MIN_VERSION)
if max_server < min_client:
msg = _("The maximum version supported by the server (%(srv)s) does "
"not meet the minimum version supported by this client "
"(%(cli)s)") % {"srv": str(max_server),
"cli": api_versions.MIN_VERSION}
raise exceptions.UnsupportedVersion(msg)
return min(max_server, max_client).get_string()
@ -769,7 +784,6 @@ def _get_client_class_and_version(version):
def get_client_class(version):
version_map = {
'2': 'cinderclient.v2.client.Client',
'3': 'cinderclient.v3.client.Client',
}
try:
@ -797,10 +811,6 @@ def discover_extensions(version):
def _discover_via_python_path():
for (module_loader, name, ispkg) in pkgutil.iter_modules():
if name.endswith('cinderclient_ext'):
if not hasattr(module_loader, 'load_module'):
# Python 2.6 compat: actually get an ImpImporter obj
module_loader = module_loader.find_module(name)
module = module_loader.load_module(name)
yield name, module
@ -845,7 +855,7 @@ def Client(version, *args, **kwargs):
Here ``VERSION`` can be a string or
``cinderclient.api_versions.APIVersion`` obj. If you prefer string value,
you can use ``2`` (deprecated now) or ``3.X`` (where X is a microversion).
you can use ``3`` or ``3.X`` (where X is a microversion).
Alternatively, you can create a client instance using the keystoneclient

41
cinderclient/shell.py
View File

@ -48,7 +48,6 @@ except Exception:
DEFAULT_MAJOR_OS_VOLUME_API_VERSION = "3"
DEFAULT_CINDER_ENDPOINT_TYPE = 'publicURL'
V2_SHELL = 'cinderclient.v2.shell'
V3_SHELL = 'cinderclient.v3.shell'
HINT_HELP_MSG = (" [hint: use '--os-volume-api-version' flag to show help "
"message for proper version]")
@ -202,7 +201,8 @@ class OpenStackCinderShell(object):
default=None),
help=_('Block Storage API version. '
'Accepts X, X.Y (where X is major and Y is minor '
'part).'
'part). NOTE: this client accepts only \'3\' for '
'the major version. '
'Default=env[OS_VOLUME_API_VERSION].'))
parser.add_argument('--os_volume_api_version',
help=argparse.SUPPRESS)
@ -356,10 +356,7 @@ class OpenStackCinderShell(object):
self.subcommands = {}
subparsers = parser.add_subparsers(metavar='<subcommand>')
if version.ver_major == 3:
actions_module = importutils.import_module(V3_SHELL)
else:
actions_module = importutils.import_module(V2_SHELL)
actions_module = importutils.import_module(V3_SHELL)
self._find_actions(subparsers, actions_module, version, do_help,
input_args)
@ -740,6 +737,10 @@ class OpenStackCinderShell(object):
except exc.AuthorizationFailure:
raise exc.CommandError("Unable to authorize user.")
# FIXME: this section figuring out the api version could use
# analysis and refactoring. See
# https://review.opendev.org/c/openstack/python-cinderclient/+/766882/
# for some ideas.
endpoint_api_version = None
# Try to get the API version from the endpoint URL. If that fails fall
# back to trying to use what the user specified via
@ -750,18 +751,26 @@ class OpenStackCinderShell(object):
self.cs.get_volume_api_version_from_endpoint()
except exc.UnsupportedVersion:
endpoint_api_version = options.os_volume_api_version
if api_version_input:
# FIXME: api_version_input is initialized as True at the beginning
# of this function and never modified
if api_version_input and endpoint_api_version:
logger.warning("Cannot determine the API version from "
"the endpoint URL. Falling back to the "
"user-specified version: %s",
endpoint_api_version)
else:
elif endpoint_api_version:
logger.warning("Cannot determine the API version from the "
"endpoint URL or user input. Falling back "
"to the default API version: %s",
endpoint_api_version)
else:
msg = _("Cannot determine API version. Please specify by "
"using --os-volume-api-version option.")
raise exc.UnsupportedVersion(msg)
API_MAX_VERSION = api_versions.APIVersion(api_versions.MAX_VERSION)
# FIXME: the endpoint_api_version[0] can ONLY be '3' now, so the
# above line should probably be ripped out and this condition removed
if endpoint_api_version[0] == '3':
disc_client = client.Client(API_MAX_VERSION,
os_username,
@ -807,14 +816,9 @@ class OpenStackCinderShell(object):
os_auth_url,
client_args):
if (os_api_version.get_major_version() in
api_versions.DEPRECATED_VERSIONS):
discovered_version = api_versions.DEPRECATED_VERSION
os_service_type = 'volume'
else:
discovered_version = api_versions.discover_version(
current_client,
os_api_version)
discovered_version = api_versions.discover_version(
current_client,
os_api_version)
if not os_endpoint_type:
os_endpoint_type = DEFAULT_CINDER_ENDPOINT_TYPE
@ -841,6 +845,11 @@ class OpenStackCinderShell(object):
return current_client, discovered_version
def _discover_service_type(self, discovered_version):
# FIXME: this function is either no longer needed or could use a
# refactoring. The official service type is 'block-storage',
# which isn't even present here. (Devstack creates 2 service
# types which it maps to v3: 'block-storage' and 'volumev3'.
# The default 'catalog_type' in tempest is 'volumev3'.)
SERVICE_TYPES = {'1': 'volume', '2': 'volumev2', '3': 'volumev3'}
major_version = discovered_version.get_major_version()
service_type = SERVICE_TYPES[major_version]

16
cinderclient/tests/unit/fixture_data/client.py
View File

@ -14,6 +14,7 @@ from keystoneauth1 import fixture
from cinderclient.tests.unit.fixture_data import base
from cinderclient.v2 import client as v2client
from cinderclient.v3 import client as v3client
class Base(base.Fixture):
@ -46,3 +47,18 @@ class V2(Base):
api_key='xx',
project_id='xx',
auth_url=self.identity_url)
class V3(Base):
def __init__(self, *args, **kwargs):
super(V3, self).__init__(*args, **kwargs)
svc = self.token.add_service('volumev3')
svc.add_endpoint(self.volume_url)
def new_client(self):
return v3client.Client(username='xx',
api_key='xx',
project_id='xx',
auth_url=self.identity_url)

4
cinderclient/tests/unit/fixture_data/keystone_client.py
View File

@ -153,7 +153,7 @@ def generate_v2_project_scoped_token(**kwargs):
],
'endpoints_links': [],
'name': None,
'type': 'volumev2'
'type': 'volumev3'
}
# Add multiple Cinder endpoints
@ -163,7 +163,7 @@ def generate_v2_project_scoped_token(**kwargs):
name = "cinder%i" % count
# Assign the service name and a unique endpoint
endpoint_copy['endpoints'][0]['publicURL'] = \
'http://%s.api.com/v2' % name
'http://%s.api.com/v3' % name
endpoint_copy['name'] = name
o['access']['serviceCatalog'].append(endpoint_copy)

39
cinderclient/tests/unit/test_api_versions.py
View File

@ -18,7 +18,6 @@ from unittest import mock
import ddt
from cinderclient import api_versions
from cinderclient import client as base_client
from cinderclient import exceptions
from cinderclient.tests.unit import test_utils
from cinderclient.tests.unit import utils
@ -212,6 +211,14 @@ class DiscoverVersionTestCase(utils.TestCase):
self.fake_client.services.server_api_version.return_value = val
@ddt.data(
# what the data mean:
# items 1, 2: client min, max
# items 3, 4: server min, max
# item 5: user's requested API version
# item 6: should this raise an exception?
# item 7: version that should be returned when no exception
# item 8: what client.services.server_api_version should return
# when called by _get_server_version_range in discover_version
("3.1", "3.3", "3.4", "3.7", "3.3", True), # Server too new
("3.9", "3.10", "3.0", "3.3", "3.10", True), # Server too old
("3.3", "3.9", "3.7", "3.17", "3.9", False), # Requested < server
@ -222,9 +229,8 @@ class DiscoverVersionTestCase(utils.TestCase):
# downgraded because of both:
("3.5", "3.7", "3.0", "3.8", "3.9", False, "3.7"),
("3.5", "3.5", "3.0", "3.5", "3.5", False), # Server & client same
("3.5", "3.5", "3.0", "3.5", "3.5", False, "2.0", []), # Pre-micro
("3.5", "3.5", None, None, "3.5", True, None, []), # Pre-micro
("3.1", "3.11", "3.4", "3.7", "3.7", False), # Requested in range
("3.1", "3.11", None, None, "3.7", False), # Server w/o support
("3.5", "3.5", "3.0", "3.5", "1.0", True) # Requested too old
)
@ddt.unpack
@ -240,21 +246,23 @@ class DiscoverVersionTestCase(utils.TestCase):
api_versions.MIN_VERSION = client_min
if exp_range:
self.assertRaisesRegex(exceptions.UnsupportedVersion,
".*range is '%s' to '%s'.*" %
(server_min, server_max),
api_versions.discover_version,
self.fake_client,
api_versions.APIVersion(requested_version))
exc = self.assertRaises(exceptions.UnsupportedVersion,
api_versions.discover_version,
self.fake_client,
api_versions.APIVersion(requested_version))
if ret_val is not None:
self.assertIn("Server does not support microversions",
str(exc))
else:
self.assertIn("range is '%s' to '%s'" %
(server_min, server_max), str(exc))
else:
discovered_version = api_versions.discover_version(
self.fake_client,
api_versions.APIVersion(requested_version))
version = requested_version
if server_min is None and server_max is None:
version = api_versions.DEPRECATED_VERSION
elif end_version is not None:
if end_version is not None:
version = end_version
self.assertEqual(version,
discovered_version.get_string())
@ -266,10 +274,3 @@ class DiscoverVersionTestCase(utils.TestCase):
highest_version = api_versions.get_highest_version(self.fake_client)
self.assertEqual("3.14", highest_version.get_string())
self.assertTrue(self.fake_client.services.server_api_version.called)
def test_get_highest_version_bad_client(self):
"""Tests that we gracefully handle the wrong version of client."""
v2_client = base_client.Client('2.0')
ex = self.assertRaises(exceptions.UnsupportedVersion,
api_versions.get_highest_version, v2_client)
self.assertIn('Invalid client version 2.0 to get', str(ex))

33
cinderclient/tests/unit/test_client.py
View File

@ -33,8 +33,9 @@ import cinderclient.v2.client
class ClientTest(utils.TestCase):
def test_get_client_class_v2(self):
output = cinderclient.client.get_client_class('2')
self.assertEqual(cinderclient.v2.client.Client, output)
self.assertRaises(cinderclient.exceptions.UnsupportedVersion,
cinderclient.client.get_client_class,
'2')
def test_get_client_class_unknown(self):
self.assertRaises(cinderclient.exceptions.UnsupportedVersion,
@ -81,10 +82,14 @@ class ClientTest(utils.TestCase):
def test_versions(self):
v2_url = 'http://fakeurl/v2/tenants'
v3_url = 'http://fakeurl/v3/tenants'
unknown_url = 'http://fakeurl/v9/tenants'
self.assertEqual('2',
cinderclient.client.get_volume_api_from_url(v2_url))
self.assertRaises(cinderclient.exceptions.UnsupportedVersion,
cinderclient.client.get_volume_api_from_url,
v2_url)
self.assertEqual('3',
cinderclient.client.get_volume_api_from_url(v3_url))
self.assertRaises(cinderclient.exceptions.UnsupportedVersion,
cinderclient.client.get_volume_api_from_url,
unknown_url)
@ -318,6 +323,7 @@ class GetAPIVersionTestCase(utils.TestCase):
@mock.patch('cinderclient.client.requests.get')
def test_get_server_version_v2(self, mock_request):
# Why are we testing this? Because we can!
mock_response = utils.TestResponse({
"status_code": 200,
@ -329,6 +335,7 @@ class GetAPIVersionTestCase(utils.TestCase):
url = "http://192.168.122.127:8776/v2/e5526285ebd741b1819393f772f11fc3"
min_version, max_version = cinderclient.client.get_server_version(url)
self.assertEqual(api_versions.APIVersion('2.0'), min_version)
self.assertEqual(api_versions.APIVersion('2.0'), max_version)
@ -427,3 +434,21 @@ class GetAPIVersionTestCase(utils.TestCase):
cinderclient.client.get_highest_client_server_version(url))
expected = version if version == '3.12' else '3.16'
self.assertEqual(expected, highest)
@mock.patch('cinderclient.client.requests.get')
def test_get_highest_client_server_version_negative(self,
mock_request):
mock_response = utils.TestResponse({
"status_code": 200,
"text": json.dumps(fakes.fake_request_get_no_v3())
})
mock_request.return_value = mock_response
url = "http://192.168.122.127:8776/v3/e5526285ebd741b1819393f772f11fc3"
self.assertRaises(exceptions.UnsupportedVersion,
cinderclient.client.
get_highest_client_server_version,
url)

24
cinderclient/tests/unit/test_shell.py
View File

@ -13,9 +13,9 @@
import argparse
import io
import json
import re
import sys
import unittest
from unittest import mock
import ddt
@ -35,6 +35,7 @@ from cinderclient import shell
from cinderclient.tests.unit import fake_actions_module
from cinderclient.tests.unit.fixture_data import keystone_client
from cinderclient.tests.unit import utils
from cinderclient.tests.unit.v3 import fakes
@ddt.ddt
@ -205,8 +206,13 @@ class ShellTest(utils.TestCase):
os_auth_url = "http://multiple.service.names/v2.0"
mocker.register_uri('POST', os_auth_url + "/tokens",
text=keystone_client.keystone_request_callback)
# microversion support requires us to make a versions request
# to the endpoint to see exactly what is supported by the server
mocker.register_uri('GET',
"http://cinder%i.api.com/v2/volumes/detail"
"http://cinder%i.api.com/"
% count, text=json.dumps(fakes.fake_request_get()))
mocker.register_uri('GET',
"http://cinder%i.api.com/v3/volumes/detail"
% count, text='{"volumes": []}')
self.make_env(include={'OS_AUTH_URL': os_auth_url,
'CINDER_SERVICE_NAME': 'cinder%i' % count})
@ -219,7 +225,6 @@ class ShellTest(utils.TestCase):
_shell.main,
['list', '--name', 'abc', '--filters', 'name=xyz'])
@unittest.skip("Skip cuz I broke it")
def test_cinder_service_name(self):
# Failing with 'No mock address' means we are not
# choosing the correct endpoint
@ -248,14 +253,19 @@ class ShellTest(utils.TestCase):
tenant_name=self.FAKE_ENV['OS_PROJECT_NAME'],
username=self.FAKE_ENV['OS_USERNAME'])
@mock.patch('cinderclient.api_versions.discover_version',
return_value=api_versions.APIVersion("3.0"))
@requests_mock.Mocker()
def test_noauth_plugin(self, mocker):
os_auth_url = "http://example.com/v2"
def test_noauth_plugin(self, mock_disco, mocker):
# just to prove i'm not crazy about the mock parameter ordering
self.assertTrue(requests_mock.mocker.Mocker, type(mocker))
os_volume_url = "http://example.com/volumes/v3"
mocker.register_uri('GET',
"%s/volumes/detail"
% os_auth_url, text='{"volumes": []}')
% os_volume_url, text='{"volumes": []}')
_shell = shell.OpenStackCinderShell()
args = ['--os-endpoint', os_auth_url,
args = ['--os-endpoint', os_volume_url,
'--os-auth-type', 'noauth', '--os-user-id',
'admin', '--os-project-id', 'admin', 'list']
_shell.main(args)

1358
cinderclient/tests/unit/v2/test_shell.py
View File

File diff suppressed because it is too large Load Diff

6
cinderclient/tests/unit/v2/test_availability_zone.py → cinderclient/tests/unit/v3/test_availability_zone.py
View File

@ -14,8 +14,8 @@
# License for the specific language governing permissions and limitations
# under the License.
from cinderclient.v2 import availability_zones
from cinderclient.v2 import shell
from cinderclient.v3 import availability_zones
from cinderclient.v3 import shell
from cinderclient.tests.unit.fixture_data import availability_zones as azfixture # noqa
from cinderclient.tests.unit.fixture_data import client
@ -24,7 +24,7 @@ from cinderclient.tests.unit import utils
class AvailabilityZoneTest(utils.FixturedTestCase):
client_fixture_class = client.V2
client_fixture_class = client.V3
data_fixture_class = azfixture.Fixture
def _assertZone(self, zone, name, status):

25
cinderclient/v3/availability_zones.py
View File

@ -16,4 +16,27 @@
"""Availability Zone interface (v3 extension)"""
from cinderclient.v2.availability_zones import * # noqa
from cinderclient import base
class AvailabilityZone(base.Resource):
NAME_ATTR = 'display_name'
def __repr__(self):
return "<AvailabilityZone: %s>" % self.zoneName
class AvailabilityZoneManager(base.ManagerWithFind):
"""Manage :class:`AvailabilityZone` resources."""
resource_class = AvailabilityZone
def list(self, detailed=False):
"""Lists all availability zones.
:rtype: list of :class:`AvailabilityZone`
"""
if detailed is True:
return self._list("/os-availability-zone/detail",
"availabilityZoneInfo")
else:
return self._list("/os-availability-zone", "availabilityZoneInfo")

4
cinderclient/v3/shell.py
View File

@ -27,8 +27,8 @@ from cinderclient import exceptions
from cinderclient import shell_utils
from cinderclient import utils
from cinderclient.v2.shell import * # noqa
from cinderclient.v2.shell import CheckSizeArgForCreate
from cinderclient.v3.shell_base import * # noqa
from cinderclient.v3.shell_base import CheckSizeArgForCreate
FILTER_DEPRECATED = ("This option is deprecated and will be removed in "
"newer release. Please use '--filters' option which "

0
cinderclient/v2/shell.py → cinderclient/v3/shell_base.py
View File