Fix the GCEPlacementZone resource

Changes:
  - Changed this to behave more like other resources.
  - Fix cases where a zone URL (ID) is passed instead of a zone name or a
    GCEPlacementZone.
  - Explicitly set the disk name of new instances created with a boot image.

Test stats after this CL
========================
Ran 58 tests in 1514.083s

FAILED (SKIP=1, errors=23, failures=8)

cloudbridge/cloud/providers/gce/resources.py

@@ -143,15 +143,9 @@ class GCEVMType(BaseVMType):
 
 class GCEPlacementZone(BasePlacementZone):
 
-    def __init__(self, provider, zone, region):
+    def __init__(self, provider, zone):
         super(GCEPlacementZone, self).__init__(provider)
-        if isinstance(zone, GCEPlacementZone):
-            # pylint:disable=protected-access
-            self._gce_zone = zone._gce_zone
-            self._gce_region = zone._gce_region
-        else:
-            self._gce_zone = zone
-            self._gce_region = region
+        self._zone = zone
 
     @property
     def id(self):
@@ -160,7 +154,7 @@ class GCEPlacementZone(BasePlacementZone):
         :rtype: ``str``
         :return: ID for this zone as returned by the cloud middleware.
         """
-        return self._gce_zone.get('selfLink')
+        return self._zone['selfLink']
 
     @property
     def name(self):
@@ -169,7 +163,7 @@ class GCEPlacementZone(BasePlacementZone):
         :rtype: ``str``
         :return: Name for this zone as returned by the cloud middleware.
         """
-        return self._gce_zone.get('name')
+        return self._zone['name']
 
     @property
     def region_name(self):
@@ -178,7 +172,8 @@ class GCEPlacementZone(BasePlacementZone):
         :rtype: ``str``
         :return: Name of this zone's region as returned by the cloud middleware
         """
-        return self._gce_region
+        parsed_region_url = self._provider.parse_url(self._zone['region'])
+        return parsed_region_url.parameters['region']
 
 
 class GCERegion(BaseRegion):
@@ -207,8 +202,7 @@ class GCERegion(BaseRegion):
                               .execute())
         zones = [zone for zone in zones_response['items']
                  if zone['region'] == self._gce_region['selfLink']]
-        return [GCEPlacementZone(self._provider, zone, self.name)
-                for zone in zones]
+        return [GCEPlacementZone(self._provider, zone) for zone in zones]
 
 
 class GCEFirewallsDelegate(object):
@@ -1886,8 +1880,11 @@ class GCESnapshot(BaseSnapshot):
                 'pd-ssd'.
             iops: Not supported by GCE.
         """
+        zone_name = placement
+        if isinstance(placement, GCEPlacementZone):
+            zone_name = placement.name
         vol_type = 'zones/{0}/diskTypes/{1}'.format(
-            placement,
+            zone_name,
             'pd-standard' if (volume_type != 'pd-standard' or
                               volume_type != 'pd-ssd') else volume_type)
         disk_body = {
@@ -1900,7 +1897,7 @@ class GCESnapshot(BaseSnapshot):
                          .gce_compute
                          .disks()
                          .insert(project=self._provider.project_name,
-                                 zone=placement,
+                                 zone=zone_name,
                                  body=disk_body)
                          .execute())
         return self._provider.storage.volumes.get(

cloudbridge/cloud/providers/gce/services.py

@@ -22,7 +22,6 @@ from cloudbridge.cloud.base.services import BaseSubnetService
 from cloudbridge.cloud.base.services import BaseVMFirewallService
 from cloudbridge.cloud.base.services import BaseVMTypeService
 from cloudbridge.cloud.base.services import BaseVolumeService
-from cloudbridge.cloud.interfaces.resources import PlacementZone
 from cloudbridge.cloud.interfaces.resources import VMFirewall
 from cloudbridge.cloud.providers.gce import helpers
 
@@ -435,8 +434,13 @@ class GCEInstanceService(BaseInstanceService):
         Creates a new virtual machine instance.
         """
         GCEInstance.assert_valid_resource_name(name)
-        if not zone:
-            zone = self.provider.default_zone
+        zone_name = self.provider.default_zone
+        if zone:
+            if not isinstance(zone, GCEPlacementZone):
+                zone = GCEPlacementZone(
+                    self.provider,
+                    self.provider.get_resource('zones', zone, zone=zone))
+            zone_name = zone.name
         if not isinstance(vm_type, GCEVMType):
             vm_type = self.provider.compute.vm_types.get(vm_type)
 
@@ -502,9 +506,14 @@ class GCEInstanceService(BaseInstanceService):
             else:
                 if not isinstance(image, GCEMachineImage):
                     image = self.provider.compute.images.get(image)
-                boot_disk = {'boot': True,
-                             'autoDelete': True,
-                             'initializeParams': {'sourceImage': image.id}}
+                # Explicitly set diskName; otherwise, instance name will be
+                # used by default which may conflict with existing disks.
+                boot_disk = {
+                    'boot': True,
+                    'autoDelete': True,
+                    'initializeParams': {
+                        'sourceImage': image.id,
+                        'diskName': 'image-disk-{0}'.format(uuid.uuid4())}}
 
         if not boot_disk:
             cb.log.warning('No boot disk is given')
@@ -533,7 +542,7 @@ class GCEInstanceService(BaseInstanceService):
             operation = (self.provider
                              .gce_compute.instances()
                              .insert(project=self.provider.project_name,
-                                     zone=zone,
+                                     zone=zone_name,
                                      body=config)
                              .execute())
         except googleapiclient.errors.HttpError as http_error:
@@ -542,10 +551,8 @@ class GCEInstanceService(BaseInstanceService):
             cb.log.warning(
                 "googleapiclient.errors.HttpError: {0}".format(http_error))
             return None
-        zone_url = self.provider.parse_url(operation['zone'])
         instance_id = operation.get('targetLink')
-        self.provider.wait_for_operation(operation,
-                                         zone=zone_url.parameters['zone'])
+        self.provider.wait_for_operation(operation, zone=zone_name)
         return self.get(instance_id)
 
     def get(self, instance_id):
@@ -941,13 +948,12 @@ class GCESubnetService(BaseSubnetService):
         self._provider.wait_for_operation(response, region=subnet.region)
 
     def _zone_to_region(self, zone):
-        if isinstance(zone, GCEPlacementZone):
+        if zone:
+            if not isinstance(zone, GCEPlacementZone):
+                zone = GCEPlacementZone(
+                    self.provider,
+                    self.provider.get_resource('zones', zone, zone=zone))
             return zone.region_name
-        elif zone:
-            for r in self.provider.compute.regions.list():
-                for z in r.zones:
-                    if zone == z.name:
-                        return z.region_name
         return self.provider.region_name
 
 
@@ -1050,7 +1056,11 @@ class GCEVolumeService(BaseVolumeService):
         cannot be a dash.
         """
         GCEVolume.assert_valid_resource_name(name)
-        zone_name = zone.name if isinstance(zone, PlacementZone) else zone
+        if not isinstance(zone, GCEPlacementZone):
+            zone = GCEPlacementZone(
+                self.provider,
+                self.provider.get_resource('zones', zone, zone=zone))
+        zone_name = zone.name
         snapshot_id = snapshot.id if isinstance(
             snapshot, GCESnapshot) and snapshot else snapshot
         disk_body = {