From 4029128d8b650b1165fd22b89f1a9216fa2b9180 Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Fri, 30 Sep 2022 20:13:49 +0200 Subject: [PATCH] Ensure lbs are properly configured for router gateway set/unset Before adding support for lbs with VIPs on the provider networks, there was no need to react to gateway chassis creation events, and nothing was done for its deletion. However, after adding the support for that, there is a need of properly handling the creation event for that type of port. For the loadbalancer VIPs on the provider network, processing the event and triggering the lb_creat_lrp_assoc_handler means that the information about the logical router will be added, i.e., the router is added to the loadbalancer external_ids as a lr_ref, while the loadbalancer is also added into the logical_router object (loadbalancer entry). In addition, the lb is also added to the logical switches connected to the router. For the loadbalancer VIPs on tenant networks (which should not be affected by the gateway port creation/deletion), this patch ensures the lb is not added to the logical switch representing the provider network that is connected to the router. Therefore differentiating between lrp ports which has gateway_chassis and the ones that don't, i.e., adding the lb to the switch when the lrp port is the one connecting a subnet with the router, and not doing so when it is the gateway port for the router to the provider network. Closes-Bug: #1991509 Change-Id: Iddd96fd9015230b3dd75aa2182055cf43eb608c1 (cherry picked from commit d889812f3c2deaf630892d65c90cdbafd4435d21) --- ovn_octavia_provider/event.py | 2 -- ovn_octavia_provider/helper.py | 12 ++++++++++- .../tests/unit/test_helper.py | 21 +++++++++++++++---- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/ovn_octavia_provider/event.py b/ovn_octavia_provider/event.py index 7ccfa3b7..39ec6308 100644 --- a/ovn_octavia_provider/event.py +++ b/ovn_octavia_provider/event.py @@ -36,8 +36,6 @@ class LogicalRouterPortEvent(row_event.RowEvent): '%(event)s, %(row)s', {'event': event, 'row': row}) - if row.gateway_chassis: - return if event == self.ROW_CREATE: self.driver.lb_create_lrp_assoc_handler(row) elif event == self.ROW_DELETE: diff --git a/ovn_octavia_provider/helper.py b/ovn_octavia_provider/helper.py index bce2636a..2708daed 100644 --- a/ovn_octavia_provider/helper.py +++ b/ovn_octavia_provider/helper.py @@ -241,7 +241,8 @@ class OvnProviderHelper(): LOG.debug("Router or network information not found") return request_info = {'network': network, - 'router': router} + 'router': router, + 'gateway_chassis': row.gateway_chassis} self.add_request({'type': ovn_const.REQ_TYPE_LB_CREATE_LRP_ASSOC, 'info': request_info}) @@ -258,6 +259,15 @@ class OvnProviderHelper(): lb.uuid, info['router'].uuid) self._update_lb_to_lr_association_by_step(lb, info['router']) + # if gateway_chassis ports, there is no need to re-add the + # loadbalancers from the router into the provider network. + # This will be already done for loadbalancer created with VIPs on + # provider networks, regardless of the gateway_chassis lrp port + # existence. And it should never be there when the VIPs are on + # tenant networks. + if info['gateway_chassis']: + return + # Add those lb to the network which are unique to the router for lb in (router_lb - network_lb): try: diff --git a/ovn_octavia_provider/tests/unit/test_helper.py b/ovn_octavia_provider/tests/unit/test_helper.py index 3fb2a8a1..a5214bc6 100644 --- a/ovn_octavia_provider/tests/unit/test_helper.py +++ b/ovn_octavia_provider/tests/unit/test_helper.py @@ -1893,7 +1893,8 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): expected = { 'info': {'router': self.router, - 'network': self.network}, + 'network': self.network, + 'gateway_chassis': []}, 'type': 'lb_create_lrp_assoc'} self.mock_add_request.assert_called_once_with(expected) @@ -1918,7 +1919,13 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): row = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'gateway_chassis': ['temp-gateway-chassis']}) self.router_port_event.run(mock.ANY, row, mock.ANY) - self.mock_add_request.assert_not_called() + expected = { + 'info': + {'router': self.router, + 'network': self.network, + 'gateway_chassis': ['temp-gateway-chassis']}, + 'type': 'lb_create_lrp_assoc'} + self.mock_add_request.assert_called_once_with(expected) def test__get_pool_listeners(self): self._get_pool_listeners.stop() @@ -2088,12 +2095,14 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): self.router, delete=True) def test_lb_create_lrp_assoc_handler(self): - lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row() + lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'gateway_chassis': []}) self.helper.lb_create_lrp_assoc_handler(lrp) expected = { 'info': {'router': self.router, - 'network': self.network}, + 'network': self.network, + 'gateway_chassis': []}, 'type': 'lb_create_lrp_assoc'} self.mock_add_request.assert_called_once_with(expected) @@ -2112,6 +2121,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): info = { 'network': self.network, 'router': self.router, + 'gateway_chassis': [], } self.helper.lb_create_lrp_assoc(info) self.helper._update_lb_to_lr_association.assert_called_once_with( @@ -2122,6 +2132,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): info = { 'network': self.network, 'router': self.router, + 'gateway_chassis': [], } self.helper._update_lb_to_ls_association.side_effect = [ idlutils.RowNotFound] @@ -2139,6 +2150,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): info = { 'network': self.network, 'router': self.router, + 'gateway_chassis': 'fake-chassis', } self.helper._update_lb_to_lr_association.side_effect = [ idlutils.RowNotFound] @@ -2155,6 +2167,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): info = { 'network': self.network, 'router': self.router, + 'gateway_chassis': 'fake-chassis', } # Make it already uniq. self.network.load_balancer = self.router.load_balancer