From 0a5ce7d27b30280e1dbe16979545aeaf30585de5 Mon Sep 17 00:00:00 2001 From: Peter Razumovsky Date: Mon, 17 Aug 2015 17:22:46 +0300 Subject: [PATCH] Refactor repeated handle_delete Add parent handle_delete method based on entity argument and remove all repeated occurrences in Resource children. Change-Id: Ic18708e3780721031ccbba61f2d949be7b837ca3 --- heat/engine/clients/os/barbican.py | 6 +++++ heat/engine/resource.py | 11 ++++++++ .../resources/openstack/barbican/order.py | 15 ----------- .../resources/openstack/barbican/secret.py | 15 ----------- .../resources/openstack/ceilometer/alarm.py | 12 +-------- .../cinder/cinder_encrypted_vol_type.py | 10 ++----- .../openstack/cinder/cinder_volume_type.py | 9 ------- .../resources/openstack/designate/domain.py | 9 ++----- .../openstack/glance/glance_image.py | 9 ------- .../resources/openstack/magnum/baymodel.py | 8 ------ .../openstack/manila/security_service.py | 9 ------- .../resources/openstack/manila/share.py | 9 ------- .../openstack/manila/share_network.py | 6 ----- .../resources/openstack/manila/share_type.py | 9 ------- .../openstack/mistral/cron_trigger.py | 9 ------- .../resources/openstack/nova/nova_flavor.py | 9 ------- .../openstack/nova/nova_floatingip.py | 7 ----- .../resources/openstack/nova/nova_keypair.py | 7 ----- .../openstack/nova/nova_servergroup.py | 7 ----- .../openstack/sahara/sahara_cluster.py | 14 ++-------- .../openstack/sahara/sahara_templates.py | 26 +++---------------- heat/tests/cinder/test_cinder_volume_type.py | 5 +++- .../cinder/test_volume_type_encryption.py | 3 ++- heat/tests/nova/test_nova_flavor.py | 3 ++- heat/tests/openstack/designate/test_domain.py | 3 ++- heat/tests/test_glance_image.py | 3 ++- 26 files changed, 40 insertions(+), 193 deletions(-) diff --git a/heat/engine/clients/os/barbican.py b/heat/engine/clients/os/barbican.py index a7075d0ba..fcc6d2c31 100644 --- a/heat/engine/clients/os/barbican.py +++ b/heat/engine/clients/os/barbican.py @@ -10,6 +10,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import six from heat.engine.clients import client_plugin @@ -30,3 +31,8 @@ class BarbicanClientPlugin(client_plugin.ClientPlugin): session=self._keystone_session, endpoint=endpoint) return client + + def is_not_found(self, ex): + # This is the only exception the client raises + # Inspecting the message to see if it's a 'Not Found' + return 'Not Found' in six.text_type(ex) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 75915e1b1..4035e487d 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -1146,6 +1146,17 @@ class Resource(object): with excutils.save_and_reraise_exception(): self._release(engine_id) + def handle_delete(self): + """Default implementation; should be overridden by resources.""" + if self.entity and self.resource_id is not None: + try: + obj = getattr(self.client(), self.entity) + obj.delete(self.resource_id) + except Exception as ex: + self.client_plugin().ignore_not_found(ex) + return None + return self.resource_id + @scheduler.wrappertask def delete(self): ''' diff --git a/heat/engine/resources/openstack/barbican/order.py b/heat/engine/resources/openstack/barbican/order.py index f64676488..1b8ac31f9 100644 --- a/heat/engine/resources/openstack/barbican/order.py +++ b/heat/engine/resources/openstack/barbican/order.py @@ -11,8 +11,6 @@ # License for the specific language governing permissions and limitations # under the License. -import six - from heat.common import exception from heat.common.i18n import _ from heat.engine import attributes @@ -189,19 +187,6 @@ class Order(resource.Resource): return order.status == 'ACTIVE' - def handle_delete(self): - if not self.resource_id: - return - - client = self.client() - try: - client.orders.delete(self.resource_id) - except Exception as exc: - # This is the only exception the client raises - # Inspecting the message to see if it's a 'Not Found' - if 'Not Found' not in six.text_type(exc): - raise - def _resolve_attribute(self, name): client = self.client() order = client.orders.get(self.resource_id) diff --git a/heat/engine/resources/openstack/barbican/secret.py b/heat/engine/resources/openstack/barbican/secret.py index 8bfd9669d..c271e0e67 100644 --- a/heat/engine/resources/openstack/barbican/secret.py +++ b/heat/engine/resources/openstack/barbican/secret.py @@ -11,8 +11,6 @@ # License for the specific language governing permissions and limitations # under the License. -import six - from heat.common.i18n import _ from heat.engine import attributes from heat.engine import constraints @@ -116,19 +114,6 @@ class Secret(resource.Resource): self.resource_id_set(secret_ref) return secret_ref - def handle_delete(self): - if not self.resource_id: - return - - client = self.client() - try: - client.secrets.delete(self.resource_id) - except Exception as exc: - # This is the only exception the client raises - # Inspecting the message to see if it's a 'Not Found' - if 'Not Found' not in six.text_type(exc): - raise - def _resolve_attribute(self, name): secret = self.client().secrets.get(self.resource_id) diff --git a/heat/engine/resources/openstack/ceilometer/alarm.py b/heat/engine/resources/openstack/ceilometer/alarm.py index 037479860..2c9e1037f 100644 --- a/heat/engine/resources/openstack/ceilometer/alarm.py +++ b/heat/engine/resources/openstack/ceilometer/alarm.py @@ -372,11 +372,7 @@ class CeilometerAlarm(resource.Resource): except exception.WatchRuleNotFound: pass - if self.resource_id is not None: - try: - self.client().alarms.delete(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) + super(CeilometerAlarm, self).handle_delete() def handle_check(self): watch_name = self.physical_resource_name() @@ -424,12 +420,6 @@ class BaseCeilometerAlarm(resource.Resource): self.client().alarms.update( alarm_id=self.resource_id, enabled=True) - def handle_delete(self): - try: - self.client().alarms.delete(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - def handle_check(self): self.client().alarms.get(self.resource_id) diff --git a/heat/engine/resources/openstack/cinder/cinder_encrypted_vol_type.py b/heat/engine/resources/openstack/cinder/cinder_encrypted_vol_type.py index cfb5a583d..d2f9d4d5e 100644 --- a/heat/engine/resources/openstack/cinder/cinder_encrypted_vol_type.py +++ b/heat/engine/resources/openstack/cinder/cinder_encrypted_vol_type.py @@ -30,6 +30,8 @@ class CinderEncryptedVolumeType(resource.Resource): default_client_name = 'cinder' + entity = 'volume_encryption_types' + PROPERTIES = ( PROVIDER, CONTROL_LOCATION, CIPHER, KEY_SIZE, VOLUME_TYPE ) = ( @@ -106,14 +108,6 @@ class CinderEncryptedVolumeType(resource.Resource): volume_type=self.resource_id, specs=prop_diff ) - def handle_delete(self): - if self.resource_id is None: - return - try: - self.client().volume_encryption_types.delete(self.resource_id) - except Exception as e: - self.client_plugin().ignore_not_found(e) - def resource_mapping(): return { diff --git a/heat/engine/resources/openstack/cinder/cinder_volume_type.py b/heat/engine/resources/openstack/cinder/cinder_volume_type.py index 46572f272..d2f8d156a 100644 --- a/heat/engine/resources/openstack/cinder/cinder_volume_type.py +++ b/heat/engine/resources/openstack/cinder/cinder_volume_type.py @@ -96,15 +96,6 @@ class CinderVolumeType(resource.Resource): if new_keys is not None: volume_type.set_keys(new_keys) - def handle_delete(self): - if self.resource_id is None: - return - - try: - self.client().volume_types.delete(self.resource_id) - except Exception as e: - self.client_plugin().ignore_not_found(e) - # TODO(huangtianhua): remove this method when bug #1479641 is fixed. def _show_resource(self): vtype = self.client().volume_types.get(self.resource_id) diff --git a/heat/engine/resources/openstack/designate/domain.py b/heat/engine/resources/openstack/designate/domain.py index 36244e468..a8152f680 100644 --- a/heat/engine/resources/openstack/designate/domain.py +++ b/heat/engine/resources/openstack/designate/domain.py @@ -77,6 +77,8 @@ class DesignateDomain(resource.Resource): default_client_name = 'designate' + entity = 'domains' + def handle_create(self): args = dict( name=self.properties[self.NAME], @@ -105,13 +107,6 @@ class DesignateDomain(resource.Resource): args['id'] = self.resource_id self.client_plugin().domain_update(**args) - def handle_delete(self): - if self.resource_id is not None: - try: - self.client().domains.delete(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - def _resolve_attribute(self, name): if name == self.SERIAL: domain = self.client().domains.get(self.resource_id) diff --git a/heat/engine/resources/openstack/glance/glance_image.py b/heat/engine/resources/openstack/glance/glance_image.py index d9c40f7f9..78930d8d2 100644 --- a/heat/engine/resources/openstack/glance/glance_image.py +++ b/heat/engine/resources/openstack/glance/glance_image.py @@ -114,15 +114,6 @@ class GlanceImage(resource.Resource): image = self.client().images.get(image_id) return image.status == 'active' - def handle_delete(self): - if self.resource_id is None: - return - - try: - self.client().images.delete(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - def _show_resource(self): if self.glance().version == 1.0: return super(GlanceImage, self)._show_resource() diff --git a/heat/engine/resources/openstack/magnum/baymodel.py b/heat/engine/resources/openstack/magnum/baymodel.py index ee249b302..3117c32fb 100644 --- a/heat/engine/resources/openstack/magnum/baymodel.py +++ b/heat/engine/resources/openstack/magnum/baymodel.py @@ -136,14 +136,6 @@ class BayModel(resource.Resource): bm = self.client().baymodels.create(**args) self.resource_id_set(bm.uuid) - def handle_delete(self): - if not self.resource_id: - return - try: - self.client().baymodels.delete(self.resource_id) - except Exception as exc: - self.client_plugin().ignore_not_found(exc) - def resource_mapping(): return { diff --git a/heat/engine/resources/openstack/manila/security_service.py b/heat/engine/resources/openstack/manila/security_service.py index 7e58be997..faa02e7b2 100644 --- a/heat/engine/resources/openstack/manila/security_service.py +++ b/heat/engine/resources/openstack/manila/security_service.py @@ -97,15 +97,6 @@ class SecurityService(resource.Resource): self.client().security_services.update(self.resource_id, **prop_diff) - def handle_delete(self): - if self.resource_id is None: - return - - try: - self.client().security_services.delete(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - def resource_mapping(): return { diff --git a/heat/engine/resources/openstack/manila/share.py b/heat/engine/resources/openstack/manila/share.py index 9bade82b0..c8f228cfa 100644 --- a/heat/engine/resources/openstack/manila/share.py +++ b/heat/engine/resources/openstack/manila/share.py @@ -260,15 +260,6 @@ class ManilaShare(resource.Resource): raise resource.ResourceUnknownStatus(status_reason=reason, resource_status=share_status) - def handle_delete(self): - if not self.resource_id: - return - - try: - self.client().shares.delete(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - def check_delete_complete(self, *args): if not self.resource_id: return True diff --git a/heat/engine/resources/openstack/manila/share_network.py b/heat/engine/resources/openstack/manila/share_network.py index 1e5b91868..f575a240e 100644 --- a/heat/engine/resources/openstack/manila/share_network.py +++ b/heat/engine/resources/openstack/manila/share_network.py @@ -164,12 +164,6 @@ class ManilaShareNetwork(resource.Resource): nova_net_id=prop_diff.get(self.NOVA_NETWORK), description=prop_diff.get(self.DESCRIPTION)) - def handle_delete(self): - try: - self.client().share_networks.delete(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - def resource_mapping(): return {'OS::Manila::ShareNetwork': ManilaShareNetwork} diff --git a/heat/engine/resources/openstack/manila/share_type.py b/heat/engine/resources/openstack/manila/share_type.py index 246ece32f..670ea36ad 100644 --- a/heat/engine/resources/openstack/manila/share_type.py +++ b/heat/engine/resources/openstack/manila/share_type.py @@ -87,15 +87,6 @@ class ManilaShareType(resource.Resource): share_type.unset_keys(extra_specs_old) share_type.set_keys(prop_diff.get(self.EXTRA_SPECS)) - def handle_delete(self): - if not self.resource_id: - return True - - try: - self.client().share_types.delete(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - def resource_mapping(): return { diff --git a/heat/engine/resources/openstack/mistral/cron_trigger.py b/heat/engine/resources/openstack/mistral/cron_trigger.py index faef95262..28da634fb 100644 --- a/heat/engine/resources/openstack/mistral/cron_trigger.py +++ b/heat/engine/resources/openstack/mistral/cron_trigger.py @@ -110,15 +110,6 @@ class CronTrigger(resource.Resource): cron_trigger = self.client().cron_triggers.create(**args) self.resource_id_set(cron_trigger.name) - def handle_delete(self): - if not self.resource_id: - return - - try: - self.client().cron_triggers.delete(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - def _resolve_attribute(self, name): try: trigger = self.client().cron_triggers.get(self.resource_id) diff --git a/heat/engine/resources/openstack/nova/nova_flavor.py b/heat/engine/resources/openstack/nova/nova_flavor.py index 111243874..b1474639d 100644 --- a/heat/engine/resources/openstack/nova/nova_flavor.py +++ b/heat/engine/resources/openstack/nova/nova_flavor.py @@ -130,15 +130,6 @@ class NovaFlavor(resource.Resource): if new_keys is not None: flavor.set_keys(new_keys) - def handle_delete(self): - if self.resource_id is None: - return - - try: - self.client().flavors.delete(self.resource_id) - except Exception as e: - self.client_plugin().ignore_not_found(e) - def resource_mapping(): return { diff --git a/heat/engine/resources/openstack/nova/nova_floatingip.py b/heat/engine/resources/openstack/nova/nova_floatingip.py index 5916a7c4f..b82b5d978 100644 --- a/heat/engine/resources/openstack/nova/nova_floatingip.py +++ b/heat/engine/resources/openstack/nova/nova_floatingip.py @@ -86,13 +86,6 @@ class NovaFloatingIp(resource.Resource): self.resource_id_set(floating_ip.id) self._floating_ip = floating_ip - def handle_delete(self): - if self.resource_id is not None: - try: - self.client().floating_ips.delete(self.resource_id) - except Exception as e: - self.client_plugin().ignore_not_found(e) - def _resolve_attribute(self, key): floating_ip = self._get_resource() attributes = { diff --git a/heat/engine/resources/openstack/nova/nova_keypair.py b/heat/engine/resources/openstack/nova/nova_keypair.py index 63655efce..b2b9c0d92 100644 --- a/heat/engine/resources/openstack/nova/nova_keypair.py +++ b/heat/engine/resources/openstack/nova/nova_keypair.py @@ -121,13 +121,6 @@ class KeyPair(resource.Resource): True) self.resource_id_set(new_keypair.id) - def handle_delete(self): - if self.resource_id: - try: - self.client().keypairs.delete(self.resource_id) - except Exception as e: - self.client_plugin().ignore_not_found(e) - def handle_check(self): self.client().keypairs.get(self.resource_id) diff --git a/heat/engine/resources/openstack/nova/nova_servergroup.py b/heat/engine/resources/openstack/nova/nova_servergroup.py index 3f852c987..2bfb2f3e9 100644 --- a/heat/engine/resources/openstack/nova/nova_servergroup.py +++ b/heat/engine/resources/openstack/nova/nova_servergroup.py @@ -60,13 +60,6 @@ class ServerGroup(resource.Resource): policies=policies) self.resource_id_set(server_group.id) - def handle_delete(self): - if self.resource_id: - try: - self.client().server_groups.delete(self.resource_id) - except Exception as e: - self.client_plugin().ignore_not_found(e) - def physical_resource_name(self): name = self.properties[self.NAME] if name: diff --git a/heat/engine/resources/openstack/sahara/sahara_cluster.py b/heat/engine/resources/openstack/sahara/sahara_cluster.py index 035acf383..20b6c77b2 100644 --- a/heat/engine/resources/openstack/sahara/sahara_cluster.py +++ b/heat/engine/resources/openstack/sahara/sahara_cluster.py @@ -118,6 +118,8 @@ class SaharaCluster(resource.Resource): default_client_name = 'sahara' + entity = 'clusters' + def _validate_depr_keys(self, properties, key, depr_key): value = properties.get(key) depr_value = properties.get(depr_key) @@ -181,18 +183,6 @@ class SaharaCluster(resource.Resource): LOG.info(_LI("Cluster '%s' has been created"), cluster.name) return True - def handle_delete(self): - if not self.resource_id: - return - - try: - self.client().clusters.delete(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - return None - - return self.resource_id - def check_delete_complete(self, resource_id): if not resource_id: return True diff --git a/heat/engine/resources/openstack/sahara/sahara_templates.py b/heat/engine/resources/openstack/sahara/sahara_templates.py index 71e4bd0a6..9337b0de0 100644 --- a/heat/engine/resources/openstack/sahara/sahara_templates.py +++ b/heat/engine/resources/openstack/sahara/sahara_templates.py @@ -161,6 +161,8 @@ class SaharaNodeGroupTemplate(resource.Resource): physical_resource_name_limit = 50 + entity = 'node_group_templates' + def _ngt_name(self): name = self.properties[self.NAME] if name: @@ -210,17 +212,6 @@ class SaharaNodeGroupTemplate(resource.Resource): self.resource_id_set(node_group_template.id) return self.resource_id - def handle_delete(self): - if not self.resource_id: - return - try: - self.client().node_group_templates.delete( - self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - LOG.info(_LI("Node Group Template '%s' has been deleted."), - self._ngt_name()) - def validate(self): res = super(SaharaNodeGroupTemplate, self).validate() if res: @@ -350,6 +341,8 @@ class SaharaClusterTemplate(resource.Resource): physical_resource_name_limit = 50 + entity = 'cluster_templates' + def _cluster_template_name(self): name = self.properties[self.NAME] if name: @@ -387,17 +380,6 @@ class SaharaClusterTemplate(resource.Resource): self.resource_id_set(cluster_template.id) return self.resource_id - def handle_delete(self): - if not self.resource_id: - return - try: - self.client().cluster_templates.delete( - self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - LOG.info(_LI("Cluster Template '%s' has been deleted."), - self._cluster_template_name()) - def validate(self): res = super(SaharaClusterTemplate, self).validate() if res: diff --git a/heat/tests/cinder/test_cinder_volume_type.py b/heat/tests/cinder/test_cinder_volume_type.py index f2caed420..e19833e8c 100644 --- a/heat/tests/cinder/test_cinder_volume_type.py +++ b/heat/tests/cinder/test_cinder_volume_type.py @@ -119,7 +119,10 @@ class CinderVolumeTypeTest(common.HeatTestCase): volume_type_id = '927202df-1afb-497f-8368-9c2d2f26e5db' self.my_volume_type.resource_id = volume_type_id self.volume_types.delete.return_value = None - self.assertIsNone(self.my_volume_type.handle_delete()) + self.assertEqual('927202df-1afb-497f-8368-9c2d2f26e5db', + self.my_volume_type.handle_delete()) + + def test_volume_type_handle_delete_not_found(self): exc = self.cinderclient.HTTPClientError('Not Found.') self.volume_types.delete.side_effect = exc self.assertIsNone(self.my_volume_type.handle_delete()) diff --git a/heat/tests/cinder/test_volume_type_encryption.py b/heat/tests/cinder/test_volume_type_encryption.py index 7057239e7..d3ddb21fa 100644 --- a/heat/tests/cinder/test_volume_type_encryption.py +++ b/heat/tests/cinder/test_volume_type_encryption.py @@ -109,7 +109,8 @@ class CinderEncryptedVolumeTypeTest(common.HeatTestCase): volume_type_id = '01bd581d-33fe-4d6d-bd7b-70ae076d39fb' self.my_encrypted_vol_type.resource_id = volume_type_id self.volume_encryption_types.delete.return_value = None - self.assertIsNone(self.my_encrypted_vol_type.handle_delete()) + self.assertEqual('01bd581d-33fe-4d6d-bd7b-70ae076d39fb', + self.my_encrypted_vol_type.handle_delete()) def test_handle_delete_rsrc_not_found(self): exc = self.cinderclient.HTTPClientError('Not Found.') diff --git a/heat/tests/nova/test_nova_flavor.py b/heat/tests/nova/test_nova_flavor.py index 2b8ac97eb..0ae2a1bfd 100644 --- a/heat/tests/nova/test_nova_flavor.py +++ b/heat/tests/nova/test_nova_flavor.py @@ -90,7 +90,8 @@ class NovaFlavorTest(common.HeatTestCase): flavor_id = '927202df-1afb-497f-8368-9c2d2f26e5db' self.my_flavor.resource_id = flavor_id self.flavors.delete.return_value = None - self.assertIsNone(self.my_flavor.handle_delete()) + self.assertEqual('927202df-1afb-497f-8368-9c2d2f26e5db', + self.my_flavor.handle_delete()) self.flavors.delete.side_effect = fakes.fake_exception() self.assertIsNone(self.my_flavor.handle_delete()) diff --git a/heat/tests/openstack/designate/test_domain.py b/heat/tests/openstack/designate/test_domain.py index 8c0147fc5..fdd3679b6 100644 --- a/heat/tests/openstack/designate/test_domain.py +++ b/heat/tests/openstack/designate/test_domain.py @@ -131,7 +131,8 @@ class DesignateDomainTest(common.HeatTestCase): self.test_resource.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151' mock_domain_delete.return_value = None - self.assertIsNone(self.test_resource.handle_delete()) + self.assertEqual('477e8273-60a7-4c41-b683-fdb0bc7cd151', + self.test_resource.handle_delete()) mock_domain_delete.assert_called_once_with( self.test_resource.resource_id ) diff --git a/heat/tests/test_glance_image.py b/heat/tests/test_glance_image.py index 6eeb952dd..acdaa442f 100644 --- a/heat/tests/test_glance_image.py +++ b/heat/tests/test_glance_image.py @@ -192,7 +192,8 @@ class GlanceImageTest(common.HeatTestCase): image_id = '41f0e60c-ebb4-4375-a2b4-845ae8b9c995' self.my_image.resource_id = image_id self.images.delete.return_value = None - self.assertIsNone(self.my_image.handle_delete()) + self.assertEqual('41f0e60c-ebb4-4375-a2b4-845ae8b9c995', + self.my_image.handle_delete()) self.images.delete.side_effect = glance_exceptions.HTTPNotFound(404) self.assertIsNone(self.my_image.handle_delete())