From 922cd0a938ac9dad462c09e3483a22e634d722e9 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Wed, 4 Apr 2018 17:19:41 -0400 Subject: [PATCH] Change ha_state property to always return a value Right now, ha_state could return any value that is in the state file, or even '' if the file is empty. Instead, return 'unknown' if it's empty. We also need to update the translation map in the HA code to deal with this new value to avoid a KeyError. Related-bug: #1755243 Change-Id: I94a39e574cf4ff5facb76df352c14cbaba793e98 --- neutron/agent/l3/agent.py | 2 +- neutron/agent/l3/ha.py | 3 ++- neutron/agent/l3/ha_router.py | 5 ++-- neutron/tests/unit/agent/l3/test_ha_router.py | 26 +++++++++++++++++++ 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index 946a1c6b864..a8b2447ce92 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -493,7 +493,7 @@ class L3NATAgent(ha.AgentMixin, is_dvr_only_agent = (self.conf.agent_mode in [lib_const.L3_AGENT_MODE_DVR, lib_const.L3_AGENT_MODE_DVR_NO_EXTERNAL]) - is_ha_router = getattr(ri, 'ha_state', None) is not None + is_ha_router = getattr(ri, 'ha_state', False) # For HA routers check that DB state matches actual state if router.get('ha') and not is_dvr_only_agent and is_ha_router: self.check_ha_state_for_router( diff --git a/neutron/agent/l3/ha.py b/neutron/agent/l3/ha.py index 7b049d34bc9..69a4b056153 100644 --- a/neutron/agent/l3/ha.py +++ b/neutron/agent/l3/ha.py @@ -30,7 +30,8 @@ KEEPALIVED_STATE_CHANGE_SERVER_BACKLOG = 4096 TRANSLATION_MAP = {'master': constants.HA_ROUTER_STATE_ACTIVE, 'backup': constants.HA_ROUTER_STATE_STANDBY, - 'fault': constants.HA_ROUTER_STATE_STANDBY} + 'fault': constants.HA_ROUTER_STATE_STANDBY, + 'unknown': constants.HA_ROUTER_STATE_UNKNOWN} class KeepalivedStateChangeHandler(object): diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index 20b6c57e7d5..771d651eea3 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -76,14 +76,15 @@ class HaRouter(router.RouterInfo): @property def ha_state(self): + state = None ha_state_path = self.keepalived_manager.get_full_config_file_path( 'state') try: with open(ha_state_path, 'r') as f: - return f.read() + state = f.read() except (OSError, IOError): LOG.debug('Error while reading HA state for %s', self.router_id) - return None + return state or 'unknown' @ha_state.setter def ha_state(self, new_state): diff --git a/neutron/tests/unit/agent/l3/test_ha_router.py b/neutron/tests/unit/agent/l3/test_ha_router.py index e5f58142918..33ec7bc902c 100644 --- a/neutron/tests/unit/agent/l3/test_ha_router.py +++ b/neutron/tests/unit/agent/l3/test_ha_router.py @@ -20,6 +20,7 @@ from oslo_utils import uuidutils from neutron.agent.l3 import ha_router from neutron.agent.l3 import router_info from neutron.tests import base +from neutron.tests import tools _uuid = uuidutils.generate_uuid @@ -115,3 +116,28 @@ class TestBasicRouterOperations(base.BaseTestCase): calls = ["sig='str(%d)'" % signal.SIGTERM, "sig='str(%d)'" % signal.SIGKILL] mock_pm.disable.has_calls(calls) + + def _test_ha_state(self, read_return, expected): + ri = self._create_router(mock.MagicMock()) + ri.keepalived_manager = mock.Mock() + ri.keepalived_manager.get_full_config_file_path.return_value = ( + 'ha_state') + self.mock_open = self.useFixture( + tools.OpenFixture('ha_state', read_return)).mock_open + self.assertEqual(expected, ri.ha_state) + + def test_ha_state_master(self): + self._test_ha_state('master', 'master') + + def test_ha_state_unknown(self): + # an empty state file should yield 'unknown' + self._test_ha_state('', 'unknown') + + def test_ha_state_ioerror(self): + # an error reading the state file should yield 'unknown' + ri = self._create_router(mock.MagicMock()) + ri.keepalived_manager = mock.Mock() + ri.keepalived_manager.get_full_config_file_path.return_value = ( + 'ha_state') + self.mock_open = IOError + self.assertEqual('unknown', ri.ha_state)