Add image create support for image v2

We have it for v1, but v2 is the future. There are two differences,
things in v2 do not go into a properties dict, and the actual image
data needs to get uploaded as a second step.

Closes-Bug: 1405562
Co-Authored-By: Niall Bunting <niall.bunting@hp.com>
Co-Authored-By: Sean Perry <sean.perry@hp.com>
Change-Id: If7b81c4a6746c8a1eb0302c96e045fb0f457d67b

4 files changed, 393 insertions, 2 deletions

diff --git a/doc/source/command-objects/image.rst b/doc/source/command-objects/image.rst
index 25741424..8e44f517 100644
--- a/doc/source/command-objects/image.rst
+++ b/doc/source/command-objects/image.rst
@@ -7,7 +7,7 @@ Image v1, v2
 image create
 ------------
 
-*Only supported for Image v1*
+*Image v1, v2*
 
 Create/upload an image
 
@@ -32,6 +32,7 @@ Create/upload an image
         [--protected | --unprotected]
         [--public | --private]
         [--property <key=value> [...] ]
+        [--tag <tag> [...] ]
         <image-name>
 
 .. option:: --id <id>
@@ -42,6 +43,8 @@ Create/upload an image
 
     Upload image to this store
 
+    *Image version 1 only.*
+
 .. option:: --container-format <container-format>
 
     Image container format (default: bare)
@@ -54,10 +57,14 @@ Create/upload an image
 
     Image owner project name or ID
 
+    *Image version 1 only.*
+
 .. option:: --size <size>
 
     Image size, in bytes (only used with --location and --copy-from)
 
+    *Image version 1 only.*
+
 .. option:: --min-disk <disk-gb>
 
     Minimum disk size needed to boot image, in gigabytes
@@ -70,10 +77,14 @@ Create/upload an image
 
     Download image from an existing URL
 
+    *Image version 1 only.*
+
 .. option:: --copy-from <image-url>
 
     Copy image from the data store (similar to --location)
 
+    *Image version 1 only.*
+
 .. option:: --file <file>
 
     Upload image from local file
@@ -90,6 +101,8 @@ Create/upload an image
 
     Image hash used for verification
 
+    *Image version 1 only.*
+
 .. option:: --protected
 
     Prevent image from being deleted
@@ -110,6 +123,12 @@ Create/upload an image
 
     Set a property on this image (repeat for multiple values)
 
+.. option:: --tag <tag>
+
+    Set a tag on this image (repeat for multiple values)
+
+    .. versionadded:: 2
+
 .. describe:: <image-name>
 
     New image name
diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py
index 4c019db6..67390118 100644
--- a/openstackclient/image/v2/image.py
+++ b/openstackclient/image/v2/image.py
@@ -22,14 +22,19 @@ import six
 from cliff import command
 from cliff import lister
 from cliff import show
-
 from glanceclient.common import utils as gc_utils
+
 from openstackclient.api import utils as api_utils
+from openstackclient.common import exceptions
 from openstackclient.common import parseractions
 from openstackclient.common import utils
 from openstackclient.identity import common
 
 
+DEFAULT_CONTAINER_FORMAT = 'bare'
+DEFAULT_DISK_FORMAT = 'raw'
+
+
 class AddProjectToImage(show.ShowOne):
     """Associate project with image"""
 
@@ -72,6 +77,186 @@ class AddProjectToImage(show.ShowOne):
         return zip(*sorted(six.iteritems(image_member._info)))
 
 
+class CreateImage(show.ShowOne):
+    """Create/upload an image"""
+
+    log = logging.getLogger(__name__ + ".CreateImage")
+    deadopts = ('owner', 'size', 'location', 'copy-from', 'checksum', 'store')
+
+    def get_parser(self, prog_name):
+        parser = super(CreateImage, self).get_parser(prog_name)
+        # TODO(mordred): add --volume and --force parameters and support
+        # TODO(bunting): There are additional arguments that v1 supported
+        # that v2 either doesn't support or supports weirdly.
+        # --checksum - could be faked clientside perhaps?
+        # --owner - could be set as an update after the put?
+        # --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(
+            "name",
+            metavar="<image-name>",
+            help="New image name",
+        )
+        parser.add_argument(
+            "--id",
+            metavar="<id>",
+            help="Image ID to reserve",
+        )
+        parser.add_argument(
+            "--container-format",
+            default=DEFAULT_CONTAINER_FORMAT,
+            metavar="<container-format>",
+            help="Image container format "
+                 "(default: %s)" % DEFAULT_CONTAINER_FORMAT,
+        )
+        parser.add_argument(
+            "--disk-format",
+            default=DEFAULT_DISK_FORMAT,
+            metavar="<disk-format>",
+            help="Image disk format "
+                 "(default: %s)" % DEFAULT_DISK_FORMAT,
+        )
+        parser.add_argument(
+            "--min-disk",
+            metavar="<disk-gb>",
+            type=int,
+            help="Minimum disk size needed to boot image, in gigabytes",
+        )
+        parser.add_argument(
+            "--min-ram",
+            metavar="<ram-mb>",
+            type=int,
+            help="Minimum RAM size needed to boot image, in megabytes",
+        )
+        parser.add_argument(
+            "--file",
+            metavar="<file>",
+            help="Upload image from local file",
+        )
+        protected_group = parser.add_mutually_exclusive_group()
+        protected_group.add_argument(
+            "--protected",
+            action="store_true",
+            help="Prevent image from being deleted",
+        )
+        protected_group.add_argument(
+            "--unprotected",
+            action="store_true",
+            help="Allow image to be deleted (default)",
+        )
+        public_group = parser.add_mutually_exclusive_group()
+        public_group.add_argument(
+            "--public",
+            action="store_true",
+            help="Image is accessible to the public",
+        )
+        public_group.add_argument(
+            "--private",
+            action="store_true",
+            help="Image is inaccessible to the public (default)",
+        )
+        parser.add_argument(
+            "--property",
+            dest="properties",
+            metavar="<key=value>",
+            action=parseractions.KeyValueAction,
+            help="Set a property on this image "
+                 "(repeat option to set multiple properties)",
+        )
+        parser.add_argument(
+            "--tag",
+            dest="tags",
+            metavar="<tag>",
+            action='append',
+            help="Set a tag on this image "
+                 "(repeat option to set multiple tags)",
+        )
+        for deadopt in self.deadopts:
+            parser.add_argument(
+                "--%s" % deadopt,
+                metavar="<%s>" % deadopt,
+                dest=deadopt.replace('-', '_'),
+                help=argparse.SUPPRESS
+            )
+        return parser
+
+    def take_action(self, parsed_args):
+        self.log.debug("take_action(%s)", parsed_args)
+        image_client = self.app.client_manager.image
+
+        for deadopt in self.deadopts:
+            if getattr(parsed_args, deadopt.replace('-', '_'), None):
+                raise exceptions.CommandError(
+                    "ERROR: --%s was given, which is an Image v1 option"
+                    " that is no longer supported in Image v2" % deadopt)
+
+        # Build an attribute dict from the parsed args, only include
+        # attributes that were actually set on the command line
+        kwargs = {}
+        copy_attrs = ('name', 'id',
+                      'container_format', 'disk_format',
+                      'min_disk', 'min_ram',
+                      'tags')
+        for attr in copy_attrs:
+            if attr in parsed_args:
+                val = getattr(parsed_args, attr, None)
+                if val:
+                    # Only include a value in kwargs for attributes that
+                    # are actually present on the command line
+                    kwargs[attr] = val
+        # properties should get flattened into the general kwargs
+        if getattr(parsed_args, 'properties', None):
+            for k, v in six.iteritems(parsed_args.properties):
+                kwargs[k] = str(v)
+        # Handle exclusive booleans with care
+        # Avoid including attributes in kwargs if an option is not
+        # present on the command line.  These exclusive booleans are not
+        # a single value for the pair of options because the default must be
+        # to do nothing when no options are present as opposed to always
+        # setting a default.
+        if parsed_args.protected:
+            kwargs['protected'] = True
+        if parsed_args.unprotected:
+            kwargs['protected'] = False
+        if parsed_args.public:
+            kwargs['visibility'] = 'public'
+        if parsed_args.private:
+            kwargs['visibility'] = 'private'
+
+        # open the file first to ensure any failures are handled before the
+        # image is created
+        fp = gc_utils.get_data_file(parsed_args)
+
+        if fp is None and parsed_args.file:
+            self.log.warning("Failed to get an image file.")
+            return {}, {}
+
+        image = image_client.images.create(**kwargs)
+
+        if fp is not None:
+            with fp:
+                try:
+                    image_client.images.upload(image.id, fp)
+                except Exception as e:
+                    # If the upload fails for some reason attempt to remove the
+                    # dangling queued image made by the create() call above but
+                    # only if the user did not specify an id which indicates
+                    # the Image already exists and should be left alone.
+                    try:
+                        if 'id' not in kwargs:
+                            image_client.images.delete(image.id)
+                    except Exception:
+                        pass  # we don't care about this one
+                    raise e  # now, throw the upload exception again
+
+                # update the image after the data has been uploaded
+                image = image_client.images.get(image.id)
+
+        return zip(*sorted(six.iteritems(image)))
+
+
 class DeleteImage(command.Command):
     """Delete image(s)"""
 
diff --git a/openstackclient/tests/image/v2/test_image.py b/openstackclient/tests/image/v2/test_image.py
index bfb94765..bb720d79 100644
--- a/openstackclient/tests/image/v2/test_image.py
+++ b/openstackclient/tests/image/v2/test_image.py
@@ -19,6 +19,7 @@ import mock
 import warlock
 
 from glanceclient.v2 import schemas
+from openstackclient.common import exceptions
 from openstackclient.image.v2 import image
 from openstackclient.tests import fakes
 from openstackclient.tests.identity.v3 import fakes as identity_fakes
@@ -41,6 +42,191 @@ class TestImage(image_fakes.TestImagev2):
         self.domain_mock.reset_mock()
 
 
+class TestImageCreate(TestImage):
+
+    def setUp(self):
+        super(TestImageCreate, self).setUp()
+
+        self.images_mock.create.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy(image_fakes.IMAGE),
+            loaded=True,
+        )
+        # This is the return value for utils.find_resource()
+        self.images_mock.get.return_value = copy.deepcopy(image_fakes.IMAGE)
+        self.images_mock.update.return_value = fakes.FakeResource(
+            None,
+            copy.deepcopy(image_fakes.IMAGE),
+            loaded=True,
+        )
+
+        # Get the command object to test
+        self.cmd = image.CreateImage(self.app, None)
+
+    def test_image_reserve_no_options(self):
+        mock_exception = {
+            'find.side_effect': exceptions.CommandError('x'),
+        }
+        self.images_mock.configure_mock(**mock_exception)
+        arglist = [
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('container_format', image.DEFAULT_CONTAINER_FORMAT),
+            ('disk_format', image.DEFAULT_DISK_FORMAT),
+            ('name', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # ImageManager.create(name=, **)
+        self.images_mock.create.assert_called_with(
+            name=image_fakes.image_name,
+            container_format=image.DEFAULT_CONTAINER_FORMAT,
+            disk_format=image.DEFAULT_DISK_FORMAT,
+        )
+
+        # Verify update() was not called, if it was show the args
+        self.assertEqual(self.images_mock.update.call_args_list, [])
+
+        self.images_mock.upload.assert_called_with(
+            mock.ANY, mock.ANY,
+        )
+
+        self.assertEqual(image_fakes.IMAGE_columns, columns)
+        self.assertEqual(image_fakes.IMAGE_data, data)
+
+    @mock.patch('glanceclient.common.utils.get_data_file', name='Open')
+    def test_image_reserve_options(self, mock_open):
+        mock_file = mock.MagicMock(name='File')
+        mock_open.return_value = mock_file
+        mock_open.read.return_value = None
+        mock_exception = {
+            'find.side_effect': exceptions.CommandError('x'),
+        }
+        self.images_mock.configure_mock(**mock_exception)
+        arglist = [
+            '--container-format', 'ovf',
+            '--disk-format', 'fs',
+            '--min-disk', '10',
+            '--min-ram', '4',
+            '--protected',
+            '--private',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('container_format', 'ovf'),
+            ('disk_format', 'fs'),
+            ('min_disk', 10),
+            ('min_ram', 4),
+            ('protected', True),
+            ('unprotected', False),
+            ('public', False),
+            ('private', True),
+            ('name', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # ImageManager.create(name=, **)
+        self.images_mock.create.assert_called_with(
+            name=image_fakes.image_name,
+            container_format='ovf',
+            disk_format='fs',
+            min_disk=10,
+            min_ram=4,
+            protected=True,
+            visibility='private',
+        )
+
+        # Verify update() was not called, if it was show the args
+        self.assertEqual(self.images_mock.update.call_args_list, [])
+
+        self.images_mock.upload.assert_called_with(
+            mock.ANY, mock.ANY,
+        )
+
+        self.assertEqual(image_fakes.IMAGE_columns, columns)
+        self.assertEqual(image_fakes.IMAGE_data, data)
+
+    @mock.patch('glanceclient.common.utils.get_data_file', name='Open')
+    def test_image_create_file(self, mock_open):
+        mock_file = mock.MagicMock(name='File')
+        mock_open.return_value = mock_file
+        mock_open.read.return_value = image_fakes.IMAGE_data
+        mock_exception = {
+            'find.side_effect': exceptions.CommandError('x'),
+        }
+        self.images_mock.configure_mock(**mock_exception)
+
+        arglist = [
+            '--file', 'filer',
+            '--unprotected',
+            '--public',
+            '--property', 'Alpha=1',
+            '--property', 'Beta=2',
+            '--tag', 'awesome',
+            '--tag', 'better',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('file', 'filer'),
+            ('protected', False),
+            ('unprotected', True),
+            ('public', True),
+            ('private', False),
+            ('properties', {'Alpha': '1', 'Beta': '2'}),
+            ('tags', ['awesome', 'better']),
+            ('name', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+
+        # ImageManager.create(name=, **)
+        self.images_mock.create.assert_called_with(
+            name=image_fakes.image_name,
+            container_format=image.DEFAULT_CONTAINER_FORMAT,
+            disk_format=image.DEFAULT_DISK_FORMAT,
+            protected=False,
+            visibility='public',
+            Alpha='1',
+            Beta='2',
+            tags=['awesome', 'better'],
+        )
+
+        # Verify update() was not called, if it was show the args
+        self.assertEqual(self.images_mock.update.call_args_list, [])
+
+        self.images_mock.upload.assert_called_with(
+            mock.ANY, mock.ANY,
+        )
+
+        self.assertEqual(image_fakes.IMAGE_columns, columns)
+        self.assertEqual(image_fakes.IMAGE_data, data)
+
+    def test_image_dead_options(self):
+
+        arglist = [
+            '--owner', 'nobody',
+            image_fakes.image_name,
+        ]
+        verifylist = [
+            ('owner', 'nobody'),
+            ('name', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action, parsed_args)
+
+
 class TestAddProjectToImage(TestImage):
 
     def setUp(self):
diff --git a/setup.cfg b/setup.cfg
index f2f4833b..0b936825 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -316,6 +316,7 @@ openstack.image.v1 =
 
 openstack.image.v2 =
     image_add_project = openstackclient.image.v2.image:AddProjectToImage
+    image_create = openstackclient.image.v2.image:CreateImage
     image_delete = openstackclient.image.v2.image:DeleteImage
     image_list = openstackclient.image.v2.image:ListImage
     image_remove_project = openstackclient.image.v2.image:RemoveProjectImage