feat(release-controller): add ignored_proposals to release-index.yaml (#2015)

pietrodimarco-dfinity · web-flow · commit 7d5355b89aad · 2026-05-18T13:52:40.000+02:00
diff --git a/release-controller/README.md b/release-controller/README.md
@@ -205,6 +205,42 @@ If the proposal was indeed submitted, you don't have to do anything -- the recon
   rm /state/<full_commit_hash>
   ```
 
+### Forcing resubmission of a previously-submitted proposal
+
+The reconciler rebuilds its in-memory state on every cycle from the public
+dashboard and the governance canister (via `dre proposals filter -t
+ic-os-version-election`), and treats every matching election proposal as
+"already submitted" regardless of its status.  A rejected or failed proposal
+will therefore prevent the reconciler from ever submitting a fresh one for
+the same version.
+
+To work around this, edit [`release-index.yaml`](../release-index.yaml)
+and add a top-level `ignored_proposals` entry listing the NNS proposal IDs
+to forget.  The reconciler reads this list on every cycle (no redeploy
+required) and drops the matching proposals from its dashboard / governance
+lookups before they reach `ReconcilerState`, so the affected versions go
+back to the "no proposal" state and a fresh proposal is submitted on the
+next cycle.
+
+```yaml
+ignored_proposals:
+  - 141776 # GuestOS election proposal rejected; resubmit the version.
+releases:
+  - rc_name: rc--...
+    versions:
+      - ...
+```
+
+Caveats:
+
+- The IC governance canister independently refuses to re-elect a version
+  that is already blessed.  This lever only helps when the prior
+  proposal did **not** result in a blessing (typically REJECTED or
+  FAILED).
+- Remove the entry from `release-index.yaml` once the replacement
+  proposal has been submitted.  Leaving it in indefinitely silently
+  swallows any future state for the same proposal id.
+
 ## Development
 
 Please see the parent folder's `README.md` for virtual environment setup.
@@ -243,11 +279,10 @@ bazel run //release-controller:release-controller \
 Typing errors preventing you from running it, because you are editing code and
 testing your changes?  Add `--output_groups=-mypy` right after `bazel run`.
 
-The optional argument `--skip-preloading-state` makes it so that the reconciler
-will not preload its list of known proposals by version from the governance
-canister.  It is useful (in conjunction with an empty reconciler state folder)
-to make the reconciler do all the work of submitting proposals again.  It should
-only be used alongside `--dry-run`, to avoid submitting proposals twice.
+To force the reconciler to resubmit a proposal whose prior submission was
+rejected or otherwise failed, edit `release-index.yaml` and add the
+proposal ID to the top-level `ignored_proposals` list.  See *Forcing
+resubmission of a previously-submitted proposal* above.
 
 ### Running the reconciler in the container it ships
 
diff --git a/release-controller/reconciler.py b/release-controller/reconciler.py
@@ -435,10 +435,80 @@ def __init__(
         self.change_determinator_factory = change_determinator_factory
         self.local_release_state: dict[str, dict[str, dict[OsKind, VersionState]]] = {}
 
+    @staticmethod
+    def _filtered_proposals_retriever(
+        retriever: typing.Callable[
+            [],
+            tuple[
+                dict[str, dre_cli.ElectionProposal],
+                dict[str, dre_cli.ElectionProposal],
+            ],
+        ],
+        source: str,
+        ignore_proposal_ids: typing.Iterable[int],
+    ) -> typing.Callable[
+        [],
+        tuple[
+            dict[str, dre_cli.ElectionProposal],
+            dict[str, dre_cli.ElectionProposal],
+        ],
+    ]:
+        """Wrap a proposal retriever so it drops ignored proposal IDs.
+
+        The wrapped retriever omits any proposal whose ``id`` is listed in
+        the index's top-level ``ignored_proposals`` so the affected versions
+        are treated by :class:`reconciler_state.ReconcilerState` as if no
+        proposal had been submitted.
+        """
+        ignore_set: set[int] = set(ignore_proposal_ids)
+
+        def _drop_ignored(
+            d: dict[str, dre_cli.ElectionProposal],
+            os_kind: OsKind,
+        ) -> dict[str, dre_cli.ElectionProposal]:
+            out: dict[str, dre_cli.ElectionProposal] = {}
+            for version, proposal in d.items():
+                if proposal["id"] in ignore_set:
+                    LOGGER.warning(
+                        "Ignoring %s %s election proposal %s for version %s"
+                        " (listed in release-index.yaml ignored_proposals);"
+                        " the reconciler will treat this version as not yet"
+                        " proposed.",
+                        source,
+                        os_kind,
+                        proposal["id"],
+                        version,
+                    )
+                    continue
+                out[version] = proposal
+            return out
+
+        def wrapped() -> tuple[
+            dict[str, dre_cli.ElectionProposal],
+            dict[str, dre_cli.ElectionProposal],
+        ]:
+            guestos, hostos = retriever()
+            if not ignore_set:
+                return guestos, hostos
+            return (
+                _drop_ignored(guestos, GUESTOS),
+                _drop_ignored(hostos, HOSTOS),
+            )
+
+        return wrapped
+
     def reconcile(self) -> None:
         """Reconcile the state of the network with the release index."""
         logger = LOGGER.getChild("reconciler")
         index = self.loader.index()
+        ignore_proposal_ids = list(index.root.ignored_proposals)
+        if ignore_proposal_ids:
+            logger.info(
+                "release-index.yaml requests that proposals %s be ignored"
+                " when populating reconciler state; the corresponding"
+                " versions will be treated as not yet proposed.",
+                ignore_proposal_ids,
+            )
 
         # As a matter of principle, we will only process the very top
         # two releases (and all its versions).  All else will be
@@ -449,7 +519,13 @@ def reconcile(self) -> None:
 
         # Fetch latest election proposals and remember their state.
         try:
-            self.state.update_state(self.dashboard.get_election_proposals_by_version)
+            self.state.update_state(
+                self._filtered_proposals_retriever(
+                    self.dashboard.get_election_proposals_by_version,
+                    source="dashboard",
+                    ignore_proposal_ids=ignore_proposal_ids,
+                )
+            )
         except Exception as e:
             logger.warning(
                 "Did not succeed in retrieving proposals from"
@@ -460,7 +536,13 @@ def reconcile(self) -> None:
         # from the DRE CLI (coming from governance canister)
         # after the dashboard, since these are the authoritative proposals
         # that have the freshest state (dashboard lags).
-        self.state.update_state(self.dre.get_election_proposals_by_version)
+        self.state.update_state(
+            self._filtered_proposals_retriever(
+                self.dre.get_election_proposals_by_version,
+                source="dre",
+                ignore_proposal_ids=ignore_proposal_ids,
+            )
+        )
 
         # Preload the cache of known successfully processed releases.
         # We will use this information as an operation plan.
diff --git a/release-controller/release_index.py b/release-controller/release_index.py
@@ -38,6 +38,7 @@ class Release(BaseModel):
 
 class ReleaseIndex(BaseModel):
     model_config = ConfigDict(extra="forbid")
+    ignored_proposals: List[int] = []
     releases: List[Release]
 
     def version(self, rc_name: str, name: str) -> str:
diff --git a/release-controller/tests/test_reconciler_integration.py b/release-controller/tests/test_reconciler_integration.py
@@ -6,6 +6,7 @@
 import git_repo
 import pydantic_yaml
 import pytest_mock.plugin
+import reconciler_state
 from dre_cli import ElectionProposal
 from public_dashboard import DashboardAPI
 from reconciler import Reconciler
@@ -140,6 +141,165 @@ def _cdf(r: git_repo.GitRepo) -> commit_annotation.ChangeDeterminatorProtocol:
     return commit_annotation.LocalCommitChangeDeterminator(r)
 
 
+def _ignore_filter_retriever_fixture() -> typing.Callable[
+    [],
+    tuple[dict[str, ElectionProposal], dict[str, ElectionProposal]],
+]:
+    """
+    Build a self-contained retriever returning one GuestOS and one HostOS
+    election proposal with distinct IDs.  This bypasses ``MockDashboard``
+    (whose ``_fake_proposal`` always emits ``hostos_version_to_elect``)
+    so the test exercises both branches of the ignore filter.
+    """
+    guestos_commit = "11" * 20
+    hostos_commit = "22" * 20
+
+    def retriever() -> tuple[
+        dict[str, ElectionProposal], dict[str, ElectionProposal]
+    ]:
+        guestos: dict[str, ElectionProposal] = {
+            guestos_commit: {
+                "id": 200001,
+                "payload": {
+                    "replica_version_to_elect": guestos_commit,
+                    "release_package_sha256_hex": "aa" * 32,
+                },
+                "proposal_timestamp_seconds": 1743789296,
+                "proposer": 61,
+                "status": "REJECTED",
+                "summary": "...stubbed out...",
+                "title": "Elect new IC/GuestOS revision (test fixture)",
+            }
+        }
+        hostos: dict[str, ElectionProposal] = {
+            hostos_commit: {
+                "id": 200002,
+                "payload": {
+                    "hostos_version_to_elect": hostos_commit,
+                    "release_package_sha256_hex": "bb" * 32,
+                },
+                "proposal_timestamp_seconds": 1743789296,
+                "proposer": 61,
+                "status": "REJECTED",
+                "summary": "...stubbed out...",
+                "title": "Elect new IC/HostOS revision (test fixture)",
+            }
+        }
+        return guestos, hostos
+
+    return retriever
+
+
+def test_reconciler_filtered_proposals_retriever_drops_ignored_ids() -> None:
+    """
+    With ``ignored_proposals`` set, the wrapper around the proposal
+    retriever must omit any matching proposal so the corresponding
+    version is treated by ``ReconcilerState`` as not yet proposed.
+    """
+    retriever = _ignore_filter_retriever_fixture()
+    guestos_commit, hostos_commit = "11" * 20, "22" * 20
+    ignored_guestos_id = 200001
+    rs = ReconcilerState()
+
+    wrapped = Reconciler._filtered_proposals_retriever(
+        retriever,
+        source="dashboard",
+        ignore_proposal_ids=[ignored_guestos_id],
+    )
+    guestos, hostos = wrapped()
+
+    assert guestos == {}, guestos
+    assert list(hostos.keys()) == [hostos_commit], hostos
+    assert hostos[hostos_commit]["id"] == 200002
+
+    rs.update_state(wrapped)
+    assert isinstance(
+        rs.version_proposal(guestos_commit, const.GUESTOS),
+        reconciler_state.NoProposal,
+    )
+    assert isinstance(
+        rs.version_proposal(hostos_commit, const.HOSTOS),
+        reconciler_state.SubmittedProposal,
+    )
+
+
+def test_reconciler_filtered_proposals_retriever_is_noop_without_ids() -> None:
+    """Without ``ignored_proposals``, the wrapper returns the data unchanged."""
+    retriever = _ignore_filter_retriever_fixture()
+
+    direct_guestos, direct_hostos = retriever()
+    wrapped_guestos, wrapped_hostos = Reconciler._filtered_proposals_retriever(
+        retriever,
+        source="dashboard",
+        ignore_proposal_ids=[],
+    )()
+
+    assert wrapped_guestos == direct_guestos
+    assert wrapped_hostos == direct_hostos
+
+
+def test_reconciler_picks_up_ignored_proposals_from_release_index(
+    ic_repo: git_repo.GitRepo,
+    mocker: pytest_mock.plugin.MockerFixture,
+) -> None:
+    """
+    ``ignored_proposals`` declared at the top of ``release-index.yaml`` must
+    propagate through the loader into the per-cycle ignore set, so a
+    previously-rejected proposal stops blocking resubmission.
+    """
+    with mocker.patch.object(ic_repo, "push_release_tags"):
+        d, f, n, rs, a, dre, s, rl, p, db = _defaults_for_test()
+        already_blocked_hostos_id = 138817
+        rl = StaticReleaseLoader(
+            pydantic_yaml.to_yaml_str(
+                ReleaseIndexModel.model_validate(
+                    {
+                        "ignored_proposals": [already_blocked_hostos_id],
+                        "releases": [
+                            _release(
+                                "rc--2025-10-02_03-13",
+                                {"base": "45657852c1eca6728ff313808db29b47c862ad13"},
+                            ),
+                            _release(
+                                "rc--2025-09-25_09-52",
+                                {"base": "206b61a8616bc93d36d6a014e5cc8edf1ba256ae"},
+                            ),
+                            _release(
+                                "rc--2025-09-19_10-17",
+                                {"base": "bf0d4d1b8cb6c0c19a5afa1454ada014847aa5c6"},
+                            ),
+                        ],
+                    }
+                )
+            )
+        )
+        reconciler = Reconciler(
+            f, rl, n, p, "", rs, ic_repo, lambda: _cdf(ic_repo), a, dre, db, s
+        )
+
+        def fake_approved_release_notes(*args):  # type: ignore
+            return f"Fake changelog for {args}"
+
+        rl.proposal_summary = fake_approved_release_notes  # type: ignore
+        reconciler.reconcile()
+
+        # The HostOS dashboard fixture pins id 138817 to both top releases.
+        # After reconcile, those versions must have been seen as "not yet
+        # proposed" — i.e. the reconciler issued a fresh proposal for them
+        # (dryrun.DRECli returns a synthetic, version-derived ID), so the
+        # state for them no longer points at the ignored id.
+        for commit in (
+            "45657852c1eca6728ff313808db29b47c862ad13",
+            "206b61a8616bc93d36d6a014e5cc8edf1ba256ae",
+        ):
+            prop = rs.version_proposal(commit, const.HOSTOS)
+            assert isinstance(prop, reconciler_state.SubmittedProposal), prop
+            assert prop.proposal_id != already_blocked_hostos_id, (
+                f"Expected a fresh proposal id for HostOS commit {commit}, "
+                f"got the previously-blocked id {already_blocked_hostos_id}."
+            )
+
+
 def test_reconciler_reconciles_without_error_already_submitted_proposals(
     ic_repo: git_repo.GitRepo,
     mocker: pytest_mock.plugin.MockerFixture,
diff --git a/release-index-schema.json b/release-index-schema.json
@@ -6,6 +6,13 @@
       "type": "object",
       "additionalProperties": false,
       "properties": {
+        "ignored_proposals": {
+          "type": "array",
+          "items": {
+            "type": "integer"
+          },
+          "description": "NNS election proposal IDs the reconciler must treat as if they had never been submitted, so it submits a fresh proposal for the underlying version on the next cycle. Intended to recover from rejected/failed proposals."
+        },
         "releases": {
           "type": "array",
           "items": {
diff --git a/release-index.yaml b/release-index.yaml
@@ -7,6 +7,15 @@
 #       HostOS or GuestOS:
 #         rc_name: <reference RC name>
 #         name: <version name>
+# - Use 'ignored_proposals' (top-level) to force the reconciler to forget
+#   specific NNS election proposals when it builds its in-memory state, so
+#   the affected versions go back to the "no proposal" state and a fresh
+#   proposal is submitted on the next cycle.  Intended to recover from
+#   rejected / failed proposals.  Remove the entry once the replacement
+#   proposal has been submitted; the IC governance canister will still
+#   refuse to re-elect an already blessed version.
+ignored_proposals:
+  - 141776 # GuestOS election proposal rejected; resubmit the version.
 releases:
   - rc_name: rc--2026-05-15_10-22
     versions:
