From 60e7c51df4cf061ebbb435a959ad63c7d3a296bf Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Fri, 13 Sep 2019 18:03:15 +0200 Subject: [PATCH] Switch image to use SDK This is a work to switch OSC from using glanceclient to OpenStackSDK. With this change only v2 is using OpenStackSDK. V1 is still using glanceclient and will be switched in a separate change. Remove the direct depend on keystoneauth- let that flow through openstacksdk. Depends-on: https://review.opendev.org/#/c/698972 Change-Id: I36f292fb70c98f6e558f58be55d533d979c47ca7 --- lower-constraints.txt | 6 +- openstackclient/common/sdk_utils.py | 60 ++ openstackclient/compute/v2/server.py | 31 +- openstackclient/compute/v2/server_backup.py | 7 +- openstackclient/compute/v2/server_image.py | 7 +- openstackclient/image/client.py | 74 +-- openstackclient/image/v2/image.py | 274 +++++---- .../tests/unit/compute/v2/test_server.py | 93 ++- .../unit/compute/v2/test_server_backup.py | 54 +- .../unit/compute/v2/test_server_image.py | 19 +- openstackclient/tests/unit/image/v2/fakes.py | 24 +- .../tests/unit/image/v2/test_image.py | 547 +++++++++--------- .../tests/unit/volume/v2/test_volume.py | 8 +- openstackclient/volume/v2/volume.py | 5 +- .../use_sdk_for_image-f49d2df38e7d9f81.yaml | 4 + requirements.txt | 3 +- 16 files changed, 659 insertions(+), 557 deletions(-) create mode 100644 openstackclient/common/sdk_utils.py create mode 100644 releasenotes/notes/use_sdk_for_image-f49d2df38e7d9f81.yaml diff --git a/lower-constraints.txt b/lower-constraints.txt index 5fdd4dd5fc..1754ed7780 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -38,7 +38,7 @@ jmespath==0.9.0 jsonpatch==1.16 jsonpointer==1.13 jsonschema==2.6.0 -keystoneauth1==3.14.0 +keystoneauth1==3.16.0 kombu==4.0.0 linecache2==1.0.0 MarkupSafe==1.0 @@ -50,9 +50,9 @@ msgpack-python==0.4.0 munch==2.1.0 netaddr==0.7.18 netifaces==0.10.4 -openstacksdk==0.17.0 +openstacksdk==0.36.0 os-client-config==1.28.0 -os-service-types==1.2.0 +os-service-types==1.7.0 os-testr==1.0.0 osc-lib==2.0.0 osc-placement==1.7.0 diff --git a/openstackclient/common/sdk_utils.py b/openstackclient/common/sdk_utils.py new file mode 100644 index 0000000000..9f0856175d --- /dev/null +++ b/openstackclient/common/sdk_utils.py @@ -0,0 +1,60 @@ +# 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 six + + +def get_osc_show_columns_for_sdk_resource( + sdk_resource, + osc_column_map, + invisible_columns=None +): + """Get and filter the display and attribute columns for an SDK resource. + + Common utility function for preparing the output of an OSC show command. + Some of the columns may need to get renamed, others made invisible. + + :param sdk_resource: An SDK resource + :param osc_column_map: A hash of mappings for display column names + :param invisible_columns: A list of invisible column names + + :returns: Two tuples containing the names of the display and attribute + columns + """ + + if getattr(sdk_resource, 'allow_get', None) is not None: + resource_dict = sdk_resource.to_dict( + body=True, headers=False, ignore_none=False) + else: + resource_dict = sdk_resource + + # Build the OSC column names to display for the SDK resource. + attr_map = {} + display_columns = list(resource_dict.keys()) + invisible_columns = [] if invisible_columns is None else invisible_columns + for col_name in invisible_columns: + if col_name in display_columns: + display_columns.remove(col_name) + for sdk_attr, osc_attr in six.iteritems(osc_column_map): + if sdk_attr in display_columns: + attr_map[osc_attr] = sdk_attr + display_columns.remove(sdk_attr) + if osc_attr not in display_columns: + display_columns.append(osc_attr) + sorted_display_columns = sorted(display_columns) + + # Build the SDK attribute names for the OSC column names. + attr_columns = [] + for column in sorted_display_columns: + new_column = attr_map[column] if column in attr_map else column + attr_columns.append(new_column) + return tuple(sorted_display_columns), tuple(attr_columns) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 5cc73284a2..8be78049c8 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -143,7 +143,7 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): if image_info: image_id = image_info.get('id', '') try: - image = utils.find_resource(image_client.images, image_id) + image = image_client.get_image(image_id) info['image'] = "%s (%s)" % (image.name, image_id) except Exception: info['image'] = image_id @@ -735,10 +735,8 @@ class CreateServer(command.ShowOne): # Lookup parsed_args.image image = None if parsed_args.image: - image = utils.find_resource( - image_client.images, - parsed_args.image, - ) + image = image_client.find_image( + parsed_args.image, ignore_missing=False) if not image and parsed_args.image_property: def emit_duplicated_warning(img, image_property): @@ -749,7 +747,7 @@ class CreateServer(command.ShowOne): 'chosen_one': img_uuid_list[0]}) def _match_image(image_api, wanted_properties): - image_list = image_api.image_list() + image_list = image_api.images() images_matched = [] for img in image_list: img_dict = {} @@ -768,7 +766,7 @@ class CreateServer(command.ShowOne): return [] return images_matched - images = _match_image(image_client.api, parsed_args.image_property) + images = _match_image(image_client, parsed_args.image_property) if len(images) > 1: emit_duplicated_warning(images, parsed_args.image_property) @@ -890,8 +888,8 @@ class CreateServer(command.ShowOne): # one specified by --image, then the compute service will # create a volume from the image and attach it to the # server as a non-root volume. - image_id = utils.find_resource( - image_client.images, dev_map[0]).id + image_id = image_client.find_image(dev_map[0], + ignore_missing=False).id mapping['uuid'] = image_id # 3. append size and delete_on_termination if exist if len(dev_map) > 2 and dev_map[2]: @@ -1324,8 +1322,8 @@ class ListServer(command.Lister): # image name is given, map it to ID. image_id = None if parsed_args.image: - image_id = utils.find_resource(image_client.images, - parsed_args.image).id + image_id = image_client.find_image(parsed_args.image, + ignore_missing=False).id search_opts = { 'reservation_id': parsed_args.reservation_id, @@ -1476,12 +1474,12 @@ class ListServer(command.Lister): (s.image.get('id') for s in data if s.image))): try: - images[i_id] = image_client.images.get(i_id) + images[i_id] = image_client.get_image(i_id) except Exception: pass else: try: - images_list = image_client.images.list() + images_list = image_client.images() for i in images_list: images[i.id] = i except Exception: @@ -1925,7 +1923,7 @@ class RebuildServer(command.ShowOne): # If parsed_args.image is not set, default to the currently used one. image_id = parsed_args.image or server.to_dict().get( 'image', {}).get('id') - image = utils.find_resource(image_client.images, image_id) + image = image_client.get_image(image_id) kwargs = {} if parsed_args.property: @@ -2195,10 +2193,7 @@ class RescueServer(command.Command): image = None if parsed_args.image: - image = utils.find_resource( - image_client.images, - parsed_args.image, - ) + image = image_client.find_image(parsed_args.image) utils.find_resource( compute_client.servers, diff --git a/openstackclient/compute/v2/server_backup.py b/openstackclient/compute/v2/server_backup.py index 1d560dc0c7..a5d43fc6f8 100644 --- a/openstackclient/compute/v2/server_backup.py +++ b/openstackclient/compute/v2/server_backup.py @@ -100,14 +100,11 @@ class CreateServerBackup(command.ShowOne): ) image_client = self.app.client_manager.image - image = utils.find_resource( - image_client.images, - backup_name, - ) + image = image_client.find_image(backup_name, ignore_missing=False) if parsed_args.wait: if utils.wait_for_status( - image_client.images.get, + image_client.get_image, image.id, callback=_show_progress, ): diff --git a/openstackclient/compute/v2/server_image.py b/openstackclient/compute/v2/server_image.py index b93cd4d887..fea87af81f 100644 --- a/openstackclient/compute/v2/server_image.py +++ b/openstackclient/compute/v2/server_image.py @@ -79,14 +79,11 @@ class CreateServerImage(command.ShowOne): ) image_client = self.app.client_manager.image - image = utils.find_resource( - image_client.images, - image_id, - ) + image = image_client.find_image(image_id) if parsed_args.wait: if utils.wait_for_status( - image_client.images.get, + image_client.get_image, image_id, callback=_show_progress, ): diff --git a/openstackclient/image/client.py b/openstackclient/image/client.py index b67c291fd0..15bea17eb6 100644 --- a/openstackclient/image/client.py +++ b/openstackclient/image/client.py @@ -27,7 +27,7 @@ API_VERSION_OPTION = 'os_image_api_version' API_NAME = "image" API_VERSIONS = { "1": "glanceclient.v1.client.Client", - "2": "glanceclient.v2.client.Client", + "2": "openstack.connection.Connection", } IMAGE_API_TYPE = 'image' @@ -38,44 +38,52 @@ IMAGE_API_VERSIONS = { def make_client(instance): - """Returns an image service client""" - image_client = utils.get_client_class( - API_NAME, - instance._api_version[API_NAME], - API_VERSIONS) - LOG.debug('Instantiating image client: %s', image_client) - endpoint = instance.get_endpoint_for_service_type( - API_NAME, - region_name=instance.region_name, - interface=instance.interface, - ) + if instance._api_version[API_NAME] != '1': + LOG.debug( + 'Image client initialized using OpenStack SDK: %s', + instance.sdk_connection.image, + ) + return instance.sdk_connection.image + else: + """Returns an image service client""" + image_client = utils.get_client_class( + API_NAME, + instance._api_version[API_NAME], + API_VERSIONS) + LOG.debug('Instantiating image client: %s', image_client) - client = image_client( - endpoint, - token=instance.auth.get_token(instance.session), - cacert=instance.cacert, - insecure=not instance.verify, - ) - - # Create the low-level API - - image_api = utils.get_client_class( - API_NAME, - instance._api_version[API_NAME], - IMAGE_API_VERSIONS) - LOG.debug('Instantiating image api: %s', image_api) - - client.api = image_api( - session=instance.session, - endpoint=instance.get_endpoint_for_service_type( - IMAGE_API_TYPE, + endpoint = instance.get_endpoint_for_service_type( + API_NAME, region_name=instance.region_name, interface=instance.interface, ) - ) - return client + client = image_client( + endpoint, + token=instance.auth.get_token(instance.session), + cacert=instance.cacert, + insecure=not instance.verify, + ) + + # Create the low-level API + + image_api = utils.get_client_class( + API_NAME, + instance._api_version[API_NAME], + IMAGE_API_VERSIONS) + LOG.debug('Instantiating image api: %s', image_api) + + client.api = image_api( + session=instance.session, + endpoint=instance.get_endpoint_for_service_type( + IMAGE_API_TYPE, + region_name=instance.region_name, + interface=instance.interface, + ) + ) + + return client def build_option_parser(parser): diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py index feeb256744..412a16cc1c 100644 --- a/openstackclient/image/v2/image.py +++ b/openstackclient/image/v2/image.py @@ -18,8 +18,9 @@ import argparse from base64 import b64encode import logging +import os +import sys -from glanceclient.common import utils as gc_utils from openstack.image import image_signer from osc_lib.api import utils as api_utils from osc_lib.cli import format_columns @@ -27,11 +28,16 @@ from osc_lib.cli import parseractions from osc_lib.command import command from osc_lib import exceptions from osc_lib import utils -import six +from openstackclient.common import sdk_utils from openstackclient.i18n import _ from openstackclient.identity import common +if os.name == "nt": + import msvcrt +else: + msvcrt = None + CONTAINER_CHOICES = ["ami", "ari", "aki", "bare", "docker", "ova", "ovf"] DEFAULT_CONTAINER_FORMAT = 'bare' @@ -44,7 +50,7 @@ MEMBER_STATUS_CHOICES = ["accepted", "pending", "rejected", "all"] LOG = logging.getLogger(__name__) -def _format_image(image): +def _format_image(image, human_readable=False): """Format an image to make it more consistent with OSC operations.""" info = {} @@ -56,15 +62,25 @@ def _format_image(image): 'min_disk', 'protected', 'id', 'file', 'checksum', 'owner', 'virtual_size', 'min_ram', 'schema'] + # TODO(gtema/anybody): actually it should be possible to drop this method, + # since SDK already delivers a proper object + image = image.to_dict(ignore_none=True, original_names=True) + # split out the usual key and the properties which are top-level - for key in six.iterkeys(image): + for key in image: if key in fields_to_show: info[key] = image.get(key) elif key == 'tags': continue # handle this later - else: + elif key == 'properties': + # NOTE(gtema): flatten content of properties + properties.update(image.get(key)) + elif key != 'location': properties[key] = image.get(key) + if human_readable: + info['size'] = utils.format_size(image['size']) + # format the tags if they are there info['tags'] = format_columns.ListColumn(image.get('tags')) @@ -75,6 +91,51 @@ def _format_image(image): return info +_formatters = { + 'tags': format_columns.ListColumn, +} + + +def _get_member_columns(item): + # Trick sdk_utils to return URI attribute + column_map = { + 'image_id': 'image_id' + } + hidden_columns = ['id', 'location', 'name'] + return sdk_utils.get_osc_show_columns_for_sdk_resource( + item.to_dict(), column_map, hidden_columns) + + +def get_data_file(args): + if args.file: + return (open(args.file, 'rb'), args.file) + else: + # distinguish cases where: + # (1) stdin is not valid (as in cron jobs): + # openstack ... <&- + # (2) image data is provided through stdin: + # openstack ... < /tmp/file + # (3) no image data provided + # openstack ... + try: + os.fstat(0) + except OSError: + # (1) stdin is not valid + return (None, None) + if not sys.stdin.isatty(): + # (2) image data is provided through stdin + image = sys.stdin + if hasattr(sys.stdin, 'buffer'): + image = sys.stdin.buffer + if msvcrt: + msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY) + + return (image, None) + else: + # (3) + return (None, None) + + class AddProjectToImage(command.ShowOne): _description = _("Associate project with image") @@ -101,16 +162,18 @@ class AddProjectToImage(command.ShowOne): parsed_args.project, parsed_args.project_domain).id - image_id = utils.find_resource( - image_client.images, - parsed_args.image).id + image = image_client.find_image(parsed_args.image, + ignore_missing=False) - image_member = image_client.image_members.create( - image_id, - project_id, + obj = image_client.add_member( + image=image.id, + member_id=project_id, ) - return zip(*sorted(image_member.items())) + display_columns, columns = _get_member_columns(obj) + data = utils.get_item_properties(obj, columns, formatters={}) + + return (display_columns, data) class CreateImage(command.ShowOne): @@ -302,9 +365,9 @@ class CreateImage(command.ShowOne): # to do nothing when no options are present as opposed to always # setting a default. if parsed_args.protected: - kwargs['protected'] = True + kwargs['is_protected'] = True if parsed_args.unprotected: - kwargs['protected'] = False + kwargs['is_protected'] = False if parsed_args.public: kwargs['visibility'] = 'public' if parsed_args.private: @@ -314,24 +377,30 @@ class CreateImage(command.ShowOne): if parsed_args.shared: kwargs['visibility'] = 'shared' if parsed_args.project: - kwargs['owner'] = common.find_project( + kwargs['owner_id'] = common.find_project( identity_client, parsed_args.project, parsed_args.project_domain, ).id # open the file first to ensure any failures are handled before the - # image is created - fp = gc_utils.get_data_file(parsed_args) + # image is created. Get the file name (if it is file, and not stdin) + # for easier further handling. + (fp, fname) = get_data_file(parsed_args) info = {} + if fp is not None and parsed_args.volume: raise exceptions.CommandError(_("Uploading data and using " "container are not allowed at " "the same time")) - if fp is None and parsed_args.file: LOG.warning(_("Failed to get an image file.")) return {}, {} + elif fname: + kwargs['filename'] = fname + elif fp: + kwargs['validate_checksum'] = False + kwargs['data'] = fp # sign an image using a given local private key file if parsed_args.sign_key_path or parsed_args.sign_cert_id: @@ -361,8 +430,8 @@ class CreateImage(command.ShowOne): sign_key_path, password=pw) except Exception: - msg = (_("Error during sign operation: private key could " - "not be loaded.")) + msg = (_("Error during sign operation: private key " + "could not be loaded.")) raise exceptions.CommandError(msg) signature = signer.generate_signature(fp) @@ -371,7 +440,8 @@ class CreateImage(command.ShowOne): kwargs['img_signature_certificate_uuid'] = sign_cert_id kwargs['img_signature_hash_method'] = signer.hash_method if signer.padding_method: - kwargs['img_signature_key_type'] = signer.padding_method + kwargs['img_signature_key_type'] = \ + signer.padding_method # If a volume is specified. if parsed_args.volume: @@ -393,26 +463,7 @@ class CreateImage(command.ShowOne): except TypeError: info['volume_type'] = None else: - image = image_client.images.create(**kwargs) - - if fp is not None: - with fp: - try: - image_client.images.upload(image.id, fp) - except Exception: - # If the upload fails for some reason attempt to remove the - # dangling queued image made by the create() call above but - # only if the user did not specify an id which indicates - # the Image already exists and should be left alone. - try: - if 'id' not in kwargs: - image_client.images.delete(image.id) - except Exception: - pass # we don't care about this one - raise # now, throw the upload exception again - - # update the image after the data has been uploaded - image = image_client.images.get(image.id) + image = image_client.create_image(**kwargs) if not info: info = _format_image(image) @@ -439,11 +490,9 @@ class DeleteImage(command.Command): image_client = self.app.client_manager.image for image in parsed_args.images: try: - image_obj = utils.find_resource( - image_client.images, - image, - ) - image_client.images.delete(image_obj.id) + image_obj = image_client.find_image(image, + ignore_missing=False) + image_client.delete_image(image_obj.id) except Exception as e: del_result += 1 LOG.error(_("Failed to delete image with name or " @@ -569,18 +618,17 @@ class ListImage(command.Lister): kwargs = {} if parsed_args.public: - kwargs['public'] = True + kwargs['visibility'] = 'public' if parsed_args.private: - kwargs['private'] = True + kwargs['visibility'] = 'private' if parsed_args.community: - kwargs['community'] = True + kwargs['visibility'] = 'community' if parsed_args.shared: - kwargs['shared'] = True + kwargs['visibility'] = 'shared' if parsed_args.limit: kwargs['limit'] = parsed_args.limit if parsed_args.marker: - kwargs['marker'] = utils.find_resource(image_client.images, - parsed_args.marker).id + kwargs['marker'] = image_client.find_image(parsed_args.marker).id if parsed_args.name: kwargs['name'] = parsed_args.name if parsed_args.status: @@ -599,8 +647,8 @@ class ListImage(command.Lister): 'Checksum', 'Status', 'visibility', - 'protected', - 'owner', + 'is_protected', + 'owner_id', 'tags', ) column_headers = ( @@ -621,24 +669,10 @@ class ListImage(command.Lister): column_headers = columns # List of image data received - data = [] - limit = None if 'limit' in kwargs: - limit = kwargs['limit'] - if 'marker' in kwargs: - data = image_client.api.image_list(**kwargs) - else: - # No pages received yet, so start the page marker at None. - marker = None - while True: - page = image_client.api.image_list(marker=marker, **kwargs) - if not page: - break - data.extend(page) - # Set the marker to the id of the last item we received - marker = page[-1]['id'] - if limit: - break + # Disable automatic pagination in SDK + kwargs['paginated'] = False + data = list(image_client.images(**kwargs)) if parsed_args.property: for attr, value in parsed_args.property.items(): @@ -653,12 +687,10 @@ class ListImage(command.Lister): return ( column_headers, - (utils.get_dict_properties( + (utils.get_item_properties( s, columns, - formatters={ - 'tags': format_columns.ListColumn, - }, + formatters=_formatters, ) for s in data) ) @@ -684,11 +716,9 @@ class ListImageProjects(command.Lister): "Status" ) - image_id = utils.find_resource( - image_client.images, - parsed_args.image).id + image_id = image_client.find_image(parsed_args.image).id - data = image_client.image_members.list(image_id) + data = image_client.members(image=image_id) return (columns, (utils.get_item_properties( @@ -722,11 +752,12 @@ class RemoveProjectImage(command.Command): parsed_args.project, parsed_args.project_domain).id - image_id = utils.find_resource( - image_client.images, - parsed_args.image).id + image = image_client.find_image(parsed_args.image, + ignore_missing=False) - image_client.image_members.delete(image_id, project_id) + image_client.remove_member( + member=project_id, + image=image.id) class SaveImage(command.Command): @@ -748,19 +779,9 @@ class SaveImage(command.Command): def take_action(self, parsed_args): image_client = self.app.client_manager.image - image = utils.find_resource( - image_client.images, - parsed_args.image, - ) - data = image_client.images.data(image.id) + image = image_client.find_image(parsed_args.image) - if data.wrapped is None: - msg = _('Image %s has no data.') % image.id - LOG.error(msg) - self.app.stdout.write(msg + '\n') - raise SystemExit - - gc_utils.save_image(data, parsed_args.file) + image_client.download_image(image.id, output=parsed_args.file) class SetImage(command.Command): @@ -979,9 +1000,9 @@ class SetImage(command.Command): # to do nothing when no options are present as opposed to always # setting a default. if parsed_args.protected: - kwargs['protected'] = True + kwargs['is_protected'] = True if parsed_args.unprotected: - kwargs['protected'] = False + kwargs['is_protected'] = False if parsed_args.public: kwargs['visibility'] = 'public' if parsed_args.private: @@ -997,17 +1018,20 @@ class SetImage(command.Command): parsed_args.project, parsed_args.project_domain, ).id - kwargs['owner'] = project_id + kwargs['owner_id'] = project_id - image = utils.find_resource( - image_client.images, parsed_args.image) + image = image_client.find_image(parsed_args.image, + ignore_missing=False) + + # image = utils.find_resource( + # image_client.images, parsed_args.image) activation_status = None if parsed_args.deactivate: - image_client.images.deactivate(image.id) + image_client.deactivate_image(image.id) activation_status = "deactivated" if parsed_args.activate: - image_client.images.reactivate(image.id) + image_client.reactivate_image(image.id) activation_status = "activated" membership_group_args = ('accept', 'reject', 'pending') @@ -1022,15 +1046,15 @@ class SetImage(command.Command): # most one item in the membership_status list. if membership_status[0] != 'pending': membership_status[0] += 'ed' # Glance expects the past form - image_client.image_members.update( - image.id, project_id, membership_status[0]) + image_client.update_member( + image=image.id, member=project_id, status=membership_status[0]) if parsed_args.tags: # Tags should be extended, but duplicates removed kwargs['tags'] = list(set(image.tags).union(set(parsed_args.tags))) try: - image = image_client.images.update(image.id, **kwargs) + image = image_client.update_image(image.id, **kwargs) except Exception: if activation_status is not None: LOG.info(_("Image %(id)s was %(status)s."), @@ -1058,14 +1082,11 @@ class ShowImage(command.ShowOne): def take_action(self, parsed_args): image_client = self.app.client_manager.image - image = utils.find_resource( - image_client.images, - parsed_args.image, - ) - if parsed_args.human_readable: - image['size'] = utils.format_size(image['size']) - info = _format_image(image) + image = image_client.find_image(parsed_args.image, + ignore_missing=False) + + info = _format_image(image, parsed_args.human_readable) return zip(*sorted(info.items())) @@ -1101,10 +1122,8 @@ class UnsetImage(command.Command): def take_action(self, parsed_args): image_client = self.app.client_manager.image - image = utils.find_resource( - image_client.images, - parsed_args.image, - ) + image = image_client.find_image(parsed_args.image, + ignore_missing=False) kwargs = {} tagret = 0 @@ -1112,7 +1131,7 @@ class UnsetImage(command.Command): if parsed_args.tags: for k in parsed_args.tags: try: - image_client.image_tags.delete(image.id, k) + image_client.remove_tag(image.id, k) except Exception: LOG.error(_("tag unset failed, '%s' is a " "nonexistent tag "), k) @@ -1120,13 +1139,26 @@ class UnsetImage(command.Command): if parsed_args.properties: for k in parsed_args.properties: - if k not in image: + if k in image: + kwargs[k] = None + elif k in image.properties: + # Since image is an "evil" object from SDK POV we need to + # pass modified properties object, so that SDK can figure + # out, what was changed inside + # NOTE: ping gtema to improve that in SDK + new_props = kwargs.get('properties', + image.get('properties').copy()) + new_props.pop(k, None) + kwargs['properties'] = new_props + else: LOG.error(_("property unset failed, '%s' is a " "nonexistent property "), k) propret += 1 - image_client.images.update( - image.id, - parsed_args.properties, + + # We must give to update a current image for the reference on what + # has changed + image_client.update_image( + image, **kwargs) tagtotal = len(parsed_args.tags) diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 27eefd8586..8ec8217dfe 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -55,6 +55,12 @@ class TestServer(compute_fakes.TestComputev2): self.images_mock = self.app.client_manager.image.images self.images_mock.reset_mock() + self.find_image_mock = self.app.client_manager.image.find_image + self.find_image_mock.reset_mock() + + self.get_image_mock = self.app.client_manager.image.get_image + self.get_image_mock.reset_mock() + # Get a shortcut to the volume client VolumeManager Mock self.volumes_mock = self.app.client_manager.volume.volumes self.volumes_mock.reset_mock() @@ -770,7 +776,8 @@ class TestServerCreate(TestServer): self.servers_mock.create.return_value = self.new_server self.image = image_fakes.FakeImage.create_one_image() - self.images_mock.get.return_value = self.image + self.find_image_mock.return_value = self.image + self.get_image_mock.return_value = self.image self.flavor = compute_fakes.FakeFlavor.create_one_flavor() self.flavors_mock.get.return_value = self.flavor @@ -1916,19 +1923,13 @@ class TestServerCreate(TestServer): ('config_drive', False), ('server_name', self.new_server.name), ] - _image = image_fakes.FakeImage.create_one_image() # create a image_info as the side_effect of the fake image_list() image_info = { - 'id': _image.id, - 'name': _image.name, - 'owner': _image.owner, 'hypervisor_type': 'qemu', } - self.api_mock = mock.Mock() - self.api_mock.image_list.side_effect = [ - [image_info], [], - ] - self.app.client_manager.image.api = self.api_mock + + _image = image_fakes.FakeImage.create_one_image(image_info) + self.images_mock.return_value = [_image] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1953,7 +1954,7 @@ class TestServerCreate(TestServer): # ServerManager.create(name, image, flavor, **kwargs) self.servers_mock.create.assert_called_with( self.new_server.name, - image_info, + _image, self.flavor, **kwargs ) @@ -1977,20 +1978,13 @@ class TestServerCreate(TestServer): ('config_drive', False), ('server_name', self.new_server.name), ] - _image = image_fakes.FakeImage.create_one_image() # create a image_info as the side_effect of the fake image_list() image_info = { - 'id': _image.id, - 'name': _image.name, - 'owner': _image.owner, 'hypervisor_type': 'qemu', 'hw_disk_bus': 'ide', } - self.api_mock = mock.Mock() - self.api_mock.image_list.side_effect = [ - [image_info], [], - ] - self.app.client_manager.image.api = self.api_mock + _image = image_fakes.FakeImage.create_one_image(image_info) + self.images_mock.return_value = [_image] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -2015,7 +2009,7 @@ class TestServerCreate(TestServer): # ServerManager.create(name, image, flavor, **kwargs) self.servers_mock.create.assert_called_with( self.new_server.name, - image_info, + _image, self.flavor, **kwargs ) @@ -2039,20 +2033,14 @@ class TestServerCreate(TestServer): ('config_drive', False), ('server_name', self.new_server.name), ] - _image = image_fakes.FakeImage.create_one_image() # create a image_info as the side_effect of the fake image_list() image_info = { - 'id': _image.id, - 'name': _image.name, - 'owner': _image.owner, 'hypervisor_type': 'qemu', 'hw_disk_bus': 'ide', } - self.api_mock = mock.Mock() - self.api_mock.image_list.side_effect = [ - [image_info], [], - ] - self.app.client_manager.image.api = self.api_mock + + _image = image_fakes.FakeImage.create_one_image(image_info) + self.images_mock.return_value = [_image] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -2585,7 +2573,10 @@ class TestServerList(TestServer): self.servers_mock.list.return_value = self.servers self.image = image_fakes.FakeImage.create_one_image() - self.images_mock.get.return_value = self.image + + # self.images_mock.return_value = [self.image] + self.find_image_mock.return_value = self.image + self.get_image_mock.return_value = self.image self.flavor = compute_fakes.FakeFlavor.create_one_flavor() self.flavors_mock.get.return_value = self.flavor @@ -2599,7 +2590,7 @@ class TestServerList(TestServer): self.data_no_name_lookup = [] Image = collections.namedtuple('Image', 'id name') - self.images_mock.list.return_value = [ + self.images_mock.return_value = [ Image(id=s.image['id'], name=self.image.name) # Image will be an empty string if boot-from-volume for s in self.servers if s.image @@ -2662,11 +2653,11 @@ class TestServerList(TestServer): columns, data = self.cmd.take_action(parsed_args) self.servers_mock.list.assert_called_with(**self.kwargs) - self.images_mock.list.assert_called() + self.images_mock.assert_called() self.flavors_mock.list.assert_called() # we did not pass image or flavor, so gets on those must be absent self.assertFalse(self.flavors_mock.get.call_count) - self.assertFalse(self.images_mock.get.call_count) + self.assertFalse(self.get_image_mock.call_count) self.assertEqual(self.columns, columns) self.assertEqual(tuple(self.data), tuple(data)) @@ -2753,7 +2744,7 @@ class TestServerList(TestServer): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertFalse(self.images_mock.list.call_count) self.assertFalse(self.flavors_mock.list.call_count) - self.images_mock.get.assert_called() + self.get_image_mock.assert_called() self.flavors_mock.get.assert_called() self.assertEqual(self.columns, columns) @@ -2771,7 +2762,8 @@ class TestServerList(TestServer): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.images_mock.get.assert_any_call(self.image.id) + self.find_image_mock.assert_called_with(self.image.id, + ignore_missing=False) self.search_opts['image'] = self.image.id self.servers_mock.list.assert_called_with(**self.kwargs) @@ -3558,7 +3550,7 @@ class TestServerRebuild(TestServer): # Return value for utils.find_resource for image self.image = image_fakes.FakeImage.create_one_image() - self.images_mock.get.return_value = self.image + self.get_image_mock.return_value = self.image # Fake the rebuilt new server. attrs = { @@ -3598,7 +3590,7 @@ class TestServerRebuild(TestServer): self.cmd.take_action(parsed_args) self.servers_mock.get.assert_called_with(self.server.id) - self.images_mock.get.assert_called_with(self.image.id) + self.get_image_mock.assert_called_with(self.image.id) self.server.rebuild.assert_called_with(self.image, None) def test_rebuild_with_current_image_and_password(self): @@ -3617,7 +3609,7 @@ class TestServerRebuild(TestServer): self.cmd.take_action(parsed_args) self.servers_mock.get.assert_called_with(self.server.id) - self.images_mock.get.assert_called_with(self.image.id) + self.get_image_mock.assert_called_with(self.image.id) self.server.rebuild.assert_called_with(self.image, password) def test_rebuild_with_description_api_older(self): @@ -3665,7 +3657,7 @@ class TestServerRebuild(TestServer): self.cmd.take_action(parsed_args) self.servers_mock.get.assert_called_with(self.server.id) - self.images_mock.get.assert_called_with(self.image.id) + self.get_image_mock.assert_called_with(self.image.id) self.server.rebuild.assert_called_with(self.image, None, description=description) @@ -3694,7 +3686,7 @@ class TestServerRebuild(TestServer): ) self.servers_mock.get.assert_called_with(self.server.id) - self.images_mock.get.assert_called_with(self.image.id) + self.get_image_mock.assert_called_with(self.image.id) self.server.rebuild.assert_called_with(self.image, None) @mock.patch.object(common_utils, 'wait_for_status', return_value=False) @@ -3718,7 +3710,7 @@ class TestServerRebuild(TestServer): ) self.servers_mock.get.assert_called_with(self.server.id) - self.images_mock.get.assert_called_with(self.image.id) + self.get_image_mock.assert_called_with(self.image.id) self.server.rebuild.assert_called_with(self.image, None) def test_rebuild_with_property(self): @@ -3738,7 +3730,7 @@ class TestServerRebuild(TestServer): self.cmd.take_action(parsed_args) self.servers_mock.get.assert_called_with(self.server.id) - self.images_mock.get.assert_called_with(self.image.id) + self.get_image_mock.assert_called_with(self.image.id) self.server.rebuild.assert_called_with( self.image, None, meta=expected_property) @@ -3767,7 +3759,7 @@ class TestServerRebuild(TestServer): key_name=self.server.key_name, ) self.servers_mock.get.assert_called_with(self.server.id) - self.images_mock.get.assert_called_with(self.image.id) + self.get_image_mock.assert_called_with(self.image.id) self.server.rebuild.assert_called_with(*args, **kwargs) def test_rebuild_with_keypair_name_older_version(self): @@ -3814,7 +3806,7 @@ class TestServerRebuild(TestServer): key_name=None, ) self.servers_mock.get.assert_called_with(self.server.id) - self.images_mock.get.assert_called_with(self.image.id) + self.get_image_mock.assert_called_with(self.image.id) self.server.rebuild.assert_called_with(*args, **kwargs) def test_rebuild_with_key_name_and_unset(self): @@ -3872,7 +3864,7 @@ class TestServerRescue(TestServer): # Return value for utils.find_resource for image self.image = image_fakes.FakeImage.create_one_image() - self.images_mock.get.return_value = self.image + self.get_image_mock.return_value = self.image new_server = compute_fakes.FakeServer.create_one_server() attrs = { @@ -3913,7 +3905,7 @@ class TestServerRescue(TestServer): def test_rescue_with_new_image(self): new_image = image_fakes.FakeImage.create_one_image() - self.images_mock.get.return_value = new_image + self.find_image_mock.return_value = new_image arglist = [ '--image', new_image.id, self.server.id, @@ -3928,7 +3920,7 @@ class TestServerRescue(TestServer): self.cmd.take_action(parsed_args) self.servers_mock.get.assert_called_with(self.server.id) - self.images_mock.get.assert_called_with(new_image.id) + self.find_image_mock.assert_called_with(new_image.id) self.server.rescue.assert_called_with(image=new_image, password=None) def test_rescue_with_current_image_and_password(self): @@ -4679,7 +4671,7 @@ class TestServerShow(TestServer): # This is the return value for utils.find_resource() self.servers_mock.get.return_value = self.server - self.images_mock.get.return_value = self.image + self.get_image_mock.return_value = self.image self.flavors_mock.get.return_value = self.flavor # Get the command object to test @@ -5140,7 +5132,8 @@ class TestServerGeneral(TestServer): 'links': u'http://xxx.yyy.com', } _server = compute_fakes.FakeServer.create_one_server(attrs=server_info) - find_resource.side_effect = [_server, _image, _flavor] + find_resource.side_effect = [_server, _flavor] + self.get_image_mock.return_value = _image # Prepare result data. info = { diff --git a/openstackclient/tests/unit/compute/v2/test_server_backup.py b/openstackclient/tests/unit/compute/v2/test_server_backup.py index 7dd459d823..5cdc20800a 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_backup.py +++ b/openstackclient/tests/unit/compute/v2/test_server_backup.py @@ -32,8 +32,8 @@ class TestServerBackup(compute_fakes.TestComputev2): self.servers_mock.reset_mock() # Get a shortcut to the image client ImageManager Mock - self.images_mock = self.app.client_manager.image.images - self.images_mock.reset_mock() + self.images_mock = self.app.client_manager.image + self.images_mock.find_image.reset_mock() # Set object attributes to be tested. Could be overwritten in subclass. self.attrs = {} @@ -60,15 +60,18 @@ class TestServerBackupCreate(TestServerBackup): # Just return whatever Image is testing with these days def image_columns(self, image): - columnlist = tuple(sorted(image.keys())) + # columnlist = tuple(sorted(image.keys())) + columnlist = ( + 'id', 'name', 'owner', 'protected', 'status', 'tags', 'visibility' + ) return columnlist def image_data(self, image): datalist = ( image['id'], image['name'], - image['owner'], - image['protected'], + image['owner_id'], + image['is_protected'], 'active', format_columns.ListColumn(image.get('tags')), image['visibility'], @@ -102,7 +105,8 @@ class TestServerBackupCreate(TestServerBackup): count=count, ) - self.images_mock.get = mock.Mock(side_effect=images) + # self.images_mock.get = mock.Mock(side_effect=images) + self.images_mock.find_image = mock.Mock(side_effect=images) return images def test_server_backup_defaults(self): @@ -174,16 +178,18 @@ class TestServerBackupCreate(TestServerBackup): @mock.patch.object(common_utils, 'wait_for_status', return_value=False) def test_server_backup_wait_fail(self, mock_wait_for_status): servers = self.setup_servers_mock(count=1) - images = image_fakes.FakeImage.create_images( - attrs={ - 'name': servers[0].name, - 'status': 'active', - }, - count=5, - ) - - self.images_mock.get = mock.Mock( - side_effect=images, + images = self.setup_images_mock(count=1, servers=servers) +# images = image_fakes.FakeImage.create_images( +# attrs={ +# 'name': servers[0].name, +# 'status': 'active', +# }, +# count=1, +# ) +# +# self.images_mock.find_image.return_value = images[0] + self.images_mock.get_image = mock.Mock( + side_effect=images[0], ) arglist = [ @@ -215,7 +221,7 @@ class TestServerBackupCreate(TestServerBackup): ) mock_wait_for_status.assert_called_once_with( - self.images_mock.get, + self.images_mock.get_image, images[0].id, callback=mock.ANY ) @@ -223,16 +229,10 @@ class TestServerBackupCreate(TestServerBackup): @mock.patch.object(common_utils, 'wait_for_status', return_value=True) def test_server_backup_wait_ok(self, mock_wait_for_status): servers = self.setup_servers_mock(count=1) - images = image_fakes.FakeImage.create_images( - attrs={ - 'name': servers[0].name, - 'status': 'active', - }, - count=5, - ) + images = self.setup_images_mock(count=1, servers=servers) - self.images_mock.get = mock.Mock( - side_effect=images, + self.images_mock.get_image = mock.Mock( + side_effect=images[0], ) arglist = [ @@ -263,7 +263,7 @@ class TestServerBackupCreate(TestServerBackup): ) mock_wait_for_status.assert_called_once_with( - self.images_mock.get, + self.images_mock.get_image, images[0].id, callback=mock.ANY ) diff --git a/openstackclient/tests/unit/compute/v2/test_server_image.py b/openstackclient/tests/unit/compute/v2/test_server_image.py index f9d7b10e75..1cec5b685f 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_image.py +++ b/openstackclient/tests/unit/compute/v2/test_server_image.py @@ -31,8 +31,8 @@ class TestServerImage(compute_fakes.TestComputev2): self.servers_mock.reset_mock() # Get a shortcut to the image client ImageManager Mock - self.images_mock = self.app.client_manager.image.images - self.images_mock.reset_mock() + self.images_mock = self.app.client_manager.image + self.images_mock.find_image.reset_mock() # Set object attributes to be tested. Could be overwritten in subclass. self.attrs = {} @@ -58,15 +58,18 @@ class TestServerImage(compute_fakes.TestComputev2): class TestServerImageCreate(TestServerImage): def image_columns(self, image): - columnlist = tuple(sorted(image.keys())) + # columnlist = tuple(sorted(image.keys())) + columnlist = ( + 'id', 'name', 'owner', 'protected', 'status', 'tags', 'visibility' + ) return columnlist def image_data(self, image): datalist = ( image['id'], image['name'], - image['owner'], - image['protected'], + image['owner_id'], + image['is_protected'], 'active', format_columns.ListColumn(image.get('tags')), image['visibility'], @@ -100,7 +103,7 @@ class TestServerImageCreate(TestServerImage): count=count, ) - self.images_mock.get = mock.Mock(side_effect=images) + self.images_mock.find_image = mock.Mock(side_effect=images) self.servers_mock.create_image = mock.Mock( return_value=images[0].id, ) @@ -188,7 +191,7 @@ class TestServerImageCreate(TestServerImage): ) mock_wait_for_status.assert_called_once_with( - self.images_mock.get, + self.images_mock.get_image, images[0].id, callback=mock.ANY ) @@ -220,7 +223,7 @@ class TestServerImageCreate(TestServerImage): ) mock_wait_for_status.assert_called_once_with( - self.images_mock.get, + self.images_mock.get_image, images[0].id, callback=mock.ANY ) diff --git a/openstackclient/tests/unit/image/v2/fakes.py b/openstackclient/tests/unit/image/v2/fakes.py index 655ae341c6..516d563001 100644 --- a/openstackclient/tests/unit/image/v2/fakes.py +++ b/openstackclient/tests/unit/image/v2/fakes.py @@ -18,9 +18,9 @@ import random from unittest import mock import uuid -from glanceclient.v2 import schemas +from openstack.image.v2 import image +from openstack.image.v2 import member from osc_lib.cli import format_columns -import warlock from openstackclient.tests.unit import fakes from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes @@ -154,6 +154,12 @@ class FakeImagev2Client(object): self.image_members.resource_class = fakes.FakeResource(None, {}) self.image_tags = mock.Mock() self.image_tags.resource_class = fakes.FakeResource(None, {}) + + self.find_image = mock.Mock() + self.find_image.resource_class = fakes.FakeResource(None, {}) + + self.get_image = mock.Mock() + self.get_image.resource_class = fakes.FakeResource(None, {}) self.auth_token = kwargs['token'] self.management_url = kwargs['endpoint'] self.version = 2.0 @@ -197,8 +203,8 @@ class FakeImage(object): image_info = { 'id': str(uuid.uuid4()), 'name': 'image-name' + uuid.uuid4().hex, - 'owner': 'image-owner' + uuid.uuid4().hex, - 'protected': bool(random.choice([0, 1])), + 'owner_id': 'image-owner' + uuid.uuid4().hex, + 'is_protected': bool(random.choice([0, 1])), 'visibility': random.choice(['public', 'private']), 'tags': [uuid.uuid4().hex for r in range(2)], } @@ -206,13 +212,7 @@ class FakeImage(object): # Overwrite default attributes if there are some attributes set image_info.update(attrs) - # Set up the schema - model = warlock.model_factory( - IMAGE_schema, - schemas.SchemaBasedModel, - ) - - return model(**image_info) + return image.Image(**image_info) @staticmethod def create_images(attrs=None, count=2): @@ -307,6 +307,8 @@ class FakeImage(object): # Overwrite default attributes if there are some attributes set image_member_info.update(attrs) + return member.Member(**image_member_info) + image_member = fakes.FakeModel( copy.deepcopy(image_member_info)) diff --git a/openstackclient/tests/unit/image/v2/test_image.py b/openstackclient/tests/unit/image/v2/test_image.py index 78d857e269..a021cfc7fc 100644 --- a/openstackclient/tests/unit/image/v2/test_image.py +++ b/openstackclient/tests/unit/image/v2/test_image.py @@ -14,13 +14,14 @@ # import copy +import io +import os +import tempfile from unittest import mock -from glanceclient.common import utils as glanceclient_utils -from glanceclient.v2 import schemas +from openstack import exceptions as sdk_exceptions from osc_lib.cli import format_columns from osc_lib import exceptions -import warlock from openstackclient.image.v2 import image from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes @@ -33,12 +34,16 @@ class TestImage(image_fakes.TestImagev2): super(TestImage, self).setUp() # Get shortcuts to the Mocks in image client - self.images_mock = self.app.client_manager.image.images - self.images_mock.reset_mock() + # SDK proxy mock + self.app.client_manager.image = mock.Mock() + self.client = self.app.client_manager.image + + self.client.remove_member = mock.Mock() + + self.client.create_image = mock.Mock() + self.client.update_image = mock.Mock() self.image_members_mock = self.app.client_manager.image.image_members - self.image_members_mock.reset_mock() self.image_tags_mock = self.app.client_manager.image.image_tags - self.image_tags_mock.reset_mock() # Get shortcut to the Mocks in identity client self.project_mock = self.app.client_manager.identity.projects @@ -49,9 +54,6 @@ class TestImage(image_fakes.TestImagev2): def setup_images_mock(self, count): images = image_fakes.FakeImage.create_images(count=count) - self.images_mock.get = image_fakes.FakeImage.get_images( - images, - 0) return images @@ -64,26 +66,22 @@ class TestImageCreate(TestImage): super(TestImageCreate, self).setUp() self.new_image = image_fakes.FakeImage.create_one_image() - self.images_mock.create.return_value = self.new_image + self.client.create_image.return_value = self.new_image self.project_mock.get.return_value = self.project self.domain_mock.get.return_value = self.domain - # This is the return value for utils.find_resource() - self.images_mock.get.return_value = copy.deepcopy( - self.new_image - ) - self.images_mock.update.return_value = self.new_image + self.client.update_image.return_value = self.new_image + + (self.expected_columns, self.expected_data) = zip( + *sorted(image._format_image(self.new_image).items())) # Get the command object to test self.cmd = image.CreateImage(self.app, None) - def test_image_reserve_no_options(self): - mock_exception = { - 'find.side_effect': exceptions.CommandError('x'), - } - self.images_mock.configure_mock(**mock_exception) + @mock.patch("sys.stdin", side_effect=[None]) + def test_image_reserve_no_options(self, raw_input): arglist = [ self.new_image.name ] @@ -100,45 +98,34 @@ class TestImageCreate(TestImage): columns, data = self.cmd.take_action(parsed_args) # ImageManager.create(name=, **) - self.images_mock.create.assert_called_with( + self.client.create_image.assert_called_with( name=self.new_image.name, container_format=image.DEFAULT_CONTAINER_FORMAT, disk_format=image.DEFAULT_DISK_FORMAT, ) # Verify update() was not called, if it was show the args - self.assertEqual(self.images_mock.update.call_args_list, []) - - self.images_mock.upload.assert_called_with( - mock.ANY, mock.ANY, - ) + self.assertEqual(self.client.update_image.call_args_list, []) self.assertEqual( - image_fakes.FakeImage.get_image_columns(self.new_image), + self.expected_columns, columns) self.assertItemEqual( - image_fakes.FakeImage.get_image_data(self.new_image), + self.expected_data, data) - @mock.patch('glanceclient.common.utils.get_data_file', name='Open') - def test_image_reserve_options(self, mock_open): - mock_file = mock.MagicMock(name='File') - mock_open.return_value = mock_file - mock_open.read.return_value = None - mock_exception = { - 'find.side_effect': exceptions.CommandError('x'), - } - self.images_mock.configure_mock(**mock_exception) + @mock.patch('sys.stdin', side_effect=[None]) + def test_image_reserve_options(self, raw_input): arglist = [ '--container-format', 'ovf', '--disk-format', 'ami', '--min-disk', '10', '--min-ram', '4', ('--protected' - if self.new_image.protected else '--unprotected'), + if self.new_image.is_protected else '--unprotected'), ('--private' if self.new_image.visibility == 'private' else '--public'), - '--project', self.new_image.owner, + '--project', self.new_image.owner_id, '--project-domain', self.domain.id, self.new_image.name, ] @@ -147,11 +134,11 @@ class TestImageCreate(TestImage): ('disk_format', 'ami'), ('min_disk', 10), ('min_ram', 4), - ('protected', self.new_image.protected), - ('unprotected', not self.new_image.protected), + ('protected', self.new_image.is_protected), + ('unprotected', not self.new_image.is_protected), ('public', self.new_image.visibility == 'public'), ('private', self.new_image.visibility == 'private'), - ('project', self.new_image.owner), + ('project', self.new_image.owner_id), ('project_domain', self.domain.id), ('name', self.new_image.name), ] @@ -163,29 +150,22 @@ class TestImageCreate(TestImage): columns, data = self.cmd.take_action(parsed_args) # ImageManager.create(name=, **) - self.images_mock.create.assert_called_with( + self.client.create_image.assert_called_with( name=self.new_image.name, container_format='ovf', disk_format='ami', min_disk=10, min_ram=4, - owner=self.project.id, - protected=self.new_image.protected, + owner_id=self.project.id, + is_protected=self.new_image.is_protected, visibility=self.new_image.visibility, ) - # Verify update() was not called, if it was show the args - self.assertEqual(self.images_mock.update.call_args_list, []) - - self.images_mock.upload.assert_called_with( - mock.ANY, mock.ANY, - ) - self.assertEqual( - image_fakes.FakeImage.get_image_columns(self.new_image), + self.expected_columns, columns) self.assertItemEqual( - image_fakes.FakeImage.get_image_data(self.new_image), + self.expected_data, data) def test_image_create_with_unexist_project(self): @@ -222,21 +202,15 @@ class TestImageCreate(TestImage): parsed_args, ) - @mock.patch('glanceclient.common.utils.get_data_file', name='Open') - def test_image_create_file(self, mock_open): - mock_file = mock.MagicMock(name='File') - mock_open.return_value = mock_file - mock_open.read.return_value = ( - image_fakes.FakeImage.get_image_data(self.new_image)) - mock_exception = { - 'find.side_effect': exceptions.CommandError('x'), - } - self.images_mock.configure_mock(**mock_exception) + def test_image_create_file(self): + imagefile = tempfile.NamedTemporaryFile(delete=False) + imagefile.write(b'\0') + imagefile.close() arglist = [ - '--file', 'filer', + '--file', imagefile.name, ('--unprotected' - if not self.new_image.protected else '--protected'), + if not self.new_image.is_protected else '--protected'), ('--public' if self.new_image.visibility == 'public' else '--private'), '--property', 'Alpha=1', @@ -246,9 +220,9 @@ class TestImageCreate(TestImage): self.new_image.name, ] verifylist = [ - ('file', 'filer'), - ('protected', self.new_image.protected), - ('unprotected', not self.new_image.protected), + ('file', imagefile.name), + ('protected', self.new_image.is_protected), + ('unprotected', not self.new_image.is_protected), ('public', self.new_image.visibility == 'public'), ('private', self.new_image.visibility == 'private'), ('properties', {'Alpha': '1', 'Beta': '2'}), @@ -263,29 +237,23 @@ class TestImageCreate(TestImage): columns, data = self.cmd.take_action(parsed_args) # ImageManager.create(name=, **) - self.images_mock.create.assert_called_with( + self.client.create_image.assert_called_with( name=self.new_image.name, container_format=image.DEFAULT_CONTAINER_FORMAT, disk_format=image.DEFAULT_DISK_FORMAT, - protected=self.new_image.protected, + is_protected=self.new_image.is_protected, visibility=self.new_image.visibility, Alpha='1', Beta='2', tags=self.new_image.tags, - ) - - # Verify update() was not called, if it was show the args - self.assertEqual(self.images_mock.update.call_args_list, []) - - self.images_mock.upload.assert_called_with( - mock.ANY, mock.ANY, + filename=imagefile.name ) self.assertEqual( - image_fakes.FakeImage.get_image_columns(self.new_image), + self.expected_columns, columns) self.assertItemEqual( - image_fakes.FakeImage.get_image_data(self.new_image), + self.expected_data, data) def test_image_create_dead_options(self): @@ -315,25 +283,31 @@ class TestAddProjectToImage(TestImage): ) columns = ( + 'created_at', 'image_id', 'member_id', + 'schema', 'status', + 'updated_at' ) datalist = ( + new_member.created_at, _image.id, new_member.member_id, + new_member.schema, new_member.status, + new_member.updated_at ) def setUp(self): super(TestAddProjectToImage, self).setUp() # This is the return value for utils.find_resource() - self.images_mock.get.return_value = self._image + self.client.find_image.return_value = self._image # Update the image_id in the MEMBER dict - self.image_members_mock.create.return_value = self.new_member + self.client.add_member.return_value = self.new_member self.project_mock.get.return_value = self.project self.domain_mock.get.return_value = self.domain # Get the command object to test @@ -354,9 +328,9 @@ class TestAddProjectToImage(TestImage): # returns a two-part tuple with a tuple of column names and a tuple of # data to be shown. columns, data = self.cmd.take_action(parsed_args) - self.image_members_mock.create.assert_called_with( - self._image.id, - self.project.id + self.client.add_member.assert_called_with( + image=self._image.id, + member_id=self.project.id ) self.assertEqual(self.columns, columns) @@ -379,10 +353,11 @@ class TestAddProjectToImage(TestImage): # returns a two-part tuple with a tuple of column names and a tuple of # data to be shown. columns, data = self.cmd.take_action(parsed_args) - self.image_members_mock.create.assert_called_with( - self._image.id, - self.project.id + self.client.add_member.assert_called_with( + image=self._image.id, + member_id=self.project.id ) + self.assertEqual(self.columns, columns) self.assertEqual(self.datalist, data) @@ -392,7 +367,7 @@ class TestImageDelete(TestImage): def setUp(self): super(TestImageDelete, self).setUp() - self.images_mock.delete.return_value = None + self.client.delete_image.return_value = None # Get the command object to test self.cmd = image.DeleteImage(self.app, None) @@ -408,9 +383,11 @@ class TestImageDelete(TestImage): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.client.find_image.side_effect = images + result = self.cmd.take_action(parsed_args) - self.images_mock.delete.assert_called_with(images[0].id) + self.client.delete_image.assert_called_with(images[0].id) self.assertIsNone(result) def test_image_delete_multi_images(self): @@ -422,10 +399,12 @@ class TestImageDelete(TestImage): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.client.find_image.side_effect = images + result = self.cmd.take_action(parsed_args) calls = [mock.call(i.id) for i in images] - self.images_mock.delete.assert_has_calls(calls) + self.client.delete_image.assert_has_calls(calls) self.assertIsNone(result) def test_image_delete_multi_images_exception(self): @@ -449,15 +428,15 @@ class TestImageDelete(TestImage): ret_find = [ images[0], images[1], - exceptions.NotFound('404'), + sdk_exceptions.ResourceNotFound() ] - self.images_mock.get = Exception() - self.images_mock.find.side_effect = ret_find + self.client.find_image.side_effect = ret_find + self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) calls = [mock.call(i.id) for i in images] - self.images_mock.delete.assert_has_calls(calls) + self.client.delete_image.assert_has_calls(calls) class TestImageList(TestImage): @@ -473,17 +452,17 @@ class TestImageList(TestImage): datalist = ( _image.id, _image.name, - '', + None, ), def setUp(self): super(TestImageList, self).setUp() self.api_mock = mock.Mock() - self.api_mock.image_list.side_effect = [ + self.api_mock.side_effect = [ [self._image], [], ] - self.app.client_manager.image.api = self.api_mock + self.client.images = self.api_mock # Get the command object to test self.cmd = image.ListImage(self.app, None) @@ -503,8 +482,8 @@ class TestImageList(TestImage): # returns a tuple containing the column names and an iterable # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - marker=self._image.id, + self.client.images.assert_called_with( + # marker=self._image.id, ) self.assertEqual(self.columns, columns) @@ -527,9 +506,8 @@ class TestImageList(TestImage): # returns a tuple containing the column names and an iterable # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - public=True, - marker=self._image.id, + self.client.images.assert_called_with( + visibility='public', ) self.assertEqual(self.columns, columns) @@ -552,9 +530,8 @@ class TestImageList(TestImage): # returns a tuple containing the column names and an iterable # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - private=True, - marker=self._image.id, + self.client.images.assert_called_with( + visibility='private', ) self.assertEqual(self.columns, columns) @@ -577,9 +554,8 @@ class TestImageList(TestImage): # returns a tuple containing the column names and an iterable # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - community=True, - marker=self._image.id, + self.client.images.assert_called_with( + visibility='community', ) self.assertEqual(self.columns, columns) @@ -602,9 +578,8 @@ class TestImageList(TestImage): # returns a tuple containing the column names and an iterable # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - shared=True, - marker=self._image.id, + self.client.images.assert_called_with( + visibility='shared', ) self.assertEqual(self.columns, columns) @@ -629,10 +604,9 @@ class TestImageList(TestImage): # returns a tuple containing the column names and an iterable # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - shared=True, + self.client.images.assert_called_with( + visibility='shared', member_status='all', - marker=self._image.id, ) self.assertEqual(self.columns, columns) @@ -666,8 +640,7 @@ class TestImageList(TestImage): # returns a tuple containing the column names and an iterable # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - marker=self._image.id, + self.client.images.assert_called_with( ) collist = ( @@ -688,14 +661,14 @@ class TestImageList(TestImage): datalist = (( self._image.id, self._image.name, - '', - '', - '', - '', - '', + None, + None, + None, + None, + None, self._image.visibility, - self._image.protected, - self._image.owner, + self._image.is_protected, + self._image.owner_id, format_columns.ListColumn(self._image.tags), ), ) self.assertListItemEqual(datalist, tuple(data)) @@ -716,8 +689,7 @@ class TestImageList(TestImage): # returns a tuple containing the column names and an iterable # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - marker=self._image.id, + self.client.images.assert_called_with( ) sf_mock.assert_called_with( [self._image], @@ -741,8 +713,7 @@ class TestImageList(TestImage): # returns a tuple containing the column names and an iterable # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - marker=self._image.id, + self.client.images.assert_called_with( ) si_mock.assert_called_with( [self._image], @@ -763,8 +734,10 @@ class TestImageList(TestImage): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - limit=ret_limit, marker=None + self.client.images.assert_called_with( + limit=ret_limit, + paginated=False + # marker=None ) self.assertEqual(self.columns, columns) @@ -775,8 +748,9 @@ class TestImageList(TestImage): # tangchen: Since image_fakes.IMAGE is a dict, it cannot offer a .id # operation. Will fix this by using FakeImage class instead # of IMAGE dict. - fr_mock.return_value = mock.Mock() - fr_mock.return_value.id = image_fakes.image_id + self.client.find_image = mock.Mock(return_value=self._image) +# fr_mock.return_value = mock.Mock() +# fr_mock.return_value.id = image_fakes.image_id arglist = [ '--marker', image_fakes.image_name, @@ -787,10 +761,12 @@ class TestImageList(TestImage): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - marker=image_fakes.image_id, + self.client.images.assert_called_with( + marker=self._image.id, ) + self.client.find_image.assert_called_with(image_fakes.image_name) + def test_image_list_name_option(self): arglist = [ '--name', 'abc', @@ -801,8 +777,9 @@ class TestImageList(TestImage): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - name='abc', marker=self._image.id + self.client.images.assert_called_with( + name='abc', + # marker=self._image.id ) def test_image_list_status_option(self): @@ -815,8 +792,8 @@ class TestImageList(TestImage): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - status='active', marker=self._image.id + self.client.images.assert_called_with( + status='active' ) def test_image_list_tag_option(self): @@ -829,8 +806,8 @@ class TestImageList(TestImage): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.api_mock.image_list.assert_called_with( - tag='abc', marker=self._image.id + self.client.images.assert_called_with( + tag='abc' ) @@ -849,17 +826,17 @@ class TestListImageProjects(TestImage): "Status" ) - datalist = (( + datalist = [( _image.id, member.member_id, member.status, - )) + )] def setUp(self): super(TestListImageProjects, self).setUp() - self.images_mock.get.return_value = self._image - self.image_members_mock.list.return_value = self.datalist + self.client.find_image.return_value = self._image + self.client.members.return_value = [self.member] self.cmd = image.ListImageProjects(self.app, None) @@ -874,10 +851,10 @@ class TestListImageProjects(TestImage): columns, data = self.cmd.take_action(parsed_args) - self.image_members_mock.list.assert_called_with(self._image.id) + self.client.members.assert_called_with(image=self._image.id) self.assertEqual(self.columns, columns) - self.assertEqual(len(self.datalist), len(tuple(data))) + self.assertEqual(self.datalist, list(data)) class TestRemoveProjectImage(TestImage): @@ -890,11 +867,11 @@ class TestRemoveProjectImage(TestImage): self._image = image_fakes.FakeImage.create_one_image() # This is the return value for utils.find_resource() - self.images_mock.get.return_value = self._image + self.client.find_image.return_value = self._image self.project_mock.get.return_value = self.project self.domain_mock.get.return_value = self.domain - self.image_members_mock.delete.return_value = None + self.client.remove_member.return_value = None # Get the command object to test self.cmd = image.RemoveProjectImage(self.app, None) @@ -911,9 +888,13 @@ class TestRemoveProjectImage(TestImage): result = self.cmd.take_action(parsed_args) - self.image_members_mock.delete.assert_called_with( + self.client.find_image.assert_called_with( self._image.id, - self.project.id, + ignore_missing=False) + + self.client.remove_member.assert_called_with( + member=self.project.id, + image=self._image.id, ) self.assertIsNone(result) @@ -932,9 +913,9 @@ class TestRemoveProjectImage(TestImage): result = self.cmd.take_action(parsed_args) - self.image_members_mock.delete.assert_called_with( - self._image.id, - self.project.id, + self.client.remove_member.assert_called_with( + member=self.project.id, + image=self._image.id, ) self.assertIsNone(result) @@ -943,21 +924,16 @@ class TestImageSet(TestImage): project = identity_fakes.FakeProject.create_one_project() domain = identity_fakes.FakeDomain.create_one_domain() + _image = image_fakes.FakeImage.create_one_image({'tags': []}) def setUp(self): super(TestImageSet, self).setUp() - # Set up the schema - self.model = warlock.model_factory( - image_fakes.IMAGE_schema, - schemas.SchemaBasedModel, - ) self.project_mock.get.return_value = self.project self.domain_mock.get.return_value = self.domain - self.images_mock.get.return_value = self.model(**image_fakes.IMAGE) - self.images_mock.update.return_value = self.model(**image_fakes.IMAGE) + self.client.find_image.return_value = self._image self.app.client_manager.auth_ref = mock.Mock( project_id=self.project.id, @@ -986,38 +962,38 @@ class TestImageSet(TestImage): attrs={'image_id': image_fakes.image_id, 'member_id': self.project.id} ) - self.image_members_mock.update.return_value = membership + self.client.update_member.return_value = membership arglist = [ '--accept', - image_fakes.image_id, + self._image.id, ] verifylist = [ ('accept', True), ('reject', False), ('pending', False), - ('image', image_fakes.image_id) + ('image', self._image.id) ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) - self.image_members_mock.update.assert_called_once_with( - image_fakes.image_id, - self.app.client_manager.auth_ref.project_id, - 'accepted', + self.client.update_member.assert_called_once_with( + image=self._image.id, + member=self.app.client_manager.auth_ref.project_id, + status='accepted', ) # Assert that the 'update image" route is also called, in addition to # the 'update membership' route. - self.images_mock.update.assert_called_with(image_fakes.image_id) + self.client.update_image.assert_called_with(self._image.id) def test_image_set_membership_option_reject(self): membership = image_fakes.FakeImage.create_one_image_member( attrs={'image_id': image_fakes.image_id, 'member_id': self.project.id} ) - self.image_members_mock.update.return_value = membership + self.client.update_member.return_value = membership arglist = [ '--reject', @@ -1033,22 +1009,22 @@ class TestImageSet(TestImage): parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) - self.image_members_mock.update.assert_called_once_with( - image_fakes.image_id, - self.app.client_manager.auth_ref.project_id, - 'rejected', + self.client.update_member.assert_called_once_with( + image=self._image.id, + member=self.app.client_manager.auth_ref.project_id, + status='rejected', ) # Assert that the 'update image" route is also called, in addition to # the 'update membership' route. - self.images_mock.update.assert_called_with(image_fakes.image_id) + self.client.update_image.assert_called_with(self._image.id) def test_image_set_membership_option_pending(self): membership = image_fakes.FakeImage.create_one_image_member( attrs={'image_id': image_fakes.image_id, 'member_id': self.project.id} ) - self.image_members_mock.update.return_value = membership + self.client.update_member.return_value = membership arglist = [ '--pending', @@ -1064,15 +1040,15 @@ class TestImageSet(TestImage): parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) - self.image_members_mock.update.assert_called_once_with( - image_fakes.image_id, - self.app.client_manager.auth_ref.project_id, - 'pending', + self.client.update_member.assert_called_once_with( + image=self._image.id, + member=self.app.client_manager.auth_ref.project_id, + status='pending', ) # Assert that the 'update image" route is also called, in addition to # the 'update membership' route. - self.images_mock.update.assert_called_with(image_fakes.image_id) + self.client.update_image.assert_called_with(self._image.id) def test_image_set_options(self): arglist = [ @@ -1083,7 +1059,7 @@ class TestImageSet(TestImage): '--disk-format', 'vmdk', '--project', self.project.name, '--project-domain', self.domain.id, - image_fakes.image_id, + self._image.id, ] verifylist = [ ('name', 'new-name'), @@ -1093,7 +1069,7 @@ class TestImageSet(TestImage): ('disk_format', 'vmdk'), ('project', self.project.name), ('project_domain', self.domain.id), - ('image', image_fakes.image_id), + ('image', self._image.id), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1101,15 +1077,15 @@ class TestImageSet(TestImage): kwargs = { 'name': 'new-name', - 'owner': self.project.id, + 'owner_id': self.project.id, 'min_disk': 2, 'min_ram': 4, 'container_format': 'ovf', 'disk_format': 'vmdk', } # ImageManager.update(image, **kwargs) - self.images_mock.update.assert_called_with( - image_fakes.image_id, **kwargs) + self.client.update_image.assert_called_with( + self._image.id, **kwargs) self.assertIsNone(result) def test_image_set_with_unexist_project(self): @@ -1148,12 +1124,12 @@ class TestImageSet(TestImage): result = self.cmd.take_action(parsed_args) kwargs = { - 'protected': True, + 'is_protected': True, 'visibility': 'private', } # ImageManager.update(image, **kwargs) - self.images_mock.update.assert_called_with( - image_fakes.image_id, + self.client.update_image.assert_called_with( + self._image.id, **kwargs ) self.assertIsNone(result) @@ -1176,12 +1152,12 @@ class TestImageSet(TestImage): result = self.cmd.take_action(parsed_args) kwargs = { - 'protected': False, + 'is_protected': False, 'visibility': 'public', } # ImageManager.update(image, **kwargs) - self.images_mock.update.assert_called_with( - image_fakes.image_id, + self.client.update_image.assert_called_with( + self._image.id, **kwargs ) self.assertIsNone(result) @@ -1205,8 +1181,8 @@ class TestImageSet(TestImage): 'Beta': '2', } # ImageManager.update(image, **kwargs) - self.images_mock.update.assert_called_with( - image_fakes.image_id, + self.client.update_image.assert_called_with( + self._image.id, **kwargs ) self.assertIsNone(result) @@ -1243,8 +1219,8 @@ class TestImageSet(TestImage): 'ramdisk_id': 'xyzpdq', } # ImageManager.update(image, **kwargs) - self.images_mock.update.assert_called_with( - image_fakes.image_id, + self.client.update_image.assert_called_with( + self._image.id, **kwargs ) self.assertIsNone(result) @@ -1266,8 +1242,8 @@ class TestImageSet(TestImage): 'tags': ['test-tag'], } # ImageManager.update(image, **kwargs) - self.images_mock.update.assert_called_with( - image_fakes.image_id, + self.client.update_image.assert_called_with( + self._image.id, **kwargs ) self.assertIsNone(result) @@ -1290,12 +1266,12 @@ class TestImageSet(TestImage): 'tags': ['test-tag'], } - self.images_mock.reactivate.assert_called_with( - image_fakes.image_id, + self.client.reactivate_image.assert_called_with( + self._image.id, ) # ImageManager.update(image, **kwargs) - self.images_mock.update.assert_called_with( - image_fakes.image_id, + self.client.update_image.assert_called_with( + self._image.id, **kwargs ) self.assertIsNone(result) @@ -1318,20 +1294,20 @@ class TestImageSet(TestImage): 'tags': ['test-tag'], } - self.images_mock.deactivate.assert_called_with( - image_fakes.image_id, + self.client.deactivate_image.assert_called_with( + self._image.id, ) # ImageManager.update(image, **kwargs) - self.images_mock.update.assert_called_with( - image_fakes.image_id, + self.client.update_image.assert_called_with( + self._image.id, **kwargs ) self.assertIsNone(result) def test_image_set_tag_merge(self): - old_image = copy.copy(image_fakes.IMAGE) + old_image = self._image old_image['tags'] = ['old1', 'new2'] - self.images_mock.get.return_value = self.model(**old_image) + self.client.find_image.return_value = old_image arglist = [ '--tag', 'test-tag', image_fakes.image_name, @@ -1348,16 +1324,16 @@ class TestImageSet(TestImage): 'tags': ['old1', 'new2', 'test-tag'], } # ImageManager.update(image, **kwargs) - a, k = self.images_mock.update.call_args - self.assertEqual(image_fakes.image_id, a[0]) + a, k = self.client.update_image.call_args + self.assertEqual(self._image.id, a[0]) self.assertIn('tags', k) self.assertEqual(set(kwargs['tags']), set(k['tags'])) self.assertIsNone(result) def test_image_set_tag_merge_dupe(self): - old_image = copy.copy(image_fakes.IMAGE) + old_image = self._image old_image['tags'] = ['old1', 'new2'] - self.images_mock.get.return_value = self.model(**old_image) + self.client.find_image.return_value = old_image arglist = [ '--tag', 'old1', image_fakes.image_name, @@ -1374,8 +1350,8 @@ class TestImageSet(TestImage): 'tags': ['new2', 'old1'], } # ImageManager.update(image, **kwargs) - a, k = self.images_mock.update.call_args - self.assertEqual(image_fakes.image_id, a[0]) + a, k = self.client.update_image.call_args + self.assertEqual(self._image.id, a[0]) self.assertIn('tags', k) self.assertEqual(set(kwargs['tags']), set(k['tags'])) self.assertIsNone(result) @@ -1416,8 +1392,8 @@ class TestImageSet(TestImage): 'min_ram': 0, } # ImageManager.update(image, **kwargs) - self.images_mock.update.assert_called_with( - image_fakes.image_id, + self.client.update_image.assert_called_with( + self._image.id, **kwargs ) self.assertIsNone(result) @@ -1428,16 +1404,25 @@ class TestImageShow(TestImage): new_image = image_fakes.FakeImage.create_one_image( attrs={'size': 1000}) + _data = image_fakes.FakeImage.create_one_image() + + columns = ( + 'id', 'name', 'owner', 'protected', 'tags', 'visibility' + ) + + data = ( + _data.id, + _data.name, + _data.owner_id, + _data.is_protected, + format_columns.ListColumn(_data.tags), + _data.visibility + ) + def setUp(self): super(TestImageShow, self).setUp() - # Set up the schema - self.model = warlock.model_factory( - image_fakes.IMAGE_schema, - schemas.SchemaBasedModel, - ) - - self.images_mock.get.return_value = self.model(**image_fakes.IMAGE) + self.client.find_image = mock.Mock(return_value=self._data) # Get the command object to test self.cmd = image.ShowImage(self.app, None) @@ -1455,15 +1440,16 @@ class TestImageShow(TestImage): # returns a two-part tuple with a tuple of column names and a tuple of # data to be shown. columns, data = self.cmd.take_action(parsed_args) - self.images_mock.get.assert_called_with( + self.client.find_image.assert_called_with( image_fakes.image_id, + ignore_missing=False ) - self.assertEqual(image_fakes.IMAGE_columns, columns) - self.assertItemEqual(image_fakes.IMAGE_SHOW_data, data) + self.assertEqual(self.columns, columns) + self.assertItemEqual(self.data, data) def test_image_show_human_readable(self): - self.images_mock.get.return_value = self.new_image + self.client.find_image.return_value = self.new_image arglist = [ '--human-readable', self.new_image.id, @@ -1478,8 +1464,9 @@ class TestImageShow(TestImage): # returns a two-part tuple with a tuple of column names and a tuple of # data to be shown. columns, data = self.cmd.take_action(parsed_args) - self.images_mock.get.assert_called_with( + self.client.find_image.assert_called_with( self.new_image.id, + ignore_missing=False ) size_index = columns.index('size') @@ -1491,19 +1478,15 @@ class TestImageUnset(TestImage): attrs = {} attrs['tags'] = ['test'] attrs['prop'] = 'test' + attrs['prop2'] = 'fake' image = image_fakes.FakeImage.create_one_image(attrs) def setUp(self): super(TestImageUnset, self).setUp() - # Set up the schema - self.model = warlock.model_factory( - image_fakes.IMAGE_schema, - schemas.SchemaBasedModel, - ) - - self.images_mock.get.return_value = self.image - self.image_tags_mock.delete.return_value = self.image + self.client.find_image.return_value = self.image + self.client.remove_tag.return_value = self.image + self.client.update_image.return_value = self.image # Get the command object to test self.cmd = image.UnsetImage(self.app, None) @@ -1535,7 +1518,7 @@ class TestImageUnset(TestImage): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.image_tags_mock.delete.assert_called_with( + self.client.remove_tag.assert_called_with( self.image.id, 'test' ) self.assertIsNone(result) @@ -1555,9 +1538,9 @@ class TestImageUnset(TestImage): result = self.cmd.take_action(parsed_args) kwargs = {} - self.images_mock.update.assert_called_with( - self.image.id, - parsed_args.properties, + self.client.update_image.assert_called_with( + self.image, + properties={'prop2': 'fake'}, **kwargs) self.assertIsNone(result) @@ -1579,12 +1562,12 @@ class TestImageUnset(TestImage): result = self.cmd.take_action(parsed_args) kwargs = {} - self.images_mock.update.assert_called_with( - self.image.id, - parsed_args.properties, + self.client.update_image.assert_called_with( + self.image, + properties={'prop2': 'fake'}, **kwargs) - self.image_tags_mock.delete.assert_called_with( + self.client.remove_tag.assert_called_with( self.image.id, 'test' ) self.assertIsNone(result) @@ -1597,18 +1580,13 @@ class TestImageSave(TestImage): def setUp(self): super(TestImageSave, self).setUp() - # Generate a request id - self.resp = mock.MagicMock() - self.resp.headers['x-openstack-request-id'] = 'req_id' + self.client.find_image.return_value = self.image + self.client.download_image.return_value = self.image # Get the command object to test self.cmd = image.SaveImage(self.app, None) def test_save_data(self): - req_id_proxy = glanceclient_utils.RequestIdProxy( - ['some_data', self.resp] - ) - self.images_mock.data.return_value = req_id_proxy arglist = ['--file', '/path/to/file', self.image.id] @@ -1618,23 +1596,58 @@ class TestImageSave(TestImage): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - with mock.patch('glanceclient.common.utils.save_image') as mocked_save: - self.cmd.take_action(parsed_args) - mocked_save.assert_called_once_with(req_id_proxy, '/path/to/file') + self.cmd.take_action(parsed_args) - def test_save_no_data(self): - req_id_proxy = glanceclient_utils.RequestIdProxy( - [None, self.resp] - ) - self.images_mock.data.return_value = req_id_proxy + self.client.download_image.assert_called_once_with( + self.image.id, + output='/path/to/file') - arglist = ['--file', '/path/to/file', self.image.id] - verifylist = [ - ('file', '/path/to/file'), - ('image', self.image.id) - ] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) +class TestImageGetData(TestImage): - # Raise SystemExit if no data was provided. - self.assertRaises(SystemExit, self.cmd.take_action, parsed_args) + def setUp(self): + super(TestImageGetData, self).setUp() + self.args = mock.Mock() + + def test_get_data_file_file(self): + (fd, fname) = tempfile.mkstemp(prefix='osc_test_image') + self.args.file = fname + + (test_fd, test_name) = image.get_data_file(self.args) + + self.assertEqual(fname, test_name) + test_fd.close() + + os.unlink(fname) + + def test_get_data_file_2(self): + + self.args.file = None + + f = io.BytesIO(b"some initial binary data: \x00\x01") + + with mock.patch('sys.stdin') as stdin: + stdin.return_value = f + stdin.isatty.return_value = False + stdin.buffer = f + + (test_fd, test_name) = image.get_data_file(self.args) + + # Ensure data written to temp file is correct + self.assertEqual(f, test_fd) + self.assertIsNone(test_name) + + def test_get_data_file_3(self): + + self.args.file = None + + f = io.BytesIO(b"some initial binary data: \x00\x01") + + with mock.patch('sys.stdin') as stdin: + # There is stdin, but interactive + stdin.return_value = f + + (test_fd, test_fname) = image.get_data_file(self.args) + + self.assertIsNone(test_fd) + self.assertIsNone(test_fname) diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py index 5d41b3a180..4e204ad1b2 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume.py +++ b/openstackclient/tests/unit/volume/v2/test_volume.py @@ -41,8 +41,8 @@ class TestVolume(volume_fakes.TestVolume): self.users_mock = self.app.client_manager.identity.users self.users_mock.reset_mock() - self.images_mock = self.app.client_manager.image.images - self.images_mock.reset_mock() + self.find_image_mock = self.app.client_manager.image.find_image + self.find_image_mock.reset_mock() self.snapshots_mock = self.app.client_manager.volume.volume_snapshots self.snapshots_mock.reset_mock() @@ -222,7 +222,7 @@ class TestVolumeCreate(TestVolume): def test_volume_create_image_id(self): image = image_fakes.FakeImage.create_one_image() - self.images_mock.get.return_value = image + self.find_image_mock.return_value = image arglist = [ '--image', image.id, @@ -260,7 +260,7 @@ class TestVolumeCreate(TestVolume): def test_volume_create_image_name(self): image = image_fakes.FakeImage.create_one_image() - self.images_mock.get.return_value = image + self.find_image_mock.return_value = image arglist = [ '--image', image.name, diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py index 4dde1340a7..1e0cb183fc 100644 --- a/openstackclient/volume/v2/volume.py +++ b/openstackclient/volume/v2/volume.py @@ -193,9 +193,8 @@ class CreateVolume(command.ShowOne): image = None if parsed_args.image: - image = utils.find_resource( - image_client.images, - parsed_args.image).id + image = image_client.find_image(parsed_args.image, + ignore_missing=False).id size = parsed_args.size diff --git a/releasenotes/notes/use_sdk_for_image-f49d2df38e7d9f81.yaml b/releasenotes/notes/use_sdk_for_image-f49d2df38e7d9f81.yaml new file mode 100644 index 0000000000..ba10f388c7 --- /dev/null +++ b/releasenotes/notes/use_sdk_for_image-f49d2df38e7d9f81.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Image service for v2 is switched from using glanceclient to OpenStackSDK. diff --git a/requirements.txt b/requirements.txt index aaea495e14..bf2a0590f3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,8 +6,7 @@ six>=1.10.0 # MIT Babel!=2.4.0,>=2.3.4 # BSD cliff!=2.9.0,>=2.8.0 # Apache-2.0 -keystoneauth1>=3.14.0 # Apache-2.0 -openstacksdk>=0.17.0 # Apache-2.0 +openstacksdk>=0.36.0 # Apache-2.0 osc-lib>=2.0.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0