From 07bb75bec9a5b56ecf7e21c57dda9f364177553b Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Wed, 4 Dec 2024 18:51:17 +0000 Subject: [PATCH] Rename openstack volume delete --purge -> --cascade This flag is called "cascade" in the Cinder API. The flag "purge" doesn't really communicate an obvious meaning in the context of volume deletion. It also has the danger of implying some kind of behavior about volume wiping that does not exist. Rename this flag to "--cascade" and preserve "--purge" as a hidden flag for compatibility. Change-Id: I8de27811222c17155697073fb9c512746d009266 --- .../tests/unit/volume/v2/test_volume.py | 10 ++--- .../tests/unit/volume/v3/test_volume.py | 41 ++++++++++++++----- openstackclient/volume/v2/volume.py | 11 ++++- openstackclient/volume/v3/volume.py | 6 +-- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py index 1667f38cd6..8b424f7124 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume.py +++ b/openstackclient/tests/unit/volume/v2/test_volume.py @@ -685,7 +685,7 @@ class TestVolumeDelete(TestVolume): arglist = [volumes[0].id] verifylist = [ ("force", False), - ("purge", False), + ("cascade", False), ("volumes", [volumes[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -703,7 +703,7 @@ class TestVolumeDelete(TestVolume): arglist = [v.id for v in volumes] verifylist = [ ('force', False), - ('purge', False), + ('cascade', False), ('volumes', arglist), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -723,7 +723,7 @@ class TestVolumeDelete(TestVolume): ] verifylist = [ ('force', False), - ('purge', False), + ('cascade', False), ('volumes', arglist), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -755,7 +755,7 @@ class TestVolumeDelete(TestVolume): ] verifylist = [ ('force', False), - ('purge', True), + ('cascade', True), ('volumes', [volumes[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -776,7 +776,7 @@ class TestVolumeDelete(TestVolume): ] verifylist = [ ('force', True), - ('purge', False), + ('cascade', False), ('volumes', [volumes[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) diff --git a/openstackclient/tests/unit/volume/v3/test_volume.py b/openstackclient/tests/unit/volume/v3/test_volume.py index 076de3ac3b..28dee4f29b 100644 --- a/openstackclient/tests/unit/volume/v3/test_volume.py +++ b/openstackclient/tests/unit/volume/v3/test_volume.py @@ -980,7 +980,7 @@ class TestVolumeDeleteLegacy(volume_fakes.TestVolume): arglist = [self.volumes[0].id] verifylist = [ ("force", False), - ("purge", False), + ("cascade", False), ("volumes", [self.volumes[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -996,7 +996,7 @@ class TestVolumeDeleteLegacy(volume_fakes.TestVolume): arglist = [v.id for v in self.volumes] verifylist = [ ('force', False), - ('purge', False), + ('cascade', False), ('volumes', arglist), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1014,7 +1014,7 @@ class TestVolumeDeleteLegacy(volume_fakes.TestVolume): ] verifylist = [ ('force', False), - ('purge', False), + ('cascade', False), ('volumes', arglist), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1044,7 +1044,26 @@ class TestVolumeDeleteLegacy(volume_fakes.TestVolume): ] verifylist = [ ('force', False), - ('purge', True), + ('cascade', True), + ('volumes', [self.volumes[0].id]), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + self.volumes_mock.delete.assert_called_once_with( + self.volumes[0].id, cascade=True + ) + self.assertIsNone(result) + + def test_volume_delete_with_cascade(self): + arglist = [ + '--cascade', + self.volumes[0].id, + ] + verifylist = [ + ('force', False), + ('cascade', True), ('volumes', [self.volumes[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1063,7 +1082,7 @@ class TestVolumeDeleteLegacy(volume_fakes.TestVolume): ] verifylist = [ ('force', True), - ('purge', False), + ('cascade', False), ('volumes', [self.volumes[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1095,7 +1114,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): verifylist = [ ("remote", True), ("force", False), - ("purge", False), + ("cascade", False), ("volumes", [vol.id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1115,7 +1134,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): verifylist = [ ('remote', True), ('force', False), - ('purge', False), + ('cascade', False), ('volumes', arglist[1:]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1137,7 +1156,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): verifylist = [ ('remote', True), ('force', False), - ('purge', True), + ('cascade', True), ('volumes', [vol.id]), ] @@ -1146,7 +1165,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): exceptions.CommandError, self.cmd.take_action, parsed_args ) self.assertIn( - "The --force and --purge options are not supported with the " + "The --force and --cascade options are not supported with the " "--remote parameter.", str(exc), ) @@ -1162,7 +1181,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): verifylist = [ ('remote', True), ('force', True), - ('purge', False), + ('cascade', False), ('volumes', [vol.id]), ] @@ -1171,7 +1190,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): exceptions.CommandError, self.cmd.take_action, parsed_args ) self.assertIn( - "The --force and --purge options are not supported with the " + "The --force and --cascade options are not supported with the " "--remote parameter.", str(exc), ) diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py index 179da19601..eaeef61e5e 100644 --- a/openstackclient/volume/v2/volume.py +++ b/openstackclient/volume/v2/volume.py @@ -353,13 +353,20 @@ class DeleteVolume(command.Command): ), ) group.add_argument( - "--purge", + "--cascade", action="store_true", help=_( "Remove any snapshots along with volume(s) " "(defaults to False)" ), ) + group.add_argument( + # now called "cascade", accept old arg for compatibility + "--purge", + action="store_true", + help=argparse.SUPPRESS, + dest='cascade', + ) return parser def take_action(self, parsed_args): @@ -373,7 +380,7 @@ class DeleteVolume(command.Command): volume_client.volumes.force_delete(volume_obj.id) else: volume_client.volumes.delete( - volume_obj.id, cascade=parsed_args.purge + volume_obj.id, cascade=parsed_args.cascade ) except Exception as e: result += 1 diff --git a/openstackclient/volume/v3/volume.py b/openstackclient/volume/v3/volume.py index 6f705ad6f9..f46d3702ec 100644 --- a/openstackclient/volume/v3/volume.py +++ b/openstackclient/volume/v3/volume.py @@ -372,9 +372,9 @@ class DeleteVolume(volume_v2.DeleteVolume): volume_client_sdk = self.app.client_manager.sdk_connection.volume result = 0 - if parsed_args.remote and (parsed_args.force or parsed_args.purge): + if parsed_args.remote and (parsed_args.force or parsed_args.cascade): msg = _( - "The --force and --purge options are not " + "The --force and --cascade options are not " "supported with the --remote parameter." ) raise exceptions.CommandError(msg) @@ -388,7 +388,7 @@ class DeleteVolume(volume_v2.DeleteVolume): volume_client.volumes.force_delete(volume_obj.id) else: volume_client.volumes.delete( - volume_obj.id, cascade=parsed_args.purge + volume_obj.id, cascade=parsed_args.cascade ) except Exception as e: result += 1