[GitHub] [kafka] cmccabe commented on a change in pull request #11179: KAFKA-13165: Validate node id, process role and quorum voters

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

cmccabe commented on a change in pull request #11179:
URL: https://github.com/apache/kafka/pull/11179#discussion_r685480957



##########
File path: core/src/test/scala/unit/kafka/raft/RaftManagerTest.scala
##########
@@ -28,27 +28,41 @@ import org.apache.kafka.common.metrics.Metrics
 import org.apache.kafka.common.utils.Time
 import org.apache.kafka.raft.KafkaRaftClient
 import org.apache.kafka.raft.RaftConfig
+import org.apache.kafka.test.TestUtils
 import org.junit.jupiter.api.Assertions._
 import org.junit.jupiter.api.Test
 import org.mockito.Mockito._
 
+import java.io.File
+
 class RaftManagerTest {
 
-  private def instantiateRaftManagerWithConfigs(processRoles: String, nodeId:String) = {
-    def configWithProcessRolesAndNodeId(processRoles: String, nodeId: String): KafkaConfig = {
+  private def instantiateRaftManagerWithConfigs(topicPartition: TopicPartition, processRoles: String, nodeId: String) = {
+    def configWithProcessRolesAndNodeId(processRoles: String, nodeId: String, logDir: File): KafkaConfig = {
       val props = new Properties
+      props.setProperty(KafkaConfig.MetadataLogDirProp, logDir.getPath)
       props.setProperty(KafkaConfig.ProcessRolesProp, processRoles)
       props.setProperty(KafkaConfig.NodeIdProp, nodeId)
       props.setProperty(KafkaConfig.ListenersProp, "PLAINTEXT://localhost:9093")
       props.setProperty(KafkaConfig.ControllerListenerNamesProp, "PLAINTEXT")
-      props.setProperty(RaftConfig.QUORUM_VOTERS_CONFIG, nodeId.concat("@localhost:9093"))
-      if (processRoles.contains("broker"))
+      if (processRoles.contains("broker")) {
         props.setProperty(KafkaConfig.InterBrokerListenerNameProp, "PLAINTEXT")
         props.setProperty(KafkaConfig.AdvertisedListenersProp, "PLAINTEXT://localhost:9092")
+        if (!processRoles.contains("controller")) {
+          val voterId = (nodeId.toInt + 1)
+          props.setProperty(KafkaConfig.QuorumVotersProp, s"${voterId}@localhost:9093")
+        }
+      } 
+
+      if (processRoles.contains("controller")) {
+        props.setProperty(KafkaConfig.QuorumVotersProp, s"${nodeId}@localhost:9093")
+      }
+
       new KafkaConfig(props)
     }
 
-    val config = configWithProcessRolesAndNodeId(processRoles, nodeId)
+    val logDir = TestUtils.tempDirectory()

Review comment:
       @jsancio , @mumrah : Can you file a follow-up JIRA for this? We can also include the other raft integration / unit tests




-- 
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