cmccabe commented on code in PR #16774:
URL: https://github.com/apache/kafka/pull/16774#discussion_r1707599312
##########
tools/src/main/java/org/apache/kafka/tools/MetadataQuorumCommand.java:
##########
@@ -291,4 +320,179 @@ public String toString() {
return sb.toString();
}
}
+
+ private static void addAddControllerSubParser(Subparsers subparsers) {
+ Subparser addControllerParser = subparsers
+ .addParser("add-controller")
+ .help("Add a controller to the KRaft controller cluster");
+
+ addControllerParser
+ .addArgument("--dry-run")
+ .help("True if we should print what would be done, but not do it.")
+ .action(Arguments.storeTrue());
+ }
+
+ static int getControllerId(Properties props) throws TerseException {
+ if (!props.containsKey(KRaftConfigs.NODE_ID_CONFIG)) {
+ throw new TerseException(KRaftConfigs.NODE_ID_CONFIG + " not found
in configuration " +
+ "file. Is this a valid controller configuration file?");
+ }
+ int nodeId =
Integer.parseInt(props.getProperty(KRaftConfigs.NODE_ID_CONFIG));
+ if (nodeId < 0) {
+ throw new TerseException(KRaftConfigs.NODE_ID_CONFIG + " was
negative in configuration " +
+ "file. Is this a valid controller configuration file?");
+ }
+ if (!props.getOrDefault(KRaftConfigs.PROCESS_ROLES_CONFIG,
"").toString().contains("controller")) {
+ throw new TerseException(KRaftConfigs.PROCESS_ROLES_CONFIG + " did
not contain 'controller' in " +
+ "configuration file. Is this a valid controller configuration
file?");
+ }
+ return nodeId;
+ }
+
+ static String getMetadataDirectory(Properties props) throws TerseException
{
+ if (props.containsKey(KRaftConfigs.METADATA_LOG_DIR_CONFIG)) {
+ return props.getProperty(KRaftConfigs.METADATA_LOG_DIR_CONFIG);
+ }
+ if (props.containsKey(ServerLogConfigs.LOG_DIRS_CONFIG)) {
+ String[] logDirs =
props.getProperty(ServerLogConfigs.LOG_DIRS_CONFIG).trim().split(",");
+ if (logDirs.length > 0) {
+ return logDirs[0];
+ }
+ }
+ throw new TerseException("Neither " +
KRaftConfigs.METADATA_LOG_DIR_CONFIG + " nor " +
+ ServerLogConfigs.LOG_DIRS_CONFIG + " were found. Is this a valid
controller " +
+ "configuration file?");
+ }
+
+ static Uuid getMetadataDirectoryId(String metadataDirectory) throws
Exception {
+ MetaPropertiesEnsemble ensemble = new MetaPropertiesEnsemble.Loader().
+ addLogDirs(Collections.singletonList(metadataDirectory)).
+ addMetadataLogDir(metadataDirectory).
+ load();
+ MetaProperties metaProperties =
ensemble.logDirProps().get(metadataDirectory);
+ if (metaProperties == null) {
+ throw new TerseException("Unable to read meta.properties from " +
metadataDirectory);
+ }
+ if (!metaProperties.directoryId().isPresent()) {
+ throw new TerseException("No directory id found in " +
metadataDirectory);
+ }
+ return metaProperties.directoryId().get();
+ }
+
+ static Set<RaftVoterEndpoint> getControllerAdvertisedListeners(
+ Properties props
+ ) throws Exception {
+ Map<String, Endpoint> listeners = new HashMap<>();
+ SocketServerConfigs.listenerListToEndPoints(
+ props.getOrDefault(SocketServerConfigs.LISTENERS_CONFIG,
"").toString(),
+ __ -> SecurityProtocol.PLAINTEXT).forEach(e ->
listeners.put(e.listenerName().get(), e));
+ SocketServerConfigs.listenerListToEndPoints(
+
props.getOrDefault(SocketServerConfigs.ADVERTISED_LISTENERS_CONFIG,
"").toString(),
+ __ -> SecurityProtocol.PLAINTEXT).forEach(e ->
listeners.put(e.listenerName().get(), e));
+ if (!props.containsKey(KRaftConfigs.CONTROLLER_LISTENER_NAMES_CONFIG))
{
+ throw new
TerseException(KRaftConfigs.CONTROLLER_LISTENER_NAMES_CONFIG +
+ " was not found. Is this a valid controller configuration
file?");
+ }
+ LinkedHashSet<RaftVoterEndpoint> results = new LinkedHashSet<>();
+ for (String listenerName : props.getProperty(
+ KRaftConfigs.CONTROLLER_LISTENER_NAMES_CONFIG).split(",")) {
+ listenerName = ListenerName.normalised(listenerName).value();
+ Endpoint endpoint = listeners.get(listenerName);
+ if (endpoint == null) {
+ throw new TerseException("Cannot find information about
controller listener name: " +
+ listenerName);
+ }
+ results.add(new RaftVoterEndpoint(endpoint.listenerName().get(),
+ endpoint.host() == null ? "localhost" : endpoint.host(),
Review Comment:
I was on the fence about this.
I think for local testing, I can definitely imagine some people putting
`CONTROLLER://:9093` in the advertised listeners, and not thinking anything
more about it. This would certainly be a bad idea in production, though.
We could throw an exception in that situation, or we could translate it to
`localhost`... Perhaps the exception is better?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
