From cebf4d78d6ed251727dd1ebef3d00dd5a2c90f85 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 Signed-off-by: Eric Harney Co-authored-by: Stephen Finucane --- .../tests/unit/volume/v2/test_volume.py | 10 ++--- .../tests/unit/volume/v3/test_volume.py | 43 ++++++++++++++----- openstackclient/volume/v2/volume.py | 11 ++++- openstackclient/volume/v3/volume.py | 15 +++++-- ...olume-delete-cascade-384003efc8896096.yaml | 6 +++ 5 files changed, 63 insertions(+), 22 deletions(-) create mode 100644 releasenotes/notes/volume-delete-cascade-384003efc8896096.yaml diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py index b68020fa95..1d4be1c399 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume.py +++ b/openstackclient/tests/unit/volume/v2/test_volume.py @@ -655,7 +655,7 @@ class TestVolumeDelete(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) @@ -674,7 +674,7 @@ class TestVolumeDelete(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) @@ -701,7 +701,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): ] verifylist = [ ('force', False), - ('purge', False), + ('cascade', False), ('volumes', [self.volumes[0].id, 'unexist_volume']), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -732,7 +732,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): ] verifylist = [ ('force', False), - ('purge', True), + ('cascade', True), ('volumes', [self.volumes[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -754,7 +754,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): ] verifylist = [ ('force', True), - ('purge', False), + ('cascade', False), ('volumes', [self.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 33dcfe5a47..1669aaf8a0 100644 --- a/openstackclient/tests/unit/volume/v3/test_volume.py +++ b/openstackclient/tests/unit/volume/v3/test_volume.py @@ -912,7 +912,7 @@ class TestVolumeDelete(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) @@ -931,7 +931,7 @@ class TestVolumeDelete(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) @@ -958,7 +958,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): ] verifylist = [ ('force', False), - ('purge', False), + ('cascade', False), ('volumes', [self.volumes[0].id, 'unexist_volume']), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -989,7 +989,29 @@ class TestVolumeDelete(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.assertIsNone(result) + + self.volume_sdk_client.find_volume.assert_called_once_with( + self.volumes[0].id, ignore_missing=False + ) + self.volume_sdk_client.delete_volume.assert_called_once_with( + self.volumes[0].id, cascade=True, force=False + ) + + 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) @@ -1011,7 +1033,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): ] verifylist = [ ('force', True), - ('purge', False), + ('cascade', False), ('volumes', [self.volumes[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1031,7 +1053,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): verifylist = [ ("remote", True), ("force", False), - ("purge", False), + ("cascade", False), ("volumes", [self.volumes[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1052,7 +1074,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) @@ -1077,7 +1099,6 @@ class TestVolumeDelete(volume_fakes.TestVolume): verifylist = [ ('remote', True), ('force', False), - ('purge', True), ('volumes', [self.volumes[0].id]), ] @@ -1086,7 +1107,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), ) @@ -1104,7 +1125,7 @@ class TestVolumeDelete(volume_fakes.TestVolume): verifylist = [ ('remote', True), ('force', True), - ('purge', False), + ('cascade', False), ('volumes', [self.volumes[0].id]), ] @@ -1113,7 +1134,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 61cce04f7b..4dd1434f3c 100644 --- a/openstackclient/volume/v2/volume.py +++ b/openstackclient/volume/v2/volume.py @@ -390,12 +390,19 @@ 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): @@ -410,7 +417,7 @@ class DeleteVolume(command.Command): volume_client.delete_volume( volume_obj.id, force=parsed_args.force, - cascade=parsed_args.purge, + 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 50ea77fb5a..77dab293bd 100644 --- a/openstackclient/volume/v3/volume.py +++ b/openstackclient/volume/v3/volume.py @@ -515,12 +515,19 @@ 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', + ) parser.add_argument( '--remote', action='store_true', @@ -532,9 +539,9 @@ class DeleteVolume(command.Command): volume_client = 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) @@ -550,7 +557,7 @@ class DeleteVolume(command.Command): volume_client.delete_volume( volume_obj.id, force=parsed_args.force, - cascade=parsed_args.purge, + cascade=parsed_args.cascade, ) except Exception as e: result += 1 diff --git a/releasenotes/notes/volume-delete-cascade-384003efc8896096.yaml b/releasenotes/notes/volume-delete-cascade-384003efc8896096.yaml new file mode 100644 index 0000000000..679964f1bc --- /dev/null +++ b/releasenotes/notes/volume-delete-cascade-384003efc8896096.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + The ``--purge`` argument to the ``volume delete`` command has been renamed + to ``--cascade`` to better match the Cinder API and the meaning of what + this argument does. An alias is provided for backwards compatibility.