summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrian Haley <bhaley@redhat.com>2018-04-04 17:19:41 -0400
committerBrian Haley <haleyb.dev@gmail.com>2018-04-17 14:23:23 +0000
commit922cd0a938ac9dad462c09e3483a22e634d722e9 (patch)
tree4a2ae54370acc72fc06011dede5440f28cbed241
parentc16d15fff2953c7427dc2671dc479d044d5b90c3 (diff)
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
Notes
Notes (review): Code-Review+2: Slawek Kaplonski <skaplons@redhat.com> Code-Review+2: Ihar Hrachyshka <ihrachys@redhat.com> Workflow+1: Ihar Hrachyshka <ihrachys@redhat.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Wed, 18 Apr 2018 05:11:50 +0000 Reviewed-on: https://review.openstack.org/558963 Project: openstack/neutron Branch: refs/heads/master
-rw-r--r--neutron/agent/l3/agent.py2
-rw-r--r--neutron/agent/l3/ha.py3
-rw-r--r--neutron/agent/l3/ha_router.py5
-rw-r--r--neutron/tests/unit/agent/l3/test_ha_router.py26
4 files changed, 32 insertions, 4 deletions
diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py
index 946a1c6..a8b2447 100644
--- a/neutron/agent/l3/agent.py
+++ b/neutron/agent/l3/agent.py
@@ -493,7 +493,7 @@ class L3NATAgent(ha.AgentMixin,
493 is_dvr_only_agent = (self.conf.agent_mode in 493 is_dvr_only_agent = (self.conf.agent_mode in
494 [lib_const.L3_AGENT_MODE_DVR, 494 [lib_const.L3_AGENT_MODE_DVR,
495 lib_const.L3_AGENT_MODE_DVR_NO_EXTERNAL]) 495 lib_const.L3_AGENT_MODE_DVR_NO_EXTERNAL])
496 is_ha_router = getattr(ri, 'ha_state', None) is not None 496 is_ha_router = getattr(ri, 'ha_state', False)
497 # For HA routers check that DB state matches actual state 497 # For HA routers check that DB state matches actual state
498 if router.get('ha') and not is_dvr_only_agent and is_ha_router: 498 if router.get('ha') and not is_dvr_only_agent and is_ha_router:
499 self.check_ha_state_for_router( 499 self.check_ha_state_for_router(
diff --git a/neutron/agent/l3/ha.py b/neutron/agent/l3/ha.py
index 7b049d3..69a4b05 100644
--- a/neutron/agent/l3/ha.py
+++ b/neutron/agent/l3/ha.py
@@ -30,7 +30,8 @@ KEEPALIVED_STATE_CHANGE_SERVER_BACKLOG = 4096
30 30
31TRANSLATION_MAP = {'master': constants.HA_ROUTER_STATE_ACTIVE, 31TRANSLATION_MAP = {'master': constants.HA_ROUTER_STATE_ACTIVE,
32 'backup': constants.HA_ROUTER_STATE_STANDBY, 32 'backup': constants.HA_ROUTER_STATE_STANDBY,
33 'fault': constants.HA_ROUTER_STATE_STANDBY} 33 'fault': constants.HA_ROUTER_STATE_STANDBY,
34 'unknown': constants.HA_ROUTER_STATE_UNKNOWN}
34 35
35 36
36class KeepalivedStateChangeHandler(object): 37class KeepalivedStateChangeHandler(object):
diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py
index 20b6c57..771d651 100644
--- a/neutron/agent/l3/ha_router.py
+++ b/neutron/agent/l3/ha_router.py
@@ -76,14 +76,15 @@ class HaRouter(router.RouterInfo):
76 76
77 @property 77 @property
78 def ha_state(self): 78 def ha_state(self):
79 state = None
79 ha_state_path = self.keepalived_manager.get_full_config_file_path( 80 ha_state_path = self.keepalived_manager.get_full_config_file_path(
80 'state') 81 'state')
81 try: 82 try:
82 with open(ha_state_path, 'r') as f: 83 with open(ha_state_path, 'r') as f:
83 return f.read() 84 state = f.read()
84 except (OSError, IOError): 85 except (OSError, IOError):
85 LOG.debug('Error while reading HA state for %s', self.router_id) 86 LOG.debug('Error while reading HA state for %s', self.router_id)
86 return None 87 return state or 'unknown'
87 88
88 @ha_state.setter 89 @ha_state.setter
89 def ha_state(self, new_state): 90 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 e5f5814..33ec7bc 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
20from neutron.agent.l3 import ha_router 20from neutron.agent.l3 import ha_router
21from neutron.agent.l3 import router_info 21from neutron.agent.l3 import router_info
22from neutron.tests import base 22from neutron.tests import base
23from neutron.tests import tools
23 24
24_uuid = uuidutils.generate_uuid 25_uuid = uuidutils.generate_uuid
25 26
@@ -115,3 +116,28 @@ class TestBasicRouterOperations(base.BaseTestCase):
115 calls = ["sig='str(%d)'" % signal.SIGTERM, 116 calls = ["sig='str(%d)'" % signal.SIGTERM,
116 "sig='str(%d)'" % signal.SIGKILL] 117 "sig='str(%d)'" % signal.SIGKILL]
117 mock_pm.disable.has_calls(calls) 118 mock_pm.disable.has_calls(calls)
119
120 def _test_ha_state(self, read_return, expected):
121 ri = self._create_router(mock.MagicMock())
122 ri.keepalived_manager = mock.Mock()
123 ri.keepalived_manager.get_full_config_file_path.return_value = (
124 'ha_state')
125 self.mock_open = self.useFixture(
126 tools.OpenFixture('ha_state', read_return)).mock_open
127 self.assertEqual(expected, ri.ha_state)
128
129 def test_ha_state_master(self):
130 self._test_ha_state('master', 'master')
131
132 def test_ha_state_unknown(self):
133 # an empty state file should yield 'unknown'
134 self._test_ha_state('', 'unknown')
135
136 def test_ha_state_ioerror(self):
137 # an error reading the state file should yield 'unknown'
138 ri = self._create_router(mock.MagicMock())
139 ri.keepalived_manager = mock.Mock()
140 ri.keepalived_manager.get_full_config_file_path.return_value = (
141 'ha_state')
142 self.mock_open = IOError
143 self.assertEqual('unknown', ri.ha_state)