From 2ae439ec52c7bb97c33e68ab8293a312af4373c5 Mon Sep 17 00:00:00 2001 From: Qubasa Date: Wed, 10 Jan 2024 19:29:16 +0100 Subject: [PATCH 1/6] cmd.py refactor part 4 --- pkgs/clan-cli/clan_cli/cmd.py | 2 ++ pkgs/clan-cli/clan_cli/config/__init__.py | 20 ++++++-------------- pkgs/clan-cli/clan_cli/config/schema.py | 15 +++++---------- pkgs/clan-cli/clan_cli/errors.py | 2 ++ pkgs/clan-cli/clan_cli/machines/list.py | 17 +++-------------- pkgs/clan-cli/clan_cli/machines/machines.py | 5 ++--- pkgs/clan-cli/clan_cli/secrets/generate.py | 12 ++++-------- 7 files changed, 24 insertions(+), 49 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/cmd.py b/pkgs/clan-cli/clan_cli/cmd.py index 5c7f8a4d..501ddfec 100644 --- a/pkgs/clan-cli/clan_cli/cmd.py +++ b/pkgs/clan-cli/clan_cli/cmd.py @@ -55,6 +55,7 @@ def run( cwd: Path = Path.cwd(), log: Log = Log.STDERR, check: bool = True, + error_msg: str | None = None, ) -> CmdOut: # Start the subprocess process = subprocess.Popen( @@ -76,6 +77,7 @@ def run( cwd=cwd, command=shlex.join(cmd), returncode=process.returncode, + msg=error_msg, ) if check and rc != 0: diff --git a/pkgs/clan-cli/clan_cli/config/__init__.py b/pkgs/clan-cli/clan_cli/config/__init__.py index 9ef8940c..b2e1585f 100644 --- a/pkgs/clan-cli/clan_cli/config/__init__.py +++ b/pkgs/clan-cli/clan_cli/config/__init__.py @@ -4,12 +4,11 @@ import json import logging import os import re -import shlex -import subprocess import sys from pathlib import Path from typing import Any, get_origin +from clan_cli.cmd import run from clan_cli.dirs import machine_settings_file from clan_cli.errors import ClanError from clan_cli.git import commit_file @@ -117,15 +116,11 @@ def options_for_machine( f"{clan_dir}#nixosConfigurations.{machine_name}.config.clanCore.optionsNix" ) cmd = nix_eval(flags=flags) - proc = subprocess.run( + proc = run( cmd, - stdout=subprocess.PIPE, - text=True, + error_msg=f"Failed to read options for machine {machine_name}", ) - if proc.returncode != 0: - raise ClanError( - f"Failed to read options for machine {machine_name}:\n{shlex.join(cmd)}\nexit with {proc.returncode}" - ) + return json.loads(proc.stdout) @@ -141,11 +136,8 @@ def read_machine_option_value( f"{clan_dir}#nixosConfigurations.{machine_name}.config.{option}", ], ) - proc = subprocess.run(cmd, stdout=subprocess.PIPE, text=True) - if proc.returncode != 0: - raise ClanError( - f"Failed to read option {option}:\n{shlex.join(cmd)}\nexit with {proc.returncode}" - ) + proc = run(cmd, error_msg=f"Failed to read option {option}") + value = json.loads(proc.stdout) # print the value so that the output can be copied and fed as an input. # for example a list should be displayed as space separated values surrounded by quotes. diff --git a/pkgs/clan-cli/clan_cli/config/schema.py b/pkgs/clan-cli/clan_cli/config/schema.py index a709068c..b4fe71d7 100644 --- a/pkgs/clan-cli/clan_cli/config/schema.py +++ b/pkgs/clan-cli/clan_cli/config/schema.py @@ -1,10 +1,9 @@ import json import os -import subprocess -import sys from pathlib import Path from tempfile import NamedTemporaryFile +from clan_cli.cmd import run from clan_cli.dirs import nixpkgs_source from clan_cli.errors import ClanError, ClanHttpError from clan_cli.nix import nix_eval @@ -25,7 +24,7 @@ def machine_schema( clan_machine_settings_file.seek(0) env["CLAN_MACHINE_SETTINGS_FILE"] = clan_machine_settings_file.name # ensure that the requested clanImports exist - proc = subprocess.run( + proc = run( nix_eval( flags=[ "--impure", @@ -47,13 +46,11 @@ def machine_schema( """, ] ), - capture_output=True, - text=True, cwd=flake_dir, env=env, + check=False, ) if proc.returncode != 0: - print(proc.stderr, file=sys.stderr) raise ClanHttpError( status_code=400, msg=f"Failed to check clanImports for existence:\n{proc.stderr}", @@ -65,7 +62,7 @@ def machine_schema( ) # get the schema - proc = subprocess.run( + proc = run( nix_eval( flags=[ "--impure", @@ -100,12 +97,10 @@ def machine_schema( """, ], ), - capture_output=True, - text=True, + check=False, cwd=flake_dir, env=env, ) if proc.returncode != 0: - print(proc.stderr, file=sys.stderr) raise ClanError(f"Failed to read schema:\n{proc.stderr}") return json.loads(proc.stdout) diff --git a/pkgs/clan-cli/clan_cli/errors.py b/pkgs/clan-cli/clan_cli/errors.py index f3f2b4a7..f1b7395d 100644 --- a/pkgs/clan-cli/clan_cli/errors.py +++ b/pkgs/clan-cli/clan_cli/errors.py @@ -8,9 +8,11 @@ class CmdOut(NamedTuple): cwd: Path command: str returncode: int + msg: str | None = None def __str__(self) -> str: return f""" +Message: {self.msg} Working Directory: '{self.cwd}' Return Code: {self.returncode} =================== Command =================== diff --git a/pkgs/clan-cli/clan_cli/machines/list.py b/pkgs/clan-cli/clan_cli/machines/list.py index 18e7a8d3..83ce2d66 100644 --- a/pkgs/clan-cli/clan_cli/machines/list.py +++ b/pkgs/clan-cli/clan_cli/machines/list.py @@ -1,11 +1,9 @@ import argparse import json import logging -import shlex -import subprocess from pathlib import Path -from ..errors import ClanError +from ..cmd import run from ..nix import nix_config, nix_eval log = logging.getLogger(__name__) @@ -22,17 +20,8 @@ def list_machines(flake_url: Path | str) -> list[str]: "--json", ] ) - proc = subprocess.run(cmd, text=True, stdout=subprocess.PIPE) - assert proc.stdout is not None - if proc.returncode != 0: - raise ClanError( - f""" -command: {shlex.join(cmd)} -exit code: {proc.returncode} -stdout: -{proc.stdout} -""" - ) + proc = run(cmd) + res = proc.stdout.strip() return json.loads(res) diff --git a/pkgs/clan-cli/clan_cli/machines/machines.py b/pkgs/clan-cli/clan_cli/machines/machines.py index 88f63bb8..a1f0b872 100644 --- a/pkgs/clan-cli/clan_cli/machines/machines.py +++ b/pkgs/clan-cli/clan_cli/machines/machines.py @@ -1,6 +1,5 @@ import json import os -import subprocess import sys from pathlib import Path @@ -70,10 +69,10 @@ class Machine: ) # TODO do this in the clanCore module env["SECRETS_DIR"] = str(secrets_dir) print(f"uploading secrets... {self.upload_secrets}") - proc = subprocess.run( + proc = run( [self.upload_secrets], env=env, - text=True, + check=False, ) if proc.returncode == 23: diff --git a/pkgs/clan-cli/clan_cli/secrets/generate.py b/pkgs/clan-cli/clan_cli/secrets/generate.py index 46813a60..9d07171d 100644 --- a/pkgs/clan-cli/clan_cli/secrets/generate.py +++ b/pkgs/clan-cli/clan_cli/secrets/generate.py @@ -1,11 +1,9 @@ import argparse import logging import os -import subprocess import sys -from clan_cli.errors import ClanError - +from ..cmd import run from ..machines.machines import Machine log = logging.getLogger(__name__) @@ -17,15 +15,13 @@ def generate_secrets(machine: Machine) -> None: env["PYTHONPATH"] = ":".join(sys.path) # TODO do this in the clanCore module print(f"generating secrets... {machine.generate_secrets}") - proc = subprocess.run( + run( [machine.generate_secrets], env=env, + error_msg="failed to generate secrets", ) - if proc.returncode != 0: - raise ClanError("failed to generate secrets") - else: - print("successfully generated secrets") + print("successfully generated secrets") def generate_command(args: argparse.Namespace) -> None: From 0133ccd5f72135b9b024a907e375e3de9df3dc84 Mon Sep 17 00:00:00 2001 From: Qubasa Date: Wed, 10 Jan 2024 20:33:41 +0100 Subject: [PATCH 2/6] Fixed missing log.BOTH and error_msg at prev refactors --- pkgs/clan-cli/clan_cli/config/machine.py | 3 ++- pkgs/clan-cli/clan_cli/git.py | 22 ++++++--------------- pkgs/clan-cli/clan_cli/machines/machines.py | 1 + pkgs/clan-cli/clan_cli/secrets/generate.py | 3 ++- 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/config/machine.py b/pkgs/clan-cli/clan_cli/config/machine.py index 9c406fd3..fa074f38 100644 --- a/pkgs/clan-cli/clan_cli/config/machine.py +++ b/pkgs/clan-cli/clan_cli/config/machine.py @@ -4,7 +4,7 @@ import re from pathlib import Path from tempfile import NamedTemporaryFile -from clan_cli.cmd import run +from clan_cli.cmd import Log, run from clan_cli.dirs import machine_settings_file, nixpkgs_source, specific_machine_dir from clan_cli.errors import ClanError, ClanHttpError from clan_cli.git import commit_file @@ -65,6 +65,7 @@ def verify_machine_config( cmd, cwd=flake, env=env, + log=Log.BOTH, ) if proc.returncode != 0: return proc.stderr diff --git a/pkgs/clan-cli/clan_cli/git.py b/pkgs/clan-cli/clan_cli/git.py index 97758101..7007413a 100644 --- a/pkgs/clan-cli/clan_cli/git.py +++ b/pkgs/clan-cli/clan_cli/git.py @@ -1,10 +1,10 @@ from pathlib import Path # from clan_cli.dirs import find_git_repo_root -from clan_cli.errors import ClanCmdError, ClanError +from clan_cli.errors import ClanError from clan_cli.nix import nix_shell -from .cmd import run +from .cmd import Log, run # generic vcs agnostic commit function @@ -42,12 +42,8 @@ def _commit_file_to_git(repo_dir: Path, file_path: Path, commit_message: str) -> ["git", "-C", str(repo_dir), "add", str(file_path)], ) # add the file to the git index - try: - run(cmd) - except ClanCmdError as e: - raise ClanError( - f"Failed to add {file_path} to git repository {repo_dir}:\n{e.cmd.command}\n exited with {e.cmd.returncode}" - ) from e + + run(cmd, log=Log.BOTH, error_msg=f"Failed to add {file_path} file to git index") # check if there is a diff cmd = nix_shell( @@ -72,11 +68,5 @@ def _commit_file_to_git(repo_dir: Path, file_path: Path, commit_message: str) -> str(file_path.relative_to(repo_dir)), ], ) - try: - run( - cmd, - ) - except ClanCmdError as e: - raise ClanError( - f"Failed to commit {file_path} to git repository {repo_dir}:\n{e.cmd.command}\n exited with {e.cmd.returncode}" - ) from e + + run(cmd, error_msg=f"Failed to commit {file_path} to git repository {repo_dir}") diff --git a/pkgs/clan-cli/clan_cli/machines/machines.py b/pkgs/clan-cli/clan_cli/machines/machines.py index a1f0b872..e640967d 100644 --- a/pkgs/clan-cli/clan_cli/machines/machines.py +++ b/pkgs/clan-cli/clan_cli/machines/machines.py @@ -18,6 +18,7 @@ def build_machine_data(machine_name: str, clan_dir: Path) -> dict: f'{clan_dir}#clanInternals.machines."{system}"."{machine_name}".config.system.clan.deployment.file' ] ), + error_msg="failed to build machine data", ) return json.loads(Path(proc.stdout.strip()).read_text()) diff --git a/pkgs/clan-cli/clan_cli/secrets/generate.py b/pkgs/clan-cli/clan_cli/secrets/generate.py index 9d07171d..88f5b02f 100644 --- a/pkgs/clan-cli/clan_cli/secrets/generate.py +++ b/pkgs/clan-cli/clan_cli/secrets/generate.py @@ -3,7 +3,7 @@ import logging import os import sys -from ..cmd import run +from ..cmd import Log, run from ..machines.machines import Machine log = logging.getLogger(__name__) @@ -19,6 +19,7 @@ def generate_secrets(machine: Machine) -> None: [machine.generate_secrets], env=env, error_msg="failed to generate secrets", + log=Log.BOTH, ) print("successfully generated secrets") From 16b043f50801c9b87da7e1c4eef61ddacc988ac1 Mon Sep 17 00:00:00 2001 From: Qubasa Date: Wed, 10 Jan 2024 20:52:14 +0100 Subject: [PATCH 3/6] cmd.py: Fixing bug: missing output because of forgotten flush() call --- pkgs/clan-cli/clan_cli/cmd.py | 2 ++ pkgs/clan-cli/clan_cli/machines/machines.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pkgs/clan-cli/clan_cli/cmd.py b/pkgs/clan-cli/clan_cli/cmd.py index 501ddfec..16a4a0fa 100644 --- a/pkgs/clan-cli/clan_cli/cmd.py +++ b/pkgs/clan-cli/clan_cli/cmd.py @@ -38,12 +38,14 @@ def handle_output(process: subprocess.Popen, log: Log) -> tuple[str, str]: ret = handle_fd(process.stdout) if log in [Log.STDOUT, Log.BOTH]: sys.stdout.buffer.write(ret) + sys.stdout.flush() stdout_buf += ret ret = handle_fd(process.stderr) if log in [Log.STDERR, Log.BOTH]: sys.stderr.buffer.write(ret) + sys.stderr.flush() stderr_buf += ret return stdout_buf.decode("utf-8"), stderr_buf.decode("utf-8") diff --git a/pkgs/clan-cli/clan_cli/machines/machines.py b/pkgs/clan-cli/clan_cli/machines/machines.py index e640967d..d8bd0be1 100644 --- a/pkgs/clan-cli/clan_cli/machines/machines.py +++ b/pkgs/clan-cli/clan_cli/machines/machines.py @@ -3,7 +3,7 @@ import os import sys from pathlib import Path -from ..cmd import run +from ..cmd import Log, run from ..nix import nix_build, nix_config, nix_eval from ..ssh import Host, parse_deployment_address @@ -18,6 +18,7 @@ def build_machine_data(machine_name: str, clan_dir: Path) -> dict: f'{clan_dir}#clanInternals.machines."{system}"."{machine_name}".config.system.clan.deployment.file' ] ), + log=Log.BOTH, error_msg="failed to build machine data", ) From f7c6ab5888c5763c1ec5e4326f452946d915b1f6 Mon Sep 17 00:00:00 2001 From: Qubasa Date: Thu, 11 Jan 2024 21:48:39 +0100 Subject: [PATCH 4/6] Working test_secrets_generate --- checks/impure/flake-module.nix | 2 +- pkgs/clan-cli/clan_cli/__init__.py | 1 + pkgs/clan-cli/clan_cli/cmd.py | 4 +++- pkgs/clan-cli/clan_cli/custom_logger.py | 19 +++++++++++++------ pkgs/clan-cli/tests/test_secrets_generate.py | 3 ++- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/checks/impure/flake-module.nix b/checks/impure/flake-module.nix index b0865c6f..faeca31e 100644 --- a/checks/impure/flake-module.nix +++ b/checks/impure/flake-module.nix @@ -12,7 +12,7 @@ ]}" ROOT=$(git rev-parse --show-toplevel) cd "$ROOT/pkgs/clan-cli" - nix develop "$ROOT#clan-cli" -c bash -c "TMPDIR=/tmp python -m pytest -m impure ./tests $@" + nix develop "$ROOT#clan-cli" -c bash -c "TMPDIR=/tmp python -m pytest -s -m impure ./tests $@" ''; }; } diff --git a/pkgs/clan-cli/clan_cli/__init__.py b/pkgs/clan-cli/clan_cli/__init__.py index 20e83461..5948035d 100644 --- a/pkgs/clan-cli/clan_cli/__init__.py +++ b/pkgs/clan-cli/clan_cli/__init__.py @@ -124,6 +124,7 @@ def main() -> None: if args.debug: setup_logging(logging.DEBUG) log.debug("Debug log activated") + setup_logging(logging.INFO) if not hasattr(args, "func"): return diff --git a/pkgs/clan-cli/clan_cli/cmd.py b/pkgs/clan-cli/clan_cli/cmd.py index 16a4a0fa..450d131d 100644 --- a/pkgs/clan-cli/clan_cli/cmd.py +++ b/pkgs/clan-cli/clan_cli/cmd.py @@ -8,9 +8,10 @@ from enum import Enum from pathlib import Path from typing import IO, Any +from .custom_logger import get_caller from .errors import ClanCmdError, CmdOut -log = logging.getLogger(__name__) +glog = logging.getLogger(__name__) class Log(Enum): @@ -59,6 +60,7 @@ def run( check: bool = True, error_msg: str | None = None, ) -> CmdOut: + glog.debug(f"running command: {shlex.join(cmd)}. Caller: {get_caller()}") # Start the subprocess process = subprocess.Popen( cmd, diff --git a/pkgs/clan-cli/clan_cli/custom_logger.py b/pkgs/clan-cli/clan_cli/custom_logger.py index 2b62be6d..d6c85427 100644 --- a/pkgs/clan-cli/clan_cli/custom_logger.py +++ b/pkgs/clan-cli/clan_cli/custom_logger.py @@ -63,11 +63,18 @@ def get_caller() -> str: def setup_logging(level: Any) -> None: - handler = logging.StreamHandler() - handler.setLevel(level) - handler.setFormatter(CustomFormatter()) - logger = logging.getLogger("registerHandler") + # Get the root logger and set its level + root_logger = logging.getLogger() + root_logger.setLevel(level) + + # Create and add the default handler + default_handler = logging.StreamHandler() + + # Create and add your custom handler + default_handler.setLevel(level) + default_handler.setFormatter(CustomFormatter()) + root_logger.addHandler(default_handler) + + # Set logging level for other modules used by this module logging.getLogger("asyncio").setLevel(logging.INFO) logging.getLogger("httpx").setLevel(level=logging.WARNING) - logger.addHandler(handler) - # logging.basicConfig(level=level, handlers=[handler]) diff --git a/pkgs/clan-cli/tests/test_secrets_generate.py b/pkgs/clan-cli/tests/test_secrets_generate.py index 94b6f836..d9605fe1 100644 --- a/pkgs/clan-cli/tests/test_secrets_generate.py +++ b/pkgs/clan-cli/tests/test_secrets_generate.py @@ -33,7 +33,8 @@ def test_generate_secret( age_keys[0].pubkey, ] ) - cli.run(["--flake", str(test_flake_with_core.path), "secrets", "generate", "vm1"]) + cmd = ["--flake", str(test_flake_with_core.path), "secrets", "generate", "vm1"] + cli.run(cmd) has_secret(test_flake_with_core.path, "vm1-age.key") has_secret(test_flake_with_core.path, "vm1-zerotier-identity-secret") has_secret(test_flake_with_core.path, "vm1-zerotier-subnet") From 4d4c09da802fa1bba4ea47708ffbbeb7d2ec97aa Mon Sep 17 00:00:00 2001 From: Qubasa Date: Thu, 11 Jan 2024 22:14:55 +0100 Subject: [PATCH 5/6] Enabled logging DEBUG in pytest --- pkgs/clan-cli/clan_cli/__init__.py | 3 ++- pkgs/clan-cli/tests/helpers/cli.py | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/__init__.py b/pkgs/clan-cli/clan_cli/__init__.py index 5948035d..017f1719 100644 --- a/pkgs/clan-cli/clan_cli/__init__.py +++ b/pkgs/clan-cli/clan_cli/__init__.py @@ -124,7 +124,8 @@ def main() -> None: if args.debug: setup_logging(logging.DEBUG) log.debug("Debug log activated") - setup_logging(logging.INFO) + else: + setup_logging(logging.INFO) if not hasattr(args, "func"): return diff --git a/pkgs/clan-cli/tests/helpers/cli.py b/pkgs/clan-cli/tests/helpers/cli.py index 699325cd..c6fdd386 100644 --- a/pkgs/clan-cli/tests/helpers/cli.py +++ b/pkgs/clan-cli/tests/helpers/cli.py @@ -3,7 +3,7 @@ import logging import shlex from clan_cli import create_parser -from clan_cli.custom_logger import get_caller +from clan_cli.custom_logger import get_caller, setup_logging log = logging.getLogger(__name__) @@ -11,10 +11,11 @@ log = logging.getLogger(__name__) class Cli: def run(self, args: list[str]) -> argparse.Namespace: parser = create_parser(prog="clan") - cmd = shlex.join(["clan", *args]) + parsed = parser.parse_args(args) + setup_logging(logging.DEBUG) + cmd = shlex.join(["clan", "--debug", *args]) log.debug(f"$ {cmd}") log.debug(f"Caller {get_caller()}") - parsed = parser.parse_args(args) if hasattr(parsed, "func"): parsed.func(parsed) return parsed From d1ca0eaf80c279b6eec56906212ca4b2e70c9369 Mon Sep 17 00:00:00 2001 From: Qubasa Date: Thu, 11 Jan 2024 22:28:35 +0100 Subject: [PATCH 6/6] Identified deadlocking funciton --- pkgs/clan-cli/clan_cli/cmd.py | 2 +- pkgs/clan-cli/clan_cli/secrets/generate.py | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/cmd.py b/pkgs/clan-cli/clan_cli/cmd.py index 450d131d..53a5a79c 100644 --- a/pkgs/clan-cli/clan_cli/cmd.py +++ b/pkgs/clan-cli/clan_cli/cmd.py @@ -72,7 +72,7 @@ def run( ) stdout_buf, stderr_buf = handle_output(process, log) - + # stdout_buf, stderr_buf = process.communicate() # Wait for the subprocess to finish rc = process.wait() cmd_out = CmdOut( diff --git a/pkgs/clan-cli/clan_cli/secrets/generate.py b/pkgs/clan-cli/clan_cli/secrets/generate.py index 88f5b02f..46813a60 100644 --- a/pkgs/clan-cli/clan_cli/secrets/generate.py +++ b/pkgs/clan-cli/clan_cli/secrets/generate.py @@ -1,9 +1,11 @@ import argparse import logging import os +import subprocess import sys -from ..cmd import Log, run +from clan_cli.errors import ClanError + from ..machines.machines import Machine log = logging.getLogger(__name__) @@ -15,14 +17,15 @@ def generate_secrets(machine: Machine) -> None: env["PYTHONPATH"] = ":".join(sys.path) # TODO do this in the clanCore module print(f"generating secrets... {machine.generate_secrets}") - run( + proc = subprocess.run( [machine.generate_secrets], env=env, - error_msg="failed to generate secrets", - log=Log.BOTH, ) - print("successfully generated secrets") + if proc.returncode != 0: + raise ClanError("failed to generate secrets") + else: + print("successfully generated secrets") def generate_command(args: argparse.Namespace) -> None: