Skip to content

Commit f8480f2

Browse files
dotnwatclaude
andcommitted
tests/rptest: exercise the fetch_controller_snapshot_rpc gate
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>
1 parent 7299281 commit f8480f2

1 file changed

Lines changed: 133 additions & 7 deletions

File tree

tests/rptest/tests/cluster_features_test.py

Lines changed: 133 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,7 +1209,13 @@ def test_get_upgrade_status_reports_in_progress(self):
12091209
# exercise or an explicit acknowledgement. Add a feature's name here when you
12101210
# cover it or knowingly skip it; entries for features no longer gated by the
12111211
# current upgrade are harmless extras, so no per-major pruning is needed.
1212-
PERTURB_EXERCISED_FEATURES = frozenset({"tiered_cloud_topics", "shadow_link_role_sync"})
1212+
PERTURB_EXERCISED_FEATURES = frozenset(
1213+
{
1214+
"tiered_cloud_topics",
1215+
"shadow_link_role_sync",
1216+
"fetch_controller_snapshot_rpc",
1217+
}
1218+
)
12131219
PERTURB_ACKNOWLEDGED_FEATURES = frozenset(
12141220
{
12151221
# Cluster-linking features, out of scope for this single-cluster test.
@@ -1218,12 +1224,6 @@ def test_get_upgrade_status_reports_in_progress(self):
12181224
# Iceberg extended-mode topic-config gate; exercising it needs Iceberg
12191225
# topic setup orthogonal to the finalization behavior under test.
12201226
"iceberg_extended_mode_config",
1221-
# Bootstrap-time internal RPC: while gated, a restarting node simply
1222-
# skips the controller-snapshot fetch and falls back to its local
1223-
# config cache (indistinguishable from prior behavior, with no
1224-
# distinctive signal to assert here). Its active-state
1225-
# behavior is covered by ClusterConfigMultiNodeBootstrapTest.
1226-
"fetch_controller_snapshot_rpc",
12271227
}
12281228
)
12291229

@@ -1455,6 +1455,7 @@ def _perturb_v26_1_to_v26_2(self, phase):
14551455
return
14561456
self._exercise_tiered_cloud_topics()
14571457
self._exercise_shadow_link_role_sync()
1458+
self._exercise_fetch_controller_snapshot_rpc()
14581459
# The other two v26.2-gated features are not exercised by this
14591460
# single-cluster perturbation: shadow_link_sr_api_sync and
14601461
# batch_mirror_topic_status are both cluster-linking features
@@ -1548,6 +1549,115 @@ def _exercise_shadow_link_role_sync(self):
15481549
"configuring role sync should be gated while unfinalized"
15491550
)
15501551

1552+
def _exercise_fetch_controller_snapshot_rpc(self):
1553+
"""Sanity-check that the fetch_controller_snapshot_rpc gate keeps an
1554+
unfinalized upgrade downgrade-safe.
1555+
1556+
This feature is safe because its only new behavior -- a restarting
1557+
broker fetching a fresh controller snapshot at bootstrap (from the "great
1558+
config bootstrap fix") -- sits behind a single client-side is_active()
1559+
check in application::bootstrap_controller_view. While the upgrade is
1560+
unfinalized the active version is held below the feature's
1561+
require_version (v26_2_1), so that gate is provably closed: the feature
1562+
never even becomes available, let alone active. With the gate closed
1563+
bootstrap_controller_view returns immediately -- it issues no RPC and,
1564+
crucially, writes no new state (no config-cache write, no feature-table
1565+
snapshot applied, no wait-for-offset). The broker boots exactly as the
1566+
pre-upgrade binary did, from its local config cache, so rolling back to
1567+
the old binary finds nothing on disk it cannot read. Finalization is the
1568+
one-way door that opens the gate; until it is crossed the new path stays
1569+
inert and leaves no trace, which is precisely what makes the downgrade
1570+
safe.
1571+
1572+
This test is a sanity check on that safety property -- not a proof of the
1573+
C++ gate logic, but a check that the *observable* behavior matches it. It
1574+
simulates the case the feature actually targets, not just any restart:
1575+
bring one broker down, change a restart-requiring, non-feature-gated
1576+
property it will miss, then restart only that broker with the other two
1577+
(and therefore the controller leader) still up. Keeping the leader
1578+
reachable is what makes this a gate test rather than a
1579+
leader-reachability test: were the gate wrongly open -- e.g. a future
1580+
change activating the feature before finalization, or moving the fetch
1581+
ahead of the is_active() check -- the broker would fetch the missed value
1582+
before starting services and come back needing no restart (as it does
1583+
once finalized -- see
1584+
ClusterConfigMultiNodeBootstrapTest.test_node_delayed_restart). Gated, it
1585+
must instead boot on the stale value and still report restart-required,
1586+
having attempted no fetch.
1587+
1588+
NOTE: the whole-cluster in-place restart in _perturb_common does not
1589+
exercise this. Restarting every broker at once leaves no controller
1590+
leader to fetch from and changes no config, so it cannot tell the gated
1591+
path from the active one."""
1592+
assert self._feature_state("fetch_controller_snapshot_rpc") == "unavailable", (
1593+
"fetch_controller_snapshot_rpc should be unavailable while unfinalized"
1594+
)
1595+
1596+
PROPERTY = "kafka_qdc_idle_depth" # restart-requiring, not feature-gated
1597+
NEW_VALUE = 77
1598+
node = self.redpanda.nodes[-1]
1599+
node_id = self.redpanda.node_id(node)
1600+
1601+
# Down one broker, then change a property it will miss. Quorum (2/3)
1602+
# survives, so the change commits and a controller leader stays up.
1603+
self.redpanda.stop_node(node)
1604+
try:
1605+
prev = self.admin.get_cluster_config_status()[0]["config_version"]
1606+
new_version = self.admin.patch_cluster_config(upsert={PROPERTY: NEW_VALUE})[
1607+
"config_version"
1608+
]
1609+
# Guard against a silent no-op that would make the check vacuous.
1610+
assert new_version > prev, (
1611+
f"patching {PROPERTY} did not advance the config version "
1612+
f"({prev} -> {new_version}); is it already {NEW_VALUE}?"
1613+
)
1614+
1615+
# Restart the broker on the same (HEAD) binary and let it rejoin.
1616+
self.redpanda.start_node(node)
1617+
self._wait_for_cluster_settled()
1618+
1619+
def restarted_node_status():
1620+
for s in self.admin.get_cluster_config_status():
1621+
if s["node_id"] == node_id:
1622+
return s
1623+
return None
1624+
1625+
# Wait until the broker has replayed the controller log up to the
1626+
# change, so its restart-required verdict is final before we assert.
1627+
def caught_up():
1628+
st = restarted_node_status()
1629+
return st is not None and st["config_version"] >= new_version
1630+
1631+
wait_until(
1632+
caught_up,
1633+
timeout_sec=60,
1634+
backoff_sec=1,
1635+
err_msg="restarted broker did not catch up to the new config version",
1636+
)
1637+
1638+
# Gated: no bootstrap fetch was attempted. This info-level line is
1639+
# logged only past the feature gate; every pre-finalize boot in this
1640+
# test is gated, so any match on this node is a real regression.
1641+
assert not self.redpanda.search_log_node(
1642+
node, "Fetching controller snapshot from"
1643+
), "a gated broker must not attempt the bootstrap controller-snapshot fetch"
1644+
1645+
# Gated: the broker bootstrapped on its stale local cache, learned of
1646+
# the change only via post-bootstrap log replay, and -- since we no
1647+
# longer promote pending needs_restart values at start -- still
1648+
# reports restart-required. restart=False here would mean the
1649+
# bootstrap fetch ran while the upgrade was unfinalized.
1650+
assert restarted_node_status()["restart"] is True, (
1651+
"a gated broker must bootstrap on its stale local cache and "
1652+
"report restart-required for the missed change"
1653+
)
1654+
finally:
1655+
# Leave no config override behind for later phases or the downgrade.
1656+
try:
1657+
self.admin.patch_cluster_config(remove=[PROPERTY])
1658+
except Exception as e:
1659+
self.logger.warn(f"cleanup: failed to reset {PROPERTY}: {e}")
1660+
15511661
def _verify_tiered_cloud_topics_working(self):
15521662
"""After finalize the active version has advanced past the feature's
15531663
require_version, so the gate opens: the feature moves from unavailable to
@@ -1593,11 +1703,27 @@ def _verify_shadow_link_role_sync_working(self):
15931703
f"role-sync config still gated after finalize: {e}"
15941704
)
15951705

1706+
def _verify_fetch_controller_snapshot_rpc_working(self):
1707+
"""After finalize the active version advances past the feature's
1708+
require_version and it auto-activates (available_policy::always), opening
1709+
the bootstrap-fetch gate. Confirm the state transition; the active-state
1710+
fetch mechanics (a delayed-restart broker picking up fresh config before
1711+
starting services, the inverse of the gated outcome asserted in
1712+
_exercise_fetch_controller_snapshot_rpc) are covered end to end by
1713+
ClusterConfigMultiNodeBootstrapTest.test_node_delayed_restart."""
1714+
wait_until(
1715+
lambda: self._feature_state("fetch_controller_snapshot_rpc") == "active",
1716+
timeout_sec=30,
1717+
backoff_sec=1,
1718+
err_msg="fetch_controller_snapshot_rpc did not activate after finalize",
1719+
)
1720+
15961721
def _verify_v26_2_features_working(self):
15971722
"""After the upgrade is finalized, confirm each v26.2 feature's gate has
15981723
opened and the feature actually works (simple per-feature predicates)."""
15991724
self._verify_tiered_cloud_topics_working()
16001725
self._verify_shadow_link_role_sync_working()
1726+
self._verify_fetch_controller_snapshot_rpc_working()
16011727

16021728
def _feature_state(self, name):
16031729
for f in self.admin.get_features()["features"]:

0 commit comments

Comments
 (0)