[BACKPORT 2024.1][#19848] Tests: map yb_backup.py commands to yb cont…

…roller

Summary:
Original commit: 6da7e87/D33204

Primary goal is to run backups in the Cpp UTs via YB Controller.
This diff has the following changes
1. Add helper functions to run backups via YB Controller.

2. Fix a lot of the backup invocations to make the restore work via YB Controller.
3. Disable tests which are not supported via YB Controller.
Jira: DB-8791

Test Plan:
Ran few UTs locally to verify backup/restore works
e.g. `YB_TEST_YB_CONTROLLER=1 ./yb_build.sh --cxx-test tools_yb-backup-cross-feature-test --gtest_filter YBBackupTest.TestPreSplitYSQLRangeSplitTableAndIndex`

Monitor the detective results to fix broken tests.

Reviewers: loginov

Reviewed By: loginov

Subscribers: ybase, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D35741

build-support/jenkins/test.sh

@@ -220,7 +220,7 @@ if [[ -n ${ybc_tar} ]]; then
   tar xf "$ybc_tar"
   cp ./ybc-*/bin/* "${ybc_dest}/"
   ( set -x; ls -l "${ybc_dest}/")
-  export YB_TEST_YB_CONTROLLER=1
+  export YB_TEST_YB_CONTROLLER=0
   export YB_DISABLE_MINICLUSTER_BACKUP_TESTS=1
 else
   log "Did not find YBC tarfile. Not setting YB_TEST_YB_CONTROLLER."

src/yb/integration-tests/cql_test_base.cc

@@ -139,6 +139,9 @@ Status CqlTestBase<MiniCluster>::StartCluster() {
 
 template <>
 Status CqlTestBase<MiniCluster>::RunBackupCommand(const vector<string>& args) {
+  if (UseYbController()) {
+    return tools::RunYbControllerCommand(cluster_.get(), *tmp_dir_, args);
+  }
   return tools::RunBackupCommand(
       HostPort(), // Not used YSQL host/port.
       cluster_->GetMasterAddresses(), cluster_->GetTserverHTTPAddresses(), *tmp_dir_, args);

src/yb/integration-tests/external_mini_cluster.cc

@@ -94,6 +94,7 @@
 #include "yb/util/pb_util.h"
 #include "yb/util/size_literals.h"
 #include "yb/util/slice.h"
+#include "yb/util/status.h"
 #include "yb/util/status_format.h"
 #include "yb/util/status_log.h"
 #include "yb/util/stopwatch.h"
@@ -406,37 +407,6 @@ Status ExternalMiniCluster::Start(rpc::Messenger* messenger) {
     }
     RETURN_NOT_OK(WaitForTabletServerCount(
         opts_.num_tablet_servers, kTabletServerRegistrationTimeout));
-
-    if (UseYbController()) {
-      vector<string> extra_flags;
-      for (const auto& flag : opts_.extra_tserver_flags) {
-        if (flag.find("certs_dir") != string::npos) {
-          extra_flags.push_back("--certs_dir_name" + flag.substr(flag.find("=")));
-        }
-      }
-      // we need 1 yb controller server for each tserver
-      // yb controller uses the same IP as corresponding tserver
-      yb_controller_servers_.reserve(opts_.num_tablet_servers);
-      // all yb controller servers need to be on the same port
-      const auto server_port = AllocateFreePort();
-      for (size_t i = 0; i < opts_.num_tablet_servers; ++i) {
-        const auto yb_controller_log_dir = GetDataPath(Format("ybc-$0/logs", i));
-        const auto yb_controller_tmp_dir = GetDataPath(Format("ybc-$0/tmp", i));
-        RETURN_NOT_OK(Env::Default()->CreateDirs(yb_controller_log_dir));
-        RETURN_NOT_OK(Env::Default()->CreateDirs(yb_controller_tmp_dir));
-        scoped_refptr<ExternalYbController> yb_controller = new ExternalYbController(
-            yb_controller_log_dir, yb_controller_tmp_dir, tablet_servers_[i]->bind_host(),
-            GetToolPath("yb-admin"), GetToolPath("../../../bin", "yb-ctl"),
-            GetToolPath("../../../bin", "ycqlsh"), GetPgToolPath("ysql_dump"),
-            GetPgToolPath("ysql_dumpall"), GetPgToolPath("ysqlsh"), server_port,
-            masters_[0]->http_port(), tablet_servers_[i]->http_port(),
-            tablet_servers_[i]->bind_host(), GetYbcToolPath("yb-controller-server"), extra_flags);
-
-        RETURN_NOT_OK_PREPEND(
-            yb_controller->Start(), "Failed to start YB Controller at index " + std::to_string(i));
-        yb_controller_servers_.push_back(yb_controller);
-      }
-    }
   } else {
     LOG(INFO) << "No need to start tablet servers";
   }
@@ -1489,6 +1459,53 @@ Status ExternalMiniCluster::AddTabletServer(
       master_hostports, SubstituteInFlags(flags, idx));
   RETURN_NOT_OK(ts->Start(start_cql_proxy));
   tablet_servers_.push_back(ts);
+
+  // add new yb controller for the new ts
+  if (UseYbController()) {
+    RETURN_NOT_OK(AddYbControllerServer(ts));
+  }
+
+  return Status::OK();
+}
+
+Status ExternalMiniCluster::AddYbControllerServer(const scoped_refptr<ExternalTabletServer> ts) {
+  // Return if we already have a Yb Controller for the given ts
+  for (auto ybController : yb_controller_servers_) {
+    if (ybController->GetServerAddress() == ts->bind_host()) {
+      return Status::OK();
+    }
+  }
+
+  size_t idx = yb_controller_servers_.size() + 1;
+  vector<string> extra_flags;
+  for (const auto& flag : opts_.extra_tserver_flags) {
+    if (flag.find("certs_dir") != string::npos) {
+      extra_flags.push_back("--certs_dir_name" + flag.substr(flag.find("=")));
+    }
+  }
+
+  // all yb controller servers need to be on the same port
+  uint16_t server_port;
+  if (idx == 1) {
+    server_port = AllocateFreePort();
+  } else {
+    server_port = yb_controller_servers_[0]->GetServerPort();
+  }
+  const auto yb_controller_log_dir = GetDataPath(Format("ybc-$0/logs", idx));
+  const auto yb_controller_tmp_dir = GetDataPath(Format("ybc-$0/tmp", idx));
+  RETURN_NOT_OK(Env::Default()->CreateDirs(yb_controller_log_dir));
+  RETURN_NOT_OK(Env::Default()->CreateDirs(yb_controller_tmp_dir));
+  scoped_refptr<ExternalYbController> yb_controller = new ExternalYbController(
+      idx, yb_controller_log_dir, yb_controller_tmp_dir, ts->bind_host(), GetToolPath("yb-admin"),
+      GetToolPath("../../../bin", "yb-ctl"), GetToolPath("../../../bin", "ycqlsh"),
+      GetPgToolPath("ysql_dump"), GetPgToolPath("ysql_dumpall"), GetPgToolPath("ysqlsh"),
+      server_port, masters_[0]->http_port(), ts->http_port(), ts->bind_host(),
+      GetYbcToolPath("yb-controller-server"), extra_flags);
+
+  RETURN_NOT_OK_PREPEND(
+      yb_controller->Start(), "Failed to start YB Controller at index " + std::to_string(idx));
+  yb_controller_servers_.push_back(yb_controller);
+
   return Status::OK();
 }
 
@@ -1901,15 +1918,6 @@ std::vector<ExternalTabletServer*> ExternalMiniCluster::tserver_daemons() const
   return result;
 }
 
-vector<ExternalYbController*> ExternalMiniCluster::yb_controller_daemons() const {
-  vector<ExternalYbController*> results;
-  results.reserve(yb_controller_servers_.size());
-  for (const scoped_refptr<ExternalYbController>& yb_controller : yb_controller_servers_) {
-    results.push_back(yb_controller.get());
-  }
-  return results;
-}
-
 HostPort ExternalMiniCluster::pgsql_hostport(int node_index) const {
   return HostPort(tablet_servers_[node_index]->bind_host(),
                   tablet_servers_[node_index]->pgsql_rpc_port());

src/yb/integration-tests/external_mini_cluster.h

@@ -248,6 +248,9 @@ class ExternalMiniCluster : public MiniClusterBase {
       const std::vector<std::string>& extra_flags = {},
       int num_drives = -1);
 
+  // Add a new YB Controller server for the given tablet server
+  Status AddYbControllerServer(const scoped_refptr<ExternalTabletServer>);
+
   void UpdateMasterAddressesOnTserver();
 
   // Shuts down the whole cluster or part of it, depending on the selected 'mode'.  Currently, this
@@ -374,7 +377,9 @@ class ExternalMiniCluster : public MiniClusterBase {
   std::vector<ExternalTabletServer*> tserver_daemons() const;
 
   // Return all YBController servers.
-  std::vector<ExternalYbController*> yb_controller_daemons() const;
+  std::vector<scoped_refptr<ExternalYbController>> yb_controller_daemons() const override {
+    return yb_controller_servers_;
+  }
 
   // Get tablet server host.
   HostPort pgsql_hostport(int node_index) const;

src/yb/integration-tests/external_yb_controller.cc

@@ -17,19 +17,21 @@
 
 #include "yb/util/path_util.h"
 #include "yb/util/status.h"
+#include "yb/util/status_format.h"
 #include "yb/util/test_util.h"
 
 using std::string;
 
 namespace yb {
 
 ExternalYbController::ExternalYbController(
-    const string& log_dir, const string& tmp_dir, const string& yb_tserver_address,
-    const string& yb_admin, const string& yb_ctl, const string& ycqlsh, const string& ysql_dump,
-    const string& ysql_dumpall, const string& ysqlsh, uint16_t server_port,
-    uint16_t yb_master_webserver_port, uint16_t yb_tserver_webserver_port,
+    const size_t idx, const string& log_dir, const string& tmp_dir,
+    const string& yb_tserver_address, const string& yb_admin, const string& yb_ctl,
+    const string& ycqlsh, const string& ysql_dump, const string& ysql_dumpall, const string& ysqlsh,
+    uint16_t server_port, uint16_t yb_master_webserver_port, uint16_t yb_tserver_webserver_port,
     const string& server_address, const string& exe, const std::vector<string>& extra_flags)
-    : exe_(exe),
+    : idx_(idx),
+      exe_(exe),
       log_dir_(log_dir),
       tmp_dir_(tmp_dir),
       server_address_(server_address),
@@ -62,6 +64,7 @@ Status ExternalYbController::Start() {
   argv.push_back("--ysql_dump=" + ysql_dump_);
   argv.push_back("--ysql_dumpall=" + ysql_dumpall_);
   argv.push_back("--ysqlsh=" + ysqlsh_);
+  argv.push_back("--logtostderr");
   argv.push_back(Format("--server_port=$0", server_port_));
   argv.push_back(Format("--yb_master_webserver_port=$0", yb_master_webserver_port_));
   argv.push_back(Format("--yb_tserver_webserver_port=$0", yb_tserver_webserver_port_));
@@ -144,8 +147,11 @@ void ExternalYbController::Shutdown() {
     WARN_NOT_OK(process_->Kill(SIGKILL), "Killing YB Controller process failed");
   }
 
-  // Delete the tmp directory.
-  CHECK_OK(Env::Default()->DeleteRecursively(tmp_dir_));
+  // Delete the tmp directory if present.
+  status = (DeleteIfExists(tmp_dir_, Env::Default()));
+  if (!status.ok()) {
+    LOG(WARNING) << "Error while deleting YB Controller temp dir: " << status;
+  }
 
   process_ = nullptr;
 }
@@ -154,6 +160,51 @@ bool ExternalYbController::IsShutdown() const {
   return process_.get() == nullptr;
 }
 
+Status ExternalYbController::RunBackupCommand(
+    const string& backup_dir, const string& backup_command, const string& ns, const string& ns_type,
+    const string& temp_dir, const bool use_tablespaces) {
+  std::vector<string> argv;
+  string bucket = BaseName(backup_dir);
+
+  // First the path to YB Controller CLI tool.
+  argv.push_back(GetYbcToolPath("yb-controller-cli"));
+  argv.push_back(backup_command);
+  argv.push_back("--bucket=" + bucket);
+  argv.push_back("--cloud_dir=yugabyte");
+  argv.push_back("--cloud_type=nfs");
+  argv.push_back("--ns_type=" + ns_type);
+  argv.push_back("--ns=" + ns);
+  argv.push_back("--wait");
+  argv.insert(argv.end(), extra_flags_.begin(), extra_flags_.end());
+  argv.push_back("--tserver_ip=" + server_address_);
+  argv.push_back(Format("--server_port=$0", server_port_));
+  argv.push_back("--max_timeout_secs=180");
+
+  if (use_tablespaces) {
+    argv.push_back("--use_tablespaces");
+  }
+
+  LOG(INFO) << "Setting YBC_NFS_DIR as " << temp_dir;
+  setenv("YBC_NFS_DIR", temp_dir.c_str(), true);
+
+  if (backup_command == "backup") {
+    RETURN_NOT_OK(Env::Default()->CreateDirs(backup_dir));
+  }
+
+  LOG(INFO) << "Run YB Controller CLI: " << AsString(argv);
+
+  string output;
+  auto status = Subprocess::Call(argv, &output);
+  LOG(INFO) << "YB Controller " << backup_command << " status: " << status << " output: " << output;
+
+  if (output.find("Final Status: OK") == string::npos) {
+    return STATUS_FORMAT(
+        InternalError,
+        "YB Controller " + backup_command + " command failed with output: " + output);
+  }
+  return Status::OK();
+}
+
 ExternalYbController::~ExternalYbController() {
   Shutdown();
 }

src/yb/integration-tests/external_yb_controller.h

@@ -24,15 +24,19 @@ namespace yb {
 class ExternalYbController : public RefCountedThreadSafe<ExternalYbController> {
  public:
   ExternalYbController(
-      const std::string& log_dir, const std::string& tmp_dir, const std::string& yb_tserver_address,
-      const std::string& yb_admin, const std::string& yb_ctl, const std::string& ycqlsh,
-      const std::string& ysql_dump, const std::string& ysql_dumpall, const std::string& ysqlsh,
-      uint16_t server_port, uint16_t yb_master_webserver_port, uint16_t yb_tserver_webserver_port,
-      const std::string& server_address, const std::string& exe,
+      const size_t idx, const std::string& log_dir, const std::string& tmp_dir,
+      const std::string& yb_tserver_address, const std::string& yb_admin, const std::string& yb_ctl,
+      const std::string& ycqlsh, const std::string& ysql_dump, const std::string& ysql_dumpall,
+      const std::string& ysqlsh, uint16_t server_port, uint16_t yb_master_webserver_port,
+      uint16_t yb_tserver_webserver_port, const std::string& server_address, const std::string& exe,
       const std::vector<std::string>& extra_flags);
 
   Status Start();
 
+  Status RunBackupCommand(
+      const std::string& backup_dir, const std::string& backup_command, const std::string& ns,
+      const std::string& ns_type, const std::string& temp_dir, const bool use_tablespaces);
+
   // Ping the YB Controller server.
   Status ping();
 
@@ -55,6 +59,7 @@ class ExternalYbController : public RefCountedThreadSafe<ExternalYbController> {
 
  private:
   std::unique_ptr<Subprocess> process_;
+  const size_t idx_;
   const std::string exe_;
   const std::string log_dir_;
   const std::string tmp_dir_;

src/yb/integration-tests/mini_cluster.cc

@@ -271,7 +271,7 @@ Status MiniCluster::StartAsync(
       RETURN_NOT_OK(Env::Default()->CreateDirs(yb_controller_tmp_dir));
       const auto server_address = mini_tablet_servers_[i]->bound_http_addr().address().to_string();
       scoped_refptr<ExternalYbController> yb_controller = new ExternalYbController(
-          yb_controller_log_dir, yb_controller_tmp_dir, server_address, GetToolPath("yb-admin"),
+          i, yb_controller_log_dir, yb_controller_tmp_dir, server_address, GetToolPath("yb-admin"),
           GetToolPath("../../../bin", "yb-ctl"), GetToolPath("../../../bin", "ycqlsh"),
           GetPgToolPath("ysql_dump"), GetPgToolPath("ysql_dumpall"), GetPgToolPath("ysqlsh"),
           server_port, master_web_ports_[0], tserver_web_ports_[i], server_address,

src/yb/integration-tests/mini_cluster.h

@@ -45,6 +45,7 @@
 
 #include "yb/gutil/macros.h"
 
+#include "yb/gutil/ref_counted.h"
 #include "yb/integration-tests/external_yb_controller.h"
 #include "yb/integration-tests/mini_cluster_base.h"
 
@@ -221,6 +222,11 @@ class MiniCluster : public MiniClusterBase {
 
   std::vector<std::shared_ptr<tablet::TabletPeer>> GetTabletPeers(size_t idx);
 
+  // Return all YBController servers.
+  std::vector<scoped_refptr<ExternalYbController>> yb_controller_daemons() const override {
+    return yb_controllers_;
+  }
+
   tserver::TSTabletManager* GetTabletManager(size_t idx);
 
   // Wait for the given tablet to have 'expected_count' replicas

src/yb/integration-tests/mini_cluster_base.h

@@ -21,6 +21,8 @@
 
 namespace yb {
 
+class ExternalYbController;
+
 namespace client {
 class YBClientBuilder;
 class YBClient;
@@ -62,6 +64,8 @@ class MiniClusterBase {
     return client;
   }
 
+  virtual std::vector<scoped_refptr<ExternalYbController>> yb_controller_daemons() const = 0;
+
  protected:
   virtual ~MiniClusterBase() = default;
 

src/yb/tools/tools_test_utils.cc

@@ -13,6 +13,7 @@
 
 #include "yb/tools/tools_test_utils.h"
 
+#include "yb/integration-tests/mini_cluster_base.h"
 #include "yb/util/jsonreader.h"
 #include "yb/util/net/net_util.h"
 #include "yb/util/path_util.h"
@@ -21,6 +22,7 @@
 #include "yb/util/subprocess.h"
 #include "yb/util/test_util.h"
 #include "yb/util/flags.h"
+#include "yb/integration-tests/external_yb_controller.h"
 
 using std::string;
 
@@ -103,6 +105,36 @@ Status RunBackupCommand(
   return Status::OK();
 }
 
+Status RunYbControllerCommand(
+    MiniClusterBase* cluster, const std::string& tmp_dir, const std::vector<std::string>& args) {
+  string backupDir, ns, ns_type, backup_command;
+  auto yb_controller = cluster->yb_controller_daemons()[0].get();
+  bool use_tablespaces = false;
+  for (size_t idx = 0; idx < args.size(); idx++) {
+    string arg = args[idx];
+    if (arg == "--keyspace") {
+      string keyspace = args[idx + 1];
+      if (keyspace.starts_with("ysql.")) {
+        ns_type = "ysql";
+        ns = keyspace.substr(keyspace.find(".") + 1);
+      } else {
+        ns_type = "ycql";
+        ns = keyspace;
+      }
+    } else if (arg == "--backup_location") {
+      backupDir = args[idx + 1];
+    } else if (arg == "create") {
+      backup_command = "backup";
+    } else if (arg == "restore") {
+      backup_command = arg;
+    } else if (arg == "--use_tablespaces") {
+      use_tablespaces = true;
+    }
+  }
+  return yb_controller->RunBackupCommand(
+      backupDir, backup_command, ns, ns_type, tmp_dir, use_tablespaces);
+}
+
 TmpDirProvider::~TmpDirProvider() {
   if (!dir_.empty()) {
     LOG(INFO) << "Deleting temporary folder: " << dir_;