From cf2de9af79cedd51ca080f5a6521997c05647418 Mon Sep 17 00:00:00 2001
From: Dean Troyer <dtroyer@gmail.com>
Date: Fri, 18 Dec 2015 14:16:45 -0600
Subject: [PATCH] Change --owner to --project in image commands

* image create and image set now use --project to specify an alternate
  project to own the image
* --owner is still silently accepted but deprecated, add warning messages
* --project and --owner are mutually exclusive to prevent precedence issues

Closes Bug: 1527833
Change-Id: Iccb1a1d9175ef9b5edcd79d294607db12641c1f0
---
 doc/source/command-objects/image.rst          | 28 ++++---
 openstackclient/image/v1/image.py             | 65 +++++++++++---
 openstackclient/image/v2/image.py             | 84 ++++++++++++++-----
 openstackclient/tests/image/v1/test_image.py  |  9 +-
 openstackclient/tests/image/v2/test_image.py  | 63 ++++++++++++--
 .../notes/bug-1527833-42cde11d28b09ac3.yaml   |  7 ++
 6 files changed, 204 insertions(+), 52 deletions(-)
 create mode 100644 releasenotes/notes/bug-1527833-42cde11d28b09ac3.yaml

diff --git a/doc/source/command-objects/image.rst b/doc/source/command-objects/image.rst
index f1893efaec..1528676091 100644
--- a/doc/source/command-objects/image.rst
+++ b/doc/source/command-objects/image.rst
@@ -19,7 +19,6 @@ Create/upload an image
         [--store <store>]
         [--container-format <container-format>]
         [--disk-format <disk-format>]
-        [--owner <project>]
         [--size <size>]
         [--min-disk <disk-gb>]
         [--min-ram <ram-mb>]
@@ -33,7 +32,7 @@ Create/upload an image
         [--public | --private]
         [--property <key=value> [...] ]
         [--tag <tag> [...] ]
-        [--project-domain <project-domain>]
+        [--project <project> [--project-domain <project-domain>]]
         <image-name>
 
 .. option:: --id <id>
@@ -54,10 +53,6 @@ Create/upload an image
 
     Image disk format (default: raw)
 
-.. option:: --owner <project>
-
-    Image owner project name or ID
-
 .. option:: --size <size>
 
     Image size, in bytes (only used with --location and --copy-from)
@@ -128,11 +123,18 @@ Create/upload an image
 
     .. versionadded:: 2
 
+.. option:: --project <project>
+
+    Set an alternate project on this image (name or ID).
+    Previously known as `--owner`.
+
 .. option:: --project-domain <project-domain>
 
     Domain the project belongs to (name or ID).
     This can be used in case collisions between project names exist.
 
+    .. versionadded:: 2
+
 .. describe:: <image-name>
 
     New image name
@@ -225,7 +227,6 @@ Set image properties
 
     os image set
         [--name <name>]
-        [--owner <project>]
         [--min-disk <disk-gb>]
         [--min-ram <disk-ram>]
         [--container-format <container-format>]
@@ -250,17 +251,13 @@ Set image properties
         [--os-version <os-version>]
         [--ramdisk-id <ramdisk-id>]
         [--activate|--deactivate]
-        [--project-domain <project-domain>]
+        [--project <project> [--project-domain <project-domain>]]
         <image>
 
 .. option:: --name <name>
 
     New image name
 
-.. option:: --owner <project>
-
-    New image owner project (name or ID)
-
 .. option:: --min-disk <disk-gb>
 
     Minimum disk size needed to boot image, in gigabytes
@@ -407,11 +404,18 @@ Set image properties
 
     .. versionadded:: 2
 
+.. option:: --project <project>
+
+    Set an alternate project on this image (name or ID).
+    Previously known as `--owner`.
+
 .. option:: --project-domain <project-domain>
 
     Domain the project belongs to (name or ID).
     This can be used in case collisions between project names exist.
 
+    .. versionadded:: 2
+
 .. describe:: <image>
 
     Image to modify (name or ID)
diff --git a/openstackclient/image/v1/image.py b/openstackclient/image/v1/image.py
index 0382501ef6..c18f3fc79d 100644
--- a/openstackclient/image/v1/image.py
+++ b/openstackclient/image/v1/image.py
@@ -35,6 +35,7 @@ from glanceclient.common import utils as gc_utils
 from openstackclient.api import utils as api_utils
 from openstackclient.common import parseractions
 from openstackclient.common import utils
+from openstackclient.i18n import _  # noqa
 
 
 DEFAULT_CONTAINER_FORMAT = 'bare'
@@ -92,11 +93,6 @@ class CreateImage(show.ShowOne):
             help="Image disk format "
                  "(default: %s)" % DEFAULT_DISK_FORMAT,
         )
-        parser.add_argument(
-            "--owner",
-            metavar="<project>",
-            help="Image owner project name or ID",
-        )
         parser.add_argument(
             "--size",
             metavar="<size>",
@@ -178,12 +174,32 @@ class CreateImage(show.ShowOne):
             help="Set a property on this image "
                  "(repeat option to set multiple properties)",
         )
+        # NOTE(dtroyer): --owner is deprecated in Jan 2016 in an early
+        #                2.x release.  Do not remove before Jan 2017
+        #                and a 3.x release.
+        project_group = parser.add_mutually_exclusive_group()
+        project_group.add_argument(
+            "--project",
+            metavar="<project>",
+            help="Set an alternate project on this image (name or ID)",
+        )
+        project_group.add_argument(
+            "--owner",
+            metavar="<project>",
+            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
 
+        if getattr(parsed_args, 'owner', None) is not None:
+            self.log.warning(_(
+                'The --owner option is deprecated, '
+                'please use --project instead.'
+            ))
+
         # Build an attribute dict from the parsed args, only include
         # attributes that were actually set on the command line
         kwargs = {}
@@ -198,6 +214,12 @@ class CreateImage(show.ShowOne):
                     # Only include a value in kwargs for attributes that are
                     # actually present on the command line
                     kwargs[attr] = val
+
+        # Special case project option back to API attribute name 'owner'
+        val = getattr(parsed_args, 'project', None)
+        if val:
+            kwargs['owner'] = val
+
         # 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
@@ -383,7 +405,7 @@ class ListImage(lister.Lister):
                 'Status',
                 'Visibility',
                 'Protected',
-                'Owner',
+                'Project',
                 'Properties',
             )
         else:
@@ -476,11 +498,6 @@ class SetImage(command.Command):
             metavar="<name>",
             help="New image name",
         )
-        parser.add_argument(
-            "--owner",
-            metavar="<project>",
-            help="New image owner project (name or ID)",
-        )
         parser.add_argument(
             "--min-disk",
             metavar="<disk-gb>",
@@ -590,12 +607,32 @@ class SetImage(command.Command):
             metavar="<checksum>",
             help="Image hash used for verification",
         )
+        # NOTE(dtroyer): --owner is deprecated in Jan 2016 in an early
+        #                2.x release.  Do not remove before Jan 2017
+        #                and a 3.x release.
+        project_group = parser.add_mutually_exclusive_group()
+        project_group.add_argument(
+            "--project",
+            metavar="<project>",
+            help="Set an alternate project on this image (name or ID)",
+        )
+        project_group.add_argument(
+            "--owner",
+            metavar="<project>",
+            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
 
+        if getattr(parsed_args, 'owner', None) is not None:
+            self.log.warning(_(
+                'The --owner option is deprecated, '
+                'please use --project instead.'
+            ))
+
         kwargs = {}
         copy_attrs = ('name', 'owner', 'min_disk', 'min_ram', 'properties',
                       'container_format', 'disk_format', 'size', 'store',
@@ -607,6 +644,12 @@ class SetImage(command.Command):
                     # Only include a value in kwargs for attributes that are
                     # actually present on the command line
                     kwargs[attr] = val
+
+        # Special case project option back to API attribute name 'owner'
+        val = getattr(parsed_args, 'project', None)
+        if val:
+            kwargs['owner'] = val
+
         # 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
diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py
index ad536ba26b..3c1faf594c 100644
--- a/openstackclient/image/v2/image.py
+++ b/openstackclient/image/v2/image.py
@@ -28,6 +28,7 @@ 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.i18n import _  # noqa
 from openstackclient.identity import common
 
 
@@ -147,11 +148,6 @@ class CreateImage(show.ShowOne):
             help="Image disk format "
                  "(default: %s)" % DEFAULT_DISK_FORMAT,
         )
-        parser.add_argument(
-            "--owner",
-            metavar="<owner>",
-            help="Image owner project name or ID",
-        )
         parser.add_argument(
             "--min-disk",
             metavar="<disk-gb>",
@@ -220,6 +216,20 @@ class CreateImage(show.ShowOne):
             help="Set a tag on this image "
                  "(repeat option to set multiple tags)",
         )
+        # NOTE(dtroyer): --owner is deprecated in Jan 2016 in an early
+        #                2.x release.  Do not remove before Jan 2017
+        #                and a 3.x release.
+        project_group = parser.add_mutually_exclusive_group()
+        project_group.add_argument(
+            "--project",
+            metavar="<project>",
+            help="Set an alternate project on this image (name or ID)",
+        )
+        project_group.add_argument(
+            "--owner",
+            metavar="<project>",
+            help=argparse.SUPPRESS,
+        )
         common.add_project_domain_option_to_parser(parser)
         for deadopt in self.deadopts:
             parser.add_argument(
@@ -246,8 +256,7 @@ class CreateImage(show.ShowOne):
         kwargs = {}
         copy_attrs = ('name', 'id',
                       'container_format', 'disk_format',
-                      'min_disk', 'min_ram',
-                      'tags', 'owner')
+                      'min_disk', 'min_ram', 'tags')
         for attr in copy_attrs:
             if attr in parsed_args:
                 val = getattr(parsed_args, attr, None)
@@ -255,6 +264,7 @@ class CreateImage(show.ShowOne):
                     # 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):
@@ -275,6 +285,21 @@ class CreateImage(show.ShowOne):
         if parsed_args.private:
             kwargs['visibility'] = 'private'
 
+        # Handle deprecated --owner option
+        project_arg = parsed_args.project
+        if parsed_args.owner:
+            project_arg = parsed_args.owner
+            self.log.warning(_(
+                'The --owner option is deprecated, '
+                'please use --project instead.'
+            ))
+        if project_arg:
+            kwargs['owner'] = common.find_project(
+                identity_client,
+                project_arg,
+                parsed_args.project_domain,
+            ).id
+
         # open the file first to ensure any failures are handled before the
         # image is created
         fp = gc_utils.get_data_file(parsed_args)
@@ -458,7 +483,7 @@ class ListImage(lister.Lister):
                 'Status',
                 'Visibility',
                 'Protected',
-                'Owner',
+                'Project',
                 'Tags',
             )
         else:
@@ -598,11 +623,6 @@ class SetImage(command.Command):
             metavar="<name>",
             help="New image name"
         )
-        parser.add_argument(
-            "--owner",
-            metavar="<project>",
-            help="New image owner project (name or ID)",
-        )
         parser.add_argument(
             "--min-disk",
             type=int,
@@ -713,6 +733,20 @@ class SetImage(command.Command):
             action="store_true",
             help="Activate the image",
         )
+        # NOTE(dtroyer): --owner is deprecated in Jan 2016 in an early
+        #                2.x release.  Do not remove before Jan 2017
+        #                and a 3.x release.
+        project_group = parser.add_mutually_exclusive_group()
+        project_group.add_argument(
+            "--project",
+            metavar="<project>",
+            help="Set an alternate project on this image (name or ID)",
+        )
+        project_group.add_argument(
+            "--owner",
+            metavar="<project>",
+            help=argparse.SUPPRESS,
+        )
         common.add_project_domain_option_to_parser(parser)
         for deadopt in self.deadopts:
             parser.add_argument(
@@ -738,7 +772,7 @@ class SetImage(command.Command):
         copy_attrs = ('architecture', 'container_format', 'disk_format',
                       'file', 'instance_id', 'kernel_id', 'locations',
                       'min_disk', 'min_ram', 'name', 'os_distro', 'os_version',
-                      'owner', 'prefix', 'progress', 'ramdisk_id', 'tags')
+                      'prefix', 'progress', 'ramdisk_id', 'tags')
         for attr in copy_attrs:
             if attr in parsed_args:
                 val = getattr(parsed_args, attr, None)
@@ -767,6 +801,21 @@ class SetImage(command.Command):
         if parsed_args.private:
             kwargs['visibility'] = 'private'
 
+        # Handle deprecated --owner option
+        project_arg = parsed_args.project
+        if parsed_args.owner:
+            project_arg = parsed_args.owner
+            self.log.warning(_(
+                'The --owner option is deprecated, '
+                'please use --project instead.'
+            ))
+        if project_arg:
+            kwargs['owner'] = common.find_project(
+                identity_client,
+                project_arg,
+                parsed_args.project_domain,
+            ).id
+
         # Checks if anything that requires getting the image
         if not (kwargs or parsed_args.deactivate or parsed_args.activate):
             self.log.warning("No arguments specified")
@@ -790,13 +839,6 @@ class SetImage(command.Command):
             # Tags should be extended, but duplicates removed
             kwargs['tags'] = list(set(image.tags).union(set(parsed_args.tags)))
 
-        if parsed_args.owner:
-            kwargs['owner'] = common.find_project(
-                identity_client,
-                parsed_args.owner,
-                parsed_args.project_domain,
-            ).id
-
         try:
             image = image_client.images.update(image.id, **kwargs)
         except Exception as e:
diff --git a/openstackclient/tests/image/v1/test_image.py b/openstackclient/tests/image/v1/test_image.py
index 4d964bdb56..60b7f3093d 100644
--- a/openstackclient/tests/image/v1/test_image.py
+++ b/openstackclient/tests/image/v1/test_image.py
@@ -103,6 +103,7 @@ class TestImageCreate(TestImage):
             '--min-ram', '4',
             '--protected',
             '--private',
+            '--project', 'q',
             image_fakes.image_name,
         ]
         verifylist = [
@@ -114,6 +115,7 @@ class TestImageCreate(TestImage):
             ('unprotected', False),
             ('public', False),
             ('private', True),
+            ('project', 'q'),
             ('name', image_fakes.image_name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -130,6 +132,7 @@ class TestImageCreate(TestImage):
             min_ram=4,
             protected=True,
             is_public=False,
+            owner='q',
             data=mock.ANY,
         )
 
@@ -358,7 +361,7 @@ class TestImageList(TestImage):
             'Status',
             'Visibility',
             'Protected',
-            'Owner',
+            'Project',
             'Properties',
         )
 
@@ -484,22 +487,22 @@ class TestImageSet(TestImage):
     def test_image_set_options(self):
         arglist = [
             '--name', 'new-name',
-            '--owner', 'new-owner',
             '--min-disk', '2',
             '--min-ram', '4',
             '--container-format', 'ovf',
             '--disk-format', 'vmdk',
             '--size', '35165824',
+            '--project', 'new-owner',
             image_fakes.image_name,
         ]
         verifylist = [
             ('name', 'new-name'),
-            ('owner', 'new-owner'),
             ('min_disk', 2),
             ('min_ram', 4),
             ('container_format', 'ovf'),
             ('disk_format', 'vmdk'),
             ('size', 35165824),
+            ('project', 'new-owner'),
             ('image', image_fakes.image_name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
diff --git a/openstackclient/tests/image/v2/test_image.py b/openstackclient/tests/image/v2/test_image.py
index 0218241397..3534a3a441 100644
--- a/openstackclient/tests/image/v2/test_image.py
+++ b/openstackclient/tests/image/v2/test_image.py
@@ -131,11 +131,11 @@ class TestImageCreate(TestImage):
             '--disk-format', 'fs',
             '--min-disk', '10',
             '--min-ram', '4',
-            '--owner', self.new_image.owner,
             ('--protected'
                 if self.new_image.protected else '--unprotected'),
             ('--private'
                 if self.new_image.visibility == 'private' else '--public'),
+            '--project', self.new_image.owner,
             '--project-domain', identity_fakes.domain_id,
             self.new_image.name,
         ]
@@ -144,11 +144,11 @@ class TestImageCreate(TestImage):
             ('disk_format', 'fs'),
             ('min_disk', 10),
             ('min_ram', 4),
-            ('owner', self.new_image.owner),
             ('protected', self.new_image.protected),
             ('unprotected', not self.new_image.protected),
             ('public', self.new_image.visibility == 'public'),
             ('private', self.new_image.visibility == 'private'),
+            ('project', self.new_image.owner),
             ('project_domain', identity_fakes.domain_id),
             ('name', self.new_image.name),
         ]
@@ -217,6 +217,40 @@ class TestImageCreate(TestImage):
             parsed_args,
         )
 
+    def test_image_create_with_unexist_project(self):
+        self.project_mock.get.side_effect = exceptions.NotFound(None)
+        self.project_mock.find.side_effect = exceptions.NotFound(None)
+
+        arglist = [
+            '--container-format', 'ovf',
+            '--disk-format', 'fs',
+            '--min-disk', '10',
+            '--min-ram', '4',
+            '--protected',
+            '--private',
+            '--project', 'unexist_owner',
+            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),
+            ('project', 'unexist_owner'),
+            ('name', image_fakes.image_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args,
+        )
+
     @mock.patch('glanceclient.common.utils.get_data_file', name='Open')
     def test_image_create_file(self, mock_open):
         mock_file = mock.MagicMock(name='File')
@@ -575,7 +609,7 @@ class TestImageList(TestImage):
             'Status',
             'Visibility',
             'Protected',
-            'Owner',
+            'Project',
             'Tags',
         )
 
@@ -755,21 +789,21 @@ class TestImageSet(TestImage):
     def test_image_set_options(self):
         arglist = [
             '--name', 'new-name',
-            '--owner', identity_fakes.project_name,
             '--min-disk', '2',
             '--min-ram', '4',
             '--container-format', 'ovf',
             '--disk-format', 'vmdk',
+            '--project', identity_fakes.project_name,
             '--project-domain', identity_fakes.domain_id,
             image_fakes.image_id,
         ]
         verifylist = [
             ('name', 'new-name'),
-            ('owner', identity_fakes.project_name),
             ('min_disk', 2),
             ('min_ram', 4),
             ('container_format', 'ovf'),
             ('disk_format', 'vmdk'),
+            ('project', identity_fakes.project_name),
             ('project_domain', identity_fakes.domain_id),
             ('image', image_fakes.image_id),
         ]
@@ -809,6 +843,25 @@ class TestImageSet(TestImage):
             exceptions.CommandError,
             self.cmd.take_action, parsed_args)
 
+    def test_image_set_with_unexist_project(self):
+        self.project_mock.get.side_effect = exceptions.NotFound(None)
+        self.project_mock.find.side_effect = exceptions.NotFound(None)
+
+        arglist = [
+            '--project', 'unexist_owner',
+            image_fakes.image_id,
+        ]
+        verifylist = [
+            ('project', 'unexist_owner'),
+            ('image', image_fakes.image_id),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action, parsed_args)
+
     def test_image_set_bools1(self):
         arglist = [
             '--protected',
diff --git a/releasenotes/notes/bug-1527833-42cde11d28b09ac3.yaml b/releasenotes/notes/bug-1527833-42cde11d28b09ac3.yaml
new file mode 100644
index 0000000000..21f748bbe0
--- /dev/null
+++ b/releasenotes/notes/bug-1527833-42cde11d28b09ac3.yaml
@@ -0,0 +1,7 @@
+---
+other:
+  - Change the `--owner` option to `--project` in `image create`
+    and `image set` commands.  `--owner` is deprecated and no longer
+    documented but is still accepted; a warning message will be shown
+    if it is used.
+    [Bug `1527833 <https://bugs.launchpad.net/bugs/1527833>`_]