Simplified test cleanup actions.

test/helpers.py

@@ -15,19 +15,21 @@ def parse_bool(val):
 
 
 @contextmanager
-def exception_action(cleanup_func):
+def cleanup_action(cleanup_func):
     """
     Context manager to carry out a given
-    cleanup action when an exception occurs.
+    cleanup action after carrying out a set
+    of tasks, or when an exception occurs.
     If any errors occur during the cleanup
     action, those are ignored, and the original
     traceback is preserved.
 
-    :params func: This function is called only
-        if an exception occurs. Any exceptions raised
+    :params func: This function is called if
+    an exception occurs or at the end of the
+    context block. If any exceptions raised
         by func are ignored.
     Usage:
-        with exception_action(lambda e: print("Oops!")):
+        with cleanup_action(lambda e: print("Oops!")):
             do_something()
     """
     try:
@@ -37,8 +39,13 @@ def exception_action(cleanup_func):
         try:
             cleanup_func()
         except Exception as e:
-            print("Error during cleanup: {0}".format(e))
+            print("Error during exception cleanup: {0}".format(e))
         reraise(ex_class, ex_val, ex_traceback)
+    try:
+        cleanup_func()
+    except Exception as e:
+        print("Error during normal cleanup: {0}".format(e))
+
 
 TEST_DATA_CONFIG = {
     "AWSCloudProvider": {
@@ -49,9 +56,9 @@ TEST_DATA_CONFIG = {
     },
     "OpenStackCloudProvider": {
         "image": os.environ.get('CB_IMAGE_OS',
-                                'd57696ba-5ed2-43fe-bf78-a587829973a9'),
-        "instance_type": os.environ.get('CB_INSTANCE_TYPE_OS', 'm2.xsmall'),
-        "placement": os.environ.get('CB_PLACEMENT_OS', 'NCI'),
+                                'fdd9a6ee-2c9b-490c-91ac-ca9b7c134264'),
+        "instance_type": os.environ.get('CB_INSTANCE_TYPE_OS', 'm1.nano'),
+        "placement": os.environ.get('CB_PLACEMENT_OS', 'nova'),
     }
 }
 
@@ -76,7 +83,7 @@ def get_provider_wait_interval(provider):
     if isinstance(provider, TestMockHelperMixin):
         return 0
     else:
-        return 5
+        return 1
 
 
 def get_test_instance(provider, name):

test/test_provider_block_store_service.py

@@ -22,7 +22,15 @@ class ProviderBlockStoreServiceTestCase(ProviderTestBase):
             name,
             1,
             helpers.get_provider_test_data(self.provider, "placement"))
-        with helpers.exception_action(lambda: test_vol.delete()):
+
+        def cleanup_vol(vol):
+            vol.delete()
+            vol.wait_for(
+                [VolumeState.DELETED, VolumeState.UNKNOWN],
+                terminal_states=[VolumeState.ERROR],
+                interval=self.get_test_wait_interval())
+
+        with helpers.cleanup_action(lambda: cleanup_vol(test_vol)):
             test_vol.wait_till_ready(interval=self.get_test_wait_interval())
             volumes = self.provider.block_store.volumes.list()
             found_volumes = [vol for vol in volumes if vol.name == name]
@@ -47,18 +55,12 @@ class ProviderBlockStoreServiceTestCase(ProviderTestBase):
                 " expected: {2}" .format(found_volumes[0].name,
                                          get_vol.name,
                                          test_vol.name))
-
-            test_vol.delete()
-            test_vol.wait_for(
-                [VolumeState.DELETED, VolumeState.UNKNOWN],
-                terminal_states=[VolumeState.ERROR],
-                interval=self.get_test_wait_interval())
-            volumes = self.provider.block_store.volumes.list()
-            found_volumes = [vol for vol in volumes if vol.name == name]
-            self.assertTrue(
-                len(found_volumes) == 0,
-                "Volume %s should have been deleted but still exists." %
-                name)
+        volumes = self.provider.block_store.volumes.list()
+        found_volumes = [vol for vol in volumes if vol.name == name]
+        self.assertTrue(
+            len(found_volumes) == 0,
+            "Volume %s should have been deleted but still exists." %
+            name)
 
     def test_attach_detach_volume(self):
         """
@@ -68,11 +70,11 @@ class ProviderBlockStoreServiceTestCase(ProviderTestBase):
             self.provider.name,
             uuid.uuid4())
         test_instance = helpers.get_test_instance(self.provider, instance_name)
-        with helpers.exception_action(lambda: test_instance.terminate()):
+        with helpers.cleanup_action(lambda: test_instance.terminate()):
             name = "CBUnitTestAttachVol-{0}".format(uuid.uuid4())
             test_vol = self.provider.block_store.volumes.create(
                 name, 1, test_instance.placement_zone)
-            with helpers.exception_action(lambda: test_vol.delete()):
+            with helpers.cleanup_action(lambda: test_vol.delete()):
                 test_vol.wait_till_ready(
                     interval=self.get_test_wait_interval())
                 test_vol.attach(test_instance, '/dev/sda2')
@@ -85,7 +87,6 @@ class ProviderBlockStoreServiceTestCase(ProviderTestBase):
                     [VolumeState.AVAILABLE],
                     terminal_states=[VolumeState.ERROR, VolumeState.DELETED],
                     interval=self.get_test_wait_interval())
-                test_vol.delete()
 
     def test_crud_snapshot(self):
         """
@@ -98,7 +99,7 @@ class ProviderBlockStoreServiceTestCase(ProviderTestBase):
             name,
             1,
             helpers.get_provider_test_data(self.provider, "placement"))
-        with helpers.exception_action(lambda: test_vol.delete()):
+        with helpers.cleanup_action(lambda: test_vol.delete()):
             test_vol.wait_till_ready(interval=self.get_test_wait_interval())
             snap_name = "CBSnapshot-{0}".format(name)
             test_snap = test_vol.create_snapshot(name=snap_name,
@@ -111,7 +112,7 @@ class ProviderBlockStoreServiceTestCase(ProviderTestBase):
                     terminal_states=[SnapshotState.ERROR],
                     interval=self.get_test_wait_interval())
 
-            with helpers.exception_action(lambda: cleanup_snap(test_snap)):
+            with helpers.cleanup_action(lambda: cleanup_snap(test_snap)):
                 test_snap.wait_till_ready(
                     interval=self.get_test_wait_interval())
                 snaps = self.provider.block_store.snapshots.list()
@@ -138,12 +139,10 @@ class ProviderBlockStoreServiceTestCase(ProviderTestBase):
                     " expected: {2}" .format(found_snaps[0].name,
                                              get_snap.name,
                                              test_snap.name))
-
-                cleanup_snap(test_snap)
-                snaps = self.provider.block_store.snapshots.list()
-                found_snaps = [snap for snap in snaps
-                               if snap.name == snap_name]
-                self.assertTrue(
-                    len(found_snaps) == 0,
-                    "Snapshot %s should have been deleted but still exists." %
-                    snap_name)
+            snaps = self.provider.block_store.snapshots.list()
+            found_snaps = [snap for snap in snaps
+                           if snap.name == snap_name]
+            self.assertTrue(
+                len(found_snaps) == 0,
+                "Snapshot %s should have been deleted but still exists." %
+                snap_name)

test/test_provider_compute_service.py

@@ -21,7 +21,15 @@ class ProviderComputeServiceTestCase(ProviderTestBase):
             self.provider.name,
             uuid.uuid4())
         inst = helpers.create_test_instance(self.provider, name)
-        with helpers.exception_action(lambda: inst.terminate()):
+
+        def cleanup_inst(instance):
+            instance.terminate()
+            instance.wait_for(
+                [InstanceState.TERMINATED, InstanceState.UNKNOWN],
+                terminal_states=[InstanceState.ERROR],
+                interval=self.get_test_wait_interval())
+
+        with helpers.cleanup_action(lambda: cleanup_inst(inst)):
             inst.wait_till_ready(interval=self.get_test_wait_interval())
             all_instances = self.provider.compute.instances.list()
             found_instances = [i for i in all_instances if i.name == name]
@@ -46,20 +54,14 @@ class ProviderComputeServiceTestCase(ProviderTestBase):
                 " expected: {2}" .format(found_instances[0].name,
                                          get_inst.name,
                                          inst.name))
-
-            inst.terminate()
-            inst.wait_for(
-                [InstanceState.TERMINATED, InstanceState.UNKNOWN],
-                terminal_states=[InstanceState.ERROR],
-                interval=self.get_test_wait_interval())
-            deleted_inst = self.provider.compute.instances.get(
-                inst.instance_id)
-            self.assertTrue(
-                deleted_inst is None or deleted_inst.state in (
-                    InstanceState.TERMINATED,
-                    InstanceState.UNKNOWN),
-                "Instance %s should have been deleted but still exists." %
-                name)
+        deleted_inst = self.provider.compute.instances.get(
+            inst.instance_id)
+        self.assertTrue(
+            deleted_inst is None or deleted_inst.state in (
+                InstanceState.TERMINATED,
+                InstanceState.UNKNOWN),
+            "Instance %s should have been deleted but still exists." %
+            name)
 
     def _is_valid_ip(self, address):
         try:
@@ -74,7 +76,7 @@ class ProviderComputeServiceTestCase(ProviderTestBase):
             uuid.uuid4())
         test_instance = helpers.get_test_instance(self.provider,
                                                   instance_name)
-        with helpers.exception_action(lambda: test_instance.terminate()):
+        with helpers.cleanup_action(lambda: test_instance.terminate()):
             self.assertEqual(
                 test_instance.name, instance_name,
                 "Instance name {0} is not equal to the expected name"
@@ -96,17 +98,16 @@ class ProviderComputeServiceTestCase(ProviderTestBase):
             self.assertTrue(
                 self._is_valid_ip(ip_address),
                 "Instance must have a valid IP address")
-            test_instance.terminate()
 
     def test_block_device_mappings(self):
         name = "CBInstBlkMap-{0}-{1}".format(
             self.provider.name,
             uuid.uuid4())
 
-        outer_inst = helpers.create_test_instance(self.provider, name)
-        with helpers.exception_action(lambda: outer_inst.terminate()):
+        outer_inst = helpers.get_test_instance(self.provider, name)
+        with helpers.cleanup_action(lambda: outer_inst.terminate()):
             img = outer_inst.create_image(name)
-            with helpers.exception_action(lambda: img.delete()):
+            with helpers.cleanup_action(lambda: img.delete()):
                 lc = self.provider.compute.instances.create_launch_config()
 
                 # specifying no size with a destination of volume should raise
@@ -163,7 +164,7 @@ class ProviderComputeServiceTestCase(ProviderTestBase):
                     self.provider,
                     name,
                     launch_config=lc)
-                with helpers.exception_action(lambda: inst.terminate()):
+                with helpers.cleanup_action(lambda: inst.terminate()):
                     inst.wait_till_ready(
                         interval=self.get_test_wait_interval())
                     inst.terminate()

test/test_provider_image_service.py

@@ -21,10 +21,18 @@ class ProviderImageServiceTestCase(ProviderTestBase):
             self.provider.name,
             uuid.uuid4())
         test_instance = helpers.get_test_instance(self.provider, instance_name)
-        with helpers.exception_action(lambda: test_instance.terminate()):
+        with helpers.cleanup_action(lambda: test_instance.terminate()):
             name = "CBUnitTestListImg-{0}".format(uuid.uuid4())
             test_image = test_instance.create_image(name)
-            with helpers.exception_action(lambda: test_image.delete()):
+
+            def cleanup_img(img):
+                img.delete()
+                img.wait_for(
+                    [MachineImageState.UNKNOWN],
+                    terminal_states=[MachineImageState.ERROR],
+                    interval=self.get_test_wait_interval())
+
+            with helpers.cleanup_action(lambda: cleanup_img(test_image)):
                 test_image.wait_till_ready(
                     interval=self.get_test_wait_interval())
 
@@ -57,18 +65,12 @@ class ProviderImageServiceTestCase(ProviderTestBase):
                     " expected: {2}" .format(found_images[0].name,
                                              get_img.name,
                                              test_image.name))
-
-                test_image.delete()
-                test_image.wait_for(
-                    [MachineImageState.UNKNOWN],
-                    terminal_states=[MachineImageState.ERROR],
-                    interval=self.get_test_wait_interval())
-                # TODO: Images take a long time to deregister on EC2. Needs
-                # investigation
-                images = self.provider.images.list()
-                found_images = [image for image in images
-                                if image.name == name]
-                self.assertTrue(
-                    len(found_images) == 0,
-                    "Image %s should have been deleted but still exists." %
-                    name)
+            # TODO: Images take a long time to deregister on EC2. Needs
+            # investigation
+            images = self.provider.images.list()
+            found_images = [image for image in images
+                            if image.name == name]
+            self.assertTrue(
+                len(found_images) == 0,
+                "Image %s should have been deleted but still exists." %
+                name)

test/test_provider_object_store_service.py

@@ -18,7 +18,7 @@ class ProviderObjectStoreServiceTestCase(ProviderTestBase):
         """
         name = "cbtestcreatecontainer-{0}".format(uuid.uuid4())
         test_container = self.provider.object_store.create(name)
-        with helpers.exception_action(lambda: test_container.delete()):
+        with helpers.cleanup_action(lambda: test_container.delete()):
             containers = self.provider.object_store.list()
             found_containers = [c for c in containers if c.name == name]
             self.assertTrue(
@@ -35,14 +35,12 @@ class ProviderObjectStoreServiceTestCase(ProviderTestBase):
                 " expected: {2}" .format(found_containers[0].name,
                                          get_container.name,
                                          test_container.name))
-
-            test_container.delete()
-            containers = self.provider.object_store.list()
-            found_containers = [c for c in containers if c.name == name]
-            self.assertTrue(
-                len(found_containers) == 0,
-                "Container %s should have been deleted but still exists." %
-                name)
+        containers = self.provider.object_store.list()
+        found_containers = [c for c in containers if c.name == name]
+        self.assertTrue(
+            len(found_containers) == 0,
+            "Container %s should have been deleted but still exists." %
+            name)
 
     def test_crud_container_objects(self):
         """
@@ -57,11 +55,11 @@ class ProviderObjectStoreServiceTestCase(ProviderTestBase):
         objects = test_container.list()
         self.assertEqual([], objects)
 
-        with helpers.exception_action(lambda: test_container.delete()):
+        with helpers.cleanup_action(lambda: test_container.delete()):
             obj_name = "hello_world.txt"
             obj = test_container.create_object(obj_name)
 
-            with helpers.exception_action(lambda: obj.delete()):
+            with helpers.cleanup_action(lambda: obj.delete()):
                 # TODO: This is wrong. We shouldn't have to have a separate
                 # call to upload some content before being able to delete
                 # the content. Maybe the create_object method should accept
@@ -73,26 +71,23 @@ class ProviderObjectStoreServiceTestCase(ProviderTestBase):
                     len(found_objs) == 1,
                     "List container objects does not return the expected"
                     " object %s" % obj_name)
-
-                obj.delete()
-                objs = test_container.list()
-                found_objs = [o for o in objs if o.name == obj_name]
-                self.assertTrue(
-                    len(found_objs) == 0,
-                    "Object %s should have been deleted but still exists." %
-                    obj_name)
-            test_container.delete()
+            objs = test_container.list()
+            found_objs = [o for o in objs if o.name == obj_name]
+            self.assertTrue(
+                len(found_objs) == 0,
+                "Object %s should have been deleted but still exists." %
+                obj_name)
 
     def test_upload_download_container_content(self):
 
         name = "cbtestcontainerobjs-{0}".format(uuid.uuid4())
         test_container = self.provider.object_store.create(name)
 
-        with helpers.exception_action(lambda: test_container.delete()):
+        with helpers.cleanup_action(lambda: test_container.delete()):
             obj_name = "hello_upload_download.txt"
             obj = test_container.create_object(obj_name)
 
-            with helpers.exception_action(lambda: obj.delete()):
+            with helpers.cleanup_action(lambda: obj.delete()):
                 content = b"Hello World. Here's some content"
                 # TODO: Upload and download methods accept different parameter
                 # types. Need to make this consistent - possibly provider
@@ -101,5 +96,3 @@ class ProviderObjectStoreServiceTestCase(ProviderTestBase):
                 target_stream = BytesIO()
                 obj.download(target_stream)
                 self.assertEqual(target_stream.getvalue(), content)
-                obj.delete()
-            test_container.delete()

test/test_provider_security_service.py

@@ -12,7 +12,7 @@ class ProviderSecurityServiceTestCase(ProviderTestBase):
     def test_crud_key_pair_service(self):
         name = 'cbtestkeypairA-{0}'.format(uuid.uuid4())
         kp = self.provider.security.key_pairs.create(name=name)
-        with helpers.exception_action(
+        with helpers.cleanup_action(
             lambda:
                 self.provider.security.key_pairs.delete(name=kp.name)
         ):
@@ -22,13 +22,12 @@ class ProviderSecurityServiceTestCase(ProviderTestBase):
                 len(found_kp) == 1,
                 "List key pairs did not return the expected key {0}."
                 .format(name))
-            self.provider.security.key_pairs.delete(name=kp.name)
-            kpl = self.provider.security.key_pairs.list()
-            found_kp = [k for k in kpl if k.name == name]
-            self.assertTrue(
-                len(found_kp) == 0,
-                "Key pair {0} should have been deleted but still exists."
-                .format(name))
+        kpl = self.provider.security.key_pairs.list()
+        found_kp = [k for k in kpl if k.name == name]
+        self.assertTrue(
+            len(found_kp) == 0,
+            "Key pair {0} should have been deleted but still exists."
+            .format(name))
         no_kp = self.provider.security.key_pairs.delete(name='bogus_kp')
         self.assertTrue(
             no_kp,
@@ -37,10 +36,7 @@ class ProviderSecurityServiceTestCase(ProviderTestBase):
     def test_key_pair(self):
         name = 'cbtestkeypairB-{0}'.format(uuid.uuid4())
         kp = self.provider.security.key_pairs.create(name=name)
-        with helpers.exception_action(
-            lambda:
-                self.provider.security.key_pairs.delete(name=kp.name)
-        ):
+        with helpers.cleanup_action(lambda: kp.delete()):
             kpl = self.provider.security.key_pairs.list()
             found_kp = [k for k in kpl if k.name == name]
             self.assertTrue(
@@ -56,19 +52,18 @@ class ProviderSecurityServiceTestCase(ProviderTestBase):
             self.assertTrue(
                 kp == kp,
                 "The same key pair should be equal to self.")
-            kp.delete()
-            kpl = self.provider.security.key_pairs.list()
-            found_kp = [k for k in kpl if k.name == name]
-            self.assertTrue(
-                len(found_kp) == 0,
-                "Key pair {0} should have been deleted but still exists."
-                .format(name))
+        kpl = self.provider.security.key_pairs.list()
+        found_kp = [k for k in kpl if k.name == name]
+        self.assertTrue(
+            len(found_kp) == 0,
+            "Key pair {0} should have been deleted but still exists."
+            .format(name))
 
     def test_crud_security_group_service(self):
         name = 'cbtestsecuritygroupA-{0}'.format(uuid.uuid4())
         sg = self.provider.security.security_groups.create(
             name=name, description=name)
-        with helpers.exception_action(
+        with helpers.cleanup_action(
             lambda:
                 self.provider.security.security_groups.delete(group_id=sg.id)
         ):
@@ -80,23 +75,19 @@ class ProviderSecurityServiceTestCase(ProviderTestBase):
                 len(found_sg) == 1,
                 "List security groups did not return the expected group {0}."
                 .format(name))
-            self.provider.security.security_groups.delete(group_id=sg.id)
-            sgl = self.provider.security.security_groups.list()
-            found_sg = [g for g in sgl if g.name == name]
-            self.assertTrue(
-                len(found_sg) == 0,
-                "Security group {0} should have been deleted but still exists."
-                .format(name))
+        sgl = self.provider.security.security_groups.list()
+        found_sg = [g for g in sgl if g.name == name]
+        self.assertTrue(
+            len(found_sg) == 0,
+            "Security group {0} should have been deleted but still exists."
+            .format(name))
 
     def test_security_group(self):
         """Test for proper creation of a security group."""
         name = 'cbtestsecuritygroupB-{0}'.format(uuid.uuid4())
         sg = self.provider.security.security_groups.create(
             name=name, description=name)
-        with helpers.exception_action(
-            lambda:
-                self.provider.security.security_groups.delete(group_id=sg.id)
-        ):
+        with helpers.cleanup_action(lambda: sg.delete()):
             sg.add_rule(ip_protocol='tcp', from_port=1111, to_port=1111,
                         cidr_ip='0.0.0.0/0')
             found_rules = [rule for rule in sg.rules if
@@ -121,23 +112,19 @@ class ProviderSecurityServiceTestCase(ProviderTestBase):
             self.assertFalse(
                 sg != sg,
                 "The same security groups should still be equal?")
-            sg.delete()
-            sgl = self.provider.security.security_groups.list()
-            found_sg = [g for g in sgl if g.name == name]
-            self.assertTrue(
-                len(found_sg) == 0,
-                "Security group {0} should have been deleted but still exists."
-                .format(name))
+        sgl = self.provider.security.security_groups.list()
+        found_sg = [g for g in sgl if g.name == name]
+        self.assertTrue(
+            len(found_sg) == 0,
+            "Security group {0} should have been deleted but still exists."
+            .format(name))
 
     def test_security_group_group_role(self):
         """Test for proper creation of a security group rule."""
         name = 'cbtestsecuritygroupC-{0}'.format(uuid.uuid4())
         sg = self.provider.security.security_groups.create(
             name=name, description=name)
-        with helpers.exception_action(
-            lambda:
-                self.provider.security.security_groups.delete(group_id=sg.id)
-        ):
+        with helpers.cleanup_action(lambda: sg.delete()):
             self.assertTrue(
                 len(sg.rules) == 0,
                 "Expected no security group group rule. Got {0}."
@@ -147,10 +134,9 @@ class ProviderSecurityServiceTestCase(ProviderTestBase):
                 sg.rules[0].group.name == name,
                 "Expected security group rule name {0}. Got {1}."
                 .format(name, sg.rules[0].group.name))
-            sg.delete()
-            sgl = self.provider.security.security_groups.list()
-            found_sg = [g for g in sgl if g.name == name]
-            self.assertTrue(
-                len(found_sg) == 0,
-                "Security group {0} should have been deleted but still exists."
-                .format(name))
+        sgl = self.provider.security.security_groups.list()
+        found_sg = [g for g in sgl if g.name == name]
+        self.assertTrue(
+            len(found_sg) == 0,
+            "Security group {0} should have been deleted but still exists."
+            .format(name))