diff --git a/proto/redpanda/core/admin/v2/broker.proto b/proto/redpanda/core/admin/v2/broker.proto index 603a7381e0046..f51f1fc3003d3 100644 --- a/proto/redpanda/core/admin/v2/broker.proto +++ b/proto/redpanda/core/admin/v2/broker.proto @@ -44,9 +44,9 @@ service BrokerService { // GetBrokerRequest returns information about a single broker in the cluster message GetBrokerRequest { - // The node ID for the broker. If set to -1, the broker handling the RPC + // The node ID for the broker. If unset, the broker handling the RPC // request returns information about itself. - int32 node_id = 1; + optional int32 node_id = 1; } // GetBrokerResponse is the response from the GetBroker RPC. @@ -69,9 +69,13 @@ message Broker { // This broker's node ID. int32 node_id = 1; // The build this broker is running. - BuildInfo build_info = 2; + // + // Only populated for `GetBroker` RPCs + optional BuildInfo build_info = 2; // The admin server information. - AdminServer admin_server = 3; + // + // Only populated for `GetBroker` RPCs + optional AdminServer admin_server = 3; } // BuildInfo contains information about the Redpanda build. diff --git a/src/v/redpanda/admin/services/broker.cc b/src/v/redpanda/admin/services/broker.cc index e8107edd02066..7277aaa70399b 100644 --- a/src/v/redpanda/admin/services/broker.cc +++ b/src/v/redpanda/admin/services/broker.cc @@ -37,11 +37,13 @@ broker_service_impl::broker_service_impl( ss::future broker_service_impl::get_broker( serde::pb::rpc::context ctx, proto::admin::get_broker_request req) { - auto target = model::node_id(req.get_node_id()); - if (target != -1 && target != _proxy_client.self_node_id()) { - co_return co_await _proxy_client - .make_client_for_node(target) - .get_broker(ctx, std::move(req)); + if (req.has_node_id()) { + auto target = model::node_id(req.get_node_id()); + if (target != -1 && target != _proxy_client.self_node_id()) { + co_return co_await _proxy_client + .make_client_for_node(target) + .get_broker(ctx, std::move(req)); + } } proto::admin::get_broker_response resp; resp.set_broker(self_broker()); @@ -56,11 +58,9 @@ broker_service_impl::list_brokers( auto clients = _proxy_client .make_clients_for_other_nodes(); - for (auto& [node_id, client] : clients) { - proto::admin::get_broker_request req; - req.set_node_id(node_id); - auto get_resp = co_await client.get_broker(ctx, std::move(req)); - list_resp.get_brokers().push_back(std::move(get_resp.get_broker())); + for (auto& [node_id, _] : clients) { + auto& broker = list_resp.get_brokers().emplace_back(); + broker.set_node_id(node_id()); } co_return list_resp; } @@ -68,17 +68,21 @@ broker_service_impl::list_brokers( proto::admin::broker broker_service_impl::self_broker() const { proto::admin::broker b; b.set_node_id(_proxy_client.self_node_id()); - b.get_build_info().set_version(ss::sstring(redpanda_git_version())); - b.get_build_info().set_build_sha(ss::sstring(redpanda_git_revision())); + proto::admin::build_info build_info; + build_info.set_version(ss::sstring(redpanda_git_version())); + build_info.set_build_sha(ss::sstring(redpanda_git_revision())); + b.set_build_info(std::move(build_info)); + proto::admin::admin_server admin_server_info; for (auto& service : *_services) { for (auto& route : service->all_routes()) { proto::rpc_route r; r.set_name( fmt::format("{}.{}", route.service_name, route.method_name)); r.set_http_route(ss::sstring(route.path)); - b.get_admin_server().get_routes().push_back(std::move(r)); + admin_server_info.get_routes().push_back(std::move(r)); } } + b.set_admin_server(std::move(admin_server_info)); return b; } diff --git a/tests/rptest/clients/admin/proto/redpanda/core/admin/v2/broker_pb2.py b/tests/rptest/clients/admin/proto/redpanda/core/admin/v2/broker_pb2.py index f5bb794ce4757..361ad09b9aa4a 100644 --- a/tests/rptest/clients/admin/proto/redpanda/core/admin/v2/broker_pb2.py +++ b/tests/rptest/clients/admin/proto/redpanda/core/admin/v2/broker_pb2.py @@ -8,7 +8,7 @@ _sym_db = _symbol_database.Default() from ......proto.redpanda.core.pbgen import options_pb2 as proto_dot_redpanda_dot_core_dot_pbgen_dot_options__pb2 from ......proto.redpanda.core.pbgen import rpc_pb2 as proto_dot_redpanda_dot_core_dot_pbgen_dot_rpc__pb2 -DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n)proto/redpanda/core/admin/v2/broker.proto\x12\x16redpanda.core.admin.v2\x1a\'proto/redpanda/core/pbgen/options.proto\x1a#proto/redpanda/core/pbgen/rpc.proto"#\n\x10GetBrokerRequest\x12\x0f\n\x07node_id\x18\x01 \x01(\x05"C\n\x11GetBrokerResponse\x12.\n\x06broker\x18\x01 \x01(\x0b2\x1e.redpanda.core.admin.v2.Broker"\x14\n\x12ListBrokersRequest"F\n\x13ListBrokersResponse\x12/\n\x07brokers\x18\x01 \x03(\x0b2\x1e.redpanda.core.admin.v2.Broker"\x8b\x01\n\x06Broker\x12\x0f\n\x07node_id\x18\x01 \x01(\x05\x125\n\nbuild_info\x18\x02 \x01(\x0b2!.redpanda.core.admin.v2.BuildInfo\x129\n\x0cadmin_server\x18\x03 \x01(\x0b2#.redpanda.core.admin.v2.AdminServer"/\n\tBuildInfo\x12\x0f\n\x07version\x18\x01 \x01(\t\x12\x11\n\tbuild_sha\x18\x02 \x01(\t"?\n\x0bAdminServer\x120\n\x06routes\x18\x01 \x03(\x0b2 .redpanda.core.admin.v2.RPCRoute",\n\x08RPCRoute\x12\x0c\n\x04name\x18\x01 \x01(\t\x12\x12\n\nhttp_route\x18\x02 \x01(\t2\xe9\x01\n\rBrokerService\x12h\n\tGetBroker\x12(.redpanda.core.admin.v2.GetBrokerRequest\x1a).redpanda.core.admin.v2.GetBrokerResponse"\x06\xea\x92\x19\x02\x10\x03\x12n\n\x0bListBrokers\x12*.redpanda.core.admin.v2.ListBrokersRequest\x1a+.redpanda.core.admin.v2.ListBrokersResponse"\x06\xea\x92\x19\x02\x10\x03B\x10\xea\x92\x19\x0cproto::adminb\x06proto3') +DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n)proto/redpanda/core/admin/v2/broker.proto\x12\x16redpanda.core.admin.v2\x1a\'proto/redpanda/core/pbgen/options.proto\x1a#proto/redpanda/core/pbgen/rpc.proto"4\n\x10GetBrokerRequest\x12\x14\n\x07node_id\x18\x01 \x01(\x05H\x00\x88\x01\x01B\n\n\x08_node_id"C\n\x11GetBrokerResponse\x12.\n\x06broker\x18\x01 \x01(\x0b2\x1e.redpanda.core.admin.v2.Broker"\x14\n\x12ListBrokersRequest"F\n\x13ListBrokersResponse\x12/\n\x07brokers\x18\x01 \x03(\x0b2\x1e.redpanda.core.admin.v2.Broker"\xb5\x01\n\x06Broker\x12\x0f\n\x07node_id\x18\x01 \x01(\x05\x12:\n\nbuild_info\x18\x02 \x01(\x0b2!.redpanda.core.admin.v2.BuildInfoH\x00\x88\x01\x01\x12>\n\x0cadmin_server\x18\x03 \x01(\x0b2#.redpanda.core.admin.v2.AdminServerH\x01\x88\x01\x01B\r\n\x0b_build_infoB\x0f\n\r_admin_server"/\n\tBuildInfo\x12\x0f\n\x07version\x18\x01 \x01(\t\x12\x11\n\tbuild_sha\x18\x02 \x01(\t"?\n\x0bAdminServer\x120\n\x06routes\x18\x01 \x03(\x0b2 .redpanda.core.admin.v2.RPCRoute",\n\x08RPCRoute\x12\x0c\n\x04name\x18\x01 \x01(\t\x12\x12\n\nhttp_route\x18\x02 \x01(\t2\xe9\x01\n\rBrokerService\x12h\n\tGetBroker\x12(.redpanda.core.admin.v2.GetBrokerRequest\x1a).redpanda.core.admin.v2.GetBrokerResponse"\x06\xea\x92\x19\x02\x10\x03\x12n\n\x0bListBrokers\x12*.redpanda.core.admin.v2.ListBrokersRequest\x1a+.redpanda.core.admin.v2.ListBrokersResponse"\x06\xea\x92\x19\x02\x10\x03B\x10\xea\x92\x19\x0cproto::adminb\x06proto3') _globals = globals() _builder.BuildMessageAndEnumDescriptors(DESCRIPTOR, _globals) _builder.BuildTopDescriptorsAndMessages(DESCRIPTOR, 'proto.redpanda.core.admin.v2.broker_pb2', _globals) @@ -20,20 +20,20 @@ _globals['_BROKERSERVICE'].methods_by_name['ListBrokers']._loaded_options = None _globals['_BROKERSERVICE'].methods_by_name['ListBrokers']._serialized_options = b'\xea\x92\x19\x02\x10\x03' _globals['_GETBROKERREQUEST']._serialized_start = 147 - _globals['_GETBROKERREQUEST']._serialized_end = 182 - _globals['_GETBROKERRESPONSE']._serialized_start = 184 - _globals['_GETBROKERRESPONSE']._serialized_end = 251 - _globals['_LISTBROKERSREQUEST']._serialized_start = 253 - _globals['_LISTBROKERSREQUEST']._serialized_end = 273 - _globals['_LISTBROKERSRESPONSE']._serialized_start = 275 - _globals['_LISTBROKERSRESPONSE']._serialized_end = 345 - _globals['_BROKER']._serialized_start = 348 - _globals['_BROKER']._serialized_end = 487 - _globals['_BUILDINFO']._serialized_start = 489 - _globals['_BUILDINFO']._serialized_end = 536 - _globals['_ADMINSERVER']._serialized_start = 538 - _globals['_ADMINSERVER']._serialized_end = 601 - _globals['_RPCROUTE']._serialized_start = 603 - _globals['_RPCROUTE']._serialized_end = 647 - _globals['_BROKERSERVICE']._serialized_start = 650 - _globals['_BROKERSERVICE']._serialized_end = 883 \ No newline at end of file + _globals['_GETBROKERREQUEST']._serialized_end = 199 + _globals['_GETBROKERRESPONSE']._serialized_start = 201 + _globals['_GETBROKERRESPONSE']._serialized_end = 268 + _globals['_LISTBROKERSREQUEST']._serialized_start = 270 + _globals['_LISTBROKERSREQUEST']._serialized_end = 290 + _globals['_LISTBROKERSRESPONSE']._serialized_start = 292 + _globals['_LISTBROKERSRESPONSE']._serialized_end = 362 + _globals['_BROKER']._serialized_start = 365 + _globals['_BROKER']._serialized_end = 546 + _globals['_BUILDINFO']._serialized_start = 548 + _globals['_BUILDINFO']._serialized_end = 595 + _globals['_ADMINSERVER']._serialized_start = 597 + _globals['_ADMINSERVER']._serialized_end = 660 + _globals['_RPCROUTE']._serialized_start = 662 + _globals['_RPCROUTE']._serialized_end = 706 + _globals['_BROKERSERVICE']._serialized_start = 709 + _globals['_BROKERSERVICE']._serialized_end = 942 \ No newline at end of file diff --git a/tests/rptest/clients/admin/proto/redpanda/core/admin/v2/broker_pb2.pyi b/tests/rptest/clients/admin/proto/redpanda/core/admin/v2/broker_pb2.pyi index 115f25e5f4924..7f10903b68877 100644 --- a/tests/rptest/clients/admin/proto/redpanda/core/admin/v2/broker_pb2.pyi +++ b/tests/rptest/clients/admin/proto/redpanda/core/admin/v2/broker_pb2.pyi @@ -34,12 +34,18 @@ class GetBrokerRequest(google.protobuf.message.Message): DESCRIPTOR: google.protobuf.descriptor.Descriptor NODE_ID_FIELD_NUMBER: builtins.int node_id: builtins.int - 'The node ID for the broker. If set to -1, the broker handling the RPC\n request returns information about itself.\n ' + 'The node ID for the broker. If unset, the broker handling the RPC\n request returns information about itself.\n ' - def __init__(self, *, node_id: builtins.int=...) -> None: + def __init__(self, *, node_id: builtins.int | None=...) -> None: ... - def ClearField(self, field_name: typing.Literal['node_id', b'node_id']) -> None: + def HasField(self, field_name: typing.Literal['_node_id', b'_node_id', 'node_id', b'node_id']) -> builtins.bool: + ... + + def ClearField(self, field_name: typing.Literal['_node_id', b'_node_id', 'node_id', b'node_id']) -> None: + ... + + def WhichOneof(self, oneof_group: typing.Literal['_node_id', b'_node_id']) -> typing.Literal['node_id'] | None: ... Global___GetBrokerRequest: typing_extensions.TypeAlias = GetBrokerRequest @@ -101,19 +107,33 @@ class Broker(google.protobuf.message.Message): @property def build_info(self) -> Global___BuildInfo: - """The build this broker is running.""" + """The build this broker is running. + + Only populated for `GetBroker` RPCs + """ @property def admin_server(self) -> Global___AdminServer: - """The admin server information.""" + """The admin server information. + + Only populated for `GetBroker` RPCs + """ def __init__(self, *, node_id: builtins.int=..., build_info: Global___BuildInfo | None=..., admin_server: Global___AdminServer | None=...) -> None: ... - def HasField(self, field_name: typing.Literal['admin_server', b'admin_server', 'build_info', b'build_info']) -> builtins.bool: + def HasField(self, field_name: typing.Literal['_admin_server', b'_admin_server', '_build_info', b'_build_info', 'admin_server', b'admin_server', 'build_info', b'build_info']) -> builtins.bool: + ... + + def ClearField(self, field_name: typing.Literal['_admin_server', b'_admin_server', '_build_info', b'_build_info', 'admin_server', b'admin_server', 'build_info', b'build_info', 'node_id', b'node_id']) -> None: + ... + + @typing.overload + def WhichOneof(self, oneof_group: typing.Literal['_admin_server', b'_admin_server']) -> typing.Literal['admin_server'] | None: ... - def ClearField(self, field_name: typing.Literal['admin_server', b'admin_server', 'build_info', b'build_info', 'node_id', b'node_id']) -> None: + @typing.overload + def WhichOneof(self, oneof_group: typing.Literal['_build_info', b'_build_info']) -> typing.Literal['build_info'] | None: ... Global___Broker: typing_extensions.TypeAlias = Broker diff --git a/tests/rptest/clients/admin/v2.py b/tests/rptest/clients/admin/v2.py index c9e898b5c3d01..ce59b63e8dc13 100644 --- a/tests/rptest/clients/admin/v2.py +++ b/tests/rptest/clients/admin/v2.py @@ -1,3 +1,4 @@ +import logging import random from typing import Literal, Protocol, final, Any @@ -40,6 +41,9 @@ class RedpandaServiceProto(Protocol): def started_nodes(self) -> list[ClusterNode]: ... + @property + def logger(self) -> logging.Logger: ... + # Re-export some protobufs for convenience broker_pb = broker_pb2 @@ -57,8 +61,11 @@ def started_nodes(self) -> list[ClusterNode]: ... # A hacky workaround for https://github.com/connectrpc/connect-python/issues/37 class HeaderInjectingClient: - def __init__(self, client, headers_to_inject: dict[str, str]): + def __init__( + self, client, logger: logging.Logger, headers_to_inject: dict[str, str] + ): self.client = client + self.logger = logger self.headers_to_inject = headers_to_inject def call_unary( @@ -69,6 +76,7 @@ def call_unary( extra_headers: dict[str, str] | None = None, timeout_seconds: float | None = None, ): + self.logger.debug(f"making admin RPC {url}") if extra_headers is None: extra_headers = self.headers_to_inject else: @@ -104,6 +112,7 @@ def __init__( def _make_service(self, service_clazz, node: ClusterNode | None = None): if not node: node = random.choice(self._rp.started_nodes()) + assert node, "must have at least one started node" client = service_clazz( base_url=f"http://{node.account.hostname}:9644", protocol=ConnectProtocol.CONNECT_PROTOBUF @@ -111,7 +120,7 @@ def _make_service(self, service_clazz, node: ClusterNode | None = None): else ConnectProtocol.CONNECT_JSON, ) client._connect_client = HeaderInjectingClient( - client._connect_client, self._headers.copy() + client._connect_client, self._rp.logger, self._headers.copy() ) return client