Ensure the use of volume endpoint in volumes apis

Currently, several of the volumes apis depend on callers to pass
service_type='volume' as a parameter when creating a client object,
to route correctly. The problem is, it makes it impossible for
callers to work with both the compute and volume endpoints at the
same time. They can either work with compute and have volumes.* calls
return 404, or they can work with volume and have servers.* images.*
flavors.* calls return 404.

The CLI sets service_type='volume' for client objects for the
following commands via a decorator:

volume-list
volume-show
volume-create
volume-delete
volume-snapshot-list
volume-snapshot-show
volume-snapshot-create
volume-snapshot-delete
volume-type-list
volume-type-create
volume-type-delete

With this change, the service_type 'volume' is set in the api,
so the decorators on the shell commands are no longer needed.

Closes-Bug: #1431154

Change-Id: I11b48ac14fa4c64d8aae528552ec5b363be384c5
changes/21/164321/10 2.23.0
melanie witt 8 years ago
parent b41d319308
commit e0f5072907
  1. 9
      novaclient/base.py
  2. 34
      novaclient/client.py
  3. 111
      novaclient/tests/functional/test_volumes_api.py
  4. 3
      novaclient/tests/unit/test_client.py
  5. 1
      novaclient/tests/unit/test_http.py
  6. 1
      novaclient/tests/unit/v2/fakes.py
  7. 1
      novaclient/tests/unit/v2/test_auth.py
  8. 11
      novaclient/v2/shell.py
  9. 26
      novaclient/v2/volume_snapshots.py
  10. 23
      novaclient/v2/volume_types.py
  11. 44
      novaclient/v2/volumes.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):
"""

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

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

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

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

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

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

@ -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='<volume>',
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='<availability-zone>',
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='<volume>', 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='<snapshot>',
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='<snapshot>',
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='<name>',
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='<id>',
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)

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

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

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

Loading…
Cancel
Save