tests/rptest: exercise the fetch_controller_snapshot_rpc gate#31063
tests/rptest: exercise the fetch_controller_snapshot_rpc gate#31063dotnwat wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds explicit perturbation coverage for the fetch_controller_snapshot_rpc feature gate during the unfinalized v26.1 → v26.2 upgrade path, ensuring a restarting broker does not perform the bootstrap controller-snapshot fetch until after finalization.
Changes:
- Move
fetch_controller_snapshot_rpcfrom acknowledged to exercised feature coverage. - Add
_exercise_fetch_controller_snapshot_rpc()to validate the gate remains closed during the unfinalized window using a restart-required config change and a single-node restart while the controller leader remains reachable. - Add
_verify_fetch_controller_snapshot_rpc_working()to confirm the feature activates after finalization.
| # survives, so the change commits and a controller leader stays up. | ||
| self.redpanda.stop_node(node) | ||
| try: | ||
| prev = self.admin.get_cluster_config_status()[0]["config_version"] |
There was a problem hiding this comment.
Good catch that this guard wasn't doing its job — though the root cause is a bit different from a stale [0]. config_frontend::patch unconditionally writes a new delta and returns an incremented version, so new_version > prev is effectively always true and cannot detect a value no-op even with a max() baseline (new_version = V+1 > max_applied = V). In this test the cleanup removes the property every cycle, so NEW_VALUE is always a real change; and if it somehow weren't, the restart is True assertion below would fail anyway. So I've dropped the ineffective guard (keeping new_version, which the catch-up wait_until still needs) rather than switching to max().
| try: | ||
| self.admin.patch_cluster_config(remove=[PROPERTY]) | ||
| except Exception as e: | ||
| self.logger.warn(f"cleanup: failed to reset {PROPERTY}: {e}") |
There was a problem hiding this comment.
Done — switched to logger.warning.
dotnwat
force-pushed
the
noah/exercise-fetch-controller-snapshot-gate
branch
from
964cfb8 to
f8480f2
Compare
It was previously acknowledged as needing no perturbation coverage, on the rationale that a gated broker's bootstrap is indistinguishable from prior behavior. It is distinguishable: exercise the gate in the unfinalized state by bringing one broker down, changing a restart-requiring property it misses, then restarting only that broker with the controller leader still up. While gated the broker boots on its stale local cache, reports restart-required, and attempts no controller-snapshot fetch -- exactly the pre-upgrade behavior, which is what keeps the unfinalized state downgrade-safe. The whole-cluster restart in _perturb_common cannot exercise this: with every broker down there is no leader to fetch from and no config delta to reconcile. Also verify the gate opens post-finalize; the active-state fetch mechanics are covered by ClusterConfigMultiNodeBootstrapTest.test_node_delayed_restart. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dotnwat
force-pushed
the
noah/exercise-fetch-controller-snapshot-gate
branch
from
f8480f2 to
0464bff
Compare
Retry command for Build#86943please wait until all jobs are finished before running the slash command |
What
fetch_controller_snapshot_rpc(v26.2) was the one v26.2-gated feature inManualFinalizationUpgradeTestmarked as needing no perturbation coverage, onthe rationale that a gated broker's bootstrap is "indistinguishable from prior
behavior, with no distinctive signal to assert."
Re-examining the feature (added in #30396, "the great config bootstrap fix"):
the gate is the client-side
is_active()check inapplication::bootstrap_controller_view. While the upgrade is unfinalized theactive version is held below the feature's
require_version(v26_2_1), so thegate is provably closed and a restarting broker falls through to its stale local
config cache — writing no new state, which is what keeps the unfinalized window
downgrade-safe. That behavior is distinguishable, and there is a signal: the
info-level
Fetching controller snapshot fromlog, and the broker'srestart-required status.
Changes
fetch_controller_snapshot_rpcfromPERTURB_ACKNOWLEDGED_FEATURES→PERTURB_EXERCISED_FEATURES._exercise_fetch_controller_snapshot_rpc(unfinalized): bring one brokerdown, change a restart-requiring, non-feature-gated property
(
kafka_qdc_idle_depth) it will miss, then restart only that broker with thecontroller leader still up. Assert the gated outcome — no bootstrap fetch
attempted, and the broker boots on the stale value still reporting
restart-required. Keeping the leader reachable is what makes this a gate
test: a wrongly-open gate would fetch the fresh value and flip restart to
False.
_verify_fetch_controller_snapshot_rpc_working(post-finalize): confirm thegate opens (feature auto-activates). The active-state fetch mechanics are
covered by
ClusterConfigMultiNodeBootstrapTest.test_node_delayed_restart.Note: the whole-cluster in-place restart already in
_perturb_commoncannotexercise this — restarting every broker at once leaves no leader to fetch from
and changes no config, so it can't tell the gated path from the active one.
Testing
RP_GIT_RELEASED_VERSIONS="v26.1.10:v26.1.9" tools/dt run --stamp tests/rptest/tests/cluster_features_test.py::ManualFinalizationUpgradeTest.test_downgrade_before_finalizerestart is Trueassertion fails cleanly withrestart-required was True, confirming the gated broker really does boot onstale config and that the check has teeth (and that the no-fetch log assertion
passed right before it).
🤖 Generated with Claude Code