From 46dd4dd60fa31aff71f3ba94cf496aa6fea0d582 Mon Sep 17 00:00:00 2001 From: Brian Rosmaita Date: Sun, 25 Mar 2018 14:47:27 -0400 Subject: [PATCH] Make image-import fail faster Add checks to image-import command so that it provides better user feedback in failure situations. Change-Id: I8b6b32c3d1d1a745aa68ff8dc629419dff9bb130 Closes-bug: #1758718 --- glanceclient/tests/unit/v2/test_shell_v2.py | 204 +++++++++++++++++++- glanceclient/v2/shell.py | 54 +++++- 2 files changed, 240 insertions(+), 18 deletions(-) diff --git a/glanceclient/tests/unit/v2/test_shell_v2.py b/glanceclient/tests/unit/v2/test_shell_v2.py index e07d0738..f2f30e01 100644 --- a/glanceclient/tests/unit/v2/test_shell_v2.py +++ b/glanceclient/tests/unit/v2/test_shell_v2.py @@ -959,6 +959,7 @@ class ShellV2Test(testtools.TestCase): expect_image['name'] = 'IMG-01' expect_image['disk_format'] = 'vhd' expect_image['container_format'] = 'bare' + expect_image['status'] = 'queued' mocked_create.return_value = expect_image mocked_get.return_value = expect_image mocked_info.return_value = self.import_info_response @@ -971,7 +972,7 @@ class ShellV2Test(testtools.TestCase): mocked_get.assert_called_with('pass') utils.print_dict.assert_called_with({ 'id': 'pass', 'name': 'IMG-01', 'disk_format': 'vhd', - 'container_format': 'bare'}) + 'container_format': 'bare', 'status': 'queued'}) def test_do_image_update_no_user_props(self): args = self._make_args({'id': 'pass', 'name': 'IMG-01', @@ -1103,16 +1104,203 @@ class ShellV2Test(testtools.TestCase): test_shell.do_image_upload(self.gc, args) mocked_upload.assert_called_once_with('IMG-01', 'testfile', 1024) - def test_image_import(self): + @mock.patch('glanceclient.common.utils.exit') + def test_neg_image_import_not_available(self, mock_utils_exit): + expected_msg = 'Target Glance does not support Image Import workflow' + mock_utils_exit.side_effect = self._mock_utils_exit + args = self._make_args( + {'id': 'IMG-01', 'import_method': 'smarty-pants', 'uri': None}) + with mock.patch.object(self.gc.images, 'import') as mocked_import: + with mock.patch.object(self.gc.images, + 'get_import_info') as mocked_info: + mocked_info.side_effect = exc.HTTPNotFound + try: + test_shell.do_image_import(self.gc, args) + self.fail("utils.exit should have been called") + except SystemExit: + pass + mock_utils_exit.assert_called_once_with(expected_msg) + mocked_import.assert_not_called() + + @mock.patch('glanceclient.common.utils.exit') + def test_neg_image_import_bad_method(self, mock_utils_exit): + expected_msg = ('Import method \'smarty-pants\' is not valid for this ' + 'cloud. Valid values can be retrieved with ' + 'import-info command.') + mock_utils_exit.side_effect = self._mock_utils_exit + args = self._make_args( + {'id': 'IMG-01', 'import_method': 'smarty-pants', 'uri': None}) + with mock.patch.object(self.gc.images, + 'get_import_info') as mocked_info: + mocked_info.return_value = self.import_info_response + try: + test_shell.do_image_import(self.gc, args) + self.fail("utils.exit should have been called") + except SystemExit: + pass + mock_utils_exit.assert_called_once_with(expected_msg) + + @mock.patch('glanceclient.common.utils.exit') + def test_neg_image_import_no_methods_configured(self, mock_utils_exit): + expected_msg = ('Import method \'glance-direct\' is not valid for ' + 'this cloud. Valid values can be retrieved with ' + 'import-info command.') + mock_utils_exit.side_effect = self._mock_utils_exit + args = self._make_args( + {'id': 'IMG-01', 'import_method': 'glance-direct', 'uri': None}) + with mock.patch.object(self.gc.images, + 'get_import_info') as mocked_info: + mocked_info.return_value = {"import-methods": {"value": []}} + try: + test_shell.do_image_import(self.gc, args) + self.fail("utils.exit should have been called") + except SystemExit: + pass + mock_utils_exit.assert_called_once_with(expected_msg) + + @mock.patch('glanceclient.common.utils.exit') + def test_neg_image_import_glance_direct_image_not_uploading_status( + self, mock_utils_exit): + expected_msg = ('The \'glance-direct\' import method can only be ' + 'applied to an image in status \'uploading\'') + mock_utils_exit.side_effect = self._mock_utils_exit + args = self._make_args( + {'id': 'IMG-01', 'import_method': 'glance-direct', 'uri': None}) + with mock.patch.object(self.gc.images, + 'get_import_info') as mocked_info: + with mock.patch.object(self.gc.images, 'get') as mocked_get: + mocked_get.return_value = {'status': 'queued', + 'container_format': 'bare', + 'disk_format': 'raw'} + mocked_info.return_value = self.import_info_response + try: + test_shell.do_image_import(self.gc, args) + self.fail("utils.exit should have been called") + except SystemExit: + pass + mock_utils_exit.assert_called_once_with(expected_msg) + + @mock.patch('glanceclient.common.utils.exit') + def test_neg_image_import_web_download_image_not_queued_status( + self, mock_utils_exit): + expected_msg = ('The \'web-download\' import method can only be ' + 'applied to an image in status \'queued\'') + mock_utils_exit.side_effect = self._mock_utils_exit + args = self._make_args( + {'id': 'IMG-01', 'import_method': 'web-download', + 'uri': 'http://joes-image-shack.com/funky.qcow2'}) + with mock.patch.object(self.gc.images, + 'get_import_info') as mocked_info: + with mock.patch.object(self.gc.images, 'get') as mocked_get: + mocked_get.return_value = {'status': 'uploading', + 'container_format': 'bare', + 'disk_format': 'raw'} + mocked_info.return_value = self.import_info_response + try: + test_shell.do_image_import(self.gc, args) + self.fail("utils.exit should have been called") + except SystemExit: + pass + mock_utils_exit.assert_called_once_with(expected_msg) + + @mock.patch('glanceclient.common.utils.exit') + def test_neg_image_import_image_no_container_format( + self, mock_utils_exit): + expected_msg = ('The \'container_format\' and \'disk_format\' ' + 'properties must be set on an image before it can be ' + 'imported.') + mock_utils_exit.side_effect = self._mock_utils_exit + args = self._make_args( + {'id': 'IMG-01', 'import_method': 'web-download', + 'uri': 'http://joes-image-shack.com/funky.qcow2'}) + with mock.patch.object(self.gc.images, + 'get_import_info') as mocked_info: + with mock.patch.object(self.gc.images, 'get') as mocked_get: + mocked_get.return_value = {'status': 'uploading', + 'disk_format': 'raw'} + mocked_info.return_value = self.import_info_response + try: + test_shell.do_image_import(self.gc, args) + self.fail("utils.exit should have been called") + except SystemExit: + pass + mock_utils_exit.assert_called_once_with(expected_msg) + + @mock.patch('glanceclient.common.utils.exit') + def test_neg_image_import_image_no_disk_format( + self, mock_utils_exit): + expected_msg = ('The \'container_format\' and \'disk_format\' ' + 'properties must be set on an image before it can be ' + 'imported.') + mock_utils_exit.side_effect = self._mock_utils_exit + args = self._make_args( + {'id': 'IMG-01', 'import_method': 'web-download', + 'uri': 'http://joes-image-shack.com/funky.qcow2'}) + with mock.patch.object(self.gc.images, + 'get_import_info') as mocked_info: + with mock.patch.object(self.gc.images, 'get') as mocked_get: + mocked_get.return_value = {'status': 'uploading', + 'container_format': 'bare'} + mocked_info.return_value = self.import_info_response + try: + test_shell.do_image_import(self.gc, args) + self.fail("utils.exit should have been called") + except SystemExit: + pass + mock_utils_exit.assert_called_once_with(expected_msg) + + def test_image_import_glance_direct(self): + args = self._make_args( + {'id': 'IMG-01', 'import_method': 'glance-direct', 'uri': None}) + with mock.patch.object(self.gc.images, 'image_import') as mock_import: + with mock.patch.object(self.gc.images, 'get') as mocked_get: + with mock.patch.object(self.gc.images, + 'get_import_info') as mocked_info: + mocked_get.return_value = {'status': 'uploading', + 'container_format': 'bare', + 'disk_format': 'raw'} + mocked_info.return_value = self.import_info_response + mock_import.return_value = None + test_shell.do_image_import(self.gc, args) + mock_import.assert_called_once_with( + 'IMG-01', 'glance-direct', None) + + def test_image_import_web_download(self): args = self._make_args( {'id': 'IMG-01', 'uri': 'http://example.com/image.qcow', - 'import_method': 'web-download', 'from_create': True}) - + 'import_method': 'web-download'}) with mock.patch.object(self.gc.images, 'image_import') as mock_import: - mock_import.return_value = None - test_shell.do_image_import(self.gc, args) - mock_import.assert_called_once_with( - 'IMG-01', 'web-download', 'http://example.com/image.qcow') + with mock.patch.object(self.gc.images, 'get') as mocked_get: + with mock.patch.object(self.gc.images, + 'get_import_info') as mocked_info: + mocked_get.return_value = {'status': 'queued', + 'container_format': 'bare', + 'disk_format': 'raw'} + mocked_info.return_value = self.import_info_response + mock_import.return_value = None + test_shell.do_image_import(self.gc, args) + mock_import.assert_called_once_with( + 'IMG-01', 'web-download', + 'http://example.com/image.qcow') + + @mock.patch('glanceclient.common.utils.print_image') + def test_image_import_no_print_image(self, mocked_utils_print_image): + args = self._make_args( + {'id': 'IMG-02', 'uri': None, 'import_method': 'glance-direct', + 'from_create': True}) + with mock.patch.object(self.gc.images, 'image_import') as mock_import: + with mock.patch.object(self.gc.images, 'get') as mocked_get: + with mock.patch.object(self.gc.images, + 'get_import_info') as mocked_info: + mocked_get.return_value = {'status': 'uploading', + 'container_format': 'bare', + 'disk_format': 'raw'} + mocked_info.return_value = self.import_info_response + mock_import.return_value = None + test_shell.do_image_import(self.gc, args) + mock_import.assert_called_once_with( + 'IMG-02', 'glance-direct', None) + mocked_utils_print_image.assert_not_called() def test_image_download(self): args = self._make_args( diff --git a/glanceclient/v2/shell.py b/glanceclient/v2/shell.py index cbc90330..d837ba19 100644 --- a/glanceclient/v2/shell.py +++ b/glanceclient/v2/shell.py @@ -483,19 +483,53 @@ def do_image_stage(gc, args): help=_('ID of image to import.')) def do_image_import(gc, args): """Initiate the image import taskflow.""" - try: - if args.import_method == 'web-download' and not args.uri: - utils.exit("Provide URI for web-download import method.") - if args.uri and args.import_method != 'web-download': - utils.exit("Import method should be 'web-download' if URI is " - "provided.") + + if getattr(args, 'from_create', False): + # this command is being called "internally" so we can skip + # validation -- just do the import and get out of here gc.images.image_import(args.id, args.import_method, args.uri) + return + + # do input validation + try: + import_methods = gc.images.get_import_info().get('import-methods') except exc.HTTPNotFound: utils.exit('Target Glance does not support Image Import workflow') - else: - if not getattr(args, 'from_create', False): - image = gc.images.get(args.id) - utils.print_image(image) + + if args.import_method not in import_methods.get('value'): + utils.exit("Import method '%s' is not valid for this cloud. " + "Valid values can be retrieved with import-info command." % + args.import_method) + + if args.import_method == 'web-download' and not args.uri: + utils.exit("Provide URI for web-download import method.") + if args.uri and args.import_method != 'web-download': + utils.exit("Import method should be 'web-download' if URI is " + "provided.") + + # check image properties + image = gc.images.get(args.id) + container_format = image.get('container_format') + disk_format = image.get('disk_format') + if not (container_format and disk_format): + utils.exit("The 'container_format' and 'disk_format' properties " + "must be set on an image before it can be imported.") + + image_status = image.get('status') + if args.import_method == 'glance-direct': + if image_status != 'uploading': + utils.exit("The 'glance-direct' import method can only be applied " + "to an image in status 'uploading'") + if args.import_method == 'web-download': + if image_status != 'queued': + utils.exit("The 'web-download' import method can only be applied " + "to an image in status 'queued'") + + # finally, do the import + gc.images.image_import(args.id, args.import_method, args.uri) + + image = gc.images.get(args.id) + utils.print_image(image) @utils.arg('id', metavar='', nargs='+',