From edc18d4937807dbe2934580449182d65cd642f53 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Thu, 19 Apr 2018 18:38:48 +0200 Subject: [PATCH] Make charm agnostic of underlying init system Replace charm custom init control functions with `service_pause` and `service_resume` functions from charm-helpers. Change-Id: I235af30a19294316f65fba0e13fe10ae50164a42 Closes-Bug: #1765215 --- hooks/nova_cc_hooks.py | 54 +++++++++++++++---------- hooks/nova_cc_utils.py | 37 ++++------------- unit_tests/test_nova_cc_hooks.py | 68 +++++++++++++++++++++++++++++--- unit_tests/test_nova_cc_utils.py | 30 ++++++++------ 4 files changed, 122 insertions(+), 67 deletions(-) diff --git a/hooks/nova_cc_hooks.py b/hooks/nova_cc_hooks.py index 48e1cba3..0e3c7316 100755 --- a/hooks/nova_cc_hooks.py +++ b/hooks/nova_cc_hooks.py @@ -46,8 +46,9 @@ from charmhelpers.core.hookenv import ( ) from charmhelpers.core.host import ( - service_reload, service_pause, + service_reload, + service_resume, ) from charmhelpers.fetch import ( @@ -84,14 +85,11 @@ from charmhelpers.contrib.peerstorage import ( from nova_cc_utils import ( add_hosts_to_cell, auth_token_config, - cmd_all_services, determine_endpoints, determine_packages, determine_ports, disable_package_apache_site, - disable_services, do_openstack_upgrade, - enable_services, is_api_ready, is_cellv2_init_ready, keystone_ca_cert_b64, @@ -159,7 +157,6 @@ CONFIGS = register_configs() COLO_CONSOLEAUTH = 'inf: res_nova_consoleauth grp_nova_vips' AGENT_CONSOLEAUTH = 'ocf:openstack:nova-consoleauth' AGENT_CA_PARAMS = 'op monitor interval="5s"' -NOVA_CONSOLEAUTH_OVERRIDE = '/etc/init/nova-consoleauth.override' def leader_init_db_if_ready(skip_acl_check=False, skip_cells_restarts=False, @@ -273,8 +270,11 @@ def install(): msg = 'Disabling services into db relation joined' log(msg) status_set('maintenance', msg) - disable_services() - cmd_all_services('stop') + if not is_unit_paused_set(): + for svc in services(): + service_pause(svc) + else: + log('Unit is in paused state, not issuing stop/pause to all services') @hooks.hook('config-changed') @@ -287,8 +287,11 @@ def config_changed(): # which will subsequently cause db migrations to fail if >= juno. # Disable neutron-server if >= juno if CompareOpenStackReleases(os_release('nova-common')) >= 'juno': - with open('/etc/init/neutron-server.override', 'wb') as out: - out.write('manual\n') + try: + service_pause('neutron-server') + except ValueError: + # neutron-server service not installed, ignore. + pass if config('prefer-ipv6'): status_set('maintenance', 'configuring ipv6') setup_ipv6() @@ -765,12 +768,21 @@ def cluster_changed(): peer_echo(includes=['dbsync_state']) dbsync_state = peer_retrieve('dbsync_state') if dbsync_state == 'complete': - enable_services() - cmd_all_services('start') + if not is_unit_paused_set(): + for svc in services(): + service_resume(svc) + else: + log('Unit is in paused state, not issuing start/resume to all ' + 'services') else: - log('Database sync not ready. Shutting down services') - disable_services() - cmd_all_services('stop') + if not is_unit_paused_set(): + log('Database sync not ready. Shutting down services') + for svc in services(): + service_pause(svc) + else: + log('Database sync not ready. Would shut down services but ' + 'unit is in paused state, not issuing stop/pause to all ' + 'services') @hooks.hook('ha-relation-joined') @@ -874,8 +886,12 @@ def db_departed(): update_cell_db_if_ready(skip_acl_check=True) for r_id in relation_ids('cluster'): relation_set(relation_id=r_id, dbsync_state='incomplete') - disable_services() - cmd_all_services('stop') + if not is_unit_paused_set(): + for svc in services(): + service_pause(svc) + else: + log('Unit is in paused state, not issuing stop/pause to all ' + 'services') @hooks.hook('amqp-relation-broken', @@ -1120,10 +1136,7 @@ def update_nova_consoleauth_config(): for rid in relation_ids('ha'): relation_set(rid, **data) - try: - os.remove(NOVA_CONSOLEAUTH_OVERRIDE) - except FileNotFoundError as e: - log(str(e), level='DEBUG') + service_resume('nova-consoleauth') def nova_api_relation_joined(rid=None): @@ -1146,5 +1159,6 @@ def main(): log('Unknown hook {} - skipping.'.format(e)) assess_status(CONFIGS) + if __name__ == '__main__': main() diff --git a/hooks/nova_cc_utils.py b/hooks/nova_cc_utils.py index 3ccde73f..48cd34de 100644 --- a/hooks/nova_cc_utils.py +++ b/hooks/nova_cc_utils.py @@ -75,7 +75,6 @@ from charmhelpers.core.hookenv import ( ) from charmhelpers.core.host import ( - service, service_pause, service_resume, service_running, @@ -493,6 +492,7 @@ def get_step_upgrade_source(new_src): return None + POLICY_RC_D = """#!/bin/bash set -e @@ -831,8 +831,12 @@ def finalize_migrate_nova_databases(): log('Informing peers that dbsync is complete', level=INFO) peer_store('dbsync_state', 'complete') log('Enabling services', level=INFO) - enable_services() - cmd_all_services('start') + if not is_unit_paused_set(): + for svc in services(): + service_resume(svc) + else: + log('Unit is in paused state, not issuing start/resume to all ' + 'services') # NOTE(jamespage): Retry deals with sync issues during one-shot HA deploys. @@ -1170,33 +1174,6 @@ def service_guard(guard_map, contexts, active=False): return wrap -def cmd_all_services(cmd): - if is_unit_paused_set(): - log('Unit is in paused state, not issuing {} to all' - 'services'.format(cmd)) - return - if cmd == 'start': - for svc in services(): - if not service_running(svc): - service_start(svc) - else: - for svc in services(): - service(cmd, svc) - - -def disable_services(): - for svc in services(): - with open('/etc/init/{}.override'.format(svc), 'wb') as out: - out.write('exec true\n') - - -def enable_services(): - for svc in services(): - override_file = '/etc/init/{}.override'.format(svc) - if os.path.isfile(override_file): - os.remove(override_file) - - def setup_ipv6(): ubuntu_rel = lsb_release()['DISTRIB_CODENAME'].lower() if CompareHostReleases(ubuntu_rel) < "trusty": diff --git a/unit_tests/test_nova_cc_hooks.py b/unit_tests/test_nova_cc_hooks.py index 8c8a376c..b9c98930 100644 --- a/unit_tests/test_nova_cc_hooks.py +++ b/unit_tests/test_nova_cc_hooks.py @@ -48,17 +48,15 @@ TO_PATCH = [ 'charm_dir', 'do_openstack_upgrade', 'openstack_upgrade_available', - 'cmd_all_services', 'config', 'config_value_changed', 'determine_endpoints', 'determine_packages', 'determine_ports', - 'disable_services', - 'enable_services', 'NovaCellContext', 'open_port', 'is_relation_made', + 'is_unit_paused_set', 'local_unit', 'log', 'os_release', @@ -71,7 +69,9 @@ TO_PATCH = [ 'ssh_known_hosts_lines', 'ssh_authorized_keys_lines', 'save_script_rc', + 'service_pause', 'service_reload', + 'service_resume', 'services', 'execd_preinstall', 'network_manager', @@ -129,12 +129,13 @@ class NovaCCHooksTests(CharmTestCase): self.determine_packages.return_value = [ 'nova-scheduler', 'nova-api-ec2'] self.determine_ports.return_value = [80, 81, 82] + self.is_unit_paused_set.return_value = False + self.services.return_value = ['dummy-service'] hooks.install() self.apt_install.assert_called_with( ['nova-scheduler', 'nova-api-ec2'], fatal=True) self.assertTrue(self.execd_preinstall.called) - self.assertTrue(self.disable_services.called) - self.cmd_all_services.assert_called_with('stop') + self.assertTrue(self.service_pause.called) @patch.object(hooks, 'update_aws_compat_services') @patch.object(hooks, 'update_nova_consoleauth_config') @@ -162,6 +163,63 @@ class NovaCCHooksTests(CharmTestCase): self.assertTrue(mock_update_nova_consoleauth_config.called) self.assertTrue(mock_update_aws_compat_services.called) + @patch.object(hooks, 'update_aws_compat_services') + @patch.object(hooks, 'update_nova_consoleauth_config') + @patch.object(hooks, 'is_db_initialised') + @patch.object(hooks, 'determine_packages') + @patch.object(utils, 'service_resume') + @patch.object(utils, 'config') + @patch.object(hooks, 'filter_installed_packages') + @patch.object(hooks, 'configure_https') + def test_config_changed_no_upgrade_juno(self, conf_https, + mock_filter_packages, + utils_config, mock_service_resume, + mock_determine_packages, + mock_is_db_initialised, + mock_update_nova_consoleauth_cfg, + mock_update_aws_compat_services): + mock_determine_packages.return_value = [] + utils_config.side_effect = self.test_config.get + self.test_config.set('console-access-protocol', 'dummy') + self.openstack_upgrade_available.return_value = False + mock_is_db_initialised.return_value = False + self.os_release.return_value = 'juno' + hooks.config_changed() + self.assertTrue(self.save_script_rc.called) + mock_filter_packages.assert_called_with([]) + self.assertTrue(mock_update_nova_consoleauth_cfg.called) + self.assertTrue(mock_update_aws_compat_services.called) + self.service_pause.assert_called_with('neutron-server') + + @patch.object(hooks, 'update_aws_compat_services') + @patch.object(hooks, 'update_nova_consoleauth_config') + @patch.object(hooks, 'is_db_initialised') + @patch.object(hooks, 'determine_packages') + @patch.object(utils, 'service_resume') + @patch.object(utils, 'config') + @patch.object(hooks, 'filter_installed_packages') + @patch.object(hooks, 'configure_https') + def test_config_changed_no_upgrade_juno_no_neutron_server( + self, conf_https, mock_filter_packages, + utils_config, mock_service_resume, + mock_determine_packages, + mock_is_db_initialised, + mock_update_nova_consoleauth_cfg, + mock_update_aws_compat_services): + mock_determine_packages.return_value = [] + utils_config.side_effect = self.test_config.get + self.test_config.set('console-access-protocol', 'dummy') + self.openstack_upgrade_available.return_value = False + mock_is_db_initialised.return_value = False + self.os_release.return_value = 'juno' + self.service_pause.side_effect = ValueError + hooks.config_changed() + self.assertTrue(self.save_script_rc.called) + mock_filter_packages.assert_called_with([]) + self.assertTrue(mock_update_nova_consoleauth_cfg.called) + self.assertTrue(mock_update_aws_compat_services.called) + self.service_pause.assert_called_with('neutron-server') + @patch.object(hooks, 'update_aws_compat_services') @patch.object(hooks, 'update_nova_consoleauth_config') @patch.object(hooks, 'is_db_initialised') diff --git a/unit_tests/test_nova_cc_utils.py b/unit_tests/test_nova_cc_utils.py index a8e7c282..e5dbe50c 100644 --- a/unit_tests/test_nova_cc_utils.py +++ b/unit_tests/test_nova_cc_utils.py @@ -34,14 +34,13 @@ TO_PATCH = [ 'apt_update', 'apt_upgrade', 'apt_install', - 'cmd_all_services', 'config', 'configure_installation_source', 'disable_policy_rcd', 'is_leader', + 'is_unit_paused_set', 'lsb_release', 'enable_policy_rcd', - 'enable_services', 'get_os_codename_install_source', 'log', 'os_release', @@ -50,6 +49,8 @@ TO_PATCH = [ 'relation_ids', 'remote_unit', '_save_script_rc', + 'service_pause', + 'service_resume', 'service_start', 'services', 'service_running', @@ -666,29 +667,33 @@ class NovaCCUtilsTests(CharmTestCase): "Migrate database with nova-manage" self.relation_ids.return_value = [] self.os_release.return_value = 'diablo' + self.is_unit_paused_set.return_value = False + self.services.return_value = ['dummy-service'] utils.migrate_nova_databases() check_output.assert_called_with(['nova-manage', 'db', 'sync']) - self.assertTrue(self.enable_services.called) - self.cmd_all_services.assert_called_with('start') + self.assertTrue(self.service_resume.called) @patch('subprocess.check_output') def test_migrate_nova_databases_cluster(self, check_output): "Migrate database with nova-manage in a clustered env" self.relation_ids.return_value = ['cluster:1'] self.os_release.return_value = 'diablo' + self.is_unit_paused_set.return_value = False + self.services.return_value = ['dummy-service'] utils.migrate_nova_databases() check_output.assert_called_with(['nova-manage', 'db', 'sync']) self.assertNotIn(call(['nova-manage', 'db', 'online_data_migrations']), check_output.mock_calls) self.peer_store.assert_called_with('dbsync_state', 'complete') - self.assertTrue(self.enable_services.called) - self.cmd_all_services.assert_called_with('start') + self.assertTrue(self.service_resume.called) @patch('subprocess.check_output') def test_migrate_nova_databases_mitaka(self, check_output): "Migrate database with nova-manage in a clustered env" self.relation_ids.return_value = ['cluster:1'] self.os_release.return_value = 'mitaka' + self.is_unit_paused_set.return_value = False + self.services.return_value = ['dummy-service'] utils.migrate_nova_databases() check_output.assert_has_calls([ call(['nova-manage', 'api_db', 'sync']), @@ -696,8 +701,7 @@ class NovaCCUtilsTests(CharmTestCase): call(['nova-manage', 'db', 'online_data_migrations']), ]) self.peer_store.assert_called_with('dbsync_state', 'complete') - self.assertTrue(self.enable_services.called) - self.cmd_all_services.assert_called_with('start') + self.assertTrue(self.service_resume.called) @patch('subprocess.Popen') @patch('subprocess.check_output') @@ -709,6 +713,8 @@ class NovaCCUtilsTests(CharmTestCase): get_cell_uuid.return_value = 'c83121db-f1c7-464a-b657-38c28fac84c6' self.relation_ids.return_value = ['cluster:1'] self.os_release.return_value = 'ocata' + self.is_unit_paused_set.return_value = False + self.services.return_value = ['dummy-service'] process_mock = MagicMock() attrs = { 'communicate.return_value': ('output', 'error'), @@ -733,8 +739,7 @@ class NovaCCUtilsTests(CharmTestCase): '--max-count', '50000'], stdout=-1) Popen.assert_has_calls([map_call]) self.peer_store.assert_called_with('dbsync_state', 'complete') - self.assertTrue(self.enable_services.called) - self.cmd_all_services.assert_called_with('start') + self.assertTrue(self.service_resume.called) @patch('subprocess.Popen') @patch('subprocess.check_output') @@ -746,6 +751,8 @@ class NovaCCUtilsTests(CharmTestCase): get_cell_uuid.return_value = 'c83121db-f1c7-464a-b657-38c28fac84c6' self.relation_ids.return_value = ['cluster:1'] self.os_release.return_value = 'pike' + self.is_unit_paused_set.return_value = False + self.services.return_value = ['dummy-service'] utils.migrate_nova_databases() check_output.assert_has_calls([ call(['nova-manage', 'api_db', 'sync']), @@ -762,8 +769,7 @@ class NovaCCUtilsTests(CharmTestCase): 'c83121db-f1c7-464a-b657-38c28fac84c6']) self.assertFalse(map_call in Popen.call_args_list) self.peer_store.assert_called_with('dbsync_state', 'complete') - self.assertTrue(self.enable_services.called) - self.cmd_all_services.assert_called_with('start') + self.assertTrue(self.service_resume.called) @patch('subprocess.check_output') def test_migrate_nova_flavors(self, check_output):