From d83999f4edd1ec17af68744a8456b479c5fed45b Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Tue, 16 Jan 2024 18:48:22 +0000 Subject: [PATCH] Add nftables support for SR-IOV VIPs This patch adds the initial nftables support in the amphora for SR-IOV VIPs. Followup patches will add rules to the nftables chain. As this point in the patch chain, SR-IOV VIPs will not pass any traffic. Change-Id: Ib2a1c3f49a26690d2e0e9c7330e047748c0b5105 --- diskimage-create/README.rst | 5 + diskimage-create/diskimage-create.sh | 2 +- doc/source/admin/sr-iov.rst | 15 ++ .../backends/agent/api_server/osutils.py | 5 +- .../backends/agent/api_server/plug.py | 5 +- .../backends/agent/api_server/server.py | 3 +- octavia/amphorae/backends/utils/interface.py | 48 ++++++ .../amphorae/backends/utils/interface_file.py | 15 +- .../amphorae/backends/utils/nftable_utils.py | 63 ++++++++ .../drivers/haproxy/rest_api_driver.py | 10 +- octavia/common/constants.py | 10 ++ .../backends/agent/api_server/test_osutils.py | 6 +- .../amphorae/backends/utils/test_interface.py | 142 ++++++++++++++++++ .../backends/utils/test_nftable_utils.py | 106 +++++++++++++ .../haproxy/test_rest_api_driver_1_0.py | 7 +- .../sample_configs/sample_configs_combined.py | 8 +- ...nftables-is-now-True-e1da3f92a4907b8c.yaml | 4 + 17 files changed, 432 insertions(+), 22 deletions(-) create mode 100644 octavia/amphorae/backends/utils/nftable_utils.py create mode 100644 octavia/tests/unit/amphorae/backends/utils/test_nftable_utils.py create mode 100644 releasenotes/notes/Use-nftables-is-now-True-e1da3f92a4907b8c.yaml diff --git a/diskimage-create/README.rst b/diskimage-create/README.rst index 45c011bcd6..f5e7208208 100644 --- a/diskimage-create/README.rst +++ b/diskimage-create/README.rst @@ -274,6 +274,11 @@ OCTAVIA_REPO_PATH - Default: - Reference: https://github.com/openstack/octavia +DIB_OCTAVIA_AMP_USE_NFTABLES + - Boolean that configures nftables inside the amphora image + - Required for SR-IOV enabled amphora + - Default: True + Using distribution packages for amphora agent --------------------------------------------- By default, amphora agent is installed from Octavia Git repository. diff --git a/diskimage-create/diskimage-create.sh b/diskimage-create/diskimage-create.sh index 6f745acaea..8c6962b57f 100755 --- a/diskimage-create/diskimage-create.sh +++ b/diskimage-create/diskimage-create.sh @@ -310,7 +310,7 @@ else fi # Make sure we have a value set for DIB_OCTAVIA_AMP_USE_NFTABLES -export DIB_OCTAVIA_AMP_USE_NFTABLES=${DIB_OCTAVIA_AMP_USE_NFTABLES:-False} +export DIB_OCTAVIA_AMP_USE_NFTABLES=${DIB_OCTAVIA_AMP_USE_NFTABLES:-True} export CLOUD_INIT_DATASOURCES=${CLOUD_INIT_DATASOURCES:-"ConfigDrive"} diff --git a/doc/source/admin/sr-iov.rst b/doc/source/admin/sr-iov.rst index a205c87303..d5ea07011d 100644 --- a/doc/source/admin/sr-iov.rst +++ b/doc/source/admin/sr-iov.rst @@ -86,4 +86,19 @@ Octavia flavor that will use the compute flavor. $ openstack loadbalancer flavorprofile create --name amphora-sriov-profile --provider amphora --flavor-data '{"compute_flavor": "amphora-sriov-flavor", "sriov_vip": true}' $ openstack loadbalancer flavor create --name SRIOV-public-members --flavorprofile amphora-sriov-profile --description "A load balancer that uses SR-IOV for the 'public' network and 'members' network." --enable +Building the Amphora Image +~~~~~~~~~~~~~~~~~~~~~~~~~~ +Neutron does not support security groups on SR-IOV ports, so Octavia will use +nftables inside the Amphroa to provide network security. The amphora image +must be built with nftables enabled for SR-IOV enabled load balancers. Images +with nftables enabled can be used for both SR-IOV enabled load balancers as +well as load balancers that are not using SR-IOV ports. When the SR-IOV for +load balancer VIP ports feature was added to Octavia, the default setting for +using nftables has been changed to `True`. Prior to this it needed to be +enabled by setting an environment variable before building the Amphora image: + +.. code-block:: bash + + $ export DIB_OCTAVIA_AMP_USE_NFTABLES=True + $ ./diskimage-create.sh diff --git a/octavia/amphorae/backends/agent/api_server/osutils.py b/octavia/amphorae/backends/agent/api_server/osutils.py index 16d34295c7..e28826e4de 100644 --- a/octavia/amphorae/backends/agent/api_server/osutils.py +++ b/octavia/amphorae/backends/agent/api_server/osutils.py @@ -64,14 +64,15 @@ class BaseOS(object): interface.write() def write_vip_interface_file(self, interface, vips, mtu, vrrp_info, - fixed_ips=None): + fixed_ips=None, is_sriov=False): vip_interface = interface_file.VIPInterfaceFile( name=interface, mtu=mtu, vips=vips, vrrp_info=vrrp_info, fixed_ips=fixed_ips, - topology=CONF.controller_worker.loadbalancer_topology) + topology=CONF.controller_worker.loadbalancer_topology, + is_sriov=is_sriov) vip_interface.write() def write_port_interface_file(self, interface, fixed_ips, mtu): diff --git a/octavia/amphorae/backends/agent/api_server/plug.py b/octavia/amphorae/backends/agent/api_server/plug.py index 414b4f1853..92c29cfad2 100644 --- a/octavia/amphorae/backends/agent/api_server/plug.py +++ b/octavia/amphorae/backends/agent/api_server/plug.py @@ -78,7 +78,7 @@ class Plug(object): def plug_vip(self, vip, subnet_cidr, gateway, mac_address, mtu=None, vrrp_ip=None, host_routes=(), - additional_vips=()): + additional_vips=(), is_sriov=False): vips = [{ 'ip_address': vip, 'subnet_cidr': subnet_cidr, @@ -118,7 +118,8 @@ class Plug(object): interface=primary_interface, vips=rendered_vips, mtu=mtu, - vrrp_info=vrrp_info) + vrrp_info=vrrp_info, + is_sriov=is_sriov) # Update the list of interfaces to add to the namespace # This is used in the amphora reboot case to re-establish the namespace diff --git a/octavia/amphorae/backends/agent/api_server/server.py b/octavia/amphorae/backends/agent/api_server/server.py index 0b935ffbac..c4c3d521f4 100644 --- a/octavia/amphorae/backends/agent/api_server/server.py +++ b/octavia/amphorae/backends/agent/api_server/server.py @@ -203,7 +203,8 @@ class Server(object): net_info.get('mtu'), net_info.get('vrrp_ip'), net_info.get('host_routes', ()), - net_info.get('additional_vips', ())) + net_info.get('additional_vips', ()), + net_info.get('is_sriov', False)) def plug_network(self): try: diff --git a/octavia/amphorae/backends/utils/interface.py b/octavia/amphorae/backends/utils/interface.py index 65e45b0889..057ce545fb 100644 --- a/octavia/amphorae/backends/utils/interface.py +++ b/octavia/amphorae/backends/utils/interface.py @@ -28,6 +28,7 @@ from pyroute2.netlink.rtnl import ifaddrmsg from pyroute2.netlink.rtnl import rt_proto from octavia.amphorae.backends.utils import interface_file +from octavia.amphorae.backends.utils import nftable_utils from octavia.common import constants as consts from octavia.common import exceptions @@ -175,9 +176,56 @@ class InterfaceController(object): ip_network = ipaddress.ip_network(address, strict=False) return ip_network.compressed + def _setup_nftables_chain(self, interface): + # TODO(johnsom) Move this to pyroute2 when the nftables library + # improves. + + # Create the nftable + cmd = [consts.NFT_CMD, consts.NFT_ADD, 'table', consts.NFT_FAMILY, + consts.NFT_VIP_TABLE] + try: + subprocess.check_output(cmd, stderr=subprocess.STDOUT) + except Exception as e: + if hasattr(e, 'output'): + LOG.error(e.output) + else: + LOG.error(e) + raise + + # Create the chain with -310 priority to put it in front of the + # lvs-masquerade configured chain + cmd = [consts.NFT_CMD, consts.NFT_ADD, 'chain', consts.NFT_FAMILY, + consts.NFT_VIP_TABLE, consts.NFT_VIP_CHAIN, + '{', 'type', 'filter', 'hook', 'ingress', 'device', + interface.name, 'priority', consts.NFT_SRIOV_PRIORITY, ';', + 'policy', 'drop', ';', '}'] + try: + subprocess.check_output(cmd, stderr=subprocess.STDOUT) + except Exception as e: + if hasattr(e, 'output'): + LOG.error(e.output) + else: + LOG.error(e) + raise + + nftable_utils.write_nftable_vip_rules_file(interface.name, []) + + cmd = [consts.NFT_CMD, '-o', '-f', consts.NFT_VIP_RULES_FILE] + try: + subprocess.check_output(cmd, stderr=subprocess.STDOUT) + except Exception as e: + if hasattr(e, 'output'): + LOG.error(e.output) + else: + LOG.error(e) + raise + def up(self, interface): LOG.info("Setting interface %s up", interface.name) + if interface.is_sriov: + self._setup_nftables_chain(interface) + with pyroute2.IPRoute() as ipr: idx = ipr.link_lookup(ifname=interface.name)[0] diff --git a/octavia/amphorae/backends/utils/interface_file.py b/octavia/amphorae/backends/utils/interface_file.py index 6b2957a89a..b627aca99d 100644 --- a/octavia/amphorae/backends/utils/interface_file.py +++ b/octavia/amphorae/backends/utils/interface_file.py @@ -25,9 +25,8 @@ CONF = cfg.CONF class InterfaceFile(object): - def __init__(self, name, if_type, - mtu=None, addresses=None, - routes=None, rules=None, scripts=None): + def __init__(self, name, if_type, mtu=None, addresses=None, + routes=None, rules=None, scripts=None, is_sriov=False): self.name = name self.if_type = if_type self.mtu = mtu @@ -38,6 +37,7 @@ class InterfaceFile(object): consts.IFACE_UP: [], consts.IFACE_DOWN: [] } + self.is_sriov = is_sriov @classmethod def get_extensions(cls): @@ -98,7 +98,8 @@ class InterfaceFile(object): consts.ADDRESSES: self.addresses, consts.ROUTES: self.routes, consts.RULES: self.rules, - consts.SCRIPTS: self.scripts + consts.SCRIPTS: self.scripts, + consts.IS_SRIOV: self.is_sriov } if self.mtu: interface[consts.MTU] = self.mtu @@ -106,12 +107,14 @@ class InterfaceFile(object): class VIPInterfaceFile(InterfaceFile): - def __init__(self, name, mtu, vips, vrrp_info, fixed_ips, topology): + def __init__(self, name, mtu, vips, vrrp_info, fixed_ips, topology, + is_sriov=False): - super().__init__(name, if_type=consts.VIP, mtu=mtu) + super().__init__(name, if_type=consts.VIP, mtu=mtu, is_sriov=is_sriov) has_ipv4 = any(vip['ip_version'] == 4 for vip in vips) has_ipv6 = any(vip['ip_version'] == 6 for vip in vips) + if vrrp_info: self.addresses.append({ consts.ADDRESS: vrrp_info['ip'], diff --git a/octavia/amphorae/backends/utils/nftable_utils.py b/octavia/amphorae/backends/utils/nftable_utils.py new file mode 100644 index 0000000000..8a84eb55a1 --- /dev/null +++ b/octavia/amphorae/backends/utils/nftable_utils.py @@ -0,0 +1,63 @@ +# Copyright 2024 Red Hat, Inc. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +import os +import stat + +from octavia.common import constants as consts + + +def write_nftable_vip_rules_file(interface_name, rules): + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC + # mode 00600 + mode = stat.S_IRUSR | stat.S_IWUSR + + # Create some strings shared on both code paths + table_string = f'table {consts.NFT_FAMILY} {consts.NFT_VIP_TABLE} {{\n' + chain_string = f' chain {consts.NFT_VIP_CHAIN} {{\n' + hook_string = (f' type filter hook ingress device {interface_name} ' + f'priority {consts.NFT_SRIOV_PRIORITY}; policy drop;\n') + + # Check if an existing rules file exists or we if need to create an + # "drop all" file with no rules except for VRRP. If it exists, we should + # not overwrite it here as it could be a reboot unless we were passed new + # rules. + if os.path.isfile(consts.NFT_VIP_RULES_FILE): + if not rules: + return + with os.fdopen( + os.open(consts.NFT_VIP_RULES_FILE, flags, mode), 'w') as file: + # Clear the existing rules in the kernel + # Note: The "nft -f" method is atomic, so clearing the rules will + # not leave the amphora exposed. + file.write(f'flush chain {consts.NFT_FAMILY} ' + f'{consts.NFT_VIP_TABLE} {consts.NFT_VIP_CHAIN}\n') + file.write(table_string) + file.write(chain_string) + file.write(hook_string) + # TODO(johnsom) Add peer ports here consts.HAPROXY_BASE_PEER_PORT + # and ip protocol 112 for VRRP. Need the peer address + for rule in rules: + file.write(f' {rule}\n') + file.write(' }\n') # close the chain + file.write('}\n') # close the table + else: # No existing rules, create the "drop all" base rules + with os.fdopen( + os.open(consts.NFT_VIP_RULES_FILE, flags, mode), 'w') as file: + file.write(table_string) + file.write(chain_string) + file.write(hook_string) + # TODO(johnsom) Add peer ports here consts.HAPROXY_BASE_PEER_PORT + # and ip protocol 112 for VRRP. Need the peer address + file.write(' }\n') # close the chain + file.write('}\n') # close the table diff --git a/octavia/amphorae/drivers/haproxy/rest_api_driver.py b/octavia/amphorae/drivers/haproxy/rest_api_driver.py index 58f95c2ede..676f2f1807 100644 --- a/octavia/amphorae/drivers/haproxy/rest_api_driver.py +++ b/octavia/amphorae/drivers/haproxy/rest_api_driver.py @@ -340,7 +340,7 @@ class HaproxyAmphoraLoadBalancerDriver( def finalize_amphora(self, amphora): pass - def _build_net_info(self, port, amphora, subnet, mtu=None): + def _build_net_info(self, port, amphora, subnet, mtu=None, sriov=False): # NOTE(blogan): using the vrrp port here because that # is what the allowed address pairs network driver sets # this particular port to. This does expose a bit of @@ -359,7 +359,8 @@ class HaproxyAmphoraLoadBalancerDriver( 'vrrp_ip': amphora[consts.VRRP_IP], 'mtu': mtu or port[consts.NETWORK][consts.MTU], 'host_routes': host_routes, - 'additional_vips': []} + 'additional_vips': [], + 'is_sriov': sriov} return net_info def post_vip_plug(self, amphora, load_balancer, amphorae_network_config, @@ -370,9 +371,12 @@ class HaproxyAmphoraLoadBalancerDriver( mtu = port[consts.NETWORK][consts.MTU] LOG.debug("Post-VIP-Plugging with vrrp_ip %s vrrp_port %s", amphora.vrrp_ip, port[consts.ID]) + sriov = False + if load_balancer.vip.vnic_type == consts.VNIC_TYPE_DIRECT: + sriov = True net_info = self._build_net_info( port, amphora.to_dict(), - vip_subnet.to_dict(recurse=True), mtu) + vip_subnet.to_dict(recurse=True), mtu, sriov) for add_vip in additional_vip_data: add_host_routes = [{'nexthop': hr.nexthop, 'destination': hr.destination} diff --git a/octavia/common/constants.py b/octavia/common/constants.py index afa7d0fe42..426377b8f4 100644 --- a/octavia/common/constants.py +++ b/octavia/common/constants.py @@ -366,6 +366,7 @@ ID = 'id' IMAGE_ID = 'image_id' IP_ADDRESS = 'ip_address' IPV6_ICMP = 'ipv6-icmp' +IS_SRIOV = 'is_sriov' LB_NETWORK_IP = 'lb_network_ip' L7POLICY = 'l7policy' L7POLICY_ID = 'l7policy_id' @@ -967,3 +968,12 @@ IFLA_IFNAME = 'IFLA_IFNAME' # Amphora network directory AMP_NET_DIR_TEMPLATE = '/etc/octavia/interfaces/' + +# Amphora nftables constants +NFT_ADD = 'add' +NFT_CMD = '/usr/sbin/nft' +NFT_FAMILY = 'inet' +NFT_VIP_RULES_FILE = '/var/lib/octavia/nftables-vip.rules' +NFT_VIP_TABLE = 'amphora-vip' +NFT_VIP_CHAIN = 'amphora-vip-chain' +NFT_SRIOV_PRIORITY = '-310' diff --git a/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py b/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py index d786d23ee5..609e1a75c9 100644 --- a/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py +++ b/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py @@ -163,7 +163,8 @@ class TestOSUtils(base.TestCase): mtu=MTU, vrrp_info=None, fixed_ips=None, - topology="SINGLE") + topology="SINGLE", + is_sriov=False) mock_vip_interface_file.return_value.write.assert_called_once() # Now test with an IPv6 VIP @@ -193,7 +194,8 @@ class TestOSUtils(base.TestCase): mtu=MTU, vrrp_info=None, fixed_ips=None, - topology="SINGLE") + topology="SINGLE", + is_sriov=False) @mock.patch('octavia.amphorae.backends.utils.interface_file.' 'PortInterfaceFile') diff --git a/octavia/tests/unit/amphorae/backends/utils/test_interface.py b/octavia/tests/unit/amphorae/backends/utils/test_interface.py index aed9aac894..831826e65b 100644 --- a/octavia/tests/unit/amphorae/backends/utils/test_interface.py +++ b/octavia/tests/unit/amphorae/backends/utils/test_interface.py @@ -448,6 +448,148 @@ class TestInterface(base.TestCase): mock.call(["post-up", "eth1"]) ]) + @mock.patch('octavia.amphorae.backends.utils.nftable_utils.' + 'write_nftable_vip_rules_file') + @mock.patch('pyroute2.IPRoute.rule') + @mock.patch('pyroute2.IPRoute.route') + @mock.patch('pyroute2.IPRoute.addr') + @mock.patch('pyroute2.IPRoute.link') + @mock.patch('pyroute2.IPRoute.get_links') + @mock.patch('pyroute2.IPRoute.link_lookup') + @mock.patch('subprocess.check_output') + def test_up_sriov(self, mock_check_output, mock_link_lookup, + mock_get_links, mock_link, mock_addr, mock_route, + mock_rule, mock_nftable): + iface = interface_file.InterfaceFile( + name="fake-eth1", + if_type="vip", + mtu=1450, + addresses=[{ + consts.ADDRESS: '192.0.2.4', + consts.PREFIXLEN: 24 + }, { + consts.ADDRESS: '198.51.100.4', + consts.PREFIXLEN: 16 + }, { + consts.ADDRESS: '2001:db8::3', + consts.PREFIXLEN: 64 + }], + routes=[{ + consts.DST: '203.0.113.0/24', + consts.GATEWAY: '192.0.2.1', + consts.TABLE: 10, + consts.ONLINK: True + }, { + consts.DST: '198.51.100.0/24', + consts.GATEWAY: '192.0.2.2', + consts.PREFSRC: '192.0.2.4', + consts.SCOPE: 'link' + }, { + consts.DST: '2001:db8:2::1/128', + consts.GATEWAY: '2001:db8::1' + }], + rules=[{ + consts.SRC: '203.0.113.1', + consts.SRC_LEN: 32, + consts.TABLE: 20, + }, { + consts.SRC: '2001:db8::1', + consts.SRC_LEN: 128, + consts.TABLE: 40, + }], + scripts={ + consts.IFACE_UP: [{ + consts.COMMAND: "post-up fake-eth1" + }], + consts.IFACE_DOWN: [{ + consts.COMMAND: "post-down fake-eth1" + }], + }, + is_sriov=True) + + idx = mock.MagicMock() + mock_link_lookup.return_value = [idx] + + mock_get_links.return_value = [{ + consts.STATE: consts.IFACE_DOWN + }] + + controller = interface.InterfaceController() + controller.up(iface) + + mock_link.assert_called_once_with( + controller.SET, + index=idx, + state=consts.IFACE_UP, + mtu=1450) + + mock_addr.assert_has_calls([ + mock.call(controller.ADD, + index=idx, + address='192.0.2.4', + prefixlen=24, + family=socket.AF_INET), + mock.call(controller.ADD, + index=idx, + address='198.51.100.4', + prefixlen=16, + family=socket.AF_INET), + mock.call(controller.ADD, + index=idx, + address='2001:db8::3', + prefixlen=64, + family=socket.AF_INET6) + ]) + + mock_route.assert_has_calls([ + mock.call(controller.ADD, + oif=idx, + dst='203.0.113.0/24', + gateway='192.0.2.1', + table=10, + onlink=True, + family=socket.AF_INET), + mock.call(controller.ADD, + oif=idx, + dst='198.51.100.0/24', + gateway='192.0.2.2', + prefsrc='192.0.2.4', + scope='link', + family=socket.AF_INET), + mock.call(controller.ADD, + oif=idx, + dst='2001:db8:2::1/128', + gateway='2001:db8::1', + family=socket.AF_INET6)]) + + mock_rule.assert_has_calls([ + mock.call(controller.ADD, + src="203.0.113.1", + src_len=32, + table=20, + family=socket.AF_INET), + mock.call(controller.ADD, + src="2001:db8::1", + src_len=128, + table=40, + family=socket.AF_INET6)]) + + mock_check_output.assert_has_calls([ + mock.call([consts.NFT_CMD, consts.NFT_ADD, 'table', + consts.NFT_FAMILY, consts.NFT_VIP_TABLE], stderr=-2), + mock.call([consts.NFT_CMD, consts.NFT_ADD, 'chain', + consts.NFT_FAMILY, consts.NFT_VIP_TABLE, + consts.NFT_VIP_CHAIN, '{', 'type', 'filter', 'hook', + 'ingress', 'device', 'fake-eth1', 'priority', + consts.NFT_SRIOV_PRIORITY, ';', 'policy', 'drop', ';', + '}'], stderr=-2), + mock.call([consts.NFT_CMD, '-o', '-f', consts.NFT_VIP_RULES_FILE], + stderr=-2), + mock.call(["post-up", "fake-eth1"]) + ]) + + mock_nftable.assert_called_once_with('fake-eth1', []) + @mock.patch('pyroute2.IPRoute.rule') @mock.patch('pyroute2.IPRoute.route') @mock.patch('pyroute2.IPRoute.addr') diff --git a/octavia/tests/unit/amphorae/backends/utils/test_nftable_utils.py b/octavia/tests/unit/amphorae/backends/utils/test_nftable_utils.py new file mode 100644 index 0000000000..f4fdaecd6f --- /dev/null +++ b/octavia/tests/unit/amphorae/backends/utils/test_nftable_utils.py @@ -0,0 +1,106 @@ +# Copyright 2024 Red Hat, Inc. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +import os +import stat +from unittest import mock + +from octavia.amphorae.backends.utils import nftable_utils +from octavia.common import constants as consts +import octavia.tests.unit.base as base + + +class TestNFTableUtils(base.TestCase): + @mock.patch('os.open') + @mock.patch('os.path.isfile') + def test_write_nftable_vip_rules_file_exists(self, mock_isfile, mock_open): + """Test when a rules file exists and no new rules + + When an existing rules file is present and we call + write_nftable_vip_rules_file with no rules, the method should not + overwrite the existing rules. + """ + mock_isfile.return_value = True + + nftable_utils.write_nftable_vip_rules_file('fake-eth2', []) + + mock_open.assert_not_called() + + @mock.patch('os.open') + @mock.patch('os.path.isfile') + def test_write_nftable_vip_rules_file_rules(self, mock_isfile, + mock_open): + """Test when a rules file exists and rules are passed in + + This should create a simple rules file with the base chain and rules. + """ + mock_isfile.return_value = True + mock_open.return_value = 'fake-fd' + + mocked_open = mock.mock_open() + with mock.patch.object(os, 'fdopen', mocked_open): + nftable_utils.write_nftable_vip_rules_file( + 'fake-eth2', ['test rule 1', 'test rule 2']) + + mocked_open.assert_called_once_with('fake-fd', 'w') + mock_open.assert_called_once_with( + consts.NFT_VIP_RULES_FILE, + (os.O_WRONLY | os.O_CREAT | os.O_TRUNC), + (stat.S_IRUSR | stat.S_IWUSR)) + + handle = mocked_open() + handle.write.assert_has_calls([ + mock.call(f'flush chain {consts.NFT_FAMILY} ' + f'{consts.NFT_VIP_TABLE} {consts.NFT_VIP_CHAIN}\n'), + mock.call(f'table {consts.NFT_FAMILY} {consts.NFT_VIP_TABLE} ' + '{\n'), + mock.call(f' chain {consts.NFT_VIP_CHAIN} {{\n'), + mock.call(' type filter hook ingress device fake-eth2 ' + f'priority {consts.NFT_SRIOV_PRIORITY}; policy drop;\n'), + mock.call(' test rule 1\n'), + mock.call(' test rule 2\n'), + mock.call(' }\n'), + mock.call('}\n') + ]) + + @mock.patch('os.open') + @mock.patch('os.path.isfile') + def test_write_nftable_vip_rules_file_missing(self, mock_isfile, + mock_open): + """Test when a rules file does not exist and no new rules + + This should create a simple rules file with the base chain. + """ + mock_isfile.return_value = False + mock_open.return_value = 'fake-fd' + + mocked_open = mock.mock_open() + with mock.patch.object(os, 'fdopen', mocked_open): + nftable_utils.write_nftable_vip_rules_file('fake-eth2', []) + + mocked_open.assert_called_once_with('fake-fd', 'w') + mock_open.assert_called_once_with( + consts.NFT_VIP_RULES_FILE, + (os.O_WRONLY | os.O_CREAT | os.O_TRUNC), + (stat.S_IRUSR | stat.S_IWUSR)) + + handle = mocked_open() + handle.write.assert_has_calls([ + mock.call(f'table {consts.NFT_FAMILY} {consts.NFT_VIP_TABLE} ' + '{\n'), + mock.call(f' chain {consts.NFT_VIP_CHAIN} {{\n'), + mock.call(' type filter hook ingress device fake-eth2 ' + f'priority {consts.NFT_SRIOV_PRIORITY}; policy drop;\n'), + mock.call(' }\n'), + mock.call('}\n') + ]) diff --git a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py index eb2c06013f..6613b6b837 100644 --- a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py +++ b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py @@ -113,7 +113,8 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): 'vrrp_ip': self.amp.vrrp_ip, 'mtu': FAKE_MTU, 'host_routes': host_routes_data, - 'additional_vips': []} + 'additional_vips': [], + 'is_sriov': False} self.timeout_dict = {constants.REQ_CONN_TIMEOUT: 1, constants.REQ_READ_TIMEOUT: 2, @@ -766,6 +767,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): 'host_routes': netinfo['host_routes'] } ] + netinfo['is_sriov'] = False self.driver.clients[API_VERSION].plug_vip.assert_called_once_with( self.amp, self.lb.vip.ip_address, netinfo) @@ -815,7 +817,8 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): vrrp_ip=self.amp.vrrp_ip, host_routes=[], additional_vips=[], - mtu=FAKE_MTU + mtu=FAKE_MTU, + is_sriov=False ))) def test_post_network_plug_with_host_routes(self): diff --git a/octavia/tests/unit/common/sample_configs/sample_configs_combined.py b/octavia/tests/unit/common/sample_configs/sample_configs_combined.py index 4828c58ab4..8835300799 100644 --- a/octavia/tests/unit/common/sample_configs/sample_configs_combined.py +++ b/octavia/tests/unit/common/sample_configs/sample_configs_combined.py @@ -670,9 +670,11 @@ def sample_vrrp_group_tuple(): smtp_connect_timeout='') -def sample_vip_tuple(ip_address='10.0.0.2', subnet_id='vip_subnet_uuid'): - vip = collections.namedtuple('vip', ('ip_address', 'subnet_id')) - return vip(ip_address=ip_address, subnet_id=subnet_id) +def sample_vip_tuple(ip_address='10.0.0.2', subnet_id='vip_subnet_uuid', + vnic_type='normal'): + vip = collections.namedtuple('vip', ('ip_address', 'subnet_id', + 'vnic_type')) + return vip(ip_address=ip_address, subnet_id=subnet_id, vnic_type=vnic_type) def sample_listener_tuple(proto=None, monitor=True, alloc_default_pool=True, diff --git a/releasenotes/notes/Use-nftables-is-now-True-e1da3f92a4907b8c.yaml b/releasenotes/notes/Use-nftables-is-now-True-e1da3f92a4907b8c.yaml new file mode 100644 index 0000000000..2d4387825b --- /dev/null +++ b/releasenotes/notes/Use-nftables-is-now-True-e1da3f92a4907b8c.yaml @@ -0,0 +1,4 @@ +--- +other: + - | + Amphora images will now be built with nftables by default.