From cf901539677d24cac0d9541752b4b3c74c2a1635 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 | 14 ++++ ...work-ip-availability-2e924f32abf01052.yaml | 7 ++ 12 files changed, 218 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 6c9a4a93f9..210240aefe 100644 --- a/octavia/api/v2/controllers/load_balancer.py +++ b/octavia/api/v2/controllers/load_balancer.py @@ -136,9 +136,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 @@ -146,10 +166,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 0ce83e7c04..2c2fbbf4e0 100644 --- a/octavia/common/utils.py +++ b/octavia/common/utils.py @@ -110,6 +110,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 e7c8bf1cc8..2f1a3a39d9 100644 --- a/octavia/network/base.py +++ b/octavia/network/base.py @@ -336,3 +336,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 751ecc0de6..8b5b7e984e 100644 --- a/octavia/network/data_models.py +++ b/octavia/network/data_models.py @@ -149,3 +149,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 0f359cf909..6dfe0dc50b 100644 --- a/octavia/network/drivers/neutron/base.py +++ b/octavia/network/drivers/neutron/base.py @@ -256,3 +256,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 3352e2e115..3d686aa03c 100644 --- a/octavia/network/drivers/noop_driver/driver.py +++ b/octavia/network/drivers/noop_driver/driver.py @@ -264,6 +264,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): @@ -345,3 +360,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 d12c12e952..aec6986e75 100644 --- a/octavia/tests/common/constants.py +++ b/octavia/tests/common/constants.py @@ -198,3 +198,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 042335ac58..f6c272b48e 100644 --- a/octavia/tests/functional/api/v2/test_load_balancer.py +++ b/octavia/tests/functional/api/v2/test_load_balancer.py @@ -225,6 +225,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 2f3d322586..5f59c715f0 100644 --- a/octavia/tests/unit/network/drivers/neutron/test_base.py +++ b/octavia/tests/unit/network/drivers/neutron/test_base.py @@ -481,3 +481,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 f5cf941fa8..3d344ff2ec 100644 --- a/octavia/tests/unit/network/drivers/neutron/test_utils.py +++ b/octavia/tests/unit/network/drivers/neutron/test_utils.py @@ -117,3 +117,17 @@ 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 + ) + self._compare_ignore_value_none(model_obj.to_dict(recurse=True), + 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.