From 567f9da619dd781146e6f5038012eda8af175b8f Mon Sep 17 00:00:00 2001 From: Simone Date: Sun, 26 Jul 2026 02:32:19 +0200 Subject: [PATCH] feat(template): skip the nightly backup when the template hasn't changed template backup ran a full multi-GB, multi-minute 7z pass over every template every night regardless of whether anything under it had actually changed -- the systemd unit's own comment already noted the resulting archives were byte-identical run to run. Now compares the template dir's newest file mtime against its most recent existing backup and skips the compress step (still honoring --keep-count via prune) when nothing is newer. --force opts back into an unconditional backup. --- deploy/systemd/ci-backup-template.service | 7 +- src/ci_orchestrator/commands/template.py | 50 +++++- tests/python/test_commands_template.py | 196 ++++++++++++++++++++++ 3 files changed, 249 insertions(+), 4 deletions(-) diff --git a/deploy/systemd/ci-backup-template.service b/deploy/systemd/ci-backup-template.service index 5575011..39659f6 100644 --- a/deploy/systemd/ci-backup-template.service +++ b/deploy/systemd/ci-backup-template.service @@ -17,8 +17,11 @@ WorkingDirectory=/opt/ci/local-ci-cd-system ExecStartPre=/usr/bin/install -d -o ci-runner -g ci-runner -m 750 /var/lib/ci/backups # --keep-count 1: three generations of Windows templates cost ~195 GB and the # archives were byte-identical between runs (the templates rarely change), so -# the extra generations bought nothing. --exclude WinBuildTest: retired test -# template, ~26 GB per generation. +# the extra generations bought nothing. Since the archives were identical, +# `template backup` now also skips the 7z pass itself by default whenever the +# template directory has not changed since its last backup (no flag needed +# here; --force would opt back into an unconditional backup). +# --exclude WinBuildTest: retired test template, ~26 GB per generation. ExecStart=/opt/ci/venv/bin/python -m ci_orchestrator template backup --all-templates --keep-count 1 --exclude WinBuildTest # Safety net: bring act-runner.service back up on any exit path (success, # failure, signal). The Python `finally` block can be skipped if the diff --git a/src/ci_orchestrator/commands/template.py b/src/ci_orchestrator/commands/template.py index c3aac53..15d9723 100644 --- a/src/ci_orchestrator/commands/template.py +++ b/src/ci_orchestrator/commands/template.py @@ -7,6 +7,9 @@ template backup 7z archives of VMware template directories and prunes old backups beyond ``--keep-count``. Stops ``act-runner.service`` before copying and restarts it afterwards; pass ``--skip-runner-stop`` to bypass this. + Skips the (multi-GB, multi-minute) 7z pass entirely when nothing under + the template directory has changed since its most recent backup — + pass ``--force`` to back up anyway. template prepare-win Provisions a Windows template VM end-to-end via pypsrp (replaces the @@ -139,6 +142,14 @@ def template() -> None: is_flag=True, help="Dry run — log what would happen without copying or deleting.", ) +@click.option( + "--force", + is_flag=True, + help=( + "Back up even if the template has not changed since its most recent " + "backup (the default skips the multi-GB 7z pass in that case)." + ), +) def template_backup( template_path: str | None, all_templates: bool, @@ -147,6 +158,7 @@ def template_backup( keep_count: int, skip_runner_stop: bool, what_if: bool, + force: bool, ) -> None: """Create timestamped backup of CI template VM directories.""" config = load_config() @@ -201,8 +213,16 @@ def template_backup( for tmpl_dir in to_backup: name_tag = f"{tmpl_dir.name}_{timestamp}" if all_templates else timestamp archive = backup_base / f"Template_{name_tag}.7z" + template_label = tmpl_dir.name if all_templates else None - if what_if: + previous = None if force else _latest_backup(backup_base, template_label) + if previous is not None and _newest_mtime(tmpl_dir) <= previous.stat().st_mtime: + verb = "WhatIf: would skip" if what_if else "Skipping" + click.echo( + f"[Backup-CITemplate] {verb} {tmpl_dir} " + f"(unchanged since {previous.name})." + ) + elif what_if: click.echo( f"[Backup-CITemplate] WhatIf: would compress {tmpl_dir} -> {archive}" ) @@ -214,7 +234,6 @@ def template_backup( f"[Backup-CITemplate] Backup complete: {archive} ({size_mb} MB)" ) - template_label = tmpl_dir.name if all_templates else None _prune_backups(backup_base, template_label, keep_count, what_if=what_if) finally: @@ -268,6 +287,33 @@ def _compress_7z(source: Path, archive: Path) -> None: ) +def _newest_mtime(source: Path) -> float: + """Latest mtime among ``source``'s files, recursively. ``0.0`` if empty.""" + return max((f.stat().st_mtime for f in source.rglob("*") if f.is_file()), default=0.0) + + +def _latest_backup(backup_base: Path, template_name: str | None) -> Path | None: + """Most recent existing archive for this template, or ``None``. + + Same glob pattern :func:`_prune_backups` matches against, so "the + template hasn't changed since its most recent backup" compares against + the exact file that backup would otherwise replace. + """ + if not backup_base.exists(): + return None + pattern = ( + f"Template_{template_name}_????????_??????.7z" + if template_name + else "Template_????????_??????.7z" + ) + existing = [ + f for f in backup_base.iterdir() if f.is_file() and fnmatch.fnmatch(f.name, pattern) + ] + if not existing: + return None + return max(existing, key=lambda f: f.stat().st_mtime) + + def _prune_backups( backup_base: Path, template_name: str | None, diff --git a/tests/python/test_commands_template.py b/tests/python/test_commands_template.py index 7f10b68..0364b5e 100644 --- a/tests/python/test_commands_template.py +++ b/tests/python/test_commands_template.py @@ -202,6 +202,202 @@ def test_what_if_no_copy(template_dir: Path, backup_dir: Path, _no_systemctl: No assert "WhatIf" in result.output +# ── skip-if-unchanged ──────────────────────────────────────────────────────── + + +def test_backup_skips_when_unchanged_since_last_backup( + template_dir: Path, backup_dir: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The nightly timer runs every night; most nights nothing in a template + changed, so the (very expensive) 7z pass over tens of GB is pure waste.""" + past = time.time() - 7200 + for f in template_dir.iterdir(): + os.utime(f, (past, past)) + # This backup's own mtime is "now" (just written) -- newer than every + # file the template directory holds. + existing = backup_dir / "Template_20250101_000000.7z" + existing.write_bytes(b"old-archive") + + compress_calls: list[Path] = [] + monkeypatch.setattr( + tmpl_mod, "_compress_7z", lambda src, dst: compress_calls.append(src) + ) + monkeypatch.setattr(tmpl_mod, "_stop_runner", lambda **_kw: False) + monkeypatch.setattr(tmpl_mod, "_start_runner", lambda **_kw: None) + + result = CliRunner().invoke( + cli, + [ + "template", + "backup", + "--template-path", + str(template_dir), + "--backup-base-dir", + str(backup_dir), + "--skip-runner-stop", + ], + ) + + assert result.exit_code == 0, result.output + assert compress_calls == [] + assert "unchanged" in result.output.lower() + archives = [f for f in backup_dir.iterdir() if f.suffix == ".7z"] + assert archives == [existing] + + +def test_backup_runs_when_changed_since_last_backup( + template_dir: Path, backup_dir: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + old_backup = backup_dir / "Template_20250101_000000.7z" + old_backup.write_bytes(b"old-archive") + old_time = time.time() - 7200 + os.utime(old_backup, (old_time, old_time)) + # template_dir's files keep their fixture-default "now" mtime -- newer + # than the existing backup. + + compress_calls: list[Path] = [] + monkeypatch.setattr( + tmpl_mod, + "_compress_7z", + lambda src, dst: (compress_calls.append(src), dst.write_bytes(b"fake"))[-1], + ) + monkeypatch.setattr(tmpl_mod, "_stop_runner", lambda **_kw: False) + monkeypatch.setattr(tmpl_mod, "_start_runner", lambda **_kw: None) + + result = CliRunner().invoke( + cli, + [ + "template", + "backup", + "--template-path", + str(template_dir), + "--backup-base-dir", + str(backup_dir), + "--skip-runner-stop", + ], + ) + + assert result.exit_code == 0, result.output + assert compress_calls == [template_dir] + + +def test_force_flag_backs_up_even_when_unchanged( + template_dir: Path, backup_dir: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + past = time.time() - 7200 + for f in template_dir.iterdir(): + os.utime(f, (past, past)) + existing = backup_dir / "Template_20250101_000000.7z" + existing.write_bytes(b"old-archive") + + compress_calls: list[Path] = [] + monkeypatch.setattr( + tmpl_mod, + "_compress_7z", + lambda src, dst: (compress_calls.append(src), dst.write_bytes(b"fake"))[-1], + ) + monkeypatch.setattr(tmpl_mod, "_stop_runner", lambda **_kw: False) + monkeypatch.setattr(tmpl_mod, "_start_runner", lambda **_kw: None) + + result = CliRunner().invoke( + cli, + [ + "template", + "backup", + "--template-path", + str(template_dir), + "--backup-base-dir", + str(backup_dir), + "--skip-runner-stop", + "--force", + ], + ) + + assert result.exit_code == 0, result.output + assert compress_calls == [template_dir] + + +def test_what_if_reports_skip_when_unchanged( + template_dir: Path, backup_dir: Path, _no_systemctl: None +) -> None: + past = time.time() - 7200 + for f in template_dir.iterdir(): + os.utime(f, (past, past)) + existing = backup_dir / "Template_20250101_000000.7z" + existing.write_bytes(b"old-archive") + + result = CliRunner().invoke( + cli, + [ + "template", + "backup", + "--template-path", + str(template_dir), + "--backup-base-dir", + str(backup_dir), + "--skip-runner-stop", + "--what-if", + ], + ) + + assert result.exit_code == 0, result.output + assert "WhatIf" in result.output + assert "unchanged" in result.output.lower() + archives = [f for f in backup_dir.iterdir() if f.suffix == ".7z"] + assert archives == [existing] + + +def test_all_templates_skips_unchanged_and_backs_up_changed( + tmp_path: Path, backup_dir: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Skip/run is decided per template, not for the whole ``--all-templates`` + pass at once.""" + tmpl_root = tmp_path / "templates" + unchanged = tmpl_root / "LinuxBuild2404" + changed = tmpl_root / "WinBuild2025" + for d in (unchanged, changed): + d.mkdir(parents=True) + (d / f"{d.name}.vmx").write_text("vmx") + + past = time.time() - 7200 + os.utime(unchanged / "LinuxBuild2404.vmx", (past, past)) + # `changed`'s file keeps its just-created "now" mtime. + + existing_unchanged = backup_dir / "Template_LinuxBuild2404_20250101_000000.7z" + existing_unchanged.write_bytes(b"old") + existing_changed = backup_dir / "Template_WinBuild2025_20250101_000000.7z" + existing_changed.write_bytes(b"old") + # Newer than `unchanged`'s file, older than `changed`'s. + mid_time = time.time() - 3600 + os.utime(existing_unchanged, (mid_time, mid_time)) + os.utime(existing_changed, (mid_time, mid_time)) + + monkeypatch.setenv("CI_TEMPLATES", str(tmpl_root)) + compress_calls: list[Path] = [] + monkeypatch.setattr( + tmpl_mod, + "_compress_7z", + lambda src, dst: (compress_calls.append(src), dst.write_bytes(b"fake"))[-1], + ) + monkeypatch.setattr(tmpl_mod, "_stop_runner", lambda **_kw: False) + monkeypatch.setattr(tmpl_mod, "_start_runner", lambda **_kw: None) + + result = CliRunner().invoke( + cli, + [ + "template", + "backup", + "--all-templates", + "--backup-base-dir", + str(backup_dir), + "--skip-runner-stop", + ], + ) + + assert result.exit_code == 0, result.output + assert compress_calls == [changed] + + # ── runner stop/start ─────────────────────────────────────────────────────────