Merge "Add support for passing image size to Glance API"

Author: Zuul

openstackclient/image/v2/image.py

@@ -151,6 +151,19 @@ def _get_member_columns(item: Any) -> tuple[tuple[str, ...], tuple[str, ...]]:
     )
 
 
+def _parse_image_size(value: str) -> int:
+    try:
+        size = int(value)
+    except ValueError:
+        raise argparse.ArgumentTypeError(
+            _("'%(value)s' is not a valid size (use a positive integer)")
+            % {'value': value}
+        )
+    if size <= 0:
+        raise argparse.ArgumentTypeError(_("Size must be a positive integer"))
+    return size
+
+
 def get_data_from_stdin() -> Any:
     # distinguish cases where:
     # (1) stdin is not valid (as in cron jobs):
@@ -291,15 +304,14 @@ def take_action(
 class CreateImage(command.ShowOne):
     _description = _("Create/upload an image")
 
-    deadopts = ('size', 'location', 'copy-from', 'checksum', 'store')
+    deadopts = ('location', 'copy-from', 'checksum', 'store')
 
     def get_parser(self, prog_name: str) -> argparse.ArgumentParser:
         parser = super().get_parser(prog_name)
         # TODO(bunting): There are additional arguments that v1 supported
         # that v2 either doesn't support or supports weirdly.
         # --checksum - could be faked clientside perhaps?
         # --location - maybe location add?
-        # --size - passing image size is actually broken in python-glanceclient
         # --copy-from - does not exist in v2
         # --store - does not exits in v2
         parser.add_argument(
@@ -352,6 +364,15 @@ def get_parser(self, prog_name: str) -> argparse.ArgumentParser:
             type=int,
             help=_("Minimum RAM size needed to boot image, in megabytes"),
         )
+        parser.add_argument(
+            "--size",
+            metavar="<size>",
+            type=_parse_image_size,
+            help=_(
+                "Size of image data in bytes. Providing this can improve "
+                "upload performance."
+            ),
+        )
         source_group = parser.add_mutually_exclusive_group()
         source_group.add_argument(
             "--file",
@@ -520,6 +541,10 @@ def _take_action_image(
             )
             raise exceptions.CommandError(msg)
 
+        if parsed_args.size is not None and fp is None:
+            msg = _("--size requires image data via --file or stdin")
+            raise exceptions.CommandError(msg)
+
         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)
@@ -590,6 +615,11 @@ def _take_action_image(
             if signer.padding_method:
                 kwargs['img_signature_key_type'] = signer.padding_method
 
+        # Pass size only when uploading data. The SDK calculates size
+        # automatically when possible if it is not provided.
+        if parsed_args.size is not None:
+            kwargs['size'] = parsed_args.size
+
         image = image_client.create_image(**kwargs)
 
         if parsed_args.filename:
@@ -1608,8 +1638,15 @@ def get_parser(self, prog_name: str) -> argparse.ArgumentParser:
                 'Alternatively, images can be passed via stdin.'
             ),
         )
-        # NOTE(stephenfin): glanceclient had a --size argument but it didn't do
-        # anything so we have chosen not to port this
+        parser.add_argument(
+            '--size',
+            metavar='<size>',
+            type=_parse_image_size,
+            help=_(
+                'Size of image data in bytes. Providing this can improve '
+                'upload performance.'
+            ),
+        )
         parser.add_argument(
             '--progress',
             action='store_true',
@@ -1646,6 +1683,10 @@ def take_action(self, parsed_args: argparse.Namespace) -> None:
         else:
             fp = get_data_from_stdin()
 
+        if parsed_args.size is not None and fp is None:
+            msg = _("--size requires image data via --file or stdin")
+            raise exceptions.CommandError(msg)
+
         kwargs: dict[str, Any] = {}
 
         if parsed_args.progress and parsed_args.filename:
@@ -1661,6 +1702,11 @@ def take_action(self, parsed_args: argparse.Namespace) -> None:
         elif fp:
             kwargs['data'] = fp
 
+        # Pass size only when uploading data. The SDK calculates size
+        # automatically when possible if it is not provided.
+        if parsed_args.size is not None:
+            kwargs['size'] = parsed_args.size
+
         image_client.stage_image(image, **kwargs)
 
 

openstackclient/tests/unit/image/v2/test_image.py

@@ -27,6 +27,7 @@
 
 from openstackclient.image.v2 import image as _image
 from openstackclient.tests.unit.image.v2 import fakes as image_fakes
+from openstackclient.tests.unit import utils as test_utils
 from openstackclient.tests.unit.volume.v3 import fakes as volume_fakes
 
 
@@ -235,6 +236,128 @@ def test_image_create_file(self):
         self.assertEqual(self.expected_columns, columns)
         self.assertCountEqual(self.expected_data, data)
 
+    def test_image_create_file_with_size(self):
+        imagefile = tempfile.NamedTemporaryFile(delete=False)
+        imagefile.write(b'\0')
+        imagefile.close()
+
+        arglist = [
+            '--file',
+            imagefile.name,
+            '--size',
+            '2048',
+            (
+                '--unprotected'
+                if not self.new_image.is_protected
+                else '--protected'
+            ),
+            (
+                '--public'
+                if self.new_image.visibility == 'public'
+                else '--private'
+            ),
+            '--property',
+            'Alpha=1',
+            '--property',
+            'Beta=2',
+            '--tag',
+            self.new_image.tags[0],
+            '--tag',
+            self.new_image.tags[1],
+            self.new_image.name,
+        ]
+        verifylist = [
+            ('filename', imagefile.name),
+            ('size', 2048),
+            ('is_protected', self.new_image.is_protected),
+            ('visibility', self.new_image.visibility),
+            ('properties', {'Alpha': '1', 'Beta': '2'}),
+            ('tags', self.new_image.tags),
+            ('name', self.new_image.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.image_client.create_image.assert_called_with(
+            name=self.new_image.name,
+            allow_duplicates=True,
+            container_format=_image.DEFAULT_CONTAINER_FORMAT,
+            disk_format=_image.DEFAULT_DISK_FORMAT,
+            is_protected=self.new_image.is_protected,
+            visibility=self.new_image.visibility,
+            Alpha='1',
+            Beta='2',
+            tags=self.new_image.tags,
+            filename=imagefile.name,
+            size=2048,
+        )
+        self.image_client.get_image.assert_called_once_with(self.new_image)
+
+        self.assertEqual(self.expected_columns, columns)
+        self.assertCountEqual(self.expected_data, data)
+
+    @mock.patch('sys.stdin', side_effect=[None])
+    def test_image_create_size_requires_upload(self, raw_input):
+        arglist = [
+            '--size',
+            '2048',
+            self.new_image.name,
+        ]
+        verifylist = [
+            ('size', 2048),
+            ('name', self.new_image.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args,
+        )
+
+    def test_image_create_size_must_be_positive(self):
+        arglist = [
+            '--size',
+            '0',
+            self.new_image.name,
+        ]
+        self.assertRaises(
+            test_utils.ParserException,
+            self.check_parser,
+            self.cmd,
+            arglist,
+            [],
+        )
+
+    @mock.patch('openstackclient.image.v2.image.get_data_from_stdin')
+    def test_image_create_stdin_with_size(self, mock_get_data_from_stdin):
+        fake_stdin = io.BytesIO(b'some fake data')
+        mock_get_data_from_stdin.return_value = fake_stdin
+
+        arglist = [
+            '--size',
+            '2048',
+            self.new_image.name,
+        ]
+        verifylist = [
+            ('size', 2048),
+            ('name', self.new_image.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+
+        self.image_client.create_image.assert_called_with(
+            name=self.new_image.name,
+            allow_duplicates=True,
+            container_format=_image.DEFAULT_CONTAINER_FORMAT,
+            disk_format=_image.DEFAULT_DISK_FORMAT,
+            data=fake_stdin,
+            validate_checksum=False,
+            size=2048,
+        )
+
     @mock.patch('openstackclient.image.v2.image.get_data_from_stdin')
     def test_image_create__progress_ignore_with_stdin(
         self,
@@ -2033,6 +2156,52 @@ def test_stage_image__from_stdin(self, mock_get_data_from_stdin):
             data=fake_stdin,
         )
 
+    def test_stage_image__with_size(self):
+        imagefile = tempfile.NamedTemporaryFile(delete=False)
+        imagefile.write(b'\0' * 1024)
+        imagefile.close()
+
+        arglist = [
+            '--file',
+            imagefile.name,
+            '--size',
+            '2048',
+            self.image.name,
+        ]
+        verifylist = [
+            ('filename', imagefile.name),
+            ('size', 2048),
+            ('image', self.image.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+
+        self.image_client.stage_image.assert_called_once_with(
+            self.image,
+            filename=imagefile.name,
+            size=2048,
+        )
+
+    @mock.patch('sys.stdin', side_effect=[None])
+    def test_stage_image__size_requires_upload(self, raw_input):
+        arglist = [
+            '--size',
+            '2048',
+            self.image.name,
+        ]
+        verifylist = [
+            ('size', 2048),
+            ('image', self.image.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args,
+        )
+
 
 class TestImageImport(image_fakes.TestImagev2):
     image = image_fakes.create_one_image(

releasenotes/notes/set-size-on-upload-3f676b9af2b75bf3.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    The ``openstack image create`` and ``openstack image stage`` commands
+    now accept a ``--size`` option to specify the size of image data being
+    uploaded in bytes. Providing this can improve upload performance.