Increased test coverage for instance properties and block device

mappings and some minor fixes.

cloudbridge/cloud/providers/aws/resources.py

@@ -279,14 +279,6 @@ class AWSInstance(BaseInstance):
         """
         return AWSPlacementZone(self._provider, self._ec2_instance.placement)
 
-    @property
-    def mac_address(self):
-        """
-        Get the MAC address for this instance.
-        """
-        raise NotImplementedError(
-            'mac_address not implemented by this provider')
-
     @property
     def security_groups(self):
         """
@@ -296,9 +288,7 @@ class AWSInstance(BaseInstance):
         # convert that into a ``SecurityGroup`` object before creating a
         # cloudbridge SecurityGroup object
         names = [group.name for group in self._ec2_instance.groups]
-        security_groups = self._provider.security.security_groups.get(names)
-        return [AWSSecurityGroup(self._provider, group)
-                for group in security_groups]
+        return self._provider.security.security_groups.get(names)
 
     @property
     def key_pair_name(self):
@@ -501,12 +491,6 @@ class AWSSnapshot(BaseSnapshot):
         raise NotImplementedError(
             'create_volume not implemented by this provider')
 
-    def share(self, user_ids=None):
-        raise NotImplementedError('share not implemented by this provider')
-
-    def unshare(self, user_ids=None):
-        raise NotImplementedError('share not implemented by this provider')
-
 
 class AWSKeyPair(BaseKeyPair):
 

cloudbridge/cloud/providers/openstack/resources.py

@@ -294,25 +294,14 @@ class OpenStackInstance(BaseInstance):
             self._provider,
             getattr(self._os_instance, 'OS-EXT-AZ:availability_zone', None))
 
-    @property
-    def mac_address(self):
-        """
-        Get the MAC address for this instance.
-        """
-        raise NotImplementedError(
-            'mac_address not implemented by this provider')
-
     @property
     def security_groups(self):
         """
         Get the security groups associated with this instance.
         """
-        security_groups = []
-        for group in self._os_instance.security_groups:
-            security_groups.append(self._provider.nova.security_groups.find(
-                name=group.get('name')))
-        return [OpenStackSecurityGroup(self._provider, group)
-                for group in security_groups]
+        names = [group.get('name')
+                 for group in self._os_instance.security_groups]
+        return self._provider.security.security_groups.get(names)
 
     @property
     def key_pair_name(self):
@@ -534,12 +523,6 @@ class OpenStackSnapshot(BaseSnapshot):
         raise NotImplementedError(
             'create_volume not implemented by this provider')
 
-    def share(self, user_ids=None):
-        raise NotImplementedError('share not implemented by this provider')
-
-    def unshare(self, user_ids=None):
-        raise NotImplementedError('share not implemented by this provider')
-
 
 class OpenStackKeyPair(BaseKeyPair):
 

requirements.txt

@@ -1,2 +1,2 @@
--e ".[full]"
+-e ".[dev]"
 git+git://github.com/openstack/python-keystoneclient.git@227e1b54cfc4a6a68268f9d5935d258b7490b7a3

test/helpers.py

@@ -41,10 +41,7 @@ def cleanup_action(cleanup_func):
         except Exception as 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))
+    cleanup_func()
 
 
 TEST_DATA_CONFIG = {
@@ -72,12 +69,15 @@ def get_provider_test_data(provider, key):
 
 
 def create_test_instance(
-        provider, instance_name, zone=None, launch_config=None):
+        provider, instance_name, zone=None, launch_config=None,
+        keypair=None, security_groups=None):
     return provider.compute.instances.create(
         instance_name,
         get_provider_test_data(provider, 'image'),
         get_provider_test_data(provider, 'instance_type'),
         zone=zone,
+        keypair=keypair,
+        security_groups=security_groups,
         launch_config=launch_config)
 
 
@@ -88,8 +88,12 @@ def get_provider_wait_interval(provider):
         return 1
 
 
-def get_test_instance(provider, name):
-    instance = create_test_instance(provider, name)
+def get_test_instance(provider, name, keypair=None, security_groups=None):
+    instance = create_test_instance(
+        provider,
+        name,
+        keypair=keypair,
+        security_groups=security_groups)
     instance.wait_till_ready(interval=get_provider_wait_interval(provider))
     return instance
 

test/test_compute_service.py

@@ -52,9 +52,9 @@ class CloudComputeServiceTestCase(ProviderTestBase):
             get_inst = self.provider.compute.instances.get(
                 inst.id)
             self.assertTrue(
-                found_instances[0].id ==
-                get_inst.id == inst.id,
-                "Ids returned by list: {0} and get: {1} are not as "
+                found_instances[0] ==
+                get_inst == inst,
+                "Objects returned by list: {0} and get: {1} are not as "
                 " expected: {2}" .format(found_instances[0].id,
                                          get_inst.id,
                                          inst.id))
@@ -82,26 +82,48 @@ class CloudComputeServiceTestCase(ProviderTestBase):
         return True
 
     def test_instance_properties(self):
-        instance_name = "CBInstProps-{0}-{1}".format(
+        name = "CBInstProps-{0}-{1}".format(
             self.provider.name,
             uuid.uuid4())
+        kp = self.provider.security.key_pairs.create(name=name)
+        sg = self.provider.security.security_groups.create(
+            name=name, description=name)
+
         test_instance = helpers.get_test_instance(self.provider,
-                                                  instance_name)
-        with helpers.cleanup_action(lambda: test_instance.terminate()):
+                                                  name, keypair=kp,
+                                                  security_groups=[sg])
+
+        def cleanup(inst, kp, sg):
+            inst.terminate()
+            inst.wait_for(
+                [InstanceState.TERMINATED, InstanceState.UNKNOWN],
+                terminal_states=[InstanceState.ERROR],
+                interval=self.get_test_wait_interval())
+            kp.delete()
+            sg.delete()
+
+        with helpers.cleanup_action(lambda: cleanup(test_instance, kp, sg)):
             self.assertTrue(
                 test_instance.id in repr(test_instance),
                 "repr(obj) should contain the object id so that the object"
                 " can be reconstructed, but does not. eval(repr(obj)) == obj")
             self.assertEqual(
-                test_instance.name, instance_name,
+                test_instance.name, name,
                 "Instance name {0} is not equal to the expected name"
-                " {1}".format(test_instance.name, instance_name))
+                " {1}".format(test_instance.name, name))
             image_id = helpers.get_provider_test_data(self.provider, "image")
             self.assertEqual(test_instance.image_id, image_id,
                              "Image id {0} is not equal to the expected id"
                              " {1}".format(test_instance.image_id, image_id))
             self.assertIsInstance(test_instance.public_ips, list)
             self.assertIsInstance(test_instance.private_ips, list)
+            self.assertEqual(
+                test_instance.key_pair_name,
+                kp.name)
+            self.assertIsInstance(test_instance.security_groups, list)
+            self.assertEqual(
+                test_instance.security_groups[0],
+                sg)
             # Must have either a public or a private ip
             ip_private = test_instance.private_ips[0] \
                 if test_instance.private_ips else None
@@ -115,11 +137,7 @@ class CloudComputeServiceTestCase(ProviderTestBase):
                 "Instance must have a valid IP address")
             self.assertIsInstance(test_instance.instance_type, InstanceType)
 
-    def test_block_device_mappings(self):
-        name = "CBInstBlkMap-{0}-{1}".format(
-            self.provider.name,
-            uuid.uuid4())
-
+    def test_block_device_mapping_launch_config(self):
         lc = self.provider.compute.instances.create_launch_config()
 
         # specifying an invalid size should raise
@@ -157,6 +175,12 @@ class CloudComputeServiceTestCase(ProviderTestBase):
         with self.assertRaises(InvalidConfigurationException):
             lc.add_volume_device(size=1, is_root=True)
 
+        # Attempting to add an incorrect source should raise an exception
+        with self.assertRaises(InvalidConfigurationException):
+            lc.add_volume_device(
+                source="invalid_source",
+                delete_on_terminate=True)
+
         # Add all available ephemeral devices
         instance_type_name = helpers.get_provider_test_data(
             self.provider,
@@ -172,22 +196,87 @@ class CloudComputeServiceTestCase(ProviderTestBase):
             "Expected %d total block devices bit found %d" %
             (2 + inst_type.num_ephemeral_disks, len(lc.block_devices)))
 
+    def test_block_device_mapping_attachments(self):
+        name = "CBInstBlkAttch-{0}-{1}".format(
+            self.provider.name,
+            uuid.uuid4())
+
+#         test_vol = self.provider.block_store.volumes.create(
+#             name,
+#             1,
+#             helpers.get_provider_test_data(self.provider, "placement"))
+#         with helpers.cleanup_action(lambda: test_vol.delete()):
+#             test_vol.wait_till_ready(interval=self.get_test_wait_interval())
+#             test_snap = test_vol.create_snapshot(name=name,
+#                                                  description=name)
+#
+#             def cleanup_snap(snap):
+#                 snap.delete()
+#                 snap.wait_for(
+#                     [SnapshotState.UNKNOWN],
+#                     terminal_states=[SnapshotState.ERROR],
+#                     interval=self.get_test_wait_interval())
+#
+#             with helpers.cleanup_action(lambda: cleanup_snap(test_snap)):
+#                 test_snap.wait_till_ready(
+#                     interval=self.get_test_wait_interval())
+
+        lc = self.provider.compute.instances.create_launch_config()
+
+        # Add a new blank volume
+        lc.add_volume_device(size=1, delete_on_terminate=True)
+
+        # Attach an existing volume
+#                 lc.add_volume_device(size=1, source=test_vol,
+#                                      delete_on_terminate=True)
+
+        # Add a new volume based on a snapshot
+#                 lc.add_volume_device(size=1, source=test_snap,
+#                                      delete_on_terminate=True)
+
+        # Override root volume size
+        image_id = helpers.get_provider_test_data(
+            self.provider,
+            "image")
+        img = self.provider.compute.images.get(image_id)
+        lc.add_volume_device(
+            is_root=True,
+            source=img,
+            # TODO: This should be greater than the ami size or tests
+            # will fail on actual infrastructure. Needs an image.size
+            # method
+            size=2,
+            delete_on_terminate=True)
+
+        # Add all available ephemeral devices
+        instance_type_name = helpers.get_provider_test_data(
+            self.provider,
+            "instance_type")
+        inst_type = next(self.provider.compute.instance_types.find(
+            name=instance_type_name))
+        for _ in range(inst_type.num_ephemeral_disks):
+            lc.add_ephemeral_device()
+
         inst = helpers.create_test_instance(
             self.provider,
             name,
-            zone=helpers.get_provider_test_data(self.provider, 'placement'),
+            zone=helpers.get_provider_test_data(
+                self.provider,
+                'placement'),
             launch_config=lc)
-        with helpers.cleanup_action(lambda: inst.terminate()):
+
+        def cleanup(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)):
             try:
                 inst.wait_till_ready(
                     interval=self.get_test_wait_interval())
             except WaitStateException as e:
-                self.fail("The block device mapped launch did not complete"
-                          " successfully: %s" % e)
-            inst.terminate()
-            inst.wait_for(
-                [InstanceState.TERMINATED, InstanceState.UNKNOWN],
-                terminal_states=[InstanceState.ERROR],
-                interval=self.get_test_wait_interval())
+                self.fail("The block device mapped launch did not "
+                          " complete successfully: %s" % e)
             # TODO: Check instance attachments and make sure they
             # correspond to requested mappings

test/test_image_service.py

@@ -30,8 +30,7 @@ class CloudImageServiceTestCase(ProviderTestBase):
             def cleanup_img(img):
                 img.delete()
                 img.wait_for(
-                    [MachineImageState.UNKNOWN],
-                    terminal_states=[MachineImageState.ERROR],
+                    [MachineImageState.UNKNOWN, MachineImageState.ERROR],
                     interval=self.get_test_wait_interval())
 
             with helpers.cleanup_action(lambda: cleanup_img(test_image)):

test/test_security_service.py

@@ -65,7 +65,7 @@ class CloudSecurityServiceTestCase(ProviderTestBase):
                 "List key pairs did not return the expected key {0}."
                 .format(name))
             self.assertTrue(
-                kp.name in repr(kp),
+                kp.id in repr(kp),
                 "repr(obj) should contain the object id so that the object"
                 " can be reconstructed, but does not. eval(repr(obj)) == obj")
             self.assertIsNotNone(
@@ -115,6 +115,10 @@ class CloudSecurityServiceTestCase(ProviderTestBase):
                 len(found_sg) == 1,
                 "List security groups did not return the expected group {0}."
                 .format(name))
+            self.assertTrue(
+                sg.id in repr(sg),
+                "repr(obj) should contain the object id so that the object"
+                " can be reconstructed, but does not. eval(repr(obj)) == obj")
         sgl = self.provider.security.security_groups.list()
         found_sg = [g for g in sgl if g.name == name]
         self.assertTrue(