Jelajahi Sumber

Changed service.delete() to support both objects and ids

Nuwan Goonasekera 7 tahun lalu
induk
melakukan
3d7237ef29

+ 6 - 6
cloudbridge/cloud/base/resources.py

@@ -324,7 +324,7 @@ class BaseInstance(BaseCloudResource, BaseObjectLifeCycleMixin, Instance):
             interval=interval)
 
     def delete(self):
-        self._provider.compute.instances.delete(self.id)
+        self._provider.compute.instances.delete(self)
 
 
 class BaseLaunchConfig(LaunchConfig):
@@ -537,7 +537,7 @@ class BaseKeyPair(BaseCloudResource, KeyPair):
         self._private_material = value
 
     def delete(self):
-        self._provider.security.key_pairs.delete(self.id)
+        self._provider.security.key_pairs.delete(self)
 
 
 class BaseVMFirewall(BaseCloudResource, VMFirewall):
@@ -586,7 +586,7 @@ class BaseVMFirewall(BaseCloudResource, VMFirewall):
         """
         Delete this VM firewall.
         """
-        return self._provider.security.vm_firewalls.delete(self.id)
+        return self._provider.security.vm_firewalls.delete(self)
 
 
 class BaseVMFirewallRuleContainer(BasePageableObjectMixin,
@@ -831,7 +831,7 @@ class BaseNetwork(BaseCloudResource, BaseObjectLifeCycleMixin, Network):
             interval=interval)
 
     def delete(self):
-        self._provider.networking.networks.delete(self.id)
+        self._provider.networking.networks.delete(self)
 
     def create_subnet(self, label, cidr_block, zone):
         return self._provider.networking.subnets.create(
@@ -872,7 +872,7 @@ class BaseSubnet(BaseCloudResource, BaseObjectLifeCycleMixin, Subnet):
             interval=interval)
 
     def delete(self):
-        self._provider.networking.subnets.delete(self.id)
+        self._provider.networking.subnets.delete(self)
 
 
 class BaseFloatingIPContainer(FloatingIPContainer, BasePageableObjectMixin):
@@ -940,7 +940,7 @@ class BaseRouter(BaseCloudResource, Router):
                 self.id == other.id)
 
     def delete(self):
-        self._provider.networking.routers.delete(self.id)
+        self._provider.networking.routers.delete(self)
 
 
 class BaseInternetGateway(BaseCloudResource, BaseObjectLifeCycleMixin,

+ 18 - 42
cloudbridge/cloud/base/services.py

@@ -152,9 +152,9 @@ class BaseKeyPairService(
         return self.dispatch(self, "provider.security.key_pairs.create",
                              name,  public_key_material=public_key_material)
 
-    def delete(self, key_pair_id):
+    def delete(self, key_pair):
         return self.dispatch(self, "provider.security.key_pairs.delete",
-                             key_pair_id)
+                             key_pair)
 
     @implement(event_pattern="provider.security.key_pairs.delete",
                priority=BaseCloudService.STANDARD_EVENT_PRIORITY)
@@ -222,15 +222,12 @@ class BaseVMFirewallService(
         return self.dispatch(self, "provider.security.vm_firewalls.create",
                              label, network, description)
 
-    def delete(self, vm_firewall_id):
+    def delete(self, vm_firewall):
         """
         Delete an existing firewall.
-
-        :type vm_firewall_id: str
-        :param vm_firewall_id: The ID of the firewall to be deleted.
         """
         return self.dispatch(self, "provider.security.vm_firewalls.delete",
-                             vm_firewall_id)
+                             vm_firewall)
 
     @implement(event_pattern="provider.security.vm_firewalls.find",
                priority=BaseCloudService.STANDARD_EVENT_PRIORITY)
@@ -312,15 +309,12 @@ class BaseVolumeService(
         return self.dispatch(self, "provider.storage.volumes.create",
                              label, size, zone, snapshot, description)
 
-    def delete(self, volume_id):
+    def delete(self, volume):
         """
         Delete an existing volume.
-
-        :type volume_id: str
-        :param volume_id: The ID of the volume to be deleted.
         """
         return self.dispatch(self, "provider.storage.volumes.delete",
-                             volume_id)
+                             volume)
 
 
 class BaseSnapshotService(
@@ -378,15 +372,12 @@ class BaseSnapshotService(
         return self.dispatch(self, "provider.storage.snapshots.create",
                              label, volume, description)
 
-    def delete(self, snapshot_id):
+    def delete(self, snapshot):
         """
         Delete an existing snapshot.
-
-        :type snapshot_id: str
-        :param snapshot_id: The ID of the snapshot to be deleted.
         """
         return self.dispatch(self, "provider.storage.snapshots.delete",
-                             snapshot_id)
+                             snapshot)
 
 
 class BaseBucketService(
@@ -458,15 +449,12 @@ class BaseBucketService(
         return self.dispatch(self, "provider.storage.buckets.create",
                              name, location=location)
 
-    def delete(self, bucket_id):
+    def delete(self, bucket):
         """
         Delete an existing bucket.
-
-        :type bucket_id: str
-        :param bucket_id: The ID of the bucket to be deleted.
         """
         return self.dispatch(self, "provider.storage.buckets.delete",
-                             bucket_id)
+                             bucket)
 
 
 class BaseBucketObjectService(
@@ -584,15 +572,12 @@ class BaseInstanceService(
                              user_data=user_data, launch_config=launch_config,
                              **kwargs)
 
-    def delete(self, instance_id):
+    def delete(self, instance):
         """
         Delete an existing instance.
-
-        :type instance_id: str
-        :param instance_id: The ID of the instance to be deleted.
         """
         return self.dispatch(self, "provider.compute.instances.delete",
-                             instance_id)
+                             instance)
 
 
 class BaseVMTypeService(
@@ -790,15 +775,12 @@ class BaseNetworkService(
         return self.dispatch(self, "provider.networking.networks.create",
                              label, cidr_block)
 
-    def delete(self, network_id):
+    def delete(self, network):
         """
         Delete an existing network.
-
-        :type network_id: str
-        :param network_id: The ID of the network to be deleted.
         """
         return self.dispatch(self, "provider.networking.networks.delete",
-                             network_id)
+                             network)
 
 
 class BaseSubnetService(
@@ -887,15 +869,12 @@ class BaseSubnetService(
         return self.dispatch(self, "provider.networking.subnets.create",
                              label, network, cidr_block, zone)
 
-    def delete(self, subnet_id):
+    def delete(self, subnet):
         """
         Delete an existing subnet.
-
-        :type subnet_id: str
-        :param subnet_id: The ID of the subnet to be deleted.
         """
         return self.dispatch(self, "provider.networking.subnets.delete",
-                             subnet_id)
+                             subnet)
 
 
 class BaseRouterService(
@@ -964,12 +943,9 @@ class BaseRouterService(
         return self.dispatch(self, "provider.networking.routers.create",
                              label, network)
 
-    def delete(self, router_id):
+    def delete(self, router):
         """
         Delete an existing router.
-
-        :type router_id: str
-        :param router_id: The ID of the router to be deleted.
         """
         return self.dispatch(self, "provider.networking.routers.delete",
-                             router_id)
+                             router)

+ 10 - 10
cloudbridge/cloud/interfaces/services.py

@@ -671,12 +671,12 @@ class NetworkService(PageableObjectMixin, CloudService):
         pass
 
     @abstractmethod
-    def delete(self, network_id):
+    def delete(self, network):
         """
         Delete an existing Network.
 
-        :type network_id: ``str``
-        :param network_id: The ID of the network to be deleted.
+        :type network: ``str`` or :class:`.Network`
+        :param network: The object or id of the network to be deleted.
         """
         pass
 
@@ -1188,12 +1188,12 @@ class KeyPairService(PageableObjectMixin, CloudService):
         pass
 
     @abstractmethod
-    def delete(self, key_pair_id):
+    def delete(self, key_pair):
         """
-        Delete an existing VMFirewall.
+        Delete an existing keypair.
 
-        :type key_pair_id: str
-        :param key_pair_id: The id of the key pair to be deleted.
+        :type key_pair: ``str`` or :class:`.KeyPair`
+        :param key_pair: The object or id of the key pair to be deleted.
 
         :rtype: ``bool``
         :return:  ``True`` if the key does not exist, ``False`` otherwise. Note
@@ -1274,12 +1274,12 @@ class VMFirewallService(PageableObjectMixin, CloudService):
         pass
 
     @abstractmethod
-    def delete(self, group_id):
+    def delete(self, firewall):
         """
         Delete an existing VMFirewall.
 
-        :type group_id: str
-        :param group_id: The VM firewall ID to be deleted.
+        :type firewall: ``str`` or :class:`.VMFirewall`
+        :param firewall: The object or VM firewall ID to be deleted.
         """
         pass
 

+ 45 - 33
cloudbridge/cloud/providers/aws/services.py

@@ -134,9 +134,11 @@ class AWSKeyPairService(BaseKeyPairService):
 
     @implement(event_pattern="provider.security.key_pairs.delete",
                priority=BaseKeyPairService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, key_pair_id):
-        aws_kp = self.svc.get_raw(key_pair_id)
-        aws_kp.delete()
+    def _delete(self, kp):
+        key_pair = kp if isinstance(kp, AWSKeyPair) else self.get(kp)
+        if key_pair:
+            # pylint:disable=protected-access
+            key_pair._key_pair.delete()
 
 
 class AWSVMFirewallService(BaseVMFirewallService):
@@ -186,9 +188,11 @@ class AWSVMFirewallService(BaseVMFirewallService):
 
     @implement(event_pattern="provider.security.vm_firewalls.delete",
                priority=BaseVMFirewallService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, vm_firewall_id):
-        aws_fw = self.svc.get_raw(vm_firewall_id)
-        aws_fw.delete()
+    def _delete(self, vmf):
+        firewall = vmf if isinstance(vmf, AWSVMFirewall) else self.get(vmf)
+        if firewall:
+            # pylint:disable=protected-access
+            firewall._vm_firewall.delete()
 
 
 class AWSStorageService(BaseStorageService):
@@ -270,12 +274,11 @@ class AWSVolumeService(BaseVolumeService):
 
     @implement(event_pattern="provider.storage.volumes.delete",
                priority=BaseVolumeService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, volume_id):
-        if isinstance(volume_id, AWSVolume):
-            aws_vol = volume_id._volume
-        else:
-            aws_vol = self.svc.get_raw(volume_id)
-        aws_vol.delete()
+    def _delete(self, vol):
+        volume = vol if isinstance(vol, AWSVolume) else self.get(vol)
+        if volume:
+            # pylint:disable=protected-access
+            volume._volume.delete()
 
 
 class AWSSnapshotService(BaseSnapshotService):
@@ -329,11 +332,11 @@ class AWSSnapshotService(BaseSnapshotService):
 
     @implement(event_pattern="provider.storage.snapshots.delete",
                priority=BaseSnapshotService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, snapshot_id):
-        if isinstance(snapshot_id, AWSSnapshot):
-            aws_snap = snapshot_id._snapshot
-        aws_snap = self.svc.get_raw(snapshot_id)
-        aws_snap.delete()
+    def _delete(self, snap):
+        snapshot = snap if isinstance(snap, AWSSnapshot) else self.get(snap)
+        if snapshot:
+            # pylint:disable=protected-access
+            snapshot._snapshot.delete()
 
 
 class AWSBucketService(BaseBucketService):
@@ -416,10 +419,11 @@ class AWSBucketService(BaseBucketService):
 
     @implement(event_pattern="provider.storage.buckets.delete",
                priority=BaseBucketService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, bucket_id):
-        bucket = self._get(bucket_id)
-        if bucket:
-            bucket._bucket.delete()
+    def _delete(self, bucket):
+        b = bucket if isinstance(bucket, AWSBucket) else self.get(bucket)
+        if b:
+            # pylint:disable=protected-access
+            b._bucket.delete()
 
 
 class AWSBucketObjectService(BaseBucketObjectService):
@@ -699,9 +703,11 @@ class AWSInstanceService(BaseInstanceService):
 
     @implement(event_pattern="provider.compute.instances.delete",
                priority=BaseInstanceService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, instance_id):
-        aws_ins = self.svc.get_raw(instance_id)
-        aws_ins.terminate()
+    def _delete(self, inst):
+        aws_inst = inst if isinstance(inst, AWSInstance) else self.get(inst)
+        if aws_inst:
+            # pylint:disable=protected-access
+            aws_inst._ec2_instance.terminate()
 
 
 class AWSVMTypeService(BaseVMTypeService):
@@ -839,9 +845,11 @@ class AWSNetworkService(BaseNetworkService):
 
     @implement(event_pattern="provider.networking.networks.delete",
                priority=BaseNetworkService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, network_id):
-        net = self.get(network_id)
-        net._vpc.delete()
+    def _delete(self, net):
+        network = net if isinstance(net, AWSNetwork) else self.get(net)
+        if network:
+            # pylint:disable=protected-access
+            network._vpc.delete()
 
     def get_or_create_default(self):
         # # Look for provided default network
@@ -919,9 +927,11 @@ class AWSSubnetService(BaseSubnetService):
 
     @implement(event_pattern="provider.networking.subnets.delete",
                priority=BaseSubnetService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, subnet_id):
-        aws_sn = self.svc.get_raw(subnet_id)
-        aws_sn.delete()
+    def _delete(self, subnet):
+        sn = subnet if isinstance(subnet, AWSSubnet) else self.get(subnet)
+        if sn:
+            # pylint:disable=protected-access
+            sn._subnet.delete()
 
     def get_or_create_default(self, zone):
         zone_name = zone.name if isinstance(zone, AWSPlacementZone) else zone
@@ -1071,6 +1081,8 @@ class AWSRouterService(BaseRouterService):
 
     @implement(event_pattern="provider.networking.routers.delete",
                priority=BaseRouterService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, router_id):
-        aws_router = self.svc.get_raw(router_id)
-        aws_router.delete()
+    def _delete(self, router):
+        r = router if isinstance(router, AWSRouter) else self.get(router)
+        if r:
+            # pylint:disable=protected-access
+            r._route_table.delete()

+ 35 - 25
cloudbridge/cloud/providers/azure/services.py

@@ -142,8 +142,9 @@ class AzureVMFirewallService(BaseVMFirewallService):
 
     @implement(event_pattern="provider.security.vm_firewalls.delete",
                priority=BaseVMFirewallService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, group_id):
-        self.provider.azure_client.delete_vm_firewall(group_id)
+    def _delete(self, vmf):
+        fw_id = vmf.id if isinstance(vmf, AzureVMFirewall) else vmf
+        self.provider.azure_client.delete_vm_firewall(fw_id)
 
 
 class AzureKeyPairService(BaseKeyPairService):
@@ -223,9 +224,11 @@ class AzureKeyPairService(BaseKeyPairService):
 
     @implement(event_pattern="provider.security.key_pairs.delete",
                priority=BaseKeyPairService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, key_pair_id):
-        az_kp = self.provider.azure_client.get_public_key(key_pair_id)
-        self.provider.azure_client.delete_public_key(az_kp)
+    def _delete(self, kp):
+        key_pair = kp if isinstance(kp, AzureKeyPair) else self.get(kp)
+        if key_pair:
+            # pylint:disable=protected-access
+            self.provider.azure_client.delete_public_key(key_pair._key_pair)
 
 
 class AzureStorageService(BaseStorageService):
@@ -460,11 +463,12 @@ class AzureBucketService(BaseBucketService):
 
     @implement(event_pattern="provider.storage.buckets.delete",
                priority=BaseBucketService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, bucket_id):
+    def _delete(self, bucket):
         """
         Delete this bucket.
         """
-        self.provider.azure_client.delete_container(bucket_id)
+        b_id = bucket.id if isinstance(bucket, AzureBucket) else bucket
+        self.provider.azure_client.delete_container(b_id)
 
 
 class AzureBucketObjectService(BaseBucketObjectService):
@@ -899,14 +903,16 @@ class AzureInstanceService(BaseInstanceService):
 
     @implement(event_pattern="provider.compute.instances.delete",
                priority=BaseInstanceService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, instance_id):
+    def _delete(self, inst):
         """
         Permanently terminate this instance.
         After deleting the VM. we are deleting the network interface
         associated to the instance, and also removing OS disk and data disks
         where tag with name 'delete_on_terminate' has value True.
         """
-        ins = self.get(instance_id)
+        ins = inst if isinstance(inst, AzureInstance) else self.get(inst)
+        if not inst:
+            return
 
         # Remove IPs first to avoid a network interface conflict
         for public_ip_id in ins._public_ip_ids:
@@ -1038,8 +1044,10 @@ class AzureNetworkService(BaseNetworkService):
 
     @implement(event_pattern="provider.networking.networks.delete",
                priority=BaseNetworkService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, network_id):
-        self.provider.azure_client.delete_network(network_id)
+    def _delete(self, net):
+        net_id = net.id if isinstance(net, AzureNetwork) else net
+        if net_id:
+            self.provider.azure_client.delete_network(net_id)
 
 
 class AzureSubnetService(BaseSubnetService):
@@ -1129,17 +1137,18 @@ class AzureSubnetService(BaseSubnetService):
 
     @implement(event_pattern="provider.networking.subnets.delete",
                priority=BaseSubnetService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, subnet_id):
-        sn = self.get(subnet_id)
-        self.provider.azure_client.delete_subnet(subnet_id)
-        # Although Subnet doesn't support labels, we use the parent Network's
-        # tags to track the subnet's labels, thus that network-level tag must
-        # be deleted with the subnet
-        net_id = sn.network_id
-        az_network = self.provider.azure_client.get_network(net_id)
-        az_network.tags.pop(sn.tag_name)
-        self._provider.azure_client.update_network_tags(
-            az_network.id, az_network)
+    def _delete(self, subnet):
+        sn = subnet if isinstance(subnet, AzureSubnet) else self.get(subnet)
+        if sn:
+            self.provider.azure_client.delete_subnet(sn.id)
+            # Although Subnet doesn't support labels, we use the parent
+            # Network's tags to track the subnet's labels, thus that
+            # network-level tag must be deleted with the subnet
+            net_id = sn.network_id
+            az_network = self.provider.azure_client.get_network(net_id)
+            az_network.tags.pop(sn.tag_name)
+            self._provider.azure_client.update_network_tags(
+                az_network.id, az_network)
 
 
 class AzureRouterService(BaseRouterService):
@@ -1197,6 +1206,7 @@ class AzureRouterService(BaseRouterService):
 
     @implement(event_pattern="provider.networking.routers.delete",
                priority=BaseRouterService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, router_id):
-        az_router = self.provider.azure_client.get_route_table(router_id)
-        self.provider.azure_client.delete_route_table(az_router.name)
+    def _delete(self, router):
+        r = router if isinstance(router, AzureRouter) else self.get(router)
+        if r:
+            self.provider.azure_client.delete_route_table(r.name)

+ 0 - 3
cloudbridge/cloud/providers/gce/resources.py

@@ -1376,9 +1376,6 @@ class GCENetwork(BaseNetwork):
     def subnets(self):
         return list(self._provider.networking.subnets.iter(network=self))
 
-    def delete(self):
-        self._provider.networking.networks.delete(self)
-
     def create_subnet(self, label, cidr_block, zone):
         return self._provider.networking.subnets.create(
             label, self, cidr_block, zone)

+ 76 - 85
cloudbridge/cloud/providers/gce/services.py

@@ -148,8 +148,8 @@ class GCEKeyPairService(BaseKeyPairService):
 
     @implement(event_pattern="provider.security.key_pairs.delete",
                priority=BaseKeyPairService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, key_pair_id):
-        kp = self.get(key_pair_id)
+    def _delete(self, kp):
+        kp = kp if isinstance(kp, GCEKeyPair) else self.get(kp)
         if kp:
             helpers.remove_metadata_item(
                 self.provider, GCEKeyPair.KP_TAG_PREFIX + kp.name)
@@ -199,8 +199,9 @@ class GCEVMFirewallService(BaseVMFirewallService):
 
     @implement(event_pattern="provider.security.vm_firewalls.delete",
                priority=BaseVMFirewallService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, vm_firewall_id):
-        return self._delegate.delete_tag_network_with_id(vm_firewall_id)
+    def _delete(self, vmf):
+        fw_id = vmf.id if isinstance(vmf, GCEVMFirewall) else vmf
+        return self._delegate.delete_tag_network_with_id(fw_id)
 
     def find_by_network_and_tags(self, network_name, tags):
         """
@@ -587,17 +588,16 @@ class GCEInstanceService(BaseInstanceService):
 
     @implement(event_pattern="provider.compute.instances.delete",
                priority=BaseInstanceService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, instance_id):
-        gce_instance = self.provider.get_resource('instances', instance_id)
-        (self._provider
-         .gce_compute
-         .instances()
-         .delete(project=self.provider.project_name,
-                 zone=self.provider
-                          .parse_url(gce_instance.get('zone'))
-                          .parameters['zone'],
-                 instance=gce_instance.get('name'))
-         .execute())
+    def _delete(self, inst):
+        instance = inst if isinstance(inst, GCEInstance) else self.get(inst)
+        if instance:
+            (self._provider
+             .gce_compute
+             .instances()
+             .delete(project=self.provider.project_name,
+                     zone=instance.zone_name,
+                     instance=instance.name)
+             .execute())
 
     def create_launch_config(self):
         return GCELaunchConfig(self.provider)
@@ -727,15 +727,15 @@ class GCENetworkService(BaseNetworkService):
 
     @implement(event_pattern="provider.networking.networks.delete",
                priority=BaseNetworkService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, network_id):
+    def _delete(self, network):
         # Accepts network object
-        if isinstance(network_id, GCENetwork):
-            name = network_id.name
+        if isinstance(network, GCENetwork):
+            name = network.name
         # Accepts both name and ID
-        elif 'googleapis' in network_id:
-            name = network_id.split('/')[-1]
+        elif 'googleapis' in network:
+            name = network.split('/')[-1]
         else:
-            name = network_id
+            name = network
         response = (self.provider
                         .gce_compute
                         .networks()
@@ -747,7 +747,7 @@ class GCENetworkService(BaseNetworkService):
         tag_name = "_".join(["network", name, "label"])
         if not helpers.remove_metadata_item(self.provider, tag_name):
             log.warning('No label was found associated with this network '
-                        '"{}" when deleted.'.format(network_id))
+                        '"{}" when deleted.'.format(network))
         return True
 
 
@@ -822,23 +822,21 @@ class GCERouterService(BaseRouterService):
 
     @implement(event_pattern="provider.networking.routers.delete",
                priority=BaseRouterService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, router_id):
-        region_name = self.provider.region_name
-        gce_router = self.provider.get_resource(
-            'routers', router_id, region=region_name)
-        name = gce_router['name']
-        (self.provider
-         .gce_compute
-         .routers()
-         .delete(project=self.provider.project_name,
-                 region=region_name,
-                 router=name)
-         .execute())
-        # Remove label
-        tag_name = "_".join(["router", name, "label"])
-        if not helpers.remove_metadata_item(self.provider, tag_name):
-            log.warning('No label was found associated with this router '
-                        '"{}" when deleted.'.format(name))
+    def _delete(self, router):
+        r = router if isinstance(router, GCERouter) else self.get(router)
+        if r:
+            (self.provider
+             .gce_compute
+             .routers()
+             .delete(project=self.provider.project_name,
+                     region=r.region_name,
+                     router=r.name)
+             .execute())
+            # Remove label
+            tag_name = "_".join(["router", r.name, "label"])
+            if not helpers.remove_metadata_item(self.provider, tag_name):
+                log.warning('No label was found associated with this router '
+                            '"{}" when deleted.'.format(r.name))
 
     def _get_in_region(self, router_id, region=None):
         region_name = self.provider.region_name
@@ -935,22 +933,23 @@ class GCESubnetService(BaseSubnetService):
 
     @implement(event_pattern="provider.networking.subnets.delete",
                priority=BaseSubnetService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, subnet_id):
-        gce_sn = self.provider.get_resource('subnetworks', subnet_id)
-        region_name = self.provider.parse_url(subnet_id).parameters['region']
+    def _delete(self, subnet):
+        sn = subnet if isinstance(subnet, GCESubnet) else self.get(subnet)
+        if not sn:
+            return
         response = (self.provider
                     .gce_compute
                     .subnetworks()
                     .delete(project=self.provider.project_name,
-                            region=region_name,
-                            subnetwork=gce_sn['name'])
+                            region=sn.region_name,
+                            subnetwork=sn.name)
                     .execute())
-        self.provider.wait_for_operation(response, region=region_name)
+        self.provider.wait_for_operation(response, region=sn.region_name)
         # Remove label
-        tag_name = "_".join(["subnet", gce_sn['name'], "label"])
+        tag_name = "_".join(["subnet", sn.name, "label"])
         if not helpers.remove_metadata_item(self._provider, tag_name):
             log.warning('No label was found associated with this subnet '
-                        '"{}" when deleted.'.format(gce_sn['name']))
+                        '"{}" when deleted.'.format(sn.name))
 
     def get_or_create_default(self, zone):
         """
@@ -1151,19 +1150,15 @@ class GCEVolumeService(BaseVolumeService):
 
     @implement(event_pattern="provider.storage.volumes.delete",
                priority=BaseVolumeService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, volume_id):
-        if isinstance(volume_id, GCEVolume):
-            gce_vol = volume_id._volume
-        else:
-            gce_vol = self.provider.get_resource('disks', volume_id)
-        zone_name = (self.provider.parse_url(gce_vol.get('zone'))
-                         .parameters['zone'])
-        (self._provider.gce_compute
-                       .disks()
-                       .delete(project=self.provider.project_name,
-                               zone=zone_name,
-                               disk=gce_vol.get('name'))
-                       .execute())
+    def _delete(self, vol):
+        volume = vol if isinstance(vol, GCEVolume) else self.get(vol)
+        if volume:
+            (self._provider.gce_compute
+                           .disks()
+                           .delete(project=self.provider.project_name,
+                                   zone=volume.zone_name,
+                                   disk=volume.name)
+                           .execute())
 
 
 class GCESnapshotService(BaseSnapshotService):
@@ -1254,17 +1249,15 @@ class GCESnapshotService(BaseSnapshotService):
 
     @implement(event_pattern="provider.storage.snapshots.delete",
                priority=BaseSnapshotService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, snapshot_id):
-        if isinstance(snapshot_id, GCESnapshot):
-            gce_snap = snapshot_id._snapshot
-        else:
-            gce_snap = self.provider.get_resource('snapshots', snapshot_id)
-        (self.provider
-             .gce_compute
-             .snapshots()
-             .delete(project=self.provider.project_name,
-                     snapshot=gce_snap.get('name'))
-             .execute())
+    def _delete(self, snap):
+        snapshot = snap if isinstance(snap, GCESnapshot) else self.get(snap)
+        if snapshot:
+            (self.provider
+                 .gce_compute
+                 .snapshots()
+                 .delete(project=self.provider.project_name,
+                         snapshot=snapshot.name)
+                 .execute())
 
 
 class GCSBucketService(BaseBucketService):
@@ -1351,23 +1344,21 @@ class GCSBucketService(BaseBucketService):
 
     @implement(event_pattern="provider.storage.buckets.delete",
                priority=BaseBucketService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, bucket_id):
+    def _delete(self, bucket):
         """
         Delete this bucket.
         """
-        # GCE uses name rather than URL to identify resources
-        name = (self.provider._storage_resources
-                    .parse_url(bucket_id)
-                    .parameters.get("bucket"))
-        (self.provider
-             .gcs_storage
-             .buckets()
-             .delete(bucket=name)
-             .execute())
-        # GCS has a rate limit of 1 operation per 2 seconds for bucket
-        # creation/deletion: https://cloud.google.com/storage/quotas. Throttle
-        # here to avoid future failures.
-        time.sleep(2)
+        b = bucket if isinstance(bucket, GCSBucket) else self.get(bucket)
+        if b:
+            (self.provider
+                 .gcs_storage
+                 .buckets()
+                 .delete(bucket=b.name)
+                 .execute())
+            # GCS has a rate limit of 1 operation per 2 seconds for bucket
+            # creation/deletion: https://cloud.google.com/storage/quotas.
+            # Throttle here to avoid future failures.
+            time.sleep(2)
 
 
 class GCSBucketObjectService(BaseBucketObjectService):

+ 47 - 51
cloudbridge/cloud/providers/openstack/services.py

@@ -200,9 +200,11 @@ class OpenStackKeyPairService(BaseKeyPairService):
 
     @implement(event_pattern="provider.security.key_pairs.delete",
                priority=BaseKeyPairService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, key_pair_id):
-        os_kp = self.provider.nova.keypairs.get(key_pair_id)
-        os_kp.delete()
+    def _delete(self, kp):
+        keypair = kp if isinstance(kp, OpenStackKeyPair) else self.get(kp)
+        if keypair:
+            # pylint:disable=protected-access
+            keypair._key_pair.delete()
 
 
 class OpenStackVMFirewallService(BaseVMFirewallService):
@@ -255,15 +257,11 @@ class OpenStackVMFirewallService(BaseVMFirewallService):
 
     @implement(event_pattern="provider.security.vm_firewalls.delete",
                priority=BaseVMFirewallService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, vm_firewall_id):
-        try:
-            os_fw = self.provider.os_conn.network.get_security_group(
-                vm_firewall_id)
-            os_fw.delete(self.provider.os_conn.session)
-            return True
-        except (ResourceNotFound, NotFoundException):
-            log.debug("Firewall %s not found.", vm_firewall_id)
-            return True
+    def _delete(self, vmf):
+        fw = vmf if isinstance(vmf, OpenStackVMFirewall) else self.get(vmf)
+        if fw:
+            # pylint:disable=protected-access
+            fw._vm_firewall.delete(self.provider.os_conn.session)
 
 
 class OpenStackStorageService(BaseStorageService):
@@ -356,12 +354,11 @@ class OpenStackVolumeService(BaseVolumeService):
 
     @implement(event_pattern="provider.storage.volumes.delete",
                priority=BaseVolumeService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, volume_id):
-        if isinstance(volume_id, OpenStackVolume):
-            os_vol = volume_id._volume
-        else:
-            os_vol = self.provider.cinder.volumes.get(volume_id)
-        os_vol.delete()
+    def _delete(self, vol):
+        volume = vol if isinstance(vol, OpenStackVolume) else self.get(vol)
+        if volume:
+            # pylint:disable=protected-access
+            volume._volume.delete()
 
 
 class OpenStackSnapshotService(BaseSnapshotService):
@@ -427,12 +424,11 @@ class OpenStackSnapshotService(BaseSnapshotService):
 
     @implement(event_pattern="provider.storage.snapshots.delete",
                priority=BaseSnapshotService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, snapshot_id):
-        if isinstance(snapshot_id, OpenStackSnapshot):
-            os_snap = snapshot_id._snapshot
-        else:
-            os_snap = self.provider.cinder.volume_snapshots.get(snapshot_id)
-        os_snap.delete()
+    def _delete(self, snap):
+        s = snap if isinstance(snap, OpenStackSnapshot) else self.get(snap)
+        if s:
+            # pylint:disable=protected-access
+            s._snapshot.delete()
 
 
 class OpenStackBucketService(BaseBucketService):
@@ -498,8 +494,9 @@ class OpenStackBucketService(BaseBucketService):
 
     @implement(event_pattern="provider.storage.buckets.delete",
                priority=BaseBucketService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, bucket_id):
-        self.provider.swift.delete_container(bucket_id)
+    def _delete(self, bucket):
+        b_id = bucket.id if isinstance(bucket, OpenStackBucket) else bucket
+        self.provider.swift.delete_container(b_id)
 
 
 class OpenStackBucketObjectService(BaseBucketObjectService):
@@ -809,18 +806,17 @@ class OpenStackInstanceService(BaseInstanceService):
 
     @implement(event_pattern="provider.compute.instances.delete",
                priority=BaseInstanceService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, instance_id):
-        try:
-            os_instance = self.provider.nova.servers.get(instance_id)
-        except NovaNotFound:
-            log.debug("Instance %s was not found.", instance_id)
-            return None
-        # delete the port we created when launching
-        # Assumption: it's the first interface in the list
-        iface_list = os_instance.interface_list()
-        if iface_list:
-            self.provider.neutron.delete_port(iface_list[0].port_id)
-        os_instance.delete()
+    def _delete(self, inst):
+        ins = inst if isinstance(inst, OpenStackInstance) else self.get(inst)
+        if ins:
+            # pylint:disable=protected-access
+            os_instance = ins._os_instance
+            # delete the port we created when launching
+            # Assumption: it's the first interface in the list
+            iface_list = os_instance.interface_list()
+            if iface_list:
+                self.provider.neutron.delete_port(iface_list[0].port_id)
+            os_instance.delete()
 
 
 class OpenStackVMTypeService(BaseVMTypeService):
@@ -954,13 +950,15 @@ class OpenStackNetworkService(BaseNetworkService):
 
     @implement(event_pattern="provider.networking.networks.delete",
                priority=BaseNetworkService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, network_id):
-        network = self.get(network_id)
-        if not network.external and network_id in str(
+    def _delete(self, net):
+        network = net if isinstance(net, OpenStackNetwork) else self.get(net)
+        if not network:
+            return
+        if not network.external and network.id in str(
                 self.provider.neutron.list_networks()):
             # If there are ports associated with the network, it won't delete
             ports = self.provider.neutron.list_ports(
-                network_id=network_id).get('ports', [])
+                network_id=network.id).get('ports', [])
             for port in ports:
                 try:
                     self.provider.neutron.delete_port(port.get('id'))
@@ -968,7 +966,7 @@ class OpenStackNetworkService(BaseNetworkService):
                     # Ports could have already been deleted if instances
                     # are terminated etc. so exceptions can be safely ignored
                     pass
-            self.provider.neutron.delete_network(network_id)
+            self.provider.neutron.delete_network(network.id)
 
 
 class OpenStackSubnetService(BaseSubnetService):
@@ -1011,12 +1009,9 @@ class OpenStackSubnetService(BaseSubnetService):
 
     @implement(event_pattern="provider.networking.subnets.delete",
                priority=BaseSubnetService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, subnet_id):
-        self.provider.neutron.delete_subnet(subnet_id)
-        # Adhere to the interface docs
-        if subnet_id not in self:
-            return True
-        return False
+    def _delete(self, subnet):
+        sn_id = subnet.id if isinstance(subnet, OpenStackSubnet) else subnet
+        self.provider.neutron.delete_subnet(sn_id)
 
     def get_or_create_default(self, zone):
         """
@@ -1079,5 +1074,6 @@ class OpenStackRouterService(BaseRouterService):
 
     @implement(event_pattern="provider.networking.routers.delete",
                priority=BaseRouterService.STANDARD_EVENT_PRIORITY)
-    def _delete(self, router_id):
-        self._provider.os_conn.delete_router(router_id)
+    def _delete(self, router):
+        r_id = router.id if isinstance(router, OpenStackRouter) else router
+        self.provider.os_conn.delete_router(r_id)

+ 1 - 1
test/test_security_service.py

@@ -44,7 +44,7 @@ class CloudSecurityServiceTestCase(ProviderTestBase):
 
         def cleanup_kp(kp):
             if kp:
-                self.provider.security.key_pairs.delete(key_pair_id=kp.id)
+                self.provider.security.key_pairs.delete(key_pair=kp.id)
 
         def extra_tests(kp):
             # Recreating existing keypair should raise an exception