From cf2de9af79cedd51ca080f5a6521997c05647418 Mon Sep 17 00:00:00 2001 From: Dean Troyer 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 ] [--container-format ] [--disk-format ] - [--owner ] [--size ] [--min-disk ] [--min-ram ] @@ -33,7 +32,7 @@ Create/upload an image [--public | --private] [--property [...] ] [--tag [...] ] - [--project-domain ] + [--project [--project-domain ]] .. option:: --id @@ -54,10 +53,6 @@ Create/upload an image Image disk format (default: raw) -.. option:: --owner - - Image owner project name or ID - .. option:: --size Image size, in bytes (only used with --location and --copy-from) @@ -128,11 +123,18 @@ Create/upload an image .. versionadded:: 2 +.. option:: --project + + Set an alternate project on this image (name or ID). + Previously known as `--owner`. + .. option:: --project-domain Domain the project belongs to (name or ID). This can be used in case collisions between project names exist. + .. versionadded:: 2 + .. describe:: New image name @@ -225,7 +227,6 @@ Set image properties os image set [--name ] - [--owner ] [--min-disk ] [--min-ram ] [--container-format ] @@ -250,17 +251,13 @@ Set image properties [--os-version ] [--ramdisk-id ] [--activate|--deactivate] - [--project-domain ] + [--project [--project-domain ]] .. option:: --name New image name -.. option:: --owner - - New image owner project (name or ID) - .. option:: --min-disk Minimum disk size needed to boot image, in gigabytes @@ -407,11 +404,18 @@ Set image properties .. versionadded:: 2 +.. option:: --project + + Set an alternate project on this image (name or ID). + Previously known as `--owner`. + .. option:: --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 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="", - help="Image owner project name or ID", - ) parser.add_argument( "--size", metavar="", @@ -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="", + help="Set an alternate project on this image (name or ID)", + ) + project_group.add_argument( + "--owner", + metavar="", + 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="", help="New image name", ) - parser.add_argument( - "--owner", - metavar="", - help="New image owner project (name or ID)", - ) parser.add_argument( "--min-disk", metavar="", @@ -590,12 +607,32 @@ class SetImage(command.Command): metavar="", 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="", + help="Set an alternate project on this image (name or ID)", + ) + project_group.add_argument( + "--owner", + metavar="", + 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="", - help="Image owner project name or ID", - ) parser.add_argument( "--min-disk", metavar="", @@ -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="", + help="Set an alternate project on this image (name or ID)", + ) + project_group.add_argument( + "--owner", + metavar="", + 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="", help="New image name" ) - parser.add_argument( - "--owner", - metavar="", - 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="", + help="Set an alternate project on this image (name or ID)", + ) + project_group.add_argument( + "--owner", + metavar="", + 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 `_]