diff --git a/ovn_octavia_provider/driver.py b/ovn_octavia_provider/driver.py index 5a42cbf5..dd8ea0f4 100644 --- a/ovn_octavia_provider/driver.py +++ b/ovn_octavia_provider/driver.py @@ -213,6 +213,8 @@ class OvnProviderDriver(driver_base.ProviderDriver): def _ip_version_differs(self, member): _, ovn_lb = self._ovn_helper._find_ovn_lb_by_pool_id(member.pool_id) + if not ovn_lb: + return False lb_vip = ovn_lb.external_ids[ovn_const.LB_EXT_IDS_VIP_KEY] return netaddr.IPNetwork(lb_vip).version != ( netaddr.IPNetwork(member.address).version) @@ -223,13 +225,16 @@ class OvnProviderDriver(driver_base.ProviderDriver): if self._ip_version_differs(member): raise ovn_exc.IPVersionsMixingNotSupportedError() admin_state_up = member.admin_state_up - if (isinstance(member.subnet_id, o_datamodels.UnsetType) or - not member.subnet_id): - msg = _('Subnet is required for Member creation ' - 'with OVN Provider Driver') - raise driver_exceptions.UnsupportedOptionError( - user_fault_string=msg, - operator_fault_string=msg) + subnet_id = member.subnet_id + if (isinstance(subnet_id, o_datamodels.UnsetType) or not subnet_id): + subnet_id = self._ovn_helper._get_subnet_from_pool(member.pool_id) + if not subnet_id: + msg = _('Subnet is required, or Loadbalancer associated with ' + 'Pool must have a subnet, for Member creation ' + 'with OVN Provider Driver') + raise driver_exceptions.UnsupportedOptionError( + user_fault_string=msg, + operator_fault_string=msg) if isinstance(admin_state_up, o_datamodels.UnsetType): admin_state_up = True @@ -237,7 +242,7 @@ class OvnProviderDriver(driver_base.ProviderDriver): 'address': member.address, 'protocol_port': member.protocol_port, 'pool_id': member.pool_id, - 'subnet_id': member.subnet_id, + 'subnet_id': subnet_id, 'admin_state_up': admin_state_up} request = {'type': ovn_const.REQ_TYPE_MEMBER_CREATE, 'info': request_info} @@ -249,7 +254,7 @@ class OvnProviderDriver(driver_base.ProviderDriver): request_info = {'id': member.member_id, 'address': member.address, 'pool_id': member.pool_id, - 'subnet_id': member.subnet_id, + 'subnet_id': subnet_id, 'action': ovn_const.REQ_INFO_MEMBER_ADDED} request = {'type': ovn_const.REQ_TYPE_HANDLE_MEMBER_DVR, 'info': request_info} diff --git a/ovn_octavia_provider/helper.py b/ovn_octavia_provider/helper.py index 9693b011..01b01ed6 100644 --- a/ovn_octavia_provider/helper.py +++ b/ovn_octavia_provider/helper.py @@ -466,6 +466,14 @@ class OvnProviderHelper(): ovn_lb = self._find_ovn_lb_with_pool_key(pool_key) return pool_key, ovn_lb + def _get_subnet_from_pool(self, pool_id): + pool = self._octavia_driver_lib.get_pool(pool_id) + if not pool: + return + lb = self._octavia_driver_lib.get_loadbalancer(pool.loadbalancer_id) + if lb and lb.vip_subnet_id: + return lb.vip_subnet_id + def _execute_commands(self, commands): with self.ovn_nbdb_api.transaction(check_error=True) as txn: for command in commands: diff --git a/ovn_octavia_provider/tests/functional/base.py b/ovn_octavia_provider/tests/functional/base.py index 08309677..0406345a 100644 --- a/ovn_octavia_provider/tests/functional/base.py +++ b/ovn_octavia_provider/tests/functional/base.py @@ -696,7 +696,7 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase, return listeners def _create_member_and_validate(self, lb_data, pool_id, subnet_id, - network_id, address): + network_id, address, expected_subnet=None): self._o_driver_lib.update_loadbalancer_status.reset_mock() pool = self._get_pool_from_lb_data(lb_data, pool_id=pool_id) pool_status = {'id': pool.pool_id, @@ -704,7 +704,15 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase, 'operating_status': o_constants.ONLINE} m_member = self._create_member_model(pool.pool_id, subnet_id, address) - pool.members.append(m_member) + # The "expected" member value, which might be different from what + # we pass to member_create(), for example, if an expected_subnet + # was given. + if expected_subnet: + e_member = copy.deepcopy(m_member) + e_member.subnet_id = expected_subnet + else: + e_member = m_member + pool.members.append(e_member) self.ovn_driver.member_create(m_member) self._update_ls_refs(lb_data, network_id) diff --git a/ovn_octavia_provider/tests/functional/test_driver.py b/ovn_octavia_provider/tests/functional/test_driver.py index aefe811f..421821ce 100644 --- a/ovn_octavia_provider/tests/functional/test_driver.py +++ b/ovn_octavia_provider/tests/functional/test_driver.py @@ -154,13 +154,39 @@ class TestOvnOctaviaProviderDriver(ovn_base.TestOvnOctaviaBase): self._delete_member_and_validate(lb_data, pool_TCP_id, net20, '20.0.0.6') - # Test creating Member without subnet - m_member = self._create_member_model(pool_TCP_id, + # Deleting the pool should also delete the members. + self._delete_pool_and_validate(lb_data, "p_TCP") + + # Delete the whole LB. + self._delete_load_balancer_and_validate(lb_data) + + def test_member_no_subnet(self): + self._o_driver_lib.get_pool.return_value = None + + # Test creating Member without subnet and unknown pool + m_member = self._create_member_model('pool_from_nowhere', None, '30.0.0.7', 80) self.assertRaises(o_exceptions.UnsupportedOptionError, self.ovn_driver.member_create, m_member) + lb_data = self._create_load_balancer_and_validate( + {'vip_network': 'vip_network', + 'cidr': '10.0.0.0/24'}) + + # TCP Pool + self._create_pool_and_validate(lb_data, "p_TCP", protocol='TCP') + pool_TCP_id = lb_data['pools'][0].pool_id + + self._o_driver_lib.get_pool.return_value = lb_data['pools'][0] + self._o_driver_lib.get_loadbalancer.return_value = lb_data['model'] + + # Test creating Member without subnet but with pool + self._create_member_and_validate( + lb_data, pool_TCP_id, None, + lb_data['vip_net_info'][0], '10.0.0.10', + expected_subnet=lb_data['vip_net_info'][1]) + # Deleting the pool should also delete the members. self._delete_pool_and_validate(lb_data, "p_TCP") diff --git a/ovn_octavia_provider/tests/unit/test_driver.py b/ovn_octavia_provider/tests/unit/test_driver.py index 45b5bf7a..aacddc7c 100644 --- a/ovn_octavia_provider/tests/unit/test_driver.py +++ b/ovn_octavia_provider/tests/unit/test_driver.py @@ -215,6 +215,10 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase): ovn_helper.OvnProviderHelper, '_find_ovn_lb_with_pool_key', return_value=self.ovn_lb).start() + self.mock_get_subnet_from_pool = mock.patch.object( + ovn_helper.OvnProviderHelper, + '_get_subnet_from_pool', + return_value=None).start() def test__ip_version_differs(self): self.assertFalse(self.driver._ip_version_differs(self.ref_member)) @@ -228,7 +232,7 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase): mock.call('pool_%s' % self.pool_id), mock.call('pool_%s:D' % self.pool_id)]) - def test_member_create(self): + def _test_member_create(self, member): info = {'id': self.ref_member.member_id, 'address': self.ref_member.address, 'protocol_port': self.ref_member.protocol_port, @@ -246,12 +250,15 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase): expected_dict_dvr = { 'type': ovn_const.REQ_TYPE_HANDLE_MEMBER_DVR, 'info': info_dvr} - self.driver.member_create(self.ref_member) + self.driver.member_create(member) expected = [ mock.call(expected_dict), mock.call(expected_dict_dvr)] self.mock_add_request.assert_has_calls(expected) + def test_member_create(self): + self._test_member_create(self.ref_member) + def test_member_create_failure(self): self.assertRaises(exceptions.UnsupportedOptionError, self.driver.member_create, self.fail_member) @@ -279,6 +286,15 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase): self.assertRaises(exceptions.UnsupportedOptionError, self.driver.member_create, self.ref_member) + def test_member_create_no_subnet_provided_get_from_pool(self): + self.driver._ovn_helper._get_subnet_from_pool.return_value = ( + self.ref_member.subnet_id) + member = copy.copy(self.ref_member) + member.subnet_id = data_models.UnsetType() + self._test_member_create(member) + member.subnet_id = None + self._test_member_create(member) + def test_member_create_monitor_opts(self): self.ref_member.monitor_address = '172.20.20.1' self.assertRaises(exceptions.UnsupportedOptionError, diff --git a/ovn_octavia_provider/tests/unit/test_helper.py b/ovn_octavia_provider/tests/unit/test_helper.py index 33ae64c8..e38b621e 100644 --- a/ovn_octavia_provider/tests/unit/test_helper.py +++ b/ovn_octavia_provider/tests/unit/test_helper.py @@ -15,6 +15,7 @@ import copy from unittest import mock from neutronclient.common import exceptions as n_exc +from octavia_lib.api.drivers import data_models from octavia_lib.api.drivers import exceptions from octavia_lib.common import constants from oslo_utils import uuidutils @@ -290,6 +291,32 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): found = f(self.ovn_lb.id) self.assertListEqual(found, [self.ovn_lb, udp_lb, sctp_lb]) + def test__get_subnet_from_pool(self): + f = self.helper._get_subnet_from_pool + + lb = data_models.LoadBalancer( + loadbalancer_id=self.loadbalancer_id, + name='The LB', + vip_address=self.vip_address, + vip_subnet_id=self.vip_subnet_id, + vip_network_id=self.vip_network_id) + + lb_pool = data_models.Pool( + loadbalancer_id=self.loadbalancer_id, + name='The pool', + pool_id=self.pool_id, + protocol='TCP') + + with mock.patch.object(self.helper, '_octavia_driver_lib') as dlib: + dlib.get_pool.return_value = None + found = f('not_found') + self.assertIsNone(found) + + dlib.get_pool.return_value = lb_pool + dlib.get_loadbalancer.return_value = lb + found = f(self.pool_id) + self.assertEqual(found, lb.vip_subnet_id) + def test__get_or_create_ovn_lb_no_lb_found(self): self.mock_find_ovn_lbs.stop() self.helper.ovn_nbdb_api.db_find_rows.return_value.\ diff --git a/releasenotes/notes/support-member-create-without-subnetid-0b4e3aa6ac453f28.yaml b/releasenotes/notes/support-member-create-without-subnetid-0b4e3aa6ac453f28.yaml new file mode 100644 index 00000000..b799c7a8 --- /dev/null +++ b/releasenotes/notes/support-member-create-without-subnetid-0b4e3aa6ac453f28.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Creating members without specifying a subnet ID is now supported. + Since the subnet ID is an optional API argument, if not given + the provider driver will now attempt to look it up via the pool + ID that is a required argument.