From 766297d010f5631c241e770b94ddff07b23538de Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Fri, 9 Dec 2016 11:28:08 -0600 Subject: [PATCH] Honor image_api_version when doing version discovery We want to grab the latest version available - unless the user has configured themselves to request a previous version. Enter Cloud Suite is an example of a cloud that has v2 but v1 must be used for uploads. Change-Id: I1603fd567a59b5c123ab2f84c7a9571381245e97 --- shade/openstackcloud.py | 36 ++++++++- shade/tests/unit/base.py | 10 ++- .../tests/unit/fixtures/image-version-v1.json | 24 ++++++ .../tests/unit/fixtures/image-version-v2.json | 44 +++++++++++ shade/tests/unit/test_image.py | 74 +++++++++++++++++++ 5 files changed, 181 insertions(+), 7 deletions(-) create mode 100644 shade/tests/unit/fixtures/image-version-v1.json create mode 100644 shade/tests/unit/fixtures/image-version-v2.json diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index 2412b2825..0c167da36 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -394,14 +394,42 @@ class OpenStackCloud(_normalize.Normalizer): @property def _image_client(self): if 'image' not in self._raw_clients: + # Get configured api version for downgrades + config_version = self.cloud_config.get_api_version('image') image_client = self._get_raw_client('image') try: # Version discovery versions = image_client.get('/') - current_version = [ - version for version in versions - if version['status'] == 'CURRENT'][0] - image_url = current_version['links'][0]['href'] + api_version = None + if config_version.startswith('1'): + api_version = [ + version for version in versions + if version['id'] in ('v1.0', 'v1.1')] + if api_version: + api_version = api_version[0] + if not api_version: + api_version = [ + version for version in versions + if version['status'] == 'CURRENT'][0] + + image_url = api_version['links'][0]['href'] + # If we detect a different version that was configured, + # set the version in occ because we have logic elsewhere + # that is different depending on which version we're using + warning_msg = None + if (config_version.startswith('2') + and api_version['id'].startswith('v1')): + self.cloud_config.config['image_api_version'] = '1' + warning_msg = ( + 'image_api_version is 2 but only 1 is available.') + elif (config_version.startswith('1') + and api_version['id'].startswith('v2')): + self.cloud_config.config['image_api_version'] = '2' + warning_msg = ( + 'image_api_version is 1 but only 2 is available.') + if warning_msg: + self.log.debug(warning_msg) + warnings.warn(warning_msg) except (keystoneauth1.exceptions.connection.ConnectFailure, OpenStackCloudURINotFound) as e: # A 404 or a connection error is a likely thing to get diff --git a/shade/tests/unit/base.py b/shade/tests/unit/base.py index 7a585f4a1..8d1ea025d 100644 --- a/shade/tests/unit/base.py +++ b/shade/tests/unit/base.py @@ -108,7 +108,11 @@ class TestCase(BaseTestCase): class RequestsMockTestCase(BaseTestCase): - def setUp(self, cloud_config_fixture='clouds.yaml'): + def setUp( + self, + cloud_config_fixture='clouds.yaml', + discovery_json='discovery.json', + image_version_json='image-version.json'): super(RequestsMockTestCase, self).setUp( cloud_config_fixture=cloud_config_fixture) @@ -119,7 +123,7 @@ class RequestsMockTestCase(BaseTestCase): text=open( os.path.join( self.fixtures_directory, - 'discovery.json'), + discovery_json), 'r').read()) self.adapter.post( 'http://example.com/v2.0/tokens', @@ -133,7 +137,7 @@ class RequestsMockTestCase(BaseTestCase): text=open( os.path.join( self.fixtures_directory, - 'image-version.json'), + image_version_json), 'r').read()) test_cloud = os.environ.get('SHADE_OS_CLOUD', '_test_cloud_') diff --git a/shade/tests/unit/fixtures/image-version-v1.json b/shade/tests/unit/fixtures/image-version-v1.json new file mode 100644 index 000000000..874cbbaa0 --- /dev/null +++ b/shade/tests/unit/fixtures/image-version-v1.json @@ -0,0 +1,24 @@ +{ + "versions": [ + { + "status": "CURRENT", + "id": "v1.1", + "links": [ + { + "href": "https://image.example.com/v1/", + "rel": "self" + } + ] + }, + { + "status": "SUPPORTED", + "id": "v1.0", + "links": [ + { + "href": "https://image.example.com/v1/", + "rel": "self" + } + ] + } + ] +} diff --git a/shade/tests/unit/fixtures/image-version-v2.json b/shade/tests/unit/fixtures/image-version-v2.json new file mode 100644 index 000000000..15874b87c --- /dev/null +++ b/shade/tests/unit/fixtures/image-version-v2.json @@ -0,0 +1,44 @@ +{ + "versions": [ + { + "status": "CURRENT", + "id": "v2.3", + "links": [ + { + "href": "https://image.example.com/v2/", + "rel": "self" + } + ] + }, + { + "status": "SUPPORTED", + "id": "v2.2", + "links": [ + { + "href": "https://image.example.com/v2/", + "rel": "self" + } + ] + }, + { + "status": "SUPPORTED", + "id": "v2.1", + "links": [ + { + "href": "https://image.example.com/v2/", + "rel": "self" + } + ] + }, + { + "status": "SUPPORTED", + "id": "v2.0", + "links": [ + { + "href": "https://image.example.com/v2/", + "rel": "self" + } + ] + } + ] +} diff --git a/shade/tests/unit/test_image.py b/shade/tests/unit/test_image.py index 4e62367b9..4a493be10 100644 --- a/shade/tests/unit/test_image.py +++ b/shade/tests/unit/test_image.py @@ -69,6 +69,26 @@ class TestImage(base.RequestsMockTestCase): self.fake_search_return = {'images': [self.fake_image_dict]} self.output = uuid.uuid4().bytes + def test_config_v1(self): + self.cloud.cloud_config.config['image_api_version'] = '1' + # We override the scheme of the endpoint with the scheme of the service + # because glance has a bug where it doesn't return https properly. + self.assertEqual( + 'http://image.example.com/v1/', + self.cloud._image_client.get_endpoint()) + self.assertEqual( + '1', self.cloud_config.get_api_version('image')) + + def test_config_v2(self): + self.cloud.cloud_config.config['image_api_version'] = '2' + # We override the scheme of the endpoint with the scheme of the service + # because glance has a bug where it doesn't return https properly. + self.assertEqual( + 'http://image.example.com/v2/', + self.cloud._image_client.get_endpoint()) + self.assertEqual( + '2', self.cloud_config.get_api_version('image')) + def test_download_image_no_output(self): self.assertRaises(exc.OpenStackCloudException, self.cloud.download_image, 'fake_image') @@ -574,3 +594,57 @@ class TestMockImage(base.TestCase): mock_image_client.get.assert_called_with('/images') self.assertEqual( self._munch_images(ret), self.cloud.list_images()) + + +class TestImageV1Only(base.RequestsMockTestCase): + + def setUp(self): + super(TestImageV1Only, self).setUp( + image_version_json='image-version-v1.json') + + def test_config_v1(self): + self.cloud.cloud_config.config['image_api_version'] = '1' + # We override the scheme of the endpoint with the scheme of the service + # because glance has a bug where it doesn't return https properly. + self.assertEqual( + 'http://image.example.com/v1/', + self.cloud._image_client.get_endpoint()) + self.assertEqual( + '1', self.cloud_config.get_api_version('image')) + + def test_config_v2(self): + self.cloud.cloud_config.config['image_api_version'] = '2' + # We override the scheme of the endpoint with the scheme of the service + # because glance has a bug where it doesn't return https properly. + self.assertEqual( + 'http://image.example.com/v1/', + self.cloud._image_client.get_endpoint()) + self.assertEqual( + '1', self.cloud_config.get_api_version('image')) + + +class TestImageV2Only(base.RequestsMockTestCase): + + def setUp(self): + super(TestImageV2Only, self).setUp( + image_version_json='image-version-v2.json') + + def test_config_v1(self): + self.cloud.cloud_config.config['image_api_version'] = '1' + # We override the scheme of the endpoint with the scheme of the service + # because glance has a bug where it doesn't return https properly. + self.assertEqual( + 'http://image.example.com/v2/', + self.cloud._image_client.get_endpoint()) + self.assertEqual( + '2', self.cloud_config.get_api_version('image')) + + def test_config_v2(self): + self.cloud.cloud_config.config['image_api_version'] = '2' + # We override the scheme of the endpoint with the scheme of the service + # because glance has a bug where it doesn't return https properly. + self.assertEqual( + 'http://image.example.com/v2/', + self.cloud._image_client.get_endpoint()) + self.assertEqual( + '2', self.cloud_config.get_api_version('image'))