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.

Change-Id: I095894ba39f23bf81d71351818d24dbb5ca459fb

Author: amotoki

openstackclient/tests/functional/compute/v2/common.py

@@ -125,12 +125,12 @@ def wait_for_status(
                 name
             ))
             status = cmd_output['status']
-            print('Waiting for {}, current status: {}'.format(
-                expected_status,
-                status,
-            ))
             if status == expected_status:
+                print('Server {} now has status {}'.format(
+                    name, status))
                 break
+            print('Server {}: Waiting for {}, current status: {}'.format(
+                name, expected_status, status))
             self.assertNotIn(status, failures)
             time.sleep(interval)
             total_sleep += interval

openstackclient/tests/functional/compute/v2/test_server.py

@@ -17,7 +17,7 @@
 
 from openstackclient.tests.functional import base
 from openstackclient.tests.functional.compute.v2 import common
-from openstackclient.tests.functional.volume.v2 import test_volume
+from openstackclient.tests.functional.volume.v2 import common as volume_common
 
 
 class ServerTests(common.ComputeTestCase):
@@ -282,9 +282,7 @@ def test_server_reboot(self):
     def test_server_boot_from_volume(self):
         """Test server create from volume, server delete"""
         # 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(
@@ -391,9 +389,8 @@ def test_server_boot_from_volume(self):
     def test_server_boot_with_bdm_snapshot(self):
         """Test server create from image with bdm snapshot, server delete"""
         # 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
+        volume_wait_for_delete = volume_common.BaseVolumeTests.wait_for_delete
 
         # create source empty volume
         empty_volume_name = uuid.uuid4().hex
@@ -419,6 +416,13 @@ def test_server_boot_with_bdm_snapshot(self):
             empty_snapshot_name
         ))
         self.assertIsNotNone(cmd_output["id"])
+        # Deleting volume snapshot take time, so we need to wait until the
+        # snapshot goes. Entries registered by self.addCleanup will be called
+        # in the reverse order, so we need to register wait_for_delete first.
+        self.addCleanup(volume_wait_for_delete,
+                        'volume snapshot', empty_snapshot_name)
+        self.addCleanup(self.openstack,
+                        'volume snapshot delete ' + empty_snapshot_name)
         self.assertEqual(
             empty_snapshot_name,
             cmd_output['name'],
@@ -489,12 +493,6 @@ def test_server_boot_with_bdm_snapshot(self):
             # the attached volume had been deleted
             pass
 
-        # clean up volume snapshot manually, make sure the snapshot and volume
-        # can be deleted sequentially, self.addCleanup so fast, that cause
-        # volume service API 400 error and the volume is left over at the end.
-        self.openstack('volume snapshot delete ' + empty_snapshot_name)
-        volume_wait_for('volume snapshot', empty_snapshot_name, 'disappear')
-
     def test_server_create_with_none_network(self):
         """Test server create with none network option."""
         server_name = uuid.uuid4().hex

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))

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

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
@@ -22,16 +21,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()
@@ -41,12 +30,12 @@ def setUpClass(cls):
             '--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 @@ def test_volume_snapshot__delete(self):
             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 @@ def test_volume_snapshot_list(self):
             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,15 +94,15 @@ def test_volume_snapshot_list(self):
             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(
             'volume snapshot create -f json ' +
             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 @@ def test_volume_snapshot_list(self):
             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 @@ def test_snapshot_set(self):
             ' --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 @@ def test_snapshot_set(self):
             '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(

openstackclient/tests/functional/volume/v1/test_volume.py

@@ -11,7 +11,6 @@
 #    under the License.
 
 import json
-import time
 import uuid
 
 from openstackclient.tests.functional.volume.v1 import common
@@ -44,8 +43,8 @@ def test_volume_create_and_delete(self):
             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 @@ def test_volume_list(self):
             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 @@ def test_volume_list(self):
             2,
             cmd_output["size"],
         )
-        self.wait_for("volume", name2, "available")
+        self.wait_for_status("volume", name2, "available")
 
         # Test list
         cmd_output = json.loads(self.openstack(
@@ -131,7 +130,7 @@ def test_volume_set_and_unset(self):
             'false',
             cmd_output["bootable"],
         )
-        self.wait_for("volume", name, "available")
+        self.wait_for_status("volume", name, "available")
 
         # Test volume set
         new_name = uuid.uuid4().hex
@@ -208,7 +207,7 @@ def test_volume_create_and_list_and_show_backward_compatibility(self):
         self.assertNotIn('name', json_output)
         self.addCleanup(self.openstack, 'volume delete ' + volume_id)
 
-        self.wait_for("volume", name1, "available")
+        self.wait_for_status("volume", name1, "available")
 
         json_output = json.loads(self.openstack(
             'volume list -f json ' +
@@ -233,20 +232,3 @@ def test_volume_create_and_list_and_show_backward_compatibility(self):
         self.assertEqual(name1, json_output['display_name'])
         self.assertIn('id', json_output)
         self.assertNotIn('name', json_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)

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