From 30db16277c4c46f4f1787b755e145a4998aebaf4 Mon Sep 17 00:00:00 2001 From: Austin Russell Date: Mon, 29 Jul 2019 15:41:15 -0400 Subject: [PATCH] loadbalancer vip-network-id IP availability check The existing code selects the first IPv4 subnet in the network without any consideration of ip availability. If not enough IPs are available, the loadbalancer creations fails. This patch uses neutron ip availability API to check the quantity of free IPs when creating loadbalancer with vip-network-id and skips subnets that do not have enough IPs for a loadbalancer on multi subnet networks. Change-Id: If3c3cf9be085bb95b4ebbaf71e24f92d42b8d6e0 Task: 36004 Story: 2006293 --- octavia/api/v2/controllers/load_balancer.py | 30 +++++-- octavia/common/utils.py | 7 ++ octavia/network/base.py | 9 ++ octavia/network/data_models.py | 14 ++++ octavia/network/drivers/neutron/base.py | 3 + octavia/network/drivers/neutron/utils.py | 9 ++ octavia/network/drivers/noop_driver/driver.py | 18 ++++ octavia/tests/common/constants.py | 16 ++++ .../functional/api/v2/test_load_balancer.py | 83 +++++++++++++++++++ .../unit/network/drivers/neutron/test_base.py | 15 ++++ .../network/drivers/neutron/test_utils.py | 16 ++++ ...work-ip-availability-2e924f32abf01052.yaml | 7 ++ 12 files changed, 220 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/fix-vip-network-ip-availability-2e924f32abf01052.yaml diff --git a/octavia/api/v2/controllers/load_balancer.py b/octavia/api/v2/controllers/load_balancer.py index c843555341..495dbb4fc7 100644 --- a/octavia/api/v2/controllers/load_balancer.py +++ b/octavia/api/v2/controllers/load_balancer.py @@ -127,9 +127,29 @@ class LoadBalancersController(base.BaseController): "VIP address specified." )) else: - # If subnet and IP are not provided, pick the first subnet, - # preferring ipv4 - for subnet_id in network.subnets: + # If subnet and IP are not provided, pick the first subnet with + # enough available IPs, preferring ipv4 + if not network.subnets: + raise exceptions.ValidationException(detail=_( + "Supplied network does not contain a subnet." + )) + ip_avail = network_driver.get_network_ip_availability( + network) + if (CONF.controller_worker.loadbalancer_topology == + constants.TOPOLOGY_SINGLE): + num_req_ips = 2 + if (CONF.controller_worker.loadbalancer_topology == + constants.TOPOLOGY_ACTIVE_STANDBY): + num_req_ips = 3 + subnets = [subnet_id for subnet_id in network.subnets if + utils.subnet_ip_availability(ip_avail, subnet_id, + num_req_ips)] + if not subnets: + raise exceptions.ValidationException(detail=_( + "Subnet(s) in the supplied network do not contain " + "enough available IPs." + )) + for subnet_id in subnets: # Use the first subnet, in case there are no ipv4 subnets if not load_balancer.vip_subnet_id: load_balancer.vip_subnet_id = subnet_id @@ -137,10 +157,6 @@ class LoadBalancersController(base.BaseController): if subnet.ip_version == 4: load_balancer.vip_subnet_id = subnet_id break - if not load_balancer.vip_subnet_id: - raise exceptions.ValidationException(detail=_( - "Supplied network does not contain a subnet." - )) @staticmethod def _validate_port_and_fill_or_validate_subnet(load_balancer): diff --git a/octavia/common/utils.py b/octavia/common/utils.py index c3b8a85cd2..eaf443c128 100644 --- a/octavia/common/utils.py +++ b/octavia/common/utils.py @@ -105,6 +105,13 @@ def get_six_compatible_server_certs_key_passphrase(): return base64.urlsafe_b64encode(key) +def subnet_ip_availability(nw_ip_avail, subnet_id, req_num_ips): + for subnet in nw_ip_avail.subnet_ip_availability: + if subnet['subnet_id'] == subnet_id: + return subnet['total_ips'] - subnet['used_ips'] >= req_num_ips + return None + + class exception_logger(object): """Wrap a function and log raised exception diff --git a/octavia/network/base.py b/octavia/network/base.py index ccc0a86951..785a669cf1 100644 --- a/octavia/network/base.py +++ b/octavia/network/base.py @@ -329,3 +329,12 @@ class AbstractNetworkDriver(object): :return: Boolean """ + + @abc.abstractmethod + def get_network_ip_availability(self, network): + """Retrieves network IP availability. + + :param network: octavia.network.data_models.Network + :return: octavia.network.data_models.Network_IP_Availability + :raises: NetworkException, NetworkNotFound + """ diff --git a/octavia/network/data_models.py b/octavia/network/data_models.py index 3fb1489330..e1fe97bb2e 100644 --- a/octavia/network/data_models.py +++ b/octavia/network/data_models.py @@ -148,3 +148,17 @@ class HostRoute(data_models.BaseDataModel): class QosPolicy(data_models.BaseDataModel): def __init__(self, id): self.id = id + + +class Network_IP_Availability(data_models.BaseDataModel): + + def __init__(self, network_id=None, tenant_id=None, project_id=None, + network_name=None, total_ips=None, used_ips=None, + subnet_ip_availability=None): + self.network_id = network_id + self.tenant_id = tenant_id + self.project_id = project_id + self.network_name = network_name + self.total_ips = total_ips + self.used_ips = used_ips + self.subnet_ip_availability = subnet_ip_availability diff --git a/octavia/network/drivers/neutron/base.py b/octavia/network/drivers/neutron/base.py index f22a6cd8c4..dea2ac3689 100644 --- a/octavia/network/drivers/neutron/base.py +++ b/octavia/network/drivers/neutron/base.py @@ -253,3 +253,6 @@ class BaseNeutronDriver(base.AbstractNetworkDriver): def qos_enabled(self): return self._qos_enabled + + def get_network_ip_availability(self, network): + return self._get_resource('network_ip_availability', network.id) diff --git a/octavia/network/drivers/neutron/utils.py b/octavia/network/drivers/neutron/utils.py index fc5149ce07..fb5e67556c 100644 --- a/octavia/network/drivers/neutron/utils.py +++ b/octavia/network/drivers/neutron/utils.py @@ -94,3 +94,12 @@ def convert_floatingip_dict_to_model(floating_ip_dict): fixed_ip_address=floating_ip.get('fixed_ip_address'), fixed_port_id=floating_ip.get('fixed_port_id') ) + + +def convert_network_ip_availability_dict_to_model( + network_ip_availability_dict): + nw_ip_avail = network_ip_availability_dict.get( + 'network_ip_availability', network_ip_availability_dict) + ip_avail = network_models.Network_IP_Availability.from_dict(nw_ip_avail) + ip_avail.subnet_ip_availability = nw_ip_avail.get('subnet_ip_availability') + return ip_avail diff --git a/octavia/network/drivers/noop_driver/driver.py b/octavia/network/drivers/noop_driver/driver.py index 7ac8c41401..535c050bc3 100644 --- a/octavia/network/drivers/noop_driver/driver.py +++ b/octavia/network/drivers/noop_driver/driver.py @@ -224,6 +224,21 @@ class NoopManager(object): def qos_enabled(self): return self._qos_extension_enabled + def get_network_ip_availability(self, network): + LOG.debug("Network %s no-op, network_ip_availability network_id %s", + self.__class__.__name__, network.id) + self.networkconfigconfig[(network.id, 'ip_availability')] = ( + network.id, 'get_network_ip_availability') + ip_avail = network_models.Network_IP_Availability( + network_id=network.id) + subnet_ip_availability = [] + network.subnets = list(network.subnets) + for subnet_id in network.subnets: + subnet_ip_availability.append({'subnet_id': subnet_id, + 'used_ips': 0, 'total_ips': 254}) + ip_avail.subnet_ip_availability = subnet_ip_availability + return ip_avail + class NoopNetworkDriver(driver_base.AbstractNetworkDriver): def __init__(self): @@ -296,3 +311,6 @@ class NoopNetworkDriver(driver_base.AbstractNetworkDriver): def qos_enabled(self): return self.driver.qos_enabled() + + def get_network_ip_availability(self, network): + return self.driver.get_network_ip_availability(network) diff --git a/octavia/tests/common/constants.py b/octavia/tests/common/constants.py index dcba133500..73a38f142e 100644 --- a/octavia/tests/common/constants.py +++ b/octavia/tests/common/constants.py @@ -197,3 +197,19 @@ MOCK_VRRP_PORT2 = {'port': {'network_id': MOCK_VIP_NET_ID, 'device_owner': MOCK_DEVICE_OWNER, 'id': MOCK_VRRP_PORT_ID2, 'fixed_ips': MOCK_VRRP_FIXED_IPS2}} + +MOCK_NETWORK_TOTAL_IPS = 254 +MOCK_NETWORK_USED_IPS = 0 +MOCK_SUBNET_TOTAL_IPS = 254 +MOCK_SUBNET_USED_IPS = 0 +MOCK_SUBNET_IP_AVAILABILITY = [{'used_ips': MOCK_SUBNET_USED_IPS, + 'subnet_id': MOCK_SUBNET_ID, + 'total_ips': MOCK_SUBNET_TOTAL_IPS}] + +MOCK_NETWORK_IP_AVAILABILITY = {'network_ip_availability': ( + {'network_id': MOCK_NETWORK_ID, + 'tenant_id': MOCK_PROJECT_ID, + 'network_name': MOCK_NETWORK_NAME, + 'total_ips': MOCK_NETWORK_TOTAL_IPS, + 'used_ips': MOCK_NETWORK_USED_IPS, + 'subnet_ip_availability': MOCK_SUBNET_IP_AVAILABILITY})} diff --git a/octavia/tests/functional/api/v2/test_load_balancer.py b/octavia/tests/functional/api/v2/test_load_balancer.py index 051e65f559..6de3cb6da7 100644 --- a/octavia/tests/functional/api/v2/test_load_balancer.py +++ b/octavia/tests/functional/api/v2/test_load_balancer.py @@ -197,6 +197,89 @@ class TestLoadBalancer(base.BaseAPITest): self.assertEqual(subnet.id, api_lb.get('vip_subnet_id')) self.assertEqual(network_id, api_lb.get('vip_network_id')) + def test_create_with_vip_network_picks_subnet_ipv4_avail_ips(self): + self.conf.config( + group='controller_worker', + loadbalancer_topology=constants.TOPOLOGY_ACTIVE_STANDBY) + network_id = uuidutils.generate_uuid() + subnet1 = network_models.Subnet(id=uuidutils.generate_uuid(), + network_id=network_id, + ip_version=4) + subnet2 = network_models.Subnet(id=uuidutils.generate_uuid(), + network_id=network_id, + ip_version=4) + subnet3 = network_models.Subnet(id=uuidutils.generate_uuid(), + network_id=network_id, + ip_version=4) + network = network_models.Network(id=network_id, + subnets=[subnet1.id, subnet2.id, + subnet3.id]) + subnet_ip_availability = [{'subnet_id': subnet1.id, 'used_ips': 254, + 'total_ips': 254}, {'subnet_id': subnet2.id, + 'used_ips': 128, 'total_ips': 254}, + {'subnet_id': subnet3.id, 'used_ips': 254, + 'total_ips': 254}] + ip_avail = network_models.Network_IP_Availability( + network_id=network.id, + subnet_ip_availability=subnet_ip_availability) + lb_json = {'vip_network_id': network.id, + 'project_id': self.project_id} + body = self._build_body(lb_json) + with mock.patch( + "octavia.network.drivers.noop_driver.driver.NoopManager" + ".get_network") as mock_get_network, mock.patch( + "octavia.network.drivers.noop_driver.driver.NoopManager" + ".get_subnet") as mock_get_subnet, mock.patch( + "octavia.network.drivers.noop_driver.driver.NoopManager" + ".get_network_ip_availability") as ( + mock_get_network_ip_availability): + mock_get_network.return_value = network + mock_get_subnet.side_effect = [subnet1, subnet2, subnet3] + mock_get_network_ip_availability.return_value = ip_avail + response = self.post(self.LBS_PATH, body) + api_lb = response.json.get(self.root_tag) + self._assert_request_matches_response(lb_json, api_lb) + self.assertEqual(subnet2.id, api_lb.get('vip_subnet_id')) + self.assertEqual(network_id, api_lb.get('vip_network_id')) + + def test_create_with_vip_network_not_enough_avail_ips(self): + self.conf.config( + group='controller_worker', + loadbalancer_topology=constants.TOPOLOGY_ACTIVE_STANDBY) + network_id = uuidutils.generate_uuid() + subnet1 = network_models.Subnet(id=uuidutils.generate_uuid(), + network_id=network_id, + ip_version=4) + subnet2 = network_models.Subnet(id=uuidutils.generate_uuid(), + network_id=network_id, + ip_version=4) + network = network_models.Network(id=network_id, + subnets=[subnet1.id, subnet2.id]) + subnet_ip_availability = [{'subnet_id': subnet1.id, 'used_ips': 254, + 'total_ips': 254}, {'subnet_id': subnet2.id, + 'used_ips': 254, 'total_ips': 254}] + ip_avail = network_models.Network_IP_Availability( + network_id=network.id, + subnet_ip_availability=subnet_ip_availability) + lb_json = {'vip_network_id': network.id, + 'project_id': self.project_id} + body = self._build_body(lb_json) + with mock.patch( + "octavia.network.drivers.noop_driver.driver.NoopManager" + ".get_network") as mock_get_network, mock.patch( + "octavia.network.drivers.noop_driver.driver.NoopManager" + ".get_subnet") as mock_get_subnet, mock.patch( + "octavia.network.drivers.noop_driver.driver.NoopManager" + ".get_network_ip_availability") as ( + mock_get_network_ip_availability): + mock_get_network.return_value = network + mock_get_subnet.side_effect = [subnet1, subnet2] + mock_get_network_ip_availability.return_value = ip_avail + response = self.post(self.LBS_PATH, body, status=400) + err_msg = ('Validation failure: Subnet(s) in the supplied network do ' + 'not contain enough available IPs.') + self.assertEqual(err_msg, response.json.get('faultstring')) + def test_create_with_vip_network_and_address(self): ip_address = '198.51.100.10' network_id = uuidutils.generate_uuid() diff --git a/octavia/tests/unit/network/drivers/neutron/test_base.py b/octavia/tests/unit/network/drivers/neutron/test_base.py index 4f27085f10..4387bfd9b3 100644 --- a/octavia/tests/unit/network/drivers/neutron/test_base.py +++ b/octavia/tests/unit/network/drivers/neutron/test_base.py @@ -479,3 +479,18 @@ class TestBaseNeutronNetworkDriver(base.TestCase): self.driver.apply_qos_on_port, t_constants.MOCK_PORT_ID, t_constants.MOCK_NEUTRON_QOS_POLICY_ID) + + def test_get_network_ip_availability(self): + show_network_ip_availability = ( + self.driver.neutron_client.show_network_ip_availability) + show_network_ip_availability.return_value = ( + {'network_ip_availability': { + 'network_id': t_constants.MOCK_NETWORK_ID, + 'subnet_ip_availability': t_constants.MOCK_SUBNET_IP_AVAILABILITY + }}) + ip_avail = self.driver.get_network_ip_availability( + network_models.Network(t_constants.MOCK_NETWORK_ID)) + self.assertIsInstance(ip_avail, network_models.Network_IP_Availability) + self.assertEqual(t_constants.MOCK_NETWORK_ID, ip_avail.network_id) + self.assertEqual(t_constants.MOCK_SUBNET_IP_AVAILABILITY, + ip_avail.subnet_ip_availability) diff --git a/octavia/tests/unit/network/drivers/neutron/test_utils.py b/octavia/tests/unit/network/drivers/neutron/test_utils.py index 24a78c5c1a..df01978350 100644 --- a/octavia/tests/unit/network/drivers/neutron/test_utils.py +++ b/octavia/tests/unit/network/drivers/neutron/test_utils.py @@ -115,3 +115,19 @@ class TestNeutronUtils(base.TestCase): fixed_port_id=t_constants.MOCK_PORT_ID2 ) self._compare_ignore_value_none(model_obj.to_dict(), assert_dict) + + def test_convert_network_ip_availability_dict_to_model(self): + model_obj = utils.convert_network_ip_availability_dict_to_model( + t_constants.MOCK_NETWORK_IP_AVAILABILITY) + assert_dict = dict( + network_id=t_constants.MOCK_NETWORK_ID, + tenant_id=t_constants.MOCK_PROJECT_ID, + network_name=t_constants.MOCK_NETWORK_NAME, + total_ips=t_constants.MOCK_NETWORK_TOTAL_IPS, + used_ips=t_constants.MOCK_NETWORK_USED_IPS, + subnet_ip_availability=t_constants.MOCK_SUBNET_IP_AVAILABILITY + ) + model_dict = model_obj.to_dict(recurse=True) + model_dict['subnet_ip_availability'] = ( + model_obj.subnet_ip_availability) + self._compare_ignore_value_none(model_dict, assert_dict) diff --git a/releasenotes/notes/fix-vip-network-ip-availability-2e924f32abf01052.yaml b/releasenotes/notes/fix-vip-network-ip-availability-2e924f32abf01052.yaml new file mode 100644 index 0000000000..90e2f14a67 --- /dev/null +++ b/releasenotes/notes/fix-vip-network-ip-availability-2e924f32abf01052.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes an issue in the selection of vip-subnet-id on multi-subnet networks + by checking the IP availability of the subnets, ensuring enough IPs are + available for loadbalancer when creating loadbalancer specifying + vip-network-id.