From 62c43e76262a80904a564a71eee256725cca9fc5 Mon Sep 17 00:00:00 2001 From: Rabi Mishra Date: Tue, 31 Mar 2020 17:52:04 +0530 Subject: [PATCH] Fix GlanceClientAdapter to use new osc release It seems osc has started using openstacksdk[1] and it probably breaks how you find image with name. [1] https://review.opendev.org/#/c/650374/ Change-Id: Ia15a065c908aa9992747f5286f2404a0947f6720 Closes-Bug: #1869736 --- .../overcloud_image/test_overcloud_image.py | 110 ++++++++++-------- tripleoclient/v1/overcloud_image.py | 32 ++--- 2 files changed, 72 insertions(+), 70 deletions(-) diff --git a/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py b/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py index 86290f8dd..11f4c9905 100644 --- a/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py +++ b/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py @@ -414,17 +414,11 @@ class TestGlanceClientAdapter(TestPluginV1): updated=self.updated ) - @mock.patch('osc_lib.utils.find_resource') - def test_get_image_exists(self, mock_find_resource): + def test_get_image_exists(self): image_mock = mock.Mock(name='imagename') - mock_find_resource.return_value = image_mock + self.app.client_manager.image.find_image.return_value = image_mock self.assertEqual(self.adapter._get_image('imagename'), image_mock) - @mock.patch('osc_lib.utils.find_resource') - def test_get_image_none(self, mock_find_resource): - mock_find_resource.side_effect = exceptions.CommandError('') - self.assertIsNone(self.adapter._get_image('noimagename')) - @mock.patch('tripleoclient.v1.overcloud_image.' 'GlanceClientAdapter._get_image', autospec=True) def test_image_try_update_no_exist(self, mock_get_image): @@ -448,7 +442,7 @@ class TestGlanceClientAdapter(TestPluginV1): image_mock ) self.assertEqual([], self.updated) - self.app.client_manager.image.images.update.assert_not_called() + self.app.client_manager.image.update_image.assert_not_called() @mock.patch('tripleoclient.v1.overcloud_image.' 'GlanceClientAdapter._image_changed', autospec=True) @@ -460,7 +454,7 @@ class TestGlanceClientAdapter(TestPluginV1): image_mock = mock.Mock(name='imagename', created_at='2015-07-31T14:37:22.000000') update_mock = mock.Mock(return_value=image_mock) - self.app.client_manager.image.images.update = update_mock + self.app.client_manager.image.update_image = update_mock mock_get_image.return_value = image_mock mock_image_changed.return_value = True self.adapter.update_existing = True @@ -482,7 +476,7 @@ class TestUploadOvercloudImage(TestPluginV1): self.app.client_manager.image = mock.Mock() self.app.client_manager.image.version = 2.0 self._arch = tripleo_common.arch.kernel_arch() - self.app.client_manager.image.images.create.return_value = ( + self.app.client_manager.image.create_image.return_value = ( mock.Mock(id=10, name='imgname', properties={'kernel_id': 10, 'ramdisk_id': 10, 'hw_architecture': self._arch}, @@ -546,20 +540,26 @@ class TestUploadOvercloudImage(TestPluginV1): ) self.assertEqual( 3, - self.app.client_manager.image.images.create.call_count + self.app.client_manager.image.create_image.call_count ) - self.app.client_manager.image.images.create.assert_has_calls([ + self.app.client_manager.image.create_image.assert_has_calls([ mock.call(name='overcloud-full-vmlinuz', disk_format='aki', container_format='bare', + data=mock.ANY, + validate_checksum=False, visibility='public'), mock.call(name='overcloud-full-initrd', disk_format='ari', container_format='bare', + data=mock.ANY, + validate_checksum=False, visibility='public'), mock.call(name='overcloud-full', disk_format='qcow2', container_format='bare', + data=mock.ANY, + validate_checksum=False, visibility='public'), ]) @@ -637,15 +637,15 @@ class TestUploadOvercloudImage(TestPluginV1): self.assertEqual( 0, - self.app.client_manager.image.images.delete.call_count + self.app.client_manager.image.delete_image.call_count ) self.assertEqual( 0, - self.app.client_manager.image.images.create.call_count + self.app.client_manager.image.create_image.call_count ) self.assertEqual( 0, - self.app.client_manager.image.images.update.call_count + self.app.client_manager.image.image_update.call_count ) self.assertEqual(mock_subprocess_call.call_count, 0) @@ -672,22 +672,22 @@ class TestUploadOvercloudImage(TestPluginV1): created_at='2015-07-31T14:37:22.000000') mock_get_image.return_value = existing_image mock_image_changed.return_value = True - self.app.client_manager.image.images.update.return_value = ( + self.app.client_manager.image.image_update.return_value = ( existing_image) self.cmd.take_action(parsed_args) self.assertEqual( 0, - self.app.client_manager.image.images.delete.call_count + self.app.client_manager.image.delete_image.call_count ) self.assertEqual( 3, - self.app.client_manager.image.images.create.call_count + self.app.client_manager.image.create_image.call_count ) self.assertEqual( 6, # 3 for new uploads, 3 updating the existsing - self.app.client_manager.image.images.update.call_count + self.app.client_manager.image.update_image.call_count ) self.assertEqual(mock_subprocess_call.call_count, 2) self.assertTrue(self.cmd.updated) @@ -703,7 +703,7 @@ class TestUploadOvercloudImageFull(TestPluginV1): self.app.client_manager.image = mock.Mock() self.app.client_manager.image.version = 2.0 self._arch = tripleo_common.arch.kernel_arch() - self.app.client_manager.image.images.create.return_value = ( + self.app.client_manager.image.create_image.return_value = ( mock.Mock(id=10, name='imgname', properties={'hw_architecture': self._arch}, created_at='2015-07-31T14:37:22.000000')) @@ -739,21 +739,23 @@ class TestUploadOvercloudImageFull(TestPluginV1): self.assertEqual( 0, - self.app.client_manager.image.images.delete.call_count + self.app.client_manager.image.delete_image.call_count ) self.assertEqual( 1, - self.app.client_manager.image.images.create.call_count + self.app.client_manager.image.create_image.call_count ) - self.app.client_manager.image.images.create.assert_has_calls([ + self.app.client_manager.image.create_image.assert_has_calls([ mock.call(name='overcloud-full', disk_format='qcow2', container_format='bare', - visibility='public'), + data=mock.ANY, + validate_checksum=False, + visibility='public',), ]) # properties are set by updating the image - self.app.client_manager.image.images.update.assert_has_calls([ + self.app.client_manager.image.update_image.assert_has_calls([ mock.call(mock.ANY, hw_architecture=self._arch), ]) @@ -784,21 +786,23 @@ class TestUploadOvercloudImageFull(TestPluginV1): self.assertEqual( 0, - self.app.client_manager.image.images.delete.call_count + self.app.client_manager.image.delete_image.call_count ) self.assertEqual( 1, - self.app.client_manager.image.images.create.call_count + self.app.client_manager.image.create_image.call_count ) - self.app.client_manager.image.images.create.assert_has_calls([ + self.app.client_manager.image.create_image.assert_has_calls([ mock.call(name='ppc64le-overcloud-full', disk_format='qcow2', container_format='bare', + data=mock.ANY, + validate_checksum=False, visibility='public'), ]) - self.app.client_manager.image.images.update.assert_has_calls([ + self.app.client_manager.image.update_image.assert_has_calls([ mock.call(mock.ANY, hw_architecture='ppc64le'), ]) self.assertEqual(mock_subprocess_call.call_count, 2) @@ -836,15 +840,15 @@ class TestUploadOvercloudImageFull(TestPluginV1): self.assertEqual( 0, - self.app.client_manager.image.images.delete.call_count + self.app.client_manager.image.delete_image.call_count ) self.assertEqual( 0, - self.app.client_manager.image.images.create.call_count + self.app.client_manager.image.create_image.call_count ) self.assertEqual( 0, - self.app.client_manager.image.images.update.call_count + self.app.client_manager.image.update_image.call_count ) self.assertEqual(mock_subprocess_call.call_count, 0) @@ -869,22 +873,22 @@ class TestUploadOvercloudImageFull(TestPluginV1): created_at='2015-07-31T14:37:22.000000') mock_get_image.return_value = existing_image mock_image_changed.return_value = True - self.app.client_manager.image.images.update.return_value = ( + self.app.client_manager.image.update_image.return_value = ( existing_image) self.cmd.take_action(parsed_args) self.assertEqual( 0, - self.app.client_manager.image.images.delete.call_count + self.app.client_manager.image.delete_image.call_count ) self.assertEqual( 1, - self.app.client_manager.image.images.create.call_count + self.app.client_manager.image.create_image.call_count ) self.assertEqual( 2, # update 1 image *and* add properties to 1 image - self.app.client_manager.image.images.update.call_count + self.app.client_manager.image.update_image.call_count ) self.assertEqual(mock_subprocess_call.call_count, 2) @@ -909,8 +913,8 @@ class TestUploadOvercloudImageFullMultiArch(TestPluginV1): # GlanceClientAdapter._upload_image() calls # self.client.images.create() and self.client.images.get() once each # call so this way we always create() and get() the same mocked "image" - self.app.client_manager.image.images.create.side_effect = self.images - self.app.client_manager.image.images.get.side_effect = self.images + self.app.client_manager.image.create_image.side_effect = self.images + self.app.client_manager.image.get_image.side_effect = self.images mock_cfe = mock.patch('tripleoclient.v1.overcloud_image.' 'BaseClientAdapter.check_file_exists', @@ -952,25 +956,29 @@ class TestUploadOvercloudImageFullMultiArch(TestPluginV1): self.assertEqual( 0, - self.app.client_manager.image.images.delete.call_count + self.app.client_manager.image.delete_image.call_count ) self.assertEqual( 2, - self.app.client_manager.image.images.create.call_count + self.app.client_manager.image.create_image.call_count ) - self.app.client_manager.image.images.create.assert_has_calls([ + self.app.client_manager.image.create_image.assert_has_calls([ mock.call(name='overcloud-full', disk_format='qcow2', container_format='bare', + data=mock.ANY, + validate_checksum=False, visibility='public'), mock.call(name='ppc64le-overcloud-full', disk_format='qcow2', container_format='bare', + data=mock.ANY, + validate_checksum=False, visibility='public'), ]) - self.app.client_manager.image.images.update.assert_has_calls([ + self.app.client_manager.image.update_image.assert_has_calls([ mock.call(10, hw_architecture='x86_64'), mock.call(11, hw_architecture='ppc64le'), ]) @@ -1020,29 +1028,35 @@ class TestUploadOvercloudImageFullMultiArch(TestPluginV1): self.assertEqual( 0, - self.app.client_manager.image.images.delete.call_count + self.app.client_manager.image.delete_image.call_count ) self.assertEqual( 3, - self.app.client_manager.image.images.create.call_count + self.app.client_manager.image.create_image.call_count ) - self.app.client_manager.image.images.create.assert_has_calls([ + self.app.client_manager.image.create_image.assert_has_calls([ mock.call(name='overcloud-full', disk_format='qcow2', container_format='bare', + data=mock.ANY, + validate_checksum=False, visibility='public'), mock.call(name='ppc64le-overcloud-full', disk_format='qcow2', container_format='bare', + data=mock.ANY, + validate_checksum=False, visibility='public'), mock.call(name='p9-ppc64le-overcloud-full', disk_format='qcow2', container_format='bare', + data=mock.ANY, + validate_checksum=False, visibility='public'), ]) - self.app.client_manager.image.images.update.assert_has_calls([ + self.app.client_manager.image.update_image.assert_has_calls([ mock.call(10, hw_architecture='x86_64'), mock.call(11, hw_architecture='ppc64le'), mock.call(12, hw_architecture='ppc64le', tripleo_platform='p9'), @@ -1073,7 +1087,7 @@ class TestUploadOnlyExisting(TestPluginV1): self.cmd = overcloud_image.UploadOvercloudImage(self.app, None) self.app.client_manager.image = mock.Mock() self.app.client_manager.image.version = 2.0 - self.app.client_manager.image.images.create.return_value = ( + self.app.client_manager.image.create_image.return_value = ( mock.Mock(id=10, name='imgname', properties={}, created_at='2015-07-31T14:37:22.000000')) mock_cfe = mock.patch('tripleoclient.v1.overcloud_image.' diff --git a/tripleoclient/v1/overcloud_image.py b/tripleoclient/v1/overcloud_image.py index df696b1b8..cb8ce52d9 100644 --- a/tripleoclient/v1/overcloud_image.py +++ b/tripleoclient/v1/overcloud_image.py @@ -27,7 +27,6 @@ import sys from glanceclient.common.progressbar import VerboseFileWrapper from osc_lib import exceptions from osc_lib.i18n import _ -from osc_lib import utils from prettytable import PrettyTable import tripleo_common.arch from tripleo_common.image import build @@ -290,21 +289,9 @@ class GlanceClientAdapter(BaseClientAdapter): print(table, file=sys.stdout) def _get_image(self, name): - try: - image = utils.find_resource(self.client.images, name) - except exceptions.CommandError as e: - # TODO(maufart): enhance error detection, when python-glanceclient - # starts provide it https://bugs.launchpad.net/glance/+bug/1480156 - if 'More than one image exists' in e.args[0]: - raise exceptions.CommandError( - 'Image "%s" already exists in glance more than once,' - ' delete all copies except the first one.' % name - ) - else: - self.log.debug('Image "%s" does not exists, no problem.' - % name) - return None - return image + # This would return None by default for an non-existent resorurce + # And DuplicateResource exception if there more than one. + return self.client.find_image(name) def _image_changed(self, image, filename): return image.checksum != plugin_utils.file_checksum(filename) @@ -314,7 +301,7 @@ class GlanceClientAdapter(BaseClientAdapter): if image: if self._image_changed(image, image_file): if self.update_existing: - self.client.images.update( + self.client.update_image( image.id, name='%s_%s' % (image.name, re.sub(r'[\-:\.]|(0+$)', '', @@ -336,18 +323,19 @@ class GlanceClientAdapter(BaseClientAdapter): def _upload_image(self, name, data, properties=None, visibility='public', disk_format='qcow2', container_format='bare'): - image = self.client.images.create( + image = self.client.create_image( name=name, visibility=visibility, disk_format=disk_format, - container_format=container_format + container_format=container_format, + data=data, + validate_checksum=False ) - self.client.images.upload(image.id, image_data=data) if properties: - self.client.images.update(image.id, **properties) + self.client.update_image(image.id, **properties) # Refresh image info - image = self.client.images.get(image.id) + image = self.client.get_image(image.id) print('Image "%s" was uploaded.' % image.name, file=sys.stdout) self._print_image_info(image)