Fix LB creation with VIP port

Octavia v2 API was failing to create the load balancer when the user
specified a VIP port ID.
This also improves the user experience when specifying a VIP address.
It also removes the un-used nova_network directory.

Change-Id: I8b533094df1e5425f824fff0454335709ce05447
Closes-Bug: #1709922

api-ref/source/v2/loadbalancer.inc

@@ -105,13 +105,16 @@ There are three ways to specify a Virtual IP (VIP) network for the load
 balancer: provide a ``vip_port_id``, supply a ``vip_subnet_id``, or provide a
 ``vip_network_id``. Providing a neutron port ID for the ``vip_port_id`` tells
 octavia to use this port for the VIP. Some port settings may be changed or
-removed as required by octavia, but the IP address will be retained.
+removed as required by octavia, but the IP address will be retained. If the
+port has more than one subnet you must specify either the ``vip_subnet_id`` or
+``vip_address`` to clarify which address should be used for the VIP.
 Specifying a neutron subnet ID will tell octavia to create a neutron port
 on this subnet and allocate an IP address from the subnet if the
 ``vip_address`` was not specified. If ``vip_address`` was specified, octavia
 will attempt to allocate the ``vip_address`` from the subnet for the VIP
 address. Finally, when a ``vip_network_ip`` is specified octavia will select
-a subnet from the network, preferring IPv4 over IPv6 subnets.
+a subnet from the network, preferring IPv4 over IPv6 subnets, if a
+``vip_address`` was not provided.
 
 An optional ``flavor`` attribute can be used to create the load balancer
 using a pre-configured octavia flavor.  Flavors are created by the operator

octavia/api/v2/controllers/load_balancer.py

@@ -97,21 +97,60 @@ class LoadBalancersController(base.BaseController):
         network = validate.network_exists_optionally_contains_subnet(
             network_id=load_balancer.vip_network_id,
             subnet_id=load_balancer.vip_subnet_id)
-        # If subnet is not provided, pick the first subnet, preferring ipv4
         if not load_balancer.vip_subnet_id:
             network_driver = utils.get_network_driver()
-            for subnet_id in network.subnets:
+            if load_balancer.vip_address:
-                # Use the first subnet, in case there are no ipv4 subnets
+                for subnet_id in network.subnets:
+                    subnet = network_driver.get_subnet(subnet_id)
+                    if validate.is_ip_member_of_cidr(load_balancer.vip_address,
+                                                     subnet.cidr):
+                        load_balancer.vip_subnet_id = subnet_id
+                        break
                 if not load_balancer.vip_subnet_id:
-                    load_balancer.vip_subnet_id = subnet_id
+                    raise exceptions.ValidationException(detail=_(
-                subnet = network_driver.get_subnet(subnet_id)
+                        "Supplied network does not contain a subnet for "
-                if subnet.ip_version == 4:
+                        "VIP address specified."
-                    load_balancer.vip_subnet_id = subnet_id
+                    ))
-                    break
+            else:
-            if not load_balancer.vip_subnet_id:
+                # If subnet and IP are not provided, pick the first subnet,
+                # preferring ipv4
+                for subnet_id in network.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
+                    subnet = network_driver.get_subnet(subnet_id)
+                    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):
+        port = validate.port_exists(port_id=load_balancer.vip_port_id)
+        load_balancer.vip_network_id = port.network_id
+
+        # Identify the subnet for this port
+        if load_balancer.vip_subnet_id:
+            validate.subnet_exists(subnet_id=load_balancer.vip_subnet_id)
+        else:
+            if load_balancer.vip_address:
+                for port_fixed_ip in port.fixed_ips:
+                    if port_fixed_ip.ip_address == load_balancer.vip_address:
+                        load_balancer.vip_subnet_id = port_fixed_ip.subnet_id
+                        break
+                if not load_balancer.vip_subnet_id:
+                    raise exceptions.ValidationException(detail=_(
+                        "Specified VIP address not found on the "
+                        "specified VIP port."))
+            elif len(port.fixed_ips) == 1:
+                load_balancer.vip_subnet_id = port.fixed_ips[0].subnet_id
+            else:
                 raise exceptions.ValidationException(detail=_(
-                    "Supplied network does not contain a subnet."
+                    "VIP port's subnet could not be determined. Please "
-                ))
+                    "specify either a VIP subnet or address."))
 
     def _validate_vip_request_object(self, load_balancer):
         allowed_network_objects = []
@@ -145,8 +184,7 @@ class LoadBalancersController(base.BaseController):
 
         # Validate the port id
         if load_balancer.vip_port_id:
-            port = validate.port_exists(port_id=load_balancer.vip_port_id)
+            self._validate_port_and_fill_or_validate_subnet(load_balancer)
-            load_balancer.vip_network_id = port.network_id
         # If no port id, validate the network id (and subnet if provided)
         elif load_balancer.vip_network_id:
             self._validate_network_and_fill_or_validate_subnet(load_balancer)

octavia/common/validate.py

@@ -21,6 +21,7 @@ Defined here so these can also be used at deeper levels than the API.
 
 import re
 
+import netaddr
 from oslo_config import cfg
 import rfc3986
 
@@ -267,3 +268,9 @@ def network_allowed_by_config(network_id):
             raise exceptions.ValidationException(detail=_(
                 'Supplied VIP network_id is not allowed by the configuration '
                 'of this deployment.'))
+
+
+def is_ip_member_of_cidr(address, cidr):
+    if netaddr.IPAddress(address) in netaddr.IPNetwork(cidr):
+        return True
+    return False

octavia/tests/functional/api/v2/test_load_balancer.py

@@ -197,6 +197,215 @@ 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_and_address(self):
+        ip_address = '198.51.100.10'
+        network_id = uuidutils.generate_uuid()
+        subnet1 = network_models.Subnet(id=uuidutils.generate_uuid(),
+                                        network_id=network_id,
+                                        cidr='2001:DB8::/32',
+                                        ip_version=6)
+        subnet2 = network_models.Subnet(id=uuidutils.generate_uuid(),
+                                        network_id=network_id,
+                                        cidr='198.51.100.0/24',
+                                        ip_version=4)
+        network = network_models.Network(id=network_id,
+                                         subnets=[subnet1.id, subnet2.id])
+        lb_json = {'vip_network_id': network.id,
+                   'vip_address': ip_address,
+                   '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_get_network.return_value = network
+            mock_get_subnet.side_effect = [subnet1, subnet2]
+            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'))
+        self.assertEqual(ip_address, api_lb.get('vip_address'))
+
+    def test_create_with_vip_network_and_address_no_subnet_match(self):
+        ip_address = '198.51.100.10'
+        network_id = uuidutils.generate_uuid()
+        subnet1 = network_models.Subnet(id=uuidutils.generate_uuid(),
+                                        network_id=network_id,
+                                        cidr='2001:DB8::/32',
+                                        ip_version=6)
+        subnet2 = network_models.Subnet(id=uuidutils.generate_uuid(),
+                                        network_id=network_id,
+                                        cidr='203.0.113.0/24',
+                                        ip_version=4)
+        network = network_models.Network(id=network_id,
+                                         subnets=[subnet1.id, subnet2.id])
+        lb_json = {'vip_network_id': network.id,
+                   'vip_address': ip_address,
+                   '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_get_network.return_value = network
+            mock_get_subnet.side_effect = [subnet1, subnet2]
+            response = self.post(self.LBS_PATH, body, status=400)
+        err_msg = ('Validation failure: Supplied network does not contain a '
+                   'subnet for VIP address specified.')
+        self.assertEqual(err_msg, response.json.get('faultstring'))
+
+    def test_create_with_vip_network_and_address_ipv6(self):
+        ip_address = '2001:DB8::10'
+        network_id = uuidutils.generate_uuid()
+        subnet1 = network_models.Subnet(id=uuidutils.generate_uuid(),
+                                        network_id=network_id,
+                                        cidr='2001:DB8::/32',
+                                        ip_version=6)
+        subnet2 = network_models.Subnet(id=uuidutils.generate_uuid(),
+                                        network_id=network_id,
+                                        cidr='198.51.100.0/24',
+                                        ip_version=4)
+        network = network_models.Network(id=network_id,
+                                         subnets=[subnet1.id, subnet2.id])
+        lb_json = {'vip_network_id': network.id,
+                   'vip_address': ip_address,
+                   '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_get_network.return_value = network
+            mock_get_subnet.side_effect = [subnet1, subnet2]
+            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(subnet1.id, api_lb.get('vip_subnet_id'))
+        self.assertEqual(network.id, api_lb.get('vip_network_id'))
+        self.assertEqual(ip_address, api_lb.get('vip_address'))
+
+    def test_create_with_vip_port_1_fixed_ip(self):
+        ip_address = '198.51.100.1'
+        subnet = network_models.Subnet(id=uuidutils.generate_uuid())
+        network = network_models.Network(id=uuidutils.generate_uuid(),
+                                         subnets=[subnet])
+        fixed_ip = network_models.FixedIP(subnet_id=subnet.id,
+                                          ip_address=ip_address)
+        port = network_models.Port(id=uuidutils.generate_uuid(),
+                                   fixed_ips=[fixed_ip],
+                                   network_id=network.id)
+        lb_json = {
+            'name': 'test1', 'description': 'test1_desc',
+            'vip_port_id': port.id, 'admin_state_up': False,
+            '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_port") as mock_get_port:
+            mock_get_network.return_value = network
+            mock_get_port.return_value = port
+            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(ip_address, api_lb.get('vip_address'))
+        self.assertEqual(subnet.id, api_lb.get('vip_subnet_id'))
+        self.assertEqual(network.id, api_lb.get('vip_network_id'))
+        self.assertEqual(port.id, api_lb.get('vip_port_id'))
+
+    def test_create_with_vip_port_2_fixed_ip(self):
+        ip_address = '198.51.100.1'
+        subnet = network_models.Subnet(id=uuidutils.generate_uuid())
+        network = network_models.Network(id=uuidutils.generate_uuid(),
+                                         subnets=[subnet])
+        fixed_ip = network_models.FixedIP(subnet_id=subnet.id,
+                                          ip_address=ip_address)
+        fixed_ip_2 = network_models.FixedIP(
+            subnet_id=uuidutils.generate_uuid(), ip_address='203.0.113.5')
+        port = network_models.Port(id=uuidutils.generate_uuid(),
+                                   fixed_ips=[fixed_ip, fixed_ip_2],
+                                   network_id=network.id)
+        lb_json = {
+            'name': 'test1', 'description': 'test1_desc',
+            'vip_port_id': port.id, 'admin_state_up': False,
+            '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_port") as mock_get_port:
+            mock_get_network.return_value = network
+            mock_get_port.return_value = port
+            response = self.post(self.LBS_PATH, body, status=400)
+            err_msg = ("Validation failure: "
+                       "VIP port's subnet could not be determined. Please "
+                       "specify either a VIP subnet or address.")
+            self.assertEqual(err_msg, response.json.get('faultstring'))
+
+    def test_create_with_vip_port_and_address(self):
+        ip_address = '198.51.100.1'
+        subnet = network_models.Subnet(id=uuidutils.generate_uuid())
+        network = network_models.Network(id=uuidutils.generate_uuid(),
+                                         subnets=[subnet])
+        fixed_ip = network_models.FixedIP(subnet_id=subnet.id,
+                                          ip_address=ip_address)
+        port = network_models.Port(id=uuidutils.generate_uuid(),
+                                   fixed_ips=[fixed_ip],
+                                   network_id=network.id)
+        lb_json = {
+            'name': 'test1', 'description': 'test1_desc',
+            'vip_port_id': port.id, 'vip_address': ip_address,
+            'admin_state_up': False, '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_port") as mock_get_port:
+            mock_get_network.return_value = network
+            mock_get_port.return_value = port
+            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(ip_address, api_lb.get('vip_address'))
+        self.assertEqual(subnet.id, api_lb.get('vip_subnet_id'))
+        self.assertEqual(network.id, api_lb.get('vip_network_id'))
+        self.assertEqual(port.id, api_lb.get('vip_port_id'))
+
+    def test_create_with_vip_port_and_bad_address(self):
+        ip_address = '198.51.100.1'
+        subnet = network_models.Subnet(id=uuidutils.generate_uuid())
+        network = network_models.Network(id=uuidutils.generate_uuid(),
+                                         subnets=[subnet])
+        fixed_ip = network_models.FixedIP(subnet_id=subnet.id,
+                                          ip_address=ip_address)
+        port = network_models.Port(id=uuidutils.generate_uuid(),
+                                   fixed_ips=[fixed_ip],
+                                   network_id=network.id)
+        lb_json = {
+            'name': 'test1', 'description': 'test1_desc',
+            'vip_port_id': port.id, 'vip_address': '203.0.113.7',
+            'admin_state_up': False, '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_port") as mock_get_port:
+            mock_get_network.return_value = network
+            mock_get_port.return_value = port
+            response = self.post(self.LBS_PATH, body, status=400)
+        err_msg = ("Validation failure: "
+                   "Specified VIP address not found on the specified VIP "
+                   "port.")
+        self.assertEqual(err_msg, response.json.get('faultstring'))
+
     def test_create_with_vip_full(self):
         subnet = network_models.Subnet(id=uuidutils.generate_uuid())
         network = network_models.Network(id=uuidutils.generate_uuid(),