From 54d6e1d1847b0bd18e4ef1276be2b7044ab5b8f8 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Fri, 7 Jul 2017 12:10:06 -0500 Subject: [PATCH] Add flag to include all images in image list Recent Glance v2 hides images that are shared but not accepted. The way to see them is to send member_status=all, which is a bit low-level for a shade user to need to know. Add a show_all to list_images that defaults to False so that behavior doesn't change. Don't add it to search_images, because at the search_images level the user has the ability to pass in whatever filters they want. Change-Id: Ida2ea943168f5be56a60a94576bdcc6c8e1a9d24 --- ...show-all-images-flag-352748b6c3d99f3f.yaml | 9 +++ shade/openstackcloud.py | 13 +++- shade/tests/unit/test_image.py | 65 +++++++++++++++++-- 3 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/add-show-all-images-flag-352748b6c3d99f3f.yaml diff --git a/releasenotes/notes/add-show-all-images-flag-352748b6c3d99f3f.yaml b/releasenotes/notes/add-show-all-images-flag-352748b6c3d99f3f.yaml new file mode 100644 index 000000000..98c320b26 --- /dev/null +++ b/releasenotes/notes/add-show-all-images-flag-352748b6c3d99f3f.yaml @@ -0,0 +1,9 @@ +--- +features: + - Added flag "show_all" to list_images. The behavior of + Glance v2 to only show shared images if they have been + accepted by the user can be confusing, and the only way + to change it is to use search_images(filters=dict(member_status='all')) + which isn't terribly obvious. "show_all=True" will set + that flag, as well as disabling the filtering of images + in "deleted" state. diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index 73325df04..be5bf29b0 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -2110,22 +2110,31 @@ class OpenStackCloud( return self._normalize_compute_limits(limits, project_id=project_id) @_utils.cache_on_arguments(should_cache_fn=_no_pending_images) - def list_images(self, filter_deleted=True): + def list_images(self, filter_deleted=True, show_all=False): """Get available images. :param filter_deleted: Control whether deleted images are returned. + :param show_all: Show all images, including images that are shared + but not accepted. (By default in glance v2 shared image that + have not been accepted are not shown) show_all will override the + value of filter_deleted to False. :returns: A list of glance images. """ + if show_all: + filter_deleted = False # First, try to actually get images from glance, it's more efficient images = [] + params = {} image_list = [] try: if self.cloud_config.get_api_version('image') == '2': endpoint = '/images' + if show_all: + params['member_status'] = 'all' else: endpoint = '/images/detail' - response = self._image_client.get(endpoint) + response = self._image_client.get(endpoint, params=params) except keystoneauth1.exceptions.catalog.EndpointNotFound: # We didn't have glance, let's try nova diff --git a/shade/tests/unit/test_image.py b/shade/tests/unit/test_image.py index 0b2032bce..ba07d8918 100644 --- a/shade/tests/unit/test_image.py +++ b/shade/tests/unit/test_image.py @@ -12,6 +12,9 @@ # License for the specific language governing permissions and limitations # under the License. +# TODO(mordred) There are mocks of the image_client in here that are not +# using requests_mock. Erradicate them. + import operator import tempfile import uuid @@ -136,6 +139,58 @@ class TestImage(BaseTestImage): self.cloud.list_images()) self.assert_calls() + def test_list_images_show_all(self): + self.register_uris([ + dict(method='GET', + uri='https://image.example.com/v2/images?member_status=all', + json=self.fake_search_return) + ]) + self.assertEqual( + self.cloud._normalize_images([self.fake_image_dict]), + self.cloud.list_images(show_all=True)) + self.assert_calls() + + def test_list_images_show_all_deleted(self): + deleted_image = self.fake_image_dict.copy() + deleted_image['status'] = 'deleted' + self.register_uris([ + dict(method='GET', + uri='https://image.example.com/v2/images?member_status=all', + json={'images': [self.fake_image_dict, deleted_image]}) + ]) + self.assertEqual( + self.cloud._normalize_images([ + self.fake_image_dict, deleted_image]), + self.cloud.list_images(show_all=True)) + self.assert_calls() + + def test_list_images_no_filter_deleted(self): + deleted_image = self.fake_image_dict.copy() + deleted_image['status'] = 'deleted' + self.register_uris([ + dict(method='GET', + uri='https://image.example.com/v2/images', + json={'images': [self.fake_image_dict, deleted_image]}) + ]) + self.assertEqual( + self.cloud._normalize_images([ + self.fake_image_dict, deleted_image]), + self.cloud.list_images(filter_deleted=False)) + self.assert_calls() + + def test_list_images_filter_deleted(self): + deleted_image = self.fake_image_dict.copy() + deleted_image['status'] = 'deleted' + self.register_uris([ + dict(method='GET', + uri='https://image.example.com/v2/images', + json={'images': [self.fake_image_dict, deleted_image]}) + ]) + self.assertEqual( + self.cloud._normalize_images([self.fake_image_dict]), + self.cloud.list_images()) + self.assert_calls() + def test_list_images_string_properties(self): image_dict = self.fake_image_dict.copy() image_dict['properties'] = 'list,of,properties' @@ -362,7 +417,7 @@ class TestImage(BaseTestImage): 'x-image-meta-checksum': mock.ANY, 'x-glance-registry-purge-props': 'false' }) - mock_image_client.get.assert_called_with('/images/detail') + mock_image_client.get.assert_called_with('/images/detail', params={}) self.assertEqual( self._munch_images(ret), self.cloud.list_images()) @@ -431,7 +486,7 @@ class TestImage(BaseTestImage): self.cloud.update_image_properties( image=self._image_dict(ret), **{'owner_specified.shade.object': 'images/42 name'}) - mock_image_client.get.assert_called_with('/images') + mock_image_client.get.assert_called_with('/images', params={}) mock_image_client.patch.assert_not_called() @mock.patch.object(occ.cloud_config.CloudConfig, 'get_api_version') @@ -516,7 +571,7 @@ class TestImage(BaseTestImage): '/images/42/file', headers={'Content-Type': 'application/octet-stream'}, data=mock.ANY) - mock_image_client.get.assert_called_with('/images') + mock_image_client.get.assert_called_with('/images', params={}) self.assertEqual( self._munch_images(ret), self.cloud.list_images()) @@ -545,7 +600,7 @@ class TestImage(BaseTestImage): ret['status'] = 'success' mock_image_client.get.return_value = [ret] mock_image_client.post.return_value = ret - mock_image_client.get.assert_called_with('/images') + mock_image_client.get.assert_called_with('/images', params={}) self.assertEqual( self._munch_images(ret), self.cloud.list_images()) @@ -613,7 +668,7 @@ class TestImage(BaseTestImage): mock_image_client.post.return_value = ret self._call_create_image( '42 name', min_disk='0', min_ram=0, properties={'int_v': 12345}) - mock_image_client.get.assert_called_with('/images') + mock_image_client.get.assert_called_with('/images', params={}) self.assertEqual( self._munch_images(ret), self.cloud.list_images())