From f62d3e64a411875a563998c56303e7c68791b4eb Mon Sep 17 00:00:00 2001 From: sarthurdev <965089+sarthurdev@users.noreply.github.com> Date: Fri, 15 Nov 2024 20:10:28 +0100 Subject: [PATCH] Test fixes for reported issues --- data/templates/load-balancing/nftables-wlb.j2 | 28 +++++++++++-------- python/vyos/wanloadbalance.py | 9 ++++-- smoketest/scripts/cli/base_vyostest_shim.py | 9 ++++++ .../scripts/cli/test_load-balancing_wan.py | 10 +++++++ src/conf_mode/load-balancing_wan.py | 8 ++---- src/helpers/vyos-load-balancer.py | 22 ++++++++++++++- 6 files changed, 66 insertions(+), 20 deletions(-) diff --git a/data/templates/load-balancing/nftables-wlb.j2 b/data/templates/load-balancing/nftables-wlb.j2 index c3d4019dcd..75604aca1b 100644 --- a/data/templates/load-balancing/nftables-wlb.j2 +++ b/data/templates/load-balancing/nftables-wlb.j2 @@ -22,15 +22,17 @@ table ip vyos_wanloadbalance { iifname "{{ ifname }}" ct state new ct mark set {{ state.mark }} {% endif %} {% endfor %} -{% for rule_id, rule_conf in rule.items() %} -{% if rule_conf.exclude is vyos_defined %} +{% if rule is vyos_defined %} +{% for rule_id, rule_conf in rule.items() %} +{% if rule_conf.exclude is vyos_defined %} {{ rule_conf | wlb_nft_rule(rule_id, exclude=True, action='accept') }} -{% else %} -{% set limit = rule_conf.limit is vyos_defined %} +{% else %} +{% set limit = rule_conf.limit is vyos_defined %} {{ rule_conf | wlb_nft_rule(rule_id, limit=limit, weight=True, health_state=health_state) }} {{ rule_conf | wlb_nft_rule(rule_id, restore_mark=True) }} -{% endif %} -{% endfor %} +{% endif %} +{% endfor %} +{% endif %} } chain wlb_mangle_output { @@ -39,15 +41,17 @@ table ip vyos_wanloadbalance { meta mark != 0x0 counter accept meta l4proto icmp counter accept ip saddr 127.0.0.0/8 ip daddr 127.0.0.0/8 counter accept -{% for rule_id, rule_conf in rule.items() %} -{% if rule_conf.exclude is vyos_defined %} +{% if rule is vyos_defined %} +{% for rule_id, rule_conf in rule.items() %} +{% if rule_conf.exclude is vyos_defined %} {{ rule_conf | wlb_nft_rule(rule_id, local=True, exclude=True, action='accept') }} -{% else %} -{% set limit = rule_conf.limit is vyos_defined %} +{% else %} +{% set limit = rule_conf.limit is vyos_defined %} {{ rule_conf | wlb_nft_rule(rule_id, local=True, limit=limit, weight=True, health_state=health_state) }} {{ rule_conf | wlb_nft_rule(rule_id, local=True, restore_mark=True) }} -{% endif %} -{% endfor %} +{% endif %} +{% endfor %} +{% endif %} {% endif %} } diff --git a/python/vyos/wanloadbalance.py b/python/vyos/wanloadbalance.py index f7f3a5a2e9..5d55458684 100644 --- a/python/vyos/wanloadbalance.py +++ b/python/vyos/wanloadbalance.py @@ -18,7 +18,7 @@ from vyos.utils.process import run -dhclient_lease = '/var/lib/dhcp/dhclient_{0}.lease' +dhclient_lease = '/run/dhclient/dhclient_{0}.lease' def nft_rule(rule_conf, rule_id, local=False, exclude=False, limit=False, weight=None, health_state=None, action=None, restore_mark=False): output = [] @@ -32,11 +32,16 @@ def nft_rule(rule_conf, rule_id, local=False, exclude=False, limit=False, weight if 'protocol' in rule_conf and rule_conf['protocol'] != 'all': protocol = rule_conf['protocol'] + operator = '' + + if protocol[:1] == '!': + operator = '!=' + protocol = protocol[1:] if protocol == 'tcp_udp': protocol = '{ tcp, udp }' - output.append(f'meta l4proto {protocol}') + output.append(f'meta l4proto {operator} {protocol}') for direction in ['source', 'destination']: if direction not in rule_conf: diff --git a/smoketest/scripts/cli/base_vyostest_shim.py b/smoketest/scripts/cli/base_vyostest_shim.py index a383e596cb..0dc199ab83 100644 --- a/smoketest/scripts/cli/base_vyostest_shim.py +++ b/smoketest/scripts/cli/base_vyostest_shim.py @@ -147,6 +147,15 @@ def verify_nftables_chain(self, nftables_search, table, chain, inverse=False, ar break self.assertTrue(not matched if inverse else matched, msg=search) + def verify_nftables_chain_exists(self, table, chain, inverse=False): + try: + cmd(f'sudo nft list chain {table} {chain}') + if inverse: + self.fail(f'Chain exists: {table} {chain}') + except OSError: + if not inverse: + self.fail(f'Chain does not exist: {table} {chain}') + # Verify ip rule output def verify_rules(self, rules_search, inverse=False, addr_family='inet'): rule_output = cmd(f'ip -family {addr_family} rule show') diff --git a/smoketest/scripts/cli/test_load-balancing_wan.py b/smoketest/scripts/cli/test_load-balancing_wan.py index 6e11afba67..23c6e70b29 100755 --- a/smoketest/scripts/cli/test_load-balancing_wan.py +++ b/smoketest/scripts/cli/test_load-balancing_wan.py @@ -54,6 +54,16 @@ def tearDown(self): self.cli_delete(base_path) self.cli_commit() + removed_chains = [ + 'wlb_mangle_isp_veth1', + 'wlb_mangle_isp_veth2', + 'wlb_mangle_isp_eth201', + 'wlb_mangle_isp_eth202' + ] + + for chain in removed_chains: + self.verify_nftables_chain_exists('ip vyos_wanloadbalance', chain, inverse=True) + def test_table_routes(self): ns1 = 'ns201' ns2 = 'ns202' diff --git a/src/conf_mode/load-balancing_wan.py b/src/conf_mode/load-balancing_wan.py index d576b7f93e..aa0c6c8f81 100755 --- a/src/conf_mode/load-balancing_wan.py +++ b/src/conf_mode/load-balancing_wan.py @@ -98,14 +98,12 @@ def generate(lb): def apply(lb): if not lb: - cmd(f'systemctl stop {service}') - return None - - cmd(f'systemctl restart {service}') + cmd(f'sudo systemctl stop {service}') + else: + cmd(f'sudo systemctl restart {service}') call_dependents() - if __name__ == '__main__': try: c = get_config() diff --git a/src/helpers/vyos-load-balancer.py b/src/helpers/vyos-load-balancer.py index efe537eb73..6ffac9e156 100755 --- a/src/helpers/vyos-load-balancer.py +++ b/src/helpers/vyos-load-balancer.py @@ -124,6 +124,17 @@ def nftables_update(lb): return True +def cleanup(lb): + if 'interface_health' in lb: + index = 1 + for ifname, health_conf in lb['interface_health'].items(): + table_num = lb['mark_offset'] + index + run(f'ip route del table {table_num} default') + run(f'ip rule del fwmark {hex(table_num)} table {table_num}') + index += 1 + + run(f'nft delete table ip vyos_wanloadbalance') + def get_config(): conf = Config() base = ['load-balancing', 'wan'] @@ -169,7 +180,6 @@ def get_config(): else: run(f'ip route replace table {table_num} default dev {ifname} via {health_conf["nexthop"]}') - run(f'ip route delete table {table_num}') run(f'ip rule add fwmark {hex(table_num)} table {table_num}') index += 1 @@ -199,9 +209,14 @@ def handle_sigterm(signum, frame): if os.path.exists(wlb_pid_file): os.unlink(wlb_pid_file) + if os.path.exists(nftables_wlb_conf): + os.unlink(nftables_wlb_conf) + + cleanup(lb) sys.exit(0) signal.signal(signal.SIGUSR2, handle_sigusr2) + signal.signal(signal.SIGINT, handle_sigterm) signal.signal(signal.SIGTERM, handle_sigterm) with open(wlb_pid_file, 'w') as f: @@ -260,3 +275,8 @@ def handle_sigterm(signum, frame): if os.path.exists(wlb_pid_file): os.unlink(wlb_pid_file) + + if os.path.exists(nftables_wlb_conf): + os.unlink(nftables_wlb_conf) + + cleanup(lb)