From 4d20d3243fc7376bb31c94834d43550f1200ffcd Mon Sep 17 00:00:00 2001 From: Flavio Fernandes Date: Sat, 12 Jun 2021 08:09:33 -0400 Subject: [PATCH] Ensure that load balancer is added to logical switch Add logic to handle cases where a race between logical router port and load balancer creation renders the affected logical switch without the proper load balancer in its attribute. Conflicts: ovn_octavia_provider/helper.py ovn_octavia_provider/tests/unit/test_helper.py Closes-bug: #1931639 Change-Id: Ie4a843fc5dd98fc25f8cecf91997e275c36f121d (cherry picked from commit b2a862fdb3d4eae440a342f6b6bbc0323022f8ba) --- ovn_octavia_provider/driver.py | 24 ++++++++++- .../tests/unit/test_driver.py | 41 ++++++++++++++++++- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/ovn_octavia_provider/driver.py b/ovn_octavia_provider/driver.py index e1a0ae48..0c4ea162 100644 --- a/ovn_octavia_provider/driver.py +++ b/ovn_octavia_provider/driver.py @@ -1598,10 +1598,32 @@ class OvnProviderHelper(object): commands.extend( self._refresh_lb_vips(ovn_lb.uuid, external_ids) ) + subnet_id = member['subnet_id'] commands.extend( self._update_lb_to_ls_association( - ovn_lb, subnet_id=member['subnet_id'], associate=True) + ovn_lb, subnet_id=subnet_id, associate=True) ) + + # Make sure that all logical switches related to logical router + # are associated with the load balancer. This is needed to handle + # potential race that happens when lrp and lb are created at the + # same time. + neutron_client = get_neutron_client() + try: + subnet = neutron_client.show_subnet(subnet_id) + ls_name = utils.ovn_name(subnet['subnet']['network_id']) + ovn_ls = self.ovn_nbdb_api.ls_get(ls_name).execute( + check_error=True) + ovn_lr = self._find_lr_of_ls(ovn_ls) + if ovn_lr: + for net in self._find_ls_for_lr(ovn_lr): + commands.append(self.ovn_nbdb_api.ls_lb_add( + net, ovn_lb.uuid, may_exist=True)) + except n_exc.NotFound: + pass + except idlutils.RowNotFound: + pass + self._execute_commands(commands) def member_create(self, member): diff --git a/ovn_octavia_provider/tests/unit/test_driver.py b/ovn_octavia_provider/tests/unit/test_driver.py index 5d0545e2..bfd7238e 100644 --- a/ovn_octavia_provider/tests/unit/test_driver.py +++ b/ovn_octavia_provider/tests/unit/test_driver.py @@ -1738,7 +1738,10 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): self.helper.ovn_nbdb_api.lb_del.assert_called_once_with( self.ovn_lb.uuid) - def test_member_create(self): + @mock.patch('ovn_octavia_provider.driver.get_neutron_client') + def test_member_create(self, net_cli): + net_cli.return_value.show_subnet.side_effect = [ + idlutils.RowNotFound, idlutils.RowNotFound] self.ovn_lb.external_ids = mock.MagicMock() status = self.helper.member_create(self.member) self.assertEqual(status['loadbalancers'][0]['provisioning_status'], @@ -1756,6 +1759,38 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): self.assertEqual(status['members'][0]['provisioning_status'], constants.ACTIVE) + @mock.patch.object(ovn_driver.OvnProviderHelper, '_find_ls_for_lr') + @mock.patch.object(ovn_driver.OvnProviderHelper, '_find_lr_of_ls') + @mock.patch('ovn_octavia_provider.driver.get_neutron_client') + def test_member_create_lb_add_from_lr(self, net_cli, f_lr, f_ls): + fake_subnet = fakes.FakeSubnet.create_one_subnet() + net_cli.return_value.show_subnet.return_value = {'subnet': fake_subnet} + f_lr.return_value = self.router + f_ls.return_value = [self.network] + self.ovn_lb.external_ids = mock.MagicMock() + status = self.helper.member_create(self.member) + self.assertEqual(status['loadbalancers'][0]['provisioning_status'], + constants.ACTIVE) + f_lr.assert_called_once_with(self.network) + f_ls.assert_called_once_with(self.router) + + @mock.patch.object(ovn_driver.OvnProviderHelper, '_find_ls_for_lr') + @mock.patch.object(ovn_driver.OvnProviderHelper, '_find_lr_of_ls') + @mock.patch('ovn_octavia_provider.driver.get_neutron_client') + def test_member_create_lb_add_from_lr_no_ls(self, net_cli, f_lr, f_ls): + fake_subnet = fakes.FakeSubnet.create_one_subnet() + net_cli.return_value.show_subnet.return_value = {'subnet': fake_subnet} + self.ovn_lb.external_ids = mock.MagicMock() + (self.helper.ovn_nbdb_api.ls_get.return_value. + execute.side_effect) = [n_exc.NotFound] + status = self.helper.member_create(self.member) + self.assertEqual(status['loadbalancers'][0]['provisioning_status'], + constants.ACTIVE) + (self.helper.ovn_nbdb_api.ls_get.return_value.execute. + assert_called_once_with(check_error=True)) + f_lr.assert_not_called() + f_ls.assert_not_called() + @mock.patch.object(ovn_driver.OvnProviderHelper, '_add_member') def test_member_create_exception(self, mock_add_member): mock_add_member.side_effect = [RuntimeError] @@ -1771,7 +1806,9 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): [mock.call('pool_%s' % self.pool_id), mock.call('pool_%s%s' % (self.pool_id, ':D'))]) - def test_member_create_listener(self): + @mock.patch('ovn_octavia_provider.driver.get_neutron_client') + def test_member_create_listener(self, net_cli): + net_cli.return_value.show_subnet.side_effect = [idlutils.RowNotFound] self.ovn_lb.external_ids = mock.MagicMock() self.helper._get_pool_listeners.return_value = ['listener1'] status = self.helper.member_create(self.member)