KAFKA-16410 kafka-leader-election / LeaderElectionCommand doesn't set exit code on error (#15591)

Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

tools/src/main/java/org/apache/kafka/tools/LeaderElectionCommand.java

@@ -28,6 +28,7 @@ import org.apache.kafka.common.TopicPartition;
 import org.apache.kafka.common.errors.ClusterAuthorizationException;
 import org.apache.kafka.common.errors.ElectionNotNeededException;
 import org.apache.kafka.common.errors.TimeoutException;
+import org.apache.kafka.common.utils.Exit;
 import org.apache.kafka.common.utils.Utils;
 import org.apache.kafka.server.common.AdminCommandFailedException;
 import org.apache.kafka.server.common.AdminOperationException;
@@ -62,11 +63,17 @@ public class LeaderElectionCommand {
     private static final DecodeJson.DecodeInteger INT = new DecodeJson.DecodeInteger();
 
     public static void main(String... args) {
+        Exit.exit(mainNoExit(args));
+    }
+
+    static int mainNoExit(String... args) {
         try {
             run(Duration.ofMillis(30000), args);
-        } catch (Exception e) {
+            return 0;
+        } catch (Throwable e) {
             System.err.println(e.getMessage());
             System.err.println(Utils.stackTrace(e));
+            return 1;
         }
     }
 

tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandErrorTest.java

@@ -22,6 +22,7 @@ import org.junit.jupiter.api.Test;
 
 import java.time.Duration;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -31,46 +32,54 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  * cluster creation and cleanup because the command is expected to fail immediately.
  */
 public class LeaderElectionCommandErrorTest {
+
     @Test
     public void testTopicWithoutPartition() {
-        String out = ToolsTestUtils.captureStandardErr(() -> LeaderElectionCommand.main(
+        String[] args = {
-                "--bootstrap-server", "nohost:9092",
+            "--bootstrap-server", "nohost:9092",
-                "--election-type", "unclean",
+            "--election-type", "unclean",
-                "--topic", "some-topic"
+            "--topic", "some-topic"
-            ));
+        };
+        assertEquals(1, LeaderElectionCommand.mainNoExit(args));
+        String out = ToolsTestUtils.captureStandardErr(() -> LeaderElectionCommand.mainNoExit(args));
         assertTrue(out.startsWith("Missing required option(s)"));
         assertTrue(out.contains(" partition"));
     }
 
     @Test
     public void testPartitionWithoutTopic() {
-        String out = ToolsTestUtils.captureStandardErr(() -> LeaderElectionCommand.main(
+        String[] args = {
             "--bootstrap-server", "nohost:9092",
             "--election-type", "unclean",
             "--all-topic-partitions",
             "--partition", "0"
-        ));
+        };
-        String[] rows = out.split("\n");
+        assertEquals(1, LeaderElectionCommand.mainNoExit(args));
+        String out = ToolsTestUtils.captureStandardErr(() -> LeaderElectionCommand.mainNoExit(args));
         assertTrue(out.startsWith("Option partition is only allowed if topic is used"));
     }
 
     @Test
     public void testMissingElectionType() {
-        String out = ToolsTestUtils.captureStandardErr(() -> LeaderElectionCommand.main(
+        String[] args = {
             "--bootstrap-server", "nohost:9092",
             "--topic", "some-topic",
             "--partition", "0"
-        ));
+        };
+        assertEquals(1, LeaderElectionCommand.mainNoExit(args));
+        String out = ToolsTestUtils.captureStandardErr(() -> LeaderElectionCommand.mainNoExit(args));
         assertTrue(out.startsWith("Missing required option(s)"));
         assertTrue(out.contains(" election-type"));
     }
 
     @Test
     public void testMissingTopicPartitionSelection() {
-        String out = ToolsTestUtils.captureStandardErr(() -> LeaderElectionCommand.main(
+        String[] args = {
             "--bootstrap-server", "nohost:9092",
             "--election-type", "preferred"
-        ));
+        };
+        assertEquals(1, LeaderElectionCommand.mainNoExit(args));
+        String out = ToolsTestUtils.captureStandardErr(() -> LeaderElectionCommand.mainNoExit(args));
         assertTrue(out.startsWith("One and only one of the following options is required: "));
         assertTrue(out.contains(" all-topic-partitions"));
         assertTrue(out.contains(" topic"));

tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java

@@ -105,11 +105,11 @@ public class LeaderElectionCommandTest {
         cluster.startBroker(broker3);
         TestUtils.waitForOnlineBroker(client, broker3);
 
-        LeaderElectionCommand.main(
+        assertEquals(0, LeaderElectionCommand.mainNoExit(
             "--bootstrap-server", cluster.bootstrapServers(),
             "--election-type", "unclean",
             "--all-topic-partitions"
-        );
+        ));
 
         TestUtils.assertLeader(client, topicPartition, broker3);
     }
@@ -121,11 +121,11 @@ public class LeaderElectionCommandTest {
         Path adminConfigPath = tempAdminConfig(defaultApiTimeoutMs, requestTimeoutMs);
 
         try (final MockedStatic<Admin> mockedAdmin = Mockito.mockStatic(Admin.class)) {
-            LeaderElectionCommand.main(
+            assertEquals(1, LeaderElectionCommand.mainNoExit(
                 "--bootstrap-server", cluster.bootstrapServers(),
                 "--election-type", "unclean", "--all-topic-partitions",
                 "--admin.config", adminConfigPath.toString()
-            );
+            ));
 
             ArgumentCaptor<Properties> argumentCaptor = ArgumentCaptor.forClass(Properties.class);
             mockedAdmin.verify(() -> Admin.create(argumentCaptor.capture()));
@@ -161,12 +161,12 @@ public class LeaderElectionCommandTest {
         cluster.startBroker(broker3);
         TestUtils.waitForOnlineBroker(client, broker3);
 
-        LeaderElectionCommand.main(
+        assertEquals(0, LeaderElectionCommand.mainNoExit(
             "--bootstrap-server", cluster.bootstrapServers(),
             "--election-type", "unclean",
             "--topic", topic,
             "--partition", Integer.toString(partition)
-        );
+        ));
 
         TestUtils.assertLeader(client, topicPartition, broker3);
     }
@@ -200,11 +200,11 @@ public class LeaderElectionCommandTest {
 
         Path topicPartitionPath = tempTopicPartitionFile(Collections.singletonList(topicPartition));
 
-        LeaderElectionCommand.main(
+        assertEquals(0, LeaderElectionCommand.mainNoExit(
             "--bootstrap-server", cluster.bootstrapServers(),
             "--election-type", "unclean",
             "--path-to-json-file", topicPartitionPath.toString()
-        );
+        ));
 
         TestUtils.assertLeader(client, topicPartition, broker3);
     }
@@ -233,11 +233,11 @@ public class LeaderElectionCommandTest {
             JavaConverters.asScalaBuffer(Collections.singletonList(broker2)).toSet()
         );
 
-        LeaderElectionCommand.main(
+        assertEquals(0, LeaderElectionCommand.mainNoExit(
             "--bootstrap-server", cluster.bootstrapServers(),
             "--election-type", "preferred",
             "--all-topic-partitions"
-        );
+        ));
 
         TestUtils.assertLeader(client, topicPartition, broker2);
     }
@@ -288,7 +288,7 @@ public class LeaderElectionCommandTest {
 
         Path topicPartitionPath = tempTopicPartitionFile(Arrays.asList(topicPartition0, topicPartition1));
         String output = ToolsTestUtils.captureStandardOut(() ->
-            LeaderElectionCommand.main(
+            LeaderElectionCommand.mainNoExit(
                 "--bootstrap-server", cluster.bootstrapServers(),
                 "--election-type", "preferred",
                 "--path-to-json-file", topicPartitionPath.toString()