Fixed remaining errors in instance tests for boto3

cloudbridge/cloud/providers/aws/resources.py

@@ -108,9 +108,13 @@ class AWSMachineImage(BaseMachineImage):
         :rtype: ``int``
         :return: The minimum disk size needed by this image
         """
-        bdm = self._ec2_image.block_device_mapping.get(
-            self._ec2_image.root_device_name)
-        return bdm.size if bdm else None
+        vols = [bdm.get('Ebs', {}) for bdm in
+                self._ec2_image.block_device_mappings if
+                bdm.get('DeviceName') == self._ec2_image.root_device_name]
+        if vols:
+            return vols[0].get('VolumeSize')
+        else:
+            return None
 
     def delete(self):
         """
@@ -387,41 +391,49 @@ class AWSInstance(BaseInstance):
                 'PublicIp': None if self._ec2_instance.vpc_id else ip_address,
                 'AllocationId':
                     None if not self._ec2_instance.vpc_id else
-                    ip_address.id if isinstance(ip_address, 'FloatingIP') else
+                    ip_address.id if isinstance(ip_address, AWSFloatingIP) else
                     [
                         x for x in
-                        self._provider.network.floating_ips()
+                        self._provider.networking.networks.floating_ips
                         if x.public_ip == ip_address
                     ][0].id
             }.items() if v is not None
         })
+        self.refresh()
 
     def remove_floating_ip(self, ip_address):
         """
         Remove a elastic IP address from this instance.
         """
-        ip_addr = self._provider._vpc_conn.get_all_addresses([ip_address])[0]
-        if self._ec2_instance.vpc_id:
-            return self._provider.ec2_conn.disassociate_address(
-                association_id=ip_addr.association_id)
-        else:
-            return self._provider.ec2_conn.disassociate_address(
-                public_ip=ip_addr.public_ip)
+        return self._provider.ec2_conn.meta.client.disassociate_address(**{
+            k: v for k, v in {
+                'PublicIp': None if self._ec2_instance.vpc_id else ip_address,
+                'AssociationId':
+                    None if not self._ec2_instance.vpc_id else
+                    ip_address._ip.association_id if
+                    isinstance(ip_address, AWSFloatingIP) else
+                    [
+                        x for x in
+                        self._ec2_instance.vpc_addresses.all()
+                        if x.public_ip == ip_address
+                    ][0].association_id
+            }.items() if v is not None
+        })
 
     def add_security_group(self, sg):
         """
         Add a security group to this instance
         """
         self._ec2_instance.modify_attribute(
-            'groupSet', [g.id for g in self._ec2_instance.groups] + [sg.id])
+            Groups=self.security_group_ids + [sg.id])
 
     def remove_security_group(self, sg):
         """
         Remove a security group from this instance
         """
         self._ec2_instance.modify_attribute(
-            'groupSet', [g.id for g in self._ec2_instance.groups
-                         if g.id != sg.id])
+            Groups=([sg_id for sg_id in self.security_group_ids
+                     if sg_id != sg.id]))
 
     @property
     def state(self):
@@ -443,9 +455,6 @@ class AWSInstance(BaseInstance):
             # set the status to unknown
             self._ec2_instance.state = {'Name': InstanceState.UNKNOWN}
 
-    def wait_till_ready(self, timeout=None, interval=None):
-        self._ec2_instance.wait_until_running()
-
     def wait_till_exists(self, timeout=None, interval=None):
         self._ec2_instance.wait_until_exists()
 

cloudbridge/cloud/providers/aws/services.py

@@ -516,12 +516,15 @@ class AWSInstanceService(BaseInstanceService):
             if device.is_volume:
                 # Generate the device path
                 bdm['DeviceName'] = \
-                    '/dev/sd' + 'a1' if device.is_root else next(next_letter)
+                    '/dev/sd' + ('a1' if device.is_root else next(next_letter))
                 ebs_def = {}
                 if isinstance(device.source, Snapshot):
                     ebs_def['SnapshotId'] = device.source.id
                 elif isinstance(device.source, Volume):
-                    ebs_def['VolumeId'] = device.source.id
+                    # TODO: We could create a snapshot from the volume
+                    # and use that instead.
+                    # Not supported
+                    pass
                 elif isinstance(device.source, MachineImage):
                     # Not supported
                     pass

test/test_block_store_service.py

@@ -161,8 +161,8 @@ class CloudBlockStoreServiceTestCase(ProviderTestBase):
                 return self.provider.block_store.snapshots.create(
                     name=name, volume=test_vol, description=name)
 
-            if not isinstance(self.provider, TestMockHelperMixin) and \
-               self.provider.PROVIDER_ID == ProviderList.AWS:
+            if (self.provider.PROVIDER_ID == ProviderList.AWS and
+                    not isinstance(self.provider, TestMockHelperMixin)):
                 time.sleep(15)  # Or get SnapshotCreationPerVolumeRateExceeded
             sit.check_crud(self, self.provider.block_store.snapshots, Snapshot,
                            "cb_snaptwo", create_snap2, cleanup_snap)

test/test_compute_service.py

@@ -7,7 +7,6 @@ from test.helpers import standard_interface_tests as sit
 from cloudbridge.cloud.factory import ProviderList
 from cloudbridge.cloud.interfaces import InstanceState
 from cloudbridge.cloud.interfaces import InvalidConfigurationException
-from cloudbridge.cloud.interfaces import TestMockHelperMixin
 from cloudbridge.cloud.interfaces.exceptions import WaitStateException
 from cloudbridge.cloud.interfaces.resources import Instance
 from cloudbridge.cloud.interfaces.resources import InstanceType
@@ -134,10 +133,6 @@ class CloudComputeServiceTestCase(ProviderTestBase):
                 itype.name, expected_type,
                 "Instance type {0} does not match expected type {1}".format(
                     itype.name, expected_type))
-#             if isinstance(self.provider, TestMockHelperMixin):
-#                 raise self.skipTest(
-#                     "Skipping rest of test because Moto is not returning the"
-#                     " instance's placement zone correctly")
             find_zone = [zone for zone in
                          self.provider.compute.regions.current.zones
                          if zone.id == test_instance.zone_id]
@@ -345,16 +340,14 @@ class CloudComputeServiceTestCase(ProviderTestBase):
                 fip = (self.provider.networking.networks
                        .create_floating_ip())
                 with helpers.cleanup_action(lambda: fip.delete()):
-                    test_inst.add_floating_ip(fip.public_ip)
-                    test_inst.refresh()
-                    # On Devstack, the floating IP is listed under private_ips.
-                    self.assertIn(fip.public_ip,
-                                  test_inst.public_ips + test_inst.private_ips)
-                    if isinstance(self.provider, TestMockHelperMixin):
-                        # TODO: Moto bug does not refresh removed public ip
-                        return
-                    # check whether removing an elastic ip works
-                    test_inst.remove_floating_ip(fip.public_ip)
+                    with helpers.cleanup_action(
+                            lambda: test_inst.remove_floating_ip(
+                                fip.public_ip)):
+                        test_inst.add_floating_ip(fip.public_ip)
+                        test_inst.refresh()
+                        # On Devstack, FloatingIP is listed under private_ips.
+                        self.assertIn(fip.public_ip, test_inst.public_ips +
+                                      test_inst.private_ips)
                     test_inst.refresh()
                     self.assertNotIn(
                         fip.public_ip,

test/test_image_service.py

@@ -3,7 +3,6 @@ from test.helpers import ProviderTestBase
 from test.helpers import standard_interface_tests as sit
 
 from cloudbridge.cloud.interfaces import MachineImageState
-from cloudbridge.cloud.interfaces import TestMockHelperMixin
 from cloudbridge.cloud.interfaces.resources import MachineImage
 
 
@@ -33,12 +32,10 @@ class CloudImageServiceTestCase(ProviderTestBase):
                 [MachineImageState.UNKNOWN, MachineImageState.ERROR])
 
         def extra_tests(img):
-            # TODO: Fix moto so that the BDM is populated correctly
-            if not isinstance(self.provider, TestMockHelperMixin):
-                # check image size
-                img.refresh()
-                self.assertGreater(img.min_disk, 0, "Minimum disk"
-                                   " size required by image is invalid")
+            # check image size
+            img.refresh()
+            self.assertGreater(img.min_disk, 0, "Minimum disk"
+                               " size required by image is invalid")
 
         with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
                 test_instance, net)):