openstack/deb-nova
Code Issues Proposed changes

Correct use of nova.image.glance in compute API

Browse Source 
The nova.api.openstack.compute.images modules was using the
nova.image.glance module directly, instead of using the new,
standardized nova.image.API module.

This patch corrects that usage and updates the associated unit tests to
use mock instead of the nova.tests.api.fake.stub_out_glance() method.

Copies the nova.tests.api.fake._make_image_fixtures() call to a new
nova.tests.image_fixtures module, and corrects a number of errors in the
original function, including not returning datetime objects for
timestamp fields (as the nova.image.API returns) and ensuring that the
ID field is a string in the returned fixture dicts.

The _make_image_fixtures() function and the stub_out_glance() function
will be removed in the next patch, which fixes up the image_metadata
module, which is the last unit test that uses them.

Change-Id: Icd22ff3f438b8ba476f2637c7d4129ea6d0f45e8
This commit is contained in:
Jay Pipes
2014-07-20 13:08:09 -04:00
parent b3144d30d3
commit c2ec61425f
6 changed files with 138 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
nova/api/openstack/compute/images.py
View File

@@ -21,7 +21,7 @@ from nova.api.openstack import wsgi
from nova.api.openstack import xmlutil
from nova import exception
from nova.i18n import _
import nova.image.glance
import nova.image
import nova.utils
@@ -89,15 +89,9 @@ class Controller(wsgi.Controller):
_view_builder_class = views_images.ViewBuilder
def __init__(self, image_service=None, **kwargs):
"""Initialize new `ImageController`.
:param image_service: `nova.image.glance:GlanceImageService`
"""
def __init__(self, **kwargs):
super(Controller, self).__init__(**kwargs)
self._image_service = (image_service or
nova.image.glance.get_default_image_service())
self._image_api = nova.image.API()
def _get_filters(self, req):
"""Return a dictionary of query param filters from the request.
@@ -136,7 +130,7 @@ class Controller(wsgi.Controller):
context = req.environ['nova.context']
try:
image = self._image_service.show(context, id)
image = self._image_api.get(context, id)
except (exception.NotFound, exception.InvalidImageRef):
explanation = _("Image not found.")
raise webob.exc.HTTPNotFound(explanation=explanation)
@@ -152,7 +146,7 @@ class Controller(wsgi.Controller):
"""
context = req.environ['nova.context']
try:
self._image_service.delete(context, id)
self._image_api.delete(context, id)
except exception.ImageNotFound:
explanation = _("Image not found.")
raise webob.exc.HTTPNotFound(explanation=explanation)
@@ -178,8 +172,8 @@ class Controller(wsgi.Controller):
params[key] = val
try:
images = self._image_service.detail(context, filters=filters,
**page_params)
images = self._image_api.get_all(context, filters=filters,
**page_params)
except exception.Invalid as e:
raise webob.exc.HTTPBadRequest(explanation=e.format_message())
return self._view_builder.index(req, images)
@@ -198,8 +192,8 @@ class Controller(wsgi.Controller):
for key, val in page_params.iteritems():
params[key] = val
try:
images = self._image_service.detail(context, filters=filters,
**page_params)
images = self._image_api.get_all(context, filters=filters,
**page_params)
except exception.Invalid as e:
raise webob.exc.HTTPBadRequest(explanation=e.format_message())

5
nova/tests/api/openstack/compute/plugins/v3/test_server_actions.py
View File

@@ -29,7 +29,6 @@ from nova import db
from nova import exception
from nova.image import glance
from nova import objects
from nova.openstack.common import importutils
from nova.openstack.common import jsonutils
from nova.openstack.common import uuidutils
from nova import test
@@ -86,13 +85,9 @@ class ServerActionsControllerTest(test.TestCase):
self.stubs.Set(db, 'instance_update_and_get_original',
instance_update_and_get_original)
fakes.stub_out_glance(self.stubs)
fakes.stub_out_nw_api(self.stubs)
fakes.stub_out_compute_api_snapshot(self.stubs)
fake.stub_out_image_service(self.stubs)
service_class = 'nova.image.glance.GlanceImageService'
self.service = importutils.import_object(service_class)
self.sent_to_glance = {}
self.flags(allow_instance_snapshots=True,
enable_instance_password=True)
self.uuid = FAKE_UUID

117
nova/tests/api/openstack/compute/test_images.py
View File

@@ -21,7 +21,7 @@ and as a WSGI layer
import copy
from lxml import etree
import six.moves.urllib.parse as urlparse
import mock
import webob
from nova.api.openstack.compute import images
@@ -31,11 +31,13 @@ from nova import exception
from nova.image import glance
from nova import test
from nova.tests.api.openstack import fakes
from nova.tests import image_fixtures
from nova.tests import matchers
NS = "{http://docs.openstack.org/compute/api/v1.1}"
ATOMNS = "{http://www.w3.org/2005/Atom}"
NOW_API_FORMAT = "2010-10-11T10:30:22Z"
IMAGE_FIXTURES = image_fixtures.get_image_fixtures()
class ImagesControllerTest(test.NoDBTestCase):
@@ -50,7 +52,6 @@ class ImagesControllerTest(test.NoDBTestCase):
fakes.stub_out_key_pair_funcs(self.stubs)
fakes.stub_out_compute_api_snapshot(self.stubs)
fakes.stub_out_compute_api_backup(self.stubs)
fakes.stub_out_glance(self.stubs)
self.controller = images.Controller()
self.uuid = 'fa95aaf5-ab3b-4cd8-88c0-2be7dd051aaf'
@@ -61,8 +62,6 @@ class ImagesControllerTest(test.NoDBTestCase):
self.server_bookmark = (
"http://localhost/fake/servers/" + self.server_uuid)
self.alternate = "%s/fake/images/%s"
self.fake_req = fakes.HTTPRequest.blank('/v2/fake/images/123')
self.actual_image = self.controller.show(self.fake_req, '124')
self.expected_image_123 = {
"image": {'id': '123',
@@ -139,16 +138,19 @@ class ImagesControllerTest(test.NoDBTestCase):
},
}
self.image_service = self.mox.CreateMockAnything()
@mock.patch('nova.image.api.API.get', return_value=IMAGE_FIXTURES[0])
def test_get_image(self, get_mocked):
request = fakes.HTTPRequest.blank('/v2/fake/images/123')
actual_image = self.controller.show(request, '123')
self.assertThat(actual_image,
matchers.DictMatches(self.expected_image_123))
get_mocked.assert_called_once_with(mock.ANY, '123')
def test_get_image(self):
self.assertThat(self.actual_image,
matchers.DictMatches(self.expected_image_124))
def test_get_image_with_custom_prefix(self):
@mock.patch('nova.image.api.API.get', return_value=IMAGE_FIXTURES[1])
def test_get_image_with_custom_prefix(self, _get_mocked):
self.flags(osapi_compute_link_prefix='https://zoo.com:42',
osapi_glance_link_prefix='http://circus.com:34')
fake_req = fakes.HTTPRequest.blank('/v2/fake/images/123')
fake_req = fakes.HTTPRequest.blank('/v2/fake/images/124')
actual_image = self.controller.show(fake_req, '124')
expected_image = self.expected_image_124
@@ -165,14 +167,18 @@ class ImagesControllerTest(test.NoDBTestCase):
self.assertThat(actual_image, matchers.DictMatches(expected_image))
def test_get_image_404(self):
@mock.patch('nova.image.api.API.get', side_effect=exception.NotFound)
def test_get_image_404(self, _get_mocked):
fake_req = fakes.HTTPRequest.blank('/v2/fake/images/unknown')
self.assertRaises(webob.exc.HTTPNotFound,
self.controller.show, fake_req, 'unknown')
def test_get_image_details(self):
@mock.patch('nova.image.api.API.get_all', return_value=IMAGE_FIXTURES)
def test_get_image_details(self, get_all_mocked):
request = fakes.HTTPRequest.blank('/v2/fake/images/detail')
response = self.controller.detail(request)
get_all_mocked.assert_called_once_with(mock.ANY, filters={})
response_list = response["images"]
image_125 = copy.deepcopy(self.expected_image_124["image"])
@@ -254,49 +260,24 @@ class ImagesControllerTest(test.NoDBTestCase):
self.assertThat(expected, matchers.DictListMatches(response_list))
def test_get_image_details_with_limit(self):
@mock.patch('nova.image.api.API.get_all')
def test_get_image_details_with_limit(self, get_all_mocked):
request = fakes.HTTPRequest.blank('/v2/fake/images/detail?limit=2')
response = self.controller.detail(request)
response_list = response["images"]
response_links = response["images_links"]
self.controller.detail(request)
get_all_mocked.assert_called_once_with(mock.ANY, limit=2, filters={})
expected = [self.expected_image_123["image"],
self.expected_image_124["image"]]
self.assertThat(expected, matchers.DictListMatches(response_list))
href_parts = urlparse.urlparse(response_links[0]['href'])
self.assertEqual('/v2/fake/images', href_parts.path)
params = urlparse.parse_qs(href_parts.query)
self.assertThat({'limit': ['2'], 'marker': ['124']},
matchers.DictMatches(params))
def test_get_image_details_with_limit_and_page_size(self):
@mock.patch('nova.image.api.API.get_all')
def test_get_image_details_with_limit_and_page_size(self, get_all_mocked):
request = fakes.HTTPRequest.blank(
'/v2/fake/images/detail?limit=2&page_size=1')
response = self.controller.detail(request)
response_list = response["images"]
response_links = response["images_links"]
self.controller.detail(request)
get_all_mocked.assert_called_once_with(mock.ANY, limit=2, filters={},
page_size=1)
expected = [self.expected_image_123["image"],
self.expected_image_124["image"]]
self.assertThat(expected, matchers.DictListMatches(response_list))
href_parts = urlparse.urlparse(response_links[0]['href'])
self.assertEqual('/v2/fake/images', href_parts.path)
params = urlparse.parse_qs(href_parts.query)
self.assertThat({'limit': ['2'], 'page_size': ['1'],
'marker': ['124']}, matchers.DictMatches(params))
def _detail_request(self, filters, request):
context = request.environ['nova.context']
self.image_service.detail(context, filters=filters).AndReturn([])
self.mox.ReplayAll()
controller = images.Controller(image_service=self.image_service)
controller.detail(request)
@mock.patch('nova.image.api.API.get_all')
def _detail_request(self, filters, request, get_all_mocked):
self.controller.detail(request)
get_all_mocked.assert_called_once_with(mock.ANY, filters=filters)
def test_image_detail_filter_with_name(self):
filters = {'name': 'testname'}
@@ -348,14 +329,11 @@ class ImagesControllerTest(test.NoDBTestCase):
request = fakes.HTTPRequest.blank('/v2/fake/images/detail')
self._detail_request(filters, request)
def test_image_detail_invalid_marker(self):
class InvalidImageService(object):
def detail(self, *args, **kwargs):
raise exception.Invalid('meow')
@mock.patch('nova.image.api.API.get_all', side_effect=exception.Invalid)
def test_image_detail_invalid_marker(self, _get_all_mocked):
request = fakes.HTTPRequest.blank('/v2/images?marker=invalid')
controller = images.Controller(image_service=InvalidImageService())
self.assertRaises(webob.exc.HTTPBadRequest, controller.detail, request)
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.detail,
request)
def test_generate_alternate_link(self):
view = images_view.ViewBuilder()
@@ -364,25 +342,26 @@ class ImagesControllerTest(test.NoDBTestCase):
actual_url = "%s/fake/images/1" % glance.generate_glance_url()
self.assertEqual(generated_url, actual_url)
def test_delete_image(self):
@mock.patch('nova.image.api.API.delete')
def test_delete_image(self, delete_mocked):
request = fakes.HTTPRequest.blank('/v2/fake/images/124')
request.method = 'DELETE'
response = self.controller.delete(request, '124')
self.assertEqual(response.status_int, 204)
delete_mocked.assert_called_once_with(mock.ANY, '124')
def test_delete_deleted_image(self):
"""If you try to delete a deleted image, you get back 403 Forbidden."""
deleted_image_id = 128
# see nova.tests.api.openstack.fakes:_make_image_fixtures
request = fakes.HTTPRequest.blank(
'/v2/fake/images/%s' % deleted_image_id)
@mock.patch('nova.image.api.API.delete',
side_effect=exception.ImageNotAuthorized(image_id='123'))
def test_delete_deleted_image(self, _delete_mocked):
# If you try to delete a deleted image, you get back 403 Forbidden.
request = fakes.HTTPRequest.blank('/v2/fake/images/123')
request.method = 'DELETE'
self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
request, '%s' % deleted_image_id)
request, '123')
def test_delete_image_not_found(self):
@mock.patch('nova.image.api.API.delete',
side_effect=exception.ImageNotFound(image_id='123'))
def test_delete_image_not_found(self, _delete_mocked):
request = fakes.HTTPRequest.blank('/v2/fake/images/300')
request.method = 'DELETE'
self.assertRaises(webob.exc.HTTPNotFound,

5
nova/tests/api/openstack/compute/test_server_actions.py
View File

@@ -30,7 +30,6 @@ from nova import db
from nova import exception
from nova.image import glance
from nova import objects
from nova.openstack.common import importutils
from nova.openstack.common import jsonutils
from nova.openstack.common import uuidutils
from nova import test
@@ -88,13 +87,9 @@ class ServerActionsControllerTest(test.TestCase):
self.stubs.Set(db, 'instance_update_and_get_original',
instance_update_and_get_original)
fakes.stub_out_glance(self.stubs)
fakes.stub_out_nw_api(self.stubs)
fakes.stub_out_compute_api_snapshot(self.stubs)
fake.stub_out_image_service(self.stubs)
service_class = 'nova.image.glance.GlanceImageService'
self.service = importutils.import_object(service_class)
self.sent_to_glance = {}
self.flags(allow_instance_snapshots=True,
enable_instance_password=True)
self.uuid = FAKE_UUID

3
nova/tests/api/openstack/fakes.py
View File

@@ -248,6 +248,9 @@ def stub_out_nw_api(stubs, cls=None, private=None, publics=None):
fake_network.stub_out_nw_api_get_instance_nw_info(stubs)
# TODO(jaypipes): Remove this when stub_out_glance() is removed after
# image metadata pieces are fixed to call nova.image.API instead of the
# nova.image.glance module directly.
def _make_image_fixtures():
NOW_GLANCE_FORMAT = "2010-10-11T10:30:22"

78
nova/tests/image_fixtures.py Normal file
View File

@@ -0,0 +1,78 @@
# 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 datetime
# nova.image.glance._translate_from_glance() returns datetime
# objects, not strings.
NOW_DATE = datetime.datetime(2010, 10, 11, 10, 30, 22)
def get_image_fixtures():
"""Returns a set of image fixture dicts for use in unit tests.
Returns a set of dicts representing images/snapshots of varying statuses
that would be returned from a call to
`glanceclient.client.Client.images.list`. The IDs of the images returned
start at 123 and go to 131, with the following brief summary of image
attributes:
ID Type Status Notes
-----------------------------------------------------------------
123 Public image active
124 Snapshot queued
125 Snapshot saving
126 Snapshot active
127 Snapshot killed
128 Snapshot deleted
129 Snapshot pending_delete
130 Public image active Has no name
"""
image_id = 123
fixtures = []
def add_fixture(**kwargs):
kwargs.update(created_at=NOW_DATE,
updated_at=NOW_DATE)
fixtures.append(kwargs)
# Public image
add_fixture(id=str(image_id), name='public image', is_public=True,
status='active', properties={'key1': 'value1'},
min_ram="128", min_disk="10", size='25165824')
image_id += 1
# Snapshot for User 1
uuid = 'aa640691-d1a7-4a67-9d3c-d35ee6b3cc74'
snapshot_properties = {'instance_uuid': uuid, 'user_id': 'fake'}
for status in ('queued', 'saving', 'active', 'killed',
'deleted', 'pending_delete'):
deleted = False if status != 'deleted' else True
deleted_at = NOW_DATE if deleted else None
add_fixture(id=str(image_id), name='%s snapshot' % status,
is_public=False, status=status,
properties=snapshot_properties, size='25165824',
deleted=deleted, deleted_at=deleted_at)
image_id += 1
# Image without a name
add_fixture(id=str(image_id), is_public=True, status='active',
properties={})
# Image for permission tests
image_id += 1
add_fixture(id=str(image_id), is_public=True, status='active',
properties={}, owner='authorized_fake')
return fixtures