From 745d21cb9db8cc535d67a6739111fba68c6e057e Mon Sep 17 00:00:00 2001 From: Ori Amizur Date: Thu, 25 Mar 2021 11:45:02 +0200 Subject: [PATCH] MGMT-4761 Clear all nat iptables rules when tearing down tests using user-managed-networking --- Makefile | 10 +------ discovery-infra/delete_nodes.py | 16 +++++++--- discovery-infra/start_discovery.py | 2 +- .../test_infra/controllers/nat_controller.py | 29 ++++++++++++------- .../test_infra/helper_classes/cluster.py | 20 +++++++++++-- 5 files changed, 49 insertions(+), 28 deletions(-) diff --git a/Makefile b/Makefile index 37b764db3e5..137348ebbaa 100644 --- a/Makefile +++ b/Makefile @@ -120,7 +120,7 @@ IPV6_SUPPORT := $(or ${IPV6_SUPPORT},true) all: create_full_environment run_full_flow_with_install -destroy: destroy_nodes delete_minikube_profile kill_port_forwardings delete_podman_localhost stop_load_balancer stop_nat +destroy: destroy_nodes delete_minikube_profile kill_port_forwardings delete_podman_localhost stop_load_balancer ############### # Environment # @@ -174,14 +174,6 @@ start_load_balancer: stop_load_balancer: @id=`podman ps --quiet --filter "name=load_balancer"`; test ! -z "$$id" && podman rm -f load_balancer ; rm -f $(HOME)/.test-infra/etc/nginx/stream.d/*.conf >& /dev/null || /bin/true -######## -# Nat # -######## - -# Remove all rules from NAT table that use mark 0x22b (555) which is used for none platform nat. It is done iptables-save which dumps the nat table rules, removing the relevant rules, and feeding the result to iptables-restore -stop_nat: - @iptables-save -t nat -c | grep -v 'mark 0x22b' | iptables-restore - ############# # Terraform # diff --git a/discovery-infra/delete_nodes.py b/discovery-infra/delete_nodes.py index e8d832a242d..10c302ea6a8 100755 --- a/discovery-infra/delete_nodes.py +++ b/discovery-infra/delete_nodes.py @@ -4,6 +4,7 @@ import argparse import os import shutil +import re from test_infra import assisted_service_api, utils, consts from test_infra.controllers.nat_controller import NatController @@ -28,18 +29,25 @@ def try_to_delete_cluster(namespace, tfvars): ) client.delete_cluster(cluster_id=cluster_id) +def _get_namespace_index(libvirt_network_if): + # Hack to retrieve namespace index - does not exist in tests + matcher = re.match(r'^tt(\d+)$', libvirt_network_if) + return int(matcher.groups()[0]) if matcher is not None else 0 + @utils.on_exception(message='Failed to remove nat', silent=True) -def _try_remove_nat(namespace, tfvars): - primary_interface = tfvars.get('libvirt_network_if', f'tt{namespace}') +def _try_remove_nat(tfvars): + primary_interface = tfvars.get('libvirt_network_if') + if primary_interface is None: + raise Exception("Could not get primary interface") secondary_interface = tfvars.get('libvirt_secondary_network_if', f's{primary_interface}') nat_controller = NatController() - nat_controller.remove_nat_rules([primary_interface, secondary_interface]) + nat_controller.remove_nat_rules([primary_interface, secondary_interface], _get_namespace_index(primary_interface)) def delete_nodes(cluster_name, namespace, tf_folder, tfvars): """ Runs terraform destroy and then cleans it with virsh cleanup to delete everything relevant. """ - _try_remove_nat(namespace, tfvars) + _try_remove_nat(tfvars) if os.path.exists(tf_folder): _try_to_delete_nodes(tf_folder) diff --git a/discovery-infra/start_discovery.py b/discovery-infra/start_discovery.py index cbd12732c82..15a79c33f12 100755 --- a/discovery-infra/start_discovery.py +++ b/discovery-infra/start_discovery.py @@ -220,7 +220,7 @@ def create_nodes_and_wait_till_registered( if is_ipv4 and is_none_platform_mode() and nodes_details['master_count'] > 1: input_interfaces = [args.network_bridge, f"s{args.network_bridge}"] nat_controller = NatController() - nat_controller.add_nat_rules(input_interfaces) + nat_controller.add_nat_rules(input_interfaces, args.ns_index) utils.wait_till_all_hosts_are_in_status( client=inventory_client, diff --git a/discovery-infra/test_infra/controllers/nat_controller.py b/discovery-infra/test_infra/controllers/nat_controller.py index 898052e22db..2d59946199f 100644 --- a/discovery-infra/test_infra/controllers/nat_controller.py +++ b/discovery-infra/test_infra/controllers/nat_controller.py @@ -7,6 +7,10 @@ # reference this mark in order to perform NAT operation on these packets. # class NatController: + # Build iptables mark + def _build_mark(self, ns_index): + return 555 + int(ns_index) + # Find all interfaces that have default route on them. Usually it is a single interface. def _get_default_interfaces(self): interfaces, _, _ = run_command(r"ip -4 route | egrep '^default ' | awk '{print $5}'", shell=True) @@ -14,14 +18,14 @@ def _get_default_interfaces(self): # Mark all packets coming from the input_interface with "555". Marking is needed because input interface # query is not available in POSTROUTING chain - def _build_mark_string(self, input_interface): - rule_template = ["PREROUTING", "-i", input_interface, "-j", "MARK", "--set-mark", "555"] + def _build_mark_string(self, input_interface, mark): + rule_template = ["PREROUTING", "-i", input_interface, "-j", "MARK", "--set-mark", f"{mark}"] return " ".join(rule_template) # Perform MASQUERADE nat operation on all marked packets with "555" and their output interface is "output_interface" - def _build_nat_string(self, output_interface): - rule_template = ["POSTROUTING", "-m", "mark", "--mark", "555", "-o", output_interface, "-j", "MASQUERADE"] + def _build_nat_string(self, output_interface, mark): + rule_template = ["POSTROUTING", "-m", "mark", "--mark", f"{mark}", "-o", output_interface, "-j", "MASQUERADE"] return " ".join(rule_template) @@ -61,16 +65,19 @@ def _remove_rule(self, rule_suffix): self._delete_rule(rule_suffix) # Add rules for the input interfaces and output interfaces - def add_nat_rules(self, input_interfaces): + def add_nat_rules(self, input_interfaces, ns_index): logging.info("Adding nat rules for interfaces %s", input_interfaces) + mark = self._build_mark(ns_index) for output_interface in self._get_default_interfaces(): - self._add_rule(self._build_nat_string(output_interface)) + self._add_rule(self._build_nat_string(output_interface, mark)) for input_interface in input_interfaces: - self._add_rule(self._build_mark_string(input_interface)) + self._add_rule(self._build_mark_string(input_interface, mark)) - # Delete nat rules. The nat rules for the output interfaces are not deleted since they may be used - # by another cluster. They may be removed by he destroy Makefile target - def remove_nat_rules(self, input_interfaces): + # Delete nat rules + def remove_nat_rules(self, input_interfaces, ns_index): logging.info("Deleting nat rules for interfaces %s", input_interfaces) + mark = self._build_mark(ns_index) for input_interface in input_interfaces: - self._remove_rule(self._build_mark_string(input_interface)) + self._remove_rule(self._build_mark_string(input_interface, mark)) + for output_interface in self._get_default_interfaces(): + self._remove_rule(self._build_nat_string(output_interface, mark)) diff --git a/discovery-infra/test_infra/helper_classes/cluster.py b/discovery-infra/test_infra/helper_classes/cluster.py index b4e0952415f..ad57e43cf19 100644 --- a/discovery-infra/test_infra/helper_classes/cluster.py +++ b/discovery-infra/test_infra/helper_classes/cluster.py @@ -3,6 +3,7 @@ import json import logging import random +import re import time from collections import Counter from typing import List @@ -539,7 +540,7 @@ def prepare_for_install( platform=env_variables['platform'] ): if platform == consts.Platforms.NONE and not nodes.controller.ipv6: - self._configure_nat(nodes.controller) + self.configure_nat(nodes.controller) if download_image: if env_variables.get('static_network_config'): @@ -737,13 +738,26 @@ def _configure_load_balancer(self, controller): lb_controller.set_load_balancing_config(load_balancer_ip, master_ips, worker_ips) @classmethod - def _configure_nat(cls, controller): + def _get_namespace_index(cls, libvirt_network_if): + # Hack to retrieve namespace index - does not exist in tests + matcher = re.match(r'^tt(\d+)$', libvirt_network_if) + return int(matcher.groups()[0]) if matcher is not None else 0 + + @classmethod + def configure_nat(cls, controller): libvirt_network_if = controller.network_conf.libvirt_network_if libvirt_secondary_network_if = controller.network_conf.libvirt_secondary_network_if input_interfaces = [libvirt_network_if, libvirt_secondary_network_if] + nat_controller = NatController() + nat_controller.add_nat_rules(input_interfaces, cls._get_namespace_index(libvirt_network_if)) + @classmethod + def unconfigure_nat(cls, controller): + libvirt_network_if = controller.network_conf.libvirt_network_if + libvirt_secondary_network_if = controller.network_conf.libvirt_secondary_network_if + input_interfaces = [libvirt_network_if, libvirt_secondary_network_if] nat_controller = NatController() - nat_controller.add_nat_rules(input_interfaces) + nat_controller.remove_nat_rules(input_interfaces, cls._get_namespace_index(libvirt_network_if)) def _find_event(self, event_to_find, reference_time, params_list, host_id): events_list = self.get_events(host_id=host_id)