diff --git a/docs/PERCONA_OBS_TOOL.md b/docs/PERCONA_OBS_TOOL.md index 5c7677e2..f25a693c 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 ab414ccb..329d6a19 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 3850399d..8656b007 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]