Refactor "snapshot" commands

1.change the command name ``snapshot create/delete/list/
show/set/unset`` to ``volume snapshot create/delete/list/
show/set/unset``.

2.change the optional parameter "--name <name>" to a
positional parameter "<snapshot-name>"; Change the
positional parameter "<volume>" to a optional
parameter "--volume <volume>"

Change-Id: If03276ecdf6f0d96893d5ecf91c2aaa64929cff3
Implements: bp backup-snapshot-renamed-for-volume-resource
Co-Authored-By: Sheel Rana <ranasheel2000@gmail.com>

5 files changed, 117 insertions, 58 deletions

diff --git a/openstackclient/tests/functional/volume/v1/test_snapshot.py b/openstackclient/tests/functional/volume/v1/test_snapshot.py
index c6d65ccc..1e1c6b21 100644
--- a/openstackclient/tests/functional/volume/v1/test_snapshot.py
+++ b/openstackclient/tests/functional/volume/v1/test_snapshot.py
@@ -16,8 +16,8 @@ import uuid
 from openstackclient.tests.functional.volume.v1 import common
 
 
-class SnapshotTests(common.BaseVolumeTests):
-    """Functional tests for snapshot. """
+class VolumeSnapshotTests(common.BaseVolumeTests):
+    """Functional tests for volume snapshot. """
 
     VOLLY = uuid.uuid4().hex
     NAME = uuid.uuid4().hex
@@ -36,24 +36,25 @@ class SnapshotTests(common.BaseVolumeTests):
 
     @classmethod
     def setUpClass(cls):
-        super(SnapshotTests, cls).setUpClass()
+        super(VolumeSnapshotTests, cls).setUpClass()
         cls.openstack('volume create --size 1 ' + cls.VOLLY)
         cls.wait_for_status('volume show ' + cls.VOLLY, 'available\n', 3)
         opts = cls.get_opts(['status'])
-        raw_output = cls.openstack('snapshot create --name ' + cls.NAME +
-                                   ' ' + cls.VOLLY + opts)
+        raw_output = cls.openstack('volume snapshot create --volume ' +
+                                   cls.VOLLY + ' ' + cls.NAME + opts)
         cls.assertOutput('creating\n', raw_output)
-        cls.wait_for_status('snapshot show ' + cls.NAME, 'available\n', 3)
+        cls.wait_for_status(
+            'volume snapshot show ' + cls.NAME, 'available\n', 3)
 
     @classmethod
     def tearDownClass(cls):
         # Rename test
         raw_output = cls.openstack(
-            'snapshot set --name ' + cls.OTHER_NAME + ' ' + cls.NAME)
+            'volume snapshot set --name ' + cls.OTHER_NAME + ' ' + cls.NAME)
         cls.assertOutput('', raw_output)
         # Delete test
         raw_output_snapshot = cls.openstack(
-            'snapshot delete ' + cls.OTHER_NAME)
+            'volume snapshot delete ' + cls.OTHER_NAME)
         cls.wait_for_status('volume show ' + cls.VOLLY, 'available\n', 6)
         raw_output_volume = cls.openstack('volume delete --force ' + cls.VOLLY)
         cls.assertOutput('', raw_output_snapshot)
@@ -61,26 +62,27 @@ class SnapshotTests(common.BaseVolumeTests):
 
     def test_snapshot_list(self):
         opts = self.get_opts(self.HEADERS)
-        raw_output = self.openstack('snapshot list' + opts)
+        raw_output = self.openstack('volume snapshot list' + opts)
         self.assertIn(self.NAME, raw_output)
 
     def test_snapshot_set_unset_properties(self):
         raw_output = self.openstack(
-            'snapshot set --property a=b --property c=d ' + self.NAME)
+            'volume snapshot set --property a=b --property c=d ' + self.NAME)
         self.assertEqual("", raw_output)
         opts = self.get_opts(["properties"])
-        raw_output = self.openstack('snapshot show ' + self.NAME + opts)
+        raw_output = self.openstack('volume snapshot show ' + self.NAME + opts)
         self.assertEqual("a='b', c='d'\n", raw_output)
 
-        raw_output = self.openstack('snapshot unset --property a ' + self.NAME)
+        raw_output = self.openstack(
+            'volume snapshot unset --property a ' + self.NAME)
         self.assertEqual("", raw_output)
-        raw_output = self.openstack('snapshot show ' + self.NAME + opts)
+        raw_output = self.openstack('volume snapshot show ' + self.NAME + opts)
         self.assertEqual("c='d'\n", raw_output)
 
     def test_snapshot_set_description(self):
         raw_output = self.openstack(
-            'snapshot set --description backup ' + self.NAME)
+            'volume snapshot set --description backup ' + self.NAME)
         self.assertEqual("", raw_output)
         opts = self.get_opts(["display_description", "display_name"])
-        raw_output = self.openstack('snapshot show ' + self.NAME + opts)
+        raw_output = self.openstack('volume snapshot show ' + self.NAME + opts)
         self.assertEqual("backup\n" + self.NAME + "\n", raw_output)
diff --git a/openstackclient/tests/functional/volume/v2/test_snapshot.py b/openstackclient/tests/functional/volume/v2/test_snapshot.py
index fcbc31cb..4eb69e9d 100644
--- a/openstackclient/tests/functional/volume/v2/test_snapshot.py
+++ b/openstackclient/tests/functional/volume/v2/test_snapshot.py
@@ -16,8 +16,8 @@ import uuid
 from openstackclient.tests.functional.volume.v2 import common
 
 
-class SnapshotTests(common.BaseVolumeTests):
-    """Functional tests for snapshot. """
+class VolumeSnapshotTests(common.BaseVolumeTests):
+    """Functional tests for volume snapshot. """
 
     VOLLY = uuid.uuid4().hex
     NAME = uuid.uuid4().hex
@@ -36,24 +36,25 @@ class SnapshotTests(common.BaseVolumeTests):
 
     @classmethod
     def setUpClass(cls):
-        super(SnapshotTests, cls).setUpClass()
+        super(VolumeSnapshotTests, cls).setUpClass()
         cls.openstack('volume create --size 1 ' + cls.VOLLY)
         cls.wait_for_status('volume show ' + cls.VOLLY, 'available\n', 3)
         opts = cls.get_opts(['status'])
-        raw_output = cls.openstack('snapshot create --name ' + cls.NAME +
-                                   ' ' + cls.VOLLY + opts)
+        raw_output = cls.openstack('volume snapshot create --volume ' +
+                                   cls.VOLLY + ' ' + cls.NAME + opts)
         cls.assertOutput('creating\n', raw_output)
-        cls.wait_for_status('snapshot show ' + cls.NAME, 'available\n', 3)
+        cls.wait_for_status(
+            'volume snapshot show ' + cls.NAME, 'available\n', 3)
 
     @classmethod
     def tearDownClass(cls):
         # Rename test
         raw_output = cls.openstack(
-            'snapshot set --name ' + cls.OTHER_NAME + ' ' + cls.NAME)
+            'volume snapshot set --name ' + cls.OTHER_NAME + ' ' + cls.NAME)
         cls.assertOutput('', raw_output)
         # Delete test
         raw_output_snapshot = cls.openstack(
-            'snapshot delete ' + cls.OTHER_NAME)
+            'volume snapshot delete ' + cls.OTHER_NAME)
         cls.wait_for_status('volume show ' + cls.VOLLY, 'available\n', 6)
         raw_output_volume = cls.openstack('volume delete --force ' + cls.VOLLY)
         cls.assertOutput('', raw_output_snapshot)
@@ -61,26 +62,27 @@ class SnapshotTests(common.BaseVolumeTests):
 
     def test_snapshot_list(self):
         opts = self.get_opts(self.HEADERS)
-        raw_output = self.openstack('snapshot list' + opts)
+        raw_output = self.openstack('volume snapshot list' + opts)
         self.assertIn(self.NAME, raw_output)
 
     def test_snapshot_properties(self):
         raw_output = self.openstack(
-            'snapshot set --property a=b --property c=d ' + self.NAME)
+            'volume snapshot set --property a=b --property c=d ' + self.NAME)
         self.assertEqual("", raw_output)
         opts = self.get_opts(["properties"])
-        raw_output = self.openstack('snapshot show ' + self.NAME + opts)
+        raw_output = self.openstack('volume snapshot show ' + self.NAME + opts)
         self.assertEqual("a='b', c='d'\n", raw_output)
 
-        raw_output = self.openstack('snapshot unset --property a ' + self.NAME)
+        raw_output = self.openstack(
+            'volume snapshot unset --property a ' + self.NAME)
         self.assertEqual("", raw_output)
-        raw_output = self.openstack('snapshot show ' + self.NAME + opts)
+        raw_output = self.openstack('volume snapshot show ' + self.NAME + opts)
         self.assertEqual("c='d'\n", raw_output)
 
     def test_snapshot_set(self):
         raw_output = self.openstack(
-            'snapshot set --description backup ' + self.NAME)
+            'volume snapshot set --description backup ' + self.NAME)
         self.assertEqual("", raw_output)
         opts = self.get_opts(["description", "name"])
-        raw_output = self.openstack('snapshot show ' + self.NAME + opts)
+        raw_output = self.openstack('volume snapshot show ' + self.NAME + opts)
         self.assertEqual("backup\n" + self.NAME + "\n", raw_output)
diff --git a/openstackclient/tests/functional/volume/v2/test_volume.py b/openstackclient/tests/functional/volume/v2/test_volume.py
index fb880578..ea891cba 100644
--- a/openstackclient/tests/functional/volume/v2/test_volume.py
+++ b/openstackclient/tests/functional/volume/v2/test_volume.py
@@ -106,11 +106,12 @@ class VolumeTests(common.BaseVolumeTests):
         opts = self.get_opts(self.FIELDS)
 
         # Create snapshot from test volume
-        raw_output = self.openstack('snapshot create ' + self.NAME +
-                                    ' --name ' + self.SNAPSHOT_NAME + opts)
+        raw_output = self.openstack('volume snapshot create ' +
+                                    self.SNAPSHOT_NAME +
+                                    ' --volume ' + self.NAME + opts)
         expected = self.SNAPSHOT_NAME + '\n'
         self.assertOutput(expected, raw_output)
-        self.wait_for("snapshot", self.SNAPSHOT_NAME, "available")
+        self.wait_for("volume snapshot", self.SNAPSHOT_NAME, "available")
 
         # Create volume from snapshot
         raw_output = self.openstack('volume create --size 2 --snapshot ' +
@@ -126,7 +127,8 @@ class VolumeTests(common.BaseVolumeTests):
         self.assertOutput('', raw_output)
 
         # Delete test snapshot
-        raw_output = self.openstack('snapshot delete ' + self.SNAPSHOT_NAME)
+        raw_output = self.openstack(
+            'volume snapshot delete ' + self.SNAPSHOT_NAME)
         self.assertOutput('', raw_output)
         self.wait_for("volume", self.NAME, "available")
 
diff --git a/openstackclient/tests/unit/volume/v1/test_snapshot.py b/openstackclient/tests/unit/volume/v1/test_snapshot.py
index edfbdc19..8e30d6a9 100644
--- a/openstackclient/tests/unit/volume/v1/test_snapshot.py
+++ b/openstackclient/tests/unit/volume/v1/test_snapshot.py
@@ -19,7 +19,7 @@ from osc_lib import exceptions
 from osc_lib import utils
 
 from openstackclient.tests.unit.volume.v1 import fakes as volume_fakes
-from openstackclient.volume.v1 import snapshot
+from openstackclient.volume.v1 import volume_snapshot
 
 
 class TestSnapshot(volume_fakes.TestVolumev1):
@@ -67,20 +67,20 @@ class TestSnapshotCreate(TestSnapshot):
         self.volumes_mock.get.return_value = self.volume
         self.snapshots_mock.create.return_value = self.new_snapshot
         # Get the command object to test
-        self.cmd = snapshot.CreateSnapshot(self.app, None)
+        self.cmd = volume_snapshot.CreateVolumeSnapshot(self.app, None)
 
     def test_snapshot_create(self):
         arglist = [
-            "--name", self.new_snapshot.display_name,
+            "--volume", self.new_snapshot.volume_id,
             "--description", self.new_snapshot.display_description,
             "--force",
-            self.new_snapshot.volume_id,
+            self.new_snapshot.display_name,
         ]
         verifylist = [
-            ("name", self.new_snapshot.display_name),
+            ("volume", self.new_snapshot.volume_id),
             ("description", self.new_snapshot.display_description),
             ("force", True),
-            ("volume", self.new_snapshot.volume_id),
+            ("snapshot_name", self.new_snapshot.display_name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
@@ -97,7 +97,7 @@ class TestSnapshotCreate(TestSnapshot):
 
     def test_snapshot_create_without_name(self):
         arglist = [
-            self.new_snapshot.volume_id,
+            "--volume", self.new_snapshot.volume_id,
             "--description", self.new_snapshot.display_description,
             "--force"
         ]
@@ -119,6 +119,32 @@ class TestSnapshotCreate(TestSnapshot):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)
 
+    def test_snapshot_create_without_volume(self):
+        arglist = [
+            "--description", self.new_snapshot.display_description,
+            "--force",
+            self.new_snapshot.display_name
+        ]
+        verifylist = [
+            ("description", self.new_snapshot.display_description),
+            ("force", True),
+            ("snapshot_name", self.new_snapshot.display_name)
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.volumes_mock.get.assert_called_once_with(
+            self.new_snapshot.display_name)
+        self.snapshots_mock.create.assert_called_once_with(
+            self.new_snapshot.volume_id,
+            True,
+            self.new_snapshot.display_name,
+            self.new_snapshot.display_description,
+        )
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.data, data)
+
 
 class TestSnapshotDelete(TestSnapshot):
 
@@ -132,7 +158,7 @@ class TestSnapshotDelete(TestSnapshot):
         self.snapshots_mock.delete.return_value = None
 
         # Get the command object to mock
-        self.cmd = snapshot.DeleteSnapshot(self.app, None)
+        self.cmd = volume_snapshot.DeleteVolumeSnapshot(self.app, None)
 
     def test_snapshot_delete(self):
         arglist = [
@@ -244,7 +270,7 @@ class TestSnapshotList(TestSnapshot):
         self.volumes_mock.list.return_value = [self.volume]
         self.snapshots_mock.list.return_value = self.snapshots
         # Get the command to test
-        self.cmd = snapshot.ListSnapshot(self.app, None)
+        self.cmd = volume_snapshot.ListVolumeSnapshot(self.app, None)
 
     def test_snapshot_list_without_options(self):
         arglist = []
@@ -307,7 +333,7 @@ class TestSnapshotSet(TestSnapshot):
         self.snapshots_mock.get.return_value = self.snapshot
         self.snapshots_mock.set_metadata.return_value = None
         # Get the command object to mock
-        self.cmd = snapshot.SetSnapshot(self.app, None)
+        self.cmd = volume_snapshot.SetVolumeSnapshot(self.app, None)
 
     def test_snapshot_set_all(self):
         arglist = [
@@ -404,7 +430,7 @@ class TestSnapshotShow(TestSnapshot):
 
         self.snapshots_mock.get.return_value = self.snapshot
         # Get the command object to test
-        self.cmd = snapshot.ShowSnapshot(self.app, None)
+        self.cmd = volume_snapshot.ShowVolumeSnapshot(self.app, None)
 
     def test_snapshot_show(self):
         arglist = [
@@ -432,7 +458,7 @@ class TestSnapshotUnset(TestSnapshot):
         self.snapshots_mock.get.return_value = self.snapshot
         self.snapshots_mock.delete_metadata.return_value = None
         # Get the command object to mock
-        self.cmd = snapshot.UnsetSnapshot(self.app, None)
+        self.cmd = volume_snapshot.UnsetVolumeSnapshot(self.app, None)
 
     def test_snapshot_unset(self):
         arglist = [
diff --git a/openstackclient/tests/unit/volume/v2/test_snapshot.py b/openstackclient/tests/unit/volume/v2/test_snapshot.py
index d355662d..b67dd6eb 100644
--- a/openstackclient/tests/unit/volume/v2/test_snapshot.py
+++ b/openstackclient/tests/unit/volume/v2/test_snapshot.py
@@ -20,7 +20,7 @@ from osc_lib import exceptions
 from osc_lib import utils
 
 from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes
-from openstackclient.volume.v2 import snapshot
+from openstackclient.volume.v2 import volume_snapshot
 
 
 class TestSnapshot(volume_fakes.TestVolume):
@@ -68,23 +68,23 @@ class TestSnapshotCreate(TestSnapshot):
         self.volumes_mock.get.return_value = self.volume
         self.snapshots_mock.create.return_value = self.new_snapshot
         # Get the command object to test
-        self.cmd = snapshot.CreateSnapshot(self.app, None)
+        self.cmd = volume_snapshot.CreateVolumeSnapshot(self.app, None)
 
     def test_snapshot_create(self):
         arglist = [
-            "--name", self.new_snapshot.name,
+            "--volume", self.new_snapshot.volume_id,
             "--description", self.new_snapshot.description,
             "--force",
             '--property', 'Alpha=a',
             '--property', 'Beta=b',
-            self.new_snapshot.volume_id,
+            self.new_snapshot.name,
         ]
         verifylist = [
-            ("name", self.new_snapshot.name),
+            ("volume", self.new_snapshot.volume_id),
             ("description", self.new_snapshot.description),
             ("force", True),
             ('property', {'Alpha': 'a', 'Beta': 'b'}),
-            ("volume", self.new_snapshot.volume_id),
+            ("snapshot_name", self.new_snapshot.name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
@@ -102,7 +102,7 @@ class TestSnapshotCreate(TestSnapshot):
 
     def test_snapshot_create_without_name(self):
         arglist = [
-            self.new_snapshot.volume_id,
+            "--volume", self.new_snapshot.volume_id,
             "--description", self.new_snapshot.description,
             "--force"
         ]
@@ -125,6 +125,33 @@ class TestSnapshotCreate(TestSnapshot):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)
 
+    def test_snapshot_create_without_volume(self):
+        arglist = [
+            "--description", self.new_snapshot.description,
+            "--force",
+            self.new_snapshot.name
+        ]
+        verifylist = [
+            ("description", self.new_snapshot.description),
+            ("force", True),
+            ("snapshot_name", self.new_snapshot.name)
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.volumes_mock.get.assert_called_once_with(
+            self.new_snapshot.name)
+        self.snapshots_mock.create.assert_called_once_with(
+            self.new_snapshot.volume_id,
+            force=True,
+            name=self.new_snapshot.name,
+            description=self.new_snapshot.description,
+            metadata=None,
+        )
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.data, data)
+
 
 class TestSnapshotDelete(TestSnapshot):
 
@@ -138,7 +165,7 @@ class TestSnapshotDelete(TestSnapshot):
         self.snapshots_mock.delete.return_value = None
 
         # Get the command object to mock
-        self.cmd = snapshot.DeleteSnapshot(self.app, None)
+        self.cmd = volume_snapshot.DeleteVolumeSnapshot(self.app, None)
 
     def test_snapshot_delete(self):
         arglist = [
@@ -250,7 +277,7 @@ class TestSnapshotList(TestSnapshot):
         self.volumes_mock.list.return_value = [self.volume]
         self.snapshots_mock.list.return_value = self.snapshots
         # Get the command to test
-        self.cmd = snapshot.ListSnapshot(self.app, None)
+        self.cmd = volume_snapshot.ListVolumeSnapshot(self.app, None)
 
     def test_snapshot_list_without_options(self):
         arglist = []
@@ -330,7 +357,7 @@ class TestSnapshotSet(TestSnapshot):
         self.snapshots_mock.set_metadata.return_value = None
         self.snapshots_mock.update.return_value = None
         # Get the command object to mock
-        self.cmd = snapshot.SetSnapshot(self.app, None)
+        self.cmd = volume_snapshot.SetVolumeSnapshot(self.app, None)
 
     def test_snapshot_set(self):
         arglist = [
@@ -457,7 +484,7 @@ class TestSnapshotShow(TestSnapshot):
 
         self.snapshots_mock.get.return_value = self.snapshot
         # Get the command object to test
-        self.cmd = snapshot.ShowSnapshot(self.app, None)
+        self.cmd = volume_snapshot.ShowVolumeSnapshot(self.app, None)
 
     def test_snapshot_show(self):
         arglist = [
@@ -485,7 +512,7 @@ class TestSnapshotUnset(TestSnapshot):
         self.snapshots_mock.get.return_value = self.snapshot
         self.snapshots_mock.delete_metadata.return_value = None
         # Get the command object to mock
-        self.cmd = snapshot.UnsetSnapshot(self.app, None)
+        self.cmd = volume_snapshot.UnsetVolumeSnapshot(self.app, None)
 
     def test_snapshot_unset(self):
         arglist = [