From fe94fb13fa87c39e3a4e2ab1917e2123ac9bc0e9 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 1 Oct 2018 16:45:02 +0200 Subject: [PATCH] Completely remove support for deprecated Glance V1 Also removes support for having several API versions for the Image API. We do not do it for Neutron and Swift. And if we ever support the (hypothetical) Glance V3 API, we better do it transparently, not with a configuration option (maybe using openstacksdk). Cleans up Glance V1 specific features like image.is_public or a separate image.properties field. The code should be consolidated into fewer modules in a follow-up. Story: #1670423 Task: #26830 Change-Id: I08a01eadbe8a2dec4c02674bed0391846632ef06 --- .../glance_service/base_image_service.py | 15 +--- ironic/common/glance_service/service.py | 45 ----------- ironic/common/glance_service/service_utils.py | 68 ++++------------ ironic/common/glance_service/v1/__init__.py | 0 .../common/glance_service/v1/image_service.py | 28 ------- .../common/glance_service/v2/image_service.py | 4 +- ironic/common/image_service.py | 22 ++---- ironic/common/images.py | 3 +- ironic/common/pxe_utils.py | 3 +- ironic/conf/glance.py | 6 -- ironic/drivers/modules/deploy_utils.py | 3 +- ironic/drivers/modules/irmc/boot.py | 25 +++++- .../tests/unit/common/test_glance_service.py | 77 ++++++++----------- .../tests/unit/common/test_image_service.py | 14 +--- .../unit/drivers/modules/irmc/test_boot.py | 22 +++++- .../unit/drivers/modules/test_deploy_utils.py | 12 +-- ironic/tests/unit/stubs.py | 18 ++--- .../notes/no-glance-v1-d249e8079f46f40c.yaml | 8 ++ 18 files changed, 123 insertions(+), 250 deletions(-) delete mode 100644 ironic/common/glance_service/service.py delete mode 100644 ironic/common/glance_service/v1/__init__.py delete mode 100644 ironic/common/glance_service/v1/image_service.py create mode 100644 releasenotes/notes/no-glance-v1-d249e8079f46f40c.yaml diff --git a/ironic/common/glance_service/base_image_service.py b/ironic/common/glance_service/base_image_service.py index fd73dc4e66..6d62704607 100644 --- a/ironic/common/glance_service/base_image_service.py +++ b/ironic/common/glance_service/base_image_service.py @@ -110,7 +110,7 @@ def check_image_service(func): if self.context.auth_token: user_auth = keystone.get_service_auth(self.context, self.endpoint, service_auth) - self.client = client.Client(self.version, session=session, + self.client = client.Client(2, session=session, auth=user_auth or service_auth, endpoint_override=self.endpoint, global_request_id=self.context.global_id) @@ -121,9 +121,8 @@ def check_image_service(func): class BaseImageService(object): - def __init__(self, client=None, version=1, context=None): + def __init__(self, client=None, context=None): self.client = client - self.version = version self.context = context self.endpoint = None @@ -134,7 +133,6 @@ class BaseImageService(object): retry the request according to CONF.glance_num_retries. :param context: The request context, for access checks. - :param version: The requested API version.v :param method: The method requested to be called. :param args: A list of positional arguments for the method called :param kwargs: A dict of keyword arguments for the method called @@ -208,9 +206,7 @@ class BaseImageService(object): """ image_id = service_utils.parse_image_id(image_href) - if (self.version == 2 - and 'file' in CONF.glance.allowed_direct_url_schemes): - + if 'file' in CONF.glance.allowed_direct_url_schemes: location = self._get_location(image_id) url = urlparse.urlparse(location) if url.scheme == "file": @@ -222,10 +218,7 @@ class BaseImageService(object): image_chunks = self.call(method, image_id) # NOTE(dtantsur): when using Glance V2, image_chunks is a wrapper # around real data, so we have to check the wrapped data for None. - # Glance V1 returns HTTP 404 in this case, so no need to fix it. - # TODO(dtantsur): remove the hasattr check when we no longer support - # Glance V1. - if hasattr(image_chunks, 'wrapped') and image_chunks.wrapped is None: + if image_chunks.wrapped is None: raise exception.ImageDownloadFailed( image_href=image_href, reason=_('image contains no data.')) diff --git a/ironic/common/glance_service/service.py b/ironic/common/glance_service/service.py deleted file mode 100644 index c4c8b18525..0000000000 --- a/ironic/common/glance_service/service.py +++ /dev/null @@ -1,45 +0,0 @@ -# Copyright 2013 Hewlett-Packard Development Company, L.P. -# All Rights Reserved. -# -# 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 abc - -import six - - -@six.add_metaclass(abc.ABCMeta) -class ImageService(object): - """Provides storage and retrieval of disk image objects within Glance.""" - - @abc.abstractmethod - def __init__(self): - """Constructor.""" - - @abc.abstractmethod - def show(self, image_id): - """Returns a dict with image data for the given opaque image id. - - :param image_id: The opaque image identifier. - :returns: A dict containing image metadata. - - :raises: ImageNotFound - """ - - @abc.abstractmethod - def download(self, image_id, data=None): - """Calls out to Glance for data and writes data. - - :param image_id: The opaque image identifier. - :param data: (Optional) File object to write data to. - """ diff --git a/ironic/common/glance_service/service_utils.py b/ironic/common/glance_service/service_utils.py index bb47d81d3b..cbcd7beddb 100644 --- a/ironic/common/glance_service/service_utils.py +++ b/ironic/common/glance_service/service_utils.py @@ -23,10 +23,8 @@ from oslo_serialization import jsonutils from oslo_utils import timeutils from oslo_utils import uuidutils import six -import six.moves.urllib.parse as urlparse from ironic.common import exception -from ironic.common import image_service from ironic.conf import CONF @@ -36,31 +34,24 @@ _GLANCE_API_SERVER = None """ iterator that cycles (indefinitely) over glance API servers. """ +_IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner', + 'container_format', 'checksum', 'id', + 'name', 'created_at', 'updated_at', + 'deleted_at', 'deleted', 'status', + 'min_disk', 'min_ram', 'tags', 'visibility', + 'protected', 'file', 'schema'] + + def _extract_attributes(image): - # TODO(pas-ha) in Queens unify these once GlanceV1 is no longer supported - IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner', - 'container_format', 'checksum', 'id', - 'name', 'created_at', 'updated_at', - 'deleted_at', 'deleted', 'status', - 'min_disk', 'min_ram', 'is_public'] - - IMAGE_ATTRIBUTES_V2 = ['tags', 'visibility', 'protected', - 'file', 'schema'] - output = {} - for attr in IMAGE_ATTRIBUTES: + for attr in _IMAGE_ATTRIBUTES: output[attr] = getattr(image, attr, None) - output['properties'] = getattr(image, 'properties', {}) + output['properties'] = {} + output['schema'] = image.schema - if hasattr(image, 'schema') and 'v2' in image['schema']: - IMAGE_ATTRIBUTES = IMAGE_ATTRIBUTES + IMAGE_ATTRIBUTES_V2 - for attr in IMAGE_ATTRIBUTES_V2: - output[attr] = getattr(image, attr, None) - output['schema'] = image['schema'] - - for image_property in set(image) - set(IMAGE_ATTRIBUTES): - output['properties'][image_property] = image[image_property] + for image_property in set(image) - set(_IMAGE_ATTRIBUTES): + output['properties'][image_property] = image[image_property] return output @@ -154,23 +145,11 @@ def is_image_available(context, image): if hasattr(context, 'auth_token') and context.auth_token: return True - if ((getattr(image, 'is_public', None) - or getattr(image, 'visibility', None) == 'public') - or context.is_admin): + if getattr(image, 'visibility', None) == 'public' or context.is_admin: return True - properties = image.properties - if context.project_id and ('owner_id' in properties): - return str(properties['owner_id']) == str(context.project_id) - if context.project_id and ('project_id' in properties): - return str(properties['project_id']) == str(context.project_id) - - try: - user_id = properties['user_id'] - except KeyError: - return False - - return str(user_id) == str(context.user_id) + return (context.project_id and + getattr(image, 'owner', None) == context.project_id) def is_image_active(image): @@ -187,18 +166,3 @@ def is_glance_image(image_href): return False return (image_href.startswith('glance://') or uuidutils.is_uuid_like(image_href)) - - -def is_image_href_ordinary_file_name(image_href): - """Check if image_href is a ordinary file name. - - This method judges if image_href is an ordinary file name or not, - which is a file supposed to be stored in share file system. - The ordinary file name is neither glance image href - nor image service href. - - :returns: True if image_href is ordinary file name, False otherwise. - """ - return not (is_glance_image(image_href) - or urlparse.urlparse(image_href).scheme.lower() in - image_service.protocol_mapping) diff --git a/ironic/common/glance_service/v1/__init__.py b/ironic/common/glance_service/v1/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/ironic/common/glance_service/v1/image_service.py b/ironic/common/glance_service/v1/image_service.py deleted file mode 100644 index fb23e43bc2..0000000000 --- a/ironic/common/glance_service/v1/image_service.py +++ /dev/null @@ -1,28 +0,0 @@ -# Copyright 2013 Hewlett-Packard Development Company, L.P. -# All Rights Reserved. -# -# 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. - - -from ironic.common.glance_service import base_image_service -from ironic.common.glance_service import service - - -class GlanceImageService(base_image_service.BaseImageService, - service.ImageService): - - def show(self, image_id): - return self._show(image_id, method='get') - - def download(self, image_id, data=None): - return self._download(image_id, method='data', data=data) diff --git a/ironic/common/glance_service/v2/image_service.py b/ironic/common/glance_service/v2/image_service.py index d7d18f29f5..db4e346657 100644 --- a/ironic/common/glance_service/v2/image_service.py +++ b/ironic/common/glance_service/v2/image_service.py @@ -23,7 +23,6 @@ from swiftclient import utils as swift_utils from ironic.common import exception as exc from ironic.common.glance_service import base_image_service -from ironic.common.glance_service import service from ironic.common.glance_service import service_utils from ironic.common.i18n import _ from ironic.common import keystone @@ -34,8 +33,7 @@ TempUrlCacheElement = collections.namedtuple('TempUrlCacheElement', ['url', 'url_expires_at']) -class GlanceImageService(base_image_service.BaseImageService, - service.ImageService): +class GlanceImageService(base_image_service.BaseImageService): # A dictionary containing cached temp URLs in namedtuples # in format: diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index 41da56b25e..cf1eefcb5c 100644 --- a/ironic/common/image_service.py +++ b/ironic/common/image_service.py @@ -21,7 +21,6 @@ import os import shutil from oslo_log import log -from oslo_utils import importutils from oslo_utils import uuidutils import requests import sendfile @@ -30,25 +29,16 @@ from six.moves import http_client import six.moves.urllib.parse as urlparse from ironic.common import exception +from ironic.common.glance_service.v2 import image_service from ironic.common.i18n import _ from ironic.common import utils -from ironic.conf import CONF IMAGE_CHUNK_SIZE = 1024 * 1024 # 1mb LOG = log.getLogger(__name__) -# TODO(pas-ha) in Queens change default to '2', -# but keep the versioned import in place (less work for possible Glance v3) -def GlanceImageService(client=None, version=None, context=None): - module_str = 'ironic.common.glance_service' - if version is None: - version = CONF.glance.glance_api_version - - module = importutils.import_versioned_module(module_str, version, - 'image_service') - service_class = getattr(module, 'GlanceImageService') - return service_class(client, version, context) +# TODO(dtantsur): temporary re-import, refactor the code and remove it. +GlanceImageService = image_service.GlanceImageService @six.add_metaclass(abc.ABCMeta) @@ -255,14 +245,12 @@ protocol_mapping = { } -def get_image_service(image_href, client=None, version=None, context=None): +def get_image_service(image_href, client=None, context=None): """Get image service instance to download the image. :param image_href: String containing href to get image service for. :param client: Glance client to be used for download, used only if image_href is Glance href. - :param version: Version of Glance API to use, used only if image_href is - Glance href. :param context: request context, used only if image_href is Glance href. :raises: exception.ImageRefValidationFailed if no image service can handle specified href. @@ -287,5 +275,5 @@ def get_image_service(image_href, client=None, version=None, context=None): ) % scheme) if cls == GlanceImageService: - return cls(client, version, context) + return cls(client, context) return cls() diff --git a/ironic/common/images.py b/ironic/common/images.py index aa6e376f39..842bd54520 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -406,8 +406,7 @@ def get_temp_url_for_glance_image(context, image_uuid): :param image_uuid: the UUID of the image in glance :returns: the tmp url for the glance image. """ - # Glance API version 2 is required for getting direct_url of the image. - glance_service = service.GlanceImageService(version=2, context=context) + glance_service = service.GlanceImageService(context=context) image_properties = glance_service.show(image_uuid) LOG.debug('Got image info: %(info)s for image %(image_uuid)s.', {'info': image_properties, 'image_uuid': image_uuid}) diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index 292c228307..6744221c96 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -505,8 +505,7 @@ def get_instance_image_info(node, ctx): labels = ('kernel', 'ramdisk') d_info = deploy_utils.get_image_instance_info(node) if not (i_info.get('kernel') and i_info.get('ramdisk')): - glance_service = service.GlanceImageService( - version=CONF.glance.glance_api_version, context=ctx) + glance_service = service.GlanceImageService(context=ctx) iproperties = glance_service.show(d_info['image_source'])['properties'] for label in labels: i_info[label] = str(iproperties[label + '_id']) diff --git a/ironic/conf/glance.py b/ironic/conf/glance.py index 0def2eaac8..af6c833b50 100644 --- a/ironic/conf/glance.py +++ b/ironic/conf/glance.py @@ -143,12 +143,6 @@ opts = [ help=_('Optional path to a CA certificate bundle to be used to ' 'validate the SSL certificate served by glance. It is ' 'used when glance_api_insecure is set to False.')), - cfg.IntOpt('glance_api_version', - help=_('Glance API version (1 or 2) to use.'), - min=1, max=2, default=2, - deprecated_for_removal=True, - deprecated_reason=_('Ironic will only support using Glance API ' - 'version 2 in the Queens release.')), ] diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 3443e9daf8..a01b60c5c5 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -1196,8 +1196,7 @@ def build_instance_info_for_deploy(task): image_source = instance_info['image_source'] if service_utils.is_glance_image(image_source): - glance = image_service.GlanceImageService(version=2, - context=task.context) + glance = image_service.GlanceImageService(context=task.context) image_info = glance.show(image_source) LOG.debug('Got image info: %(info)s for node %(node)s.', {'info': image_info, 'node': node.uuid}) diff --git a/ironic/drivers/modules/irmc/boot.py b/ironic/drivers/modules/irmc/boot.py index c1e4738ee0..e046f48ab7 100644 --- a/ironic/drivers/modules/irmc/boot.py +++ b/ironic/drivers/modules/irmc/boot.py @@ -24,11 +24,13 @@ from ironic_lib import metrics_utils from ironic_lib import utils as ironic_utils from oslo_log import log as logging from oslo_utils import importutils +import six.moves.urllib.parse as urlparse from ironic.common import boot_devices from ironic.common import exception from ironic.common.glance_service import service_utils from ironic.common.i18n import _ +from ironic.common import image_service from ironic.common import images from ironic.common import states from ironic.conductor import utils as manager_utils @@ -84,6 +86,21 @@ COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy() COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES) +def _is_image_href_ordinary_file_name(image_href): + """Check if image_href is a ordinary file name. + + This method judges if image_href is an ordinary file name or not, + which is a file supposed to be stored in share file system. + The ordinary file name is neither glance image href + nor image service href. + + :returns: True if image_href is ordinary file name, False otherwise. + """ + return not (service_utils.is_glance_image(image_href) + or urlparse.urlparse(image_href).scheme.lower() in + image_service.protocol_mapping) + + def _parse_config_option(): """Parse config file options. @@ -134,7 +151,7 @@ def _parse_driver_info(node, mode='deploy'): "parameters were missing in node's driver_info") % mode) deploy_utils.check_for_missing_params(deploy_info, error_msg) - if service_utils.is_image_href_ordinary_file_name(image_iso): + if _is_image_href_ordinary_file_name(image_iso): image_iso_file = os.path.join(CONF.irmc.remote_image_share_root, image_iso) if not os.path.isfile(image_iso_file): @@ -166,7 +183,7 @@ def _parse_instance_info(node): if i_info.get('irmc_boot_iso'): deploy_info['irmc_boot_iso'] = i_info['irmc_boot_iso'] - if service_utils.is_image_href_ordinary_file_name( + if _is_image_href_ordinary_file_name( deploy_info['irmc_boot_iso']): boot_iso = os.path.join(CONF.irmc.remote_image_share_root, deploy_info['irmc_boot_iso']) @@ -227,7 +244,7 @@ def _setup_vmedia(task, mode, ramdisk_options): else: iso = task.node.driver_info['irmc_deploy_iso'] - if service_utils.is_image_href_ordinary_file_name(iso): + if _is_image_href_ordinary_file_name(iso): iso_file = iso else: iso_file = _get_iso_name(task.node, label=mode) @@ -266,7 +283,7 @@ def _prepare_boot_iso(task, root_uuid): # fetch boot iso if deploy_info.get('irmc_boot_iso'): boot_iso_href = deploy_info['irmc_boot_iso'] - if service_utils.is_image_href_ordinary_file_name(boot_iso_href): + if _is_image_href_ordinary_file_name(boot_iso_href): driver_internal_info['irmc_boot_iso'] = boot_iso_href else: boot_iso_filename = _get_iso_name(task.node, label='boot') diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index 782c4423b5..6d86867c6c 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -48,7 +48,6 @@ class NullWriter(object): class TestGlanceSerializer(testtools.TestCase): def test_serialize(self): metadata = {'name': 'image1', - 'is_public': True, 'foo': 'bar', 'properties': { 'prop1': 'propvalue1', @@ -62,7 +61,6 @@ class TestGlanceSerializer(testtools.TestCase): expected = { 'name': 'image1', - 'is_public': True, 'foo': 'bar', 'properties': {'prop1': 'propvalue1', 'mappings': [ @@ -95,7 +93,7 @@ class TestGlanceImageService(base.TestCase): self.context = context.RequestContext(auth_token=True) self.context.user_id = 'fake' self.context.project_id = 'fake' - self.service = service.GlanceImageService(self.client, 2, self.context) + self.service = service.GlanceImageService(self.client, self.context) self.config(glance_api_servers=['http://localhost'], group='glance') self.config(auth_strategy='keystone', group='glance') @@ -103,9 +101,7 @@ class TestGlanceImageService(base.TestCase): @staticmethod def _make_fixture(**kwargs): fixture = {'name': None, - 'properties': {}, - 'status': "active", - 'is_public': None} + 'status': "active"} fixture.update(kwargs) return stubs.FakeImage(fixture) @@ -128,25 +124,28 @@ class TestGlanceImageService(base.TestCase): def test_show_passes_through_to_client(self): image_id = uuidutils.generate_uuid() - image = self._make_fixture(name='image1', is_public=True, - id=image_id) + image = self._make_fixture(name='image1', id=image_id) expected = { + 'checksum': None, + 'container_format': None, + 'created_at': None, + 'deleted': None, + 'deleted_at': None, + 'disk_format': None, + 'file': None, 'id': image_id, - 'name': 'image1', - 'is_public': True, - 'size': None, 'min_disk': None, 'min_ram': None, - 'disk_format': None, - 'container_format': None, - 'checksum': None, - 'created_at': None, - 'updated_at': None, - 'deleted_at': None, - 'deleted': None, - 'status': "active", - 'properties': {}, + 'name': 'image1', 'owner': None, + 'properties': {}, + 'protected': None, + 'schema': None, + 'size': None, + 'status': "active", + 'tags': None, + 'updated_at': None, + 'visibility': None, } with mock.patch.object(self.service, 'call', return_value=image, autospec=True): @@ -172,8 +171,7 @@ class TestGlanceImageService(base.TestCase): def test_show_raises_when_image_not_active(self): image_id = uuidutils.generate_uuid() - image = self._make_fixture(name='image1', is_public=True, - id=image_id, status="queued") + image = self._make_fixture(name='image1', id=image_id, status="queued") with mock.patch.object(self.service, 'call', return_value=image, autospec=True): self.assertRaises(exception.ImageUnacceptable, @@ -196,7 +194,7 @@ class TestGlanceImageService(base.TestCase): stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' stub_context.project_id = 'fake' - stub_service = service.GlanceImageService(stub_client, 1, stub_context) + stub_service = service.GlanceImageService(stub_client, stub_context) image_id = uuidutils.generate_uuid() writer = NullWriter() @@ -243,8 +241,7 @@ class TestGlanceImageService(base.TestCase): stub_client = MyGlanceStubClient() stub_service = service.GlanceImageService(stub_client, - context=stub_context, - version=2) + context=stub_context) image_id = uuidutils.generate_uuid() self.config(allowed_direct_url_schemes=['file'], group='glance') @@ -279,7 +276,7 @@ class TestGlanceImageService(base.TestCase): stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' stub_context.project_id = 'fake' - stub_service = service.GlanceImageService(stub_client, 1, stub_context) + stub_service = service.GlanceImageService(stub_client, stub_context) image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotAuthorized, stub_service.download, @@ -295,7 +292,7 @@ class TestGlanceImageService(base.TestCase): stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' stub_context.project_id = 'fake' - stub_service = service.GlanceImageService(stub_client, 1, stub_context) + stub_service = service.GlanceImageService(stub_client, stub_context) image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotAuthorized, stub_service.download, @@ -311,7 +308,7 @@ class TestGlanceImageService(base.TestCase): stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' stub_context.project_id = 'fake' - stub_service = service.GlanceImageService(stub_client, 1, stub_context) + stub_service = service.GlanceImageService(stub_client, stub_context) image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotFound, stub_service.download, @@ -327,7 +324,7 @@ class TestGlanceImageService(base.TestCase): stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' stub_context.project_id = 'fake' - stub_service = service.GlanceImageService(stub_client, 1, stub_context) + stub_service = service.GlanceImageService(stub_client, stub_context) image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotFound, stub_service.download, @@ -346,7 +343,7 @@ class CheckImageServiceTestCase(base.TestCase): def setUp(self): super(CheckImageServiceTestCase, self).setUp() self.context = context.RequestContext(global_request_id='global') - self.service = service.GlanceImageService(None, 1, self.context) + self.service = service.GlanceImageService(None, self.context) # NOTE(pas-ha) register keystoneauth dynamic options manually plugin = kaloading.get_plugin_loader('password') opts = kaloading.get_auth_plugin_conf_options(plugin) @@ -381,7 +378,7 @@ class CheckImageServiceTestCase(base.TestCase): def _assert_client_call(self, mock_gclient, url, user=False): mock_gclient.assert_called_once_with( - 1, + 2, session=mock.sentinel.session, global_request_id='global', auth=mock.sentinel.sauth if user else mock.sentinel.auth, @@ -504,7 +501,7 @@ class TestGlanceSwiftTempURL(base.TestCase): client = stubs.StubGlanceClient() self.context = context.RequestContext() self.context.auth_token = 'fake' - self.service = service.GlanceImageService(client, 2, self.context) + self.service = service.GlanceImageService(client, self.context) self.config(swift_temp_url_key='correcthorsebatterystaple', group='glance') self.config(swift_endpoint_url='https://swift.example.com', @@ -769,7 +766,7 @@ class TestSwiftTempUrlCache(base.TestCase): group='glance') self.config(swift_store_multiple_containers_seed=0, group='glance') - self.glance_service = service.GlanceImageService(client, version=2, + self.glance_service = service.GlanceImageService(client, context=self.context) @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) @@ -1004,17 +1001,3 @@ class TestServiceUtils(base.TestCase): self.assertFalse(service_utils.is_glance_image(image_href)) image_href = None self.assertFalse(service_utils.is_glance_image(image_href)) - - def test_is_image_href_ordinary_file_name_true(self): - image = u"\u0111eploy.iso" - result = service_utils.is_image_href_ordinary_file_name(image) - self.assertTrue(result) - - def test_is_image_href_ordinary_file_name_false(self): - for image in ('733d1c44-a2ea-414b-aca7-69decf20d810', - u'glance://\u0111eploy_iso', - u'http://\u0111eploy_iso', - u'https://\u0111eploy_iso', - u'file://\u0111eploy_iso',): - result = service_utils.is_image_href_ordinary_file_name(image) - self.assertFalse(result) diff --git a/ironic/tests/unit/common/test_image_service.py b/ironic/tests/unit/common/test_image_service.py index c3a61432b7..91d46159df 100644 --- a/ironic/tests/unit/common/test_image_service.py +++ b/ironic/tests/unit/common/test_image_service.py @@ -24,7 +24,6 @@ import six.moves.builtins as __builtin__ from six.moves import http_client from ironic.common import exception -from ironic.common.glance_service.v1 import image_service as glance_v1_service from ironic.common.glance_service.v2 import image_service as glance_v2_service from ironic.common import image_service from ironic.tests import base @@ -271,16 +270,7 @@ class ServiceGetterTestCase(base.TestCase): def test_get_glance_image_service(self, glance_service_mock): image_href = uuidutils.generate_uuid() image_service.get_image_service(image_href, context=self.context) - glance_service_mock.assert_called_once_with(mock.ANY, None, 2, - self.context) - - @mock.patch.object(glance_v1_service.GlanceImageService, '__init__', - return_value=None, autospec=True) - def test_get_glance_image_service_default_v1(self, glance_service_mock): - self.config(glance_api_version=1, group='glance') - image_href = uuidutils.generate_uuid() - image_service.get_image_service(image_href, context=self.context) - glance_service_mock.assert_called_once_with(mock.ANY, None, 1, + glance_service_mock.assert_called_once_with(mock.ANY, None, self.context) @mock.patch.object(glance_v2_service.GlanceImageService, '__init__', @@ -288,7 +278,7 @@ class ServiceGetterTestCase(base.TestCase): def test_get_glance_image_service_url(self, glance_service_mock): image_href = 'glance://%s' % uuidutils.generate_uuid() image_service.get_image_service(image_href, context=self.context) - glance_service_mock.assert_called_once_with(mock.ANY, None, 2, + glance_service_mock.assert_called_once_with(mock.ANY, None, self.context) @mock.patch.object(image_service.HttpImageService, '__init__', diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py index e1937a3c2d..dfd1af84c2 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py @@ -40,6 +40,7 @@ from ironic.drivers.modules.irmc import boot as irmc_boot from ironic.drivers.modules.irmc import common as irmc_common from ironic.drivers.modules.irmc import management as irmc_management from ironic.drivers.modules import pxe +from ironic.tests import base from ironic.tests.unit.db import utils as db_utils from ironic.tests.unit.drivers.modules.irmc import test_common from ironic.tests.unit.drivers.modules import test_pxe @@ -117,7 +118,7 @@ class IRMCDeployPrivateMethodsTestCase(test_common.BaseIRMCTest): '/remote_image_share_root/deploy.iso') self.assertEqual(driver_info_expected, driver_info_actual) - @mock.patch.object(service_utils, 'is_image_href_ordinary_file_name', + @mock.patch.object(irmc_boot, '_is_image_href_ordinary_file_name', spec_set=True, autospec=True) def test__parse_driver_info_not_in_share( self, is_image_href_ordinary_file_name_mock): @@ -427,7 +428,7 @@ class IRMCDeployPrivateMethodsTestCase(test_common.BaseIRMCTest): autospec=True) @mock.patch.object(irmc_boot, '_parse_deploy_info', spec_set=True, autospec=True) - @mock.patch.object(service_utils, 'is_image_href_ordinary_file_name', + @mock.patch.object(irmc_boot, '_is_image_href_ordinary_file_name', spec_set=True, autospec=True) def test__prepare_boot_iso_fetch_ok(self, is_image_href_ordinary_file_name_mock, @@ -1893,3 +1894,20 @@ class IRMCPXEBootBasicTestCase(test_pxe.PXEBootTestCase): properties = task.driver.get_properties() for p in pxe.COMMON_PROPERTIES: self.assertIn(p, properties) + + +class IsImageHrefOrdinaryFileNameTestCase(base.TestCase): + + def test_is_image_href_ordinary_file_name_true(self): + image = u"\u0111eploy.iso" + result = irmc_boot._is_image_href_ordinary_file_name(image) + self.assertTrue(result) + + def test_is_image_href_ordinary_file_name_false(self): + for image in ('733d1c44-a2ea-414b-aca7-69decf20d810', + u'glance://\u0111eploy_iso', + u'http://\u0111eploy_iso', + u'https://\u0111eploy_iso', + u'file://\u0111eploy_iso',): + result = irmc_boot._is_image_href_ordinary_file_name(image) + self.assertFalse(result) diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index fa345cf03b..08da1a06b8 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -2257,8 +2257,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase): utils.build_instance_info_for_deploy(task) - glance_mock.assert_called_once_with(version=2, - context=task.context) + glance_mock.assert_called_once_with(context=task.context) glance_mock.return_value.show.assert_called_once_with( self.node.instance_info['image_source']) glance_mock.return_value.swift_temp_url.assert_called_once_with( @@ -2318,8 +2317,7 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase): info = utils.build_instance_info_for_deploy(task) - glance_mock.assert_called_once_with(version=2, - context=task.context) + glance_mock.assert_called_once_with(context=task.context) glance_mock.return_value.show.assert_called_once_with( self.node.instance_info['image_source']) glance_mock.return_value.swift_temp_url.assert_called_once_with( @@ -2474,8 +2472,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): instance_info = utils.build_instance_info_for_deploy(task) - glance_mock.assert_called_once_with(version=2, - context=task.context) + glance_mock.assert_called_once_with(context=task.context) glance_mock.return_value.show.assert_called_once_with( self.node.instance_info['image_source']) self.cache_image_mock.assert_called_once_with(task.context, @@ -2511,8 +2508,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): instance_info = utils.build_instance_info_for_deploy(task) - glance_mock.assert_called_once_with(version=2, - context=task.context) + glance_mock.assert_called_once_with(context=task.context) glance_mock.return_value.show.assert_called_once_with( self.node.instance_info['image_source']) self.cache_image_mock.assert_called_once_with(task.context, diff --git a/ironic/tests/unit/stubs.py b/ironic/tests/unit/stubs.py index ba66e15f22..58ef51f96f 100644 --- a/ironic/tests/unit/stubs.py +++ b/ironic/tests/unit/stubs.py @@ -51,27 +51,27 @@ class StubGlanceClient(object): return _GlanceWrapper(self.fake_wrapped) -class FakeImage(object): +class FakeImage(dict): def __init__(self, metadata): IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner', 'container_format', 'checksum', 'id', - 'name', - 'deleted', 'status', - 'min_disk', 'min_ram', 'is_public'] + 'name', 'deleted', 'status', + 'min_disk', 'min_ram', 'tags', 'visibility', + 'protected', 'file', 'schema'] raw = dict.fromkeys(IMAGE_ATTRIBUTES) raw.update(metadata) # raw['created_at'] = NOW_GLANCE_FORMAT # raw['updated_at'] = NOW_GLANCE_FORMAT - self.__dict__['raw'] = raw + super(FakeImage, self).__init__(raw) def __getattr__(self, key): try: - return self.__dict__['raw'][key] + return self[key] except KeyError: raise AttributeError(key) def __setattr__(self, key, value): - try: - self.__dict__['raw'][key] = value - except KeyError: + if key in self: + self[key] = value + else: raise AttributeError(key) diff --git a/releasenotes/notes/no-glance-v1-d249e8079f46f40c.yaml b/releasenotes/notes/no-glance-v1-d249e8079f46f40c.yaml new file mode 100644 index 0000000000..1164ae3e5b --- /dev/null +++ b/releasenotes/notes/no-glance-v1-d249e8079f46f40c.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + Support for using the Image API v1 was removed. It was removed from Glance + in the Rocky release. + - | + The deprecated option ``[glance]glance_api_version`` was removed. Only v2 + is now used.