kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject [kafka] branch 1.0 updated: KAFKA-6656; Config tool should return non-zero status code on failure (#4711)
Date Thu, 15 Mar 2018 17:49:52 GMT
This is an automated email from the ASF dual-hosted git repository.

jgus pushed a commit to branch 1.0
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/1.0 by this push:
     new ca83677  KAFKA-6656; Config tool should return non-zero status code on failure (#4711)
ca83677 is described below

commit ca836774c7bfa1e17f4ed0dd8f6780db3dd74c64
Author: Jason Gustafson <jason@confluent.io>
AuthorDate: Thu Mar 15 10:33:29 2018 -0700

    KAFKA-6656; Config tool should return non-zero status code on failure (#4711)
    
    Prior to this patch, we caught some exceptions when executing the command, which meant
that it would return with status code zero. This patch fixes this and makes the expected exit
behavior explicit. Test cases have been added to verify the change.
    
    Reviewers: Ismael Juma <ismael@juma.me.uk>
---
 .../src/main/scala/kafka/admin/ConfigCommand.scala | 47 +++++++++++++---------
 .../scala/unit/kafka/admin/ConfigCommandTest.scala | 40 +++++++++++++++++-
 2 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/core/src/main/scala/kafka/admin/ConfigCommand.scala b/core/src/main/scala/kafka/admin/ConfigCommand.scala
index febf40f..c0cdff2 100644
--- a/core/src/main/scala/kafka/admin/ConfigCommand.scala
+++ b/core/src/main/scala/kafka/admin/ConfigCommand.scala
@@ -24,7 +24,7 @@ import kafka.common.Config
 import kafka.common.InvalidConfigException
 import kafka.log.LogConfig
 import kafka.server.{ConfigEntityName, ConfigType, DynamicConfig}
-import kafka.utils.{CommandLineUtils, ZkUtils}
+import kafka.utils.{CommandLineUtils, Exit, ZkUtils}
 import kafka.utils.Implicits._
 import org.apache.kafka.common.security.JaasUtils
 import org.apache.kafka.common.security.scram._
@@ -53,30 +53,37 @@ object ConfigCommand extends Config {
   val DefaultScramIterations = 4096
 
   def main(args: Array[String]): Unit = {
+    try {
+      val opts = new ConfigCommandOptions(args)
 
-    val opts = new ConfigCommandOptions(args)
-
-    if(args.length == 0)
-      CommandLineUtils.printUsageAndDie(opts.parser, "Add/Remove entity config for a topic,
client, user or broker")
+      if (args.length == 0)
+        CommandLineUtils.printUsageAndDie(opts.parser, "Add/Remove entity config for a topic,
client, user or broker")
 
-    opts.checkArgs()
+      opts.checkArgs()
 
-    val zkUtils = ZkUtils(opts.options.valueOf(opts.zkConnectOpt),
-                          30000,
-                          30000,
-                          JaasUtils.isZkSecurityEnabled())
+      val zkUtils = ZkUtils(opts.options.valueOf(opts.zkConnectOpt),
+        30000,
+        30000,
+        JaasUtils.isZkSecurityEnabled())
 
-    try {
-      if (opts.options.has(opts.alterOpt))
-        alterConfig(zkUtils, opts)
-      else if (opts.options.has(opts.describeOpt))
-        describeConfig(zkUtils, opts)
+      try {
+        if (opts.options.has(opts.alterOpt))
+          alterConfig(zkUtils, opts)
+        else if (opts.options.has(opts.describeOpt))
+          describeConfig(zkUtils, opts)
+      } finally {
+        zkUtils.close()
+      }
     } catch {
-      case e: Throwable =>
-        println("Error while executing config command " + e.getMessage)
-        println(Utils.stackTrace(e))
-    } finally {
-      zkUtils.close()
+      case e @ (_: IllegalArgumentException | _: InvalidConfigException | _: OptionException)
=>
+        logger.debug(s"Failed config command with args $args", e)
+        System.err.println(e.getMessage)
+        Exit.exit(1)
+
+      case t: Throwable =>
+        System.err.println(s"Error while executing config command with args $args")
+        t.printStackTrace(System.err)
+        Exit.exit(1)
     }
   }
 
diff --git a/core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala b/core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala
index 87ce46e..02591bd 100644
--- a/core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala
+++ b/core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala
@@ -21,8 +21,9 @@ import java.util.Properties
 import kafka.admin.ConfigCommand.ConfigCommandOptions
 import kafka.common.InvalidConfigException
 import kafka.server.ConfigEntityName
-import kafka.utils.{Logging, ZkUtils}
+import kafka.utils.ZkUtils
 import kafka.zk.ZooKeeperTestHarness
+import kafka.utils.{Exit, Logging}
 import org.apache.kafka.common.security.scram.ScramCredentialUtils
 import org.apache.kafka.common.utils.Sanitizer
 import org.easymock.EasyMock
@@ -33,6 +34,40 @@ import scala.collection.mutable
 import scala.collection.JavaConverters._
 
 class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
+
+  @Test
+  def shouldExitWithNonZeroStatusOnArgError(): Unit = {
+    assertNonZeroStatusExit(Array("--blah"))
+  }
+
+  @Test
+  def shouldExitWithNonZeroStatusOnZkCommandError(): Unit = {
+    assertNonZeroStatusExit(Array(
+      "--zookeeper", zkConnect,
+      "--entity-name", "my-topic",
+      "--entity-type", "topics",
+      "--alter",
+      "--add-config", "message.max.size=100000"))
+  }
+
+  private def assertNonZeroStatusExit(args: Array[String]): Unit = {
+    var exitStatus: Option[Int] = None
+    Exit.setExitProcedure { (status, _) =>
+      exitStatus = Some(status)
+      throw new RuntimeException
+    }
+
+    try {
+      ConfigCommand.main(args)
+    } catch {
+      case e: RuntimeException =>
+    } finally {
+      Exit.resetExitProcedure()
+    }
+
+    assertEquals(Some(1), exitStatus)
+  }
+
   @Test
   def shouldParseArgumentsForClientsEntityType() {
     testArgumentParse("clients")
@@ -253,7 +288,7 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
         "--delete-config", mechanism))
 
     val credentials = mutable.Map[String, Properties]()
-    case class CredentialChange(val user: String, val mechanisms: Set[String], val iterations:
Int) extends TestAdminUtils {
+    case class CredentialChange(user: String, mechanisms: Set[String], iterations: Int) extends
TestAdminUtils {
       override def fetchEntityConfig(zkUtils: ZkUtils, entityType: String, entityName: String):
Properties = {
         credentials.getOrElse(entityName, new Properties())
       }
@@ -456,4 +491,5 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging {
         Map("users" -> Seq("<default>", sanitizedPrincipal)) ++ defaultUserMap ++
userMap,
         Seq("<default>/clients/client-3", sanitizedPrincipal + "/clients/client-2"))
   }
+
 }

-- 
To stop receiving notification emails like this one, please contact
jgus@apache.org.

Mime
View raw message