[kudu] 02/02: [master] Check for 'last_known_addr' field when adding master

[ba...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

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

commit ed3d916e92decf7333f4200e88c93fbcc211404d
Author: Bankim Bhavsar <ba...@cloudera.com>
AuthorDate: Thu Mar 11 15:49:28 2021 -0800

    [master] Check for 'last_known_addr' field when adding master
    
    In a single master configuration, the 'last_known_addr' field
    may not be populated in the Raft config and Raft implementation will
    throw an error on adding a new master which is not actionable.
    
    Hence adding an explicit check with an actionable error message
    to fix the problem.
    
    Change-Id: Ic991c82acbb2a2149f647b5c1a5b323fecb958cb
    Reviewed-on: http://gerrit.cloudera.org:8080/17176
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
    Tested-by: Kudu Jenkins
---
 src/kudu/master/dynamic_multi_master-test.cc |  3 ++-
 src/kudu/master/master.cc                    | 20 +++++++++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/kudu/master/dynamic_multi_master-test.cc b/src/kudu/master/dynamic_multi_master-test.cc
index f1e051d..09a1d7b 100644
--- a/src/kudu/master/dynamic_multi_master-test.cc
+++ b/src/kudu/master/dynamic_multi_master-test.cc
@@ -1127,7 +1127,8 @@ TEST_F(DynamicMultiMasterTest, TestAddMasterWithNoLastKnownAddr) {
   string err;
   Status actual = AddMasterToClusterUsingCLITool(reserved_hp_, &err);
   ASSERT_TRUE(actual.IsRuntimeError()) << actual.ToString();
-  ASSERT_STR_MATCHES(err, "Invalid config to set as pending: Peer:.* has no address");
+  ASSERT_STR_MATCHES(err, "'last_known_addr' field in single master Raft configuration not set. "
+                          "Please restart master with --master_addresses flag");
 
   // Verify no change in number of masters.
   NO_FATALS(VerifyVoterMasters(orig_num_masters_));
diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc
index 68552da..7b9dd47 100644
--- a/src/kudu/master/master.cc
+++ b/src/kudu/master/master.cc
@@ -485,7 +485,7 @@ Status Master::GetMasterHostPorts(vector<HostPort>* hostports, MasterType type)
 
   hostports->clear();
   consensus::RaftConfigPB config = consensus->CommittedConfig();
-  for (const auto& peer : *config.mutable_peers()) {
+  for (const auto& peer : config.peers()) {
     if (type == MasterType::ALL || get_raft_member_type(type) == peer.member_type()) {
       // In non-distributed master configurations, we may not store our own
       // last known address in the Raft config. So, we'll fill it in from
@@ -512,6 +512,24 @@ Status Master::AddMaster(const HostPort& hp, rpc::RpcContext* rpc) {
     return Status::AlreadyPresent("Master already present");
   }
 
+  // If a new master is being added to a single master configuration, check that
+  // the current leader master has the 'last_known_addr' populated. Otherwise
+  // Raft will return an error which isn't very actionable.
+  if (masters.size() == 1) {
+    auto consensus = catalog_manager_->master_consensus();
+    if (!consensus) {
+      return Status::IllegalState("consensus not running");
+    }
+    const auto& config = consensus->CommittedConfig();
+    DCHECK_EQ(1, config.peers_size());
+    if (!config.peers(0).has_last_known_addr()) {
+      return Status::IllegalState("'last_known_addr' field in single master Raft configuration not "
+                                  "set. Please restart master with --master_addresses flag set "
+                                  "to the single master which will populate the 'last_known_addr' "
+                                  "field.");
+    }
+  }
+
   // Check whether the master to be added is reachable and fetch its uuid.
   ServerEntryPB peer_entry;
   RETURN_NOT_OK(GetMasterEntryForHost(messenger_, hp, &peer_entry));