From c128ae19694eb3b4871481ec180bca8c8467f6a1 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 13 Feb 2024 12:08:08 +0000 Subject: [PATCH] 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 --- openstackclient/compute/v2/server.py | 19 ++++++---- openstackclient/compute/v2/server_backup.py | 4 +- openstackclient/compute/v2/server_group.py | 8 +++- .../compute/v2/server_migration.py | 7 ++-- openstackclient/image/v1/image.py | 37 ++++++++++++++++--- openstackclient/image/v2/image.py | 18 ++++----- openstackclient/network/v2/network_flavor.py | 2 +- .../tests/unit/compute/v2/test_server.py | 6 ++- .../unit/compute/v2/test_server_group.py | 10 ++--- .../unit/compute/v2/test_server_migration.py | 4 +- .../tests/unit/image/v1/test_image.py | 4 +- openstackclient/volume/v2/volume_backup.py | 6 +-- 12 files changed, 82 insertions(+), 43 deletions(-) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index b34e20f229..d5d60c4b13 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -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, diff --git a/openstackclient/compute/v2/server_backup.py b/openstackclient/compute/v2/server_backup.py index c88a0b6ca4..ae21547563 100644 --- a/openstackclient/compute/v2/server_backup.py +++ b/openstackclient/compute/v2/server_backup.py @@ -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: diff --git a/openstackclient/compute/v2/server_group.py b/openstackclient/compute/v2/server_group.py index e5a1a4cead..046057600c 100644 --- a/openstackclient/compute/v2/server_group.py +++ b/openstackclient/compute/v2/server_group.py @@ -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, diff --git a/openstackclient/compute/v2/server_migration.py b/openstackclient/compute/v2/server_migration.py index 8a4440d6ab..307303793e 100644 --- a/openstackclient/compute/v2/server_migration.py +++ b/openstackclient/compute/v2/server_migration.py @@ -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: diff --git a/openstackclient/image/v1/image.py b/openstackclient/image/v1/image.py index 9edec48d1c..10f997cd11 100644 --- a/openstackclient/image/v1/image.py +++ b/openstackclient/image/v1/image.py @@ -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) - image_client.delete_image(image_obj.id) + 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 diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py index 2cf0e40ea4..46d374e9ec 100644 --- a/openstackclient/image/v2/image.py +++ b/openstackclient/image/v2/image.py @@ -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 = _( diff --git a/openstackclient/network/v2/network_flavor.py b/openstackclient/network/v2/network_flavor.py index 7c088d5095..2f308bed4d 100644 --- a/openstackclient/network/v2/network_flavor.py +++ b/openstackclient/network/v2/network_flavor.py @@ -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, } diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index ea944f9b33..fb7715a70c 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -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): diff --git a/openstackclient/tests/unit/compute/v2/test_server_group.py b/openstackclient/tests/unit/compute/v2/test_server_group.py index 3356218ecc..ab27288ea6 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_group.py +++ b/openstackclient/tests/unit/compute/v2/test_server_group.py @@ -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 diff --git a/openstackclient/tests/unit/compute/v2/test_server_migration.py b/openstackclient/tests/unit/compute/v2/test_server_migration.py index 8cb4df4fe1..cff5917d10 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_migration.py +++ b/openstackclient/tests/unit/compute/v2/test_server_migration.py @@ -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) diff --git a/openstackclient/tests/unit/image/v1/test_image.py b/openstackclient/tests/unit/image/v1/test_image.py index 2edbe1e2af..bf62d10c29 100644 --- a/openstackclient/tests/unit/image/v1/test_image.py +++ b/openstackclient/tests/unit/image/v1/test_image.py @@ -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') diff --git a/openstackclient/volume/v2/volume_backup.py b/openstackclient/volume/v2/volume_backup.py index 72e8871565..64c7865129 100644 --- a/openstackclient/volume/v2/volume_backup.py +++ b/openstackclient/volume/v2/volume_backup.py @@ -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: