diff --git a/novaclient/base.py b/novaclient/base.py index 8cee35ae2..01b4bac94 100644 --- a/novaclient/base.py +++ b/novaclient/base.py @@ -80,6 +80,15 @@ class Manager(base.HookableMixin): return [obj_class(self, res, loaded=True) for res in data if res] + @contextlib.contextmanager + def alternate_service_type(self, service_type): + original_service_type = self.api.client.service_type + self.api.client.service_type = service_type + try: + yield + finally: + self.api.client.service_type = original_service_type + @contextlib.contextmanager def completion_cache(self, cache_type, obj_class, mode): """ diff --git a/novaclient/client.py b/novaclient/client.py index c40c21a18..32eb77505 100644 --- a/novaclient/client.py +++ b/novaclient/client.py @@ -212,6 +212,11 @@ class HTTPClient(object): # otherwise we will get all the requests logging messages rql.setLevel(logging.WARNING) + # NOTE(melwitt): Service catalog is only set if bypass_url isn't + # used. Otherwise, we can cache using services_url. + self.service_catalog = None + self.services_url = {} + def use_token_cache(self, use_it): self.os_cache = use_it @@ -405,7 +410,12 @@ class HTTPClient(object): path = re.sub(r'v[1-9]/[a-z0-9]+$', '', path) url = parse.urlunsplit((scheme, netloc, path, None, None)) else: - url = self.management_url + url + if self.service_catalog: + url = self.get_service_url(self.service_type) + url + else: + # NOTE(melwitt): The service catalog is not available + # when bypass_url is used. + url = self.management_url + url # Perform the request once. If we get a 401 back then it # might be because the auth token expired, so try to @@ -448,6 +458,19 @@ class HTTPClient(object): def delete(self, url, **kwargs): return self._cs_request(url, 'DELETE', **kwargs) + def get_service_url(self, service_type): + if service_type not in self.services_url: + url = self.service_catalog.url_for( + attr='region', + filter_value=self.region_name, + endpoint_type=self.endpoint_type, + service_type=service_type, + service_name=self.service_name, + volume_service_name=self.volume_service_name,) + url = url.rstrip('/') + self.services_url[service_type] = url + return self.services_url[service_type] + def _extract_service_catalog(self, url, resp, body, extract_token=True): """See what the auth service told us and process the response. We may get redirected to another site, fail or actually get @@ -464,14 +487,7 @@ class HTTPClient(object): self.auth_token = self.service_catalog.get_token() self.tenant_id = self.service_catalog.get_tenant_id() - management_url = self.service_catalog.url_for( - attr='region', - filter_value=self.region_name, - endpoint_type=self.endpoint_type, - service_type=self.service_type, - service_name=self.service_name, - volume_service_name=self.volume_service_name,) - self.management_url = management_url.rstrip('/') + self.management_url = self.get_service_url(self.service_type) return None except exceptions.AmbiguousEndpoints: print(_("Found more than one valid endpoint. Use a more " diff --git a/novaclient/tests/functional/test_volumes_api.py b/novaclient/tests/functional/test_volumes_api.py new file mode 100644 index 000000000..e427c2e21 --- /dev/null +++ b/novaclient/tests/functional/test_volumes_api.py @@ -0,0 +1,111 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import os +import time +import uuid + +import six.moves + +from novaclient import client +from novaclient import exceptions +from novaclient.tests.functional import base + + +def wait_for_delete(test, name, thing, get_func): + thing.delete() + for x in six.moves.range(60): + try: + thing = get_func(thing.id) + except exceptions.NotFound: + break + time.sleep(1) + else: + test.fail('%s %s still not deleted after 60s' % (name, thing.id)) + + +class TestVolumesAPI(base.ClientTestBase): + + def setUp(self): + super(TestVolumesAPI, self).setUp() + user = os.environ['OS_USERNAME'] + passwd = os.environ['OS_PASSWORD'] + tenant = os.environ['OS_TENANT_NAME'] + auth_url = os.environ['OS_AUTH_URL'] + + self.client = client.Client(2, user, passwd, tenant, auth_url=auth_url) + + def test_volumes_snapshots_types_create_get_list_delete(self): + # Create a volume + volume = self.client.volumes.create(1) + + # Make sure we can still list servers after using the volume endpoint + self.client.servers.list() + + # This cleanup tests volume delete + self.addCleanup(volume.delete) + + # Wait for the volume to become available + for x in six.moves.range(60): + volume = self.client.volumes.get(volume.id) + if volume.status == 'available': + break + elif volume.status == 'error': + self.fail('Volume %s is in error state' % volume.id) + time.sleep(1) + else: + self.fail('Volume %s not available after 60s' % volume.id) + + # List all volumes + self.client.volumes.list() + + # Create a volume snapshot + snapshot = self.client.volume_snapshots.create(volume.id) + + # This cleanup tests volume snapshot delete. The volume + # can't be deleted until the dependent snapshot is gone + self.addCleanup(wait_for_delete, self, 'Snapshot', snapshot, + self.client.volume_snapshots.get) + + # Wait for the snapshot to become available + for x in six.moves.range(60): + snapshot = self.client.volume_snapshots.get(snapshot.id) + if snapshot.status == 'available': + break + elif snapshot.status == 'error': + self.fail('Snapshot %s is in error state' % snapshot.id) + time.sleep(1) + else: + self.fail('Snapshot %s not available after 60s' % snapshot.id) + + # List snapshots + self.client.volume_snapshots.list() + + # List servers again to make sure things are still good + self.client.servers.list() + + # Create a volume type + # TODO(melwitt): Use a better random name + name = str(uuid.uuid4()) + volume_type = self.client.volume_types.create(name) + + # This cleanup tests volume type delete + self.addCleanup(self.client.volume_types.delete, volume_type.id) + + # Get the volume type + volume_type = self.client.volume_types.get(volume_type.id) + + # List all volume types + self.client.volume_types.list() + + # One more servers list + self.client.servers.list() diff --git a/novaclient/tests/unit/test_client.py b/novaclient/tests/unit/test_client.py index db4c87d09..d240321f7 100644 --- a/novaclient/tests/unit/test_client.py +++ b/novaclient/tests/unit/test_client.py @@ -86,6 +86,7 @@ class ClientTest(utils.TestCase): auth_url="http://www.blah.com") instance.auth_token = 'foobar' instance.management_url = 'http://example.com' + instance.get_service_url = mock.Mock(return_value='http://example.com') instance.version = 'v2.0' mock_request = mock.Mock() mock_request.side_effect = novaclient.exceptions.Unauthorized(401) @@ -136,6 +137,8 @@ class ClientTest(utils.TestCase): auth_url="http://www.blah.com") instance.auth_token = 'foobar' instance.management_url = management_url % projectid + mock_get_service_url = mock.Mock(return_value=instance.management_url) + instance.get_service_url = mock_get_service_url instance.version = 'v2.0' # If passing None as the part of url, a client accesses the url which diff --git a/novaclient/tests/unit/test_http.py b/novaclient/tests/unit/test_http.py index 5e676828a..a80d94426 100644 --- a/novaclient/tests/unit/test_http.py +++ b/novaclient/tests/unit/test_http.py @@ -82,6 +82,7 @@ def get_authed_client(): cl = get_client() cl.management_url = "http://example.com" cl.auth_token = "token" + cl.get_service_url = mock.Mock(return_value="http://example.com") return cl diff --git a/novaclient/tests/unit/v2/fakes.py b/novaclient/tests/unit/v2/fakes.py index 07db412b3..343c421e2 100644 --- a/novaclient/tests/unit/v2/fakes.py +++ b/novaclient/tests/unit/v2/fakes.py @@ -2196,6 +2196,7 @@ class FakeSessionMockClient(base_client.SessionClient, FakeHTTPClient): self.callstack = [] self.auth = mock.Mock() self.session = mock.Mock() + self.service_type = 'service_type' self.auth.get_auth_ref.return_value.project_id = 'tenant_id' diff --git a/novaclient/tests/unit/v2/test_auth.py b/novaclient/tests/unit/v2/test_auth.py index 95e96ded1..696428a4a 100644 --- a/novaclient/tests/unit/v2/test_auth.py +++ b/novaclient/tests/unit/v2/test_auth.py @@ -363,6 +363,7 @@ class AuthenticationTests(utils.TestCase): "project_id", utils.AUTH_URL) http_client = cs.client http_client.management_url = '' + http_client.get_service_url = mock.Mock(return_value='') mock_request = mock.Mock(return_value=(None, None)) @mock.patch.object(http_client, 'request', mock_request) diff --git a/novaclient/v2/shell.py b/novaclient/v2/shell.py index 6ce1681ab..7e3a53100 100644 --- a/novaclient/v2/shell.py +++ b/novaclient/v2/shell.py @@ -1983,7 +1983,6 @@ def _translate_availability_zone_keys(collection): type=int, const=1, help=argparse.SUPPRESS) -@cliutils.service_type('volume') def do_volume_list(cs, args): """List all the volumes.""" search_opts = {'all_tenants': args.all_tenants} @@ -2002,7 +2001,6 @@ def do_volume_list(cs, args): 'volume', metavar='', help=_('Name or ID of the volume.')) -@cliutils.service_type('volume') def do_volume_show(cs, args): """Show details about a volume.""" volume = _find_volume(cs, args.volume) @@ -2055,7 +2053,6 @@ def do_volume_show(cs, args): '--availability-zone', metavar='', help=_('Optional Availability Zone for volume. (Default=None)'), default=None) -@cliutils.service_type('volume') def do_volume_create(cs, args): """Add a new volume.""" volume = cs.volumes.create(args.size, @@ -2072,7 +2069,6 @@ def do_volume_create(cs, args): 'volume', metavar='', nargs='+', help=_('Name or ID of the volume(s) to delete.')) -@cliutils.service_type('volume') def do_volume_delete(cs, args): """Remove volume(s).""" for volume in args.volume: @@ -2139,7 +2135,6 @@ def do_volume_detach(cs, args): args.attachment_id) -@cliutils.service_type('volume') def do_volume_snapshot_list(cs, _args): """List all the snapshots.""" snapshots = cs.volume_snapshots.list() @@ -2152,7 +2147,6 @@ def do_volume_snapshot_list(cs, _args): 'snapshot', metavar='', help=_('Name or ID of the snapshot.')) -@cliutils.service_type('volume') def do_volume_snapshot_show(cs, args): """Show details about a snapshot.""" snapshot = _find_volume_snapshot(cs, args.snapshot) @@ -2185,7 +2179,6 @@ def do_volume_snapshot_show(cs, args): @cliutils.arg( '--display_description', help=argparse.SUPPRESS) -@cliutils.service_type('volume') def do_volume_snapshot_create(cs, args): """Add a new snapshot.""" snapshot = cs.volume_snapshots.create(args.volume_id, @@ -2199,7 +2192,6 @@ def do_volume_snapshot_create(cs, args): 'snapshot', metavar='', help=_('Name or ID of the snapshot to delete.')) -@cliutils.service_type('volume') def do_volume_snapshot_delete(cs, args): """Remove a snapshot.""" snapshot = _find_volume_snapshot(cs, args.snapshot) @@ -2210,7 +2202,6 @@ def _print_volume_type_list(vtypes): utils.print_list(vtypes, ['ID', 'Name']) -@cliutils.service_type('volume') def do_volume_type_list(cs, args): """Print a list of available 'volume types'.""" vtypes = cs.volume_types.list() @@ -2221,7 +2212,6 @@ def do_volume_type_list(cs, args): 'name', metavar='', help=_("Name of the new volume type")) -@cliutils.service_type('volume') def do_volume_type_create(cs, args): """Create a new volume type.""" vtype = cs.volume_types.create(args.name) @@ -2232,7 +2222,6 @@ def do_volume_type_create(cs, args): 'id', metavar='', help=_("Unique ID of the volume type to delete")) -@cliutils.service_type('volume') def do_volume_type_delete(cs, args): """Delete a specific volume type.""" cs.volume_types.delete(args.id) diff --git a/novaclient/v2/volume_snapshots.py b/novaclient/v2/volume_snapshots.py index d3bc91808..6ff6deb9e 100644 --- a/novaclient/v2/volume_snapshots.py +++ b/novaclient/v2/volume_snapshots.py @@ -55,11 +55,12 @@ class SnapshotManager(base.ManagerWithFind): :param display_description: Description of the snapshot :rtype: :class:`Snapshot` """ - body = {'snapshot': {'volume_id': volume_id, - 'force': force, - 'display_name': display_name, - 'display_description': display_description}} - return self._create('/snapshots', body, 'snapshot') + with self.alternate_service_type('volume'): + body = {'snapshot': {'volume_id': volume_id, + 'force': force, + 'display_name': display_name, + 'display_description': display_description}} + return self._create('/snapshots', body, 'snapshot') def get(self, snapshot_id): """ @@ -68,7 +69,8 @@ class SnapshotManager(base.ManagerWithFind): :param snapshot_id: The ID of the snapshot to get. :rtype: :class:`Snapshot` """ - return self._get("/snapshots/%s" % snapshot_id, "snapshot") + with self.alternate_service_type('volume'): + return self._get("/snapshots/%s" % snapshot_id, "snapshot") def list(self, detailed=True): """ @@ -76,10 +78,11 @@ class SnapshotManager(base.ManagerWithFind): :rtype: list of :class:`Snapshot` """ - if detailed is True: - return self._list("/snapshots/detail", "snapshots") - else: - return self._list("/snapshots", "snapshots") + with self.alternate_service_type('volume'): + if detailed is True: + return self._list("/snapshots/detail", "snapshots") + else: + return self._list("/snapshots", "snapshots") def delete(self, snapshot): """ @@ -87,4 +90,5 @@ class SnapshotManager(base.ManagerWithFind): :param snapshot: The :class:`Snapshot` to delete. """ - self._delete("/snapshots/%s" % base.getid(snapshot)) + with self.alternate_service_type('volume'): + self._delete("/snapshots/%s" % base.getid(snapshot)) diff --git a/novaclient/v2/volume_types.py b/novaclient/v2/volume_types.py index 3d1c7f531..e36225b45 100644 --- a/novaclient/v2/volume_types.py +++ b/novaclient/v2/volume_types.py @@ -41,7 +41,8 @@ class VolumeTypeManager(base.ManagerWithFind): :rtype: list of :class:`VolumeType`. """ - return self._list("/types", "volume_types") + with self.alternate_service_type('volume'): + return self._list("/types", "volume_types") def get(self, volume_type): """ @@ -50,7 +51,9 @@ class VolumeTypeManager(base.ManagerWithFind): :param volume_type: The ID of the :class:`VolumeType` to get. :rtype: :class:`VolumeType` """ - return self._get("/types/%s" % base.getid(volume_type), "volume_type") + with self.alternate_service_type('volume'): + return self._get("/types/%s" % base.getid(volume_type), + "volume_type") def delete(self, volume_type): """ @@ -58,7 +61,8 @@ class VolumeTypeManager(base.ManagerWithFind): :param volume_type: The ID of the :class:`VolumeType` to get. """ - self._delete("/types/%s" % base.getid(volume_type)) + with self.alternate_service_type('volume'): + self._delete("/types/%s" % base.getid(volume_type)) def create(self, name): """ @@ -67,11 +71,10 @@ class VolumeTypeManager(base.ManagerWithFind): :param name: Descriptive name of the volume type :rtype: :class:`VolumeType` """ - - body = { - "volume_type": { - "name": name, + with self.alternate_service_type('volume'): + body = { + "volume_type": { + "name": name, + } } - } - - return self._create("/types", body, "volume_type") + return self._create("/types", body, "volume_type") diff --git a/novaclient/v2/volumes.py b/novaclient/v2/volumes.py index 99e3d6fed..182698ff5 100644 --- a/novaclient/v2/volumes.py +++ b/novaclient/v2/volumes.py @@ -60,14 +60,16 @@ class VolumeManager(base.ManagerWithFind): :rtype: :class:`Volume` :param imageRef: reference to an image stored in glance """ - body = {'volume': {'size': size, - 'snapshot_id': snapshot_id, - 'display_name': display_name, - 'display_description': display_description, - 'volume_type': volume_type, - 'availability_zone': availability_zone, - 'imageRef': imageRef}} - return self._create('/volumes', body, 'volume') + # NOTE(melwitt): Ensure we use the volume endpoint for this call + with self.alternate_service_type('volume'): + body = {'volume': {'size': size, + 'snapshot_id': snapshot_id, + 'display_name': display_name, + 'display_description': display_description, + 'volume_type': volume_type, + 'availability_zone': availability_zone, + 'imageRef': imageRef}} + return self._create('/volumes', body, 'volume') def get(self, volume_id): """ @@ -76,7 +78,8 @@ class VolumeManager(base.ManagerWithFind): :param volume_id: The ID of the volume to get. :rtype: :class:`Volume` """ - return self._get("/volumes/%s" % volume_id, "volume") + with self.alternate_service_type('volume'): + return self._get("/volumes/%s" % volume_id, "volume") def list(self, detailed=True, search_opts=None): """ @@ -84,19 +87,21 @@ class VolumeManager(base.ManagerWithFind): :rtype: list of :class:`Volume` """ - search_opts = search_opts or {} + with self.alternate_service_type('volume'): + search_opts = search_opts or {} - if 'name' in search_opts.keys(): - search_opts['display_name'] = search_opts.pop('name') + if 'name' in search_opts.keys(): + search_opts['display_name'] = search_opts.pop('name') - qparams = dict((k, v) for (k, v) in six.iteritems(search_opts) if v) + qparams = dict((k, v) for (k, v) in + six.iteritems(search_opts) if v) - query_string = '?%s' % parse.urlencode(qparams) if qparams else '' + query_str = '?%s' % parse.urlencode(qparams) if qparams else '' - if detailed is True: - return self._list("/volumes/detail%s" % query_string, "volumes") - else: - return self._list("/volumes%s" % query_string, "volumes") + if detailed is True: + return self._list("/volumes/detail%s" % query_str, "volumes") + else: + return self._list("/volumes%s" % query_str, "volumes") def delete(self, volume): """ @@ -104,7 +109,8 @@ class VolumeManager(base.ManagerWithFind): :param volume: The :class:`Volume` to delete. """ - self._delete("/volumes/%s" % base.getid(volume)) + with self.alternate_service_type('volume'): + self._delete("/volumes/%s" % base.getid(volume)) def create_server_volume(self, server_id, volume_id, device): """