openstack/ovn-bgp-agent
Code Issues Proposed changes

Merge "Fix running sync method for every external_ids update."

Browse Source
This commit is contained in:
Zuul
2025-04-01 09:41:35 +00:00
committed by Gerrit Code Review
parent 81d53f0d48 f17e6f66f2
commit 7bea7a3a00
3 changed files with 143 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py
View File

@@ -1063,14 +1063,20 @@ class DistributedFlagChangedEvent(base_watcher.Event):
try:
if (old.external_ids.get(constants.OVN_FIP_DISTRIBUTED) ==
row.external_ids[constants.OVN_FIP_DISTRIBUTED]):
return False
return False # no change
distributed = row.external_ids[constants.OVN_FIP_DISTRIBUTED]
distributed_mode = (distributed == "True")
except KeyError:
# Distributed flag was deleted, behave like distributed agent
pass
# Distributed flag was not present, behave like distributed agent
distributed_mode = True
except AttributeError:
# external_ids was not set in old, not our event
return False
return True
# Only match if we are changing the distributed flag to prevent
# unnecessary syncs
return self.agent.distributed != distributed_mode
def run(self, event, row, old):
if row.external_ids.get(constants.OVN_FIP_DISTRIBUTED) == "True":

56
ovn_bgp_agent/tests/functional/drivers/openstack/watchers/test_nb_bgp_watcher.py
View File

@@ -17,6 +17,7 @@ import copy
from ovsdbapp.backend.ovs_idl import event
from ovn_bgp_agent import constants
from ovn_bgp_agent.drivers.openstack.watchers import nb_bgp_watcher
from ovn_bgp_agent.tests.functional import base
from ovn_bgp_agent.tests import utils
@@ -33,6 +34,22 @@ class DistributedWaitEvent(event.WaitEvent):
return row.external_ids != getattr(old, 'external_ids')
class DistributedFlagChangedWaitEvent(
nb_bgp_watcher.DistributedFlagChangedEvent, event.WaitEvent):
'''Same event as the watcher, but only to inherit the match_fn method'''
event_name = 'DistributedFlagChangedWaitEvent'
def __init__(self, agent, timeout=5):
table = 'NB_Global'
events = (self.ROW_UPDATE,)
self.agent = agent
event.WaitEvent.__init__(self, events, table, None, timeout=timeout)
def run(self, *a, **kw):
event.WaitEvent.run(self, *a, **kw)
class DistributedFlagChangedEventTestCase(
base.BaseFunctionalNBAgentTestCase):
@@ -49,6 +66,14 @@ class DistributedFlagChangedEventTestCase(
nb_global_event)
return nb_global_event
def _make_nb_changed_event(self):
nb_flag_event = DistributedFlagChangedWaitEvent(
agent=self.agent, timeout=2
)
self.nb_api.ovsdb_connection.idl.notify_handler.watch_event(
nb_flag_event)
return nb_flag_event
def _wait_for_events_added(self, events):
def _events_intersect():
registered_events = set(idl.notify_handler._watched_events)
@@ -78,6 +103,17 @@ class DistributedFlagChangedEventTestCase(
exception=AssertionError(
"Events %s still registered in the agent" % events))
def _wait_for_agent_distributed_changed(self, new: bool):
def _check_distributed_flag():
return self.agent.distributed == new
utils.wait_until_true(
_check_distributed_flag,
timeout=5,
sleep=0.1,
exception=AssertionError(
f"Agent distributed flag did not change to {new}"))
def test_distributed_flag_changed(self):
distributed = self.nb_api.db_get(
'NB_Global', '.', 'external_ids').execute(check_error=True).get(
@@ -92,18 +128,33 @@ class DistributedFlagChangedEventTestCase(
# At start there is no distributed flag but the agent should default to
# distributed
self.assertTrue(self.agent.distributed)
self._wait_for_events_added(distributed_events)
self._wait_for_events_removed(centralized_events)
nb_global_event = self._make_nb_global_event() # matches every change
nb_flag_event = self._make_nb_changed_event()
self.nb_api.db_set('NB_Global', '.', external_ids={
"some_other_val": "True"}).execute(check_error=True)
self.assertTrue(nb_global_event.wait())
self.assertFalse(nb_flag_event.wait())
# Make sure this is remained True, as nothing should've changed
self.assertTrue(self.agent.distributed)
nb_global_event = self._make_nb_global_event()
self.nb_api.db_set('NB_Global', '.', external_ids={
constants.OVN_FIP_DISTRIBUTED: "False"}).execute(check_error=True)
self.assertTrue(nb_global_event.wait())
self._wait_for_events_added(centralized_events)
self._wait_for_events_removed(distributed_events)
# Make sure the real event has changed the flag as well
self._wait_for_agent_distributed_changed(new=False)
nb_global_event = self._make_nb_global_event()
self.nb_api.db_set('NB_Global', '.', external_ids={
constants.OVN_FIP_DISTRIBUTED: "True"}).execute(check_error=True)
@@ -112,6 +163,7 @@ class DistributedFlagChangedEventTestCase(
self._wait_for_events_added(distributed_events)
self._wait_for_events_removed(centralized_events)
self._wait_for_agent_distributed_changed(new=True)
nb_global_event = self._make_nb_global_event()
self.nb_api.db_set('NB_Global', '.', external_ids={
@@ -121,6 +173,7 @@ class DistributedFlagChangedEventTestCase(
self._wait_for_events_added(centralized_events)
self._wait_for_events_removed(distributed_events)
self._wait_for_agent_distributed_changed(new=False)
nb_global_event = self._make_nb_global_event()
self.nb_api.db_remove(
@@ -131,3 +184,4 @@ class DistributedFlagChangedEventTestCase(
self._wait_for_events_added(distributed_events)
self._wait_for_events_removed(centralized_events)
self._wait_for_agent_distributed_changed(new=True)

78
ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_nb_bgp_watcher.py
View File

@@ -1733,3 +1733,81 @@ class TestNATMACAddedEvent(test_base.TestCase):
external_mac=""
)
self.assertTrue(self._call_match(old))
class TestDistributedFlagChangedEvent(test_base.TestCase):
def setUp(self):
super(TestDistributedFlagChangedEvent, self).setUp()
self.chassis = "fake-chassis"
self.agent = mock.Mock(chassis=self.chassis)
self.agent.distributed = True # unless configured, this is the default
self.event = nb_bgp_watcher.DistributedFlagChangedEvent(self.agent)
def test_match_fn_distributed_key_unrelated_change(self):
event = self.event.ROW_UPDATE
row = utils.create_row(
external_ids={
"someother-key": "bar-value",
},
)
old = utils.create_row(
external_ids={
"someother-key": "foo-value",
},
)
self.assertFalse(self.event.match_fn(event, row, old))
# Another unrelated change is if external_ids is not in the row.
self.assertFalse(self.event.match_fn(event, row, utils.create_row()))
def test_match_fn_distributed_key_no_change(self):
event = self.event.ROW_UPDATE
row = utils.create_row(
external_ids={
constants.OVN_FIP_DISTRIBUTED: "True",
},
)
old = utils.create_row(
external_ids={
constants.OVN_FIP_DISTRIBUTED: "True",
},
)
self.assertFalse(self.event.match_fn(event, row, old))
def test_match_fn_distributed_changed(self):
event = self.event.ROW_UPDATE
row = utils.create_row(
external_ids={
constants.OVN_FIP_DISTRIBUTED: "False",
},
)
old = utils.create_row(
external_ids={
constants.OVN_FIP_DISTRIBUTED: "True",
},
)
self.assertTrue(self.event.match_fn(event, row, old))
self.agent.distributed = False
# Same change should not match again
self.assertFalse(self.event.match_fn(event, row, old))
# But changing back should match.
self.assertTrue(self.event.match_fn(event, old, row))
def test_match_fn_distributed_appear(self):
event = self.event.ROW_UPDATE
row = utils.create_row(
external_ids={
"someother-key": "foo-value",
},
)
old = utils.create_row(
external_ids={
constants.OVN_FIP_DISTRIBUTED: "False",
},
)
self.agent.distributed = False
self.assertTrue(self.event.match_fn(event, row, old))
self.agent.distributed = True
self.assertTrue(self.event.match_fn(event, old, row))