From 9756a0f75f178df131b89e559e2764c5aefbc4ee Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 26 Aug 2025 11:15:59 +0200 Subject: [PATCH] Extend repair scripts with env-file support and unit tests - Added detect_env_file() to both sys-ctl-rpr-docker-soft and sys-ctl-rpr-docker-hard * prefer .env, fallback to .env/env * append --env-file parameter automatically - Refactored soft script to use compose_cmd() for consistent command building - Adjusted error recovery path in soft script to also respect env-file - Extended unit tests for soft script to cover env-file priority and restart commands - Added new unit tests for hard script verifying env-file priority, cwd handling, and --only filter logic Ref: https://chatgpt.com/share/68ad7b30-7510-800f-8172-56f03a2f40f5 --- roles/sys-ctl-rpr-docker-hard/files/script.py | 32 +++- roles/sys-ctl-rpr-docker-soft/files/script.py | 36 ++++- .../roles/sys-ctl-rpr-docker-hard/__init__.py | 0 .../sys-ctl-rpr-docker-hard/files/__init__.py | 0 .../files/test_script.py | 140 ++++++++++++++++++ .../files/test_script.py | 72 ++++++++- 6 files changed, 269 insertions(+), 11 deletions(-) create mode 100644 tests/unit/roles/sys-ctl-rpr-docker-hard/__init__.py create mode 100644 tests/unit/roles/sys-ctl-rpr-docker-hard/files/__init__.py create mode 100644 tests/unit/roles/sys-ctl-rpr-docker-hard/files/test_script.py diff --git a/roles/sys-ctl-rpr-docker-hard/files/script.py b/roles/sys-ctl-rpr-docker-hard/files/script.py index 2471285e..a014e343 100644 --- a/roles/sys-ctl-rpr-docker-hard/files/script.py +++ b/roles/sys-ctl-rpr-docker-hard/files/script.py @@ -3,15 +3,41 @@ import sys import subprocess import argparse + +def detect_env_file(dir_path: str) -> str | None: + """ + Return the path to a Compose env file if present (.env preferred, fallback to env). + """ + candidates = [os.path.join(dir_path, ".env"), os.path.join(dir_path, ".env", "env")] + for candidate in candidates: + if os.path.isfile(candidate): + return candidate + return None + + def hard_restart_docker_services(dir_path): """ Perform a hard restart of docker-compose services in the given directory - using docker-compose down and docker-compose up -d. + using docker-compose down and docker-compose up -d, adding --env-file if present. """ try: print(f"Performing hard restart for docker-compose services in: {dir_path}") - subprocess.run(["docker-compose", "down"], cwd=dir_path, check=True) - subprocess.run(["docker-compose", "up", "-d"], cwd=dir_path, check=True) + + env_file = detect_env_file(dir_path) + base = ["docker-compose"] + down_cmd = base.copy() + up_cmd = base.copy() + + if env_file: + down_cmd += ["--env-file", env_file] + up_cmd += ["--env-file", env_file] + + down_cmd += ["down"] + up_cmd += ["up", "-d"] + + subprocess.run(down_cmd, cwd=dir_path, check=True) + subprocess.run(up_cmd, cwd=dir_path, check=True) + print(f"Hard restart completed successfully in: {dir_path}") except subprocess.CalledProcessError as e: print(f"Error during hard restart in {dir_path}: {e}") diff --git a/roles/sys-ctl-rpr-docker-soft/files/script.py b/roles/sys-ctl-rpr-docker-soft/files/script.py index f9f80021..6e7179e9 100644 --- a/roles/sys-ctl-rpr-docker-soft/files/script.py +++ b/roles/sys-ctl-rpr-docker-soft/files/script.py @@ -43,6 +43,32 @@ def find_docker_compose_file(directory: str) -> str | None: return None +def detect_env_file(project_path: str) -> str | None: + """ + Return the path to a Compose env file if present (.env preferred, fallback to env). + """ + candidates = [os.path.join(project_path, ".env"), os.path.join(project_path, ".env", "env")] + for candidate in candidates: + if os.path.isfile(candidate): + return candidate + return None + + +def compose_cmd(subcmd: str, project_path: str, project_name: str | None = None) -> str: + """ + Build a docker-compose command string with optional -p and --env-file if present. + Example: compose_cmd("restart", "/opt/docker/foo", "foo") + """ + parts: List[str] = [f'cd "{project_path}" && docker-compose'] + if project_name: + parts += ['-p', f'"{project_name}"'] + env_file = detect_env_file(project_path) + if env_file: + parts += ['--env-file', f'"{env_file}"'] + parts += subcmd.split() + return " ".join(parts) + + def normalize_services_arg(raw: List[str] | None, raw_str: str | None) -> List[str]: """ Accept either: @@ -57,6 +83,7 @@ def normalize_services_arg(raw: List[str] | None, raw_str: str | None) -> List[s return [p for p in parts if p] return [] + def wait_while_manipulation_running( services: List[str], waiting_time: int = 600, @@ -91,6 +118,7 @@ def wait_while_manipulation_running( print("No blocking service is running.") break + def main(base_directory: str, manipulation_services: List[str], timeout: int | None) -> int: errors = 0 wait_while_manipulation_running(manipulation_services, waiting_time=600, timeout=timeout) @@ -117,13 +145,15 @@ def main(base_directory: str, manipulation_services: List[str], timeout: int | N print("Restarting unhealthy container in:", compose_file_path) project_path = os.path.dirname(compose_file_path) try: - print_bash(f'cd "{project_path}" && docker-compose -p "{repo}" restart') + # restart with optional --env-file and -p + print_bash(compose_cmd("restart", project_path, repo)) except Exception as e: if "port is already allocated" in str(e): print("Detected port allocation problem. Executing recovery steps...") - print_bash(f'cd "{project_path}" && docker-compose down') + # down (no -p needed), then engine restart, then up -d with -p + print_bash(compose_cmd("down", project_path)) print_bash("systemctl restart docker") - print_bash(f'cd "{project_path}" && docker-compose -p "{repo}" up -d') + print_bash(compose_cmd("up -d", project_path, repo)) else: print("Unhandled exception during restart:", e) errors += 1 diff --git a/tests/unit/roles/sys-ctl-rpr-docker-hard/__init__.py b/tests/unit/roles/sys-ctl-rpr-docker-hard/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/roles/sys-ctl-rpr-docker-hard/files/__init__.py b/tests/unit/roles/sys-ctl-rpr-docker-hard/files/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/roles/sys-ctl-rpr-docker-hard/files/test_script.py b/tests/unit/roles/sys-ctl-rpr-docker-hard/files/test_script.py new file mode 100644 index 00000000..922c9d61 --- /dev/null +++ b/tests/unit/roles/sys-ctl-rpr-docker-hard/files/test_script.py @@ -0,0 +1,140 @@ +import unittest +import sys +from pathlib import Path +from importlib.util import spec_from_file_location, module_from_spec + + +def load_script_module(): + """ + Import the script under test from roles/sys-ctl-rpr-docker-hard/files/script.py + """ + test_file = Path(__file__).resolve() + repo_root = test_file.parents[5] # .../tests/unit/roles/sys-ctl-rpr-docker-hard/files -> repo root + script_path = repo_root / "roles" / "sys-ctl-rpr-docker-hard" / "files" / "script.py" + if not script_path.exists(): + raise FileNotFoundError(f"script.py not found at {script_path}") + spec = spec_from_file_location("rpr_hard_script", str(script_path)) + mod = module_from_spec(spec) + assert spec.loader is not None + spec.loader.exec_module(mod) # type: ignore[attr-defined] + return mod + + +class TestRepairDockerHard(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.script = load_script_module() + + def test_detect_env_file_priority(self): + s = self.script + base = "/proj" + old_isfile = s.os.path.isfile + try: + # only .env + s.os.path.isfile = lambda p: p == f"{base}/.env" + self.assertEqual(s.detect_env_file(base), f"{base}/.env") + + # only .env/env + s.os.path.isfile = lambda p: p == f"{base}/.env/env" + self.assertEqual(s.detect_env_file(base), f"{base}/.env/env") + + # both -> prefer .env + s.os.path.isfile = lambda p: p in (f"{base}/.env", f"{base}/.env/env") + self.assertEqual(s.detect_env_file(base), f"{base}/.env") + + # none + s.os.path.isfile = lambda p: False + self.assertIsNone(s.detect_env_file(base)) + finally: + s.os.path.isfile = old_isfile + + def test_hard_restart_uses_envfile_and_cwd(self): + s = self.script + calls = [] + + def fake_run(cmd, cwd=None, check=None): + calls.append({"cmd": cmd, "cwd": cwd, "check": check}) + class R: pass + return R() + + old_run = s.subprocess.run + old_detect = s.detect_env_file + try: + s.subprocess.run = fake_run + s.detect_env_file = lambda d: f"{d}/.env/env" # erzwinge .env/env + + s.hard_restart_docker_services("/X/APP") + + # Wir erwarten zwei Aufrufe: docker-compose --env-file ... down / up -d + self.assertEqual(len(calls), 2) + self.assertEqual(calls[0]["cwd"], "/X/APP") + self.assertEqual(calls[1]["cwd"], "/X/APP") + # down + self.assertIn("docker-compose", calls[0]["cmd"]) + self.assertIn("--env-file", calls[0]["cmd"]) + self.assertIn("/X/APP/.env/env", calls[0]["cmd"]) + self.assertIn("down", calls[0]["cmd"]) + # up -d + self.assertIn("docker-compose", calls[1]["cmd"]) + self.assertIn("--env-file", calls[1]["cmd"]) + self.assertIn("/X/APP/.env/env", calls[1]["cmd"]) + self.assertIn("up", calls[1]["cmd"]) + self.assertIn("-d", calls[1]["cmd"]) + finally: + s.subprocess.run = old_run + s.detect_env_file = old_detect + + def test_main_scans_parent_and_filters_only(self): + s = self.script + seen = {"scandir": [], "called": []} + + class FakeDirEntry: + def __init__(self, path, is_dir=True): + self.path = path + self._is_dir = is_dir + def is_dir(self): + return self._is_dir + + def fake_scandir(parent): + seen["scandir"].append(parent) + return [ + FakeDirEntry(f"{parent}/app1"), + FakeDirEntry(f"{parent}/app2"), + FakeDirEntry(f"{parent}/notdir", is_dir=False), + ] + + def fake_isdir(p): + return p == "/PARENT" + + def fake_isfile(p): + # Nur app2 hat docker-compose.yml + return p in ("/PARENT/app2/docker-compose.yml",) + + def fake_hard_restart(dir_path): + seen["called"].append(dir_path) + + old_scandir = s.os.scandir + old_isdir = s.os.path.isdir + old_isfile = s.os.path.isfile + old_restart = s.hard_restart_docker_services + try: + s.os.scandir = fake_scandir + s.os.path.isdir = fake_isdir + s.os.path.isfile = fake_isfile + s.hard_restart_docker_services = fake_hard_restart + + # Mit --only app2 -> nur app2 wird aufgerufen + sys_argv = sys.argv + sys.argv = ["x", "/PARENT", "--only", "app2"] + s.main() + self.assertEqual(seen["called"], ["/PARENT/app2"]) + finally: + s.os.scandir = old_scandir + s.os.path.isdir = old_isdir + s.os.path.isfile = old_isfile + s.hard_restart_docker_services = old_restart + sys.argv = sys_argv + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/roles/sys-ctl-rpr-docker-soft/files/test_script.py b/tests/unit/roles/sys-ctl-rpr-docker-soft/files/test_script.py index 8d49e48d..003ccb39 100644 --- a/tests/unit/roles/sys-ctl-rpr-docker-soft/files/test_script.py +++ b/tests/unit/roles/sys-ctl-rpr-docker-soft/files/test_script.py @@ -42,6 +42,38 @@ class TestRepairDockerSoft(unittest.TestCase): ) self.assertEqual(s.normalize_services_arg([], ""), []) + def test_detect_env_file_priority(self): + s = self.script + base = "/proj" + # Monkeypatch os.path.isfile + old_isfile = s.os.path.isfile + try: + def fake_isfile(path): + # Only .env exists + return path == f"{base}/.env" + s.os.path.isfile = fake_isfile + self.assertEqual(s.detect_env_file(base), f"{base}/.env") + + # Only .env/env exists + def fake_isfile2(path): + return path == f"{base}/.env/env" + s.os.path.isfile = fake_isfile2 + self.assertEqual(s.detect_env_file(base), f"{base}/.env/env") + + # Both exist -> prefer .env + def fake_isfile3(path): + return path in (f"{base}/.env", f"{base}/.env/env") + s.os.path.isfile = fake_isfile3 + self.assertEqual(s.detect_env_file(base), f"{base}/.env") + + # Neither exists + def fake_isfile4(path): + return False + s.os.path.isfile = fake_isfile4 + self.assertIsNone(s.detect_env_file(base)) + finally: + s.os.path.isfile = old_isfile + def test_wait_while_manipulation_running_respects_timeout(self): s = self.script calls = {"checks": 0, "sleeps": 0} @@ -77,7 +109,7 @@ class TestRepairDockerSoft(unittest.TestCase): s.time.sleep = old_sleep s.time.time = old_time - def test_main_restarts_and_counts_errors(self): + def test_main_restarts_and_counts_errors_and_envfile_usage(self): s = self.script cmd_log = [] @@ -92,25 +124,55 @@ class TestRepairDockerSoft(unittest.TestCase): return [] def fake_find_docker_compose(path): + # Compose-Projekte: app1, db -> vorhanden; "other" -> nicht vorhanden if path.endswith("/app1") or path.endswith("/db"): return str(Path(path) / "docker-compose.yml") return None + # Steuere die detect_env_file-Antwort: + # - Für app1 existiert nur .env/env + # - Für db existiert .env + def fake_detect_env_file(project_path: str): + if project_path.endswith("/app1"): + return f"{project_path}/.env/env" + if project_path.endswith("/db"): + return f"{project_path}/.env" + return None + old_print_bash = s.print_bash old_find = s.find_docker_compose_file + old_detect = s.detect_env_file try: s.print_bash = fake_print_bash - s.find_docker_compose_file = fake_find_docker_compose # <-- jetzt gleicher Name! + s.find_docker_compose_file = fake_find_docker_compose + s.detect_env_file = fake_detect_env_file errors = s.main("/BASE", manipulation_services=[], timeout=None) + # one error expected for "other" (no compose file) self.assertEqual(errors, 1) - restart_cmds = [c for c in cmd_log if "docker-compose -p" in c and " restart" in c] - self.assertTrue(any('cd "/BASE/app1"' in c and 'docker-compose -p "app1" restart' in c for c in restart_cmds)) - self.assertTrue(any('cd "/BASE/db"' in c and 'docker-compose -p "db" restart' in c for c in restart_cmds)) + restart_cmds = [c for c in cmd_log if ' docker-compose' in c and " restart" in c] + # app1: --env-file "/BASE/app1/.env/env" + -p "app1" + self.assertTrue(any( + 'cd "/BASE/app1"' in c and + '--env-file "/BASE/app1/.env/env"' in c and + '-p "app1"' in c and + ' restart' in c + for c in restart_cmds + )) + # db: --env-file "/BASE/db/.env" + -p "db" + self.assertTrue(any( + 'cd "/BASE/db"' in c and + '--env-file "/BASE/db/.env"' in c and + '-p "db"' in c and + ' restart' in c + for c in restart_cmds + )) finally: s.print_bash = old_print_bash s.find_docker_compose_file = old_find + s.detect_env_file = old_detect + if __name__ == "__main__": unittest.main()