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 <eharney@redhat.com>
Co-authored-by: Stephen Finucane <stephenfin@redhat.com>

Author: eharney

openstackclient/tests/unit/volume/v2/test_volume.py

@@ -655,7 +655,7 @@ def test_volume_delete_one_volume(self):
         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 @@ def test_volume_delete_multi_volumes(self):
         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 @@ def test_volume_delete_multi_volumes_with_exception(self):
         ]
         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 @@ def test_volume_delete_with_purge(self):
         ]
         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 @@ def test_volume_delete_with_force(self):
         ]
         verifylist = [
             ('force', True),
-            ('purge', False),
+            ('cascade', False),
             ('volumes', [self.volumes[0].id]),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)

openstackclient/tests/unit/volume/v3/test_volume.py

@@ -912,7 +912,7 @@ def test_volume_delete_one_volume(self):
         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 @@ def test_volume_delete_multi_volumes(self):
         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 @@ def test_volume_delete_multi_volumes_with_exception(self):
         ]
         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 @@ def test_volume_delete_with_purge(self):
         ]
         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 @@ def test_volume_delete_with_force(self):
         ]
         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 @@ def test_volume_delete_remote(self):
         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 @@ def test_volume_delete_multi_volumes_remote(self):
         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 @@ def test_volume_delete_remote_with_purge(self):
         verifylist = [
             ('remote', True),
             ('force', False),
-            ('purge', True),
             ('volumes', [self.volumes[0].id]),
         ]
 
@@ -1086,7 +1107,7 @@ def test_volume_delete_remote_with_purge(self):
             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 @@ def test_volume_delete_remote_with_force(self):
         verifylist = [
             ('remote', True),
             ('force', True),
-            ('purge', False),
+            ('cascade', False),
             ('volumes', [self.volumes[0].id]),
         ]
 
@@ -1113,7 +1134,7 @@ def test_volume_delete_remote_with_force(self):
             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),
         )

openstackclient/volume/v2/volume.py

@@ -390,12 +390,19 @@ def get_parser(self, prog_name):
             ),
         )
         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 @@ def take_action(self, parsed_args):
                 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

openstackclient/volume/v3/volume.py

@@ -515,12 +515,19 @@ def get_parser(self, prog_name):
             ),
         )
         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 @@ def take_action(self, parsed_args):
         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 @@ def take_action(self, parsed_args):
                     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

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.