tool: port kudu-admin to 'kudu table' and 'kudu tablet'

The config change operations go to the revived 'tablet' mode, which is now
"operations against remote Kudu tablets". The table operations go to
'kudu table'.

In doing so, the ported functionality can now also work in multi-master
mode; previously, the ClusterAdminClient refused to accept multiple masters.

I opted to group the config change operations as a common mode rather than
an additional parameter. I think this makes them more discoverable.

Change-Id: Id6e96fc5b0106946f29a2faee8e2667be738b740
Reviewed-on: http://gerrit.cloudera.org:8080/4180
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <[email protected]>

build-support/dist_test.py

@@ -77,9 +77,8 @@
      #".../example-deletes.txt",
      #".../example-tweets.txt",
 
-     # Tests that require tooling require these.
+     # Tests that require tooling require this.
      "build/latest/bin/kudu",
-     "build/latest/bin/kudu-admin",
      ]
 
 

docs/release_notes.adoc

@@ -61,6 +61,9 @@ Kudu 1.0.0 are not supported.
 - The `log-dump` tool has been removed. The same functionality is now
   implemented as `kudu wal dump` and `kudu local_replica dump_wals`.
 
+- The `kudu-admin` tool has been removed. The same functionality is now
+  implemented within `kudu table` and `kudu tablet`.
+
 - KuduSession methods in the C++ library are no longer advertised as thread-safe
   to have one set of semantics for both C++ and Java Kudu client libraries.
 

src/kudu/tools/CMakeLists.txt

@@ -44,10 +44,6 @@ target_link_libraries(insert-generated-rows
   kudu_tools_util
   ${LINK_LIBS})
 
-add_executable(kudu-admin kudu-admin.cc)
-target_link_libraries(kudu-admin
-  ${LINK_LIBS})
-
 add_executable(kudu-ts-cli ts-cli.cc)
 target_link_libraries(kudu-ts-cli
   ${LINK_LIBS})
@@ -91,6 +87,8 @@ add_executable(kudu
   tool_action_fs.cc
   tool_action_local_replica.cc
   tool_action_pbc.cc
+  tool_action_table.cc
+  tool_action_tablet.cc
   tool_action_wal.cc
   tool_main.cc
 )
@@ -117,7 +115,7 @@ ADD_KUDU_TEST(ksck-test)
 ADD_KUDU_TEST(ksck_remote-test RESOURCE_LOCK "master-rpc-ports")
 ADD_KUDU_TEST(kudu-admin-test)
 ADD_KUDU_TEST_DEPENDENCIES(kudu-admin-test
-  kudu-admin)
+  kudu)
 ADD_KUDU_TEST(kudu-tool-test)
 ADD_KUDU_TEST_DEPENDENCIES(kudu-tool-test
   kudu)

src/kudu/tools/kudu-admin-test.cc

@@ -14,8 +14,9 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-//
-// Tests for the kudu-admin command-line tool.
+
+#include <string>
+#include <vector>
 
 #include <gtest/gtest.h>
 
@@ -35,13 +36,15 @@ using client::KuduClientBuilder;
 using client::sp::shared_ptr;
 using itest::TabletServerMap;
 using itest::TServerDetails;
+using std::string;
+using std::vector;
 using strings::Substitute;
 
-static const char* const kAdminToolName = "kudu-admin";
+static const char* const kAdminToolName = "kudu";
 
 class AdminCliTest : public tserver::TabletServerIntegrationTestBase {
  protected:
-  // Figure out where the admin tool is.
+  // Figure out where the tool is.
   string GetAdminToolPath() const;
 };
 
@@ -50,24 +53,22 @@ string AdminCliTest::GetAdminToolPath() const {
   CHECK_OK(Env::Default()->GetExecutablePath(&exe));
   string binroot = DirName(exe);
   string tool_path = JoinPathSegments(binroot, kAdminToolName);
-  CHECK(Env::Default()->FileExists(tool_path)) << "kudu-admin tool not found at " << tool_path;
+  CHECK(Env::Default()->FileExists(tool_path))
+      << kAdminToolName << " tool not found at " << tool_path;
   return tool_path;
 }
 
-// Test kudu-admin config change while running a workload.
+// Test config change while running a workload.
 // 1. Instantiate external mini cluster with 3 TS.
 // 2. Create table with 2 replicas.
-// 3. Invoke kudu-admin CLI to invoke a config change.
+// 3. Invoke CLI to trigger a config change.
 // 4. Wait until the new server bootstraps.
 // 5. Profit!
 TEST_F(AdminCliTest, TestChangeConfig) {
   FLAGS_num_tablet_servers = 3;
   FLAGS_num_replicas = 2;
-
-  vector<string> ts_flags, master_flags;
-  ts_flags.push_back("--enable_leader_failure_detection=false");
-  master_flags.push_back("--catalog_manager_wait_for_new_tablets_to_elect_leader=false");
-  BuildAndStart(ts_flags, master_flags);
+  BuildAndStart({ "--enable_leader_failure_detection=false" },
+                { "--catalog_manager_wait_for_new_tablets_to_elect_leader=false" });
 
   vector<TServerDetails*> tservers;
   AppendValuesFromMap(tablet_servers_, &tservers);
@@ -110,12 +111,16 @@ TEST_F(AdminCliTest, TestChangeConfig) {
   ASSERT_EQ(leader->uuid(), master_observed_leader->uuid());
 
   LOG(INFO) << "Adding tserver with uuid " << new_node->uuid() << " as VOTER...";
-  string exe_path = GetAdminToolPath();
-  string arg_str = Substitute("$0 -master_addresses $1 change_config $2 ADD_SERVER $3 VOTER",
-                              exe_path,
-                              cluster_->master()->bound_rpc_addr().ToString(),
-                              tablet_id_, new_node->uuid());
-  ASSERT_OK(Subprocess::Call(arg_str));
+  ASSERT_OK(Subprocess::Call({
+    GetAdminToolPath(),
+    "tablet",
+    "change_config",
+    "add_replica",
+    cluster_->master()->bound_rpc_addr().ToString(),
+    tablet_id_,
+    new_node->uuid(),
+    "VOTER"
+  }));
 
   InsertOrDie(&active_tablet_servers, new_node->uuid(), new_node);
   ASSERT_OK(WaitUntilCommittedConfigNumVotersIs(active_tablet_servers.size(),
@@ -144,12 +149,15 @@ TEST_F(AdminCliTest, TestChangeConfig) {
 
   // Now remove the server once again.
   LOG(INFO) << "Removing tserver with uuid " << new_node->uuid() << " from the config...";
-  arg_str = Substitute("$0 -master_addresses $1 change_config $2 REMOVE_SERVER $3",
-                       exe_path,
-                       cluster_->master()->bound_rpc_addr().ToString(),
-                       tablet_id_, new_node->uuid());
-
-  ASSERT_OK(Subprocess::Call(arg_str));
+  ASSERT_OK(Subprocess::Call({
+    GetAdminToolPath(),
+    "tablet",
+    "change_config",
+    "remove_replica",
+    cluster_->master()->bound_rpc_addr().ToString(),
+    tablet_id_,
+    new_node->uuid()
+  }));
 
   ASSERT_EQ(1, active_tablet_servers.erase(new_node->uuid()));
   ASSERT_OK(WaitUntilCommittedConfigNumVotersIs(active_tablet_servers.size(),
@@ -160,31 +168,46 @@ TEST_F(AdminCliTest, TestChangeConfig) {
 TEST_F(AdminCliTest, TestDeleteTable) {
   FLAGS_num_tablet_servers = 1;
   FLAGS_num_replicas = 1;
+  BuildAndStart({}, {});
 
-  vector<string> ts_flags, master_flags;
-  BuildAndStart(ts_flags, master_flags);
   string master_address = cluster_->master()->bound_rpc_addr().ToString();
-
   shared_ptr<KuduClient> client;
-  CHECK_OK(KuduClientBuilder()
-        .add_master_server_addr(master_address)
-        .Build(&client));
-
-  // Default table that gets created;
-  string table_name = "TestTable";
-
-  string exe_path = GetAdminToolPath();
-  string arg_str = Substitute("$0 -master_addresses $1 delete_table $2",
-                              exe_path,
-                              master_address,
-                              table_name);
-
-  ASSERT_OK(Subprocess::Call(arg_str));
+  ASSERT_OK(KuduClientBuilder()
+            .add_master_server_addr(master_address)
+            .Build(&client));
+
+  ASSERT_OK(Subprocess::Call({
+    GetAdminToolPath(),
+    "table",
+    "delete",
+    master_address,
+    kTableId
+  }));
 
   vector<string> tables;
   ASSERT_OK(client->ListTables(&tables));
   ASSERT_TRUE(tables.empty());
 }
 
+TEST_F(AdminCliTest, TestListTables) {
+  FLAGS_num_tablet_servers = 1;
+  FLAGS_num_replicas = 1;
+
+  BuildAndStart({}, {});
+
+  string stdout;
+  ASSERT_OK(Subprocess::Call({
+    GetAdminToolPath(),
+    "table",
+    "list",
+    cluster_->master()->bound_rpc_addr().ToString()
+  }, &stdout, nullptr));
+
+  vector<string> stdout_lines = strings::Split(stdout, ",",
+                                               strings::SkipEmpty());
+  ASSERT_EQ(1, stdout_lines.size());
+  ASSERT_EQ(Substitute("$0\n", kTableId), stdout_lines[0]);
+}
+
 } // namespace tools
 } // namespace kudu