From 39531c1c6a8c129c08945b29a3d8a599353b25a6 Mon Sep 17 00:00:00 2001 From: Nashwan Azhari Date: Tue, 17 Dec 2024 17:39:54 +0200 Subject: [PATCH] integration: fix test_containerd_path_cleanup_on_failed_init. Previously, the test was inducing the `bootstrap` failure by attempting to pre-allocate the port of the kube-controller-manager. This methodology wasn't applicable on any non-LocalHarness testing setups, so the test was updated to induce the failure by pre-creating the containerd socket directory (`/run/containerd`), which should lead to the same cleanup logic being triggered. Signed-off-by: Nashwan Azhari --- tests/integration/tests/test_cleanup.py | 57 ++++++++++++----------- tests/integration/tests/test_util/util.py | 34 -------------- 2 files changed, 31 insertions(+), 60 deletions(-) diff --git a/tests/integration/tests/test_cleanup.py b/tests/integration/tests/test_cleanup.py index be32d4719..71f0853ab 100644 --- a/tests/integration/tests/test_cleanup.py +++ b/tests/integration/tests/test_cleanup.py @@ -11,11 +11,11 @@ LOG = logging.getLogger(__name__) -KUBE_CONTROLLER_MANAGER_SNAP_PORT = 10257 +CONTAINERD_SOCKET_DIRECTORY_CLASSIC = "/run/containerd" CONTAINERD_PATHS = [ "/etc/containerd", - "/run/containerd", + CONTAINERD_SOCKET_DIRECTORY_CLASSIC, "/var/lib/containerd", ] CNI_PATH = "/opt/cni/bin" @@ -114,7 +114,6 @@ def test_node_cleanup_new_containerd_path(instances: List[harness.Instance]): @pytest.mark.node_count(1) @pytest.mark.no_setup() @pytest.mark.tags(tags.NIGHTLY) -@pytest.mark.skip(reason="the test fails when using a harness other than 'local'") def test_containerd_path_cleanup_on_failed_init( instances: List[harness.Instance], tmp_path ): @@ -122,38 +121,44 @@ def test_containerd_path_cleanup_on_failed_init( containerd-related paths it may have created as part of the failed `bootstrap`. - It induces a bootstrap failure by pre-binding a required k8s service - port (10257 for the kube-controller-manager) before running `k8s bootstrap`. + It introduces a bootstrap failure by pre-creating the containerd socket + path before running `k8s-bootstrap`. + + The bootstrap/join-cluster aborting behavior was added in this PR: + https://github.com/canonical/k8s-snap/pull/772 NOTE: a failed `join-cluster` will trigger the exact same cleanup hook, so the test implicitly applies to it as well. """ instance = instances[0] expected_code = 1 - expected_message = ( - "Encountered error(s) while verifying port availability for Kubernetes " - "services: Port 10257 (needed by: kube-controller-manager) is already in use." - ) + expected_message = "containerd socket already exists" + + util.setup_k8s_snap(instance, tmp_path, config.SNAP, connect_interfaces=False) - with util.open_port(KUBE_CONTROLLER_MANAGER_SNAP_PORT) as _: - util.setup_k8s_snap(instance, tmp_path, config.SNAP, connect_interfaces=False) + # Pre-create the containerd socket directory in the test instance: + instance.exec(["mkdir", "-p", CONTAINERD_SOCKET_DIRECTORY_CLASSIC], check=True) - proc = instance.exec( - ["k8s", "bootstrap"], capture_output=True, text=True, check=False + proc = instance.exec( + ["k8s", "bootstrap"], capture_output=True, text=True, check=False + ) + + if proc.returncode != expected_code: + raise AssertionError( + f"Expected `k8s bootstrap` to exit with code {expected_code}, " + f"but it exited with {proc.returncode}.\n" + f"Stdout was: \n{proc.stdout}.\nStderr was: \n{proc.stderr}" ) - if proc.returncode != expected_code: - raise AssertionError( - f"Expected `k8s bootstrap` to exit with code {expected_code}, " - f"but it exited with {proc.returncode}.\n" - f"Stdout was: \n{proc.stdout}.\nStderr was: \n{proc.stderr}" - ) + if expected_message not in proc.stderr: + raise AssertionError( + f"Expected to find socket-related warning '{expected_message}' in " + "stderr of the `k8s bootstrap` command.\n" + f"Stdout was: \n{proc.stdout}.\nStderr was: \n{proc.stderr}" + ) - if expected_message not in proc.stderr: - raise AssertionError( - f"Expected to find port-related warning '{expected_message}' in " - "stderr of the `k8s bootstrap` command.\n" - f"Stdout was: \n{proc.stdout}.\nStderr was: \n{proc.stderr}" - ) + _assert_paths_not_exist(instance, CONTAINERD_PATHS) - _assert_paths_not_exist(instance, CONTAINERD_PATHS) + # Remove the directory and ensure the bootstrap succeeds: + instance.exec(["rmdir", CONTAINERD_SOCKET_DIRECTORY_CLASSIC], check=True) + instance.exec(["k8s", "bootstrap"]) diff --git a/tests/integration/tests/test_util/util.py b/tests/integration/tests/test_util/util.py index f1d0d31b2..99fa3dfce 100644 --- a/tests/integration/tests/test_util/util.py +++ b/tests/integration/tests/test_util/util.py @@ -1,13 +1,11 @@ # # Copyright 2024 Canonical, Ltd. # -import contextlib import ipaddress import json import logging import re import shlex -import socket import subprocess import urllib.request from datetime import datetime @@ -518,38 +516,6 @@ def find_suitable_cidr(parent_cidr: str, excluded_ips: List[str]): raise RuntimeError("Could not find a suitable CIDR for LoadBalancer services") -@contextlib.contextmanager -def open_port( - port: int, - host: str = "", - address_family: socket.AddressFamily = socket.AF_INET, - socket_kind: socket.SocketKind = socket.SOCK_STREAM, - max_backlogged_connections: int = 0, -): - """Context manager which opens a socket with the given properties - and binds it to the given port. - - Yields the already setup and listening socket object for use. - - By default, it will only allow one single active connection - and instantly refuse any new ones. Use the `max_backlogged_connections` - argument if you'd like it to accept more connections as `pending`. - """ - sock = socket.socket(family=address_family, type=socket_kind) - if not host: - host = socket.gethostname() - sock.bind((host, port)) - LOG.info(f"Successfully bound new socket on '{host}:{port}'") - - try: - sock.listen(max_backlogged_connections) - LOG.info(f"Successfully started listening on '{host}:{port}'") - yield sock - finally: - sock.close() - LOG.info(f"Closed socket on '{host}:{port}'") - - def check_file_paths_exist( instance: harness.Instance, paths: List[str] ) -> Mapping[str, bool]: