kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [kudu] branch master updated: [tool] Add cluster name resolver for CLI tools
Date Wed, 29 May 2019 16:36:28 GMT
This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 3b58cfb  [tool] Add cluster name resolver for CLI tools
3b58cfb is described below

commit 3b58cfb3bd7ff39ed3f7382d8cca5e00d44d9c2d
Author: Yingchun Lai <405403881@qq.com>
AuthorDate: Thu May 16 18:40:30 2019 +0800

    [tool] Add cluster name resolver for CLI tools
    
    Kudu master rpc addresses are not easy to remember when use CLI
    tools, even worse, when manage several Kudu clusters.
    This patch add a feature to allow CLI tools to resolve cluster
    name configured in a YAML file ${$KUDU_CONFIG}/cluster_info.yaml.
    
    Change-Id: I2fc235906adbf25fcc2059fa8443e3013830ec01
    Reviewed-on: http://gerrit.cloudera.org:8080/13354
    Reviewed-by: Adar Dembo <adar@cloudera.com>
    Tested-by: Adar Dembo <adar@cloudera.com>
---
 src/kudu/tools/kudu-tool-test.cc      | 135 ++++++++++++++++++++++++++++++++--
 src/kudu/tools/tool_action_cluster.cc |  10 +--
 src/kudu/tools/tool_action_common.cc  | 104 ++++++++++++++++++++++----
 src/kudu/tools/tool_action_common.h   |  39 +++++++++-
 src/kudu/tools/tool_action_hms.cc     |   6 +-
 src/kudu/tools/tool_action_master.cc  |   6 +-
 src/kudu/tools/tool_action_perf.cc    |   5 +-
 src/kudu/tools/tool_action_table.cc   |  15 ++--
 src/kudu/tools/tool_action_tablet.cc  |  15 ++--
 9 files changed, 282 insertions(+), 53 deletions(-)

diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 422a719..26b042e 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -15,17 +15,17 @@
 // specific language governing permissions and limitations
 // under the License.
 
+#include <stdlib.h>
 #include <sys/stat.h>
 
 #include <algorithm>
 #include <cstdint>
 #include <cstdio>
-#include <cstdlib>
+#include <fstream>
 #include <iterator>
 #include <map>
 #include <memory>
 #include <set>
-#include <sstream>
 #include <string>
 #include <tuple>  // IWYU pragma: keep
 #include <type_traits>
@@ -124,6 +124,7 @@
 #include "kudu/util/path_util.h"
 #include "kudu/util/pb_util.h"
 #include "kudu/util/random.h"
+#include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/slice.h"
 #include "kudu/util/status.h"
 #include "kudu/util/subprocess.h"
@@ -606,6 +607,25 @@ class ToolTest : public KuduTest {
     ww.Cleanup();
   }
 
+  // Write YAML content to file ${KUDU_CONFIG}/kudurc.
+  void PrepareConfigFile(const string& content) {
+    string fname = GetTestPath("kudurc");
+    unique_ptr<WritableFile> writable_file;
+    ASSERT_OK(env_->NewWritableFile(fname, &writable_file));
+    ASSERT_OK(writable_file->Append(Slice(content)));
+    ASSERT_OK(writable_file->Close());
+  }
+
+  void CheckCorruptClusterInfoConfigFile(const string& content,
+                                         const string& expect_err) {
+    const string kClusterName = "external_mini_cluster";
+    NO_FATALS(PrepareConfigFile(content));
+    string stderr;
+    Status s = RunActionStderrString(Substitute("master list @$0", kClusterName), &stderr);
+    ASSERT_TRUE(s.IsRuntimeError());
+    ASSERT_STR_CONTAINS(stderr, expect_err);
+  }
+
  protected:
   void RunLoadgen(int num_tservers = 1,
                   const vector<string>& tool_args = {},
@@ -677,7 +697,7 @@ class ToolTestCopyTableParameterized :
     }
   }
 
-  std::vector<RunCopyTableCheckArgs> GenerateArgs() {
+  vector<RunCopyTableCheckArgs> GenerateArgs() {
     RunCopyTableCheckArgs args = { kTableName,
                                    "",
                                    1,
@@ -702,7 +722,7 @@ class ToolTestCopyTableParameterized :
         return { args };
       case kTestCopyTablePredicates: {
         auto mid = total_rows_ / 2;
-        std::vector<RunCopyTableCheckArgs> multi_args;
+        vector<RunCopyTableCheckArgs> multi_args;
         {
           auto args_temp = args;
           multi_args.emplace_back(std::move(args_temp));
@@ -879,7 +899,7 @@ void ToolTest::StartExternalMiniCluster(ExternalMiniClusterOptions opts)
{
   cluster_.reset(new ExternalMiniCluster(std::move(opts)));
   ASSERT_OK(cluster_->Start());
   inspect_.reset(new MiniClusterFsInspector(cluster_.get()));
-  ASSERT_OK(CreateTabletServerMap(cluster_->master_proxy(),
+  ASSERT_OK(CreateTabletServerMap(cluster_->master_proxy(0),
                                   cluster_->messenger(), &ts_map_));
 }
 
@@ -4626,6 +4646,111 @@ TEST_F(ToolTest, TestParseStacks) {
                              "invalid JSON payload.*Missing a closing quotation mark in string");
 }
 
+TEST_F(ToolTest, ClusterNameResolverEnvNotSet) {
+  const string kClusterName = "external_mini_cluster";
+  CHECK_ERR(unsetenv("KUDU_CONFIG"));
+  string stderr;
+  Status s = RunActionStderrString(
+        Substitute("master list @$0", kClusterName),
+        &stderr);
+  ASSERT_TRUE(s.IsRuntimeError());
+  ASSERT_STR_CONTAINS(
+      stderr, "Not found: ${KUDU_CONFIG} is missing");
+}
+
+TEST_F(ToolTest, ClusterNameResolverFileNotExist) {
+  const string kClusterName = "external_mini_cluster";
+  CHECK_ERR(setenv("KUDU_CONFIG", GetTestDataDirectory().c_str(), 1));
+  SCOPED_CLEANUP({
+    CHECK_ERR(unsetenv("KUDU_CONFIG"));
+  });
+
+  string stderr;
+  Status s = RunActionStderrString(
+        Substitute("master list @$0", kClusterName),
+        &stderr);
+  ASSERT_TRUE(s.IsRuntimeError());
+  ASSERT_STR_CONTAINS(
+      stderr, Substitute("Not found: configuration file $0/kudurc was not found",
+              GetTestDataDirectory()));
+}
+
+TEST_F(ToolTest, ClusterNameResolverFileCorrupt) {
+  ExternalMiniClusterOptions opts;
+  opts.num_masters = 3;
+  NO_FATALS(StartExternalMiniCluster(std::move(opts)));
+  auto master_addrs_str = HostPort::ToCommaSeparatedString(cluster_->master_rpc_addrs());
+
+  // Prepare ${KUDU_CONFIG}.
+  const string kClusterName = "external_mini_cluster";
+  CHECK_ERR(setenv("KUDU_CONFIG", GetTestDataDirectory().c_str(), 1));
+  SCOPED_CLEANUP({
+    CHECK_ERR(unsetenv("KUDU_CONFIG"));
+  });
+
+  // Missing 'clusters_info' section.
+  NO_FATALS(CheckCorruptClusterInfoConfigFile(
+              Substitute(R"*($0:)*""\n"
+                         R"*(  master_addresses: $1)*", kClusterName, master_addrs_str),
+              Substitute("Not found: parse field clusters_info error: invalid node; ")));
+
+  // Missing specified cluster name.
+  NO_FATALS(CheckCorruptClusterInfoConfigFile(
+              Substitute(R"*(clusters_info:)*""\n"
+                         R"*(  $0some_suffix:)*""\n"
+                         R"*(    master_addresses: $1)*", kClusterName, master_addrs_str),
+              Substitute("Not found: parse field $0 error: invalid node; ",
+                         kClusterName)));
+
+  // Missing 'master_addresses' section.
+  NO_FATALS(CheckCorruptClusterInfoConfigFile(
+              Substitute(R"*(clusters_info:)*""\n"
+                         R"*(  $0:)*""\n"
+                         R"*(    master_addresses_some_suffix: $1)*", kClusterName, master_addrs_str),
+              Substitute("Not found: parse field master_addresses error: invalid node; ")));
+
+  // Invalid 'master_addresses' value.
+  NO_FATALS(CheckCorruptClusterInfoConfigFile(
+              Substitute(R"*(clusters_info:)*""\n"
+                         R"*(  $0:)*""\n"
+                         R"*(    master_addresses: bad,masters,addresses)*", kClusterName),
+              Substitute("Network error: Could not connect to the cluster: unable to resolve
"
+                         "address for bad: Name or service not known")));
+}
+
+TEST_F(ToolTest, ClusterNameResolverNormal) {
+  ExternalMiniClusterOptions opts;
+  opts.num_masters = 3;
+  NO_FATALS(StartExternalMiniCluster(std::move(opts)));
+  auto master_addrs_str = HostPort::ToCommaSeparatedString(cluster_->master_rpc_addrs());
+
+  // Prepare ${KUDU_CONFIG}.
+  const string kClusterName = "external_mini_cluster";
+  CHECK_ERR(setenv("KUDU_CONFIG", GetTestDataDirectory().c_str(), 1));
+  SCOPED_CLEANUP({
+    CHECK_ERR(unsetenv("KUDU_CONFIG"));
+  });
+
+  string content = Substitute(
+      R"*(# some header comments)*""\n"
+      R"*(clusters_info:)*""\n"
+      R"*(  $0:  # some section comments)*""\n"
+      R"*(    master_addresses: $1  # some key comments)*", kClusterName, master_addrs_str);
+
+  NO_FATALS(PrepareConfigFile(content));
+
+  // Verify output of the two methods.
+  string out1;
+  NO_FATALS(RunActionStdoutString(
+        Substitute("master list $0 --columns=uuid,rpc-addresses", master_addrs_str),
+        &out1));
+  string out2;
+  NO_FATALS(RunActionStdoutString(
+        Substitute("master list @$0 --columns=uuid,rpc-addresses", kClusterName),
+        &out2));
+  ASSERT_EQ(out1, out2);
+}
+
 class Is343ReplicaUtilTest :
     public ToolTest,
     public ::testing::WithParamInterface<bool> {
diff --git a/src/kudu/tools/tool_action_cluster.cc b/src/kudu/tools/tool_action_cluster.cc
index 3d78edf..7bbe8d0 100644
--- a/src/kudu/tools/tool_action_cluster.cc
+++ b/src/kudu/tools/tool_action_cluster.cc
@@ -22,7 +22,6 @@
 #include <memory>
 #include <string>
 #include <tuple>
-#include <unordered_map>
 #include <vector>
 
 #include <boost/algorithm/string/predicate.hpp>
@@ -32,7 +31,6 @@
 #include <glog/logging.h>
 
 #include "kudu/gutil/basictypes.h"
-#include "kudu/gutil/map-util.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/substitute.h"
@@ -168,8 +166,8 @@ namespace tools {
 namespace {
 
 Status RunKsck(const RunnerContext& context) {
-  vector<string> master_addresses = Split(
-      FindOrDie(context.required_args, kMasterAddressesArg), ",");
+  vector<string> master_addresses;
+  RETURN_NOT_OK(ParseMasterAddresses(context, &master_addresses));
   shared_ptr<KsckCluster> cluster;
   RETURN_NOT_OK_PREPEND(RemoteKsckCluster::Build(master_addresses, &cluster),
                         "unable to build KsckCluster");
@@ -283,8 +281,8 @@ Status EvaluateMoveSingleReplicasFlag(const vector<string>&
master_addresses,
 // can be the source and the destination of no more than the specified number of
 // move operations.
 Status RunRebalance(const RunnerContext& context) {
-  const vector<string> master_addresses = Split(
-      FindOrDie(context.required_args, kMasterAddressesArg), ",");
+  vector<string> master_addresses;
+  RETURN_NOT_OK(ParseMasterAddresses(context, &master_addresses));
   const vector<string> table_filters =
       Split(FLAGS_tables, ",", strings::SkipEmpty());
 
diff --git a/src/kudu/tools/tool_action_common.cc b/src/kudu/tools/tool_action_common.cc
index 7fb62a6..ce0efa4 100644
--- a/src/kudu/tools/tool_action_common.cc
+++ b/src/kudu/tools/tool_action_common.cc
@@ -17,11 +17,11 @@
 
 #include "kudu/tools/tool_action_common.h"
 
+#include <stdlib.h>
 #include <unistd.h>
 
 #include <algorithm>
 #include <cerrno>
-#include <cstddef>
 #include <iomanip>
 #include <iostream>
 #include <iterator>
@@ -36,6 +36,8 @@
 #include <gflags/gflags.h>
 #include <glog/logging.h>
 #include <google/protobuf/util/json_util.h>
+// IWYU pragma: no_include <yaml-cpp/node/impl.h>
+// IWYU pragma: no_include <yaml-cpp/node/node.h>
 
 #include "kudu/client/client-internal.h"  // IWYU pragma: keep
 #include "kudu/client/client.h"
@@ -73,6 +75,7 @@
 #include "kudu/tserver/tserver_admin.proxy.h"   // IWYU pragma: keep
 #include "kudu/tserver/tserver_service.proxy.h" // IWYU pragma: keep
 #include "kudu/util/async_util.h"
+#include "kudu/util/env.h"
 #include "kudu/util/faststring.h"
 #include "kudu/util/jsonwriter.h"
 #include "kudu/util/mem_tracker.pb.h"
@@ -80,8 +83,10 @@
 #include "kudu/util/monotime.h"
 #include "kudu/util/net/net_util.h"
 #include "kudu/util/net/sockaddr.h"
+#include "kudu/util/path_util.h"
 #include "kudu/util/pb_util.h"
 #include "kudu/util/status.h"
+#include "kudu/util/yamlreader.h"
 
 DEFINE_bool(force, false, "If true, allows the set_flag command to set a flag "
             "which is not explicitly marked as runtime-settable. Such flag "
@@ -184,8 +189,13 @@ namespace kudu {
 namespace tools {
 
 const char* const kMasterAddressesArg = "master_addresses";
-const char* const kMasterAddressesArgDesc = "Comma-separated list of Kudu "
-    "Master addresses where each address is of form 'hostname:port'";
+const char* const kMasterAddressesArgDesc = "Either comma-separated list of Kudu "
+    "master addresses where each address is of form 'hostname:port', or a cluster name if
it has "
+    "been configured in ${KUDU_CONFIG}/kudurc";
+const char* const kDestMasterAddressesArg = "dest_master_addresses";
+const char* const kDestMasterAddressesArgDesc = "Either comma-separated list of destination
Kudu "
+    "master addresses where each address is of form 'hostname:port', or a cluster name if
it has "
+    "been configured in ${KUDU_CONFIG}/kudurc";
 const char* const kTableNameArg = "table_name";
 const char* const kTabletIdArg = "tablet_id";
 const char* const kTabletIdArgDesc = "Tablet Identifier";
@@ -293,6 +303,19 @@ Status PrintDecoded(const LogEntryPB& entry, const Schema& tablet_schema)
{
   return Status::OK();
 }
 
+// A valid 'cluster name' is beginning with a special character '@'.
+// '@' is a character which has no special significance in shells and
+// it's an invalid character in hostname list, so we can use it to
+// distinguish cluster name from master addresses.
+bool GetClusterName(const string& master_addresses_str, string* cluster_name) {
+  CHECK(cluster_name);
+  if (HasPrefixString(master_addresses_str, "@")) {
+    *cluster_name = master_addresses_str.substr(1);  // Trim the first '@'.
+    return true;
+  }
+  return false;
+}
+
 } // anonymous namespace
 
 template<class ProxyClass>
@@ -386,11 +409,11 @@ Status PrintSegment(const scoped_refptr<ReadableLogSegment>&
segment) {
   return Status::OK();
 }
 
-Status GetServerFlags(const std::string& address,
+Status GetServerFlags(const string& address,
                       uint16_t default_port,
                       bool all_flags,
-                      const std::string& flag_tags,
-                      std::vector<server::GetFlagsResponsePB_Flag>* flags) {
+                      const string& flag_tags,
+                      vector<server::GetFlagsResponsePB_Flag>* flags) {
   unique_ptr<GenericServiceProxy> proxy;
   RETURN_NOT_OK(BuildProxy(address, default_port, &proxy));
 
@@ -472,9 +495,8 @@ bool MatchesAnyPattern(const vector<string>& patterns, const
string& str) {
 Status CreateKuduClient(const RunnerContext& context,
                         const char* master_addresses_arg,
                         client::sp::shared_ptr<KuduClient>* client) {
-  const string& master_addresses_str = FindOrDie(context.required_args,
-                                                 master_addresses_arg);
-  vector<string> master_addresses = Split(master_addresses_str, ",");
+  vector<string> master_addresses;
+  RETURN_NOT_OK(ParseMasterAddresses(context, master_addresses_arg, &master_addresses));
   return KuduClientBuilder()
              .master_server_addrs(master_addresses)
              .Build(client);
@@ -485,6 +507,61 @@ Status CreateKuduClient(const RunnerContext& context,
   return CreateKuduClient(context, kMasterAddressesArg, client);
 }
 
+Status ParseMasterAddressesStr(const RunnerContext& context,
+                               const char* master_addresses_arg,
+                               string* master_addresses_str) {
+  CHECK(master_addresses_str);
+  *master_addresses_str = FindOrDie(context.required_args, master_addresses_arg);
+  string cluster_name;
+  if (!GetClusterName(*master_addresses_str, &cluster_name)) {
+    // Treat it as master addresses.
+    return Status::OK();
+  }
+
+  // Try to resolve cluster name.
+  char* kudu_config_path = getenv("KUDU_CONFIG");
+  if (!kudu_config_path) {
+    return Status::NotFound("${KUDU_CONFIG} is missing");
+  }
+  auto config_file = JoinPathSegments(kudu_config_path, "kudurc");
+  if (!Env::Default()->FileExists(config_file)) {
+    return Status::NotFound(Substitute("configuration file $0 was not found", config_file));
+  }
+  YamlReader reader(config_file);
+  RETURN_NOT_OK(reader.Init());
+  YAML::Node clusters_info;
+  RETURN_NOT_OK(YamlReader::ExtractMap(reader.node(), "clusters_info", &clusters_info));
+  YAML::Node cluster_info;
+  RETURN_NOT_OK(YamlReader::ExtractMap(&clusters_info, cluster_name, &cluster_info));
+  RETURN_NOT_OK(YamlReader::ExtractScalar(&cluster_info, "master_addresses",
+                                          master_addresses_str));
+  return Status::OK();
+}
+
+Status ParseMasterAddressesStr(
+    const RunnerContext& context,
+    string* master_addresses_str) {
+  CHECK(master_addresses_str);
+  return ParseMasterAddressesStr(context, kMasterAddressesArg, master_addresses_str);
+}
+
+Status ParseMasterAddresses(const RunnerContext& context,
+                            const char* master_addresses_arg,
+                            vector<string>* master_addresses) {
+  CHECK(master_addresses);
+  string master_addresses_str;
+  RETURN_NOT_OK(ParseMasterAddressesStr(context, master_addresses_arg, &master_addresses_str));
+  *master_addresses = strings::Split(master_addresses_str, ",");
+  return Status::OK();
+}
+
+Status ParseMasterAddresses(
+    const RunnerContext& context,
+    vector<string>* master_addresses) {
+  CHECK(master_addresses);
+  return ParseMasterAddresses(context, kMasterAddressesArg, master_addresses);
+}
+
 Status PrintServerStatus(const string& address, uint16_t default_port) {
   ServerStatusPB status;
   RETURN_NOT_OK(GetServerStatus(address, default_port, &status));
@@ -660,12 +737,12 @@ void PrintTable(const vector<vector<string>>& columns,
const string& separator,
 
 } // anonymous namespace
 
-DataTable::DataTable(std::vector<string> col_names)
+DataTable::DataTable(vector<string> col_names)
     : column_names_(std::move(col_names)),
       columns_(column_names_.size()) {
 }
 
-void DataTable::AddRow(std::vector<string> row) {
+void DataTable::AddRow(vector<string> row) {
   CHECK_EQ(row.size(), columns_.size());
   int i = 0;
   for (auto& v : row) {
@@ -710,8 +787,9 @@ Status LeaderMasterProxy::Init(const vector<string>& master_addrs,
const MonoDel
 }
 
 Status LeaderMasterProxy::Init(const RunnerContext& context) {
-  const string& master_addrs = FindOrDie(context.required_args, kMasterAddressesArg);
-  return Init(strings::Split(master_addrs, ","), MonoDelta::FromMilliseconds(FLAGS_timeout_ms));
+  vector<string> master_addresses;
+  RETURN_NOT_OK(ParseMasterAddresses(context, &master_addresses));
+  return Init(master_addresses, MonoDelta::FromMilliseconds(FLAGS_timeout_ms));
 }
 
 template<typename Req, typename Resp>
diff --git a/src/kudu/tools/tool_action_common.h b/src/kudu/tools/tool_action_common.h
index 0682107..cf65a73 100644
--- a/src/kudu/tools/tool_action_common.h
+++ b/src/kudu/tools/tool_action_common.h
@@ -68,6 +68,8 @@ struct RunnerContext;
 // Constants for parameters and descriptions.
 extern const char* const kMasterAddressesArg;
 extern const char* const kMasterAddressesArgDesc;
+extern const char* const kDestMasterAddressesArg;
+extern const char* const kDestMasterAddressesArgDesc;
 extern const char* const kTableNameArg;
 extern const char* const kTabletIdArg;
 extern const char* const kTabletIdArgDesc;
@@ -150,11 +152,46 @@ Status CreateKuduClient(const RunnerContext& context,
                         const char* master_addresses_arg,
                         client::sp::shared_ptr<client::KuduClient>* client);
 
-// Creates a Kudu client connected to the cluster whose master addresses are defined by
+// Creates a Kudu client connected to the cluster whose master addresses are specified by
 // the kMasterAddressesArg argument in 'context'.
 Status CreateKuduClient(const RunnerContext& context,
                         client::sp::shared_ptr<client::KuduClient>* client);
 
+// Parses 'master_addresses_arg' from 'context' into 'master_addresses_str', a
+// comma-separated string of host/port pairs.
+//
+// If 'master_addresses_arg' starts with a '@' it is interpreted as a cluster name and
+// resolved against a config file in ${KUDU_CONFIG}/kudurc with content like:
+//
+// clusters_info:
+//   cluster1:
+//     master_addresses: ip1:port1,ip2:port2,ip3:port3
+//   cluster2:
+//     master_addresses: ip4:port4
+Status ParseMasterAddressesStr(
+    const RunnerContext& context,
+    const char* master_addresses_arg,
+    std::string* master_addresses_str);
+
+// Like above, but parse Kudu master addresses into a string according to the
+// kMasterAddressesArg argument in 'context'.
+Status ParseMasterAddressesStr(
+    const RunnerContext& context,
+    std::string* master_addresses_str);
+
+// Like above, but parse Kudu master addresses into a string vector according to the
+// 'master_addresses_arg' argument in 'context'.
+Status ParseMasterAddresses(
+    const RunnerContext& context,
+    const char* master_addresses_arg,
+    std::vector<std::string>* master_addresses);
+
+// Like above, but parse Kudu master addresses into a string vector according to the
+// kMasterAddressesArg argument in 'context'.
+Status ParseMasterAddresses(
+    const RunnerContext& context,
+    std::vector<std::string>* master_addresses);
+
 // A table of data to present to the user.
 //
 // Supports formatting based on the --format flag.
diff --git a/src/kudu/tools/tool_action_hms.cc b/src/kudu/tools/tool_action_hms.cc
index 84ad849..b4bd3e3 100644
--- a/src/kudu/tools/tool_action_hms.cc
+++ b/src/kudu/tools/tool_action_hms.cc
@@ -102,7 +102,8 @@ Status Init(const RunnerContext& context,
             shared_ptr<KuduClient>* kudu_client,
             unique_ptr<HmsCatalog>* hms_catalog,
             string* master_addrs) {
-  const string& master_addrs_flag = FindOrDie(context.required_args, kMasterAddressesArg);
+  string master_addrs_flag;
+  RETURN_NOT_OK(ParseMasterAddressesStr(context, &master_addrs_flag));
 
   // Create a Kudu Client.
   RETURN_NOT_OK(KuduClientBuilder()
@@ -681,7 +682,8 @@ Status FixHmsMetadata(const RunnerContext& context) {
 }
 
 Status Precheck(const RunnerContext& context) {
-  const string& master_addrs = FindOrDie(context.required_args, kMasterAddressesArg);
+  string master_addrs;
+  RETURN_NOT_OK(ParseMasterAddressesStr(context, &master_addrs));
   shared_ptr<KuduClient> client;
   RETURN_NOT_OK(KuduClientBuilder()
       .default_rpc_timeout(MonoDelta::FromMilliseconds(FLAGS_timeout_ms))
diff --git a/src/kudu/tools/tool_action_master.cc b/src/kudu/tools/tool_action_master.cc
index 015037a..9be7f3e 100644
--- a/src/kudu/tools/tool_action_master.cc
+++ b/src/kudu/tools/tool_action_master.cc
@@ -263,10 +263,8 @@ Status ResetAuthzCacheAtMaster(const string& master_address) {
 }
 
 Status ResetAuthzCache(const RunnerContext& context) {
-  const string& master_addresses_str =
-      FindOrDie(context.required_args, kMasterAddressesArg);
-  const vector<string>& master_addresses =
-      strings::Split(master_addresses_str, ",");
+  vector<string> master_addresses;
+  RETURN_NOT_OK(ParseMasterAddresses(context, &master_addresses));
 
   if (!FLAGS_force) {
     // Make sure the list of master addresses specified for the command
diff --git a/src/kudu/tools/tool_action_perf.cc b/src/kudu/tools/tool_action_perf.cc
index 5fe4730..d763880 100644
--- a/src/kudu/tools/tool_action_perf.cc
+++ b/src/kudu/tools/tool_action_perf.cc
@@ -818,10 +818,7 @@ unique_ptr<Mode> BuildPerfMode() {
           "an existing or auto-created table as fast as possible. "
           "If requested, also scan the inserted rows to check whether the "
           "actual count of inserted rows matches the expected one.")
-      .AddRequiredParameter({ kMasterAddressesArg,
-          "Comma-separated list of master addresses to run against. "
-          "Addresses are in 'hostname:port' form where port may be omitted "
-          "if a master server listens at the default port." })
+      .AddRequiredParameter({ kMasterAddressesArg, kMasterAddressesArgDesc })
       .AddOptionalParameter("auto_database")
       .AddOptionalParameter("buffer_flush_watermark_pct")
       .AddOptionalParameter("buffer_size_bytes")
diff --git a/src/kudu/tools/tool_action_table.cc b/src/kudu/tools/tool_action_table.cc
index 5854858..8502dc8 100644
--- a/src/kudu/tools/tool_action_table.cc
+++ b/src/kudu/tools/tool_action_table.cc
@@ -138,7 +138,6 @@ const char* const kNewTableNameArg = "new_table_name";
 const char* const kColumnNameArg = "column_name";
 const char* const kNewColumnNameArg = "new_column_name";
 const char* const kKeyArg = "primary_key";
-const char* const kDestMasterAddressesArg = "dest_master_addresses";
 
 Status DeleteTable(const RunnerContext& context) {
   const string& table_name = FindOrDie(context.required_args, kTableNameArg);
@@ -382,9 +381,9 @@ Status RenameColumn(const RunnerContext& context) {
 }
 
 Status ListTables(const RunnerContext& context) {
-  const string& master_addresses_str = FindOrDie(context.required_args,
-                                                 kMasterAddressesArg);
-  return TableLister::ListTablets(Split(master_addresses_str, ","));
+  vector<string> master_addresses;
+  RETURN_NOT_OK(ParseMasterAddresses(context, &master_addresses));
+  return TableLister::ListTablets(master_addresses);
 }
 
 Status ScanTable(const RunnerContext &context) {
@@ -503,13 +502,9 @@ unique_ptr<Mode> BuildTableMode() {
                         "could have different partition schemas. Alternatively, the tool
can "
                         "create the new table using the same table and partition schema as
the "
                         "source table.")
-      .AddRequiredParameter({ kMasterAddressesArg,
-                              "Comma-separated list of Kudu Master addresses (source) "
-                              "where each address is of form 'hostname:port'" })
+      .AddRequiredParameter({ kMasterAddressesArg, kMasterAddressesArgDesc })
       .AddRequiredParameter({ kTableNameArg, "Name of the source table" })
-      .AddRequiredParameter({ kDestMasterAddressesArg,
-                              "Comma-separated list of Kudu Master addresses (destination)
"
-                              "where each address is of form 'hostname:port'" })
+      .AddRequiredParameter({ kDestMasterAddressesArg, kDestMasterAddressesArgDesc })
       .AddOptionalParameter("create_table")
       .AddOptionalParameter("dst_table")
       .AddOptionalParameter("num_threads")
diff --git a/src/kudu/tools/tool_action_tablet.cc b/src/kudu/tools/tool_action_tablet.cc
index f38c084..41c282b 100644
--- a/src/kudu/tools/tool_action_tablet.cc
+++ b/src/kudu/tools/tool_action_tablet.cc
@@ -106,9 +106,6 @@ Status WaitForCleanKsck(const vector<string>& master_addresses,
 }
 
 Status ChangeConfig(const RunnerContext& context, ChangeConfigType cc_type) {
-  const string& master_addresses_str = FindOrDie(context.required_args,
-                                                 kMasterAddressesArg);
-  vector<string> master_addresses = Split(master_addresses_str, ",");
   const string& tablet_id = FindOrDie(context.required_args, kTabletIdArg);
   const string& replica_uuid = FindOrDie(context.required_args, kTsUuidArg);
   boost::optional<RaftPeerPB::MemberType> member_type;
@@ -123,6 +120,8 @@ Status ChangeConfig(const RunnerContext& context, ChangeConfigType
cc_type) {
     member_type = member_type_val;
   }
 
+  vector<string> master_addresses;
+  RETURN_NOT_OK(ParseMasterAddresses(context, &master_addresses));
   return DoChangeConfig(master_addresses, tablet_id, replica_uuid, member_type, cc_type);
 }
 
@@ -139,9 +138,9 @@ Status RemoveReplica(const RunnerContext& context) {
 }
 
 Status LeaderStepDown(const RunnerContext& context) {
-  const string& master_addresses_str = FindOrDie(context.required_args,
-                                                 kMasterAddressesArg);
-  vector<string> master_addresses = Split(master_addresses_str, ",");
+  vector<string> master_addresses;
+  RETURN_NOT_OK(ParseMasterAddresses(context, &master_addresses));
+
   const string& tablet_id = FindOrDie(context.required_args, kTabletIdArg);
   const LeaderStepDownMode mode = FLAGS_abrupt ? LeaderStepDownMode::ABRUPT :
                                                  LeaderStepDownMode::GRACEFUL;
@@ -213,8 +212,8 @@ Status WaitForMoveToComplete(const vector<string>& master_addresses,
 }
 
 Status MoveReplica(const RunnerContext& context) {
-  const vector<string> master_addresses = Split(
-        FindOrDie(context.required_args, kMasterAddressesArg), ",");
+  vector<string> master_addresses;
+  RETURN_NOT_OK(ParseMasterAddresses(context, &master_addresses));
   const string& tablet_id = FindOrDie(context.required_args, kTabletIdArg);
   const string& from_ts_uuid = FindOrDie(context.required_args, kFromTsUuidArg);
   const string& to_ts_uuid = FindOrDie(context.required_args, kToTsUuidArg);


Mime
View raw message