[GitHub] [kafka] hachikuji commented on a change in pull request #11503: KAFKA-13456: controller.listener.names required in KRaft

[GitBox <gi...@apache.org>]

hachikuji commented on a change in pull request #11503:
URL: https://github.com/apache/kafka/pull/11503#discussion_r749765601



##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -2007,8 +2007,13 @@ class KafkaConfig private(doLog: Boolean, val props: java.util.Map[_, _], dynami
       "offsets.commit.required.acks must be greater or equal -1 and less or equal to offsets.topic.replication.factor")
     require(BrokerCompressionCodec.isValid(compressionType), "compression.type : " + compressionType + " is not valid." +
       " Valid options are " + BrokerCompressionCodec.brokerCompressionOptions.mkString(","))
-    require(!processRoles.contains(ControllerRole) || controllerListeners.nonEmpty,
-      s"${KafkaConfig.ControllerListenerNamesProp} cannot be empty if the server has the controller role")
+    if (usesSelfManagedQuorum) {

Review comment:
       A couple additional validations we can do:
   
   1. Ensure control plane listener is empty. We have this check in BrokerServer already, but we should check it here as well.
   2. Verify port consistency between the controller listener and the quorum voter string. As you noted offline, the port gets duplicated, so we should be sure they match. 
   3. We currently assume that the first listener in `controller.listener.names` is the one that matches the voter string. We should either require this to be the case, or change the logic in `RaftManager` to choose the right listener.




-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org