From 2a407649c8d1ee06f1a9b545f1ba1fd22c82e3ec Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Tue, 1 Jul 2025 17:35:35 -0700 Subject: [PATCH 01/19] Remove obsolete Snyk reference --- requirements.txt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements.txt b/requirements.txt index 6b13d8f..af3fdda 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ -paramiko -requests -python-dotenv -scp -cryptography>=39.0.1 # not directly required, pinned by Snyk to avoid a vulnerability +paramiko==3.5.1 +requests==2.32.4 +python-dotenv==1.1.1 +scp==0.15.0 +cryptography>=45.0.4 # not directly required, pinned to avoid a vulnerability From 6c774043d75a758a87833ea7461f63cd8ac8345b Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Tue, 1 Jul 2025 17:38:26 -0700 Subject: [PATCH 02/19] Add GitHub Actions build --- .github/workflows/python-build.yml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .github/workflows/python-build.yml diff --git a/.github/workflows/python-build.yml b/.github/workflows/python-build.yml new file mode 100644 index 0000000..ee21bcf --- /dev/null +++ b/.github/workflows/python-build.yml @@ -0,0 +1,21 @@ +name: Build + +on: + push: + branches: ["master", "main"] + pull_request: + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.x' + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + - name: Compile + run: python -m compileall -q zdeploy From 6871a6bdf42a6f839112c83106b476c83e4ee083 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Tue, 1 Jul 2025 23:11:52 -0700 Subject: [PATCH 03/19] Add test configuration --- .github/workflows/python-build.yml | 7 +++++++ README.md | 13 ++++++++++++- TODO | 3 --- requirements.txt | 3 +++ tests/conftest.py | 3 +++ tests/test_config.py | 8 ++++++++ tests/test_shell.py | 6 ++++++ tests/test_utils.py | 4 ++++ 8 files changed, 43 insertions(+), 4 deletions(-) delete mode 100644 TODO create mode 100644 tests/conftest.py create mode 100644 tests/test_config.py create mode 100644 tests/test_shell.py create mode 100644 tests/test_utils.py diff --git a/.github/workflows/python-build.yml b/.github/workflows/python-build.yml index ee21bcf..91659ac 100644 --- a/.github/workflows/python-build.yml +++ b/.github/workflows/python-build.yml @@ -17,5 +17,12 @@ jobs: run: | python -m pip install --upgrade pip pip install -r requirements.txt + pip install pylint pytest pyright + - name: Lint + run: pylint zdeploy + - name: Type check + run: pyright zdeploy + - name: Test + run: pytest - name: Compile run: python -m compileall -q zdeploy diff --git a/README.md b/README.md index 3877878..bac181a 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,18 @@ Here is a list of all supported config parameters to date: | password | Default password (used in case a private key isn't auto-detected). | No | String | None | | port | Default port number (used for recipes that don't specify a port number, i.e. RECIPE_PORT). | No | Integer | 22 | -> NOTE: This table will be updated to always support the most recent release of Zdeploy. +> NOTE: This table will be updated to always support the most recent release of Zdeploy. + +## Development + +Install development tools and run lint, type checks, and the test suite: + +```bash +pip install -r requirements.txt +pylint zdeploy +pyright zdeploy +pytest +``` ## Author [Fadi Hanna Al-Kass](https://github.com/alkass) diff --git a/TODO b/TODO deleted file mode 100644 index 2831134..0000000 --- a/TODO +++ /dev/null @@ -1,3 +0,0 @@ -* hooks -* apps -* links diff --git a/requirements.txt b/requirements.txt index af3fdda..3142875 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,3 +3,6 @@ requests==2.32.4 python-dotenv==1.1.1 scp==0.15.0 cryptography>=45.0.4 # not directly required, pinned to avoid a vulnerability +pylint==3.3.7 +pytest==8.4.1 +pyright==1.1.402 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..8abafde --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,3 @@ +import os +import sys +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))) diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..db3d5d0 --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,8 @@ +from zdeploy import config + +def test_load_defaults(tmp_path): + cfg = config.load(str(tmp_path / 'missing.json')) + assert cfg.configs == 'configs' + assert cfg.recipes == 'recipes' + assert cfg.cache == 'cache' + assert cfg.logs == 'logs' diff --git a/tests/test_shell.py b/tests/test_shell.py new file mode 100644 index 0000000..d1cf9fe --- /dev/null +++ b/tests/test_shell.py @@ -0,0 +1,6 @@ +from zdeploy.shell import execute + +def test_execute_echo(): + output, rc = execute('echo hello') + assert output.strip() == 'hello' + assert rc == 0 diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 0000000..d636322 --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,4 @@ +import zdeploy.utils as utils + +def test_reformat_time(): + assert utils.reformat_time('1:02:03') == '1h, 2m, and 3s' From 6a4ce62b21e2e0fea922a83fc79aa354a8f1c0d3 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Tue, 1 Jul 2025 23:20:40 -0700 Subject: [PATCH 04/19] Add basic docstrings and improve style --- zdeploy/__init__.py | 67 ++++++++++++++++--------- zdeploy/app.py | 110 +++++++++++++++++++++++++++------------- zdeploy/clients.py | 50 ++++++++++++------ zdeploy/config.py | 78 +++++++++++++++-------------- zdeploy/log.py | 32 ++++++++---- zdeploy/recipe.py | 117 +++++++++++++++++++++++++++++++------------ zdeploy/recipeset.py | 45 ++++++++++++----- zdeploy/shell.py | 17 +++++-- zdeploy/utils.py | 9 +++- 9 files changed, 353 insertions(+), 172 deletions(-) diff --git a/zdeploy/__init__.py b/zdeploy/__init__.py index 29b2310..24c2a07 100644 --- a/zdeploy/__init__.py +++ b/zdeploy/__init__.py @@ -1,60 +1,77 @@ +"""Command line interface for zdeploy.""" + from argparse import ArgumentParser -from os.path import isdir from os import listdir, makedirs +from os.path import isdir from sys import stdout from datetime import datetime + from zdeploy.log import Log from zdeploy.app import deploy from zdeploy.config import load as load_config -def str2bool(v): - if isinstance(v, bool): - return v - if v.lower() in ('yes', 'y'): + +def str2bool(value): + """Return a boolean for common yes/no strings.""" + + if isinstance(value, bool): + return value + if value.lower() in ("yes", "y"): return True - elif v.lower() in ('no', 'n'): + if value.lower() in ("no", "n"): return False - raise Exception('Invalid value: %s' % v) + raise Exception(f"Invalid value: {value}") + def handle_config(config_name, args, cfg): - # TODO: document - log_dir_path = '%s/%s' % (cfg.logs, config_name) - cache_dir_path = '%s/%s' % (cfg.cache, config_name) + """Deploy a single configuration.""" + + log_dir_path = f"{cfg.logs}/{config_name}" + cache_dir_path = f"{cfg.cache}/{config_name}" if not isdir(log_dir_path): makedirs(log_dir_path) if not isdir(cache_dir_path): makedirs(cache_dir_path) log = Log() log.register_logger(stdout) - log.register_logger(open('%s/%s.log' % (log_dir_path, '{0:%Y-%m-%d %H:%M:%S}'.format(datetime.now())), 'w')) + log.register_logger( + open( + f"{log_dir_path}/{datetime.now():%Y-%m-%d %H:%M:%S}.log", + "w", + encoding="utf-8", + ) + ) deploy(config_name, cache_dir_path, log, args, cfg) + def handle_configs(args, cfg): - ''' - Iterate over all retrieved configs and deploy them in a pipelined order. - ''' + """Deploy each config provided on the command line.""" for config_name in args.configs: handle_config(config_name, args, cfg) + def main(): + """CLI entry point.""" + # Default config file name is config.json, so it needs not be specified in our case. cfg = load_config() parser = ArgumentParser() parser.add_argument( - '-c', - '--configs', - help='Deployment destination(s)', - nargs='+', + "-c", + "--configs", + help="Deployment destination(s)", + nargs="+", required=True, - choices=listdir(cfg.configs) if isdir(cfg.configs) else ()) + choices=listdir(cfg.configs) if isdir(cfg.configs) else (), + ) parser.add_argument( - '-f', - '--force', - help='Force full deployment (overlooks the cache)', - nargs='?', + "-f", + "--force", + help="Force full deployment (overlooks the cache)", + nargs="?", required=False, - default=cfg.force, # Default behavior can be defined by the user in a config file + default=cfg.force, # Default behavior can be defined by the user in a config file const=True, - type=str2bool + type=str2bool, ) handle_configs(parser.parse_args(), cfg) diff --git a/zdeploy/app.py b/zdeploy/app.py index 8f03390..97c56af 100644 --- a/zdeploy/app.py +++ b/zdeploy/app.py @@ -1,3 +1,5 @@ +"""Deployment core logic.""" + from os import listdir, makedirs, environ from os.path import isdir, isfile from shutil import rmtree @@ -7,25 +9,37 @@ from zdeploy.recipeset import RecipeSet from zdeploy.utils import reformat_time + def deploy(config_name, cache_dir_path, log, args, cfg): - config_path = '%s/%s' % (cfg.configs, config_name) - print('Config:', config_path) + """Deploy recipes defined in ``config_name``.""" + config_path = "%s/%s" % (cfg.configs, config_name) + print("Config:", config_path) load_dotenv(config_path) recipes = RecipeSet(cfg, log) - recipe_names = environ.get('RECIPES') - if recipe_names.startswith('(') and recipe_names.endswith(')'): + recipe_names = environ.get("RECIPES") + if recipe_names.startswith("(") and recipe_names.endswith(")"): recipe_names = recipe_names[1:-1] - for recipe_name in recipe_names.split(' '): + for recipe_name in recipe_names.split(" "): recipe_name = recipe_name.strip() HOST_IP = environ.get(recipe_name) if HOST_IP is None: - log.fatal('%s is undefined in %s' % (recipe_name, config_path)) - HOST_USER = environ.get('%s_USER' % recipe_name, cfg.user) - HOST_PASSWORD = environ.get('%s_PASSWORD' % recipe_name, cfg.password) - HOST_PORT = environ.get('%s_PORT' % recipe_name, cfg.port) - recipe = Recipe(recipe_name, None, config_path, HOST_IP, HOST_USER, HOST_PASSWORD, HOST_PORT, log, cfg) + log.fatal("%s is undefined in %s" % (recipe_name, config_path)) + HOST_USER = environ.get("%s_USER" % recipe_name, cfg.user) + HOST_PASSWORD = environ.get("%s_PASSWORD" % recipe_name, cfg.password) + HOST_PORT = environ.get("%s_PORT" % recipe_name, cfg.port) + recipe = Recipe( + recipe_name, + None, + config_path, + HOST_IP, + HOST_USER, + HOST_PASSWORD, + HOST_PORT, + log, + cfg, + ) for env in environ: if env.startswith(recipe_name) and env != recipe_name: # Properties aren't used anywhere internally. We only @@ -38,44 +52,68 @@ def deploy(config_name, cache_dir_path, log, args, cfg): recipes.add_recipe(recipe) started_all = datetime.now() - log.info('Started %s deployment at %s on %s' % - (config_path, - started_all.strftime('%H:%M:%S'), - started_all.strftime('%Y-%m-%d'))) - deployment_cache_path = '%s/%s' % (cache_dir_path, recipes.get_hash()) + log.info( + "Started %s deployment at %s on %s" + % ( + config_path, + started_all.strftime("%H:%M:%S"), + started_all.strftime("%Y-%m-%d"), + ) + ) + deployment_cache_path = "%s/%s" % (cache_dir_path, recipes.get_hash()) if not isdir(deployment_cache_path): makedirs(deployment_cache_path) for dir in listdir(cache_dir_path): # Delete all stale cache tracks so we don't run into issues # when reverting deployments. - dir = '%s/%s' % (cache_dir_path, dir) + dir = "%s/%s" % (cache_dir_path, dir) if dir != deployment_cache_path: - log.info('Deleting %s' % dir) + log.info("Deleting %s" % dir) rmtree(dir) for recipe in recipes: - recipe_cache_path = '%s/%s' % (deployment_cache_path, recipe.get_name()) - if isfile(recipe_cache_path) and recipe.get_deep_hash() in open(recipe_cache_path, 'r').read() and not args.force: - log.warn('%s is already deployed. Skipping...' % recipe.get_name()) + recipe_cache_path = "%s/%s" % (deployment_cache_path, recipe.get_name()) + if ( + isfile(recipe_cache_path) + and recipe.get_deep_hash() in open(recipe_cache_path, "r").read() + and not args.force + ): + log.warn("%s is already deployed. Skipping..." % recipe.get_name()) continue started_recipe = datetime.now() - log.info('Started %s recipe deployment at %s on %s' % - (recipe.get_name(), - started_recipe.strftime('%H:%M:%S'), - started_all.strftime('%Y-%m-%d'))) + log.info( + "Started %s recipe deployment at %s on %s" + % ( + recipe.get_name(), + started_recipe.strftime("%H:%M:%S"), + started_all.strftime("%Y-%m-%d"), + ) + ) recipe.deploy() ended_recipe = datetime.now() - log.info('Ended %s recipe deployment at %s on %s' % - (recipe.get_name(), - ended_recipe.strftime('%H:%M:%S'), - started_all.strftime('%Y-%m-%d'))) + log.info( + "Ended %s recipe deployment at %s on %s" + % ( + recipe.get_name(), + ended_recipe.strftime("%H:%M:%S"), + started_all.strftime("%Y-%m-%d"), + ) + ) total_recipe_time = ended_recipe - started_recipe - log.success('%s finished in %s' % (recipe.get_name(), reformat_time(total_recipe_time))) - open(recipe_cache_path, 'w').write(recipe.get_deep_hash()) + log.success( + "%s finished in %s" % (recipe.get_name(), reformat_time(total_recipe_time)) + ) + open(recipe_cache_path, "w").write(recipe.get_deep_hash()) ended_all = datetime.now() total_deployment_time = ended_all - started_all - log.info('Ended %s deployment at %s on %s' % - (config_path, - ended_all.strftime('%H:%M:%S'), - started_all.strftime('%Y-%m-%d'))) - log.success('%s finished in %s' % (config_path, reformat_time(total_deployment_time))) - log.info('Deployment hash is %s' % recipes.get_hash()) + log.info( + "Ended %s deployment at %s on %s" + % ( + config_path, + ended_all.strftime("%H:%M:%S"), + started_all.strftime("%Y-%m-%d"), + ) + ) + log.success( + "%s finished in %s" % (config_path, reformat_time(total_deployment_time)) + ) + log.info("Deployment hash is %s" % recipes.get_hash()) diff --git a/zdeploy/clients.py b/zdeploy/clients.py index 7b4fe91..a88ea28 100644 --- a/zdeploy/clients.py +++ b/zdeploy/clients.py @@ -1,42 +1,62 @@ +"""Remote client helpers for recipes.""" + from paramiko import SSHClient, AutoAddPolicy from scp import SCPClient + class SSH(SSHClient): + """SSH helper that exposes a simple execute function.""" + def __init__(self, recipe, log, hostname, username, password, port): - SSHClient.__init__(self) + super().__init__() self.load_system_host_keys() self.set_missing_host_key_policy(AutoAddPolicy()) self.connect(hostname=hostname, port=port, username=username, password=password) self.recipe = recipe self.log = log + def __del__(self): - ''' - Auto close connection - ''' + """Ensure the SSH connection is closed.""" + self.close() - def execute(self, *args, bail_on_failure=True, show_command=True, show_output=True, show_error=True): - cmd = ' '.join(args) + + def execute( + self, + *args, + bail_on_failure=True, + show_command=True, + show_output=True, + show_error=True, + ): + """Run ``args`` over SSH and return the exit code.""" + cmd = " ".join(args) if show_command: - self.log.info('Running', cmd) - _, stdout, _ = self.exec_command('%s 2>&1' % cmd) + self.log.info("Running", cmd) + _, stdout, _ = self.exec_command("%s 2>&1" % cmd) if show_output: for line in stdout: - self.log.info('%s: %s' % (self.recipe, line.rstrip())) + self.log.info("%s: %s" % (self.recipe, line.rstrip())) rc = stdout.channel.recv_exit_status() if rc != 0: if show_error: self.log.fail("Failed to run '%s'. Exit code: %d" % (cmd, rc)) if bail_on_failure: - raise Exception('failed to execute %s' % cmd) + raise Exception("failed to execute %s" % cmd) return rc + class SCP(SCPClient): + """SCP helper for transferring files over SSH.""" + def __init__(self, transport): - SCPClient.__init__(self, transport) + super().__init__(transport) + def __del__(self): - ''' - Auto close connection - ''' + """Ensure the SCP connection is closed.""" + self.close() + def upload(self, src, dest): - pass + """Upload ``src`` to ``dest`` on the remote host.""" + + self.put(src, remote_path=dest) diff --git a/zdeploy/config.py b/zdeploy/config.py index 0328028..c5dded3 100644 --- a/zdeploy/config.py +++ b/zdeploy/config.py @@ -1,40 +1,44 @@ +"""Configuration loader for zdeploy.""" + from json import loads from os.path import isfile -def load(cfg_path = 'config.json'): - # Initiate an empty config dictionary in case a config - # file isn't present. - cfg = {} - - if isfile(cfg_path): - # Load data in JSON form. - # TODO: try..catch with an informative exception - cfg = loads(open(cfg_path).read()) - - # Set defaults - cfg['configs'] = cfg.get('configs', 'configs') - cfg['recipes'] = cfg.get('recipes', 'recipes') - cfg['cache'] = cfg.get('cache', 'cache') - cfg['logs'] = cfg.get('logs', 'logs') - - # Default installer is apt-get. This is used for virtual - # recipes (recipes that aren't defined by a directory - # structure). - cfg['installer'] = cfg.get('installer', 'apt-get install -y') - - # Force is disabled by default. This sets the behavior to - # only deploy undeployed recipes and/or pick up where a - # previous deployment was halted or had crashed. - cfg['force'] = cfg.get('force', 'no') - - # Default username is root - cfg['user'] = cfg.get('user', 'root') - - # The default is no password (private key present) - cfg['password'] = cfg.get('password', None) - - cfg['port'] = cfg.get('port', 22) - - # Turn cfg into a class allowing us to reference all the field via the dot operator, - # e.g.: cfg.logs instead of cfg['logs'] - return type('', (), cfg) + +def load(cfg_path="config.json"): + """Load configuration from ``cfg_path`` or return defaults.""" + + # Initiate an empty config dictionary in case a config file isn't present. + cfg = {} + + if isfile(cfg_path): + # Load data in JSON form. + with open(cfg_path, "r", encoding="utf-8") as fp: + cfg = loads(fp.read()) + + # Set defaults + cfg["configs"] = cfg.get("configs", "configs") + cfg["recipes"] = cfg.get("recipes", "recipes") + cfg["cache"] = cfg.get("cache", "cache") + cfg["logs"] = cfg.get("logs", "logs") + + # Default installer is apt-get. This is used for virtual + # recipes (recipes that aren't defined by a directory + # structure). + cfg["installer"] = cfg.get("installer", "apt-get install -y") + + # Force is disabled by default. This sets the behavior to + # only deploy undeployed recipes and/or pick up where a + # previous deployment was halted or had crashed. + cfg["force"] = cfg.get("force", "no") + + # Default username is root + cfg["user"] = cfg.get("user", "root") + + # The default is no password (private key present) + cfg["password"] = cfg.get("password", None) + + cfg["port"] = cfg.get("port", 22) + + # Turn cfg into a class allowing us to reference all the field via the dot operator, + # e.g.: cfg.logs instead of cfg['logs'] + return type("", (), cfg) diff --git a/zdeploy/log.py b/zdeploy/log.py index 158392a..9219acc 100644 --- a/zdeploy/log.py +++ b/zdeploy/log.py @@ -1,36 +1,50 @@ +"""Simple logging utilities.""" + + class Log: - ''' + """ Log is a generic logging class that can write to anything with a `write` function. The recommended use of this class is to log to the standard output (stdout) and to a file, e.g.: logger = Log() logger.register_logger(sys.stdout) logger.register_logger(open('mylog.log', 'w')) - ''' + """ + def __init__(self, *loggers): self.loggers = list(loggers) + def register_logger(self, logger): self.loggers.append(logger) + def register_loggers(self, loggers): for logger in loggers: self.register_logger(logger) + def write(self, *args): - message = ' '.join(args) + message = " ".join(args) for logger in self.loggers: - logger.write('%s\n' % message) + logger.write("%s\n" % message) + def fatal(self, *args): self.fail(*args) exit(1) + def fail(self, *args): - self.write('\033[0;31m', *args, '\033[0;00m') + self.write("\033[0;31m", *args, "\033[0;00m") + def warn(self, *args): - self.write('\033[1;33m', *args, '\033[0;00m') + self.write("\033[1;33m", *args, "\033[0;00m") + def success(self, *args): - self.write('\033[0;32m', *args, '\033[0;00m') + self.write("\033[0;32m", *args, "\033[0;00m") + def info(self, *args): - self.write('\033[1;35m', *args, '\033[0;00m') + self.write("\033[1;35m", *args, "\033[0;00m") + def close(self): for logger in self.loggers: logger.close() + def __del__(self): - self.close() \ No newline at end of file + self.close() diff --git a/zdeploy/recipe.py b/zdeploy/recipe.py index bc9b16a..0327fed 100644 --- a/zdeploy/recipe.py +++ b/zdeploy/recipe.py @@ -1,3 +1,5 @@ +"""Recipe abstraction for deploying and tracking dependencies.""" + from os import listdir from os.path import isdir, isfile from datetime import datetime @@ -5,22 +7,39 @@ from zdeploy.clients import SSH, SCP from zdeploy.shell import execute as shell_execute + class Recipe: + """Represents a deployable script or package.""" + class Type: DEFINED = 1 VIRTUAL = 2 - def __init__(self, recipe, parent_recipe, config, hostname, username, password, port, log, cfg): + + def __init__( + self, + recipe, + parent_recipe, + config, + hostname, + username, + password, + port, + log, + cfg, + ): + """Initialize a recipe instance.""" + self.log = log if not config or not len(config.strip()): - self.log.fatal('Invalid value for config') + self.log.fatal("Invalid value for config") if not recipe or not len(recipe.strip()): - self.log.fatal('Invalid value for recipe') + self.log.fatal("Invalid value for recipe") if not hostname or not len(hostname.strip()): - self.log.fatal('Invalid value for hostname') + self.log.fatal("Invalid value for hostname") try: self.port = int(port) except ValueError: - self.log.fatal('Invalid value for port: %s' % port) + self.log.fatal("Invalid value for port: %s" % port) self.cfg = cfg self.parent_recipe = parent_recipe @@ -30,41 +49,59 @@ def __init__(self, recipe, parent_recipe, config, hostname, username, password, self.username = username self.password = password self.properties = {} + def set_property(self, key, value): self.properties[key] = value + def set_recipe_name_and_type(self, recipe): for r in listdir(self.cfg.recipes): if recipe.lower() == r.lower(): self.recipe = r if self.parent_recipe == r: # Recipe references itself - self.log.fatal('Invalid recipe: %s references itself' % r) + self.log.fatal("Invalid recipe: %s references itself" % r) else: self._type = self.Type.DEFINED return self.recipe = recipe self._type = self.Type.VIRTUAL + def __str__(self): - return '%s -> %s@%s:%d :: %s' % (self.recipe, self.username, self.hostname, self.port, self.properties) + return "%s -> %s@%s:%d :: %s" % ( + self.recipe, + self.username, + self.hostname, + self.port, + self.properties, + ) + def get_name(self): return self.recipe + def __hash__(self): return hash(str(self)) + def __eq__(self, other): return hash(self) == hash(other) + def get_deep_hash(self, dir_path=None): - hashes = '' + """Return an MD5 hash representing the recipe and its requirements.""" + + hashes = "" if self._type == self.Type.VIRTUAL: hashes = md5(self.recipe.encode()).hexdigest() elif self._type == self.Type.DEFINED: if dir_path is None: - dir_path = '%s/%s' % (self.cfg.recipes, self.recipe) + dir_path = "%s/%s" % (self.cfg.recipes, self.recipe) # Execute the hash script and copy its output into our hashes variable. # NOTE: We perform this check specifically inside this block because when # dir_path is None, we know we're at the main recipe directory path. - if isfile('%s/hash' % dir_path): - cmd_out, cmd_rc = shell_execute('chmod +x %s/hash && bash %s && ./%s/hash' % (dir_path, self.config, dir_path)) + if isfile("%s/hash" % dir_path): + cmd_out, cmd_rc = shell_execute( + "chmod +x %s/hash && bash %s && ./%s/hash" + % (dir_path, self.config, dir_path) + ) if cmd_rc != 0: raise Exception(cmd_out) hashes += cmd_out @@ -73,20 +110,23 @@ def get_deep_hash(self, dir_path=None): hashes += recipe.get_deep_hash() hashes += md5(str(self).encode()).hexdigest() for node in listdir(dir_path): - rel_path = '%s/%s' % (dir_path, node) + rel_path = "%s/%s" % (dir_path, node) if isfile(rel_path): - file_hash = md5(open(rel_path, 'rb').read()).hexdigest() + file_hash = md5(open(rel_path, "rb").read()).hexdigest() hashes += file_hash elif isdir(rel_path): hashes += self.get_deep_hash(rel_path) return md5(hashes.encode()).hexdigest() + def get_requirements(self): - req_file = '%s/%s/require' % (self.cfg.recipes, self.recipe) + """Return a list of Recipe objects this recipe depends on.""" + + req_file = "%s/%s/require" % (self.cfg.recipes, self.recipe) requirements = [] if isfile(req_file): - for requirement in open(req_file).read().split('\n'): + for requirement in open(req_file).read().split("\n"): requirement = requirement.strip() - if requirement == '' or requirement.startswith('#'): + if requirement == "" or requirement.startswith("#"): continue recipe = Recipe( recipe=requirement, @@ -97,45 +137,60 @@ def get_requirements(self): password=self.password, port=self.port, log=self.log, - cfg=self.cfg) + cfg=self.cfg, + ) for req in recipe.get_requirements(): requirements.append(req) requirements.append(recipe) return requirements + def deploy(self): - self.log.info('Deploying %s to %s' % (self.recipe, self.hostname)) - ssh = SSH(recipe=self.recipe, + """Deploy this recipe using SSH/SCP.""" + + self.log.info("Deploying %s to %s" % (self.recipe, self.hostname)) + ssh = SSH( + recipe=self.recipe, log=self.log, hostname=self.hostname, username=self.username, password=self.password, - port=self.port) + port=self.port, + ) if self._type == self.Type.DEFINED: - ssh.execute('rm -rf /opt/%s' % self.recipe, show_command=False) + ssh.execute("rm -rf /opt/%s" % self.recipe, show_command=False) scp = SCP(ssh.get_transport()) - scp.put('%s/%s' % (self.cfg.recipes, self.recipe), remote_path='/opt/%s' % self.recipe, recursive=True) - scp.put(self.config, remote_path='/opt/%s/config' % self.recipe) + scp.put( + "%s/%s" % (self.cfg.recipes, self.recipe), + remote_path="/opt/%s" % self.recipe, + recursive=True, + ) + scp.put(self.config, remote_path="/opt/%s/config" % self.recipe) try: if self._type == self.Type.VIRTUAL: - ssh.execute('%s %s' % (self.cfg.installer, self.recipe)) + ssh.execute("%s %s" % (self.cfg.installer, self.recipe)) elif self._type == self.Type.DEFINED: - if not isfile('%s/%s/run' % (self.cfg.recipes, self.recipe)): + if not isfile("%s/%s/run" % (self.cfg.recipes, self.recipe)): # Recipes with no run file are acceptable since they (may) have a require file # and don't necessarily require the execution of anything of their own. - self.log.warn("%s doesn't have a run file. Continuing..." % self.recipe) + self.log.warn( + "%s doesn't have a run file. Continuing..." % self.recipe + ) else: - ssh.execute('cd /opt/%s && chmod +x ./run && ./run' % self.recipe, show_command=False) + ssh.execute( + "cd /opt/%s && chmod +x ./run && ./run" % self.recipe, + show_command=False, + ) passed = True except Exception: passed = False finally: if self._type == self.Type.DEFINED: - self.log.info('Deleting /opt/%s from remote host' % self.recipe) - ssh.execute('rm -rf /opt/%s' % self.recipe, show_command=False) + self.log.info("Deleting /opt/%s from remote host" % self.recipe) + ssh.execute("rm -rf /opt/%s" % self.recipe, show_command=False) if not passed: - self.log.fatal('Failed to deploy %s' % self.recipe) - self.log.success('Done with %s' % self.recipe) + self.log.fatal("Failed to deploy %s" % self.recipe) + self.log.success("Done with %s" % self.recipe) diff --git a/zdeploy/recipeset.py b/zdeploy/recipeset.py index 7c9f607..563c275 100644 --- a/zdeploy/recipeset.py +++ b/zdeploy/recipeset.py @@ -1,37 +1,58 @@ +"""Helper for managing sets of ``Recipe`` objects.""" + from hashlib import md5 from zdeploy.recipe import Recipe + class RecipeSet: - ''' - RecipeSet is a unique set of Recipes maintainer. - ''' + """ + RecipeSet is a unique set of Recipes maintainer. + """ + def __init__(self, cfg, log): self.recipes = [] self.cfg = cfg self.log = log + def add_recipes(self, recipes): for recipe in recipes: self.add_recipe(recipe) + def add_recipe(self, recipe): if recipe in self.recipes: - self.log.warn('%s is already added to the recipes list. Skipping...' % recipe.get_name()) + self.log.warn( + "%s is already added to the recipes list. Skipping..." + % recipe.get_name() + ) return self.log.info("Adding '%s' to the recipes list" % recipe.get_name()) self.recipes.append(recipe) if recipe._type == Recipe.Type.VIRTUAL: - self.log.warn("'%s' doesn't correspond to anything defined under the %s directory" % (recipe.recipe, self.cfg.recipes)) - self.log.warn("this recipe will be marked virtual and execute as `%s %s`" % (recipe.cfg.installer, recipe.recipe)) - self.log.warn("If you want to use a different package manager, add an 'installer' field to the config.json file") + self.log.warn( + "'%s' doesn't correspond to anything defined under the %s directory" + % (recipe.recipe, self.cfg.recipes) + ) + self.log.warn( + "this recipe will be marked virtual and execute as `%s %s`" + % (recipe.cfg.installer, recipe.recipe) + ) + self.log.warn( + "If you want to use a different package manager, add an 'installer' field to the config.json file" + ) + def get_hash(self): - ''' + """ Return an MD5 hash out of the hash of all recipes combined. The end result is used to create a cache directory under deployments cache. - ''' - return md5(' '.join([str(recipe) for recipe in self.recipes]).encode()).hexdigest() + """ + return md5( + " ".join([str(recipe) for recipe in self.recipes]).encode() + ).hexdigest() + def __iter__(self): - ''' + """ Allow caller to iterate over recipes with a regular for loop, e.g.: for recipe in recipes: print(recipe) - ''' + """ return iter(self.recipes) diff --git a/zdeploy/shell.py b/zdeploy/shell.py index d70727f..fcd3454 100644 --- a/zdeploy/shell.py +++ b/zdeploy/shell.py @@ -1,11 +1,18 @@ +"""Simple shell helpers.""" + import subprocess + def execute(cmd): - proc = subprocess.Popen('%s 2>&1' % cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - shell=True, - universal_newlines=True) + """Execute ``cmd`` in a shell and return output and return code.""" + + proc = subprocess.Popen( + f"{cmd} 2>&1", + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + shell=True, + universal_newlines=True, + ) std_out, _ = proc.communicate() rc = proc.returncode return std_out, rc diff --git a/zdeploy/utils.py b/zdeploy/utils.py index 390fb11..66e473f 100644 --- a/zdeploy/utils.py +++ b/zdeploy/utils.py @@ -1,3 +1,8 @@ +"""Utility helpers for zdeploy.""" + + def reformat_time(time): - h, m, s = [int(float(x)) for x in ('%s' % time).split(':')] - return '%sh, %sm, and %ds' % (h, m, s) + """Return ``time`` in "Nh, Nm, and Ns" format.""" + + h, m, s = [int(float(x)) for x in (f"{time}").split(":")] + return f"{h}h, {m}m, and {s}s" From abe22021490f665caa9ecb60280fa60dc67c4ac5 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Tue, 1 Jul 2025 23:27:36 -0700 Subject: [PATCH 05/19] Use dataclass for config --- zdeploy/app.py | 2 +- zdeploy/config.py | 44 ++++++++++++++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/zdeploy/app.py b/zdeploy/app.py index 97c56af..cad730a 100644 --- a/zdeploy/app.py +++ b/zdeploy/app.py @@ -18,7 +18,7 @@ def deploy(config_name, cache_dir_path, log, args, cfg): recipes = RecipeSet(cfg, log) - recipe_names = environ.get("RECIPES") + recipe_names = environ.get("RECIPES", "") if recipe_names.startswith("(") and recipe_names.endswith(")"): recipe_names = recipe_names[1:-1] for recipe_name in recipe_names.split(" "): diff --git a/zdeploy/config.py b/zdeploy/config.py index c5dded3..b6f8f7c 100644 --- a/zdeploy/config.py +++ b/zdeploy/config.py @@ -1,14 +1,31 @@ """Configuration loader for zdeploy.""" +from dataclasses import dataclass from json import loads from os.path import isfile +from typing import Any, Dict, cast -def load(cfg_path="config.json"): +@dataclass +class Config: + """Simple container for configuration settings.""" + + configs: str = "configs" + recipes: str = "recipes" + cache: str = "cache" + logs: str = "logs" + installer: str = "apt-get install -y" + force: str = "no" + user: str = "root" + password: str | None = None + port: int = 22 + + +def load(cfg_path: str = "config.json") -> Config: """Load configuration from ``cfg_path`` or return defaults.""" # Initiate an empty config dictionary in case a config file isn't present. - cfg = {} + cfg: Dict[str, Any] = {} if isfile(cfg_path): # Load data in JSON form. @@ -16,29 +33,28 @@ def load(cfg_path="config.json"): cfg = loads(fp.read()) # Set defaults - cfg["configs"] = cfg.get("configs", "configs") - cfg["recipes"] = cfg.get("recipes", "recipes") - cfg["cache"] = cfg.get("cache", "cache") - cfg["logs"] = cfg.get("logs", "logs") + cfg["configs"] = cfg.get("configs", Config.configs) + cfg["recipes"] = cfg.get("recipes", Config.recipes) + cfg["cache"] = cfg.get("cache", Config.cache) + cfg["logs"] = cfg.get("logs", Config.logs) # Default installer is apt-get. This is used for virtual # recipes (recipes that aren't defined by a directory # structure). - cfg["installer"] = cfg.get("installer", "apt-get install -y") + cfg["installer"] = cfg.get("installer", Config.installer) # Force is disabled by default. This sets the behavior to # only deploy undeployed recipes and/or pick up where a # previous deployment was halted or had crashed. - cfg["force"] = cfg.get("force", "no") + cfg["force"] = cfg.get("force", Config.force) # Default username is root - cfg["user"] = cfg.get("user", "root") + cfg["user"] = cfg.get("user", Config.user) # The default is no password (private key present) - cfg["password"] = cfg.get("password", None) + cfg["password"] = cfg.get("password", Config.password) - cfg["port"] = cfg.get("port", 22) + cfg["port"] = cfg.get("port", Config.port) - # Turn cfg into a class allowing us to reference all the field via the dot operator, - # e.g.: cfg.logs instead of cfg['logs'] - return type("", (), cfg) + # Convert the dictionary into a Config instance to allow attribute access + return Config(**cast(Dict[str, Any], cfg)) From 4dcb6a5b2f8c47e1aad56b276960a2df89e47bf2 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 00:07:44 -0700 Subject: [PATCH 06/19] Refactor strings to f-strings and clean imports --- zdeploy/app.py | 59 +++++++++++++++----------------------------- zdeploy/clients.py | 8 +++--- zdeploy/log.py | 2 +- zdeploy/recipe.py | 50 ++++++++++++++++--------------------- zdeploy/recipeset.py | 11 +++------ 5 files changed, 50 insertions(+), 80 deletions(-) diff --git a/zdeploy/app.py b/zdeploy/app.py index cad730a..d8e2abd 100644 --- a/zdeploy/app.py +++ b/zdeploy/app.py @@ -12,7 +12,7 @@ def deploy(config_name, cache_dir_path, log, args, cfg): """Deploy recipes defined in ``config_name``.""" - config_path = "%s/%s" % (cfg.configs, config_name) + config_path = f"{cfg.configs}/{config_name}" print("Config:", config_path) load_dotenv(config_path) @@ -25,10 +25,10 @@ def deploy(config_name, cache_dir_path, log, args, cfg): recipe_name = recipe_name.strip() HOST_IP = environ.get(recipe_name) if HOST_IP is None: - log.fatal("%s is undefined in %s" % (recipe_name, config_path)) - HOST_USER = environ.get("%s_USER" % recipe_name, cfg.user) - HOST_PASSWORD = environ.get("%s_PASSWORD" % recipe_name, cfg.password) - HOST_PORT = environ.get("%s_PORT" % recipe_name, cfg.port) + log.fatal(f"{recipe_name} is undefined in {config_path}") + HOST_USER = environ.get(f"{recipe_name}_USER", cfg.user) + HOST_PASSWORD = environ.get(f"{recipe_name}_PASSWORD", cfg.password) + HOST_PORT = environ.get(f"{recipe_name}_PORT", cfg.port) recipe = Recipe( recipe_name, None, @@ -53,67 +53,48 @@ def deploy(config_name, cache_dir_path, log, args, cfg): started_all = datetime.now() log.info( - "Started %s deployment at %s on %s" - % ( - config_path, - started_all.strftime("%H:%M:%S"), - started_all.strftime("%Y-%m-%d"), - ) + f"Started {config_path} deployment at {started_all:%H:%M:%S} on {started_all:%Y-%m-%d}" ) - deployment_cache_path = "%s/%s" % (cache_dir_path, recipes.get_hash()) + deployment_cache_path = f"{cache_dir_path}/{recipes.get_hash()}" if not isdir(deployment_cache_path): makedirs(deployment_cache_path) for dir in listdir(cache_dir_path): # Delete all stale cache tracks so we don't run into issues # when reverting deployments. - dir = "%s/%s" % (cache_dir_path, dir) + dir = f"{cache_dir_path}/{dir}" if dir != deployment_cache_path: - log.info("Deleting %s" % dir) + log.info(f"Deleting {dir}") rmtree(dir) for recipe in recipes: - recipe_cache_path = "%s/%s" % (deployment_cache_path, recipe.get_name()) + recipe_cache_path = f"{deployment_cache_path}/{recipe.get_name()}" if ( isfile(recipe_cache_path) - and recipe.get_deep_hash() in open(recipe_cache_path, "r").read() + and recipe.get_deep_hash() in open(recipe_cache_path, "r", encoding="utf-8").read() and not args.force ): - log.warn("%s is already deployed. Skipping..." % recipe.get_name()) + log.warn(f"{recipe.get_name()} is already deployed. Skipping...") continue started_recipe = datetime.now() log.info( - "Started %s recipe deployment at %s on %s" - % ( - recipe.get_name(), - started_recipe.strftime("%H:%M:%S"), - started_all.strftime("%Y-%m-%d"), - ) + f"Started {recipe.get_name()} recipe deployment at {started_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" ) recipe.deploy() ended_recipe = datetime.now() log.info( - "Ended %s recipe deployment at %s on %s" - % ( - recipe.get_name(), - ended_recipe.strftime("%H:%M:%S"), - started_all.strftime("%Y-%m-%d"), - ) + f"Ended {recipe.get_name()} recipe deployment at {ended_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" ) total_recipe_time = ended_recipe - started_recipe log.success( - "%s finished in %s" % (recipe.get_name(), reformat_time(total_recipe_time)) + f"{recipe.get_name()} finished in {reformat_time(total_recipe_time)}" ) - open(recipe_cache_path, "w").write(recipe.get_deep_hash()) + with open(recipe_cache_path, "w", encoding="utf-8") as fp: + fp.write(recipe.get_deep_hash()) ended_all = datetime.now() total_deployment_time = ended_all - started_all log.info( - "Ended %s deployment at %s on %s" - % ( - config_path, - ended_all.strftime("%H:%M:%S"), - started_all.strftime("%Y-%m-%d"), - ) + f"Ended {config_path} deployment at {ended_all:%H:%M:%S} on {started_all:%Y-%m-%d}" ) log.success( - "%s finished in %s" % (config_path, reformat_time(total_deployment_time)) + f"{config_path} finished in {reformat_time(total_deployment_time)}" ) - log.info("Deployment hash is %s" % recipes.get_hash()) + log.info(f"Deployment hash is {recipes.get_hash()}") diff --git a/zdeploy/clients.py b/zdeploy/clients.py index a88ea28..5db1e80 100644 --- a/zdeploy/clients.py +++ b/zdeploy/clients.py @@ -32,16 +32,16 @@ def execute( cmd = " ".join(args) if show_command: self.log.info("Running", cmd) - _, stdout, _ = self.exec_command("%s 2>&1" % cmd) + _, stdout, _ = self.exec_command(f"{cmd} 2>&1") if show_output: for line in stdout: - self.log.info("%s: %s" % (self.recipe, line.rstrip())) + self.log.info(f"{self.recipe}: {line.rstrip()}") rc = stdout.channel.recv_exit_status() if rc != 0: if show_error: - self.log.fail("Failed to run '%s'. Exit code: %d" % (cmd, rc)) + self.log.fail(f"Failed to run '{cmd}'. Exit code: {rc}") if bail_on_failure: - raise Exception("failed to execute %s" % cmd) + raise Exception(f"failed to execute {cmd}") return rc diff --git a/zdeploy/log.py b/zdeploy/log.py index 9219acc..0910742 100644 --- a/zdeploy/log.py +++ b/zdeploy/log.py @@ -24,7 +24,7 @@ def register_loggers(self, loggers): def write(self, *args): message = " ".join(args) for logger in self.loggers: - logger.write("%s\n" % message) + logger.write(f"{message}\n") def fatal(self, *args): self.fail(*args) diff --git a/zdeploy/recipe.py b/zdeploy/recipe.py index 0327fed..e65ecd0 100644 --- a/zdeploy/recipe.py +++ b/zdeploy/recipe.py @@ -2,7 +2,6 @@ from os import listdir from os.path import isdir, isfile -from datetime import datetime from hashlib import md5 from zdeploy.clients import SSH, SCP from zdeploy.shell import execute as shell_execute @@ -39,7 +38,7 @@ def __init__( try: self.port = int(port) except ValueError: - self.log.fatal("Invalid value for port: %s" % port) + self.log.fatal(f"Invalid value for port: {port}") self.cfg = cfg self.parent_recipe = parent_recipe @@ -59,7 +58,7 @@ def set_recipe_name_and_type(self, recipe): self.recipe = r if self.parent_recipe == r: # Recipe references itself - self.log.fatal("Invalid recipe: %s references itself" % r) + self.log.fatal(f"Invalid recipe: {r} references itself") else: self._type = self.Type.DEFINED return @@ -67,13 +66,7 @@ def set_recipe_name_and_type(self, recipe): self._type = self.Type.VIRTUAL def __str__(self): - return "%s -> %s@%s:%d :: %s" % ( - self.recipe, - self.username, - self.hostname, - self.port, - self.properties, - ) + return f"{self.recipe} -> {self.username}@{self.hostname}:{self.port} :: {self.properties}" def get_name(self): return self.recipe @@ -92,15 +85,14 @@ def get_deep_hash(self, dir_path=None): hashes = md5(self.recipe.encode()).hexdigest() elif self._type == self.Type.DEFINED: if dir_path is None: - dir_path = "%s/%s" % (self.cfg.recipes, self.recipe) + dir_path = f"{self.cfg.recipes}/{self.recipe}" # Execute the hash script and copy its output into our hashes variable. # NOTE: We perform this check specifically inside this block because when # dir_path is None, we know we're at the main recipe directory path. - if isfile("%s/hash" % dir_path): + if isfile(f"{dir_path}/hash"): cmd_out, cmd_rc = shell_execute( - "chmod +x %s/hash && bash %s && ./%s/hash" - % (dir_path, self.config, dir_path) + f"chmod +x {dir_path}/hash && bash {self.config} && ./{dir_path}/hash" ) if cmd_rc != 0: raise Exception(cmd_out) @@ -110,7 +102,7 @@ def get_deep_hash(self, dir_path=None): hashes += recipe.get_deep_hash() hashes += md5(str(self).encode()).hexdigest() for node in listdir(dir_path): - rel_path = "%s/%s" % (dir_path, node) + rel_path = f"{dir_path}/{node}" if isfile(rel_path): file_hash = md5(open(rel_path, "rb").read()).hexdigest() hashes += file_hash @@ -121,7 +113,7 @@ def get_deep_hash(self, dir_path=None): def get_requirements(self): """Return a list of Recipe objects this recipe depends on.""" - req_file = "%s/%s/require" % (self.cfg.recipes, self.recipe) + req_file = f"{self.cfg.recipes}/{self.recipe}/require" requirements = [] if isfile(req_file): for requirement in open(req_file).read().split("\n"): @@ -147,7 +139,7 @@ def get_requirements(self): def deploy(self): """Deploy this recipe using SSH/SCP.""" - self.log.info("Deploying %s to %s" % (self.recipe, self.hostname)) + self.log.info(f"Deploying {self.recipe} to {self.hostname}") ssh = SSH( recipe=self.recipe, log=self.log, @@ -158,29 +150,29 @@ def deploy(self): ) if self._type == self.Type.DEFINED: - ssh.execute("rm -rf /opt/%s" % self.recipe, show_command=False) + ssh.execute(f"rm -rf /opt/{self.recipe}", show_command=False) scp = SCP(ssh.get_transport()) scp.put( - "%s/%s" % (self.cfg.recipes, self.recipe), - remote_path="/opt/%s" % self.recipe, + f"{self.cfg.recipes}/{self.recipe}", + remote_path=f"/opt/{self.recipe}", recursive=True, ) - scp.put(self.config, remote_path="/opt/%s/config" % self.recipe) + scp.put(self.config, remote_path=f"/opt/{self.recipe}/config") try: if self._type == self.Type.VIRTUAL: - ssh.execute("%s %s" % (self.cfg.installer, self.recipe)) + ssh.execute(f"{self.cfg.installer} {self.recipe}") elif self._type == self.Type.DEFINED: - if not isfile("%s/%s/run" % (self.cfg.recipes, self.recipe)): + if not isfile(f"{self.cfg.recipes}/{self.recipe}/run"): # Recipes with no run file are acceptable since they (may) have a require file # and don't necessarily require the execution of anything of their own. self.log.warn( - "%s doesn't have a run file. Continuing..." % self.recipe + f"{self.recipe} doesn't have a run file. Continuing..." ) else: ssh.execute( - "cd /opt/%s && chmod +x ./run && ./run" % self.recipe, + f"cd /opt/{self.recipe} && chmod +x ./run && ./run", show_command=False, ) passed = True @@ -188,9 +180,9 @@ def deploy(self): passed = False finally: if self._type == self.Type.DEFINED: - self.log.info("Deleting /opt/%s from remote host" % self.recipe) - ssh.execute("rm -rf /opt/%s" % self.recipe, show_command=False) + self.log.info(f"Deleting /opt/{self.recipe} from remote host") + ssh.execute(f"rm -rf /opt/{self.recipe}", show_command=False) if not passed: - self.log.fatal("Failed to deploy %s" % self.recipe) - self.log.success("Done with %s" % self.recipe) + self.log.fatal(f"Failed to deploy {self.recipe}") + self.log.success(f"Done with {self.recipe}") diff --git a/zdeploy/recipeset.py b/zdeploy/recipeset.py index 563c275..0aabc41 100644 --- a/zdeploy/recipeset.py +++ b/zdeploy/recipeset.py @@ -21,20 +21,17 @@ def add_recipes(self, recipes): def add_recipe(self, recipe): if recipe in self.recipes: self.log.warn( - "%s is already added to the recipes list. Skipping..." - % recipe.get_name() + f"{recipe.get_name()} is already added to the recipes list. Skipping..." ) return - self.log.info("Adding '%s' to the recipes list" % recipe.get_name()) + self.log.info(f"Adding '{recipe.get_name()}' to the recipes list") self.recipes.append(recipe) if recipe._type == Recipe.Type.VIRTUAL: self.log.warn( - "'%s' doesn't correspond to anything defined under the %s directory" - % (recipe.recipe, self.cfg.recipes) + f"'{recipe.recipe}' doesn't correspond to anything defined under the {self.cfg.recipes} directory" ) self.log.warn( - "this recipe will be marked virtual and execute as `%s %s`" - % (recipe.cfg.installer, recipe.recipe) + f"this recipe will be marked virtual and execute as `{recipe.cfg.installer} {recipe.recipe}`" ) self.log.warn( "If you want to use a different package manager, add an 'installer' field to the config.json file" From feb585852e9cdf0389d49ea3a538d0e5224a31df Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 10:06:16 -0700 Subject: [PATCH 07/19] Add missing docstrings --- zdeploy/clients.py | 4 ++++ zdeploy/log.py | 22 ++++++++++++++++++++++ zdeploy/recipe.py | 14 ++++++++++++++ zdeploy/recipeset.py | 6 ++++++ 4 files changed, 46 insertions(+) diff --git a/zdeploy/clients.py b/zdeploy/clients.py index 5db1e80..5079965 100644 --- a/zdeploy/clients.py +++ b/zdeploy/clients.py @@ -8,6 +8,8 @@ class SSH(SSHClient): """SSH helper that exposes a simple execute function.""" def __init__(self, recipe, log, hostname, username, password, port): + """Establish an SSH connection to ``hostname``.""" + super().__init__() self.load_system_host_keys() self.set_missing_host_key_policy(AutoAddPolicy()) @@ -49,6 +51,8 @@ class SCP(SCPClient): """SCP helper for transferring files over SSH.""" def __init__(self, transport): + """Create an SCP client using ``transport``.""" + super().__init__(transport) def __del__(self): diff --git a/zdeploy/log.py b/zdeploy/log.py index 0910742..65ee1fe 100644 --- a/zdeploy/log.py +++ b/zdeploy/log.py @@ -12,39 +12,61 @@ class Log: """ def __init__(self, *loggers): + """Create a new ``Log`` instance and register ``loggers`` if provided.""" + self.loggers = list(loggers) def register_logger(self, logger): + """Register a single ``logger`` object.""" + self.loggers.append(logger) def register_loggers(self, loggers): + """Register multiple ``loggers`` objects.""" + for logger in loggers: self.register_logger(logger) def write(self, *args): + """Write ``args`` to all registered loggers.""" + message = " ".join(args) for logger in self.loggers: logger.write(f"{message}\n") def fatal(self, *args): + """Log ``args`` as a failure and terminate the program.""" + self.fail(*args) exit(1) def fail(self, *args): + """Log ``args`` as an error.""" + self.write("\033[0;31m", *args, "\033[0;00m") def warn(self, *args): + """Log ``args`` as a warning.""" + self.write("\033[1;33m", *args, "\033[0;00m") def success(self, *args): + """Log ``args`` as a success message.""" + self.write("\033[0;32m", *args, "\033[0;00m") def info(self, *args): + """Log ``args`` as an informational message.""" + self.write("\033[1;35m", *args, "\033[0;00m") def close(self): + """Close all registered loggers.""" + for logger in self.loggers: logger.close() def __del__(self): + """Ensure all loggers are closed when this object is destroyed.""" + self.close() diff --git a/zdeploy/recipe.py b/zdeploy/recipe.py index e65ecd0..d8eaffe 100644 --- a/zdeploy/recipe.py +++ b/zdeploy/recipe.py @@ -11,6 +11,8 @@ class Recipe: """Represents a deployable script or package.""" class Type: + """Supported recipe types.""" + DEFINED = 1 VIRTUAL = 2 @@ -50,9 +52,13 @@ def __init__( self.properties = {} def set_property(self, key, value): + """Store an arbitrary ``key``/``value`` pair.""" + self.properties[key] = value def set_recipe_name_and_type(self, recipe): + """Resolve ``recipe`` name and determine if it is defined or virtual.""" + for r in listdir(self.cfg.recipes): if recipe.lower() == r.lower(): self.recipe = r @@ -66,15 +72,23 @@ def set_recipe_name_and_type(self, recipe): self._type = self.Type.VIRTUAL def __str__(self): + """Return a string representation of this recipe.""" + return f"{self.recipe} -> {self.username}@{self.hostname}:{self.port} :: {self.properties}" def get_name(self): + """Return the recipe name.""" + return self.recipe def __hash__(self): + """Return a hash so recipes can be used in sets and dictionaries.""" + return hash(str(self)) def __eq__(self, other): + """Compare recipes by their hash.""" + return hash(self) == hash(other) def get_deep_hash(self, dir_path=None): diff --git a/zdeploy/recipeset.py b/zdeploy/recipeset.py index 0aabc41..f992669 100644 --- a/zdeploy/recipeset.py +++ b/zdeploy/recipeset.py @@ -10,15 +10,21 @@ class RecipeSet: """ def __init__(self, cfg, log): + """Create an empty ``RecipeSet`` using ``cfg`` and ``log``.""" + self.recipes = [] self.cfg = cfg self.log = log def add_recipes(self, recipes): + """Add a sequence of ``recipes`` to the set.""" + for recipe in recipes: self.add_recipe(recipe) def add_recipe(self, recipe): + """Add a single ``recipe`` if not already present.""" + if recipe in self.recipes: self.log.warn( f"{recipe.get_name()} is already added to the recipes list. Skipping..." From 54bcdf60052a06bbe44ec4dfa1bbcbb96b7de659 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 10:10:22 -0700 Subject: [PATCH 08/19] Address linter feedback --- zdeploy/__init__.py | 17 ++++++++--------- zdeploy/log.py | 3 ++- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/zdeploy/__init__.py b/zdeploy/__init__.py index 24c2a07..2d60280 100644 --- a/zdeploy/__init__.py +++ b/zdeploy/__init__.py @@ -20,7 +20,7 @@ def str2bool(value): return True if value.lower() in ("no", "n"): return False - raise Exception(f"Invalid value: {value}") + raise ValueError(f"Invalid value: {value}") def handle_config(config_name, args, cfg): @@ -34,14 +34,13 @@ def handle_config(config_name, args, cfg): makedirs(cache_dir_path) log = Log() log.register_logger(stdout) - log.register_logger( - open( - f"{log_dir_path}/{datetime.now():%Y-%m-%d %H:%M:%S}.log", - "w", - encoding="utf-8", - ) - ) - deploy(config_name, cache_dir_path, log, args, cfg) + with open( + f"{log_dir_path}/{datetime.now():%Y-%m-%d %H:%M:%S}.log", + "w", + encoding="utf-8", + ) as log_file: + log.register_logger(log_file) + deploy(config_name, cache_dir_path, log, args, cfg) def handle_configs(args, cfg): diff --git a/zdeploy/log.py b/zdeploy/log.py index 65ee1fe..37cbe66 100644 --- a/zdeploy/log.py +++ b/zdeploy/log.py @@ -1,5 +1,6 @@ """Simple logging utilities.""" +import sys class Log: """ @@ -38,7 +39,7 @@ def fatal(self, *args): """Log ``args`` as a failure and terminate the program.""" self.fail(*args) - exit(1) + sys.exit(1) def fail(self, *args): """Log ``args`` as an error.""" From 655f5b34525b21c8b8ccbd4c2eb2c33befd7f0b2 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 12:25:14 -0700 Subject: [PATCH 09/19] Fix pylint issues --- zdeploy/app.py | 6 ++++-- zdeploy/recipe.py | 6 +++--- zdeploy/recipeset.py | 15 ++++++++++++--- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/zdeploy/app.py b/zdeploy/app.py index d8e2abd..0b70fc3 100644 --- a/zdeploy/app.py +++ b/zdeploy/app.py @@ -76,12 +76,14 @@ def deploy(config_name, cache_dir_path, log, args, cfg): continue started_recipe = datetime.now() log.info( - f"Started {recipe.get_name()} recipe deployment at {started_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" + f"Started {recipe.get_name()} recipe deployment at " + f"{started_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" ) recipe.deploy() ended_recipe = datetime.now() log.info( - f"Ended {recipe.get_name()} recipe deployment at {ended_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" + f"Ended {recipe.get_name()} recipe deployment at " + f"{ended_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" ) total_recipe_time = ended_recipe - started_recipe log.success( diff --git a/zdeploy/recipe.py b/zdeploy/recipe.py index d8eaffe..c4ad5bf 100644 --- a/zdeploy/recipe.py +++ b/zdeploy/recipe.py @@ -31,11 +31,11 @@ def __init__( """Initialize a recipe instance.""" self.log = log - if not config or not len(config.strip()): + if not config or not config.strip(): self.log.fatal("Invalid value for config") - if not recipe or not len(recipe.strip()): + if not recipe or not recipe.strip(): self.log.fatal("Invalid value for recipe") - if not hostname or not len(hostname.strip()): + if not hostname or not hostname.strip(): self.log.fatal("Invalid value for hostname") try: self.port = int(port) diff --git a/zdeploy/recipeset.py b/zdeploy/recipeset.py index f992669..0d7ac9c 100644 --- a/zdeploy/recipeset.py +++ b/zdeploy/recipeset.py @@ -34,13 +34,22 @@ def add_recipe(self, recipe): self.recipes.append(recipe) if recipe._type == Recipe.Type.VIRTUAL: self.log.warn( - f"'{recipe.recipe}' doesn't correspond to anything defined under the {self.cfg.recipes} directory" + ( + f"'{recipe.recipe}' doesn't correspond to anything defined " + f"under the {self.cfg.recipes} directory" + ) ) self.log.warn( - f"this recipe will be marked virtual and execute as `{recipe.cfg.installer} {recipe.recipe}`" + ( + "this recipe will be marked virtual and execute as " + f"`{recipe.cfg.installer} {recipe.recipe}`" + ) ) self.log.warn( - "If you want to use a different package manager, add an 'installer' field to the config.json file" + ( + "If you want to use a different package manager, add an " + "'installer' field to the config.json file" + ) ) def get_hash(self): From 1cccd84d3882bdb40c9f506e31858387e28b6e0c Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 14:31:08 -0700 Subject: [PATCH 10/19] Refine linting --- zdeploy/app.py | 43 ++++++++++++++++++------------------- zdeploy/clients.py | 3 ++- zdeploy/config.py | 3 ++- zdeploy/recipe.py | 51 ++++++++++++++++++++++++++------------------ zdeploy/recipeset.py | 3 +-- zdeploy/shell.py | 8 +++---- 6 files changed, 60 insertions(+), 51 deletions(-) diff --git a/zdeploy/app.py b/zdeploy/app.py index 0b70fc3..055374f 100644 --- a/zdeploy/app.py +++ b/zdeploy/app.py @@ -10,7 +10,7 @@ from zdeploy.utils import reformat_time -def deploy(config_name, cache_dir_path, log, args, cfg): +def deploy(config_name, cache_dir_path, log, args, cfg): # pylint: disable=too-many-locals """Deploy recipes defined in ``config_name``.""" config_path = f"{cfg.configs}/{config_name}" print("Config:", config_path) @@ -23,20 +23,20 @@ def deploy(config_name, cache_dir_path, log, args, cfg): recipe_names = recipe_names[1:-1] for recipe_name in recipe_names.split(" "): recipe_name = recipe_name.strip() - HOST_IP = environ.get(recipe_name) - if HOST_IP is None: + host_ip = environ.get(recipe_name) + if host_ip is None: log.fatal(f"{recipe_name} is undefined in {config_path}") - HOST_USER = environ.get(f"{recipe_name}_USER", cfg.user) - HOST_PASSWORD = environ.get(f"{recipe_name}_PASSWORD", cfg.password) - HOST_PORT = environ.get(f"{recipe_name}_PORT", cfg.port) + host_user = environ.get(f"{recipe_name}_USER", cfg.user) + host_password = environ.get(f"{recipe_name}_PASSWORD", cfg.password) + host_port = environ.get(f"{recipe_name}_PORT", cfg.port) recipe = Recipe( recipe_name, None, config_path, - HOST_IP, - HOST_USER, - HOST_PASSWORD, - HOST_PORT, + host_ip, + host_user, + host_password, + host_port, log, cfg, ) @@ -58,22 +58,21 @@ def deploy(config_name, cache_dir_path, log, args, cfg): deployment_cache_path = f"{cache_dir_path}/{recipes.get_hash()}" if not isdir(deployment_cache_path): makedirs(deployment_cache_path) - for dir in listdir(cache_dir_path): + for directory in listdir(cache_dir_path): # Delete all stale cache tracks so we don't run into issues # when reverting deployments. - dir = f"{cache_dir_path}/{dir}" - if dir != deployment_cache_path: - log.info(f"Deleting {dir}") - rmtree(dir) + directory = f"{cache_dir_path}/{directory}" + if directory != deployment_cache_path: + log.info(f"Deleting {directory}") + rmtree(directory) for recipe in recipes: recipe_cache_path = f"{deployment_cache_path}/{recipe.get_name()}" - if ( - isfile(recipe_cache_path) - and recipe.get_deep_hash() in open(recipe_cache_path, "r", encoding="utf-8").read() - and not args.force - ): - log.warn(f"{recipe.get_name()} is already deployed. Skipping...") - continue + if isfile(recipe_cache_path): + with open(recipe_cache_path, "r", encoding="utf-8") as fp: + cache_contents = fp.read() + if recipe.get_deep_hash() in cache_contents and not args.force: + log.warn(f"{recipe.get_name()} is already deployed. Skipping...") + continue started_recipe = datetime.now() log.info( f"Started {recipe.get_name()} recipe deployment at " diff --git a/zdeploy/clients.py b/zdeploy/clients.py index 5079965..28e7816 100644 --- a/zdeploy/clients.py +++ b/zdeploy/clients.py @@ -7,6 +7,7 @@ class SSH(SSHClient): """SSH helper that exposes a simple execute function.""" + # pylint: disable=too-many-arguments,too-many-positional-arguments def __init__(self, recipe, log, hostname, username, password, port): """Establish an SSH connection to ``hostname``.""" @@ -43,7 +44,7 @@ def execute( if show_error: self.log.fail(f"Failed to run '{cmd}'. Exit code: {rc}") if bail_on_failure: - raise Exception(f"failed to execute {cmd}") + raise RuntimeError(f"failed to execute {cmd}") return rc diff --git a/zdeploy/config.py b/zdeploy/config.py index b6f8f7c..ee0aeb4 100644 --- a/zdeploy/config.py +++ b/zdeploy/config.py @@ -1,4 +1,5 @@ """Configuration loader for zdeploy.""" +# pylint: disable=too-many-instance-attributes from dataclasses import dataclass from json import loads @@ -6,7 +7,7 @@ from typing import Any, Dict, cast -@dataclass +@dataclass # pylint: disable=too-many-instance-attributes class Config: """Simple container for configuration settings.""" diff --git a/zdeploy/recipe.py b/zdeploy/recipe.py index c4ad5bf..bce1f12 100644 --- a/zdeploy/recipe.py +++ b/zdeploy/recipe.py @@ -1,4 +1,5 @@ """Recipe abstraction for deploying and tracking dependencies.""" +# pylint: disable=too-many-instance-attributes,too-few-public-methods,too-many-arguments,too-many-positional-arguments from os import listdir from os.path import isdir, isfile @@ -91,6 +92,11 @@ def __eq__(self, other): return hash(self) == hash(other) + def is_virtual(self): + """Return ``True`` if this is a virtual recipe.""" + + return self._type == self.Type.VIRTUAL + def get_deep_hash(self, dir_path=None): """Return an MD5 hash representing the recipe and its requirements.""" @@ -109,7 +115,7 @@ def get_deep_hash(self, dir_path=None): f"chmod +x {dir_path}/hash && bash {self.config} && ./{dir_path}/hash" ) if cmd_rc != 0: - raise Exception(cmd_out) + raise RuntimeError(cmd_out) hashes += cmd_out for recipe in self.get_requirements(): @@ -118,7 +124,8 @@ def get_deep_hash(self, dir_path=None): for node in listdir(dir_path): rel_path = f"{dir_path}/{node}" if isfile(rel_path): - file_hash = md5(open(rel_path, "rb").read()).hexdigest() + with open(rel_path, "rb") as fp: + file_hash = md5(fp.read()).hexdigest() hashes += file_hash elif isdir(rel_path): hashes += self.get_deep_hash(rel_path) @@ -130,24 +137,25 @@ def get_requirements(self): req_file = f"{self.cfg.recipes}/{self.recipe}/require" requirements = [] if isfile(req_file): - for requirement in open(req_file).read().split("\n"): - requirement = requirement.strip() - if requirement == "" or requirement.startswith("#"): - continue - recipe = Recipe( - recipe=requirement, - parent_recipe=self.recipe, - config=self.config, - hostname=self.hostname, - username=self.username, - password=self.password, - port=self.port, - log=self.log, - cfg=self.cfg, - ) - for req in recipe.get_requirements(): - requirements.append(req) - requirements.append(recipe) + with open(req_file, "r", encoding="utf-8") as req_fp: + for requirement in req_fp.read().split("\n"): + requirement = requirement.strip() + if requirement == "" or requirement.startswith("#"): + continue + recipe = Recipe( + recipe=requirement, + parent_recipe=self.recipe, + config=self.config, + hostname=self.hostname, + username=self.username, + password=self.password, + port=self.port, + log=self.log, + cfg=self.cfg, + ) + for req in recipe.get_requirements(): + requirements.append(req) + requirements.append(recipe) return requirements def deploy(self): @@ -190,7 +198,8 @@ def deploy(self): show_command=False, ) passed = True - except Exception: + except Exception as exc: # pylint: disable=broad-except + self.log.fail(str(exc)) passed = False finally: if self._type == self.Type.DEFINED: diff --git a/zdeploy/recipeset.py b/zdeploy/recipeset.py index 0d7ac9c..2fd958a 100644 --- a/zdeploy/recipeset.py +++ b/zdeploy/recipeset.py @@ -1,7 +1,6 @@ """Helper for managing sets of ``Recipe`` objects.""" from hashlib import md5 -from zdeploy.recipe import Recipe class RecipeSet: @@ -32,7 +31,7 @@ def add_recipe(self, recipe): return self.log.info(f"Adding '{recipe.get_name()}' to the recipes list") self.recipes.append(recipe) - if recipe._type == Recipe.Type.VIRTUAL: + if recipe.is_virtual(): self.log.warn( ( f"'{recipe.recipe}' doesn't correspond to anything defined " diff --git a/zdeploy/shell.py b/zdeploy/shell.py index fcd3454..71b1c20 100644 --- a/zdeploy/shell.py +++ b/zdeploy/shell.py @@ -6,13 +6,13 @@ def execute(cmd): """Execute ``cmd`` in a shell and return output and return code.""" - proc = subprocess.Popen( + with subprocess.Popen( f"{cmd} 2>&1", stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True, universal_newlines=True, - ) - std_out, _ = proc.communicate() - rc = proc.returncode + ) as proc: + std_out, _ = proc.communicate() + rc = proc.returncode return std_out, rc From 47388af6a0fc2f48c49d8acb42de62a001093cbc Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 14:41:59 -0700 Subject: [PATCH 11/19] Add type hints --- zdeploy/__init__.py | 18 ++++++--------- zdeploy/app.py | 16 ++++++++++++-- zdeploy/clients.py | 34 +++++++++++++++++++---------- zdeploy/log.py | 23 ++++++++++---------- zdeploy/recipe.py | 52 ++++++++++++++++++++++++-------------------- zdeploy/recipeset.py | 17 ++++++++++----- zdeploy/shell.py | 3 ++- zdeploy/utils.py | 10 +++++++-- 8 files changed, 105 insertions(+), 68 deletions(-) diff --git a/zdeploy/__init__.py b/zdeploy/__init__.py index 2d60280..874e869 100644 --- a/zdeploy/__init__.py +++ b/zdeploy/__init__.py @@ -1,6 +1,6 @@ """Command line interface for zdeploy.""" -from argparse import ArgumentParser +from argparse import ArgumentParser, Namespace from os import listdir, makedirs from os.path import isdir from sys import stdout @@ -8,12 +8,11 @@ from zdeploy.log import Log from zdeploy.app import deploy -from zdeploy.config import load as load_config +from zdeploy.config import load as load_config, Config -def str2bool(value): +def str2bool(value: str | bool) -> bool: """Return a boolean for common yes/no strings.""" - if isinstance(value, bool): return value if value.lower() in ("yes", "y"): @@ -23,9 +22,8 @@ def str2bool(value): raise ValueError(f"Invalid value: {value}") -def handle_config(config_name, args, cfg): +def handle_config(config_name: str, args: Namespace, cfg: Config) -> None: """Deploy a single configuration.""" - log_dir_path = f"{cfg.logs}/{config_name}" cache_dir_path = f"{cfg.cache}/{config_name}" if not isdir(log_dir_path): @@ -43,16 +41,14 @@ def handle_config(config_name, args, cfg): deploy(config_name, cache_dir_path, log, args, cfg) -def handle_configs(args, cfg): +def handle_configs(args: Namespace, cfg: Config) -> None: """Deploy each config provided on the command line.""" for config_name in args.configs: handle_config(config_name, args, cfg) -def main(): +def main() -> None: """CLI entry point.""" - - # Default config file name is config.json, so it needs not be specified in our case. cfg = load_config() parser = ArgumentParser() parser.add_argument( @@ -69,7 +65,7 @@ def main(): help="Force full deployment (overlooks the cache)", nargs="?", required=False, - default=cfg.force, # Default behavior can be defined by the user in a config file + default=cfg.force, const=True, type=str2bool, ) diff --git a/zdeploy/app.py b/zdeploy/app.py index 055374f..cf1eca1 100644 --- a/zdeploy/app.py +++ b/zdeploy/app.py @@ -4,13 +4,23 @@ from os.path import isdir, isfile from shutil import rmtree from datetime import datetime +from argparse import Namespace + from dotenv import load_dotenv from zdeploy.recipe import Recipe from zdeploy.recipeset import RecipeSet from zdeploy.utils import reformat_time +from zdeploy.log import Log +from zdeploy.config import Config -def deploy(config_name, cache_dir_path, log, args, cfg): # pylint: disable=too-many-locals +def deploy( + config_name: str, + cache_dir_path: str, + log: Log, + args: Namespace, + cfg: Config, +) -> None: # pylint: disable=too-many-locals """Deploy recipes defined in ``config_name``.""" config_path = f"{cfg.configs}/{config_name}" print("Config:", config_path) @@ -26,9 +36,11 @@ def deploy(config_name, cache_dir_path, log, args, cfg): # pylint: disable=too- host_ip = environ.get(recipe_name) if host_ip is None: log.fatal(f"{recipe_name} is undefined in {config_path}") + raise RuntimeError("undefined host") host_user = environ.get(f"{recipe_name}_USER", cfg.user) host_password = environ.get(f"{recipe_name}_PASSWORD", cfg.password) - host_port = environ.get(f"{recipe_name}_PORT", cfg.port) + host_port_str = environ.get(f"{recipe_name}_PORT") + host_port = int(host_port_str) if host_port_str is not None else cfg.port recipe = Recipe( recipe_name, None, diff --git a/zdeploy/clients.py b/zdeploy/clients.py index 28e7816..c658a67 100644 --- a/zdeploy/clients.py +++ b/zdeploy/clients.py @@ -1,14 +1,24 @@ """Remote client helpers for recipes.""" -from paramiko import SSHClient, AutoAddPolicy +from paramiko import SSHClient, AutoAddPolicy, Transport from scp import SCPClient +from zdeploy.log import Log + class SSH(SSHClient): """SSH helper that exposes a simple execute function.""" # pylint: disable=too-many-arguments,too-many-positional-arguments - def __init__(self, recipe, log, hostname, username, password, port): + def __init__( + self, + recipe: str, + log: Log, + hostname: str, + username: str, + password: str | None, + port: int, + ) -> None: """Establish an SSH connection to ``hostname``.""" super().__init__() @@ -18,19 +28,19 @@ def __init__(self, recipe, log, hostname, username, password, port): self.recipe = recipe self.log = log - def __del__(self): + def __del__(self) -> None: """Ensure the SSH connection is closed.""" self.close() def execute( self, - *args, - bail_on_failure=True, - show_command=True, - show_output=True, - show_error=True, - ): + *args: str, + bail_on_failure: bool = True, + show_command: bool = True, + show_output: bool = True, + show_error: bool = True, + ) -> int: """Run ``args`` over SSH and return the exit code.""" cmd = " ".join(args) if show_command: @@ -51,17 +61,17 @@ def execute( class SCP(SCPClient): """SCP helper for transferring files over SSH.""" - def __init__(self, transport): + def __init__(self, transport: Transport) -> None: """Create an SCP client using ``transport``.""" super().__init__(transport) - def __del__(self): + def __del__(self) -> None: """Ensure the SCP connection is closed.""" self.close() - def upload(self, src, dest): + def upload(self, src: str, dest: str) -> None: """Upload ``src`` to ``dest`` on the remote host.""" self.put(src, remote_path=dest) diff --git a/zdeploy/log.py b/zdeploy/log.py index 37cbe66..05e4a4a 100644 --- a/zdeploy/log.py +++ b/zdeploy/log.py @@ -1,6 +1,7 @@ """Simple logging utilities.""" import sys +from typing import IO, Iterable class Log: """ @@ -12,62 +13,62 @@ class Log: logger.register_logger(open('mylog.log', 'w')) """ - def __init__(self, *loggers): + def __init__(self, *loggers: IO[str]) -> None: """Create a new ``Log`` instance and register ``loggers`` if provided.""" self.loggers = list(loggers) - def register_logger(self, logger): + def register_logger(self, logger: IO[str]) -> None: """Register a single ``logger`` object.""" self.loggers.append(logger) - def register_loggers(self, loggers): + def register_loggers(self, loggers: Iterable[IO[str]]) -> None: """Register multiple ``loggers`` objects.""" for logger in loggers: self.register_logger(logger) - def write(self, *args): + def write(self, *args: str) -> None: """Write ``args`` to all registered loggers.""" message = " ".join(args) for logger in self.loggers: logger.write(f"{message}\n") - def fatal(self, *args): + def fatal(self, *args: str) -> None: """Log ``args`` as a failure and terminate the program.""" self.fail(*args) sys.exit(1) - def fail(self, *args): + def fail(self, *args: str) -> None: """Log ``args`` as an error.""" self.write("\033[0;31m", *args, "\033[0;00m") - def warn(self, *args): + def warn(self, *args: str) -> None: """Log ``args`` as a warning.""" self.write("\033[1;33m", *args, "\033[0;00m") - def success(self, *args): + def success(self, *args: str) -> None: """Log ``args`` as a success message.""" self.write("\033[0;32m", *args, "\033[0;00m") - def info(self, *args): + def info(self, *args: str) -> None: """Log ``args`` as an informational message.""" self.write("\033[1;35m", *args, "\033[0;00m") - def close(self): + def close(self) -> None: """Close all registered loggers.""" for logger in self.loggers: logger.close() - def __del__(self): + def __del__(self) -> None: """Ensure all loggers are closed when this object is destroyed.""" self.close() diff --git a/zdeploy/recipe.py b/zdeploy/recipe.py index bce1f12..9c5ab06 100644 --- a/zdeploy/recipe.py +++ b/zdeploy/recipe.py @@ -4,8 +4,12 @@ from os import listdir from os.path import isdir, isfile from hashlib import md5 +from typing import Dict, List, Optional + from zdeploy.clients import SSH, SCP from zdeploy.shell import execute as shell_execute +from zdeploy.log import Log +from zdeploy.config import Config class Recipe: @@ -19,16 +23,16 @@ class Type: def __init__( self, - recipe, - parent_recipe, - config, - hostname, - username, - password, - port, - log, - cfg, - ): + recipe: str, + parent_recipe: Optional[str], + config: str, + hostname: str, + username: str, + password: str | None, + port: int, + log: Log, + cfg: Config, + ) -> None: """Initialize a recipe instance.""" self.log = log @@ -50,14 +54,14 @@ def __init__( self.hostname = hostname self.username = username self.password = password - self.properties = {} + self.properties: Dict[str, str | None] = {} - def set_property(self, key, value): + def set_property(self, key: str, value: str | None) -> None: """Store an arbitrary ``key``/``value`` pair.""" self.properties[key] = value - def set_recipe_name_and_type(self, recipe): + def set_recipe_name_and_type(self, recipe: str) -> None: """Resolve ``recipe`` name and determine if it is defined or virtual.""" for r in listdir(self.cfg.recipes): @@ -72,32 +76,32 @@ def set_recipe_name_and_type(self, recipe): self.recipe = recipe self._type = self.Type.VIRTUAL - def __str__(self): + def __str__(self) -> str: """Return a string representation of this recipe.""" return f"{self.recipe} -> {self.username}@{self.hostname}:{self.port} :: {self.properties}" - def get_name(self): + def get_name(self) -> str: """Return the recipe name.""" return self.recipe - def __hash__(self): + def __hash__(self) -> int: """Return a hash so recipes can be used in sets and dictionaries.""" return hash(str(self)) - def __eq__(self, other): + def __eq__(self, other: object) -> bool: """Compare recipes by their hash.""" return hash(self) == hash(other) - def is_virtual(self): + def is_virtual(self) -> bool: """Return ``True`` if this is a virtual recipe.""" return self._type == self.Type.VIRTUAL - def get_deep_hash(self, dir_path=None): + def get_deep_hash(self, dir_path: str | None = None) -> str: """Return an MD5 hash representing the recipe and its requirements.""" hashes = "" @@ -131,11 +135,11 @@ def get_deep_hash(self, dir_path=None): hashes += self.get_deep_hash(rel_path) return md5(hashes.encode()).hexdigest() - def get_requirements(self): + def get_requirements(self) -> List["Recipe"]: """Return a list of Recipe objects this recipe depends on.""" req_file = f"{self.cfg.recipes}/{self.recipe}/require" - requirements = [] + requirements: List["Recipe"] = [] if isfile(req_file): with open(req_file, "r", encoding="utf-8") as req_fp: for requirement in req_fp.read().split("\n"): @@ -158,7 +162,7 @@ def get_requirements(self): requirements.append(recipe) return requirements - def deploy(self): + def deploy(self) -> None: """Deploy this recipe using SSH/SCP.""" self.log.info(f"Deploying {self.recipe} to {self.hostname}") @@ -174,7 +178,9 @@ def deploy(self): if self._type == self.Type.DEFINED: ssh.execute(f"rm -rf /opt/{self.recipe}", show_command=False) - scp = SCP(ssh.get_transport()) + transport = ssh.get_transport() + assert transport is not None + scp = SCP(transport) scp.put( f"{self.cfg.recipes}/{self.recipe}", remote_path=f"/opt/{self.recipe}", diff --git a/zdeploy/recipeset.py b/zdeploy/recipeset.py index 2fd958a..5de4f0c 100644 --- a/zdeploy/recipeset.py +++ b/zdeploy/recipeset.py @@ -1,6 +1,11 @@ """Helper for managing sets of ``Recipe`` objects.""" from hashlib import md5 +from typing import Iterable, Iterator, List + +from zdeploy.config import Config +from zdeploy.log import Log +from zdeploy.recipe import Recipe class RecipeSet: @@ -8,20 +13,20 @@ class RecipeSet: RecipeSet is a unique set of Recipes maintainer. """ - def __init__(self, cfg, log): + def __init__(self, cfg: Config, log: Log) -> None: """Create an empty ``RecipeSet`` using ``cfg`` and ``log``.""" - self.recipes = [] + self.recipes: List[Recipe] = [] self.cfg = cfg self.log = log - def add_recipes(self, recipes): + def add_recipes(self, recipes: Iterable[Recipe]) -> None: """Add a sequence of ``recipes`` to the set.""" for recipe in recipes: self.add_recipe(recipe) - def add_recipe(self, recipe): + def add_recipe(self, recipe: Recipe) -> None: """Add a single ``recipe`` if not already present.""" if recipe in self.recipes: @@ -51,7 +56,7 @@ def add_recipe(self, recipe): ) ) - def get_hash(self): + def get_hash(self) -> str: """ Return an MD5 hash out of the hash of all recipes combined. The end result is used to create a cache directory under deployments cache. @@ -60,7 +65,7 @@ def get_hash(self): " ".join([str(recipe) for recipe in self.recipes]).encode() ).hexdigest() - def __iter__(self): + def __iter__(self) -> Iterator[Recipe]: """ Allow caller to iterate over recipes with a regular for loop, e.g.: for recipe in recipes: diff --git a/zdeploy/shell.py b/zdeploy/shell.py index 71b1c20..405e397 100644 --- a/zdeploy/shell.py +++ b/zdeploy/shell.py @@ -1,9 +1,10 @@ """Simple shell helpers.""" import subprocess +from typing import Tuple -def execute(cmd): +def execute(cmd: str) -> Tuple[str, int]: """Execute ``cmd`` in a shell and return output and return code.""" with subprocess.Popen( diff --git a/zdeploy/utils.py b/zdeploy/utils.py index 66e473f..c43ba42 100644 --- a/zdeploy/utils.py +++ b/zdeploy/utils.py @@ -1,8 +1,14 @@ """Utility helpers for zdeploy.""" -def reformat_time(time): +from datetime import timedelta +from typing import Union + + +def reformat_time(time: Union[str, timedelta]) -> str: """Return ``time`` in "Nh, Nm, and Ns" format.""" - h, m, s = [int(float(x)) for x in (f"{time}").split(":")] + if isinstance(time, timedelta): + time = str(time) + h, m, s = [int(float(x)) for x in time.split(":")] return f"{h}h, {m}m, and {s}s" From 567772b81cbd43e78eaccf08c367c5bca88d44ee Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 14:50:16 -0700 Subject: [PATCH 12/19] Refactor deploy to reduce complexity --- zdeploy/app.py | 128 +++++++++++++++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 52 deletions(-) diff --git a/zdeploy/app.py b/zdeploy/app.py index cf1eca1..53eca6c 100644 --- a/zdeploy/app.py +++ b/zdeploy/app.py @@ -14,20 +14,12 @@ from zdeploy.config import Config -def deploy( - config_name: str, - cache_dir_path: str, - log: Log, - args: Namespace, - cfg: Config, -) -> None: # pylint: disable=too-many-locals - """Deploy recipes defined in ``config_name``.""" - config_path = f"{cfg.configs}/{config_name}" - print("Config:", config_path) +def _load_recipes(config_path: str, log: Log, cfg: Config) -> RecipeSet: + """Return a ``RecipeSet`` loaded from environment variables.""" + load_dotenv(config_path) recipes = RecipeSet(cfg, log) - recipe_names = environ.get("RECIPES", "") if recipe_names.startswith("(") and recipe_names.endswith(")"): recipe_names = recipe_names[1:-1] @@ -41,6 +33,7 @@ def deploy( host_password = environ.get(f"{recipe_name}_PASSWORD", cfg.password) host_port_str = environ.get(f"{recipe_name}_PORT") host_port = int(host_port_str) if host_port_str is not None else cfg.port + recipe = Recipe( recipe_name, None, @@ -52,62 +45,93 @@ def deploy( log, cfg, ) + for env in environ: if env.startswith(recipe_name) and env != recipe_name: - # Properties aren't used anywhere internally. We only - # monitor them so hashes are generated properly. That - # said, if a recipe-name-related environment variable - # changes, we should assume a level of relevancy at - # the recipe level. recipe.set_property(env, environ.get(env)) + recipes.add_recipes(recipe.get_requirements()) recipes.add_recipe(recipe) + return recipes + + +def _clean_cache(cache_dir_path: str, deployment_cache_path: str, log: Log) -> None: + """Remove stale cache directories inside ``cache_dir_path``.""" + + if not isdir(deployment_cache_path): + makedirs(deployment_cache_path) + for directory in listdir(cache_dir_path): + path = f"{cache_dir_path}/{directory}" + if path != deployment_cache_path: + log.info(f"Deleting {path}") + rmtree(path) + + +def _deploy_recipe( + recipe: Recipe, + deployment_cache_path: str, + force: bool, + started_all: datetime, + log: Log, +) -> None: + """Deploy a single ``recipe`` and update its cache entry.""" + + recipe_cache_path = f"{deployment_cache_path}/{recipe.get_name()}" + if isfile(recipe_cache_path): + with open(recipe_cache_path, "r", encoding="utf-8") as fp: + cache_contents = fp.read() + if recipe.get_deep_hash() in cache_contents and not force: + log.warn(f"{recipe.get_name()} is already deployed. Skipping...") + return + + started_recipe = datetime.now() + log.info( + f"Started {recipe.get_name()} recipe deployment at " + f"{started_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" + ) + recipe.deploy() + ended_recipe = datetime.now() + log.info( + f"Ended {recipe.get_name()} recipe deployment at " + f"{ended_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" + ) + + total_recipe_time = ended_recipe - started_recipe + log.success(f"{recipe.get_name()} finished in {reformat_time(total_recipe_time)}") + with open(recipe_cache_path, "w", encoding="utf-8") as fp: + fp.write(recipe.get_deep_hash()) + + +def deploy( + config_name: str, + cache_dir_path: str, + log: Log, + args: Namespace, + cfg: Config, +) -> None: + """Deploy recipes defined in ``config_name``.""" + + config_path = f"{cfg.configs}/{config_name}" + print("Config:", config_path) + + recipes = _load_recipes(config_path, log, cfg) + started_all = datetime.now() log.info( f"Started {config_path} deployment at {started_all:%H:%M:%S} on {started_all:%Y-%m-%d}" ) + deployment_cache_path = f"{cache_dir_path}/{recipes.get_hash()}" - if not isdir(deployment_cache_path): - makedirs(deployment_cache_path) - for directory in listdir(cache_dir_path): - # Delete all stale cache tracks so we don't run into issues - # when reverting deployments. - directory = f"{cache_dir_path}/{directory}" - if directory != deployment_cache_path: - log.info(f"Deleting {directory}") - rmtree(directory) + _clean_cache(cache_dir_path, deployment_cache_path, log) + for recipe in recipes: - recipe_cache_path = f"{deployment_cache_path}/{recipe.get_name()}" - if isfile(recipe_cache_path): - with open(recipe_cache_path, "r", encoding="utf-8") as fp: - cache_contents = fp.read() - if recipe.get_deep_hash() in cache_contents and not args.force: - log.warn(f"{recipe.get_name()} is already deployed. Skipping...") - continue - started_recipe = datetime.now() - log.info( - f"Started {recipe.get_name()} recipe deployment at " - f"{started_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" - ) - recipe.deploy() - ended_recipe = datetime.now() - log.info( - f"Ended {recipe.get_name()} recipe deployment at " - f"{ended_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" - ) - total_recipe_time = ended_recipe - started_recipe - log.success( - f"{recipe.get_name()} finished in {reformat_time(total_recipe_time)}" - ) - with open(recipe_cache_path, "w", encoding="utf-8") as fp: - fp.write(recipe.get_deep_hash()) + _deploy_recipe(recipe, deployment_cache_path, args.force, started_all, log) + ended_all = datetime.now() total_deployment_time = ended_all - started_all log.info( f"Ended {config_path} deployment at {ended_all:%H:%M:%S} on {started_all:%Y-%m-%d}" ) - log.success( - f"{config_path} finished in {reformat_time(total_deployment_time)}" - ) + log.success(f"{config_path} finished in {reformat_time(total_deployment_time)}") log.info(f"Deployment hash is {recipes.get_hash()}") From 8b8bf5ec350d997794d79304f797933139fcec6d Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 15:24:42 -0700 Subject: [PATCH 13/19] Refactor str2bool and update configuration handling --- README.md | 2 +- tests/test_config.py | 1 + tests/test_utils.py | 8 ++++++++ zdeploy/__init__.py | 13 ++----------- zdeploy/config.py | 9 +++++++-- zdeploy/utils.py | 11 +++++++++++ 6 files changed, 30 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index bac181a..34d5b37 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ Here is a list of all supported config parameters to date: | cache | Deployment cache directory path | No | String | cache | | logs | Deployment logs directory path | No | String | logs | | installer | Default installer, used when an unrecognized dependency is found in the `require` file | No | String | apt-get install -y | -| force | Force entire deployment every time (default is to pick up with a previous failing deployment left off | No | String | no | +| force | Force entire deployment every time (default is to pick up with a previous failing deployment left off | No | Boolean | False | | user | Default username (used for recipes that don't specify a username, i.e. RECIPE_USER). | No | String | root | | password | Default password (used in case a private key isn't auto-detected). | No | String | None | | port | Default port number (used for recipes that don't specify a port number, i.e. RECIPE_PORT). | No | Integer | 22 | diff --git a/tests/test_config.py b/tests/test_config.py index db3d5d0..bf78757 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -6,3 +6,4 @@ def test_load_defaults(tmp_path): assert cfg.recipes == 'recipes' assert cfg.cache == 'cache' assert cfg.logs == 'logs' + assert cfg.force is False diff --git a/tests/test_utils.py b/tests/test_utils.py index d636322..4d9eff1 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,4 +1,12 @@ import zdeploy.utils as utils +import pytest def test_reformat_time(): assert utils.reformat_time('1:02:03') == '1h, 2m, and 3s' + + +def test_str2bool(): + assert utils.str2bool('yes') is True + assert utils.str2bool('no') is False + with pytest.raises(ValueError): + utils.str2bool('maybe') diff --git a/zdeploy/__init__.py b/zdeploy/__init__.py index 874e869..cb6f936 100644 --- a/zdeploy/__init__.py +++ b/zdeploy/__init__.py @@ -6,22 +6,13 @@ from sys import stdout from datetime import datetime +from zdeploy.utils import str2bool + from zdeploy.log import Log from zdeploy.app import deploy from zdeploy.config import load as load_config, Config -def str2bool(value: str | bool) -> bool: - """Return a boolean for common yes/no strings.""" - if isinstance(value, bool): - return value - if value.lower() in ("yes", "y"): - return True - if value.lower() in ("no", "n"): - return False - raise ValueError(f"Invalid value: {value}") - - def handle_config(config_name: str, args: Namespace, cfg: Config) -> None: """Deploy a single configuration.""" log_dir_path = f"{cfg.logs}/{config_name}" diff --git a/zdeploy/config.py b/zdeploy/config.py index ee0aeb4..b9f3bb2 100644 --- a/zdeploy/config.py +++ b/zdeploy/config.py @@ -6,6 +6,8 @@ from os.path import isfile from typing import Any, Dict, cast +from zdeploy.utils import str2bool + @dataclass # pylint: disable=too-many-instance-attributes class Config: @@ -16,7 +18,7 @@ class Config: cache: str = "cache" logs: str = "logs" installer: str = "apt-get install -y" - force: str = "no" + force: bool = False user: str = "root" password: str | None = None port: int = 22 @@ -47,7 +49,10 @@ def load(cfg_path: str = "config.json") -> Config: # Force is disabled by default. This sets the behavior to # only deploy undeployed recipes and/or pick up where a # previous deployment was halted or had crashed. - cfg["force"] = cfg.get("force", Config.force) + force_value = cfg.get("force", Config.force) + if isinstance(force_value, str): + force_value = str2bool(force_value) + cfg["force"] = force_value # Default username is root cfg["user"] = cfg.get("user", Config.user) diff --git a/zdeploy/utils.py b/zdeploy/utils.py index c43ba42..8044b73 100644 --- a/zdeploy/utils.py +++ b/zdeploy/utils.py @@ -5,6 +5,17 @@ from typing import Union +def str2bool(value: str) -> bool: + """Return ``True`` for 'yes' or 'y' and ``False`` for 'no' or 'n'.""" + + value = value.lower() + if value in ("yes", "y"): + return True + if value in ("no", "n"): + return False + raise ValueError(f"Invalid value: {value}") + + def reformat_time(time: Union[str, timedelta]) -> str: """Return ``time`` in "Nh, Nm, and Ns" format.""" From ca0f4794d1d25ca21151e1a034f3fca86b15fc3d Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 15:33:52 -0700 Subject: [PATCH 14/19] Improve str2bool --- tests/test_utils.py | 6 +++--- zdeploy/utils.py | 14 +++++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 4d9eff1..bdac598 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,5 +1,4 @@ import zdeploy.utils as utils -import pytest def test_reformat_time(): assert utils.reformat_time('1:02:03') == '1h, 2m, and 3s' @@ -7,6 +6,7 @@ def test_reformat_time(): def test_str2bool(): assert utils.str2bool('yes') is True + assert utils.str2bool('true') is True + assert utils.str2bool('enable') is True assert utils.str2bool('no') is False - with pytest.raises(ValueError): - utils.str2bool('maybe') + assert utils.str2bool('maybe') is False diff --git a/zdeploy/utils.py b/zdeploy/utils.py index 8044b73..6155f6c 100644 --- a/zdeploy/utils.py +++ b/zdeploy/utils.py @@ -2,18 +2,22 @@ from datetime import timedelta +from sys import stderr from typing import Union def str2bool(value: str) -> bool: - """Return ``True`` for 'yes' or 'y' and ``False`` for 'no' or 'n'.""" + """Return ``True`` if ``value`` is a truthy string.""" value = value.lower() - if value in ("yes", "y"): + if value in {"yes", "y", "true", "t", "e", "enable"}: return True - if value in ("no", "n"): - return False - raise ValueError(f"Invalid value: {value}") + + print( + f"'{value}' is not a recognized true value; treating as False", + file=stderr, + ) + return False def reformat_time(time: Union[str, timedelta]) -> str: From be1fe4108db20481db96d37ac54971d5cefe64a1 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 15:43:54 -0700 Subject: [PATCH 15/19] Refine paths and logs --- zdeploy/__init__.py | 27 +++++++++++------------ zdeploy/app.py | 51 ++++++++++++++++++++++---------------------- zdeploy/recipe.py | 41 ++++++++++++++++++----------------- zdeploy/recipeset.py | 15 ++++++------- zdeploy/utils.py | 2 +- 5 files changed, 67 insertions(+), 69 deletions(-) diff --git a/zdeploy/__init__.py b/zdeploy/__init__.py index cb6f936..1d1e514 100644 --- a/zdeploy/__init__.py +++ b/zdeploy/__init__.py @@ -1,10 +1,10 @@ """Command line interface for zdeploy.""" from argparse import ArgumentParser, Namespace -from os import listdir, makedirs -from os.path import isdir -from sys import stdout from datetime import datetime +from os import listdir +from pathlib import Path +from sys import stdout from zdeploy.utils import str2bool @@ -15,19 +15,16 @@ def handle_config(config_name: str, args: Namespace, cfg: Config) -> None: """Deploy a single configuration.""" - log_dir_path = f"{cfg.logs}/{config_name}" - cache_dir_path = f"{cfg.cache}/{config_name}" - if not isdir(log_dir_path): - makedirs(log_dir_path) - if not isdir(cache_dir_path): - makedirs(cache_dir_path) + log_dir_path = Path(cfg.logs) / config_name + cache_dir_path = Path(cfg.cache) / config_name + if not log_dir_path.is_dir(): + log_dir_path.mkdir(parents=True) + if not cache_dir_path.is_dir(): + cache_dir_path.mkdir(parents=True) log = Log() log.register_logger(stdout) - with open( - f"{log_dir_path}/{datetime.now():%Y-%m-%d %H:%M:%S}.log", - "w", - encoding="utf-8", - ) as log_file: + log_file_path = log_dir_path / f"{datetime.now():%Y-%m-%d %H:%M:%S}.log" + with log_file_path.open("w", encoding="utf-8") as log_file: log.register_logger(log_file) deploy(config_name, cache_dir_path, log, args, cfg) @@ -48,7 +45,7 @@ def main() -> None: help="Deployment destination(s)", nargs="+", required=True, - choices=listdir(cfg.configs) if isdir(cfg.configs) else (), + choices=listdir(cfg.configs) if Path(cfg.configs).is_dir() else (), ) parser.add_argument( "-f", diff --git a/zdeploy/app.py b/zdeploy/app.py index 53eca6c..bd40cc5 100644 --- a/zdeploy/app.py +++ b/zdeploy/app.py @@ -1,7 +1,7 @@ """Deployment core logic.""" -from os import listdir, makedirs, environ -from os.path import isdir, isfile +from os import environ +from pathlib import Path from shutil import rmtree from datetime import datetime from argparse import Namespace @@ -14,10 +14,10 @@ from zdeploy.config import Config -def _load_recipes(config_path: str, log: Log, cfg: Config) -> RecipeSet: +def _load_recipes(config_path: Path, log: Log, cfg: Config) -> RecipeSet: """Return a ``RecipeSet`` loaded from environment variables.""" - load_dotenv(config_path) + load_dotenv(str(config_path)) recipes = RecipeSet(cfg, log) recipe_names = environ.get("RECIPES", "") @@ -56,73 +56,74 @@ def _load_recipes(config_path: str, log: Log, cfg: Config) -> RecipeSet: return recipes -def _clean_cache(cache_dir_path: str, deployment_cache_path: str, log: Log) -> None: +def _clean_cache(cache_dir_path: Path, deployment_cache_path: Path, log: Log) -> None: """Remove stale cache directories inside ``cache_dir_path``.""" - if not isdir(deployment_cache_path): - makedirs(deployment_cache_path) - for directory in listdir(cache_dir_path): - path = f"{cache_dir_path}/{directory}" - if path != deployment_cache_path: - log.info(f"Deleting {path}") - rmtree(path) + if not deployment_cache_path.is_dir(): + deployment_cache_path.mkdir(parents=True) + for directory in cache_dir_path.iterdir(): + if directory != deployment_cache_path: + log.info(f"Removing stale cache directory {directory}") + rmtree(directory) def _deploy_recipe( recipe: Recipe, - deployment_cache_path: str, + deployment_cache_path: Path, force: bool, started_all: datetime, log: Log, ) -> None: """Deploy a single ``recipe`` and update its cache entry.""" - recipe_cache_path = f"{deployment_cache_path}/{recipe.get_name()}" - if isfile(recipe_cache_path): - with open(recipe_cache_path, "r", encoding="utf-8") as fp: + recipe_cache_path = deployment_cache_path / recipe.get_name() + if recipe_cache_path.is_file(): + with recipe_cache_path.open("r", encoding="utf-8") as fp: cache_contents = fp.read() if recipe.get_deep_hash() in cache_contents and not force: - log.warn(f"{recipe.get_name()} is already deployed. Skipping...") + log.warn( + f"Skipping {recipe.get_name()} because it is already deployed" + ) return started_recipe = datetime.now() log.info( - f"Started {recipe.get_name()} recipe deployment at " + f"Starting recipe '{recipe.get_name()}' at " f"{started_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" ) recipe.deploy() ended_recipe = datetime.now() log.info( - f"Ended {recipe.get_name()} recipe deployment at " + f"Finished recipe '{recipe.get_name()}' at " f"{ended_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" ) total_recipe_time = ended_recipe - started_recipe log.success(f"{recipe.get_name()} finished in {reformat_time(total_recipe_time)}") - with open(recipe_cache_path, "w", encoding="utf-8") as fp: + with recipe_cache_path.open("w", encoding="utf-8") as fp: fp.write(recipe.get_deep_hash()) def deploy( config_name: str, - cache_dir_path: str, + cache_dir_path: Path, log: Log, args: Namespace, cfg: Config, ) -> None: """Deploy recipes defined in ``config_name``.""" - config_path = f"{cfg.configs}/{config_name}" + config_path = Path(cfg.configs) / config_name print("Config:", config_path) recipes = _load_recipes(config_path, log, cfg) started_all = datetime.now() log.info( - f"Started {config_path} deployment at {started_all:%H:%M:%S} on {started_all:%Y-%m-%d}" + f"Starting deployment of {config_path} at {started_all:%H:%M:%S} on {started_all:%Y-%m-%d}" ) - deployment_cache_path = f"{cache_dir_path}/{recipes.get_hash()}" + deployment_cache_path = cache_dir_path / recipes.get_hash() _clean_cache(cache_dir_path, deployment_cache_path, log) for recipe in recipes: @@ -131,7 +132,7 @@ def deploy( ended_all = datetime.now() total_deployment_time = ended_all - started_all log.info( - f"Ended {config_path} deployment at {ended_all:%H:%M:%S} on {started_all:%Y-%m-%d}" + f"Completed deployment of {config_path} at {ended_all:%H:%M:%S} on {started_all:%Y-%m-%d}" ) log.success(f"{config_path} finished in {reformat_time(total_deployment_time)}") log.info(f"Deployment hash is {recipes.get_hash()}") diff --git a/zdeploy/recipe.py b/zdeploy/recipe.py index 9c5ab06..7b46b5d 100644 --- a/zdeploy/recipe.py +++ b/zdeploy/recipe.py @@ -2,7 +2,7 @@ # pylint: disable=too-many-instance-attributes,too-few-public-methods,too-many-arguments,too-many-positional-arguments from os import listdir -from os.path import isdir, isfile +from pathlib import Path from hashlib import md5 from typing import Dict, List, Optional @@ -25,7 +25,7 @@ def __init__( self, recipe: str, parent_recipe: Optional[str], - config: str, + config: Path, hostname: str, username: str, password: str | None, @@ -36,7 +36,7 @@ def __init__( """Initialize a recipe instance.""" self.log = log - if not config or not config.strip(): + if not str(config).strip(): self.log.fatal("Invalid value for config") if not recipe or not recipe.strip(): self.log.fatal("Invalid value for recipe") @@ -101,7 +101,7 @@ def is_virtual(self) -> bool: return self._type == self.Type.VIRTUAL - def get_deep_hash(self, dir_path: str | None = None) -> str: + def get_deep_hash(self, dir_path: Path | None = None) -> str: """Return an MD5 hash representing the recipe and its requirements.""" hashes = "" @@ -109,14 +109,15 @@ def get_deep_hash(self, dir_path: str | None = None) -> str: hashes = md5(self.recipe.encode()).hexdigest() elif self._type == self.Type.DEFINED: if dir_path is None: - dir_path = f"{self.cfg.recipes}/{self.recipe}" + dir_path = Path(self.cfg.recipes) / self.recipe # Execute the hash script and copy its output into our hashes variable. # NOTE: We perform this check specifically inside this block because when # dir_path is None, we know we're at the main recipe directory path. - if isfile(f"{dir_path}/hash"): + hash_path = dir_path / "hash" + if hash_path.is_file(): cmd_out, cmd_rc = shell_execute( - f"chmod +x {dir_path}/hash && bash {self.config} && ./{dir_path}/hash" + f"chmod +x {hash_path} && bash {self.config} && ./{hash_path}" ) if cmd_rc != 0: raise RuntimeError(cmd_out) @@ -126,22 +127,22 @@ def get_deep_hash(self, dir_path: str | None = None) -> str: hashes += recipe.get_deep_hash() hashes += md5(str(self).encode()).hexdigest() for node in listdir(dir_path): - rel_path = f"{dir_path}/{node}" - if isfile(rel_path): - with open(rel_path, "rb") as fp: + rel_path = dir_path / node + if rel_path.is_file(): + with rel_path.open("rb") as fp: file_hash = md5(fp.read()).hexdigest() hashes += file_hash - elif isdir(rel_path): + elif rel_path.is_dir(): hashes += self.get_deep_hash(rel_path) return md5(hashes.encode()).hexdigest() def get_requirements(self) -> List["Recipe"]: """Return a list of Recipe objects this recipe depends on.""" - req_file = f"{self.cfg.recipes}/{self.recipe}/require" + req_file = Path(self.cfg.recipes) / self.recipe / "require" requirements: List["Recipe"] = [] - if isfile(req_file): - with open(req_file, "r", encoding="utf-8") as req_fp: + if req_file.is_file(): + with req_file.open("r", encoding="utf-8") as req_fp: for requirement in req_fp.read().split("\n"): requirement = requirement.strip() if requirement == "" or requirement.startswith("#"): @@ -165,7 +166,7 @@ def get_requirements(self) -> List["Recipe"]: def deploy(self) -> None: """Deploy this recipe using SSH/SCP.""" - self.log.info(f"Deploying {self.recipe} to {self.hostname}") + self.log.info(f"Deploying '{self.recipe}' to {self.hostname}") ssh = SSH( recipe=self.recipe, log=self.log, @@ -182,21 +183,21 @@ def deploy(self) -> None: assert transport is not None scp = SCP(transport) scp.put( - f"{self.cfg.recipes}/{self.recipe}", + str(Path(self.cfg.recipes) / self.recipe), remote_path=f"/opt/{self.recipe}", recursive=True, ) - scp.put(self.config, remote_path=f"/opt/{self.recipe}/config") + scp.put(str(self.config), remote_path=f"/opt/{self.recipe}/config") try: if self._type == self.Type.VIRTUAL: ssh.execute(f"{self.cfg.installer} {self.recipe}") elif self._type == self.Type.DEFINED: - if not isfile(f"{self.cfg.recipes}/{self.recipe}/run"): + if not (Path(self.cfg.recipes) / self.recipe / "run").is_file(): # Recipes with no run file are acceptable since they (may) have a require file # and don't necessarily require the execution of anything of their own. self.log.warn( - f"{self.recipe} doesn't have a run file. Continuing..." + f"Recipe '{self.recipe}' has no run file; continuing" ) else: ssh.execute( @@ -209,7 +210,7 @@ def deploy(self) -> None: passed = False finally: if self._type == self.Type.DEFINED: - self.log.info(f"Deleting /opt/{self.recipe} from remote host") + self.log.info(f"Removing /opt/{self.recipe} from remote host") ssh.execute(f"rm -rf /opt/{self.recipe}", show_command=False) if not passed: diff --git a/zdeploy/recipeset.py b/zdeploy/recipeset.py index 5de4f0c..1b7f8a2 100644 --- a/zdeploy/recipeset.py +++ b/zdeploy/recipeset.py @@ -31,28 +31,27 @@ def add_recipe(self, recipe: Recipe) -> None: if recipe in self.recipes: self.log.warn( - f"{recipe.get_name()} is already added to the recipes list. Skipping..." + f"Recipe '{recipe.get_name()}' is already added; skipping" ) return - self.log.info(f"Adding '{recipe.get_name()}' to the recipes list") + self.log.info(f"Registering recipe '{recipe.get_name()}'") self.recipes.append(recipe) if recipe.is_virtual(): self.log.warn( ( - f"'{recipe.recipe}' doesn't correspond to anything defined " - f"under the {self.cfg.recipes} directory" + f"Recipe '{recipe.recipe}' is not found under {self.cfg.recipes} " + "and will be treated as a system package" ) ) self.log.warn( ( - "this recipe will be marked virtual and execute as " - f"`{recipe.cfg.installer} {recipe.recipe}`" + f"The package will be installed using `{recipe.cfg.installer} {recipe.recipe}`" ) ) self.log.warn( ( - "If you want to use a different package manager, add an " - "'installer' field to the config.json file" + "To use a different package manager, specify " + "an 'installer' entry in config.json" ) ) diff --git a/zdeploy/utils.py b/zdeploy/utils.py index 6155f6c..24c72ae 100644 --- a/zdeploy/utils.py +++ b/zdeploy/utils.py @@ -14,7 +14,7 @@ def str2bool(value: str) -> bool: return True print( - f"'{value}' is not a recognized true value; treating as False", + f"Warning: '{value}' is not recognized as a true value, defaulting to False", file=stderr, ) return False From 84828dc960ae3e38048d6c8f53402e48bb731bf8 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 16:18:03 -0700 Subject: [PATCH 16/19] Make RecipeSet iterable and rename helpers --- tests/test_recipeset.py | 25 ++++++++++++++++++++++++ zdeploy/__init__.py | 8 ++++---- zdeploy/app.py | 18 +++++++++--------- zdeploy/recipe.py | 19 ++++++++++--------- zdeploy/recipeset.py | 42 +++++++++++++---------------------------- 5 files changed, 61 insertions(+), 51 deletions(-) create mode 100644 tests/test_recipeset.py diff --git a/tests/test_recipeset.py b/tests/test_recipeset.py new file mode 100644 index 0000000..e451efa --- /dev/null +++ b/tests/test_recipeset.py @@ -0,0 +1,25 @@ +import zdeploy.recipe as recipe_mod +from zdeploy.recipeset import RecipeSet +from zdeploy.log import Log +from zdeploy.config import Config + + +def test_recipeset_iterable(tmp_path): + cfg = Config(recipes=str(tmp_path)) + log = Log() + r = recipe_mod.Recipe( + "pkg1", + None, + tmp_path / "cfg", + "host", + "user", + None, + 22, + log, + cfg, + ) + rs = RecipeSet(cfg, log) + rs.add(r) + for item in rs: + assert item is r + assert len(list(rs)) == 1 diff --git a/zdeploy/__init__.py b/zdeploy/__init__.py index 1d1e514..4d88268 100644 --- a/zdeploy/__init__.py +++ b/zdeploy/__init__.py @@ -13,7 +13,7 @@ from zdeploy.config import load as load_config, Config -def handle_config(config_name: str, args: Namespace, cfg: Config) -> None: +def deploy_config(config_name: str, args: Namespace, cfg: Config) -> None: """Deploy a single configuration.""" log_dir_path = Path(cfg.logs) / config_name cache_dir_path = Path(cfg.cache) / config_name @@ -29,10 +29,10 @@ def handle_config(config_name: str, args: Namespace, cfg: Config) -> None: deploy(config_name, cache_dir_path, log, args, cfg) -def handle_configs(args: Namespace, cfg: Config) -> None: +def deploy_configs(args: Namespace, cfg: Config) -> None: """Deploy each config provided on the command line.""" for config_name in args.configs: - handle_config(config_name, args, cfg) + deploy_config(config_name, args, cfg) def main() -> None: @@ -57,4 +57,4 @@ def main() -> None: const=True, type=str2bool, ) - handle_configs(parser.parse_args(), cfg) + deploy_configs(parser.parse_args(), cfg) diff --git a/zdeploy/app.py b/zdeploy/app.py index bd40cc5..f5733ec 100644 --- a/zdeploy/app.py +++ b/zdeploy/app.py @@ -50,8 +50,8 @@ def _load_recipes(config_path: Path, log: Log, cfg: Config) -> RecipeSet: if env.startswith(recipe_name) and env != recipe_name: recipe.set_property(env, environ.get(env)) - recipes.add_recipes(recipe.get_requirements()) - recipes.add_recipe(recipe) + recipes.update(recipe.load_requirements()) + recipes.add(recipe) return recipes @@ -76,32 +76,32 @@ def _deploy_recipe( ) -> None: """Deploy a single ``recipe`` and update its cache entry.""" - recipe_cache_path = deployment_cache_path / recipe.get_name() + recipe_cache_path = deployment_cache_path / recipe.name if recipe_cache_path.is_file(): with recipe_cache_path.open("r", encoding="utf-8") as fp: cache_contents = fp.read() - if recipe.get_deep_hash() in cache_contents and not force: + if recipe.deep_hash() in cache_contents and not force: log.warn( - f"Skipping {recipe.get_name()} because it is already deployed" + f"Skipping {recipe.name} because it is already deployed" ) return started_recipe = datetime.now() log.info( - f"Starting recipe '{recipe.get_name()}' at " + f"Starting recipe '{recipe.name}' at " f"{started_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" ) recipe.deploy() ended_recipe = datetime.now() log.info( - f"Finished recipe '{recipe.get_name()}' at " + f"Finished recipe '{recipe.name}' at " f"{ended_recipe:%H:%M:%S} on {started_all:%Y-%m-%d}" ) total_recipe_time = ended_recipe - started_recipe - log.success(f"{recipe.get_name()} finished in {reformat_time(total_recipe_time)}") + log.success(f"{recipe.name} finished in {reformat_time(total_recipe_time)}") with recipe_cache_path.open("w", encoding="utf-8") as fp: - fp.write(recipe.get_deep_hash()) + fp.write(recipe.deep_hash()) def deploy( diff --git a/zdeploy/recipe.py b/zdeploy/recipe.py index 7b46b5d..a1a25af 100644 --- a/zdeploy/recipe.py +++ b/zdeploy/recipe.py @@ -49,7 +49,7 @@ def __init__( self.cfg = cfg self.parent_recipe = parent_recipe - self.set_recipe_name_and_type(recipe) + self._resolve_name_and_type(recipe) self.config = config self.hostname = hostname self.username = username @@ -61,7 +61,7 @@ def set_property(self, key: str, value: str | None) -> None: self.properties[key] = value - def set_recipe_name_and_type(self, recipe: str) -> None: + def _resolve_name_and_type(self, recipe: str) -> None: """Resolve ``recipe`` name and determine if it is defined or virtual.""" for r in listdir(self.cfg.recipes): @@ -81,7 +81,8 @@ def __str__(self) -> str: return f"{self.recipe} -> {self.username}@{self.hostname}:{self.port} :: {self.properties}" - def get_name(self) -> str: + @property + def name(self) -> str: """Return the recipe name.""" return self.recipe @@ -101,7 +102,7 @@ def is_virtual(self) -> bool: return self._type == self.Type.VIRTUAL - def get_deep_hash(self, dir_path: Path | None = None) -> str: + def deep_hash(self, dir_path: Path | None = None) -> str: """Return an MD5 hash representing the recipe and its requirements.""" hashes = "" @@ -123,8 +124,8 @@ def get_deep_hash(self, dir_path: Path | None = None) -> str: raise RuntimeError(cmd_out) hashes += cmd_out - for recipe in self.get_requirements(): - hashes += recipe.get_deep_hash() + for recipe in self.load_requirements(): + hashes += recipe.deep_hash() hashes += md5(str(self).encode()).hexdigest() for node in listdir(dir_path): rel_path = dir_path / node @@ -133,10 +134,10 @@ def get_deep_hash(self, dir_path: Path | None = None) -> str: file_hash = md5(fp.read()).hexdigest() hashes += file_hash elif rel_path.is_dir(): - hashes += self.get_deep_hash(rel_path) + hashes += self.deep_hash(rel_path) return md5(hashes.encode()).hexdigest() - def get_requirements(self) -> List["Recipe"]: + def load_requirements(self) -> List["Recipe"]: """Return a list of Recipe objects this recipe depends on.""" req_file = Path(self.cfg.recipes) / self.recipe / "require" @@ -158,7 +159,7 @@ def get_requirements(self) -> List["Recipe"]: log=self.log, cfg=self.cfg, ) - for req in recipe.get_requirements(): + for req in recipe.load_requirements(): requirements.append(req) requirements.append(recipe) return requirements diff --git a/zdeploy/recipeset.py b/zdeploy/recipeset.py index 1b7f8a2..59816d6 100644 --- a/zdeploy/recipeset.py +++ b/zdeploy/recipeset.py @@ -1,41 +1,37 @@ """Helper for managing sets of ``Recipe`` objects.""" from hashlib import md5 -from typing import Iterable, Iterator, List +from typing import Iterable from zdeploy.config import Config from zdeploy.log import Log from zdeploy.recipe import Recipe -class RecipeSet: - """ - RecipeSet is a unique set of Recipes maintainer. - """ +class RecipeSet(set[Recipe]): + """Container for ``Recipe`` objects with convenience helpers.""" def __init__(self, cfg: Config, log: Log) -> None: """Create an empty ``RecipeSet`` using ``cfg`` and ``log``.""" - self.recipes: List[Recipe] = [] + super().__init__() self.cfg = cfg self.log = log - def add_recipes(self, recipes: Iterable[Recipe]) -> None: + def update(self, recipes: Iterable[Recipe]) -> None: # type: ignore[override] """Add a sequence of ``recipes`` to the set.""" for recipe in recipes: - self.add_recipe(recipe) + self.add(recipe) - def add_recipe(self, recipe: Recipe) -> None: + def add(self, recipe: Recipe) -> None: # type: ignore[override] """Add a single ``recipe`` if not already present.""" - if recipe in self.recipes: - self.log.warn( - f"Recipe '{recipe.get_name()}' is already added; skipping" - ) + if recipe in self: + self.log.warn(f"Recipe '{recipe.name}' is already added; skipping") return - self.log.info(f"Registering recipe '{recipe.get_name()}'") - self.recipes.append(recipe) + self.log.info(f"Registering recipe '{recipe.name}'") + super().add(recipe) if recipe.is_virtual(): self.log.warn( ( @@ -56,18 +52,6 @@ def add_recipe(self, recipe: Recipe) -> None: ) def get_hash(self) -> str: - """ - Return an MD5 hash out of the hash of all recipes combined. - The end result is used to create a cache directory under deployments cache. - """ - return md5( - " ".join([str(recipe) for recipe in self.recipes]).encode() - ).hexdigest() + """Return an MD5 hash of all recipes combined.""" - def __iter__(self) -> Iterator[Recipe]: - """ - Allow caller to iterate over recipes with a regular for loop, e.g.: - for recipe in recipes: - print(recipe) - """ - return iter(self.recipes) + return md5(" ".join(str(recipe) for recipe in self).encode()).hexdigest() From db0788d1d76b1c8ff0f3262033227d2d52daaab9 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 16:24:47 -0700 Subject: [PATCH 17/19] Use module-level logger --- zdeploy/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/zdeploy/__init__.py b/zdeploy/__init__.py index 4d88268..6588a61 100644 --- a/zdeploy/__init__.py +++ b/zdeploy/__init__.py @@ -12,6 +12,9 @@ from zdeploy.app import deploy from zdeploy.config import load as load_config, Config +log = Log() +log.register_logger(stdout) + def deploy_config(config_name: str, args: Namespace, cfg: Config) -> None: """Deploy a single configuration.""" @@ -21,12 +24,11 @@ def deploy_config(config_name: str, args: Namespace, cfg: Config) -> None: log_dir_path.mkdir(parents=True) if not cache_dir_path.is_dir(): cache_dir_path.mkdir(parents=True) - log = Log() - log.register_logger(stdout) log_file_path = log_dir_path / f"{datetime.now():%Y-%m-%d %H:%M:%S}.log" with log_file_path.open("w", encoding="utf-8") as log_file: log.register_logger(log_file) deploy(config_name, cache_dir_path, log, args, cfg) + log.loggers.remove(log_file) def deploy_configs(args: Namespace, cfg: Config) -> None: From 2fe3e903f27f92ae0167f36f646aca9e66d1c647 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 17:16:54 -0700 Subject: [PATCH 18/19] Remove module-level logger --- zdeploy/__init__.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/zdeploy/__init__.py b/zdeploy/__init__.py index 6588a61..18248aa 100644 --- a/zdeploy/__init__.py +++ b/zdeploy/__init__.py @@ -12,9 +12,6 @@ from zdeploy.app import deploy from zdeploy.config import load as load_config, Config -log = Log() -log.register_logger(stdout) - def deploy_config(config_name: str, args: Namespace, cfg: Config) -> None: """Deploy a single configuration.""" @@ -25,10 +22,12 @@ def deploy_config(config_name: str, args: Namespace, cfg: Config) -> None: if not cache_dir_path.is_dir(): cache_dir_path.mkdir(parents=True) log_file_path = log_dir_path / f"{datetime.now():%Y-%m-%d %H:%M:%S}.log" + logger = Log() + logger.register_logger(stdout) with log_file_path.open("w", encoding="utf-8") as log_file: - log.register_logger(log_file) - deploy(config_name, cache_dir_path, log, args, cfg) - log.loggers.remove(log_file) + logger.register_logger(log_file) + deploy(config_name, cache_dir_path, logger, args, cfg) + logger.loggers.remove(log_file) def deploy_configs(args: Namespace, cfg: Config) -> None: From 52adb6129023d5a03867770a1c15f205435360e4 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Al-Kass Date: Wed, 2 Jul 2025 21:11:01 -0700 Subject: [PATCH 19/19] Use built-in logging --- tests/test_recipeset.py | 4 +-- zdeploy/__init__.py | 22 ++++++++---- zdeploy/app.py | 20 +++++------ zdeploy/clients.py | 9 +++-- zdeploy/log.py | 74 ----------------------------------------- zdeploy/recipe.py | 32 ++++++++++-------- zdeploy/recipeset.py | 14 ++++---- zdeploy/utils.py | 7 ++-- 8 files changed, 59 insertions(+), 123 deletions(-) delete mode 100644 zdeploy/log.py diff --git a/tests/test_recipeset.py b/tests/test_recipeset.py index e451efa..2045def 100644 --- a/tests/test_recipeset.py +++ b/tests/test_recipeset.py @@ -1,12 +1,12 @@ import zdeploy.recipe as recipe_mod from zdeploy.recipeset import RecipeSet -from zdeploy.log import Log +import logging from zdeploy.config import Config def test_recipeset_iterable(tmp_path): cfg = Config(recipes=str(tmp_path)) - log = Log() + log = logging.getLogger("test") r = recipe_mod.Recipe( "pkg1", None, diff --git a/zdeploy/__init__.py b/zdeploy/__init__.py index 18248aa..9944196 100644 --- a/zdeploy/__init__.py +++ b/zdeploy/__init__.py @@ -2,13 +2,13 @@ from argparse import ArgumentParser, Namespace from datetime import datetime +import logging from os import listdir from pathlib import Path from sys import stdout from zdeploy.utils import str2bool -from zdeploy.log import Log from zdeploy.app import deploy from zdeploy.config import load as load_config, Config @@ -22,12 +22,22 @@ def deploy_config(config_name: str, args: Namespace, cfg: Config) -> None: if not cache_dir_path.is_dir(): cache_dir_path.mkdir(parents=True) log_file_path = log_dir_path / f"{datetime.now():%Y-%m-%d %H:%M:%S}.log" - logger = Log() - logger.register_logger(stdout) - with log_file_path.open("w", encoding="utf-8") as log_file: - logger.register_logger(log_file) + + logger = logging.getLogger(config_name) + logger.setLevel(logging.INFO) + formatter = logging.Formatter("%(message)s") + stream_handler = logging.StreamHandler(stdout) + stream_handler.setFormatter(formatter) + file_handler = logging.FileHandler(log_file_path, encoding="utf-8") + file_handler.setFormatter(formatter) + logger.addHandler(stream_handler) + logger.addHandler(file_handler) + + try: deploy(config_name, cache_dir_path, logger, args, cfg) - logger.loggers.remove(log_file) + finally: + logger.removeHandler(file_handler) + file_handler.close() def deploy_configs(args: Namespace, cfg: Config) -> None: diff --git a/zdeploy/app.py b/zdeploy/app.py index f5733ec..f6e67bc 100644 --- a/zdeploy/app.py +++ b/zdeploy/app.py @@ -5,16 +5,16 @@ from shutil import rmtree from datetime import datetime from argparse import Namespace +import logging from dotenv import load_dotenv from zdeploy.recipe import Recipe from zdeploy.recipeset import RecipeSet from zdeploy.utils import reformat_time -from zdeploy.log import Log from zdeploy.config import Config -def _load_recipes(config_path: Path, log: Log, cfg: Config) -> RecipeSet: +def _load_recipes(config_path: Path, log: logging.Logger, cfg: Config) -> RecipeSet: """Return a ``RecipeSet`` loaded from environment variables.""" load_dotenv(str(config_path)) @@ -27,7 +27,7 @@ def _load_recipes(config_path: Path, log: Log, cfg: Config) -> RecipeSet: recipe_name = recipe_name.strip() host_ip = environ.get(recipe_name) if host_ip is None: - log.fatal(f"{recipe_name} is undefined in {config_path}") + log.error(f"{recipe_name} is undefined in {config_path}") raise RuntimeError("undefined host") host_user = environ.get(f"{recipe_name}_USER", cfg.user) host_password = environ.get(f"{recipe_name}_PASSWORD", cfg.password) @@ -56,7 +56,7 @@ def _load_recipes(config_path: Path, log: Log, cfg: Config) -> RecipeSet: return recipes -def _clean_cache(cache_dir_path: Path, deployment_cache_path: Path, log: Log) -> None: +def _clean_cache(cache_dir_path: Path, deployment_cache_path: Path, log: logging.Logger) -> None: """Remove stale cache directories inside ``cache_dir_path``.""" if not deployment_cache_path.is_dir(): @@ -72,7 +72,7 @@ def _deploy_recipe( deployment_cache_path: Path, force: bool, started_all: datetime, - log: Log, + log: logging.Logger, ) -> None: """Deploy a single ``recipe`` and update its cache entry.""" @@ -81,7 +81,7 @@ def _deploy_recipe( with recipe_cache_path.open("r", encoding="utf-8") as fp: cache_contents = fp.read() if recipe.deep_hash() in cache_contents and not force: - log.warn( + log.warning( f"Skipping {recipe.name} because it is already deployed" ) return @@ -99,7 +99,7 @@ def _deploy_recipe( ) total_recipe_time = ended_recipe - started_recipe - log.success(f"{recipe.name} finished in {reformat_time(total_recipe_time)}") + log.info(f"{recipe.name} finished in {reformat_time(total_recipe_time)}") with recipe_cache_path.open("w", encoding="utf-8") as fp: fp.write(recipe.deep_hash()) @@ -107,14 +107,14 @@ def _deploy_recipe( def deploy( config_name: str, cache_dir_path: Path, - log: Log, + log: logging.Logger, args: Namespace, cfg: Config, ) -> None: """Deploy recipes defined in ``config_name``.""" config_path = Path(cfg.configs) / config_name - print("Config:", config_path) + log.info("Config: %s", config_path) recipes = _load_recipes(config_path, log, cfg) @@ -134,5 +134,5 @@ def deploy( log.info( f"Completed deployment of {config_path} at {ended_all:%H:%M:%S} on {started_all:%Y-%m-%d}" ) - log.success(f"{config_path} finished in {reformat_time(total_deployment_time)}") + log.info(f"{config_path} finished in {reformat_time(total_deployment_time)}") log.info(f"Deployment hash is {recipes.get_hash()}") diff --git a/zdeploy/clients.py b/zdeploy/clients.py index c658a67..fc20259 100644 --- a/zdeploy/clients.py +++ b/zdeploy/clients.py @@ -1,10 +1,9 @@ """Remote client helpers for recipes.""" +import logging from paramiko import SSHClient, AutoAddPolicy, Transport from scp import SCPClient -from zdeploy.log import Log - class SSH(SSHClient): """SSH helper that exposes a simple execute function.""" @@ -13,7 +12,7 @@ class SSH(SSHClient): def __init__( self, recipe: str, - log: Log, + log: logging.Logger, hostname: str, username: str, password: str | None, @@ -44,7 +43,7 @@ def execute( """Run ``args`` over SSH and return the exit code.""" cmd = " ".join(args) if show_command: - self.log.info("Running", cmd) + self.log.info("Running %s", cmd) _, stdout, _ = self.exec_command(f"{cmd} 2>&1") if show_output: for line in stdout: @@ -52,7 +51,7 @@ def execute( rc = stdout.channel.recv_exit_status() if rc != 0: if show_error: - self.log.fail(f"Failed to run '{cmd}'. Exit code: {rc}") + self.log.error("Failed to run '%s'. Exit code: %s", cmd, rc) if bail_on_failure: raise RuntimeError(f"failed to execute {cmd}") return rc diff --git a/zdeploy/log.py b/zdeploy/log.py deleted file mode 100644 index 05e4a4a..0000000 --- a/zdeploy/log.py +++ /dev/null @@ -1,74 +0,0 @@ -"""Simple logging utilities.""" - -import sys -from typing import IO, Iterable - -class Log: - """ - Log is a generic logging class that can write to anything with - a `write` function. The recommended use of this class is to log - to the standard output (stdout) and to a file, e.g.: - logger = Log() - logger.register_logger(sys.stdout) - logger.register_logger(open('mylog.log', 'w')) - """ - - def __init__(self, *loggers: IO[str]) -> None: - """Create a new ``Log`` instance and register ``loggers`` if provided.""" - - self.loggers = list(loggers) - - def register_logger(self, logger: IO[str]) -> None: - """Register a single ``logger`` object.""" - - self.loggers.append(logger) - - def register_loggers(self, loggers: Iterable[IO[str]]) -> None: - """Register multiple ``loggers`` objects.""" - - for logger in loggers: - self.register_logger(logger) - - def write(self, *args: str) -> None: - """Write ``args`` to all registered loggers.""" - - message = " ".join(args) - for logger in self.loggers: - logger.write(f"{message}\n") - - def fatal(self, *args: str) -> None: - """Log ``args`` as a failure and terminate the program.""" - - self.fail(*args) - sys.exit(1) - - def fail(self, *args: str) -> None: - """Log ``args`` as an error.""" - - self.write("\033[0;31m", *args, "\033[0;00m") - - def warn(self, *args: str) -> None: - """Log ``args`` as a warning.""" - - self.write("\033[1;33m", *args, "\033[0;00m") - - def success(self, *args: str) -> None: - """Log ``args`` as a success message.""" - - self.write("\033[0;32m", *args, "\033[0;00m") - - def info(self, *args: str) -> None: - """Log ``args`` as an informational message.""" - - self.write("\033[1;35m", *args, "\033[0;00m") - - def close(self) -> None: - """Close all registered loggers.""" - - for logger in self.loggers: - logger.close() - - def __del__(self) -> None: - """Ensure all loggers are closed when this object is destroyed.""" - - self.close() diff --git a/zdeploy/recipe.py b/zdeploy/recipe.py index a1a25af..732a542 100644 --- a/zdeploy/recipe.py +++ b/zdeploy/recipe.py @@ -5,10 +5,10 @@ from pathlib import Path from hashlib import md5 from typing import Dict, List, Optional +import logging from zdeploy.clients import SSH, SCP from zdeploy.shell import execute as shell_execute -from zdeploy.log import Log from zdeploy.config import Config @@ -30,22 +30,26 @@ def __init__( username: str, password: str | None, port: int, - log: Log, + log: logging.Logger, cfg: Config, ) -> None: """Initialize a recipe instance.""" self.log = log if not str(config).strip(): - self.log.fatal("Invalid value for config") + self.log.error("Invalid value for config") + raise ValueError("invalid config") if not recipe or not recipe.strip(): - self.log.fatal("Invalid value for recipe") + self.log.error("Invalid value for recipe") + raise ValueError("invalid recipe") if not hostname or not hostname.strip(): - self.log.fatal("Invalid value for hostname") + self.log.error("Invalid value for hostname") + raise ValueError("invalid hostname") try: self.port = int(port) except ValueError: - self.log.fatal(f"Invalid value for port: {port}") + self.log.error("Invalid value for port: %s", port) + raise self.cfg = cfg self.parent_recipe = parent_recipe @@ -69,9 +73,9 @@ def _resolve_name_and_type(self, recipe: str) -> None: self.recipe = r if self.parent_recipe == r: # Recipe references itself - self.log.fatal(f"Invalid recipe: {r} references itself") - else: - self._type = self.Type.DEFINED + self.log.error("Invalid recipe: %s references itself", r) + raise ValueError("recipe references itself") + self._type = self.Type.DEFINED return self.recipe = recipe self._type = self.Type.VIRTUAL @@ -197,8 +201,8 @@ def deploy(self) -> None: if not (Path(self.cfg.recipes) / self.recipe / "run").is_file(): # Recipes with no run file are acceptable since they (may) have a require file # and don't necessarily require the execution of anything of their own. - self.log.warn( - f"Recipe '{self.recipe}' has no run file; continuing" + self.log.warning( + "Recipe '%s' has no run file; continuing", self.recipe ) else: ssh.execute( @@ -207,7 +211,7 @@ def deploy(self) -> None: ) passed = True except Exception as exc: # pylint: disable=broad-except - self.log.fail(str(exc)) + self.log.error(str(exc)) passed = False finally: if self._type == self.Type.DEFINED: @@ -215,5 +219,5 @@ def deploy(self) -> None: ssh.execute(f"rm -rf /opt/{self.recipe}", show_command=False) if not passed: - self.log.fatal(f"Failed to deploy {self.recipe}") - self.log.success(f"Done with {self.recipe}") + self.log.error("Failed to deploy %s", self.recipe) + self.log.info("Done with %s", self.recipe) diff --git a/zdeploy/recipeset.py b/zdeploy/recipeset.py index 59816d6..fa258f1 100644 --- a/zdeploy/recipeset.py +++ b/zdeploy/recipeset.py @@ -2,16 +2,16 @@ from hashlib import md5 from typing import Iterable +import logging from zdeploy.config import Config -from zdeploy.log import Log from zdeploy.recipe import Recipe class RecipeSet(set[Recipe]): """Container for ``Recipe`` objects with convenience helpers.""" - def __init__(self, cfg: Config, log: Log) -> None: + def __init__(self, cfg: Config, log: logging.Logger) -> None: """Create an empty ``RecipeSet`` using ``cfg`` and ``log``.""" super().__init__() @@ -28,23 +28,23 @@ def add(self, recipe: Recipe) -> None: # type: ignore[override] """Add a single ``recipe`` if not already present.""" if recipe in self: - self.log.warn(f"Recipe '{recipe.name}' is already added; skipping") + self.log.warning("Recipe '%s' is already added; skipping", recipe.name) return - self.log.info(f"Registering recipe '{recipe.name}'") + self.log.info("Registering recipe '%s'", recipe.name) super().add(recipe) if recipe.is_virtual(): - self.log.warn( + self.log.warning( ( f"Recipe '{recipe.recipe}' is not found under {self.cfg.recipes} " "and will be treated as a system package" ) ) - self.log.warn( + self.log.warning( ( f"The package will be installed using `{recipe.cfg.installer} {recipe.recipe}`" ) ) - self.log.warn( + self.log.warning( ( "To use a different package manager, specify " "an 'installer' entry in config.json" diff --git a/zdeploy/utils.py b/zdeploy/utils.py index 24c72ae..ab134b2 100644 --- a/zdeploy/utils.py +++ b/zdeploy/utils.py @@ -2,8 +2,8 @@ from datetime import timedelta -from sys import stderr from typing import Union +import logging def str2bool(value: str) -> bool: @@ -13,10 +13,7 @@ def str2bool(value: str) -> bool: if value in {"yes", "y", "true", "t", "e", "enable"}: return True - print( - f"Warning: '{value}' is not recognized as a true value, defaulting to False", - file=stderr, - ) + logging.warning("'%s' is not recognized as a true value, defaulting to False", value) return False