openstack/ovn-bgp-agent
Code Issues Proposed changes

Ensure PortBindingChassis Events consider the port up status

Browse Source 
There may be cases where the port is not up, so it does not matter
if the chassis is set, we don't need to expose the IP, and the
opposite for the deletion. This also avoid races where the chassis
gets re-added but the port is still down, avoiding to process extra
events that are not needed

Change-Id: I64680c4ce8d776e57c16e06e0f376d7d9fba3733
This commit is contained in:
Luis Tomas Bolivar 2023-06-15 09:12:26 +02:00
parent e3828acc48
commit b5ac73f138
2 changed files with 61 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py
View File

@ -35,8 +35,17 @@ class PortBindingChassisCreatedEvent(base_watcher.PortBindingChassisEvent):
# single and dual-stack format
if not self._check_ip_associated(row.mac[0]):
return False
return (row.chassis[0].name == self.agent.chassis and
(not old.chassis or row.chassis != old.chassis))
if not bool(row.up[0]):
return False
if row.chassis[0].name != self.agent.chassis:
return False
if hasattr(old, 'chassis'):
if not old.chassis or row.chassis != old.chassis:
return True
if hasattr(old, 'up'):
if not bool(old.up[0]):
return True
except (IndexError, AttributeError):
return False
@ -59,11 +68,16 @@ class PortBindingChassisDeletedEvent(base_watcher.PortBindingChassisEvent):
# single and dual-stack format
if not self._check_ip_associated(row.mac[0]):
return False
if event == self.ROW_UPDATE:
return (old.chassis[0].name == self.agent.chassis and
(not row.chassis or row.chassis != old.chassis))
else:
if event == self.ROW_DELETE:
return row.chassis[0].name == self.agent.chassis
if hasattr(old, 'chassis'):
if (old.chassis[0].name == self.agent.chassis and
(not row.chassis or row.chassis != old.chassis)):
return True
if hasattr(old, 'up'):
if bool(old.up[0]) and not bool(row.up[0]):
return True
except (IndexError, AttributeError):
return False

49
ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_bgp_watcher.py
View File

@ -37,7 +37,8 @@ class TestPortBindingChassisCreatedEvent(test_base.TestCase):
def test_match_fn(self):
ch = utils.create_row(name=self.chassis)
row = utils.create_row(chassis=[ch],
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'])
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'],
up=[True])
old = utils.create_row(chassis=[])
self.assertTrue(self.event.match_fn(mock.Mock(), row, old))
@ -51,14 +52,16 @@ class TestPortBindingChassisCreatedEvent(test_base.TestCase):
def test_match_fn_old_chassis_set(self):
ch = utils.create_row(name=self.chassis)
row = utils.create_row(chassis=[ch],
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'])
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'],
up=[True])
old = utils.create_row(chassis=[ch])
self.assertFalse(self.event.match_fn(mock.Mock(), row, old))
def test_match_fn_no_old_chassis(self):
ch = utils.create_row(name=self.chassis)
row = utils.create_row(chassis=[ch],
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'])
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'],
up=[True])
old = utils.create_row(chassis=[])
self.assertTrue(self.event.match_fn(mock.Mock(), row, old))
@ -66,10 +69,27 @@ class TestPortBindingChassisCreatedEvent(test_base.TestCase):
ch = utils.create_row(name=self.chassis)
ch_old = utils.create_row(name='old-chassis')
row = utils.create_row(chassis=[ch],
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'])
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'],
up=[True])
old = utils.create_row(chassis=[ch_old])
self.assertTrue(self.event.match_fn(mock.Mock(), row, old))
def test_match_fn_no_up(self):
ch = utils.create_row(name=self.chassis)
row = utils.create_row(chassis=[ch],
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'],
up=[False])
self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock))
def test_match_fn_no_old_up(self):
ch = utils.create_row(name=self.chassis)
row = utils.create_row(chassis=[ch],
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'],
up=[True])
old = utils.create_row(chassis=[ch],
up=[False])
self.assertTrue(self.event.match_fn(mock.Mock(), row, old))
def test_match_fn_index_error(self):
row = utils.create_row(chassis=[],
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'])
@ -106,11 +126,12 @@ class TestPortBindingChassisDeletedEvent(test_base.TestCase):
self.event = bgp_watcher.PortBindingChassisDeletedEvent(self.agent)
def test_match_fn(self):
event = self.event.ROW_DELETE
ch = utils.create_row(name=self.chassis)
row = utils.create_row(chassis=[ch],
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'])
old = utils.create_row(chassis=[])
self.assertTrue(self.event.match_fn(mock.Mock(), row, old))
self.assertTrue(self.event.match_fn(event, row, old))
def test_match_fn_not_single_or_dual_stack(self):
ch = utils.create_row(name=self.chassis)
@ -127,14 +148,26 @@ class TestPortBindingChassisDeletedEvent(test_base.TestCase):
old = utils.create_row(chassis=[ch])
self.assertTrue(self.event.match_fn(event, row, old))
def test_match_fn_update_old_chassis_set(self):
def test_match_fn_update_old_chassis_set_up_false(self):
event = self.event.ROW_UPDATE
ch = utils.create_row(name=self.chassis)
row = utils.create_row(chassis=[ch],
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'])
old = utils.create_row(chassis=[ch])
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'],
up=[True])
old = utils.create_row(chassis=[ch],
up=[False])
self.assertFalse(self.event.match_fn(event, row, old))
def test_match_fn_update_old_chassis_set_up_true(self):
event = self.event.ROW_UPDATE
ch = utils.create_row(name=self.chassis)
row = utils.create_row(chassis=[ch],
mac=['aa:bb:cc:dd:ee:ff 10.10.1.16'],
up=[False])
old = utils.create_row(chassis=[ch],
up=[True])
self.assertTrue(self.event.match_fn(event, row, old))
def test_match_fn_update_no_chassis(self):
event = self.event.ROW_UPDATE
ch = utils.create_row(name=self.chassis)