diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py index 1ff8ad3ae9..53cfadede0 100644 --- a/openstackclient/image/v2/image.py +++ b/openstackclient/image/v2/image.py @@ -134,34 +134,32 @@ def _get_member_columns(item): ) -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) +def get_data_from_stdin(): + # 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 - return (image, None) - else: - # (3) - 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 + else: + # (3) + return None class AddProjectToImage(command.ShowOne): @@ -277,6 +275,7 @@ class CreateImage(command.ShowOne): source_group = parser.add_mutually_exclusive_group() source_group.add_argument( "--file", + dest="filename", metavar="", help=_("Upload image from local file"), ) @@ -299,7 +298,10 @@ class CreateImage(command.ShowOne): "--progress", action="store_true", default=False, - help=_("Show upload progress bar."), + help=_( + "Show upload progress bar " + "(ignored if passing data via stdin)" + ), ) parser.add_argument( '--sign-key-path', @@ -463,7 +465,15 @@ class CreateImage(command.ShowOne): # open the file first to ensure any failures are handled before the # 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) + if parsed_args.filename: + try: + fp = open(parsed_args.filename, 'rb') + except FileNotFoundError: + raise exceptions.CommandError( + '%r is not a valid file' % parsed_args.filename, + ) + else: + fp = get_data_from_stdin() if fp is not None and parsed_args.volume: msg = _( @@ -472,26 +482,24 @@ class CreateImage(command.ShowOne): ) raise exceptions.CommandError(msg) - if fp is None and parsed_args.file: - LOG.warning(_("Failed to get an image file.")) - return {}, {} - - if parsed_args.progress and parsed_args.file: + if parsed_args.progress and parsed_args.filename: # NOTE(stephenfin): we only show a progress bar if the user # requested it *and* we're reading from a file (not stdin) - filesize = os.path.getsize(fname) + filesize = os.path.getsize(parsed_args.filename) if filesize is not None: kwargs['validate_checksum'] = False kwargs['data'] = progressbar.VerboseFileWrapper(fp, filesize) - elif fname: - kwargs['filename'] = fname + else: + kwargs['data'] = fp + elif parsed_args.filename: + kwargs['filename'] = parsed_args.filename 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: - if not parsed_args.file: + if not parsed_args.filename: msg = _( "signing an image requires the --file option, " "passing files via stdin when signing is not " @@ -546,6 +554,10 @@ class CreateImage(command.ShowOne): kwargs['img_signature_key_type'] = signer.padding_method image = image_client.create_image(**kwargs) + + if parsed_args.filename: + fp.close() + return _format_image(image) def _take_action_volume(self, parsed_args): @@ -980,6 +992,7 @@ class SaveImage(command.Command): parser.add_argument( "--file", metavar="", + dest="filename", help=_("Downloaded image save filename (default: stdout)"), ) parser.add_argument( @@ -993,7 +1006,7 @@ class SaveImage(command.Command): image_client = self.app.client_manager.image image = image_client.find_image(parsed_args.image) - output_file = parsed_args.file + output_file = parsed_args.filename if output_file is None: output_file = getattr(sys.stdout, "buffer", sys.stdout) diff --git a/openstackclient/tests/unit/image/v2/test_image.py b/openstackclient/tests/unit/image/v2/test_image.py index e17363a500..ac9ddae6e9 100644 --- a/openstackclient/tests/unit/image/v2/test_image.py +++ b/openstackclient/tests/unit/image/v2/test_image.py @@ -14,7 +14,6 @@ import copy import io -import os import tempfile from unittest import mock @@ -104,15 +103,8 @@ class TestImageCreate(TestImage): disk_format=image.DEFAULT_DISK_FORMAT, ) - # Verify update() was not called, if it was show the args - self.assertEqual(self.client.update_image.call_args_list, []) - - self.assertEqual( - self.expected_columns, - columns) - self.assertCountEqual( - self.expected_data, - data) + self.assertEqual(self.expected_columns, columns) + self.assertCountEqual(self.expected_data, data) @mock.patch('sys.stdin', side_effect=[None]) def test_image_reserve_options(self, raw_input): @@ -121,10 +113,11 @@ class TestImageCreate(TestImage): '--disk-format', 'ami', '--min-disk', '10', '--min-ram', '4', - ('--protected' - if self.new_image.is_protected else '--unprotected'), - ('--private' - if self.new_image.visibility == 'private' else '--public'), + '--protected' if self.new_image.is_protected else '--unprotected', + ( + '--private' + if self.new_image.visibility == 'private' else '--public' + ), '--project', self.new_image.owner_id, '--project-domain', self.domain.id, self.new_image.name, @@ -160,12 +153,8 @@ class TestImageCreate(TestImage): visibility=self.new_image.visibility, ) - self.assertEqual( - self.expected_columns, - columns) - self.assertCountEqual( - self.expected_data, - data) + self.assertEqual(self.expected_columns, columns) + self.assertCountEqual(self.expected_data, data) def test_image_create_with_unexist_project(self): self.project_mock.get.side_effect = exceptions.NotFound(None) @@ -217,7 +206,7 @@ class TestImageCreate(TestImage): self.new_image.name, ] verifylist = [ - ('file', imagefile.name), + ('filename', imagefile.name), ('is_protected', self.new_image.is_protected), ('visibility', self.new_image.visibility), ('properties', {'Alpha': '1', 'Beta': '2'}), @@ -252,12 +241,12 @@ class TestImageCreate(TestImage): self.expected_data, data) - @mock.patch('openstackclient.image.v2.image.get_data_file') + @mock.patch('openstackclient.image.v2.image.get_data_from_stdin') def test_image_create__progress_ignore_with_stdin( - self, mock_get_data_file, + self, mock_get_data_from_stdin, ): fake_stdin = io.StringIO('fake-image-data') - mock_get_data_file.return_value = (fake_stdin, None) + mock_get_data_from_stdin.return_value = fake_stdin arglist = [ '--progress', @@ -322,11 +311,11 @@ class TestImageCreate(TestImage): ) @mock.patch('osc_lib.utils.find_resource') - @mock.patch('openstackclient.image.v2.image.get_data_file') + @mock.patch('openstackclient.image.v2.image.get_data_from_stdin') def test_image_create_from_volume(self, mock_get_data_f, mock_get_vol): fake_vol_id = 'fake-volume-id' - mock_get_data_f.return_value = (None, None) + mock_get_data_f.return_value = None class FakeVolume: id = fake_vol_id @@ -353,12 +342,12 @@ class TestImageCreate(TestImage): ) @mock.patch('osc_lib.utils.find_resource') - @mock.patch('openstackclient.image.v2.image.get_data_file') + @mock.patch('openstackclient.image.v2.image.get_data_from_stdin') def test_image_create_from_volume_fail(self, mock_get_data_f, mock_get_vol): fake_vol_id = 'fake-volume-id' - mock_get_data_f.return_value = (None, None) + mock_get_data_f.return_value = None class FakeVolume: id = fake_vol_id @@ -379,7 +368,7 @@ class TestImageCreate(TestImage): parsed_args) @mock.patch('osc_lib.utils.find_resource') - @mock.patch('openstackclient.image.v2.image.get_data_file') + @mock.patch('openstackclient.image.v2.image.get_data_from_stdin') def test_image_create_from_volume_v31(self, mock_get_data_f, mock_get_vol): @@ -387,7 +376,7 @@ class TestImageCreate(TestImage): api_versions.APIVersion('3.1')) fake_vol_id = 'fake-volume-id' - mock_get_data_f.return_value = (None, None) + mock_get_data_f.return_value = None class FakeVolume: id = fake_vol_id @@ -1798,7 +1787,7 @@ class TestImageSave(TestImage): arglist = ['--file', '/path/to/file', self.image.id] verifylist = [ - ('file', '/path/to/file'), + ('filename', '/path/to/file'), ('image', self.image.id), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1813,49 +1802,26 @@ class TestImageSave(TestImage): class TestImageGetData(TestImage): - def setUp(self): - super().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") + def test_get_data_from_stdin(self): + fd = io.BytesIO(b"some initial binary data: \x00\x01") with mock.patch('sys.stdin') as stdin: - stdin.return_value = f + stdin.return_value = fd stdin.isatty.return_value = False - stdin.buffer = f + stdin.buffer = fd - (test_fd, test_name) = image.get_data_file(self.args) + test_fd = image.get_data_from_stdin() # Ensure data written to temp file is correct - self.assertEqual(f, test_fd) - self.assertIsNone(test_name) + self.assertEqual(fd, test_fd) - def test_get_data_file_3(self): - - self.args.file = None - - f = io.BytesIO(b"some initial binary data: \x00\x01") + def test_get_data_from_stdin__interactive(self): + fd = 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 + stdin.return_value = fd - (test_fd, test_fname) = image.get_data_file(self.args) + test_fd = image.get_data_from_stdin() self.assertIsNone(test_fd) - self.assertIsNone(test_fname)