Clean up Image v2 image set command
Make the Image v2 image set command meet at the intersection of the v1 image set command and the v2 image create command: * Add visibility to the deadopts list and remove the option * Put the options in the same order as v1 image set * Make the help text match * Add --properties * Move the additional options that do not appear in either v1 image set or v2 image create after --property as they are really pre-defined properties * Add tests for v2 image set to match v1 and then some * Put the SetImage class in v2/image.py in alphabetical order Change-Id: I102b914e8ad09a014f6fdd846c5766b6c2eaadb8
This commit is contained in:
Dean Troyer
lin-hua-cheng
parent
c71c78df92
commit
201b1cee86
@ -238,6 +238,12 @@ Set image properties
|
||||
[--checksum <checksum>]
|
||||
[--stdin]
|
||||
[--property <key=value> [...] ]
|
||||
[--architecture <architecture>]
|
||||
[--instance-id <instance-id>]
|
||||
[--kernel-id <kernel-id>]
|
||||
[--os-distro <os-distro>]
|
||||
[--os-version <os-version>]
|
||||
[--ramdisk-id <ramdisk-id>]
|
||||
<image>
|
||||
|
||||
.. option:: --name <name>
|
||||
@ -258,14 +264,11 @@ Set image properties
|
||||
|
||||
.. option:: --container-format <container-format>
|
||||
|
||||
Container format of image.
|
||||
Acceptable formats: ['ami', 'ari', 'aki', 'bare', 'ovf']
|
||||
Image container format (default: bare)
|
||||
|
||||
.. option:: --disk-format <disk-format>
|
||||
|
||||
Disk format of image.
|
||||
Acceptable formats: ['ami', 'ari', 'aki', 'vhd', 'vmdk', 'raw', 'qcow2',
|
||||
'vdi', 'iso']
|
||||
Image disk format (default: raw)
|
||||
|
||||
.. option:: --size <size>
|
||||
|
||||
@ -339,45 +342,41 @@ Set image properties
|
||||
|
||||
.. option:: --property <key=value>
|
||||
|
||||
Set a property on this image (repeat for multiple values)
|
||||
|
||||
*Image version 1 only.*
|
||||
Set a property on this image (repeat option to set multiple properties)
|
||||
|
||||
.. option:: --architecture <architecture>
|
||||
|
||||
Operating system Architecture
|
||||
Operating system architecture
|
||||
|
||||
.. versionadded:: 2
|
||||
|
||||
.. option:: --ramdisk-id <ramdisk-id>
|
||||
.. option:: --instance-id <instance-id>
|
||||
|
||||
ID of image stored in Glance that should be used as
|
||||
the ramdisk when booting an AMI-style image
|
||||
|
||||
.. versionadded:: 2
|
||||
|
||||
.. option:: --os-distro <os-distro>
|
||||
|
||||
Common name of operating system distribution
|
||||
|
||||
.. versionadded:: 2
|
||||
|
||||
.. option:: --os-version <os-version>
|
||||
|
||||
Operating system version as specified by the distributor
|
||||
ID of server instance used to create this image
|
||||
|
||||
.. versionadded:: 2
|
||||
|
||||
.. option:: --kernel-id <kernel-id>
|
||||
|
||||
ID of image in Glance that should be used as the
|
||||
kernel when booting an AMI-style image
|
||||
ID of kernel image used to boot this disk image
|
||||
|
||||
.. versionadded:: 2
|
||||
|
||||
.. option:: --instance-uuid <instance_uuid>
|
||||
.. option:: --os-distro <os-distro>
|
||||
|
||||
ID of instance used to create this image
|
||||
Operating system distribution name
|
||||
|
||||
.. versionadded:: 2
|
||||
|
||||
.. option:: --os-version <os-version>
|
||||
|
||||
Operating system distribution version
|
||||
|
||||
.. versionadded:: 2
|
||||
|
||||
.. option:: --ramdisk-id <ramdisk-id>
|
||||
|
||||
ID of ramdisk image used to boot this disk image
|
||||
|
||||
.. versionadded:: 2
|
||||
|
||||
|
||||
@ -210,7 +210,7 @@ class CreateImage(show.ShowOne):
|
||||
"--%s" % deadopt,
|
||||
metavar="<%s>" % deadopt,
|
||||
dest=deadopt.replace('-', '_'),
|
||||
help=argparse.SUPPRESS
|
||||
help=argparse.SUPPRESS,
|
||||
)
|
||||
return parser
|
||||
|
||||
@ -522,38 +522,11 @@ class SaveImage(command.Command):
|
||||
gc_utils.save_image(data, parsed_args.file)
|
||||
|
||||
|
||||
class ShowImage(show.ShowOne):
|
||||
"""Display image details"""
|
||||
|
||||
log = logging.getLogger(__name__ + ".ShowImage")
|
||||
|
||||
def get_parser(self, prog_name):
|
||||
parser = super(ShowImage, self).get_parser(prog_name)
|
||||
parser.add_argument(
|
||||
"image",
|
||||
metavar="<image>",
|
||||
help="Image to display (name or ID)",
|
||||
)
|
||||
return parser
|
||||
|
||||
def take_action(self, parsed_args):
|
||||
self.log.debug("take_action(%s)", parsed_args)
|
||||
|
||||
image_client = self.app.client_manager.image
|
||||
image = utils.find_resource(
|
||||
image_client.images,
|
||||
parsed_args.image,
|
||||
)
|
||||
|
||||
info = _format_image(image)
|
||||
return zip(*sorted(six.iteritems(info)))
|
||||
|
||||
|
||||
class SetImage(show.ShowOne):
|
||||
"""Set image properties"""
|
||||
|
||||
log = logging.getLogger(__name__ + ".SetImage")
|
||||
deadopts = ('size', 'store', 'location', 'copy-from', 'checksum')
|
||||
deadopts = ('visibility',)
|
||||
|
||||
def get_parser(self, prog_name):
|
||||
parser = super(SetImage, self).get_parser(prog_name)
|
||||
@ -567,7 +540,6 @@ class SetImage(show.ShowOne):
|
||||
# --force - needs adding
|
||||
# --checksum - maybe could be done client side
|
||||
# --stdin - could be implemented
|
||||
# --property - needs adding
|
||||
# --tags - needs adding
|
||||
parser.add_argument(
|
||||
"image",
|
||||
@ -580,20 +552,44 @@ class SetImage(show.ShowOne):
|
||||
help="New image name"
|
||||
)
|
||||
parser.add_argument(
|
||||
"--architecture",
|
||||
metavar="<architecture>",
|
||||
help="Operating system Architecture"
|
||||
"--owner",
|
||||
metavar="<project>",
|
||||
help="New image owner project (name or ID)",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--min-disk",
|
||||
type=int,
|
||||
metavar="<disk-gb>",
|
||||
help="Minimum disk size needed to boot image, in gigabytes"
|
||||
)
|
||||
parser.add_argument(
|
||||
"--min-ram",
|
||||
type=int,
|
||||
metavar="<ram-mb>",
|
||||
help="Minimum RAM size needed to boot image, in megabytes",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--container-format",
|
||||
metavar="<container-format>",
|
||||
help="Image container format "
|
||||
"(default: %s)" % DEFAULT_CONTAINER_FORMAT,
|
||||
)
|
||||
parser.add_argument(
|
||||
"--disk-format",
|
||||
metavar="<disk-format>",
|
||||
help="Image disk format "
|
||||
"(default: %s)" % DEFAULT_DISK_FORMAT,
|
||||
)
|
||||
protected_group = parser.add_mutually_exclusive_group()
|
||||
protected_group.add_argument(
|
||||
"--protected",
|
||||
action="store_true",
|
||||
help="Prevent image from being deleted"
|
||||
help="Prevent image from being deleted",
|
||||
)
|
||||
protected_group.add_argument(
|
||||
"--unprotected",
|
||||
action="store_true",
|
||||
help="Allow image to be deleted (default)"
|
||||
help="Allow image to be deleted (default)",
|
||||
)
|
||||
public_group = parser.add_mutually_exclusive_group()
|
||||
public_group.add_argument(
|
||||
@ -607,82 +603,55 @@ class SetImage(show.ShowOne):
|
||||
help="Image is inaccessible to the public (default)",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--instance-uuid",
|
||||
metavar="<instance_uuid>",
|
||||
help="ID of instance used to create this image"
|
||||
"--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(
|
||||
"--min-disk",
|
||||
type=int,
|
||||
metavar="<disk-gb>",
|
||||
help="Minimum disk size needed to boot image, in gigabytes"
|
||||
"--architecture",
|
||||
metavar="<architecture>",
|
||||
help="Operating system architecture",
|
||||
)
|
||||
visibility_choices = ["public", "private"]
|
||||
public_group.add_argument(
|
||||
"--visibility",
|
||||
metavar="<visibility>",
|
||||
choices=visibility_choices,
|
||||
help=argparse.SUPPRESS
|
||||
parser.add_argument(
|
||||
"--instance-id",
|
||||
metavar="<instance-id>",
|
||||
help="ID of server instance used to create this image",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--instance-uuid",
|
||||
metavar="<instance-id>",
|
||||
dest="instance_id",
|
||||
help=argparse.SUPPRESS,
|
||||
)
|
||||
help_msg = ("ID of image in Glance that should be used as the kernel"
|
||||
" when booting an AMI-style image")
|
||||
parser.add_argument(
|
||||
"--kernel-id",
|
||||
metavar="<kernel-id>",
|
||||
help=help_msg
|
||||
)
|
||||
parser.add_argument(
|
||||
"--os-version",
|
||||
metavar="<os-version>",
|
||||
help="Operating system version as specified by the distributor"
|
||||
)
|
||||
disk_choices = ["None", "ami", "ari", "aki", "vhd", "vmdk", "raw",
|
||||
"qcow2", "vdi", "iso"]
|
||||
help_msg = ("Format of the disk. Valid values: %s" % disk_choices)
|
||||
parser.add_argument(
|
||||
"--disk-format",
|
||||
metavar="<disk-format>",
|
||||
choices=disk_choices,
|
||||
help=help_msg
|
||||
help="ID of kernel image used to boot this disk image",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--os-distro",
|
||||
metavar="<os-distro>",
|
||||
help="Common name of operating system distribution"
|
||||
help="Operating system distribution name",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--owner",
|
||||
metavar="<owner>",
|
||||
help="New Owner of the image"
|
||||
"--os-version",
|
||||
metavar="<os-version>",
|
||||
help="Operating system distribution version",
|
||||
)
|
||||
msg = ("ID of image stored in Glance that should be used as the "
|
||||
"ramdisk when booting an AMI-style image")
|
||||
parser.add_argument(
|
||||
"--ramdisk-id",
|
||||
metavar="<ramdisk-id>",
|
||||
help=msg
|
||||
)
|
||||
parser.add_argument(
|
||||
"--min-ram",
|
||||
type=int,
|
||||
metavar="<ram-mb>",
|
||||
help="Amount of RAM (in MB) required to boot image"
|
||||
)
|
||||
container_choices = ["None", "ami", "ari", "aki", "bare", "ovf", "ova"]
|
||||
help_msg = ("Format of the container. Valid values: %s"
|
||||
% container_choices)
|
||||
parser.add_argument(
|
||||
"--container-format",
|
||||
metavar="<container-format>",
|
||||
choices=container_choices,
|
||||
help=help_msg
|
||||
help="ID of ramdisk image used to boot this disk image",
|
||||
)
|
||||
for deadopt in self.deadopts:
|
||||
parser.add_argument(
|
||||
"--%s" % deadopt,
|
||||
metavar="<%s>" % deadopt,
|
||||
dest=deadopt.replace('-', '_'),
|
||||
help=argparse.SUPPRESS
|
||||
help=argparse.SUPPRESS,
|
||||
)
|
||||
return parser
|
||||
|
||||
@ -698,10 +667,9 @@ class SetImage(show.ShowOne):
|
||||
|
||||
kwargs = {}
|
||||
copy_attrs = ('architecture', 'container_format', 'disk_format',
|
||||
'file', 'kernel_id', 'locations', 'name',
|
||||
'file', 'instance_id', 'kernel_id', 'locations',
|
||||
'min_disk', 'min_ram', 'name', 'os_distro', 'os_version',
|
||||
'owner', 'prefix', 'progress', 'ramdisk_id',
|
||||
'visibility')
|
||||
'owner', 'prefix', 'progress', 'ramdisk_id')
|
||||
for attr in copy_attrs:
|
||||
if attr in parsed_args:
|
||||
val = getattr(parsed_args, attr, None)
|
||||
@ -710,6 +678,11 @@ class SetImage(show.ShowOne):
|
||||
# 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
|
||||
@ -736,3 +709,30 @@ class SetImage(show.ShowOne):
|
||||
info = {}
|
||||
info.update(image)
|
||||
return zip(*sorted(six.iteritems(info)))
|
||||
|
||||
|
||||
class ShowImage(show.ShowOne):
|
||||
"""Display image details"""
|
||||
|
||||
log = logging.getLogger(__name__ + ".ShowImage")
|
||||
|
||||
def get_parser(self, prog_name):
|
||||
parser = super(ShowImage, self).get_parser(prog_name)
|
||||
parser.add_argument(
|
||||
"image",
|
||||
metavar="<image>",
|
||||
help="Image to display (name or ID)",
|
||||
)
|
||||
return parser
|
||||
|
||||
def take_action(self, parsed_args):
|
||||
self.log.debug("take_action(%s)", parsed_args)
|
||||
|
||||
image_client = self.app.client_manager.image
|
||||
image = utils.find_resource(
|
||||
image_client.images,
|
||||
parsed_args.image,
|
||||
)
|
||||
|
||||
info = _format_image(image)
|
||||
return zip(*sorted(six.iteritems(info)))
|
||||
|
||||
@ -210,7 +210,7 @@ class TestImageCreate(TestImage):
|
||||
self.assertEqual(image_fakes.IMAGE_columns, columns)
|
||||
self.assertEqual(image_fakes.IMAGE_data, data)
|
||||
|
||||
def test_image_dead_options(self):
|
||||
def test_image_create_dead_options(self):
|
||||
|
||||
arglist = [
|
||||
'--owner', 'nobody',
|
||||
@ -639,6 +639,196 @@ class TestRemoveProjectImage(TestImage):
|
||||
)
|
||||
|
||||
|
||||
class TestImageSet(TestImage):
|
||||
|
||||
def setUp(self):
|
||||
super(TestImageSet, self).setUp()
|
||||
# Set up the schema
|
||||
self.model = warlock.model_factory(
|
||||
image_fakes.IMAGE_schema,
|
||||
schemas.SchemaBasedModel,
|
||||
)
|
||||
|
||||
self.images_mock.get.return_value = self.model(**image_fakes.IMAGE)
|
||||
self.images_mock.update.return_value = self.model(**image_fakes.IMAGE)
|
||||
# Get the command object to test
|
||||
self.cmd = image.SetImage(self.app, None)
|
||||
|
||||
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',
|
||||
image_fakes.image_id,
|
||||
]
|
||||
verifylist = [
|
||||
('name', 'new-name'),
|
||||
('owner', 'new-owner'),
|
||||
('min_disk', 2),
|
||||
('min_ram', 4),
|
||||
('container_format', 'ovf'),
|
||||
('disk_format', 'vmdk'),
|
||||
('image', image_fakes.image_id),
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
# DisplayCommandBase.take_action() returns two tuples
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
|
||||
kwargs = {
|
||||
'name': 'new-name',
|
||||
'owner': 'new-owner',
|
||||
'min_disk': 2,
|
||||
'min_ram': 4,
|
||||
'container_format': 'ovf',
|
||||
'disk_format': 'vmdk',
|
||||
}
|
||||
# ImageManager.update(image, **kwargs)
|
||||
self.images_mock.update.assert_called_with(
|
||||
image_fakes.image_id, **kwargs)
|
||||
|
||||
self.assertEqual(image_fakes.IMAGE_columns, columns)
|
||||
self.assertEqual(image_fakes.IMAGE_data, data)
|
||||
|
||||
def test_image_set_bools1(self):
|
||||
arglist = [
|
||||
'--protected',
|
||||
'--private',
|
||||
image_fakes.image_name,
|
||||
]
|
||||
verifylist = [
|
||||
('protected', True),
|
||||
('unprotected', False),
|
||||
('public', False),
|
||||
('private', True),
|
||||
('image', image_fakes.image_name),
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
# DisplayCommandBase.take_action() returns two tuples
|
||||
self.cmd.take_action(parsed_args)
|
||||
|
||||
kwargs = {
|
||||
'protected': True,
|
||||
'visibility': 'private',
|
||||
}
|
||||
# ImageManager.update(image, **kwargs)
|
||||
self.images_mock.update.assert_called_with(
|
||||
image_fakes.image_id,
|
||||
**kwargs
|
||||
)
|
||||
|
||||
def test_image_set_bools2(self):
|
||||
arglist = [
|
||||
'--unprotected',
|
||||
'--public',
|
||||
image_fakes.image_name,
|
||||
]
|
||||
verifylist = [
|
||||
('protected', False),
|
||||
('unprotected', True),
|
||||
('public', True),
|
||||
('private', False),
|
||||
('image', image_fakes.image_name),
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
# DisplayCommandBase.take_action() returns two tuples
|
||||
self.cmd.take_action(parsed_args)
|
||||
|
||||
kwargs = {
|
||||
'protected': False,
|
||||
'visibility': 'public',
|
||||
}
|
||||
# ImageManager.update(image, **kwargs)
|
||||
self.images_mock.update.assert_called_with(
|
||||
image_fakes.image_id,
|
||||
**kwargs
|
||||
)
|
||||
|
||||
def test_image_set_properties(self):
|
||||
arglist = [
|
||||
'--property', 'Alpha=1',
|
||||
'--property', 'Beta=2',
|
||||
image_fakes.image_name,
|
||||
]
|
||||
verifylist = [
|
||||
('properties', {'Alpha': '1', 'Beta': '2'}),
|
||||
('image', image_fakes.image_name),
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
# DisplayCommandBase.take_action() returns two tuples
|
||||
self.cmd.take_action(parsed_args)
|
||||
|
||||
kwargs = {
|
||||
'Alpha': '1',
|
||||
'Beta': '2',
|
||||
}
|
||||
# ImageManager.update(image, **kwargs)
|
||||
self.images_mock.update.assert_called_with(
|
||||
image_fakes.image_id,
|
||||
**kwargs
|
||||
)
|
||||
|
||||
def test_image_set_fake_properties(self):
|
||||
arglist = [
|
||||
'--architecture', 'z80',
|
||||
'--instance-id', '12345',
|
||||
'--kernel-id', '67890',
|
||||
'--os-distro', 'cpm',
|
||||
'--os-version', '2.2H',
|
||||
'--ramdisk-id', 'xyzpdq',
|
||||
image_fakes.image_name,
|
||||
]
|
||||
verifylist = [
|
||||
('architecture', 'z80'),
|
||||
('instance_id', '12345'),
|
||||
('kernel_id', '67890'),
|
||||
('os_distro', 'cpm'),
|
||||
('os_version', '2.2H'),
|
||||
('ramdisk_id', 'xyzpdq'),
|
||||
('image', image_fakes.image_name),
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
# DisplayCommandBase.take_action() returns two tuples
|
||||
self.cmd.take_action(parsed_args)
|
||||
|
||||
kwargs = {
|
||||
'architecture': 'z80',
|
||||
'instance_id': '12345',
|
||||
'kernel_id': '67890',
|
||||
'os_distro': 'cpm',
|
||||
'os_version': '2.2H',
|
||||
'ramdisk_id': 'xyzpdq',
|
||||
}
|
||||
# ImageManager.update(image, **kwargs)
|
||||
self.images_mock.update.assert_called_with(
|
||||
image_fakes.image_id,
|
||||
**kwargs
|
||||
)
|
||||
|
||||
def test_image_set_dead_options(self):
|
||||
|
||||
arglist = [
|
||||
'--visibility', '1-mile',
|
||||
image_fakes.image_name,
|
||||
]
|
||||
verifylist = [
|
||||
('visibility', '1-mile'),
|
||||
('image', image_fakes.image_name),
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
self.assertRaises(
|
||||
exceptions.CommandError,
|
||||
self.cmd.take_action, parsed_args)
|
||||
|
||||
|
||||
class TestImageShow(TestImage):
|
||||
|
||||
def setUp(self):
|
||||
@ -672,52 +862,3 @@ class TestImageShow(TestImage):
|
||||
|
||||
self.assertEqual(image_fakes.IMAGE_columns, columns)
|
||||
self.assertEqual(image_fakes.IMAGE_data, data)
|
||||
|
||||
|
||||
class TestImageSet(TestImage):
|
||||
|
||||
def setUp(self):
|
||||
super(TestImageSet, self).setUp()
|
||||
# Set up the schema
|
||||
self.model = warlock.model_factory(
|
||||
image_fakes.IMAGE_schema,
|
||||
schemas.SchemaBasedModel,
|
||||
)
|
||||
|
||||
self.images_mock.get.return_value = self.model(**image_fakes.IMAGE)
|
||||
self.images_mock.update.return_value = self.model(**image_fakes.IMAGE)
|
||||
# Get the command object to test
|
||||
self.cmd = image.SetImage(self.app, None)
|
||||
|
||||
def test_image_set_options(self):
|
||||
arglist = [
|
||||
'--name', 'new-name',
|
||||
'--owner', 'new-owner',
|
||||
'--min-disk', '2',
|
||||
'--min-ram', '4',
|
||||
image_fakes.image_id,
|
||||
]
|
||||
verifylist = [
|
||||
('name', 'new-name'),
|
||||
('owner', 'new-owner'),
|
||||
('min_disk', 2),
|
||||
('min_ram', 4),
|
||||
('image', image_fakes.image_id),
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
# DisplayCommandBase.take_action() returns two tuples
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
|
||||
kwargs = {
|
||||
'name': 'new-name',
|
||||
'owner': 'new-owner',
|
||||
'min_disk': 2,
|
||||
'min_ram': 4
|
||||
}
|
||||
# ImageManager.update(image, **kwargs)
|
||||
self.images_mock.update.assert_called_with(
|
||||
image_fakes.image_id, **kwargs)
|
||||
|
||||
self.assertEqual(image_fakes.IMAGE_columns, columns)
|
||||
self.assertEqual(image_fakes.IMAGE_data, data)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user