trivial: Don't ignore missing resources
An openstacksdk 'find_foo' proxy method will return None by default if a resource is not found. You can change this behavior by setting 'ignore_missing=False'. We were doing this in most, but not all cases: correct the issue. In the event of calling 'image delete' with multiple images, it will no longer fail on the first missing image and will instead attempt to delete remaining images before failing. Change-Id: I1e01d3c096dcaab731c28e496a182dd911229227 Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
parent
99c7f583df
commit
c128ae1969
@ -2555,10 +2555,9 @@ class ListServer(command.Lister):
|
||||
# flavor name is given, map it to ID.
|
||||
flavor_id = None
|
||||
if parsed_args.flavor:
|
||||
flavor = compute_client.find_flavor(parsed_args.flavor)
|
||||
if flavor is None:
|
||||
msg = _('Unable to find flavor: %s') % parsed_args.flavor
|
||||
raise exceptions.CommandError(msg)
|
||||
flavor = compute_client.find_flavor(
|
||||
parsed_args.flavor, ignore_missing=False
|
||||
)
|
||||
flavor_id = flavor.id
|
||||
|
||||
# Nova only supports list servers searching by image ID. So if a
|
||||
@ -2811,7 +2810,9 @@ class ListServer(command.Lister):
|
||||
if parsed_args.deleted:
|
||||
marker_id = parsed_args.marker
|
||||
else:
|
||||
marker_id = compute_client.find_server(parsed_args.marker).id
|
||||
marker_id = compute_client.find_server(
|
||||
parsed_args.marker, ignore_missing=False
|
||||
).id
|
||||
search_opts['marker'] = marker_id
|
||||
|
||||
data = list(compute_client.servers(**search_opts))
|
||||
@ -2871,7 +2872,9 @@ class ListServer(command.Lister):
|
||||
# "Flavor Name" is not crucial, so we swallow any
|
||||
# exceptions
|
||||
try:
|
||||
flavors[f_id] = compute_client.find_flavor(f_id)
|
||||
flavors[f_id] = compute_client.find_flavor(
|
||||
f_id, ignore_missing=False
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
else:
|
||||
@ -4039,7 +4042,9 @@ server booted from a volume."""
|
||||
|
||||
image = None
|
||||
if parsed_args.image:
|
||||
image = image_client.find_image(parsed_args.image)
|
||||
image = image_client.find_image(
|
||||
parsed_args.image, ignore_missing=False
|
||||
)
|
||||
|
||||
utils.find_resource(
|
||||
compute_client.servers,
|
||||
|
@ -73,7 +73,9 @@ class CreateServerBackup(command.ShowOne):
|
||||
|
||||
compute_client = self.app.client_manager.sdk_connection.compute
|
||||
|
||||
server = compute_client.find_server(parsed_args.server)
|
||||
server = compute_client.find_server(
|
||||
parsed_args.server, ignore_missing=False
|
||||
)
|
||||
|
||||
# Set sane defaults as this API wants all mouths to be fed
|
||||
if parsed_args.name is None:
|
||||
|
@ -157,7 +157,9 @@ class DeleteServerGroup(command.Command):
|
||||
result = 0
|
||||
for group in parsed_args.server_group:
|
||||
try:
|
||||
group_obj = compute_client.find_server_group(group)
|
||||
group_obj = compute_client.find_server_group(
|
||||
group, ignore_missing=False
|
||||
)
|
||||
compute_client.delete_server_group(group_obj.id)
|
||||
# Catch all exceptions in order to avoid to block the next deleting
|
||||
except Exception as e:
|
||||
@ -263,7 +265,9 @@ class ShowServerGroup(command.ShowOne):
|
||||
|
||||
def take_action(self, parsed_args):
|
||||
compute_client = self.app.client_manager.sdk_connection.compute
|
||||
group = compute_client.find_server_group(parsed_args.server_group)
|
||||
group = compute_client.find_server_group(
|
||||
parsed_args.server_group, ignore_missing=False
|
||||
)
|
||||
display_columns, columns = _get_server_group_columns(
|
||||
group,
|
||||
compute_client,
|
||||
|
@ -166,10 +166,9 @@ class ListMigration(command.Lister):
|
||||
search_opts['status'] = parsed_args.status
|
||||
|
||||
if parsed_args.server:
|
||||
server = compute_client.find_server(parsed_args.server)
|
||||
if server is None:
|
||||
msg = _('Unable to find server: %s') % parsed_args.server
|
||||
raise exceptions.CommandError(msg)
|
||||
server = compute_client.find_server(
|
||||
parsed_args.server, ignore_missing=False
|
||||
)
|
||||
search_opts['instance_uuid'] = server.id
|
||||
|
||||
if parsed_args.type:
|
||||
|
@ -26,6 +26,7 @@ from osc_lib.api import utils as api_utils
|
||||
from osc_lib.cli import format_columns
|
||||
from osc_lib.cli import parseractions
|
||||
from osc_lib.command import command
|
||||
from osc_lib import exceptions
|
||||
from osc_lib import utils
|
||||
|
||||
from openstackclient.i18n import _
|
||||
@ -372,10 +373,30 @@ class DeleteImage(command.Command):
|
||||
return parser
|
||||
|
||||
def take_action(self, parsed_args):
|
||||
result = 0
|
||||
image_client = self.app.client_manager.image
|
||||
for image in parsed_args.images:
|
||||
image_obj = image_client.find_image(image)
|
||||
try:
|
||||
image_obj = image_client.find_image(
|
||||
image,
|
||||
ignore_missing=False,
|
||||
)
|
||||
image_client.delete_image(image_obj.id)
|
||||
except Exception as e:
|
||||
result += 1
|
||||
msg = _(
|
||||
"Failed to delete image with name or "
|
||||
"ID '%(image)s': %(e)s"
|
||||
)
|
||||
LOG.error(msg, {'image': image, 'e': e})
|
||||
|
||||
total = len(parsed_args.images)
|
||||
if result > 0:
|
||||
msg = _("Failed to delete %(result)s of %(total)s images.") % {
|
||||
'result': result,
|
||||
'total': total,
|
||||
}
|
||||
raise exceptions.CommandError(msg)
|
||||
|
||||
|
||||
class ListImage(command.Lister):
|
||||
@ -528,7 +549,9 @@ class SaveImage(command.Command):
|
||||
|
||||
def take_action(self, parsed_args):
|
||||
image_client = self.app.client_manager.image
|
||||
image = image_client.find_image(parsed_args.image)
|
||||
image = image_client.find_image(
|
||||
parsed_args.image, ignore_missing=False
|
||||
)
|
||||
|
||||
output_file = parsed_args.file
|
||||
if output_file is None:
|
||||
@ -719,7 +742,9 @@ class SetImage(command.Command):
|
||||
|
||||
# Wrap the call to catch exceptions in order to close files
|
||||
try:
|
||||
image = image_client.find_image(parsed_args.image)
|
||||
image = image_client.find_image(
|
||||
parsed_args.image, ignore_missing=False
|
||||
)
|
||||
|
||||
if not parsed_args.location and not parsed_args.copy_from:
|
||||
if parsed_args.volume:
|
||||
@ -800,7 +825,9 @@ class ShowImage(command.ShowOne):
|
||||
|
||||
def take_action(self, parsed_args):
|
||||
image_client = self.app.client_manager.image
|
||||
image = image_client.find_image(parsed_args.image)
|
||||
image = image_client.find_image(
|
||||
parsed_args.image, ignore_missing=False
|
||||
)
|
||||
|
||||
if parsed_args.human_readable:
|
||||
_formatters['size'] = HumanReadableSizeColumn
|
||||
|
@ -683,7 +683,7 @@ class DeleteImage(command.Command):
|
||||
return parser
|
||||
|
||||
def take_action(self, parsed_args):
|
||||
del_result = 0
|
||||
result = 0
|
||||
image_client = self.app.client_manager.image
|
||||
for image in parsed_args.images:
|
||||
try:
|
||||
@ -691,10 +691,6 @@ class DeleteImage(command.Command):
|
||||
image,
|
||||
ignore_missing=False,
|
||||
)
|
||||
except sdk_exceptions.ResourceNotFound as e:
|
||||
msg = _("Unable to process request: %(e)s") % {'e': e}
|
||||
raise exceptions.CommandError(msg)
|
||||
try:
|
||||
image_client.delete_image(
|
||||
image_obj.id,
|
||||
store=parsed_args.store,
|
||||
@ -704,7 +700,7 @@ class DeleteImage(command.Command):
|
||||
msg = _("Multi Backend support not enabled.")
|
||||
raise exceptions.CommandError(msg)
|
||||
except Exception as e:
|
||||
del_result += 1
|
||||
result += 1
|
||||
msg = _(
|
||||
"Failed to delete image with name or "
|
||||
"ID '%(image)s': %(e)s"
|
||||
@ -712,9 +708,9 @@ class DeleteImage(command.Command):
|
||||
LOG.error(msg, {'image': image, 'e': e})
|
||||
|
||||
total = len(parsed_args.images)
|
||||
if del_result > 0:
|
||||
msg = _("Failed to delete %(dresult)s of %(total)s images.") % {
|
||||
'dresult': del_result,
|
||||
if result > 0:
|
||||
msg = _("Failed to delete %(result)s of %(total)s images.") % {
|
||||
'result': result,
|
||||
'total': total,
|
||||
}
|
||||
raise exceptions.CommandError(msg)
|
||||
@ -1797,7 +1793,9 @@ class ImportImage(command.ShowOne):
|
||||
)
|
||||
raise exceptions.CommandError(msg)
|
||||
|
||||
image = image_client.find_image(parsed_args.image)
|
||||
image = image_client.find_image(
|
||||
parsed_args.image, ignore_missing=False
|
||||
)
|
||||
|
||||
if not image.container_format and not image.disk_format:
|
||||
msg = _(
|
||||
|
@ -175,7 +175,7 @@ class DeleteNetworkFlavor(command.Command):
|
||||
)
|
||||
if result > 0:
|
||||
total = len(parsed_args.flavor)
|
||||
msg = _("%(result)s of %(total)s flavors failed " "to delete.") % {
|
||||
msg = _("%(result)s of %(total)s flavors failed to delete.") % {
|
||||
"result": result,
|
||||
"total": total,
|
||||
}
|
||||
|
@ -4965,7 +4965,7 @@ class TestServerList(_TestServerList):
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
|
||||
self.compute_sdk_client.find_flavor.assert_has_calls(
|
||||
[mock.call(self.flavor.id)]
|
||||
[mock.call(self.flavor.id, ignore_missing=False)]
|
||||
)
|
||||
|
||||
self.kwargs['flavor'] = self.flavor.id
|
||||
@ -7119,7 +7119,9 @@ class TestServerRescue(TestServer):
|
||||
self.cmd.take_action(parsed_args)
|
||||
|
||||
self.servers_mock.get.assert_called_with(self.server.id)
|
||||
self.image_client.find_image.assert_called_with(new_image.id)
|
||||
self.image_client.find_image.assert_called_with(
|
||||
new_image.id, ignore_missing=False
|
||||
)
|
||||
self.server.rescue.assert_called_with(image=new_image, password=None)
|
||||
|
||||
def test_rescue_with_current_image_and_password(self):
|
||||
|
@ -188,7 +188,7 @@ class TestServerGroupDelete(TestServerGroup):
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
result = self.cmd.take_action(parsed_args)
|
||||
self.compute_sdk_client.find_server_group.assert_called_once_with(
|
||||
'affinity_group'
|
||||
'affinity_group', ignore_missing=False
|
||||
)
|
||||
self.compute_sdk_client.delete_server_group.assert_called_once_with(
|
||||
self.fake_server_group.id
|
||||
@ -203,10 +203,10 @@ class TestServerGroupDelete(TestServerGroup):
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
result = self.cmd.take_action(parsed_args)
|
||||
self.compute_sdk_client.find_server_group.assert_any_call(
|
||||
'affinity_group'
|
||||
'affinity_group', ignore_missing=False
|
||||
)
|
||||
self.compute_sdk_client.find_server_group.assert_any_call(
|
||||
'anti_affinity_group'
|
||||
'anti_affinity_group', ignore_missing=False
|
||||
)
|
||||
self.compute_sdk_client.delete_server_group.assert_called_with(
|
||||
self.fake_server_group.id
|
||||
@ -248,10 +248,10 @@ class TestServerGroupDelete(TestServerGroup):
|
||||
self.assertEqual('1 of 2 server groups failed to delete.', str(e))
|
||||
|
||||
self.compute_sdk_client.find_server_group.assert_any_call(
|
||||
'affinity_group'
|
||||
'affinity_group', ignore_missing=False
|
||||
)
|
||||
self.compute_sdk_client.find_server_group.assert_any_call(
|
||||
'anti_affinity_group'
|
||||
'anti_affinity_group', ignore_missing=False
|
||||
)
|
||||
self.assertEqual(
|
||||
2, self.compute_sdk_client.find_server_group.call_count
|
||||
|
@ -141,7 +141,9 @@ class TestListMigration(TestServerMigration):
|
||||
'migration_type': 'migration',
|
||||
}
|
||||
|
||||
self.compute_sdk_client.find_server.assert_called_with('server1')
|
||||
self.compute_sdk_client.find_server.assert_called_with(
|
||||
'server1', ignore_missing=False
|
||||
)
|
||||
self.compute_sdk_client.migrations.assert_called_with(**kwargs)
|
||||
|
||||
self.assertEqual(self.MIGRATION_COLUMNS, columns)
|
||||
|
@ -731,7 +731,7 @@ class TestImageShow(image_fakes.TestImagev1):
|
||||
# data to be shown.
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
self.image_client.find_image.assert_called_with(
|
||||
self._image.id,
|
||||
self._image.id, ignore_missing=False
|
||||
)
|
||||
|
||||
self.assertEqual(self.columns, columns)
|
||||
@ -753,7 +753,7 @@ class TestImageShow(image_fakes.TestImagev1):
|
||||
# data to be shown.
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
self.image_client.find_image.assert_called_with(
|
||||
self._image.id,
|
||||
self._image.id, ignore_missing=False
|
||||
)
|
||||
|
||||
size_index = columns.index('size')
|
||||
|
@ -203,10 +203,10 @@ class DeleteVolumeBackup(command.Command):
|
||||
volume_client = self.app.client_manager.sdk_connection.volume
|
||||
result = 0
|
||||
|
||||
for i in parsed_args.backups:
|
||||
for backup in parsed_args.backups:
|
||||
try:
|
||||
backup_id = volume_client.find_backup(
|
||||
i, ignore_missing=False
|
||||
backup, ignore_missing=False
|
||||
).id
|
||||
volume_client.delete_backup(
|
||||
backup_id,
|
||||
@ -220,7 +220,7 @@ class DeleteVolumeBackup(command.Command):
|
||||
"Failed to delete backup with "
|
||||
"name or ID '%(backup)s': %(e)s"
|
||||
)
|
||||
% {'backup': i, 'e': e}
|
||||
% {'backup': backup, 'e': e}
|
||||
)
|
||||
|
||||
if result > 0:
|
||||
|
Loading…
Reference in New Issue
Block a user