tinaselenge commented on code in PR #17524: URL: https://github.com/apache/kafka/pull/17524#discussion_r1830805096
########## core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala: ########## @@ -615,6 +615,42 @@ class PlaintextAdminIntegrationTest extends BaseAdminIntegrationTest { assertEquals(brokerStrs.mkString(","), nodeStrs.mkString(",")) } + @ParameterizedTest + @ValueSource(strings = Array("kraft")) + def testListNodesNotIncludingFencedBrokers(quorum: String): Unit = { + client = createAdminClient + val fencedBrokerId = brokers.last.config.brokerId + killBroker(fencedBrokerId) + Thread.sleep(10000) //sleep is needed to ensure the broker is fenced after the shutdown Review Comment: This was my concern too but the broker takes a few seconds to get fenced after being killed and 10000ms seems to be sufficient, although I had planned to run this test several times in Jenkins to confirm it and adjust if needed. I pushed another commit to merge these two test cases together, so that we only wait for a broker to get fenced once instead of twice. Also used TestUtils.retry to retry the request until we receive 2 of 3 brokers in the response or reach the max wait time. I set the max wait time to a larger value, in case it takes longer sometimes, but it shouldn't take more than 10s normally. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org