Remove update_initial_state() method from the HA router

This method was intended to check state of the HA router on the
node and update it in the neutron server.
Patch [1] added check of the initial status to the
neutron_keepalived_state_change_monitor process.
It also could cause some race conditions and event which is setting
correct state of the router will be not processed thus router may endup
with two nodes with "primary" state in the Neutron's DB.

Neutron_keepalived_state_change_monitor was notifying agent about
router's initial state only if this state was 'primary'.
Now it will notify agent always to let agent set router's state as
'backup' if needed (that was previously done by this removed
update_initial_state() method).

[1] https://review.opendev.org/c/openstack/neutron/+/642295

Conflicts:
    neutron/agent/l3/ha_router.py
    neutron/agent/l3/keepalived_state_change.py
    neutron/tests/unit/agent/l3/test_dvr_local_router.py

Change-Id: I2cc58c30cf844ee0ecf0611ecdec430086464790
Closes-Bug: #1916022
(cherry picked from commit 0d8ae15767)
This commit is contained in:
Slawek Kaplonski 2021-02-18 12:48:13 +01:00
parent 4381f792c8
commit 3b2b7f4fe7
5 changed files with 11 additions and 27 deletions

View File

@ -442,7 +442,6 @@ class L3NATAgent(ha.AgentMixin,
if router.get('ha'): if router.get('ha'):
features.append('ha') features.append('ha')
kwargs['state_change_callback'] = self.enqueue_state_change
if router.get('distributed') and router.get('ha'): if router.get('distributed') and router.get('ha'):
# Case 1: If the router contains information about the HA interface # Case 1: If the router contains information about the HA interface
@ -457,7 +456,6 @@ class L3NATAgent(ha.AgentMixin,
if (not router.get(lib_const.HA_INTERFACE_KEY) or if (not router.get(lib_const.HA_INTERFACE_KEY) or
self.conf.agent_mode != lib_const.L3_AGENT_MODE_DVR_SNAT): self.conf.agent_mode != lib_const.L3_AGENT_MODE_DVR_SNAT):
features.remove('ha') features.remove('ha')
kwargs.pop('state_change_callback')
return self.router_factory.create(features, **kwargs) return self.router_factory.create(features, **kwargs)

View File

@ -66,12 +66,11 @@ class HaRouterNamespace(namespaces.RouterNamespace):
class HaRouter(router.RouterInfo): class HaRouter(router.RouterInfo):
def __init__(self, state_change_callback, *args, **kwargs): def __init__(self, *args, **kwargs):
super(HaRouter, self).__init__(*args, **kwargs) super(HaRouter, self).__init__(*args, **kwargs)
self.ha_port = None self.ha_port = None
self.keepalived_manager = None self.keepalived_manager = None
self.state_change_callback = state_change_callback
self._ha_state = None self._ha_state = None
self._ha_state_path = None self._ha_state_path = None
@ -151,7 +150,6 @@ class HaRouter(router.RouterInfo):
self._init_keepalived_manager(process_monitor) self._init_keepalived_manager(process_monitor)
self._check_and_set_real_state() self._check_and_set_real_state()
self.ha_network_added() self.ha_network_added()
self.update_initial_state(self.state_change_callback)
self.spawn_state_change_monitor(process_monitor) self.spawn_state_change_monitor(process_monitor)
def _init_keepalived_manager(self, process_monitor): def _init_keepalived_manager(self, process_monitor):
@ -443,15 +441,6 @@ class HaRouter(router.RouterInfo):
except common_utils.WaitTimeout: except common_utils.WaitTimeout:
pm.disable(sig=str(int(signal.SIGKILL))) pm.disable(sig=str(int(signal.SIGKILL)))
def update_initial_state(self, callback):
addresses = ip_lib.get_devices_with_ip(self.ha_namespace,
name=self.get_ha_device_name())
cidrs = (address['cidr'] for address in addresses)
ha_cidr = self._get_primary_vip()
state = 'master' if ha_cidr in cidrs else 'backup'
self.ha_state = state
callback(self.router_id, state)
@staticmethod @staticmethod
def _gateway_ports_equal(port1, port2): def _gateway_ports_equal(port1, port2):
def _get_filtered_dict(d, ignore): def _get_filtered_dict(d, ignore):

View File

@ -106,12 +106,12 @@ class MonitorDaemon(daemon.Daemon):
for address in ip.addr.list(): for address in ip.addr.list():
if address.get('cidr') == self.cidr: if address.get('cidr') == self.cidr:
state = 'master' state = 'master'
self.write_state_change(state)
self.notify_agent(state)
break break
LOG.debug('Initial status of router %s is %s', LOG.debug('Initial status of router %s is %s',
self.router_id, state) self.router_id, state)
self.write_state_change(state)
self.notify_agent(state)
except Exception: except Exception:
LOG.exception('Failed to get initial status of router %s', LOG.exception('Failed to get initial status of router %s',
self.router_id) self.router_id)

View File

@ -860,7 +860,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
fip = {'id': _uuid()} fip = {'id': _uuid()}
fip_cidr = '11.22.33.44/24' fip_cidr = '11.22.33.44/24'
ri = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, [], **self.ri_kwargs) ri = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, **self.ri_kwargs)
ri.is_router_master = mock.Mock(return_value=False) ri.is_router_master = mock.Mock(return_value=False)
ri._add_vip = mock.Mock() ri._add_vip = mock.Mock()
interface_name = ri.get_snat_external_device_interface_name( interface_name = ri.get_snat_external_device_interface_name(
@ -871,7 +871,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
router[lib_constants.HA_INTERFACE_KEY]['status'] = 'DOWN' router[lib_constants.HA_INTERFACE_KEY]['status'] = 'DOWN'
self._set_ri_kwargs(agent, router['id'], router) self._set_ri_kwargs(agent, router['id'], router)
ri_1 = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, [], **self.ri_kwargs) ri_1 = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, **self.ri_kwargs)
ri_1.is_router_master = mock.Mock(return_value=True) ri_1.is_router_master = mock.Mock(return_value=True)
ri_1._add_vip = mock.Mock() ri_1._add_vip = mock.Mock()
interface_name = ri_1.get_snat_external_device_interface_name( interface_name = ri_1.get_snat_external_device_interface_name(
@ -882,7 +882,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
router[lib_constants.HA_INTERFACE_KEY]['status'] = 'ACTIVE' router[lib_constants.HA_INTERFACE_KEY]['status'] = 'ACTIVE'
self._set_ri_kwargs(agent, router['id'], router) self._set_ri_kwargs(agent, router['id'], router)
ri_2 = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, [], **self.ri_kwargs) ri_2 = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, **self.ri_kwargs)
ri_2.is_router_master = mock.Mock(return_value=True) ri_2.is_router_master = mock.Mock(return_value=True)
ri_2._add_vip = mock.Mock() ri_2._add_vip = mock.Mock()
interface_name = ri_2.get_snat_external_device_interface_name( interface_name = ri_2.get_snat_external_device_interface_name(
@ -904,14 +904,14 @@ class TestDvrRouterOperations(base.BaseTestCase):
self._set_ri_kwargs(agent, router['id'], router) self._set_ri_kwargs(agent, router['id'], router)
fip_cidr = '11.22.33.44/24' fip_cidr = '11.22.33.44/24'
ri = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, [], **self.ri_kwargs) ri = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, **self.ri_kwargs)
ri.is_router_master = mock.Mock(return_value=False) ri.is_router_master = mock.Mock(return_value=False)
ri._remove_vip = mock.Mock() ri._remove_vip = mock.Mock()
ri.remove_centralized_floatingip(fip_cidr) ri.remove_centralized_floatingip(fip_cidr)
ri._remove_vip.assert_called_once_with(fip_cidr) ri._remove_vip.assert_called_once_with(fip_cidr)
super_remove_centralized_floatingip.assert_not_called() super_remove_centralized_floatingip.assert_not_called()
ri1 = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, [], **self.ri_kwargs) ri1 = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, **self.ri_kwargs)
ri1.is_router_master = mock.Mock(return_value=True) ri1.is_router_master = mock.Mock(return_value=True)
ri1._remove_vip = mock.Mock() ri1._remove_vip = mock.Mock()
ri1.remove_centralized_floatingip(fip_cidr) ri1.remove_centralized_floatingip(fip_cidr)
@ -927,10 +927,9 @@ class TestDvrRouterOperations(base.BaseTestCase):
router[lib_constants.HA_INTERFACE_KEY]['status'] = 'ACTIVE' router[lib_constants.HA_INTERFACE_KEY]['status'] = 'ACTIVE'
self.mock_driver.unplug.reset_mock() self.mock_driver.unplug.reset_mock()
self._set_ri_kwargs(agent, router['id'], router) self._set_ri_kwargs(agent, router['id'], router)
ri = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, [], **self.ri_kwargs) ri = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, **self.ri_kwargs)
ri._ha_state_path = self.get_temp_file_path('router_ha_state') ri._ha_state_path = self.get_temp_file_path('router_ha_state')
ri._create_snat_namespace = mock.Mock() ri._create_snat_namespace = mock.Mock()
ri.update_initial_state = mock.Mock()
ri._plug_external_gateway = mock.Mock() ri._plug_external_gateway = mock.Mock()
ri.initialize(mock.Mock()) ri.initialize(mock.Mock())
ri._create_dvr_gateway(mock.Mock(), mock.Mock()) ri._create_dvr_gateway(mock.Mock(), mock.Mock())
@ -946,14 +945,13 @@ class TestDvrRouterOperations(base.BaseTestCase):
self.mock_driver.unplug.reset_mock() self.mock_driver.unplug.reset_mock()
self._set_ri_kwargs(agent, router['id'], router) self._set_ri_kwargs(agent, router['id'], router)
ri = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, [], **self.ri_kwargs) ri = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, **self.ri_kwargs)
ri._ha_state_path = self.get_temp_file_path('router_ha_state') ri._ha_state_path = self.get_temp_file_path('router_ha_state')
with open(ri._ha_state_path, "w") as f: with open(ri._ha_state_path, "w") as f:
f.write("master") f.write("master")
ri._create_snat_namespace = mock.Mock() ri._create_snat_namespace = mock.Mock()
ri.update_initial_state = mock.Mock()
ri._plug_external_gateway = mock.Mock() ri._plug_external_gateway = mock.Mock()
with mock.patch("neutron.agent.linux.keepalived." with mock.patch("neutron.agent.linux.keepalived."
"KeepalivedManager.check_processes", "KeepalivedManager.check_processes",

View File

@ -40,8 +40,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
router = mock.MagicMock() router = mock.MagicMock()
self.agent_conf = mock.Mock() self.agent_conf = mock.Mock()
self.router_id = _uuid() self.router_id = _uuid()
return ha_router.HaRouter(mock.sentinel.enqueue_state, return ha_router.HaRouter(mock.sentinel.agent,
mock.sentinel.agent,
self.router_id, self.router_id,
router, router,
self.agent_conf, self.agent_conf,