From ea87280a37bca5f04c036238171927acf2e66979 Mon Sep 17 00:00:00 2001 From: Ricardo Dias Date: Fri, 30 Oct 2026 22:04:22 +0000 Subject: [PATCH] release freeze: restore only the element on unfreeze restore_builds pushed the whole pre-freeze meta snapshot back after osc release. On isv:percona:ppg:staging:18 the snapshot still carried 11 releasetargets that OBS had rewritten to project="deleted" (left by a cancelled populate), so the restore put back what the release hop had just removed (meta rev 73 clean, rev 74 restored them). The freeze only rewrites , so restore now swaps only the snapshot's into the current meta (projects and packages) and keeps everything changed inside the freeze window. If the current meta cannot be read it falls back to the full snapshot so builds are never left disabled. Signed-off-by: Ricardo Dias --- docs/PERCONA_OBS_TOOL.md | 9 ++++-- percona_obs/release_freeze.py | 51 +++++++++++++++++++++++++------ tests/test_release_freeze.py | 56 ++++++++++++++++++++++++++++++++--- 3 files changed, 100 insertions(+), 16 deletions(-) diff --git a/docs/PERCONA_OBS_TOOL.md b/docs/PERCONA_OBS_TOOL.md index 5c7677e26..f25a693c4 100644 --- a/docs/PERCONA_OBS_TOOL.md +++ b/docs/PERCONA_OBS_TOOL.md @@ -1469,9 +1469,12 @@ Flags: 5. **Verify** — still inside the freeze window, polls (bounded by `--verify-timeout`) until each released repo holds binaries. 6. **Restore** — in a `finally` (so it still runs even if verify times out or - raises), re-apply each project's exact snapshotted meta last (never a blanket - enable, since subprojects carry per-repo flags — e.g. tarballs' `publish:` - flags — that must survive the round trip). + raises), put back each project's snapshotted `` element last (never a + blanket enable, since subprojects carry per-repo build flags that must survive + the round trip). Only `` is restored, onto the meta as it is now: changes + made inside the freeze window (the hop's own release targets, added and removed + around `osc release`) are kept, not rolled back to the pre-freeze snapshot. If the + current meta cannot be read, the full snapshot is restored instead. 5. Release-tier OBS projects with no local mirror are reported loudly as orphans; deletion is never automatic — run `sync delete` manually. diff --git a/percona_obs/release_freeze.py b/percona_obs/release_freeze.py index ab414ccb3..329d6a196 100644 --- a/percona_obs/release_freeze.py +++ b/percona_obs/release_freeze.py @@ -158,6 +158,32 @@ def _disabled_meta(raw: str) -> str: return ET.tostring(root, encoding="unicode") +def _restored_meta(current: str, snapshot: str) -> str: + """*current* meta with its ```` replaced by the snapshot's. + + The freeze only rewrites ````; everything else (e.g. release + targets added and removed inside the freeze window) stays as it is now + rather than being rolled back to the pre-freeze snapshot. + """ + root = ET.fromstring(current) + for build_elem in root.findall("build"): + root.remove(build_elem) + for build_elem in ET.fromstring(snapshot).findall("build"): + root.append(build_elem) + ET.indent(root, space=" ") + return ET.tostring(root, encoding="unicode") + + +def _meta_to_restore(fetch_current, snapshot: str) -> str: + """Restore target for *snapshot*; the snapshot itself if the current meta + cannot be read or parsed (unfreezing beats leaving builds disabled).""" + try: + return _restored_meta(_decode_obs_response(fetch_current()), snapshot) + except Exception as exc: + print(f"warning: restoring the full pre-freeze meta: {exc}", flush=True) + return snapshot + + def freeze_packages( apiurl: str, packages: "dict[str, set[str]]" ) -> "dict[tuple[str, str], str]": @@ -190,9 +216,12 @@ def freeze_packages( def restore_packages(apiurl: str, snapshots: "dict[tuple[str, str], str]") -> None: - """Push package meta snapshots back verbatim. Never raises.""" - for (prj, pkg), meta in snapshots.items(): + """Restore the snapshotted ```` flags on each package. Never raises.""" + for (prj, pkg), snapshot in snapshots.items(): try: + meta = _meta_to_restore( + lambda: osc.core.show_package_meta(apiurl, prj, pkg), snapshot + ) osc.core.edit_meta( metatype="pkg", path_args=(prj, pkg), @@ -211,9 +240,9 @@ def restore_packages(apiurl: str, snapshots: "dict[tuple[str, str], str]") -> No def freeze_builds(apiurl: str, obs_projects: "list[str]") -> "dict[str, str]": """Disable builds on every project; return {project: prior_meta_xml}. - The snapshot is the exact meta XML fetched before modification, so - restore_builds round-trips per-repo flags (publish, debuginfo, partial - build enables) without reconstructing them. + The snapshot is the exact meta XML fetched before modification; + restore_builds puts back only its ```` element (per-repo partial + build enables included), the one element the freeze rewrites. If freezing any project fails partway through, every project frozen so far is restored before the exception propagates — otherwise the earlier @@ -235,11 +264,15 @@ def freeze_builds(apiurl: str, obs_projects: "list[str]") -> "dict[str, str]": def restore_builds(apiurl: str, snapshots: "dict[str, str]") -> None: - """Push snapshotted meta back verbatim. Never raises: a restore failure - is printed as a warning so the caller's finally-block does not mask the - primary exception, and the remaining projects are still restored.""" - for prj, meta in snapshots.items(): + """Restore the snapshotted ```` flags on each project. Never + raises: a restore failure is printed as a warning so the caller's + finally-block does not mask the primary exception, and the remaining + projects are still restored.""" + for prj, snapshot in snapshots.items(): try: + meta = _meta_to_restore( + lambda: osc.core.show_project_meta(apiurl, prj), snapshot + ) _edit_project_meta(apiurl, prj, meta, force=True) _print_update(f"{prj} (builds restored)") except Exception as exc: diff --git a/tests/test_release_freeze.py b/tests/test_release_freeze.py index 3850399dd..8656b007c 100644 --- a/tests/test_release_freeze.py +++ b/tests/test_release_freeze.py @@ -119,7 +119,41 @@ def test_freeze_snapshots_and_restores_exact_meta(monkeypatch): assert 'repository="R8"' in written[0][1] rf.restore_builds("http://obs", snaps) - assert written[-1] == ("prj", orig, True) + prj, restored, force = written[-1] + assert prj == "prj" and force + assert " is restored: a release target removed inside the freeze + window must not come back from the pre-freeze snapshot.""" + store = { + "prj": '<description/>' + '<build><disable repository="R9"/></build>' + '<repository name="R8">' + '<releasetarget project="deleted" repository="deleted" trigger="manual"/>' + "</repository></project>" + } + + def fake_edit(apiurl, prj, meta, force): + store[prj] = meta + + monkeypatch.setattr( + rf.osc.core, "show_project_meta", lambda apiurl, prj: store[prj].encode() + ) + monkeypatch.setattr(rf, "_edit_project_meta", fake_edit) + snaps = rf.freeze_builds("http://obs", ["prj"]) + assert "<disable />" in store["prj"] + # the release hop cleans the targets while the project is frozen + store["prj"] = store["prj"].replace( + '<releasetarget project="deleted" repository="deleted" trigger="manual" />', + "", + ) + assert "releasetarget" not in store["prj"] + rf.restore_builds("http://obs", snaps) + assert "releasetarget" not in store["prj"] + assert '<disable repository="R9" />' in store["prj"] + assert "<disable />" not in store["prj"] def test_freeze_partial_failure_restores_already_frozen_projects(monkeypatch): @@ -148,17 +182,31 @@ def fake_edit(apiurl, prj, meta, force): # p1 was frozen then restored; p2's failed edit is the second write. assert [w[0] for w in written] == ["p1", "p2", "p1"] assert "<disable" in written[0][1] # p1 frozen - assert written[2] == ("p1", origs["p1"], True) # p1 restored to exact prior meta + assert written[2][0] == "p1" and "<build" not in written[2][1] # p1 unfrozen def test_restore_never_raises(monkeypatch): def boom(*a, **k): raise RuntimeError("obs down") + monkeypatch.setattr(rf.osc.core, "show_project_meta", boom) monkeypatch.setattr(rf, "_edit_project_meta", boom) rf.restore_builds("http://obs", {"prj": "<project/>"}) # must not raise +def test_restore_falls_back_to_snapshot_when_meta_unreadable(monkeypatch): + def boom(*a, **k): + raise RuntimeError("obs down") + + written = [] + monkeypatch.setattr(rf.osc.core, "show_project_meta", boom) + monkeypatch.setattr( + rf, "_edit_project_meta", lambda a, prj, meta, force: written.append(meta) + ) + rf.restore_builds("http://obs", {"prj": "<project/>"}) + assert written == ["<project/>"] + + def test_verify_release_landed_ok(monkeypatch): sets = {"src": {"a", "b"}, "rel": {"a", "b", "extra"}} monkeypatch.setattr( @@ -243,7 +291,7 @@ def fake_edit(metatype, path_args, data, force, apiurl): assert edits[0][0] == "pkg" and edits[0][1] == ("deps", "etcd") assert "<disable" in edits[0][2] rf.restore_packages("http://obs", snaps) - assert edits[1][2] == metas[("deps", "etcd")] + assert "<build" not in edits[1][2] and 'name="etcd"' in edits[1][2] def test_freeze_packages_restores_on_partial_failure(monkeypatch): @@ -270,4 +318,4 @@ def fake_show(api, prj, pkg): rf.freeze_packages("http://obs", {"deps": {"a", "b"}}) # a was frozen then restored assert edits[0][0] == ("deps", "a") and "<disable" in edits[0][1] - assert edits[-1] == (("deps", "a"), metas[("deps", "a")]) + assert edits[-1][0] == ("deps", "a") and "<build" not in edits[-1][1]