admin/broker: restore build_info in ListBrokers response by pgellert · Pull Request #29785 · redpanda-data/redpanda

pgellert · 2026-03-10T16:43:54Z

bf9fe02 earlier removed build_info from
ListBrokers responses to avoid requiring all brokers to be reachable.
However, this broke rpk cluster connections list, which checks
build_info to validate version compatibility.

Populate build_info for all brokers in ListBrokers using the responding
broker's version. This fixes rpk cluster connections list which checks
build_info to validate version compatibility.

Fixes https://redpandadata.atlassian.net/browse/CORE-15134

Backports Required

Release Notes

none

tests/rptest/tests/admin_api_auth_test.py

Copilot

Pull request overview

Restores build_info population in AdminV2 ListBrokers responses (to unblock rpk cluster connections list version compatibility checks) while keeping the endpoint resilient by returning partial broker entries when some nodes are unreachable.

Changes:

Update broker admin service to fetch broker info from other nodes in parallel via best-effort GetBroker RPCs.
Add rptest coverage for rpk cluster connections list and for ListBrokers behavior when a broker is down.
Update proto / type-stub comments to document that build_info / admin_server may be absent in ListBrokers if unreachable.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/v/redpanda/admin/services/broker.cc`	Parallelizes best-effort broker info fetching to repopulate `build_info` in `ListBrokers`.
`proto/redpanda/core/admin/v2/broker.proto`	Documents new `ListBrokers` field presence expectations for unreachable brokers.
`tests/rptest/clients/admin/proto/redpanda/core/admin/v2/broker_pb2.pyi`	Mirrors proto comment changes in Python typing stubs.
`tests/rptest/tests/list_kafka_connections_test.py`	Adds an rptest that exercises the `rpk cluster connections list` compatibility path.
`tests/rptest/tests/admin_api_auth_test.py`	Adds resilience test ensuring `ListBrokers` returns all brokers even if one is down.

src/v/redpanda/admin/services/broker.cc

tests/rptest/tests/admin_api_auth_test.py

rockwotj · 2026-03-10T16:50:56Z

proto/redpanda/core/admin/v2/broker.proto

-    // Only populated for `GetBroker` RPCs
+    // May be absent in ListBrokers responses if the broker was unreachable.
    optional BuildInfo build_info = 2;
    // The admin server information.
    //
-    // Only populated for `GetBroker` RPCs
+    // May be absent in ListBrokers responses if the broker was unreachable.


I really don't like this being non-deterministic 😄

Can we leave documented it's not populated for GetBroker RPCs, and then have rpk call GetBroker for future versions?

Fair enough. The reason why I didn't go with that is that then this will still fail with 25.3 rpk versions.

No I'm saying let's populate it for backwards compat (or maybe just use the called broker's version and assume there is no partial rollout?), but just document it as removed so we can maybe remove it down the road.

Makes sense. I've adjusted the rpk implementation now so that we can make the broker-side removal later on. And I think assuming no partial rollout seems reasonable since the only user should be rpk's version check.

Populate build_info for all brokers in ListBrokers using the responding broker's version. This fixes rpk cluster connections list which checks build_info to validate version compatibility.

Verifies that rpk cluster connections list works correctly, exercising the code path where rpk calls ListBrokers to check version compatibility.

Use GetBroker instead of ListBrokers for the version compatibility check. This avoids relying on build_info being populated in ListBrokers responses, allowing that field to be removed in the future.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Copilot · 2026-03-10T17:49:30Z

src/v/redpanda/admin/services/broker.cc

+        // compatibility check.
+        broker.set_build_info(std::move(self_broker().get_build_info()));


ListBrokers is populating build_info for other brokers by copying the full self_broker().build_info (including build_sha). That makes the response report an incorrect build_sha (and any other future fields) for non-responding brokers. If the intent is only to preserve version compatibility checks, consider setting only the version field (and leaving build_sha unset/empty) for these synthetic broker entries, or omit build_sha explicitly.

Suggested change

// compatibility check.

broker.set_build_info(std::move(self_broker().get_build_info()));

// compatibility check. Do not set build_sha here, because we do not

// know the exact build of non-responding brokers.

proto::admin::build_info build_info;

build_info.set_version(ss::sstring(redpanda_git_version()));

broker.set_build_info(std::move(build_info));

Copilot · 2026-03-10T17:49:30Z

src/v/redpanda/admin/services/broker.cc

    for (auto& [node_id, _] : clients) {
        auto& broker = list_resp.get_brokers().emplace_back();
        broker.set_node_id(node_id());
+
+        // Populate build_info for all brokers with this broker's version for
+        // backwards compatibility with rpk's cluster connections list version
+        // compatibility check.
+        broker.set_build_info(std::move(self_broker().get_build_info()));
    }


self_broker() builds and populates admin_server routes; calling it inside the loop means you rebuild that route list once per broker just to fetch build_info. This is avoidable overhead on every ListBrokers call; compute the responding broker's build info once outside the loop (or construct it directly) and reuse it for each synthetic broker entry.

Copilot · 2026-03-10T17:49:31Z

tests/rptest/tests/list_kafka_connections_test.py

+        """
+        Tests that rpk cluster connections list works correctly.
+
+        This specifically exercises the code path where rpk calls ListBrokers


The test docstring mentions that rpk calls ListBrokers for version compatibility, but in this PR rpk cluster connections list now checks compatibility via GetBroker instead. Please update the comment to match the actual code path being exercised so the test remains accurate over time.

Suggested change

This specifically exercises the code path where rpk calls ListBrokers

This specifically exercises the code path where rpk calls GetBroker

pgellert · 2026-03-10T17:49:46Z

force-push: use hardcoded version instead and adjust rpk implementation for 26.1 based on feedback

nguyen-andrew · 2026-03-10T17:56:12Z

src/go/rpk/pkg/cli/cluster/connections/list.go

-	for _, broker := range brokers.Msg.Brokers {
-		if broker.BuildInfo == nil {
-			return false
-		}
-		version, err := redpanda.VersionFromString(broker.BuildInfo.Version)
-		if err != nil || !version.IsAtLeast(minVersion) {
-			return false
-		}


This seemed to check that all brokers had a version greater than minVersion to support the feature, but the new code only seems to check that a single broker version meets the min version. Is that okay? And which broker is checked (is it determined by the client parameter)?

Or is there somewhere else in the code that calls isFeatureSupported with a client for each broker to ensure that all brokers support it? Does list connections requires all brokers in the cluster to support it or can it work on some brokers in a cluster but not others (i.e. partial upgrades)?

This seemed to check that all brokers had a version greater than minVersion to support the feature, but the new code only seems to check that a single broker version meets the min version. Is that okay?

Yes, this should be fine since this is purely for better UX (showing a friendly "feature not supported" message instead of an API error).

And which broker is checked (is it determined by the client parameter)?

Whichever broker the admin client connects to, which is, I believe, is randomly chosen from the set of configured brokers.

Or is there somewhere else in the code that calls isFeatureSupported with a client for each broker to ensure that all brokers support it?

No.

Does list connections requires all brokers in the cluster to support it or can it work on some brokers in a cluster but not others (i.e. partial upgrades)?

Yes, the ListKafkaConnections API must be available on all brokers. If rpk connects to a supported broker but then queries an unsupported one, the user would see a different error (an API error rather than our friendly "feature not supported" message). Given the narrow scenario where this would matter (mid-upgrade, pre-25.3 brokers, post-26.1 rpk), and the minor impact (just a less friendly error message), this seems like a reasonable trade-off to simplify the admin API.

Got it, thanks for the in-depth context!

vbotbuildovich · 2026-03-10T20:48:41Z

CI test results

test results on build#81555

test_class	test_method	test_arguments	test_kind	job_url	test_status	passed	reason	test_history
DatalakeClusterRestoreTest	test_restore_partition_spec	{"catalog_type": "rest_jdbc", "cloud_storage_type": 1}	integration	https://buildkite.com/redpanda/redpanda/builds/81555#019cd8ed-bad9-4857-8072-64b5df6141c3	FLAKY	10/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0012, p0=1.0000, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3487, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=DatalakeClusterRestoreTest&test_method=test_restore_partition_spec
RedpandaNodeOperationsSmokeTest	test_node_ops_smoke_test	{"cloud_storage_type": 1, "mixed_versions": false}	integration	https://buildkite.com/redpanda/redpanda/builds/81555#019cd8ed-22c9-40aa-88c9-7515a4b7ce71	FLAKY	36/41	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0477, p0=0.1218, reject_threshold=0.0100. adj_baseline=0.1363, p1=0.3480, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=RedpandaNodeOperationsSmokeTest&test_method=test_node_ops_smoke_test

pgellert · 2026-03-11T11:02:17Z

@graham-rp @r-vasquez When you have a moment, could you take a look at the rpk changes here? It's a small adjustment to use GetBroker instead of ListBrokers for the version check.

rockwotj · 2026-03-11T14:38:09Z

src/go/rpk/pkg/cli/cluster/connections/list.go

 	// Determine whether the broker version supports client connection monitoring
-	brokers, err := client.BrokerService().ListBrokers(ctx, &connect.Request[adminv2.ListBrokersRequest]{})
-	if err != nil || len(brokers.Msg.Brokers) == 0 {
+	resp, err := client.BrokerService().GetBroker(ctx, &connect.Request[adminv2.GetBrokerRequest]{})


Ah for 25.3 this needs to be an explicit -1 node ID otherwise it will try to find node 0 which may or may not be available.

pgellert self-assigned this Mar 10, 2026

pgellert requested review from a team and rockwotj as code owners March 10, 2026 16:43

pgellert requested review from Copilot and nguyen-andrew and removed request for a team March 10, 2026 16:43

github-actions bot added the area/redpanda label Mar 10, 2026

Copilot started reviewing on behalf of pgellert March 10, 2026 16:44 View session

pgellert commented Mar 10, 2026

View reviewed changes

tests/rptest/tests/admin_api_auth_test.py Outdated Show resolved Hide resolved

Copilot AI reviewed Mar 10, 2026

View reviewed changes

src/v/redpanda/admin/services/broker.cc Outdated Show resolved Hide resolved

tests/rptest/tests/admin_api_auth_test.py Outdated Show resolved Hide resolved

rockwotj reviewed Mar 10, 2026

View reviewed changes

admin/broker: populate build_info in ListBrokers for compat

79f6184

Populate build_info for all brokers in ListBrokers using the responding broker's version. This fixes rpk cluster connections list which checks build_info to validate version compatibility.

pgellert force-pushed the fix/conn-list-version-check branch from edf11c4 to 0dbc49a Compare March 10, 2026 17:42

pgellert requested review from a team, kbatuigas and r-vasquez as code owners March 10, 2026 17:42

github-actions bot added the area/rpk label Mar 10, 2026

pgellert requested review from Copilot and graham-rp March 10, 2026 17:43

Copilot started reviewing on behalf of pgellert March 10, 2026 17:46 View session

pgellert added 2 commits March 10, 2026 17:47

test: add rpk cluster connections list regression test

d67bf0c

Verifies that rpk cluster connections list works correctly, exercising the code path where rpk calls ListBrokers to check version compatibility.

rpk/connections: use GetBroker for version check

d4c98c4

Use GetBroker instead of ListBrokers for the version compatibility check. This avoids relying on build_info being populated in ListBrokers responses, allowing that field to be removed in the future.

pgellert force-pushed the fix/conn-list-version-check branch from 0dbc49a to d4c98c4 Compare March 10, 2026 17:48

Copilot AI reviewed Mar 10, 2026

View reviewed changes

pgellert requested a review from rockwotj March 10, 2026 17:51

nguyen-andrew reviewed Mar 10, 2026

View reviewed changes

rockwotj approved these changes Mar 10, 2026

View reviewed changes

pgellert requested a review from nguyen-andrew March 11, 2026 10:59

nguyen-andrew approved these changes Mar 11, 2026

View reviewed changes

rockwotj reviewed Mar 11, 2026

View reviewed changes

		// compatibility check.
		broker.set_build_info(std::move(self_broker().get_build_info()));

-        // compatibility check.
-        broker.set_build_info(std::move(self_broker().get_build_info()));
+        // compatibility check. Do not set build_sha here, because we do not
+        // know the exact build of non-responding brokers.
+        proto::admin::build_info build_info;
+        build_info.set_version(ss::sstring(redpanda_git_version()));
+        broker.set_build_info(std::move(build_info));

	This specifically exercises the code path where rpk calls ListBrokers
	This specifically exercises the code path where rpk calls GetBroker

Conversation

pgellert commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backports Required

Release Notes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

pgellert commented Mar 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbotbuildovich commented Mar 10, 2026

CI test results

Uh oh!

pgellert commented Mar 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pgellert commented Mar 10, 2026 •

edited

Loading