From 8c635a4d98cbf9b48044555f64d29f5ddf47d9cf Mon Sep 17 00:00:00 2001 From: Xing Zhang Date: Wed, 19 Jul 2017 23:17:41 +0800 Subject: [PATCH] Fix haproxy_check_script for delete listener When delete one of the listeners in active standby mode, the haproxy_check_script should be update too. Delete method dose not have an action and according to the defination of vrrp_check_script_update, the action should be consts.AMP_ACTION_STOP to remove from listener_ids. This path will fix keepalived in fault state by updating the haproxy_check_script. Change-Id: I22c9e4194fc3ea4a5d7ae4a81b2a0cf3a8219697 Closes-Bug: 1705288 --- .../backends/agent/api_server/listener.py | 7 ++ .../backend/agent/api_server/test_server.py | 98 +++++++++++++++++-- 2 files changed, 98 insertions(+), 7 deletions(-) diff --git a/octavia/amphorae/backends/agent/api_server/listener.py b/octavia/amphorae/backends/agent/api_server/listener.py index 7442b8773a..718f6a86f1 100644 --- a/octavia/amphorae/backends/agent/api_server/listener.py +++ b/octavia/amphorae/backends/agent/api_server/listener.py @@ -286,6 +286,13 @@ class Listener(object): except Exception: pass + # Since this script should be deleted at LB delete time + # we can check for this path to see if VRRP is enabled + # on this amphora and not write the file if VRRP is not in use + if os.path.exists(util.keepalived_check_script_path()): + self.vrrp_check_script_update( + listener_id, action=consts.AMP_ACTION_STOP) + # delete the ssl files try: shutil.rmtree(self._cert_dir(listener_id)) diff --git a/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py b/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py index ef55012da1..d83fea740e 100644 --- a/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py +++ b/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py @@ -424,14 +424,17 @@ class TestServerTestCase(base.TestCase): @mock.patch('os.path.exists') @mock.patch('subprocess.check_output') + @mock.patch('octavia.amphorae.backends.agent.api_server.listener.Listener.' + 'vrrp_check_script_update') @mock.patch('octavia.amphorae.backends.agent.api_server.util.' + 'get_haproxy_pid') @mock.patch('shutil.rmtree') @mock.patch('os.remove') def _test_delete_listener(self, init_system, distro, mock_init_system, - mock_remove, mock_rmtree, mock_pid, + mock_remove, mock_rmtree, mock_pid, mock_vrrp, mock_check_output, mock_exists): self.assertIn(distro, [consts.UBUNTU, consts.CENTOS]) + # no listener mock_exists.return_value = False if distro == consts.UBUNTU: rv = self.ubuntu_app.delete('/' + api_server.VERSION + @@ -446,8 +449,8 @@ class TestServerTestCase(base.TestCase): json.loads(rv.data.decode('utf-8'))) mock_exists.assert_called_with('/var/lib/octavia/123/haproxy.cfg') - # service is stopped + no upstart script - mock_exists.side_effect = [True, False, False] + # service is stopped + no upstart script + no vrrp + mock_exists.side_effect = [True, False, False, False] if distro == consts.UBUNTU: rv = self.ubuntu_app.delete('/' + api_server.VERSION + '/listeners/123') @@ -473,8 +476,35 @@ class TestServerTestCase(base.TestCase): mock_exists.assert_any_call('/var/lib/octavia/123/123.pid') - # service is stopped + upstart script - mock_exists.side_effect = [True, False, True] + # service is stopped + no upstart script + vrrp + mock_exists.side_effect = [True, False, True, False] + if distro == consts.UBUNTU: + rv = self.ubuntu_app.delete('/' + api_server.VERSION + + '/listeners/123') + elif distro == consts.CENTOS: + rv = self.centos_app.delete('/' + api_server.VERSION + + '/listeners/123') + self.assertEqual(200, rv.status_code) + self.assertEqual({u'message': u'OK'}, + json.loads(rv.data.decode('utf-8'))) + mock_rmtree.assert_called_with('/var/lib/octavia/123') + + if init_system == consts.INIT_SYSTEMD: + mock_exists.assert_called_with(consts.SYSTEMD_DIR + + '/haproxy-123.service') + elif init_system == consts.INIT_UPSTART: + mock_exists.assert_called_with(consts.UPSTART_DIR + + '/haproxy-123.conf') + elif init_system == consts.INIT_SYSVINIT: + mock_exists.assert_called_with(consts.SYSVINIT_DIR + + '/haproxy-123') + else: + self.assertIn(init_system, consts.VALID_INIT_SYSTEMS) + + mock_exists.assert_any_call('/var/lib/octavia/123/123.pid') + + # service is stopped + upstart script + no vrrp + mock_exists.side_effect = [True, False, False, True] if distro == consts.UBUNTU: rv = self.ubuntu_app.delete('/' + api_server.VERSION + '/listeners/123') @@ -497,8 +527,32 @@ class TestServerTestCase(base.TestCase): else: self.assertIn(init_system, consts.VALID_INIT_SYSTEMS) - # service is running + upstart script - mock_exists.side_effect = [True, True, True, True] + # service is stopped + upstart script + vrrp + mock_exists.side_effect = [True, False, True, True] + if distro == consts.UBUNTU: + rv = self.ubuntu_app.delete('/' + api_server.VERSION + + '/listeners/123') + elif distro == consts.CENTOS: + rv = self.centos_app.delete('/' + api_server.VERSION + + '/listeners/123') + self.assertEqual(200, rv.status_code) + self.assertEqual({u'message': u'OK'}, + json.loads(rv.data.decode('utf-8'))) + + if init_system == consts.INIT_SYSTEMD: + mock_remove.assert_called_with(consts.SYSTEMD_DIR + + '/haproxy-123.service') + elif init_system == consts.INIT_UPSTART: + mock_remove.assert_called_with(consts.UPSTART_DIR + + '/haproxy-123.conf') + elif init_system == consts.INIT_SYSVINIT: + mock_remove.assert_called_with(consts.SYSVINIT_DIR + + '/haproxy-123') + else: + self.assertIn(init_system, consts.VALID_INIT_SYSTEMS) + + # service is running + upstart script + no vrrp + mock_exists.side_effect = [True, True, True, False, True] mock_pid.return_value = '456' if distro == consts.UBUNTU: rv = self.ubuntu_app.delete('/' + api_server.VERSION + @@ -527,6 +581,36 @@ class TestServerTestCase(base.TestCase): else: self.assertIn(init_system, consts.VALID_INIT_SYSTEMS) + # service is running + upstart script + vrrp + mock_exists.side_effect = [True, True, True, True, True] + mock_pid.return_value = '456' + if distro == consts.UBUNTU: + rv = self.ubuntu_app.delete('/' + api_server.VERSION + + '/listeners/123') + elif distro == consts.CENTOS: + rv = self.centos_app.delete('/' + api_server.VERSION + + '/listeners/123') + self.assertEqual(200, rv.status_code) + self.assertEqual({u'message': u'OK'}, + json.loads(rv.data.decode('utf-8'))) + mock_pid.assert_called_with('123') + mock_check_output.assert_any_call( + ['/usr/sbin/service', 'haproxy-123', 'stop'], stderr=-2) + + if init_system == consts.INIT_SYSTEMD: + mock_check_output.assert_any_call( + "systemctl disable haproxy-123".split(), + stderr=subprocess.STDOUT) + elif init_system == consts.INIT_UPSTART: + mock_remove.assert_any_call(consts.UPSTART_DIR + + '/haproxy-123.conf') + elif init_system == consts.INIT_SYSVINIT: + mock_check_output.assert_any_call( + "insserv -r /etc/init.d/haproxy-123".split(), + stderr=subprocess.STDOUT) + else: + self.assertIn(init_system, consts.VALID_INIT_SYSTEMS) + # service is running + stopping fails mock_exists.side_effect = [True, True, True] mock_check_output.side_effect = subprocess.CalledProcessError(