From a9a11abf7b212585c07e8bec5662f04feceea4e8 Mon Sep 17 00:00:00 2001 From: Phong Tran Date: Wed, 16 Sep 2026 14:38:54 -0400 Subject: [PATCH 1/4] SVCPLAN-9489: Remove inventory generation --- README.md | 30 ++-- pyproject.toml | 1 - sync_inventory/generate_inventory.py | 143 ------------------- sync_inventory/generate_playbook_commands.py | 126 ++++++++++------ sync_inventory/run_play.py | 23 ++- sync_inventory/sync_inventory.py | 26 ++-- 6 files changed, 127 insertions(+), 222 deletions(-) delete mode 100644 sync_inventory/generate_inventory.py diff --git a/README.md b/README.md index 5d1a672..c502ce0 100644 --- a/README.md +++ b/README.md @@ -17,13 +17,13 @@ automatically: 3. **Install each branch's dependencies** (roles and collections), so different branches can rely on different dependency versions without stepping on each other. -4. **Build an Ansible inventory per environment**, complete with that - environment's own variables — so real settings actually apply, and two - environments with the same group name never share values by accident. -5. **Generate the actual `ansible-playbook` commands** to run, one per +4. **Generate the actual `ansible-playbook` commands** to run, one per environment/role, each fully self-contained (its own config, its own dependencies) so running several environments back to back never lets - one leak into another. + one leak into another. No inventory file is generated — each branch's + own `ansible.cfg` is expected to declare its own inventory, the same as + it would for a human running `ansible-playbook` by hand from that + checkout. The end result sitting in the project directory: a `commands/` folder with one ready-to-run script per environment/role (e.g. @@ -51,8 +51,8 @@ source .venv/bin/activate pip install -e . ``` -This installs eight commands into your virtualenv: `sync-inventory`, -`fetch-meta`, `pull-repo`, `install-requirements`, `generate-inventory`, +This installs seven commands into your virtualenv: `sync-inventory`, +`fetch-meta`, `pull-repo`, `install-requirements`, `generate-playbook-commands`, `run-play`, and `list-vms`. ### Configuration @@ -80,8 +80,8 @@ export REPO_URL=git@example.com:org/ansible-playbooks.git ## Quick guide Run the whole pipeline (fetch metadata, mirror branches, regenerate -inventories and commands). `-u/--repo-url` (or `REPO_URL` in the -environment) is required — there's no default: +commands). `-u/--repo-url` (or `REPO_URL` in the environment) is required +— there's no default: ```bash sync-inventory -u git@example.com:org/ansible-playbooks.git @@ -100,13 +100,20 @@ run-play --all Each run is logged to its own file under `logs/`, named after the script (e.g. `logs/pttran3_test_branch_proxmox.log`). -Target a single host instead of the script's whole group, e.g. to test one -box before rolling out to the rest: +Target a single host on top of whatever the playbook's own `hosts:` key +already targets, e.g. to test one box before rolling out to the rest: ```bash run-play -s pttran3_test_branch_proxmox -H some-host.example.com ``` +Override the inventory file instead of relying on the branch's own +`ansible.cfg` (works with `-s` or `--all`): + +```bash +run-play -s pttran3_test_branch_proxmox -i other/hosts.yml +``` + See what's available to run (the exact names `-s` accepts): ```bash @@ -154,7 +161,6 @@ Run an individual step on its own (each accepts `--help` for its own flags): fetch-meta pull-repo git@example.com:org/ansible-playbooks.git install-requirements -generate-inventory generate-playbook-commands run-play -s pttran3_test_branch_proxmox ``` diff --git a/pyproject.toml b/pyproject.toml index 11b9fde..b517cd3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,7 +17,6 @@ sync-inventory = "sync_inventory.sync_inventory:main" fetch-meta = "sync_inventory.fetch_meta:main" pull-repo = "sync_inventory.pull_repo:main" install-requirements = "sync_inventory.install_requirements:main" -generate-inventory = "sync_inventory.generate_inventory:main" generate-playbook-commands = "sync_inventory.generate_playbook_commands:main" run-play = "sync_inventory.run_play:main" list-vms = "sync_inventory.list_vms:main" diff --git a/sync_inventory/generate_inventory.py b/sync_inventory/generate_inventory.py deleted file mode 100644 index 0e071eb..0000000 --- a/sync_inventory/generate_inventory.py +++ /dev/null @@ -1,143 +0,0 @@ -#!/usr/bin/env python3 -"""Generate one Ansible inventory directory per env from a netbox-style hosts JSON file. - -Each env gets its own subdirectory under --inventory-dir: - - inventory//hosts.yml - the generated inventory - inventory//group_vars/ - copied from repo//inventory/group_vars/, if present - inventory//host_vars/ - copied from repo//inventory/host_vars/, if present - -This matters because Ansible discovers group_vars/host_vars relative to the -directory containing the inventory file passed to -i, not the playbook -being run. Keeping the generated hosts.yml, group_vars, and host_vars -together per env means the real per-branch group_vars/host_vars in the -playbook repo actually get applied, and keeping each env in its own -subdirectory (rather than one shared group_vars/ for every env) avoids -different envs' same-named groups colliding with different values. - -Every run first removes any existing per-env subdirectories, then writes -fresh ones for envs currently present in the hosts file. This keeps the -directory in sync even when an env loses all its hosts (its subdirectory -is removed rather than left behind). A missing hosts file is treated the -same as an empty one (zero hosts, --inventory-dir ends up empty) rather -than raising an error. Role values are used as-is for the Ansible group -name (generate-playbook-commands relies on this matching the real -playbook filename on disk, so it's deliberately not sanitized -- Ansible -will warn about invalid group-name characters itself if a role has any). -Env values become a directory name, so "/" and "-" are replaced with "_" -the same way pull-repo sanitizes branch directory names, keeping -repo// and inventory// referring to the same branch. -""" - -import argparse -import json -import shutil -from collections import defaultdict -from pathlib import Path - -import yaml - -from sync_inventory.naming import sanitize_dir_name - - -def load_hosts(hosts_file, verbose=False): - try: - with open(hosts_file) as f: - return json.load(f) - except FileNotFoundError: - if verbose: - print(f"warning: hosts file '{hosts_file}' not found; treating as empty") - return {} - - -def sanitize_env_name(env, verbose=False): - """Match pull-repo's branch-directory sanitization, so repo// and - inventory// refer to the same branch for a feature branch like - "pttran3/SVCPLAN-1234/test".""" - sanitized = sanitize_dir_name(env) - if sanitized != env and verbose: - print(f"warning: env '{env}' has invalid directory characters; using '{sanitized}' instead") - return sanitized - - -def group_by_env(hosts, verbose=False): - envs = defaultdict(lambda: defaultdict(list)) - for hostname, meta in hosts.items(): - env = sanitize_env_name(meta["env"], verbose=verbose) - envs[env][meta["role"]].append(hostname) - return envs - - -def copy_vars(env_source_dir, env_dir, subdir_name, verbose=False): - src = env_source_dir / subdir_name - dest = env_dir / subdir_name - if not src.is_dir(): - return - shutil.copytree(src, dest) - if verbose: - print(f"Copied {src} -> {dest}") - - -def write_inventory(env, roles, inventory_dir, repo_dir, verbose=False): - env_dir = inventory_dir / env - env_dir.mkdir(parents=True, exist_ok=True) - - inventory = { - "all": { - "children": { - role: {"hosts": {hostname: None for hostname in sorted(hostnames)}} - for role, hostnames in sorted(roles.items()) - } - } - } - - out_path = env_dir / "hosts.yml" - with open(out_path, "w") as f: - yaml.safe_dump(inventory, f, sort_keys=False) - if verbose: - print(f"Wrote {out_path}") - - repo_inventory_dir = repo_dir / env / "inventory" - copy_vars(repo_inventory_dir, env_dir, "group_vars", verbose=verbose) - copy_vars(repo_inventory_dir, env_dir, "host_vars", verbose=verbose) - - -def generate_inventory(hosts_file="hosts.json", inventory_dir="inventory", repo_dir="repo", verbose=False): - inventory_dir = Path(inventory_dir) - repo_dir = Path(repo_dir) - inventory_dir.mkdir(parents=True, exist_ok=True) - - for stale in inventory_dir.iterdir(): - if stale.is_dir(): - shutil.rmtree(stale) - if verbose: - print(f"Removed stale {stale}") - - hosts = load_hosts(hosts_file, verbose=verbose) - envs = group_by_env(hosts, verbose=verbose) - for env, roles in envs.items(): - write_inventory(env, roles, inventory_dir, repo_dir, verbose=verbose) - - -def main(): - parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) - parser.add_argument( - "--hosts-file", default="hosts.json", - help="Path to the netbox-style hosts JSON file (default: %(default)s)", - ) - parser.add_argument( - "--inventory-dir", default="inventory", - help="Directory to write per-env inventory subdirectories into (default: %(default)s)", - ) - parser.add_argument( - "--repo-dir", default="repo", - help="Directory containing per-branch checkouts, to copy each env's real group_vars/host_vars from (default: %(default)s)", - ) - parser.add_argument("-v", "--verbose", action="store_true", help="Print each inventory file written and vars copied") - args = parser.parse_args() - - generate_inventory(args.hosts_file, args.inventory_dir, args.repo_dir, verbose=args.verbose) - - -if __name__ == "__main__": - main() diff --git a/sync_inventory/generate_playbook_commands.py b/sync_inventory/generate_playbook_commands.py index 419154f..b6f9505 100644 --- a/sync_inventory/generate_playbook_commands.py +++ b/sync_inventory/generate_playbook_commands.py @@ -1,18 +1,26 @@ #!/usr/bin/env python3 -"""Generate one ansible-playbook command script per group in each env inventory. +"""Generate one ansible-playbook command script per role assigned to each env. -Each subdirectory of --inventory-dir is named after an env, which is -expected to match a branch checked out under repo/ (see pull-repo). For -each group (role name) found in that env's inventory//hosts.yml, write -an executable script at commands/_.sh that runs the matching -playbook from that branch's checkout, limited to that group by default: +For every env with hosts in --hosts-file, and every role assigned to that +env, write an executable script at commands/_.sh that runs the +matching playbook from that branch's checkout: - ansible-playbook -i inventory//hosts.yml --limit repo//playbooks/.yml + ansible-playbook repo//playbooks/.yml -Each script takes an optional first argument overriding what --limit is -passed, e.g. `commands/_.sh some-host.example.com` runs just -that host instead of the whole group -- this is what run-play's -H/--host -uses under the hood. +No --limit is passed by default -- the playbook's own `hosts:` key decides +which host(s)/group(s) it targets, the same way it would if a human ran +ansible-playbook by hand. Each script takes an optional first argument that +adds `--limit ` on top of whatever the playbook already +targets, e.g. `commands/_.sh some-host.example.com` narrows the +run to just that host -- this is what run-play's -H/--host uses under the +hood. + +No -i/--inventory is passed by default either -- each branch's own +ansible.cfg (see below) already declares its own inventory file, so +Ansible finds it on its own. It can still be overridden by exporting +INVENTORY before running the script, e.g. +`INVENTORY=other/hosts.yml commands/_.sh` -- this is what +run-play's -i/--inventory uses under the hood. Each script is self-contained and safe to run directly (e.g. to debug one command by hand) -- its output goes straight to stdout/stderr, nothing is @@ -20,47 +28,82 @@ script in --commands-dir and handles logging/failure-tracking itself. If a branch isn't checked out under repo/, an error is reported for that env -and its commands are skipped. Groups whose playbook file doesn't actually -exist in that branch's checkout are reported as a warning and skipped (the +and its commands are skipped. A role whose playbook file doesn't actually +exist in that branch's checkout is reported as a warning and skipped (the netbox data only records intent, not what playbooks actually exist). If that branch has its own ansible.cfg, ANSIBLE_CONFIG is set to it for that command (Ansible only auto-discovers ansible.cfg via the current directory, not the playbook's path, so without this the branch's own config -- vault -password file, remote_user, etc. -- would otherwise be silently ignored). -If install-requirements has installed that branch's roles/collections into -/.ansible/{roles,collections}, ANSIBLE_ROLES_PATH and -ANSIBLE_COLLECTIONS_PATH are set to them too. +password file, remote_user, inventory, etc. -- would otherwise be silently +ignored). If install-requirements has installed that branch's +roles/collections into /.ansible/{roles,collections}, +ANSIBLE_ROLES_PATH and ANSIBLE_COLLECTIONS_PATH are set to them too. -Every run first removes any existing scripts in --commands-dir, so a group +Every run first removes any existing scripts in --commands-dir, so a role that no longer applies doesn't leave a stale script behind. """ import argparse +import json import shutil import stat +from collections import defaultdict from pathlib import Path -import yaml +from sync_inventory.naming import sanitize_dir_name + + +def load_hosts(hosts_file, verbose=False): + try: + with open(hosts_file) as f: + return json.load(f) + except FileNotFoundError: + if verbose: + print(f"warning: hosts file '{hosts_file}' not found; treating as empty") + return {} + + +def sanitize_env_name(env, verbose=False): + """Match pull-repo's branch-directory sanitization, so repo// + refers to the same branch for a feature branch like + "pttran3/SVCPLAN-1234/test".""" + sanitized = sanitize_dir_name(env) + if sanitized != env and verbose: + print(f"warning: env '{env}' has invalid directory characters; using '{sanitized}' instead") + return sanitized -def groups_in_inventory(inventory_path): - with open(inventory_path) as f: - inventory = yaml.safe_load(f) - return list(inventory["all"]["children"].keys()) +def group_by_env(hosts, verbose=False): + envs = defaultdict(lambda: defaultdict(list)) + for hostname, meta in hosts.items(): + env = sanitize_env_name(meta["env"], verbose=verbose) + envs[env][meta["role"]].append(hostname) + return envs -def write_command_script(path, env_vars, ansible_cmd, verbose=False): +def write_command_script(path, env_vars, playbook_path, verbose=False): export_lines = [f"export {key}={value}" for key, value in env_vars.items()] - lines = ["#!/bin/bash", *export_lines, ansible_cmd, ""] + lines = [ + "#!/bin/bash", + *export_lines, + "extra_args=()", + 'if [[ -n "$1" ]]; then', + ' extra_args+=(--limit "$1")', + "fi", + 'if [[ -n "$INVENTORY" ]]; then', + ' extra_args+=(-i "$INVENTORY")', + "fi", + f'ansible-playbook "${{extra_args[@]}}" {playbook_path}', + "", + ] path.write_text("\n".join(lines)) path.chmod(path.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) if verbose: print(f"Wrote {path}") -def generate_playbook_commands(inventory_dir="inventory", repo_dir="repo", commands_dir="commands", verbose=False): - inventory_dir = Path(inventory_dir) +def generate_playbook_commands(hosts_file="hosts.json", repo_dir="repo", commands_dir="commands", verbose=False): repo_dir = Path(repo_dir) commands_dir = Path(commands_dir) @@ -68,15 +111,11 @@ def generate_playbook_commands(inventory_dir="inventory", repo_dir="repo", comma shutil.rmtree(commands_dir) commands_dir.mkdir(parents=True, exist_ok=True) - for env_dir in sorted(p for p in inventory_dir.iterdir() if p.is_dir()): - branch = env_dir.name - inventory_path = env_dir / "hosts.yml" - branch_dir = repo_dir / branch + hosts = load_hosts(hosts_file, verbose=verbose) + envs = group_by_env(hosts, verbose=verbose) - if not inventory_path.is_file(): - if verbose: - print(f"ERROR: no hosts.yml found under {env_dir}") - continue + for branch, roles in sorted(envs.items()): + branch_dir = repo_dir / branch if not branch_dir.is_dir(): if verbose: @@ -94,22 +133,21 @@ def generate_playbook_commands(inventory_dir="inventory", repo_dir="repo", comma if collections_path.is_dir(): env_vars["ANSIBLE_COLLECTIONS_PATH"] = str(collections_path) - for group in groups_in_inventory(inventory_path): - playbook_path = branch_dir / "playbooks" / f"{group}.yml" + for role in sorted(roles): + playbook_path = branch_dir / "playbooks" / f"{role}.yml" if not playbook_path.is_file(): if verbose: - print(f"WARNING: role '{group}' has no playbook at {playbook_path}; skipping") + print(f"WARNING: role '{role}' has no playbook at {playbook_path}; skipping") continue - ansible_cmd = f'ansible-playbook -i {inventory_path} --limit "${{1:-{group}}}" {playbook_path}' - script_path = commands_dir / f"{branch}_{group}.sh" - write_command_script(script_path, env_vars, ansible_cmd, verbose=verbose) + script_path = commands_dir / f"{branch}_{role}.sh" + write_command_script(script_path, env_vars, playbook_path, verbose=verbose) def main(): parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) parser.add_argument( - "--inventory-dir", default="inventory", - help="Directory containing per-env inventory subdirectories (default: %(default)s)", + "--hosts-file", default="hosts.json", + help="Path to the netbox-style hosts JSON file, to determine which roles are assigned to each env (default: %(default)s)", ) parser.add_argument( "--repo-dir", default="repo", @@ -122,7 +160,7 @@ def main(): parser.add_argument("-v", "--verbose", action="store_true", help="Print branch/role warnings and errors, and each script written") args = parser.parse_args() - generate_playbook_commands(args.inventory_dir, args.repo_dir, args.commands_dir, verbose=args.verbose) + generate_playbook_commands(args.hosts_file, args.repo_dir, args.commands_dir, verbose=args.verbose) if __name__ == "__main__": diff --git a/sync_inventory/run_play.py b/sync_inventory/run_play.py index faf7906..d0a2a60 100644 --- a/sync_inventory/run_play.py +++ b/sync_inventory/run_play.py @@ -14,26 +14,36 @@ Only valid with -s/--script -- not with --all, since that would run every script against that one host. +The inventory file can be overridden with -i/--inventory, which sets +INVENTORY in the script's environment -- each script only passes -i to +ansible-playbook when INVENTORY is set, otherwise Ansible falls back to +whatever the branch's own ansible.cfg declares. Valid with both -s/--script +and --all. + Usage: run-play -s pttran3_test_branch_proxmox run-play -s pttran3_test_branch_proxmox -H some-host.example.com + run-play -s pttran3_test_branch_proxmox -i other/hosts.yml run-play --all run-play --list """ import argparse +import os import subprocess import sys from pathlib import Path -def run_script(script, logs_dir, host=None, quiet=False, verbose=False): +def run_script(script, logs_dir, host=None, inventory=None, quiet=False, verbose=False): log_file = logs_dir / f"{script.stem}.log" cmd = ["bash", str(script)] + ([host] if host else []) + env = {**os.environ, "INVENTORY": inventory} if inventory else None if verbose: - print(f"+ {' '.join(cmd)} (log: {log_file})") + prefix = f"INVENTORY={inventory} " if inventory else "" + print(f"+ {prefix}{' '.join(cmd)} (log: {log_file})") with open(log_file, "w") as f: - process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True) + process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, env=env) for line in process.stdout: f.write(line) if not quiet: @@ -55,7 +65,7 @@ def list_commands(commands_dir="commands"): print(name) -def run_play(script_name=None, run_all=False, commands_dir="commands", logs_dir="logs", host=None, quiet=False, verbose=False): +def run_play(script_name=None, run_all=False, commands_dir="commands", logs_dir="logs", host=None, inventory=None, quiet=False, verbose=False): if run_all and host: raise SystemExit("-H/--host can only be used with -s/--script, not --all") @@ -73,7 +83,7 @@ def run_play(script_name=None, run_all=False, commands_dir="commands", logs_dir= raise SystemExit(f"No command script found at {script}") scripts = [script] - failures = sum(not run_script(script, logs_dir, host=host, quiet=quiet, verbose=verbose) for script in scripts) + failures = sum(not run_script(script, logs_dir, host=host, inventory=inventory, quiet=quiet, verbose=verbose) for script in scripts) if failures: raise SystemExit(f"{failures} command(s) failed") @@ -87,6 +97,7 @@ def main(): parser.add_argument("--commands-dir", default="commands", help="Directory containing generated command scripts (default: %(default)s)") parser.add_argument("--logs-dir", default="logs", help="Directory to write each command's log file into (default: %(default)s)") parser.add_argument("-H", "--host", help="Limit the run to a single host instead of the script's whole group") + parser.add_argument("-i", "--inventory", help="Override the inventory file ansible-playbook uses, instead of falling back to the branch's own ansible.cfg") parser.add_argument("-q", "--quiet", action="store_true", help="Only log output to --logs-dir; don't also print it to stdout") parser.add_argument("-v", "--verbose", action="store_true", help="Print each command as it runs") args = parser.parse_args() @@ -97,7 +108,7 @@ def main(): list_commands(args.commands_dir) return - run_play(args.script, args.all, args.commands_dir, args.logs_dir, host=args.host, quiet=args.quiet, verbose=args.verbose) + run_play(args.script, args.all, args.commands_dir, args.logs_dir, host=args.host, inventory=args.inventory, quiet=args.quiet, verbose=args.verbose) if __name__ == "__main__": diff --git a/sync_inventory/sync_inventory.py b/sync_inventory/sync_inventory.py index f3fffe3..922aba0 100644 --- a/sync_inventory/sync_inventory.py +++ b/sync_inventory/sync_inventory.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Sync inventory: fetch metadata, pull each branch, regenerate inventories, regenerate commands. +"""Sync inventory: fetch metadata, pull each branch, regenerate commands. Steps: @@ -13,17 +13,16 @@ Install each branch's roles/collections from its requirements.yml into repo//.ansible/{roles,collections}. - 4. generate-inventory - Rebuild inventory//hosts.yml from the hosts file, copying that - branch's real group_vars/host_vars in alongside it. - - 5. generate-playbook-commands - Rebuild commands/_.sh scripts from inventory/ + repo/, each - pointing its ANSIBLE_CONFIG/ANSIBLE_ROLES_PATH/ANSIBLE_COLLECTIONS_PATH - at that branch's own config and installed deps. + 4. generate-playbook-commands + Rebuild commands/_.sh scripts from the hosts file + repo/, + each pointing its ANSIBLE_CONFIG/ANSIBLE_ROLES_PATH/ANSIBLE_COLLECTIONS_PATH + at that branch's own config and installed deps. No inventory file is + generated -- each branch's own ansible.cfg is expected to declare its + own inventory, the same as it would for a human running ansible-playbook + by hand from that checkout. A failure in step 1, 2, or 3 (e.g. NetBox/network unreachable) does not -block the rest, since steps 4/5 just need whatever hosts file, repo +block the rest, since step 4 just needs whatever hosts file, repo checkouts, and installed dependencies already exist on disk. This command only regenerates commands/; it never runs them. Use run-play @@ -44,7 +43,6 @@ from pathlib import Path from sync_inventory.fetch_meta import fetch_meta -from sync_inventory.generate_inventory import generate_inventory from sync_inventory.generate_playbook_commands import generate_playbook_commands from sync_inventory.install_requirements import install_requirements from sync_inventory.pull_repo import pull_repo @@ -64,7 +62,6 @@ def main(): help="Git repo to mirror branches from (env: REPO_URL)", ) parser.add_argument("-r", "--repo-dir", default="repo", help="Where branch checkouts are written (default: %(default)s)") - parser.add_argument("-i", "--inventory-dir", default="inventory", help="Where generated per-env inventories are written (default: %(default)s)") parser.add_argument("-n", "--hosts-file", default="hosts.json", help="NetBox-style hosts JSON (default: %(default)s)") parser.add_argument("-c", "--commands-dir", default="commands", help="Where each generated ansible-playbook command script is written (default: %(default)s)") parser.add_argument( @@ -114,11 +111,8 @@ def main(): if args.verbose: print(f"WARNING: install-requirements failed ({e}); continuing with existing {args.repo_dir}/ dependencies") - section("generate-inventory", args.verbose) - generate_inventory(args.hosts_file, args.inventory_dir, args.repo_dir, verbose=args.verbose) - section("generate-playbook-commands", args.verbose) - generate_playbook_commands(args.inventory_dir, args.repo_dir, args.commands_dir, verbose=args.verbose) + generate_playbook_commands(args.hosts_file, args.repo_dir, args.commands_dir, verbose=args.verbose) finally: LOCK_DIR.rmdir() From 96f7bb09d89350d2cb7d64598188296997e6a23a Mon Sep 17 00:00:00 2001 From: Phong Tran Date: Wed, 16 Sep 2026 15:56:47 -0400 Subject: [PATCH 2/4] SVCPLAN-9489: output warnings + errors to stdout and stderr w/o -v --- README.md | 26 +++++++++----- sync_inventory/fetch_meta.py | 20 +++++------ sync_inventory/generate_playbook_commands.py | 31 ++++++++-------- sync_inventory/install_requirements.py | 33 ++++++++++++----- sync_inventory/pull_repo.py | 38 +++++++++++--------- sync_inventory/sync_inventory.py | 20 +++++------ 6 files changed, 99 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index c502ce0..88ef9fd 100644 --- a/README.md +++ b/README.md @@ -32,12 +32,22 @@ only generates these; running one (or all of them) is a separate step, with `run-play` (see [Quick guide](#quick-guide) below). It's built to keep working even when something's incomplete or unreachable. -A host pointed at a branch that doesn't exist, or a role with no matching -playbook, gets skipped and reported rather than stopping everything else. -Losing the connection to NetBox or the git remote just means it falls back -to whatever it already had, rather than failing outright. All of this -reporting is quiet by default — pass `-v`/`--verbose` (on `sync-inventory` -or any individual command) when you want to see it. +An env pointed at a branch that isn't checked out, or a role with no +matching playbook, gets skipped and reported rather than stopping +everything else. Losing the connection to NetBox or the git remote just +means it falls back to whatever it already had, rather than failing +outright. Warnings and errors always print to stderr, regardless of +`-v`/`--verbose` — that flag only adds routine progress output on top. + +Skipping happens per **env** or per **(env, role)**, never per individual +host — no inventory file is generated anymore (see below), so individual +hostnames in `hosts.json` aren't used to decide anything; they only +determine which `(env, role)` combinations need a script. Concretely: if +an env's branch isn't checked out under `repo/`, every role/host assigned +to that env is skipped. If a role has no matching playbook in that branch, +just that role is skipped, but for every host that shares it in that env — +there's no way to skip one specific host while keeping others in the same +`(env, role)` pair generating a script. ## Install @@ -134,8 +144,8 @@ Point it at a different playbook repo, or override other non-default paths sync-inventory --repo-url git@example.com:org/other-repo.git --commands-dir ~/generated-commands ``` -See routine progress and every warning/error as it happens (quiet by -default otherwise): +See routine progress too, on top of the warnings/errors that always print +(quiet by default otherwise): ```bash sync-inventory -u git@example.com:org/ansible-playbooks.git -v diff --git a/sync_inventory/fetch_meta.py b/sync_inventory/fetch_meta.py index edaeb21..267a9c0 100644 --- a/sync_inventory/fetch_meta.py +++ b/sync_inventory/fetch_meta.py @@ -8,10 +8,10 @@ expects. Only entries with role/env actually set are included; entries missing -either are reported as a warning and skipped rather than silently dropped. -By default, entries where nbmeta's "ansible" flag is explicitly false are -excluded too, since those hosts are marked as not managed by this Ansible -controller. +either are reported as a warning (always printed, regardless of --verbose) +and skipped rather than silently dropped. By default, entries where +nbmeta's "ansible" flag is explicitly false are excluded too, since those +hosts are marked as not managed by this Ansible controller. Configuration is read from the environment, same as nbmeta: NETBOX_URL - NetBox instance URL @@ -78,14 +78,12 @@ def fetch_hosts(nb, owner_ids, ansible_only=True, verbose=False): hosts = {} for ip in nb.ipam.ip_addresses.filter(owner_id=owner_ids): if not ip.dns_name: - if verbose: - print(f"warning: skipping {ip.address}, no dns_name set", file=sys.stderr) + print(f"warning: skipping {ip.address}, no dns_name set", file=sys.stderr) continue meta = split_leading_json(ip.description or "") if "role" not in meta or "env" not in meta: - if verbose: - print(f"warning: skipping {ip.dns_name} ({ip.address}), missing role/env in description", file=sys.stderr) + print(f"warning: skipping {ip.dns_name} ({ip.address}), missing role/env in description", file=sys.stderr) continue if ansible_only and not meta.get("ansible", True): @@ -109,11 +107,11 @@ def fetch_meta(hosts_file="hosts.json", ansible_only=True, verbose=False): nb = get_client() owner_names = get_owner_names() owner_ids, unresolved = resolve_owner_ids(nb, owner_names) - if unresolved and verbose: + if unresolved: print(f"warning: NETBOX_OWNERS not found in NetBox, ignoring: {unresolved}", file=sys.stderr) hosts = fetch_hosts(nb, owner_ids, ansible_only=ansible_only, verbose=verbose) - if not hosts and verbose: + if not hosts: print( "warning: no matching hosts found (check NETBOX_OWNERS, and that entries have " "role/env set via nbmeta)", @@ -136,7 +134,7 @@ def main(): "--include-non-ansible", action="store_true", help="Also include hosts where nbmeta's ansible flag is false (default: only ansible=true hosts)", ) - parser.add_argument("-v", "--verbose", action="store_true", help="Print per-host warnings and the final Wrote message") + parser.add_argument("-v", "--verbose", action="store_true", help="Print the final Wrote message (per-host and config warnings always print)") args = parser.parse_args() try: diff --git a/sync_inventory/generate_playbook_commands.py b/sync_inventory/generate_playbook_commands.py index b6f9505..5a03684 100644 --- a/sync_inventory/generate_playbook_commands.py +++ b/sync_inventory/generate_playbook_commands.py @@ -30,7 +30,8 @@ If a branch isn't checked out under repo/, an error is reported for that env and its commands are skipped. A role whose playbook file doesn't actually exist in that branch's checkout is reported as a warning and skipped (the -netbox data only records intent, not what playbooks actually exist). +netbox data only records intent, not what playbooks actually exist). Both +of these print to stderr always, regardless of --verbose. If that branch has its own ansible.cfg, ANSIBLE_CONFIG is set to it for that command (Ansible only auto-discovers ansible.cfg via the current directory, @@ -48,36 +49,36 @@ import json import shutil import stat +import sys from collections import defaultdict from pathlib import Path from sync_inventory.naming import sanitize_dir_name -def load_hosts(hosts_file, verbose=False): +def load_hosts(hosts_file): try: with open(hosts_file) as f: return json.load(f) except FileNotFoundError: - if verbose: - print(f"warning: hosts file '{hosts_file}' not found; treating as empty") + print(f"warning: hosts file '{hosts_file}' not found; treating as empty", file=sys.stderr) return {} -def sanitize_env_name(env, verbose=False): +def sanitize_env_name(env): """Match pull-repo's branch-directory sanitization, so repo// refers to the same branch for a feature branch like "pttran3/SVCPLAN-1234/test".""" sanitized = sanitize_dir_name(env) - if sanitized != env and verbose: - print(f"warning: env '{env}' has invalid directory characters; using '{sanitized}' instead") + if sanitized != env: + print(f"warning: env '{env}' has invalid directory characters; using '{sanitized}' instead", file=sys.stderr) return sanitized -def group_by_env(hosts, verbose=False): +def group_by_env(hosts): envs = defaultdict(lambda: defaultdict(list)) for hostname, meta in hosts.items(): - env = sanitize_env_name(meta["env"], verbose=verbose) + env = sanitize_env_name(meta["env"]) envs[env][meta["role"]].append(hostname) return envs @@ -111,15 +112,14 @@ def generate_playbook_commands(hosts_file="hosts.json", repo_dir="repo", command shutil.rmtree(commands_dir) commands_dir.mkdir(parents=True, exist_ok=True) - hosts = load_hosts(hosts_file, verbose=verbose) - envs = group_by_env(hosts, verbose=verbose) + hosts = load_hosts(hosts_file) + envs = group_by_env(hosts) for branch, roles in sorted(envs.items()): branch_dir = repo_dir / branch if not branch_dir.is_dir(): - if verbose: - print(f"ERROR: branch '{branch}' not found under {repo_dir} (expected {branch_dir})") + print(f"ERROR: branch '{branch}' not found under {repo_dir} (expected {branch_dir})", file=sys.stderr) continue env_vars = {} @@ -136,8 +136,7 @@ def generate_playbook_commands(hosts_file="hosts.json", repo_dir="repo", command for role in sorted(roles): playbook_path = branch_dir / "playbooks" / f"{role}.yml" if not playbook_path.is_file(): - if verbose: - print(f"WARNING: role '{role}' has no playbook at {playbook_path}; skipping") + print(f"WARNING: role '{role}' has no playbook at {playbook_path}; skipping", file=sys.stderr) continue script_path = commands_dir / f"{branch}_{role}.sh" write_command_script(script_path, env_vars, playbook_path, verbose=verbose) @@ -157,7 +156,7 @@ def main(): "--commands-dir", default="commands", help="Directory to write one script per ansible-playbook command into (default: %(default)s)", ) - parser.add_argument("-v", "--verbose", action="store_true", help="Print branch/role warnings and errors, and each script written") + parser.add_argument("-v", "--verbose", action="store_true", help="Print each script written (branch/role warnings and errors always print, regardless of this flag)") args = parser.parse_args() generate_playbook_commands(args.hosts_file, args.repo_dir, args.commands_dir, verbose=args.verbose) diff --git a/sync_inventory/install_requirements.py b/sync_inventory/install_requirements.py index 5fe5892..57440ac 100644 --- a/sync_inventory/install_requirements.py +++ b/sync_inventory/install_requirements.py @@ -11,11 +11,16 @@ generate-playbook-commands points ANSIBLE_ROLES_PATH/ANSIBLE_COLLECTIONS_PATH at these same directories for that branch's generated commands, so installed dependencies are actually found at playbook-run time. + +If ansible-galaxy fails for a branch, the failure (including its actual +output) is printed to stderr -- always, regardless of --verbose -- and +that branch is skipped rather than aborting the rest of the run. """ import argparse import os import subprocess +import sys from pathlib import Path @@ -33,6 +38,21 @@ def needs_install(req_file, marker): return not marker.is_file() or req_file.stat().st_mtime > marker.stat().st_mtime +def run_galaxy(cmd, env, verbose=False): + if verbose: + print(f"$ {' '.join(cmd)}", flush=True) + try: + subprocess.run(cmd, check=True, env=env, capture_output=not verbose, text=True) + except subprocess.CalledProcessError as e: + print(f"error: `{' '.join(cmd)}` failed (exit {e.returncode})", file=sys.stderr) + if e.stdout: + print(e.stdout.rstrip(), file=sys.stderr) + if e.stderr: + print(e.stderr.rstrip(), file=sys.stderr) + return False + return True + + def install_requirements(repo_dir="repo", verbose=False): for branch_dir, req_file in requirements_files(repo_dir): ansible_dir = branch_dir / ".ansible" @@ -56,13 +76,10 @@ def install_requirements(repo_dir="repo", verbose=False): "ANSIBLE_COLLECTIONS_PATH": str(collections_path), } - if verbose: - print(f"$ {' '.join(role_cmd)}", flush=True) - subprocess.run(role_cmd, check=True, env=env, capture_output=not verbose) - - if verbose: - print(f"$ {' '.join(collection_cmd)}", flush=True) - subprocess.run(collection_cmd, check=True, env=env, capture_output=not verbose) + if not run_galaxy(role_cmd, env, verbose=verbose): + continue + if not run_galaxy(collection_cmd, env, verbose=verbose): + continue marker.touch() if verbose: @@ -75,7 +92,7 @@ def main(): "--repo-dir", default="repo", help="Directory containing per-branch checkouts (default: %(default)s)", ) - parser.add_argument("-v", "--verbose", action="store_true", help="Print install progress") + parser.add_argument("-v", "--verbose", action="store_true", help="Print install progress (failures always print, regardless of this flag)") args = parser.parse_args() install_requirements(args.repo_dir, verbose=args.verbose) diff --git a/sync_inventory/pull_repo.py b/sync_inventory/pull_repo.py index 8d67a7f..53e12a4 100644 --- a/sync_inventory/pull_repo.py +++ b/sync_inventory/pull_repo.py @@ -16,9 +16,9 @@ Quiet by default; pass --verbose to see one line per branch ("No update to repo/", "Updated repo/", or "Cloned repo/"). Git's own (much noisier) output is never shown, except when a branch's update or -clone fails, where it's printed as-is (still only with --verbose) so the -actual error is visible. A branch that fails is skipped rather than -aborting the rest of the run. +clone fails, where it's printed as-is to stderr -- always, regardless of +--verbose -- so the actual error is visible. A branch that fails is +skipped rather than aborting the rest of the run. Usage: pull-repo [-r REPO_DIR] [-v] @@ -26,6 +26,7 @@ import argparse import subprocess +import sys import time from pathlib import Path @@ -35,6 +36,10 @@ RETRY_DELAY = 5 +class PullRepoError(RuntimeError): + pass + + def run(cmd, verbose=False): for attempt in range(1, RETRIES + 1): result = subprocess.run(cmd, capture_output=True, text=True) @@ -47,14 +52,12 @@ def run(cmd, verbose=False): raise subprocess.CalledProcessError(result.returncode, cmd, output=result.stdout, stderr=result.stderr) -def print_git_error(action, e, verbose=False): - if not verbose: - return - print(f"ERROR: {action} failed: `{' '.join(e.cmd)}` (exit {e.returncode})") +def print_git_error(action, e): + print(f"ERROR: {action} failed: `{' '.join(e.cmd)}` (exit {e.returncode})", file=sys.stderr) if e.stdout: - print(e.stdout.rstrip()) + print(e.stdout.rstrip(), file=sys.stderr) if e.stderr: - print(e.stderr.rstrip()) + print(e.stderr.rstrip(), file=sys.stderr) def list_remote_branches(repo_url, verbose=False): @@ -78,7 +81,7 @@ def sync_branch(repo_url, branch, branch_dir, verbose=False): run(["git", "-C", str(branch_dir), "reset", "--hard", "FETCH_HEAD"], verbose=verbose) run(["git", "-C", str(branch_dir), "clean", "-fd"], verbose=verbose) except subprocess.CalledProcessError as e: - print_git_error(f"updating {branch_dir}", e, verbose=verbose) + print_git_error(f"updating {branch_dir}", e) return if verbose: print(f"{'No update to' if before == after else 'Updated'} {branch_dir}") @@ -86,7 +89,7 @@ def sync_branch(repo_url, branch, branch_dir, verbose=False): try: run(["git", "clone", "--branch", branch, "--single-branch", repo_url, str(branch_dir)], verbose=verbose) except subprocess.CalledProcessError as e: - print_git_error(f"cloning {branch_dir}", e, verbose=verbose) + print_git_error(f"cloning {branch_dir}", e) return if verbose: print(f"Cloned {branch_dir}") @@ -99,10 +102,10 @@ def pull_repo(repo_url, repo_dir="repo", verbose=False): try: branches = list_remote_branches(repo_url, verbose=verbose) except subprocess.CalledProcessError as e: - print_git_error(f"listing branches in {repo_url}", e, verbose=verbose) - raise SystemExit(f"Could not list branches in {repo_url}") + print_git_error(f"listing branches in {repo_url}", e) + raise PullRepoError(f"Could not list branches in {repo_url}") if not branches: - raise SystemExit(f"No branches found in {repo_url}") + raise PullRepoError(f"No branches found in {repo_url}") for branch in branches: dir_name = sanitize_dir_name(branch) @@ -116,10 +119,13 @@ def main(): parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) parser.add_argument("repo_url", help="URL or path of the git repository to clone") parser.add_argument("-r", "--repo-dir", default="repo", help="Directory to hold per-branch checkouts (default: repo)") - parser.add_argument("-v", "--verbose", action="store_true", help="Print one line per branch, and git's own output on error") + parser.add_argument("-v", "--verbose", action="store_true", help="Print one line per branch (git's own output on error always prints, regardless of this flag)") args = parser.parse_args() - pull_repo(args.repo_url, args.repo_dir, verbose=args.verbose) + try: + pull_repo(args.repo_url, args.repo_dir, verbose=args.verbose) + except PullRepoError as exc: + raise SystemExit(f"error: {exc}") if __name__ == "__main__": diff --git a/sync_inventory/sync_inventory.py b/sync_inventory/sync_inventory.py index 922aba0..58d07b5 100644 --- a/sync_inventory/sync_inventory.py +++ b/sync_inventory/sync_inventory.py @@ -23,7 +23,9 @@ A failure in step 1, 2, or 3 (e.g. NetBox/network unreachable) does not block the rest, since step 4 just needs whatever hosts file, repo -checkouts, and installed dependencies already exist on disk. +checkouts, and installed dependencies already exist on disk. Each such +failure is reported to stderr as "WARNING: failed (...)" -- always, +regardless of --verbose. This command only regenerates commands/; it never runs them. Use run-play to actually execute a generated script (or all of them). @@ -31,8 +33,8 @@ -u/--repo-url can also be set via the REPO_URL environment variable, same as NETBOX_URL/NETBOX_TOKEN/NETBOX_OWNERS are for fetch-meta. -Quiet by default: routine progress and warning/error messages are only -printed with --verbose. +Quiet by default: routine progress is only printed with --verbose. +Warnings and errors always print, regardless of --verbose. Refuses to run if another instance is already in progress (lock: .sync_inventory.lock in the current directory) regardless of verbosity. @@ -40,6 +42,7 @@ import argparse import os +import sys from pathlib import Path from sync_inventory.fetch_meta import fetch_meta @@ -70,7 +73,7 @@ def main(): ) parser.add_argument( "-v", "--verbose", action="store_true", - help="Print routine progress plus warning/error messages (quiet by default)", + help="Print routine progress (quiet by default); warnings and errors always print", ) args = parser.parse_args() if not args.repo_url: @@ -94,22 +97,19 @@ def main(): try: fetch_meta(args.hosts_file, verbose=args.verbose) except Exception as e: - if args.verbose: - print(f"WARNING: fetch-meta failed ({e}); continuing with existing {args.hosts_file}") + print(f"WARNING: fetch-meta failed ({e}); continuing with existing {args.hosts_file}", file=sys.stderr) section("pull-repo", args.verbose) try: pull_repo(args.repo_url, args.repo_dir, verbose=args.verbose) except Exception as e: - if args.verbose: - print(f"WARNING: pull-repo failed ({e}); continuing with existing {args.repo_dir}/ state") + print(f"WARNING: pull-repo failed ({e}); continuing with existing {args.repo_dir}/ state", file=sys.stderr) section("install-requirements", args.verbose) try: install_requirements(args.repo_dir, verbose=args.verbose) except Exception as e: - if args.verbose: - print(f"WARNING: install-requirements failed ({e}); continuing with existing {args.repo_dir}/ dependencies") + print(f"WARNING: install-requirements failed ({e}); continuing with existing {args.repo_dir}/ dependencies", file=sys.stderr) section("generate-playbook-commands", args.verbose) generate_playbook_commands(args.hosts_file, args.repo_dir, args.commands_dir, verbose=args.verbose) From e1a976bb353d951c9e2e4d9c6573812843ad97e9 Mon Sep 17 00:00:00 2001 From: Phong Tran Date: Wed, 16 Sep 2026 16:06:07 -0400 Subject: [PATCH 3/4] SVCPLAN-9489: add host warnings to only verbose --- README.md | 7 +++++-- sync_inventory/fetch_meta.py | 16 +++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 88ef9fd..de18a9a 100644 --- a/README.md +++ b/README.md @@ -36,8 +36,11 @@ An env pointed at a branch that isn't checked out, or a role with no matching playbook, gets skipped and reported rather than stopping everything else. Losing the connection to NetBox or the git remote just means it falls back to whatever it already had, rather than failing -outright. Warnings and errors always print to stderr, regardless of -`-v`/`--verbose` — that flag only adds routine progress output on top. +outright. Most warnings and errors always print to stderr, regardless of +`-v`/`--verbose` — that flag mainly adds routine progress output on top, +with one exception: `fetch-meta`'s per-host warnings (a NetBox entry with +no `dns_name`, or missing `role`/`env`) are noisy at NetBox-fleet scale, so +those stay `-v`-gated like routine progress does. Skipping happens per **env** or per **(env, role)**, never per individual host — no inventory file is generated anymore (see below), so individual diff --git a/sync_inventory/fetch_meta.py b/sync_inventory/fetch_meta.py index 267a9c0..7eb43f7 100644 --- a/sync_inventory/fetch_meta.py +++ b/sync_inventory/fetch_meta.py @@ -8,10 +8,10 @@ expects. Only entries with role/env actually set are included; entries missing -either are reported as a warning (always printed, regardless of --verbose) -and skipped rather than silently dropped. By default, entries where -nbmeta's "ansible" flag is explicitly false are excluded too, since those -hosts are marked as not managed by this Ansible controller. +either are reported as a warning (only with --verbose) and skipped rather +than silently dropped. By default, entries where nbmeta's "ansible" flag +is explicitly false are excluded too, since those hosts are marked as not +managed by this Ansible controller. Configuration is read from the environment, same as nbmeta: NETBOX_URL - NetBox instance URL @@ -78,12 +78,14 @@ def fetch_hosts(nb, owner_ids, ansible_only=True, verbose=False): hosts = {} for ip in nb.ipam.ip_addresses.filter(owner_id=owner_ids): if not ip.dns_name: - print(f"warning: skipping {ip.address}, no dns_name set", file=sys.stderr) + if verbose: + print(f"warning: skipping {ip.address}, no dns_name set", file=sys.stderr) continue meta = split_leading_json(ip.description or "") if "role" not in meta or "env" not in meta: - print(f"warning: skipping {ip.dns_name} ({ip.address}), missing role/env in description", file=sys.stderr) + if verbose: + print(f"warning: skipping {ip.dns_name} ({ip.address}), missing role/env in description", file=sys.stderr) continue if ansible_only and not meta.get("ansible", True): @@ -134,7 +136,7 @@ def main(): "--include-non-ansible", action="store_true", help="Also include hosts where nbmeta's ansible flag is false (default: only ansible=true hosts)", ) - parser.add_argument("-v", "--verbose", action="store_true", help="Print the final Wrote message (per-host and config warnings always print)") + parser.add_argument("-v", "--verbose", action="store_true", help="Print per-host warnings and the final Wrote message") args = parser.parse_args() try: From eff85134ee480d169f8473160cbb63844eb2e39f Mon Sep 17 00:00:00 2001 From: Phong Tran Date: Wed, 16 Sep 2026 16:18:27 -0400 Subject: [PATCH 4/4] SVCPLAN-9489: Reorganize WARNING/ERROR classification --- sync_inventory/fetch_meta.py | 12 ++++++------ sync_inventory/generate_playbook_commands.py | 20 +++++++++++--------- sync_inventory/install_requirements.py | 9 +++++---- sync_inventory/pull_repo.py | 19 +++++++++++-------- sync_inventory/run_play.py | 17 ++++++++++++----- sync_inventory/sync_inventory.py | 18 +++++++++++------- 6 files changed, 56 insertions(+), 39 deletions(-) diff --git a/sync_inventory/fetch_meta.py b/sync_inventory/fetch_meta.py index 7eb43f7..66604db 100644 --- a/sync_inventory/fetch_meta.py +++ b/sync_inventory/fetch_meta.py @@ -79,13 +79,13 @@ def fetch_hosts(nb, owner_ids, ansible_only=True, verbose=False): for ip in nb.ipam.ip_addresses.filter(owner_id=owner_ids): if not ip.dns_name: if verbose: - print(f"warning: skipping {ip.address}, no dns_name set", file=sys.stderr) + print(f"WARNING: {ip.address} has no dns_name set; skipping this entry", file=sys.stderr) continue meta = split_leading_json(ip.description or "") if "role" not in meta or "env" not in meta: if verbose: - print(f"warning: skipping {ip.dns_name} ({ip.address}), missing role/env in description", file=sys.stderr) + print(f"WARNING: {ip.dns_name} ({ip.address}) is missing role/env in its description; skipping this entry", file=sys.stderr) continue if ansible_only and not meta.get("ansible", True): @@ -110,13 +110,13 @@ def fetch_meta(hosts_file="hosts.json", ansible_only=True, verbose=False): owner_names = get_owner_names() owner_ids, unresolved = resolve_owner_ids(nb, owner_names) if unresolved: - print(f"warning: NETBOX_OWNERS not found in NetBox, ignoring: {unresolved}", file=sys.stderr) + print(f"WARNING: {unresolved} not found in NetBox; ignoring and continuing with the rest of NETBOX_OWNERS", file=sys.stderr) hosts = fetch_hosts(nb, owner_ids, ansible_only=ansible_only, verbose=verbose) if not hosts: print( - "warning: no matching hosts found (check NETBOX_OWNERS, and that entries have " - "role/env set via nbmeta)", + f"WARNING: no matching hosts found (check NETBOX_OWNERS, and that entries have " + f"role/env set via nbmeta); writing an empty {hosts_file}", file=sys.stderr, ) @@ -142,7 +142,7 @@ def main(): try: fetch_meta(args.hosts_file, ansible_only=not args.include_non_ansible, verbose=args.verbose) except NetBoxConfigError as exc: - raise SystemExit(f"error: {exc}") + raise SystemExit(f"ERROR: {exc}") if __name__ == "__main__": diff --git a/sync_inventory/generate_playbook_commands.py b/sync_inventory/generate_playbook_commands.py index 5a03684..5ccb0de 100644 --- a/sync_inventory/generate_playbook_commands.py +++ b/sync_inventory/generate_playbook_commands.py @@ -27,11 +27,13 @@ redirected to a log file by the script itself. run-play runs one or every script in --commands-dir and handles logging/failure-tracking itself. -If a branch isn't checked out under repo/, an error is reported for that env -and its commands are skipped. A role whose playbook file doesn't actually -exist in that branch's checkout is reported as a warning and skipped (the -netbox data only records intent, not what playbooks actually exist). Both -of these print to stderr always, regardless of --verbose. +If a branch isn't checked out under repo/, that's a WARNING: that env is +skipped (stated in the message) and the run continues with the rest. A +role whose playbook file doesn't actually exist in that branch's checkout +is likewise a WARNING and is skipped (the netbox data only records intent, +not what playbooks actually exist). Both print to stderr always, regardless +of --verbose, since nothing here ever aborts generate-playbook-commands +itself -- there's no ERROR-level condition in this command. If that branch has its own ansible.cfg, ANSIBLE_CONFIG is set to it for that command (Ansible only auto-discovers ansible.cfg via the current directory, @@ -61,7 +63,7 @@ def load_hosts(hosts_file): with open(hosts_file) as f: return json.load(f) except FileNotFoundError: - print(f"warning: hosts file '{hosts_file}' not found; treating as empty", file=sys.stderr) + print(f"WARNING: hosts file '{hosts_file}' not found; treating as empty", file=sys.stderr) return {} @@ -71,7 +73,7 @@ def sanitize_env_name(env): "pttran3/SVCPLAN-1234/test".""" sanitized = sanitize_dir_name(env) if sanitized != env: - print(f"warning: env '{env}' has invalid directory characters; using '{sanitized}' instead", file=sys.stderr) + print(f"WARNING: env '{env}' has invalid directory characters; using '{sanitized}' instead", file=sys.stderr) return sanitized @@ -119,7 +121,7 @@ def generate_playbook_commands(hosts_file="hosts.json", repo_dir="repo", command branch_dir = repo_dir / branch if not branch_dir.is_dir(): - print(f"ERROR: branch '{branch}' not found under {repo_dir} (expected {branch_dir})", file=sys.stderr) + print(f"WARNING: branch '{branch}' not found under {repo_dir} (expected {branch_dir}); skipping this env", file=sys.stderr) continue env_vars = {} @@ -156,7 +158,7 @@ def main(): "--commands-dir", default="commands", help="Directory to write one script per ansible-playbook command into (default: %(default)s)", ) - parser.add_argument("-v", "--verbose", action="store_true", help="Print each script written (branch/role warnings and errors always print, regardless of this flag)") + parser.add_argument("-v", "--verbose", action="store_true", help="Print each script written (branch/role warnings always print, regardless of this flag)") args = parser.parse_args() generate_playbook_commands(args.hosts_file, args.repo_dir, args.commands_dir, verbose=args.verbose) diff --git a/sync_inventory/install_requirements.py b/sync_inventory/install_requirements.py index 57440ac..51e2a45 100644 --- a/sync_inventory/install_requirements.py +++ b/sync_inventory/install_requirements.py @@ -12,9 +12,10 @@ at these same directories for that branch's generated commands, so installed dependencies are actually found at playbook-run time. -If ansible-galaxy fails for a branch, the failure (including its actual -output) is printed to stderr -- always, regardless of --verbose -- and -that branch is skipped rather than aborting the rest of the run. +If ansible-galaxy fails for a branch, that's a WARNING: the failure +(including its actual output) is printed to stderr -- always, regardless +of --verbose -- that branch's install is skipped (stated in the message), +and the run continues with the remaining branches rather than aborting. """ import argparse @@ -44,7 +45,7 @@ def run_galaxy(cmd, env, verbose=False): try: subprocess.run(cmd, check=True, env=env, capture_output=not verbose, text=True) except subprocess.CalledProcessError as e: - print(f"error: `{' '.join(cmd)}` failed (exit {e.returncode})", file=sys.stderr) + print(f"WARNING: `{' '.join(cmd)}` failed (exit {e.returncode}); skipping this branch's install", file=sys.stderr) if e.stdout: print(e.stdout.rstrip(), file=sys.stderr) if e.stderr: diff --git a/sync_inventory/pull_repo.py b/sync_inventory/pull_repo.py index 53e12a4..68c46d3 100644 --- a/sync_inventory/pull_repo.py +++ b/sync_inventory/pull_repo.py @@ -17,8 +17,11 @@ repo/", "Updated repo/", or "Cloned repo/"). Git's own (much noisier) output is never shown, except when a branch's update or clone fails, where it's printed as-is to stderr -- always, regardless of ---verbose -- so the actual error is visible. A branch that fails is -skipped rather than aborting the rest of the run. +--verbose -- so the actual error is visible. A branch that fails to update +or clone is a WARNING: it's skipped (the action taken is stated in the +message) rather than aborting the rest of the run. Not being able to list +branches at all (e.g. the repo URL is unreachable) is an ERROR: nothing +else can proceed without a branch list, so pull-repo aborts. Usage: pull-repo [-r REPO_DIR] [-v] @@ -52,8 +55,8 @@ def run(cmd, verbose=False): raise subprocess.CalledProcessError(result.returncode, cmd, output=result.stdout, stderr=result.stderr) -def print_git_error(action, e): - print(f"ERROR: {action} failed: `{' '.join(e.cmd)}` (exit {e.returncode})", file=sys.stderr) +def print_git_error(label, action, e, next_step): + print(f"{label}: {action} failed: `{' '.join(e.cmd)}` (exit {e.returncode}); {next_step}", file=sys.stderr) if e.stdout: print(e.stdout.rstrip(), file=sys.stderr) if e.stderr: @@ -81,7 +84,7 @@ def sync_branch(repo_url, branch, branch_dir, verbose=False): run(["git", "-C", str(branch_dir), "reset", "--hard", "FETCH_HEAD"], verbose=verbose) run(["git", "-C", str(branch_dir), "clean", "-fd"], verbose=verbose) except subprocess.CalledProcessError as e: - print_git_error(f"updating {branch_dir}", e) + print_git_error("WARNING", f"updating {branch_dir}", e, "skipping this branch") return if verbose: print(f"{'No update to' if before == after else 'Updated'} {branch_dir}") @@ -89,7 +92,7 @@ def sync_branch(repo_url, branch, branch_dir, verbose=False): try: run(["git", "clone", "--branch", branch, "--single-branch", repo_url, str(branch_dir)], verbose=verbose) except subprocess.CalledProcessError as e: - print_git_error(f"cloning {branch_dir}", e) + print_git_error("WARNING", f"cloning {branch_dir}", e, "skipping this branch") return if verbose: print(f"Cloned {branch_dir}") @@ -102,7 +105,7 @@ def pull_repo(repo_url, repo_dir="repo", verbose=False): try: branches = list_remote_branches(repo_url, verbose=verbose) except subprocess.CalledProcessError as e: - print_git_error(f"listing branches in {repo_url}", e) + print_git_error("ERROR", f"listing branches in {repo_url}", e, "aborting") raise PullRepoError(f"Could not list branches in {repo_url}") if not branches: raise PullRepoError(f"No branches found in {repo_url}") @@ -125,7 +128,7 @@ def main(): try: pull_repo(args.repo_url, args.repo_dir, verbose=args.verbose) except PullRepoError as exc: - raise SystemExit(f"error: {exc}") + raise SystemExit(f"ERROR: {exc}") if __name__ == "__main__": diff --git a/sync_inventory/run_play.py b/sync_inventory/run_play.py index d0a2a60..8201044 100644 --- a/sync_inventory/run_play.py +++ b/sync_inventory/run_play.py @@ -20,6 +20,13 @@ whatever the branch's own ansible.cfg declares. Valid with both -s/--script and --all. +One script failing is a WARNING: it's recorded as failed (stated in the +message) and the rest of the run continues to the next script. Only once +every script has been attempted does run-play quit -- with an ERROR +summarizing how many failed. Anything that stops run-play before it even +starts running scripts (a bad flag combination, no matching script found) +is likewise an ERROR. + Usage: run-play -s pttran3_test_branch_proxmox run-play -s pttran3_test_branch_proxmox -H some-host.example.com @@ -50,7 +57,7 @@ def run_script(script, logs_dir, host=None, inventory=None, quiet=False, verbose sys.stdout.write(line) returncode = process.wait() if returncode != 0: - print(f"FAILED: {' '.join(cmd)} (see {log_file})", file=sys.stderr) + print(f"WARNING: {' '.join(cmd)} failed (see {log_file}); marking it failed and continuing", file=sys.stderr) return False return True @@ -67,7 +74,7 @@ def list_commands(commands_dir="commands"): def run_play(script_name=None, run_all=False, commands_dir="commands", logs_dir="logs", host=None, inventory=None, quiet=False, verbose=False): if run_all and host: - raise SystemExit("-H/--host can only be used with -s/--script, not --all") + raise SystemExit("ERROR: -H/--host can only be used with -s/--script, not --all") commands_dir = Path(commands_dir) logs_dir = Path(logs_dir) @@ -76,16 +83,16 @@ def run_play(script_name=None, run_all=False, commands_dir="commands", logs_dir= if run_all: scripts = sorted(commands_dir.glob("*.sh")) if not scripts: - raise SystemExit(f"No command scripts found under {commands_dir}") + raise SystemExit(f"ERROR: no command scripts found under {commands_dir}") else: script = commands_dir / f"{script_name}.sh" if not script.is_file(): - raise SystemExit(f"No command script found at {script}") + raise SystemExit(f"ERROR: no command script found at {script}") scripts = [script] failures = sum(not run_script(script, logs_dir, host=host, inventory=inventory, quiet=quiet, verbose=verbose) for script in scripts) if failures: - raise SystemExit(f"{failures} command(s) failed") + raise SystemExit(f"ERROR: {failures} command(s) failed") def main(): diff --git a/sync_inventory/sync_inventory.py b/sync_inventory/sync_inventory.py index 58d07b5..20c642a 100644 --- a/sync_inventory/sync_inventory.py +++ b/sync_inventory/sync_inventory.py @@ -21,11 +21,13 @@ own inventory, the same as it would for a human running ansible-playbook by hand from that checkout. -A failure in step 1, 2, or 3 (e.g. NetBox/network unreachable) does not -block the rest, since step 4 just needs whatever hosts file, repo -checkouts, and installed dependencies already exist on disk. Each such -failure is reported to stderr as "WARNING: failed (...)" -- always, -regardless of --verbose. +A failure in step 1, 2, or 3 (e.g. NetBox/network unreachable) is a +WARNING, not an ERROR: it does not block the rest, since step 4 just needs +whatever hosts file, repo checkouts, and installed dependencies already +exist on disk. Each such failure is reported to stderr as "WARNING: +failed (...); continuing with " -- always, +regardless of --verbose. The only ERROR in this command is the lock check +below, which does stop sync-inventory immediately. This command only regenerates commands/; it never runs them. Use run-play to actually execute a generated script (or all of them). @@ -34,7 +36,9 @@ same as NETBOX_URL/NETBOX_TOKEN/NETBOX_OWNERS are for fetch-meta. Quiet by default: routine progress is only printed with --verbose. -Warnings and errors always print, regardless of --verbose. +Warnings and errors always print, regardless of --verbose. ERROR means +sync-inventory quits immediately; WARNING means it acknowledges the issue, +states what it's doing about it, and keeps going. Refuses to run if another instance is already in progress (lock: .sync_inventory.lock in the current directory) regardless of verbosity. @@ -83,7 +87,7 @@ def main(): LOCK_DIR.mkdir() except FileExistsError: raise SystemExit( - f"Another sync-inventory is already in progress (lock: {LOCK_DIR}). Exiting.\n" + f"ERROR: another sync-inventory is already in progress (lock: {LOCK_DIR}); exiting.\n" f"If no other run is actually in progress (e.g. a previous run was killed), " f"remove the stale lock with: rmdir {LOCK_DIR}" )