From 03a25013d51f0550afd1e9adc26e1f5e8a7aa8fc Mon Sep 17 00:00:00 2001 From: xiaodongwang Date: Tue, 18 Nov 2014 13:24:11 -0800 Subject: [PATCH] refactor delete cluster code to make it clean Change-Id: I60264674d51151f70986182c8edac9314ce459f7 --- bin/delete_clusters.py | 35 +--- compass/actions/delete.py | 29 ---- compass/actions/util.py | 20 +-- compass/api/api.py | 60 ++++--- compass/db/api/cluster.py | 240 ++++++++++++++++++--------- compass/db/api/host.py | 54 +++--- compass/tests/api/test_api.py | 6 +- compass/tests/db/api/test_cluster.py | 2 +- compass/tests/db/api/test_host.py | 8 +- 9 files changed, 252 insertions(+), 202 deletions(-) diff --git a/bin/delete_clusters.py b/bin/delete_clusters.py index fef0a9d8..37a24bb5 100755 --- a/bin/delete_clusters.py +++ b/bin/delete_clusters.py @@ -28,19 +28,15 @@ sys.path.append(current_dir) import switch_virtualenv -from compass.actions import delete from compass.db.api import cluster as cluster_api from compass.db.api import database +from compass.db.api import host as host_api from compass.db.api import user as user_api -from compass.tasks.client import celery from compass.utils import flags from compass.utils import logsetting from compass.utils import setting_wrapper as setting -flags.add_bool('async', - help='run in async mode', - default=True) flags.add('clusternames', help='comma seperated cluster names', default='') @@ -65,34 +61,9 @@ def delete_clusters(): delete_underlying_host = flags.OPTIONS.delete_hosts for cluster in clusters: cluster_id = cluster['id'] - hosts = cluster_api.list_cluster_hosts(user, cluster_id) - host_id_list = [host['id'] for host in hosts] - logging.info( - 'delete cluster %s and cluster hosts %s', - cluster_id, host_id_list + cluster_api.del_cluster( + user, cluster_id, True, False, delete_underlying_host ) - logging.info('delete underlying host? %s', delete_underlying_host) - if flags.OPTIONS.async: - celery.send_task( - 'compass.tasks.delete_cluster', - ( - setting.COMPASS_ADMIN_EMAIL, - cluster_id, - host_id_list, - delete_underlying_host - ) - ) - else: - try: - delete.delete_cluster( - cluster_id, - host_id_list, - setting.COMPASS_ADMIN_EMAIL, - delete_underlying_host - ) - except Exception as error: - logging.error('failed to delete cluster %s', cluster) - logging.exception(error) if __name__ == '__main__': diff --git a/compass/actions/delete.py b/compass/actions/delete.py index 7762a866..010e9ebd 100644 --- a/compass/actions/delete.py +++ b/compass/actions/delete.py @@ -42,25 +42,6 @@ def delete_cluster( user = user_db.get_user_object(username) - for host_id in host_id_list: - cluster_api.update_cluster_host_state( - user, cluster_id, host_id, state='ERROR' - ) - host_api.update_host_state( - user, host_id, state='ERROR' - ) - cluster_api.update_cluster_state( - user, cluster_id, state='ERROR' - ) - - cluster_api.update_cluster( - user, cluster_id, reinstall_distributed_system=True - ) - for host_id in host_id_list: - cluster_api.update_cluster_host( - user, cluster_id, host_id, reinstall_os=True - ) - cluster_info = util.ActionHelper.get_cluster_info(cluster_id, user) adapter_id = cluster_info[const.ADAPTER_ID] @@ -69,9 +50,6 @@ def delete_cluster( hosts_info = util.ActionHelper.get_hosts_info( cluster_id, host_id_list, user) - logging.debug('adapter info: %s', adapter_info) - logging.debug('cluster info: %s', cluster_info) - logging.debug('hosts info: %s', hosts_info) deploy_manager = DeployManager(adapter_info, cluster_info, hosts_info) deploy_manager.remove_hosts( @@ -102,8 +80,6 @@ def delete_cluster_host( cluster_id, [host_id], user) deploy_manager = DeployManager(adapter_info, cluster_info, hosts_info) - logging.debug('Created deploy manager with %s %s %s' - % (adapter_info, cluster_info, hosts_info)) deploy_manager.remove_hosts( package_only=not delete_underlying_host, @@ -135,8 +111,6 @@ def delete_host( deploy_manager = DeployManager( adapter_info, cluster_info, hosts_info) - logging.debug('Created deploy manager with %s %s %s' - % (adapter_info, cluster_info, hosts_info)) deploy_manager.remove_hosts( package_only=True, @@ -146,6 +120,3 @@ def delete_host( util.ActionHelper.delete_host( host_id, user ) - - -ActionHelper = util.ActionHelper diff --git a/compass/actions/util.py b/compass/actions/util.py index f8783119..054fe9b7 100644 --- a/compass/actions/util.py +++ b/compass/actions/util.py @@ -247,11 +247,11 @@ class ActionHelper(object): ): if delete_underlying_host: for host_id in host_id_list: - host_db.del_host_from_database( - user, host_id + host_db.del_host( + user, host_id, True, True ) - cluster_db.del_cluster_from_database( - user, cluster_id + cluster_db.del_cluster( + user, cluster_id, True, True ) @staticmethod @@ -259,17 +259,17 @@ class ActionHelper(object): cluster_id, host_id, user, delete_underlying_host=False ): if delete_underlying_host: - host_db.del_host_from_database( - user, host_id + host_db.del_host( + user, host_id, True, True ) - cluster_db.del_cluster_host_from_database( - user, cluster_id, host_id + cluster_db.del_cluster_host( + user, cluster_id, host_id, True, True ) @staticmethod def delete_host(host_id, user): - host_db.del_host_from_database( - user, host_id + host_db.del_host( + user, host_id, True, True ) @staticmethod diff --git a/compass/api/api.py b/compass/api/api.py index 098be35a..c90437eb 100644 --- a/compass/api/api.py +++ b/compass/api/api.py @@ -1345,12 +1345,17 @@ def update_cluster(cluster_id): def delete_cluster(cluster_id): """Delete cluster.""" data = _get_request_data() - return utils.make_json_response( - 202, - cluster_api.del_cluster( - current_user, cluster_id, **data - ) + response = cluster_api.del_cluster( + current_user, cluster_id, **data ) + if 'status' in response: + return utils.make_json_response( + 202, response + ) + else: + return utils.make_json_response( + 200, response + ) @app.route("/clusters//config", methods=['GET']) @@ -1612,12 +1617,17 @@ def patch_clusterhost(clusterhost_id): def delete_cluster_host(cluster_id, host_id): """Delete cluster host.""" data = _get_request_data() - return utils.make_json_response( - 202, - cluster_api.del_cluster_host( - current_user, cluster_id, host_id, **data - ) + response = cluster_api.del_cluster_host( + current_user, cluster_id, host_id, **data ) + if 'status' in response: + return utils.make_json_response( + 202, response + ) + else: + return utils.make_json_response( + 200, response + ) @app.route( @@ -1629,12 +1639,17 @@ def delete_cluster_host(cluster_id, host_id): def delete_clusterhost(clusterhost_id): """Delete cluster host.""" data = _get_request_data() - return utils.make_json_response( - 202, - cluster_api.del_clusterhost( - current_user, clusterhost_id, **data - ) + response = cluster_api.del_clusterhost( + current_user, clusterhost_id, **data ) + if 'status' in response: + return utils.make_json_response( + 202, response + ) + else: + return utils.make_json_response( + 200, response + ) @app.route( @@ -1917,12 +1932,17 @@ def update_hosts(): def delete_host(host_id): """Delete host.""" data = _get_request_data() - return utils.make_json_response( - 202, - host_api.del_host( - current_user, host_id, **data - ) + response = host_api.del_host( + current_user, host_id, **data ) + if 'status' in response: + return utils.make_json_response( + 202, response + ) + else: + return utils.make_json_response( + 200, response + ) @app.route("/hosts//clusters", methods=['GET']) diff --git a/compass/db/api/cluster.py b/compass/db/api/cluster.py index 02f0855d..1ea0a4fd 100644 --- a/compass/db/api/cluster.py +++ b/compass/db/api/cluster.py @@ -218,6 +218,9 @@ def is_cluster_editable( cluster.distributed_system and not cluster.reinstall_distributed_system ): + logging.debug( + 'cluster is not editable when not reinstall_distributed_system' + ) return _conditional_exception( cluster, exception_when_not_editable ) @@ -293,47 +296,73 @@ def update_cluster(session, updater, cluster_id, **kwargs): permission.PERMISSION_DEL_CLUSTER ) @utils.wrap_to_dict( - ['status', 'cluster', 'hosts'], + RESP_FIELDS + ['status', 'cluster', 'hosts'], cluster=RESP_FIELDS, hosts=RESP_CLUSTERHOST_FIELDS ) -def del_cluster(session, deleter, cluster_id, **kwargs): +def del_cluster( + session, deleter, cluster_id, + force=False, from_database_only=False, + delete_underlying_host=False, **kwargs +): """Delete a cluster.""" - from compass.tasks import client as celery_client cluster = utils.get_db_object( session, models.Cluster, id=cluster_id ) + logging.debug( + 'delete cluster %s with force=%s ' + 'from_database_only=%s delete_underlying_host=%s', + cluster.id, force, from_database_only, delete_underlying_host + ) + for clusterhost in cluster.clusterhosts: + if clusterhost.state.state != 'UNINITIALIZED' and force: + clusterhost.state.state = 'ERROR' + if delete_underlying_host: + host = clusterhost.host + if host.state.state != 'UNINITIALIZED' and force: + host.state.state = 'ERROR' + if cluster.state.state != 'UNINITIALIZED' and force: + cluster.state.state = 'ERROR' + is_cluster_editable( session, cluster, deleter, reinstall_distributed_system_set=True ) - clusterhosts = [] + for clusterhost in cluster.clusterhosts: - clusterhosts.append(clusterhost) - - celery_client.celery.send_task( - 'compass.tasks.delete_cluster', - ( - deleter.email, cluster_id, - [clusterhost.host_id for clusterhost in clusterhosts] + from compass.db.api import host as host_api + host = clusterhost.host + host_api.is_host_editable( + session, host, deleter, reinstall_os_set=True ) - ) - return { - 'status': 'delete action sent', - 'cluster': cluster, - 'hosts': clusterhosts - } + if host.state.state == 'UNINITIALIZED' or from_database_only: + utils.del_db_object( + session, host + ) + if cluster.state.state == 'UNINITIALIZED' or from_database_only: + return utils.del_db_object( + session, cluster + ) + else: + from compass.tasks import client as celery_client + clusterhosts = [] + for clusterhost in cluster.clusterhosts: + clusterhosts.append(clusterhost) - -@database.run_in_session() -@user_api.check_user_permission_in_session( - permission.PERMISSION_DEL_CLUSTER -) -def del_cluster_from_database(session, deleter, cluster_id): - cluster = utils.get_db_object( - session, models.Cluster, id=cluster_id - ) - return utils.del_db_object(session, cluster) + logging.info('send del cluster %s task to celery', cluster_id) + celery_client.celery.send_task( + 'compass.tasks.delete_cluster', + ( + deleter.email, cluster_id, + [clusterhost.host_id for clusterhost in clusterhosts], + delete_underlying_host + ) + ) + return { + 'status': 'delete action sent', + 'cluster': cluster, + 'hosts': clusterhosts + } @utils.supported_filters([]) @@ -876,39 +905,67 @@ def patch_clusterhost( @user_api.check_user_permission_in_session( permission.PERMISSION_DEL_CLUSTER_HOST ) -@utils.wrap_to_dict(['status', 'host'], host=RESP_CLUSTERHOST_FIELDS) -def del_cluster_host(session, deleter, cluster_id, host_id, **kwargs): +@utils.wrap_to_dict( + RESP_CLUSTERHOST_FIELDS + ['status', 'host'], + host=RESP_CLUSTERHOST_FIELDS +) +def del_cluster_host( + session, deleter, cluster_id, host_id, + force=False, from_database_only=False, + delete_underlying_host=False, + **kwargs +): """Delete cluster host.""" - from compass.tasks import client as celery_client clusterhost = utils.get_db_object( session, models.ClusterHost, cluster_id=cluster_id, host_id=host_id ) - is_cluster_editable( - session, clusterhost.cluster, deleter, - reinstall_distributed_system_set=True - ) - celery_client.celery.send_task( - 'compass.tasks.delete_cluster_host', - ( - deleter.email, cluster_id, host_id + if clusterhost.state.state != 'UNINITIALIZED' and force: + clusterhost.state.state = 'ERROR' + if not force: + is_cluster_editable( + session, clusterhost.cluster, deleter, + reinstall_distributed_system_set=True ) - ) - return { - 'status': 'delete action sent', - 'host': clusterhost, - } + else: + raise Exception( + 'cluster is not editable: %s', clusterhost.cluster.state.state + ) + if delete_underlying_host: + host = clusterhost.host + if host.state.state != 'UNINITIALIZED' and force: + host.state.state = 'ERROR' + import compass.db.api.host as host_api + host_api.is_host_editable( + session, host, deleter, + reinstall_os_set=True + ) + if host.state.state == 'UNINITIALIZED' or from_database_only: + utils.del_db_object( + session, host + ) - -@database.run_in_session() -@user_api.check_user_permission_in_session( - permission.PERMISSION_DEL_CLUSTER_HOST -) -def del_cluster_host_from_database(session, deleter, cluster_id, host_id): - clusterhost = utils.get_db_object( - session, models.ClusterHost, id=cluster_id, host_id=host_id - ) - return utils.del_db_object(session, clusterhost) + if clusterhost.state.state == 'UNINITIALIZED' or from_database_only: + return utils.del_db_object( + session, clusterhost + ) + else: + logging.info( + 'send del cluster %s host %s task to celery', + cluster_id, host_id + ) + from compass.tasks import client as celery_client + celery_client.celery.send_task( + 'compass.tasks.delete_cluster_host', + ( + deleter.email, cluster_id, host_id, + delete_underlying_host + ) + ) + return { + 'status': 'delete action sent', + 'host': clusterhost, + } @utils.supported_filters([]) @@ -916,39 +973,64 @@ def del_cluster_host_from_database(session, deleter, cluster_id, host_id): @user_api.check_user_permission_in_session( permission.PERMISSION_DEL_CLUSTER_HOST ) -@utils.wrap_to_dict(['status', 'host'], host=RESP_CLUSTERHOST_FIELDS) -def del_clusterhost(session, deleter, clusterhost_id, **kwargs): +@utils.wrap_to_dict( + RESP_CLUSTERHOST_FIELDS + ['status', 'host'], + host=RESP_CLUSTERHOST_FIELDS +) +def del_clusterhost( + session, deleter, clusterhost_id, + force=False, from_database_only=False, + delete_underlying_host=False, + **kwargs +): """Delete cluster host.""" - from compass.tasks import client as celery_client clusterhost = utils.get_db_object( session, models.ClusterHost, clusterhost_id=clusterhost_id ) - is_cluster_editable( - session, clusterhost.cluster, deleter, - reinstall_distributed_system_set=True - ) - celery_client.celery.send_task( - 'compass.tasks.delete_cluster_host', - ( - deleter.email, clusterhost.cluster_id, clusterhost.host_id + if clusterhost.state.state != 'UNINITIALIZED' and force: + clusterhost.state.state = 'ERROR' + if not force: + is_cluster_editable( + session, clusterhost.cluster, deleter, + reinstall_distributed_system_set=True ) - ) - return { - 'status': 'delete action sent', - 'host': clusterhost, - } + if delete_underlying_host: + host = clusterhost.host + if host.state.state != 'UNINITIALIZED' and force: + host.state.state = 'ERROR' + import compass.db.api.host as host_api + host_api.is_host_editable( + session, host, deleter, + reinstall_os_set=True + ) + if host.state.state == 'UNINITIALIZED' or from_database_only: + utils.del_db_object( + session, host + ) - -@database.run_in_session() -@user_api.check_user_permission_in_session( - permission.PERMISSION_DEL_CLUSTER_HOST -) -def del_clusterhost_from_database(session, deleter, clusterhost_id): - clusterhost = utils.get_db_object( - session, models.ClusterHost, clusterhost_id=clusterhost_id - ) - return utils.del_db_object(session, clusterhost) + if clusterhost.state.state == 'UNINITIALIZED' or from_database_only: + return utils.del_db_object( + session, clusterhost + ) + else: + logging.info( + 'send del cluster %s host %s task to celery', + clusterhost.cluster_id, clusterhost.host_id + ) + from compass.tasks import client as celery_client + celery_client.celery.send_task( + 'compass.tasks.delete_cluster_host', + ( + deleter.email, clusterhost.cluster_id, + clusterhost.host_id, + delete_underlying_host + ) + ) + return { + 'status': 'delete action sent', + 'host': clusterhost + } @utils.supported_filters([]) diff --git a/compass/db/api/host.py b/compass/db/api/host.py index 7998cedb..19c43cab 100644 --- a/compass/db/api/host.py +++ b/compass/db/api/host.py @@ -226,7 +226,8 @@ def is_host_editable( ) elif not host.reinstall_os: logging.debug( - 'host is not editable when not reinstall os') + 'host is not editable when not reinstall os' + ) return _conditional_exception( host, exception_when_not_editable ) @@ -323,47 +324,52 @@ def update_hosts(session, updater, data=[]): @user_api.check_user_permission_in_session( permission.PERMISSION_DEL_HOST ) -@utils.wrap_to_dict(['status', 'host'], host=RESP_FIELDS) -def del_host(session, deleter, host_id, **kwargs): +@utils.wrap_to_dict( + RESP_FIELDS + ['status', 'host'], + host=RESP_FIELDS +) +def del_host( + session, deleter, host_id, + force=False, from_database_only=False, **kwargs +): """Delete a host.""" from compass.db.api import cluster as cluster_api - from compass.tasks import client as celery_client host = utils.get_db_object( session, models.Host, id=host_id ) + if host.state.state != 'UNINITIALIZED' and force: + host.state.state = 'ERROR' is_host_editable( session, host, deleter, reinstall_os_set=True ) cluster_ids = [] for clusterhost in host.clusterhosts: + if clusterhost.state.state != 'UNINITIALIZED' and force: + clusterhost.state.state = 'ERROR' cluster_api.is_cluster_editable( session, clusterhost.cluster, deleter, reinstall_distributed_system_set=True ) cluster_ids.append(clusterhost.cluster_id) - celery_client.celery.send_task( - 'compass.tasks.delete_host', - ( - deleter.email, host_id, cluster_ids + if host.state.state == 'UNINITIALIZED' or from_database_only: + return utils.del_db_object(session, host) + else: + logging.info( + 'send del host %s task to celery', host_id ) - ) - return { - 'status': 'delete action sent', - 'host': host, - } - - -@database.run_in_session() -@user_api.check_user_permission_in_session( - permission.PERMISSION_DEL_HOST -) -def del_host_from_database(session, deleter, host_id): - host = utils.get_db_object( - session, models.Host, id=host_id - ) - return utils.del_db_object(session, host) + from compass.tasks import client as celery_client + celery_client.celery.send_task( + 'compass.tasks.delete_host', + ( + deleter.email, host_id, cluster_ids + ) + ) + return { + 'status': 'delete action sent', + 'host': host, + } @utils.supported_filters([]) diff --git a/compass/tests/api/test_api.py b/compass/tests/api/test_api.py index 491ffacf..29c5e66b 100644 --- a/compass/tests/api/test_api.py +++ b/compass/tests/api/test_api.py @@ -387,7 +387,7 @@ class TestClusterAPI(ApiTestCase): # delete a cluster sucessfully url = '/clusters/1' return_value = self.delete(url) - self.assertEqual(return_value.status_code, 202) + self.assertEqual(return_value.status_code, 200) def test_list_cluster_hosts(self): # list cluster_hosts successfully @@ -453,7 +453,7 @@ class TestClusterAPI(ApiTestCase): # delete a cluster_host successfully url = '/clusters/1/hosts/1' return_value = self.delete(url) - self.assertEqual(return_value.status_code, 202) + self.assertEqual(return_value.status_code, 200) # give a non-existed cluster_id url = '/clusters/99/hosts/1' @@ -864,7 +864,7 @@ class TestHostAPI(ApiTestCase): # delete a host successfully url = '/hosts/2' return_value = self.delete(url) - self.assertEqual(return_value.status_code, 202) + self.assertEqual(return_value.status_code, 200) # give a non-existed id url = '/hosts/99' diff --git a/compass/tests/db/api/test_cluster.py b/compass/tests/db/api/test_cluster.py index 733fc949..85fd4d8c 100644 --- a/compass/tests/db/api/test_cluster.py +++ b/compass/tests/db/api/test_cluster.py @@ -389,7 +389,7 @@ class TestDelCluster(ClusterTestCase): self.user_object, self.cluster_id ) - self.assertIsNotNone(del_cluster['status']) + self.assertIsNotNone(del_cluster) def test_is_cluster_editable(self): #state is INSTALLING diff --git a/compass/tests/db/api/test_host.py b/compass/tests/db/api/test_host.py index df230734..80e48ee5 100644 --- a/compass/tests/db/api/test_host.py +++ b/compass/tests/db/api/test_host.py @@ -267,11 +267,11 @@ class TestListMachinesOrHosts(HostTestCase): self.assertIn(item, ['newname1', 'newname2']) def test_list_machines(self): - host.del_host_from_database( + host.del_host( self.user_object, self.host_ids[0] ) - host.del_host_from_database( + host.del_host( self.user_object, self.host_ids[1] ) @@ -327,7 +327,7 @@ class TestGetMachineOrHost(HostTestCase): self.assertEqual(get_host['mac'], '28:6e:d4:46:c4:25') def test_get_machine(self): - host.del_host_from_database( + host.del_host( self.user_object, self.host_ids[0] ) @@ -454,7 +454,7 @@ class TestDelHost(HostTestCase): self.user_object, self.host_ids[0] ) - self.assertIsNotNone(del_host['status']) + self.assertIsNotNone(del_host) def test_is_host_editable(self): host.update_host_state(