Allow LB members to mix IPv4 and IPv6 for the multivip LB

When creating a load balancer with both IPv4 and IPv6 protocols
for the LB VIP and additional_vips field, it is essential to
allow the mixing of IPv4 and IPv6 backend members.

This patch enables this use case and ensures that the 'vips'
field in the OVN NB DB associates IPv4-type LB VIPs with IPv4
members and IPv6-type LB VIPs with IPv6 members exclusively.

Closes-Bug: 2047055
Change-Id: I173a6456e8a5f776cac207390e670afa34f83d7c

Author: fernandoroyosanchez

ovn_octavia_provider/driver.py

@@ -265,9 +265,22 @@ 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)
+        lb_vips = [ovn_lb.external_ids.get(
+            ovn_const.LB_EXT_IDS_VIP_KEY)]
+        if ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY in ovn_lb.external_ids:
+            lb_vips.extend(ovn_lb.external_ids.get(
+                ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY).split(','))
+
+        # NOTE(froyo): Allow mixing member IP version when VIP LB and any
+        # additional vip is also mixing version
+        vip_version = netaddr.IPNetwork(lb_vips[0]).version
+        vips_mixed = any(netaddr.IPNetwork(vip).version != vip_version
+                         for vip in lb_vips if vip)
+
+        if vips_mixed:
+            return False
+        else:
+            return vip_version != (netaddr.IPNetwork(member.address).version)
 
     def member_create(self, member):
         # Validate monitoring options if present

ovn_octavia_provider/helper.py

@@ -968,30 +968,36 @@ def _frame_vip_ips(self, ovn_lb, lb_external_ids):
             if pool_id not in lb_external_ids or not lb_external_ids[pool_id]:
                 continue
 
-            ips = []
+            ips_v4 = []
+            ips_v6 = []
             for mb_ip, mb_port, mb_subnet, mb_id in self._extract_member_info(
                     lb_external_ids[pool_id]):
                 if not self._is_member_offline(ovn_lb, mb_id):
-                    if netaddr.IPNetwork(mb_ip).version == 6:
-                        ips.append(f'[{mb_ip}]:{mb_port}')
+                    if netaddr.IPNetwork(
+                            mb_ip).version == n_const.IP_VERSION_6:
+                        ips_v6.append(f'[{mb_ip}]:{mb_port}')
                     else:
-                        ips.append(f'{mb_ip}:{mb_port}')
-            if ips:
-                for lb_vip in lb_vips:
-                    if netaddr.IPNetwork(lb_vip).version == 6:
-                        lb_vip = f'[{lb_vip}]'
-                    vip_ips[lb_vip + ':' + vip_port] = ','.join(ips)
-
-                if vip_fip:
-                    if netaddr.IPNetwork(vip_fip).version == 6:
-                        vip_fip = f'[{vip_fip}]'
-                    vip_ips[vip_fip + ':' + vip_port] = ','.join(ips)
-
-                if additional_vip_fips:
-                    for addi_vip_fip in additional_vip_fips.split(','):
-                        if netaddr.IPNetwork(addi_vip_fip).version == 6:
-                            addi_vip_fip = f'[{addi_vip_fip}]'
-                        vip_ips[addi_vip_fip + ':' + vip_port] = ','.join(ips)
+                        ips_v4.append(f'{mb_ip}:{mb_port}')
+
+            for lb_vip in lb_vips:
+                if ips_v4 and netaddr.IPNetwork(
+                        lb_vip).version == n_const.IP_VERSION_4:
+                    vip_ips[lb_vip + ':' + vip_port] = ','.join(ips_v4)
+                if ips_v6 and netaddr.IPNetwork(
+                        lb_vip).version == n_const.IP_VERSION_6:
+                    lb_vip = f'[{lb_vip}]'
+                    vip_ips[lb_vip + ':' + vip_port] = ','.join(ips_v6)
+
+            if ips_v4 and vip_fip:
+                if netaddr.IPNetwork(vip_fip).version == n_const.IP_VERSION_4:
+                    vip_ips[vip_fip + ':' + vip_port] = ','.join(ips_v4)
+
+            if ips_v4 and additional_vip_fips:
+                for addi_vip_fip in additional_vip_fips.split(','):
+                    if netaddr.IPNetwork(
+                            addi_vip_fip).version == n_const.IP_VERSION_4:
+                        vip_ips[addi_vip_fip + ':' + vip_port] = ','.join(
+                            ips_v4)
         return vip_ips
 
     def _refresh_lb_vips(self, ovn_lb, lb_external_ids):
@@ -2646,7 +2652,7 @@ def _add_lbhc(self, ovn_lb, pool_key, info):
                     # then this could just be self.ovn_nbdb_api.lb_hm_add()
                     external_ids_vip = copy.deepcopy(external_ids)
                     external_ids_vip[ovn_const.LB_EXT_IDS_HM_VIP] = vip
-                    if netaddr.IPNetwork(vip).version == 6:
+                    if netaddr.IPNetwork(vip).version == n_const.IP_VERSION_6:
                         vip = f'[{vip}]'
                     kwargs = {
                         'vip': vip + ':' + str(vip_port) if vip_port else '',
@@ -2673,7 +2679,8 @@ def _add_lbhc(self, ovn_lb, pool_key, info):
                     external_ids_fip = copy.deepcopy(external_ids)
                     for fip in fips:
                         external_ids_fip[ovn_const.LB_EXT_IDS_HM_VIP] = fip
-                        if netaddr.IPNetwork(fip).version == 6:
+                        if netaddr.IPNetwork(
+                                fip).version == n_const.IP_VERSION_6:
                             fip = f'[{fip}]'
                         fip_kwargs = {
                             'vip': fip + ':' + str(vip_port)
@@ -2705,7 +2712,7 @@ def _update_lbhc_vip_port(self, lbhc, vip_port):
             # will be empty, so get it from lbhc external_ids
             vip = lbhc.external_ids.get(ovn_const.LB_EXT_IDS_HM_VIP, '')
             if vip:
-                if netaddr.IPNetwork(vip).version == 6:
+                if netaddr.IPNetwork(vip).version == n_const.IP_VERSION_6:
                     vip = f'[{vip}]'
                 vip = vip + ':' + str(vip_port)
         commands = []
@@ -3100,11 +3107,11 @@ def _get_lbs_on_hm_event(self, row):
         hm_source_ip = str(row.src_ip)
         member_ip = str(row.ip)
         member_src = f'{row.logical_port}:'
-        if netaddr.IPNetwork(hm_source_ip).version == 6:
+        if netaddr.IPNetwork(hm_source_ip).version == n_const.IP_VERSION_6:
             member_src += f'[{hm_source_ip}]'
         else:
             member_src += f'{hm_source_ip}'
-        if netaddr.IPNetwork(member_ip).version == 6:
+        if netaddr.IPNetwork(member_ip).version == n_const.IP_VERSION_6:
             member_ip = f'[{member_ip}]'
         mappings[member_ip] = member_src
         lbs = self.ovn_nbdb_api.db_find_rows(

ovn_octavia_provider/tests/functional/base.py

@@ -17,6 +17,7 @@
 from unittest import mock
 
 from neutron.common import utils as n_utils
+from neutron_lib import constants as n_const
 from neutron_lib.plugins import directory
 from octavia_lib.api.drivers import data_models as octavia_data_model
 from octavia_lib.api.drivers import driver_lib
@@ -283,9 +284,10 @@ def _create_net(self, name):
         n1 = self._make_network(self.fmt, name, True)
         return n1
 
-    def _create_subnet_from_net(self, net, cidr, router_id=None):
+    def _create_subnet_from_net(self, net, cidr, router_id=None,
+                                ip_version=n_const.IP_VERSION_4):
         res = self._create_subnet(self.fmt, net['network']['id'],
-                                  cidr)
+                                  cidr, ip_version=ip_version)
         subnet = self.deserialize(self.fmt, res)['subnet']
         self._local_net_cache[subnet['id']] = net['network']['id']
         self._local_cidr_cache[subnet['id']] = subnet['cidr']

ovn_octavia_provider/tests/functional/test_driver.py

@@ -13,9 +13,9 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from neutron_lib import constants as n_const
 from octavia_lib.api.drivers import exceptions as o_exceptions
 from octavia_lib.common import constants as o_constants
-
 from oslo_utils import uuidutils
 
 from ovn_octavia_provider.tests.functional import base as ovn_base
@@ -29,7 +29,8 @@ def test_loadbalancer(self):
         sbnet_info = self._create_subnet_from_net(network_N1, '10.0.0.0/24',
                                                   router_id=r1_id)
         sbnet_additional_info = self._create_subnet_from_net(
-            network_N1, '10.0.1.0/24', router_id=r1_id)
+            network_N1, '2001:db8:0:1::/64', router_id=r1_id,
+            ip_version=n_const.IP_VERSION_6)
         additional_vips_list = [{
             'ip_address': sbnet_additional_info[2],
             'port_id': sbnet_additional_info[3],
@@ -266,7 +267,8 @@ def test_hm(self):
         sbnet_info = self._create_subnet_from_net(network_N1, '10.0.0.0/24',
                                                   router_id=r1_id)
         sbnet_additional_info = self._create_subnet_from_net(
-            network_N1, '10.1.1.0/24', router_id=r1_id)
+            network_N1, '2001:db8:0:1::/64', router_id=r1_id,
+            ip_version=n_const.IP_VERSION_6)
         additional_vips_list = [{
             'ip_address': sbnet_additional_info[2],
             'port_id': sbnet_additional_info[3],

ovn_octavia_provider/tests/unit/base.py

@@ -50,7 +50,7 @@ def setUp(self):
         self.vip_output = {'vip_network_id': self.vip_dict['vip_network_id'],
                            'vip_subnet_id': self.vip_dict['vip_subnet_id']}
         self.additional_vips = [{
-            'ip_address': '192.148.110.109',
+            'ip_address': '2001:db8:0:1::12',
             'network_id': self.vip_dict['vip_network_id'],
             'port_id': uuidutils.generate_uuid(),
             'subnet_id': uuidutils.generate_uuid()

ovn_octavia_provider/tests/unit/test_driver.py

@@ -37,12 +37,24 @@ def setUp(self):
             'member_%s_%s:%s_%s' %
             (self.member_id, self.member_address,
              self.member_port, self.member_subnet_id))
+        self.member_line_additional_vips = (
+            'member_%s_%s:%s_%s' %
+            (self.member_id, self.member_address,
+             self.member_port, self.member_subnet_id))
         self.ovn_lb = mock.MagicMock()
         self.ovn_lb.name = 'foo_ovn_lb'
         self.ovn_lb.external_ids = {
             ovn_const.LB_EXT_IDS_VIP_KEY: '10.22.33.4',
             'pool_%s' % self.pool_id: self.member_line,
             'listener_%s' % self.listener_id: '80:pool_%s' % self.pool_id}
+        self.ovn_lb_addi_vips = mock.MagicMock()
+        self.ovn_lb_addi_vips.name = 'foo_ovn_lb_addi_vips'
+        self.ovn_lb_addi_vips.external_ids = {
+            ovn_const.LB_EXT_IDS_VIP_KEY: '10.22.33.4',
+            ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY: '2001:db8:0:1::203',
+            'pool_%s' % self.pool_id: ','.join([
+                self.member_line, self.member_line_additional_vips]),
+            'listener_%s' % self.listener_id: '80:pool_%s' % self.pool_id}
         self.mock_add_request = add_req_thread.start()
         self.project_id = uuidutils.generate_uuid()
 
@@ -251,6 +263,15 @@ def test__ip_version_differs(self):
         self.ref_member.address = 'fc00::1'
         self.assertTrue(self.driver._ip_version_differs(self.ref_member))
 
+    def test__ip_version_differs_lb_additional_vips(self):
+        self.mock_find_ovn_lb_by_pool_id = mock.patch.object(
+            ovn_helper.OvnProviderHelper,
+            '_find_ovn_lb_by_pool_id').start()
+        self.mock_find_ovn_lb_by_pool_id.return_value = (_,
+                                                         self.ovn_lb_addi_vips)
+        self.ref_member.address = 'fc00::1'
+        self.assertFalse(self.driver._ip_version_differs(self.ref_member))
+
     def test__ip_version_differs_lb_not_found(self):
         self.mock_find_ovn_lb_by_pool_id = mock.patch.object(
             ovn_helper.OvnProviderHelper,

ovn_octavia_provider/tests/unit/test_helper.py

@@ -4165,24 +4165,45 @@ def test__frame_lb_vips_no_vip_fip(self):
         expected = {'10.22.33.4:80': '192.168.2.149:1010'}
         self.assertEqual(expected, ret)
 
-    def test__frame_lb_vips_additional_vips(self):
+    def test__frame_lb_vips_additional_vips_only_member_ipv4(self):
         self.ovn_lb.external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY] = \
             '10.24.34.4,2001:db8::1'
         ret = self.helper._frame_vip_ips(self.ovn_lb, self.ovn_lb.external_ids)
         expected = {'10.22.33.4:80': '192.168.2.149:1010',
                     '10.24.34.4:80': '192.168.2.149:1010',
-                    '[2001:db8::1]: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_additional_vip_fips(self):
-        self.ovn_lb.external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_FIP_KEY] = \
-            '172.24.34.4,2001:db8::1'
+    def test__frame_lb_vips_additional_vips_mixing_member_ipv4_ipv6(self):
+        self.ovn_lb.external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY] = \
+            '10.24.34.4,2001:db8::1'
+        self.member_address = '2001:db8::3'
+        self.member_line = (
+            'member_%s_%s:%s_%s' %
+            (self.member_id, self.member_address,
+             self.member_port, self.member_subnet_id))
+        self.ovn_lb.external_ids['pool_%s' % self.pool_id] = ','.join([
+            self.ovn_lb.external_ids['pool_%s' % self.pool_id],
+            self.member_line])
+
         ret = self.helper._frame_vip_ips(self.ovn_lb, self.ovn_lb.external_ids)
         expected = {'10.22.33.4:80': '192.168.2.149:1010',
-                    '172.24.34.4:80': '192.168.2.149:1010',
-                    '[2001:db8::1]:80': '192.168.2.149:1010',
-                    '123.123.123.123:80': '192.168.2.149:1010'}
+                    '10.24.34.4:80': '192.168.2.149:1010',
+                    '123.123.123.123:80': '192.168.2.149:1010',
+                    '[2001:db8::1]:80': '[2001:db8::3]:1010'}
+        self.assertEqual(expected, ret)
+
+    def test__frame_lb_vips_additional_vips_only_member_ipv6(self):
+        self.ovn_lb.external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY] = \
+            '10.24.34.4,2001:db8::1'
+        self.member_address = '2001:db8::3'
+        self.member_line = (
+            'member_%s_%s:%s_%s' %
+            (self.member_id, self.member_address,
+             self.member_port, self.member_subnet_id))
+        self.ovn_lb.external_ids['pool_%s' % self.pool_id] = self.member_line
+        ret = self.helper._frame_vip_ips(self.ovn_lb, self.ovn_lb.external_ids)
+        expected = {'[2001:db8::1]:80': '[2001:db8::3]:1010'}
         self.assertEqual(expected, ret)
 
     def test__frame_lb_vips_disabled(self):
@@ -4198,12 +4219,10 @@ def test__frame_lb_vips_ipv6(self):
              self.member_port, self.member_subnet_id))
         self.ovn_lb.external_ids = {
             ovn_const.LB_EXT_IDS_VIP_KEY: 'fc00::',
-            ovn_const.LB_EXT_IDS_VIP_FIP_KEY: '2002::',
             'pool_%s' % self.pool_id: self.member_line,
             'listener_%s' % self.listener_id: '80:pool_%s' % self.pool_id}
         ret = self.helper._frame_vip_ips(self.ovn_lb, self.ovn_lb.external_ids)
-        expected = {'[2002::]:80': '[2001:db8::1]:1010',
-                    '[fc00::]:80': '[2001:db8::1]:1010'}
+        expected = {'[fc00::]:80': '[2001:db8::1]:1010'}
         self.assertEqual(expected, ret)
 
     def test_check_lb_protocol(self):