From eb1e24ac46f458887a31ffde54342ec2e34345f7 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Mon, 23 Mar 2020 16:00:15 -0400 Subject: [PATCH] Improve test coverage Added a number of unit tests based on coverage report data, it's up to 79% now. Fixed test_pool_create_exception() to call pool_create() instead of pool_update(). Also fixed a bug in the driver.pool_update() exception code path found when adding a new test, status['pools'] is the correct element. Change-Id: Ic72aad21e0ecf5b0334b321b18515d909daa3ba4 --- ovn_octavia_provider/driver.py | 6 +- .../tests/unit/test_driver.py | 148 +++++++++++++++++- tox.ini | 2 +- 3 files changed, 151 insertions(+), 5 deletions(-) diff --git a/ovn_octavia_provider/driver.py b/ovn_octavia_provider/driver.py index 84df1e8b..170357dc 100644 --- a/ovn_octavia_provider/driver.py +++ b/ovn_octavia_provider/driver.py @@ -1278,7 +1278,7 @@ class OvnProviderHelper(object): protocol=listener['protocol']) except idlutils.RowNotFound: LOG.exception(EXCEPTION_MSG, "update of listener") - # LB row not found during updating a listner. That is a problem. + # LB row not found during update of a listener. That is a problem. status['listeners'][0]['provisioning_status'] = constants.ERROR status['loadbalancers'][0]['provisioning_status'] = constants.ERROR return status @@ -1483,8 +1483,8 @@ class OvnProviderHelper(object): pool['loadbalancer_id'], protocol=pool['protocol']) except idlutils.RowNotFound: LOG.exception(EXCEPTION_MSG, "update of pool") - # LB row not found during updating a listner. That is a problem. - status['pool'][0]['provisioning_status'] = constants.ERROR + # LB row not found during update of a listener. That is a problem. + status['pools'][0]['provisioning_status'] = constants.ERROR status['loadbalancers'][0]['provisioning_status'] = constants.ERROR return status diff --git a/ovn_octavia_provider/tests/unit/test_driver.py b/ovn_octavia_provider/tests/unit/test_driver.py index fe5735b1..f1432819 100644 --- a/ovn_octavia_provider/tests/unit/test_driver.py +++ b/ovn_octavia_provider/tests/unit/test_driver.py @@ -1169,6 +1169,13 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): self.assertEqual(status['loadbalancers'][0]['operating_status'], constants.ERROR) + def test_lb_update_no_admin_state_up(self): + self.lb.pop('admin_state_up') + status = self.helper.lb_update(self.lb) + self.assertEqual(status['loadbalancers'][0]['provisioning_status'], + constants.ACTIVE) + self.helper._find_ovn_lbs.assert_not_called() + @mock.patch.object(ovn_driver.OvnProviderHelper, '_refresh_lb_vips') def test_listener_create_disabled(self, refresh_vips): self.ovn_lb.external_ids.pop('listener_%s' % self.listener_id) @@ -1331,6 +1338,33 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): refresh_vips.assert_called_once_with( self.ovn_lb.uuid, self.ovn_lb.external_ids) + @mock.patch.object(ovn_driver.OvnProviderHelper, '_refresh_lb_vips') + def test_listener_update_no_admin_state_up(self, refresh_vips): + self.listener.pop('admin_state_up') + status = self.helper.listener_update(self.listener) + self.assertEqual(status['loadbalancers'][0]['provisioning_status'], + constants.ACTIVE) + self.assertEqual(status['listeners'][0]['provisioning_status'], + constants.ACTIVE) + self.assertEqual(status['pools'][0]['provisioning_status'], + constants.ACTIVE) + self.helper.ovn_nbdb_api.db_remove.assert_not_called() + refresh_vips.assert_called_once_with( + self.ovn_lb.uuid, self.ovn_lb.external_ids) + + @mock.patch.object(ovn_driver.OvnProviderHelper, '_refresh_lb_vips') + def test_listener_update_no_admin_state_up_or_default_pool_id( + self, refresh_vips): + self.listener.pop('admin_state_up') + self.listener.pop('default_pool_id') + status = self.helper.listener_update(self.listener) + self.assertEqual(status['loadbalancers'][0]['provisioning_status'], + constants.ACTIVE) + self.assertEqual(status['listeners'][0]['provisioning_status'], + constants.ACTIVE) + self.helper.ovn_nbdb_api.db_remove.assert_not_called() + refresh_vips.assert_not_called() + def test_listener_delete_no_external_id(self): self.ovn_lb.external_ids.pop('listener_%s' % self.listener_id) status = self.helper.listener_delete(self.listener) @@ -1447,7 +1481,7 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): def test_pool_create_exception(self): self.helper.ovn_nbdb_api.db_set.side_effect = [RuntimeError] - status = self.helper.pool_update(self.pool) + status = self.helper.pool_create(self.pool) self.assertEqual(status['pools'][0]['provisioning_status'], constants.ERROR) self.assertEqual(status['loadbalancers'][0]['provisioning_status'], @@ -1470,6 +1504,12 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): self.assertEqual(status['pools'][0]['operating_status'], constants.ONLINE) + def test_pool_update_exception_not_found(self): + self.helper._find_ovn_lbs.side_effect = [idlutils.RowNotFound] + status = self.helper.pool_update(self.pool) + self.assertEqual(status['pools'][0]['provisioning_status'], + constants.ERROR) + def test_pool_update_exception(self): self.helper._get_pool_listeners.side_effect = [RuntimeError] status = self.helper.pool_update(self.pool) @@ -2276,6 +2316,43 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): returned_lr = self.helper._find_lr_of_ls(ls) self.assertEqual(lr, returned_lr) + def test__find_lr_of_ls_gw_port_id(self): + lsp = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + 'external_ids': { + ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router1'}, + 'type': 'router', + 'options': { + 'router-port': 'lrp-lrp-foo-name'} + }) + lr = fakes.FakeOVNRouter.create_one_router( + attrs={ + 'name': 'router1', + 'ports': [], + 'external_ids': { + 'neutron:gw_port_id': 'lrp-foo-name'}}) + ls = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'ports': [lsp]}) + + (self.helper.ovn_nbdb_api.tables['Logical_Router'].rows. + values.return_value) = [lr] + returned_lr = self.helper._find_lr_of_ls(ls) + self.assertEqual(lr, returned_lr) + + def test__find_lr_of_ls_no_lrp_name(self): + lsp = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + 'external_ids': { + ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router1'}, + 'type': 'router', + 'options': { + 'router-port': None} + }) + ls = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'ports': [lsp]}) + returned_lr = self.helper._find_lr_of_ls(ls) + self.assertIsNone(returned_lr) + def test__find_lr_of_ls_no_lrp(self): ls = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'ports': []}) @@ -2594,6 +2671,38 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): self.helper.handle_member_dvr(info) self.helper.ovn_nbdb_api.db_clear.assert_not_called() + @mock.patch('ovn_octavia_provider.driver.get_neutron_client') + def test_handle_member_dvr_lb_fip_no_subnet(self, net_cli): + lb = mock.MagicMock() + info = { + 'id': self.member_id, + 'subnet_id': self.member_subnet_id, + 'pool_id': self.pool_id, + 'action': ovn_driver.REQ_INFO_MEMBER_ADDED} + external_ids = { + 'neutron:vip_fip': '11.11.11.11'} + lb.external_ids = external_ids + self.mock_find_lb_pool_key.return_value = lb + net_cli.return_value.show_subnet.side_effect = [n_exc.NotFound] + self.helper.handle_member_dvr(info) + self.helper.ovn_nbdb_api.db_clear.assert_not_called() + + @mock.patch('ovn_octavia_provider.driver.get_neutron_client') + def test_handle_member_dvr_lb_fip_no_ls(self, net_cli): + lb = mock.MagicMock() + info = { + 'id': self.member_id, + 'subnet_id': self.member_subnet_id, + 'pool_id': self.pool_id, + 'action': ovn_driver.REQ_INFO_MEMBER_ADDED} + external_ids = { + 'neutron:vip_fip': '11.11.11.11'} + lb.external_ids = external_ids + self.mock_find_lb_pool_key.return_value = lb + self.helper.ovn_nbdb_api.lookup.side_effect = [idlutils.RowNotFound] + self.helper.handle_member_dvr(info) + self.helper.ovn_nbdb_api.db_clear.assert_not_called() + def _test_handle_member_dvr_lb_fip( self, net_cli, action=ovn_driver.REQ_INFO_MEMBER_ADDED): lb = mock.MagicMock() @@ -2778,12 +2887,25 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): ret = self.helper._get_existing_pool_members(self.pool_id) self.assertEqual(ret, self.member_line) + @mock.patch('ovn_octavia_provider.driver.OvnProviderHelper.' + '_find_ovn_lb_by_pool_id') + def test__get_existing_pool_members_exception(self, folbpi): + folbpi.return_value = (None, None) + self.assertRaises(exceptions.DriverError, + self.helper._get_existing_pool_members, + self.pool_id) + def test__frame_lb_vips(self): ret = self.helper._frame_vip_ips(self.ovn_lb.external_ids) expected = {'10.22.33.4:80': '192.168.2.149:1010', '123.123.123.123:80': '192.168.2.149:1010'} self.assertEqual(expected, ret) + def test__frame_lb_vips_disabled(self): + self.ovn_lb.external_ids['enabled'] = 'False' + ret = self.helper._frame_vip_ips(self.ovn_lb.external_ids) + self.assertEqual({}, ret) + def test__frame_lb_vips_ipv6(self): self.member_address = '2001:db8::1' self.member_line = ( @@ -2799,3 +2921,27 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): expected = {'[2002::]:80': '[2001:db8::1]:1010', '[fc00::]:80': '[2001:db8::1]:1010'} self.assertEqual(expected, ret) + + def test_check_lb_protocol(self): + self.ovn_lb.protocol = ['tcp'] + ret = self.helper.check_lb_protocol(self.listener_id, 'udp') + self.assertFalse(ret) + ret = self.helper.check_lb_protocol(self.listener_id, 'UDP') + self.assertFalse(ret) + + ret = self.helper.check_lb_protocol(self.listener_id, 'tcp') + self.assertTrue(ret) + ret = self.helper.check_lb_protocol(self.listener_id, 'TCP') + self.assertTrue(ret) + + def test_check_lb_protocol_no_listener(self): + self.ovn_lb.external_ids = [] + ret = self.helper.check_lb_protocol(self.listener_id, 'TCP') + self.assertTrue(ret) + + @mock.patch('ovn_octavia_provider.driver.OvnProviderHelper.' + '_find_ovn_lbs') + def test_check_lb_protocol_no_lb(self, fol): + fol.return_value = None + ret = self.helper.check_lb_protocol(self.listener_id, 'TCP') + self.assertFalse(ret) diff --git a/tox.ini b/tox.ini index 49e19f3f..63964e97 100644 --- a/tox.ini +++ b/tox.ini @@ -63,7 +63,7 @@ setenv = commands = stestr run --no-subunit-trace {posargs} coverage combine - coverage report --fail-under=75 --skip-covered + coverage report --fail-under=79 --skip-covered coverage html -d cover coverage xml -o cover/coverage.xml