volume functest: ensure snapshots deleted when volume delete

Deleting snapshot may take time. The current volume API does not allow
to delete volumes with snapshots, so if deleting snapshot may take time,
a delete request for a parent volume will fail.
This sometimes causes functional test failures in slow environments.

wait_for_status() checks whether volume status is in error statuses
but previously the expected error status was wrong. Cinder API uses
lower case as volume status, so it did not work expectedly.

Conflicts:
  openstackclient/tests/functional/compute/v2/common.py
  openstackclient/tests/functional/compute/v2/test_server.py
  openstackclient/tests/functional/volume/v1/test_volume.py
  openstackclient/tests/functional/volume/v2/test_volume.py
  openstackclient/tests/functional/volume/v3/common.py

Change-Id: I095894ba39f23bf81d71351818d24dbb5ca459fb
(cherry picked from commit eeb614c47759fa9a01e6d886ed07acceb8d9ff61)
Closes-Bug: #1714977

7 files changed, 108 insertions, 81 deletions

diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py
index 119ef05c..4bd7da31 100644
--- a/openstackclient/tests/functional/compute/v2/test_server.py
+++ b/openstackclient/tests/functional/compute/v2/test_server.py
@@ -16,7 +16,7 @@ import time
 from tempest.lib.common.utils import data_utils
 
 from openstackclient.tests.functional import base
-from openstackclient.tests.functional.volume.v2 import test_volume
+from openstackclient.tests.functional.volume.v2 import common as volume_common
 from tempest.lib import exceptions
 
 
@@ -331,9 +331,7 @@ class ServerTests(base.TestCase):
         """
         # server_image = self.get_image()
         # get volume status wait function
-        volume_wait_for = test_volume.VolumeTests(
-            methodName='wait_for',
-        ).wait_for
+        volume_wait_for = volume_common.BaseVolumeTests.wait_for_status
 
         # get image size
         cmd_output = json.loads(self.openstack(
diff --git a/openstackclient/tests/functional/volume/base.py b/openstackclient/tests/functional/volume/base.py
new file mode 100644
index 00000000..53032606
--- /dev/null
+++ b/openstackclient/tests/functional/volume/base.py
@@ -0,0 +1,64 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import json
+import time
+
+from openstackclient.tests.functional import base
+
+
+class BaseVolumeTests(base.TestCase):
+    """Base class for Volume functional tests. """
+
+    @classmethod
+    def wait_for_status(cls, check_type, check_name, desired_status,
+                        wait=120, interval=5, failures=None):
+        current_status = "notset"
+        if failures is None:
+            failures = ['error']
+        total_sleep = 0
+        while total_sleep < wait:
+            output = json.loads(cls.openstack(
+                check_type + ' show -f json ' + check_name))
+            current_status = output['status']
+            if (current_status == desired_status):
+                print('{} {} now has status {}'
+                      .format(check_type, check_name, current_status))
+                return
+            print('Checking {} {} Waiting for {} current status: {}'
+                  .format(check_type, check_name,
+                          desired_status, current_status))
+            if current_status in failures:
+                raise Exception(
+                    'Current status {} of {} {} is one of failures {}'
+                    .format(current_status, check_type, check_name, failures))
+            time.sleep(interval)
+            total_sleep += interval
+        cls.assertOutput(desired_status, current_status)
+
+    @classmethod
+    def wait_for_delete(cls, check_type, check_name, wait=120, interval=5,
+                        name_field=None):
+        total_sleep = 0
+        name_field = name_field or 'Name'
+        while total_sleep < wait:
+            result = json.loads(cls.openstack(check_type + ' list -f json'))
+            names = [x[name_field] for x in result]
+            if check_name not in names:
+                print('{} {} is now deleted'.format(check_type, check_name))
+                return
+            print('Checking {} {} Waiting for deleted'
+                  .format(check_type, check_name))
+            time.sleep(interval)
+            total_sleep += interval
+        raise Exception('Timeout: {} {} was not deleted in {} seconds'
+                        .format(check_type, check_name, wait))
diff --git a/openstackclient/tests/functional/volume/v1/common.py b/openstackclient/tests/functional/volume/v1/common.py
index a442850d..f9d96bbb 100644
--- a/openstackclient/tests/functional/volume/v1/common.py
+++ b/openstackclient/tests/functional/volume/v1/common.py
@@ -12,10 +12,10 @@
 
 import os
 
-from openstackclient.tests.functional import base
+from openstackclient.tests.functional.volume import base
 
 
-class BaseVolumeTests(base.TestCase):
+class BaseVolumeTests(base.BaseVolumeTests):
     """Base class for Volume functional tests. """
 
     @classmethod
diff --git a/openstackclient/tests/functional/volume/v1/test_snapshot.py b/openstackclient/tests/functional/volume/v1/test_snapshot.py
index 89a98661..28726762 100644
--- a/openstackclient/tests/functional/volume/v1/test_snapshot.py
+++ b/openstackclient/tests/functional/volume/v1/test_snapshot.py
@@ -11,7 +11,6 @@
 #    under the License.
 
 import json
-import time
 import uuid
 
 from openstackclient.tests.functional.volume.v1 import common
@@ -23,16 +22,6 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
     VOLLY = uuid.uuid4().hex
 
     @classmethod
-    def wait_for_status(cls, command, status, tries):
-        opts = cls.get_opts(['status'])
-        for attempt in range(tries):
-            time.sleep(1)
-            raw_output = cls.openstack(command + opts)
-            if (raw_output.rstrip() == status):
-                return
-        cls.assertOutput(status, raw_output)
-
-    @classmethod
     def setUpClass(cls):
         super(VolumeSnapshotTests, cls).setUpClass()
         # create a volume for all tests to create snapshot
@@ -41,12 +30,12 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             '--size 1 ' +
             cls.VOLLY
         ))
-        cls.wait_for_status('volume show ' + cls.VOLLY, 'available', 6)
+        cls.wait_for_status('volume', cls.VOLLY, 'available')
         cls.VOLUME_ID = cmd_output['id']
 
     @classmethod
     def tearDownClass(cls):
-        cls.wait_for_status('volume show ' + cls.VOLLY, 'available', 6)
+        cls.wait_for_status('volume', cls.VOLLY, 'available')
         raw_output = cls.openstack('volume delete --force ' + cls.VOLLY)
         cls.assertOutput('', raw_output)
 
@@ -74,14 +63,14 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             cmd_output["display_name"],
         )
 
-        self.wait_for_status(
-            'volume snapshot show ' + name1, 'available', 6)
-        self.wait_for_status(
-            'volume snapshot show ' + name2, 'available', 6)
+        self.wait_for_status('volume snapshot', name1, 'available')
+        self.wait_for_status('volume snapshot', name2, 'available')
 
         del_output = self.openstack(
             'volume snapshot delete ' + name1 + ' ' + name2)
         self.assertOutput('', del_output)
+        self.wait_for_delete('volume snapshot', name1)
+        self.wait_for_delete('volume snapshot', name2)
 
     def test_volume_snapshot_list(self):
         """Test create, list filter"""
@@ -91,6 +80,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             name1 +
             ' --volume ' + self.VOLLY
         ))
+        self.addCleanup(self.wait_for_delete, 'volume snapshot', name1)
         self.addCleanup(self.openstack, 'volume snapshot delete ' + name1)
         self.assertEqual(
             name1,
@@ -104,8 +94,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             1,
             cmd_output["size"],
         )
-        self.wait_for_status(
-            'volume snapshot show ' + name1, 'available', 6)
+        self.wait_for_status('volume snapshot', name1, 'available')
 
         name2 = uuid.uuid4().hex
         cmd_output = json.loads(self.openstack(
@@ -113,6 +102,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             name2 +
             ' --volume ' + self.VOLLY
         ))
+        self.addCleanup(self.wait_for_delete, 'volume snapshot', name2)
         self.addCleanup(self.openstack, 'volume snapshot delete ' + name2)
         self.assertEqual(
             name2,
@@ -126,8 +116,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             1,
             cmd_output["size"],
         )
-        self.wait_for_status(
-            'volume snapshot show ' + name2, 'available', 6)
+        self.wait_for_status('volume snapshot', name2, 'available')
 
         # Test list --long, --status
         cmd_output = json.loads(self.openstack(
@@ -167,6 +156,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             ' --description aaaa ' +
             name
         ))
+        self.addCleanup(self.wait_for_delete, 'volume snapshot', new_name)
         self.addCleanup(self.openstack, 'volume snapshot delete ' + new_name)
         self.assertEqual(
             name,
@@ -180,8 +170,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             'aaaa',
             cmd_output["display_description"],
         )
-        self.wait_for_status(
-            'volume snapshot show ' + name, 'available', 6)
+        self.wait_for_status('volume snapshot', name, 'available')
 
         # Test volume snapshot set
         raw_output = self.openstack(
diff --git a/openstackclient/tests/functional/volume/v2/common.py b/openstackclient/tests/functional/volume/v2/common.py
index e279a6f6..1d14719f 100644
--- a/openstackclient/tests/functional/volume/v2/common.py
+++ b/openstackclient/tests/functional/volume/v2/common.py
@@ -12,10 +12,10 @@
 
 import os
 
-from openstackclient.tests.functional import base
+from openstackclient.tests.functional.volume import base
 
 
-class BaseVolumeTests(base.TestCase):
+class BaseVolumeTests(base.BaseVolumeTests):
     """Base class for Volume functional tests. """
 
     @classmethod
diff --git a/openstackclient/tests/functional/volume/v2/test_snapshot.py b/openstackclient/tests/functional/volume/v2/test_snapshot.py
index 422e5b7c..2fff43c8 100644
--- a/openstackclient/tests/functional/volume/v2/test_snapshot.py
+++ b/openstackclient/tests/functional/volume/v2/test_snapshot.py
@@ -11,7 +11,6 @@
 #    under the License.
 
 import json
-import time
 import uuid
 
 from openstackclient.tests.functional.volume.v2 import common
@@ -23,16 +22,6 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
     VOLLY = uuid.uuid4().hex
 
     @classmethod
-    def wait_for_status(cls, command, status, tries):
-        opts = cls.get_opts(['status'])
-        for attempt in range(tries):
-            time.sleep(1)
-            raw_output = cls.openstack(command + opts)
-            if (raw_output.rstrip() == status):
-                return
-        cls.assertOutput(status, raw_output)
-
-    @classmethod
     def setUpClass(cls):
         super(VolumeSnapshotTests, cls).setUpClass()
         # create a volume for all tests to create snapshot
@@ -41,13 +30,14 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             '--size 1 ' +
             cls.VOLLY
         ))
-        cls.wait_for_status('volume show ' + cls.VOLLY, 'available', 6)
+        cls.wait_for_status('volume', cls.VOLLY, 'available')
         cls.VOLUME_ID = cmd_output['id']
 
     @classmethod
     def tearDownClass(cls):
-        cls.wait_for_status('volume show ' + cls.VOLLY, 'available', 6)
-        raw_output = cls.openstack('volume delete --force ' + cls.VOLLY)
+        cls.wait_for_status('volume', cls.VOLLY, 'available')
+        raw_output = cls.openstack(
+            'volume delete --force ' + cls.VOLLY)
         cls.assertOutput('', raw_output)
 
     def test_volume_snapshot__delete(self):
@@ -74,14 +64,14 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             cmd_output["name"],
         )
 
-        self.wait_for_status(
-            'volume snapshot show ' + name1, 'available', 6)
-        self.wait_for_status(
-            'volume snapshot show ' + name2, 'available', 6)
+        self.wait_for_status('volume snapshot', name1, 'available')
+        self.wait_for_status('volume snapshot', name2, 'available')
 
         del_output = self.openstack(
             'volume snapshot delete ' + name1 + ' ' + name2)
         self.assertOutput('', del_output)
+        self.wait_for_delete('volume snapshot', name1)
+        self.wait_for_delete('volume snapshot', name2)
 
     def test_volume_snapshot_list(self):
         """Test create, list filter"""
@@ -91,6 +81,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             name1 +
             ' --volume ' + self.VOLLY
         ))
+        self.addCleanup(self.wait_for_delete, 'volume snapshot', name1)
         self.addCleanup(self.openstack, 'volume snapshot delete ' + name1)
         self.assertEqual(
             name1,
@@ -104,8 +95,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             1,
             cmd_output["size"],
         )
-        self.wait_for_status(
-            'volume snapshot show ' + name1, 'available', 6)
+        self.wait_for_status('volume snapshot', name1, 'available')
 
         name2 = uuid.uuid4().hex
         cmd_output = json.loads(self.openstack(
@@ -113,6 +103,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             name2 +
             ' --volume ' + self.VOLLY
         ))
+        self.addCleanup(self.wait_for_delete, 'volume snapshot', name2)
         self.addCleanup(self.openstack, 'volume snapshot delete ' + name2)
         self.assertEqual(
             name2,
@@ -126,8 +117,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             1,
             cmd_output["size"],
         )
-        self.wait_for_status(
-            'volume snapshot show ' + name2, 'available', 6)
+        self.wait_for_status('volume snapshot', name2, 'available')
         raw_output = self.openstack(
             'volume snapshot set ' +
             '--state error ' +
@@ -174,6 +164,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             '--property Alpha=a ' +
             name
         ))
+        self.addCleanup(self.wait_for_delete, 'volume snapshot', new_name)
         self.addCleanup(self.openstack, 'volume snapshot delete ' + new_name)
         self.assertEqual(
             name,
@@ -191,8 +182,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests):
             "Alpha='a'",
             cmd_output["properties"],
         )
-        self.wait_for_status(
-            'volume snapshot show ' + name, 'available', 6)
+        self.wait_for_status('volume snapshot', name, 'available')
 
         # Test volume snapshot set
         raw_output = self.openstack(
diff --git a/openstackclient/tests/functional/volume/v2/test_volume.py b/openstackclient/tests/functional/volume/v2/test_volume.py
index 203ca819..7934da54 100644
--- a/openstackclient/tests/functional/volume/v2/test_volume.py
+++ b/openstackclient/tests/functional/volume/v2/test_volume.py
@@ -11,7 +11,6 @@
 #    under the License.
 
 import json
-import time
 import uuid
 
 from openstackclient.tests.functional.volume.v2 import common
@@ -44,8 +43,8 @@ class VolumeTests(common.BaseVolumeTests):
             cmd_output["size"],
         )
 
-        self.wait_for("volume", name1, "available")
-        self.wait_for("volume", name2, "available")
+        self.wait_for_status("volume", name1, "available")
+        self.wait_for_status("volume", name2, "available")
         del_output = self.openstack('volume delete ' + name1 + ' ' + name2)
         self.assertOutput('', del_output)
 
@@ -62,7 +61,7 @@ class VolumeTests(common.BaseVolumeTests):
             1,
             cmd_output["size"],
         )
-        self.wait_for("volume", name1, "available")
+        self.wait_for_status("volume", name1, "available")
 
         name2 = uuid.uuid4().hex
         cmd_output = json.loads(self.openstack(
@@ -75,7 +74,7 @@ class VolumeTests(common.BaseVolumeTests):
             2,
             cmd_output["size"],
         )
-        self.wait_for("volume", name2, "available")
+        self.wait_for_status("volume", name2, "available")
         raw_output = self.openstack(
             'volume set ' +
             '--state error ' +
@@ -136,7 +135,7 @@ class VolumeTests(common.BaseVolumeTests):
             'false',
             cmd_output["bootable"],
         )
-        self.wait_for("volume", name, "available")
+        self.wait_for_status("volume", name, "available")
 
         # Test volume set
         raw_output = self.openstack(
@@ -204,7 +203,7 @@ class VolumeTests(common.BaseVolumeTests):
             '--size 1 ' +
             volume_name
         ))
-        self.wait_for("volume", volume_name, "available")
+        self.wait_for_status("volume", volume_name, "available")
         self.assertEqual(
             volume_name,
             cmd_output["name"],
@@ -214,9 +213,10 @@ class VolumeTests(common.BaseVolumeTests):
             snapshot_name +
             ' --volume ' + volume_name
         ))
-        self.wait_for("volume snapshot", snapshot_name, "available")
+        self.wait_for_status("volume snapshot", snapshot_name, "available")
 
         name = uuid.uuid4().hex
+        # Create volume from snapshot
         cmd_output = json.loads(self.openstack(
             'volume create -f json ' +
             '--snapshot ' + snapshot_name +
@@ -228,26 +228,12 @@ class VolumeTests(common.BaseVolumeTests):
             name,
             cmd_output["name"],
         )
-        self.wait_for("volume", name, "available")
+        self.wait_for_status("volume", name, "available")
 
         # Delete snapshot
         raw_output = self.openstack(
             'volume snapshot delete ' + snapshot_name)
         self.assertOutput('', raw_output)
-
-    def wait_for(self, check_type, check_name, desired_status, wait=120,
-                 interval=5, failures=['ERROR']):
-        status = "notset"
-        total_sleep = 0
-        opts = self.get_opts(['status'])
-        while total_sleep < wait:
-            status = self.openstack(check_type + ' show ' + check_name + opts)
-            status = status.rstrip()
-            print('Checking {} {} Waiting for {} current status: {}'
-                  .format(check_type, check_name, desired_status, status))
-            if status == desired_status:
-                break
-            self.assertNotIn(status, failures)
-            time.sleep(interval)
-            total_sleep += interval
-        self.assertEqual(desired_status, status)
+        # Deleting snapshot may take time. If volume snapshot still exists when
+        # a parent volume delete is requested, the volume deletion will fail.
+        self.wait_for_delete('volume snapshot', snapshot_name)