SERVER-21236 Don't automatically add 'configsvr':true to replset config when loading it from disk if started with --configsvr

6 files changed, 173 insertions, 37 deletions

diff --git a/jstests/replsets/config_server_checks.js b/jstests/replsets/config_server_checks.js
index 16ebb70565c..b7627923d8d 100644
--- a/jstests/replsets/config_server_checks.js
+++ b/jstests/replsets/config_server_checks.js
@@ -33,7 +33,7 @@ try {
 } catch (e) {
     // expected since we close all connections after going into REMOVED
 }
-expectState(rst, 10 /*REMOVED*/);
+expectState(rst, rst.REMOVED);
 rst.stopSet();
 })();
 
@@ -52,7 +52,7 @@ conf.configsvr = true;
 assert.commandWorked(rst.nodes[0].adminCommand({replSetInitiate: conf}));
 
 rst.getMaster();
-expectState(rst, 1 /*PRIMARY*/);
+expectState(rst, rst.PRIMARY);
 rst.stopSet();
 })();
 
@@ -72,7 +72,81 @@ conf.configsvr = true;
 assert.commandWorked(rst.nodes[0].adminCommand({replSetInitiate: conf}));
 
 rst.getMaster();
-expectState(rst, 1 /*PRIMARY*/);
+expectState(rst, rst.PRIMARY);
+
+var conf = rst.getMaster().getDB('local').system.replset.findOne();
+assert(conf.configsvr, tojson(conf));
+
+rst.stopSet();
+})();
+
+(function() {
+// Test that node with --configsvr cmd line and initiated with an empty replset config
+// will result in configsvr:true getting automatically added to the config (SERVER-20247).
+jsTestLog("--configsvr cmd line, empty config to replSetInitiate");
+var rst = new ReplSetTest({name: "configrs6",
+                           nodes: 1,
+                           nodeOptions: {configsvr: "",
+                                         journal: "",
+                                         storageEngine: "wiredTiger"}});
+
+rst.startSet();
+assert.commandWorked(rst.nodes[0].adminCommand({replSetInitiate: 1}));
+
+rst.getMaster();
+expectState(rst, rst.PRIMARY);
+rst.stopSet();
+})();
+
+(function() {
+// Test that a set initialized without --configsvr but then restarted with --configsvr will fail to
+// start up and won't automatically add "configsvr" to the replset config (SERVER-21236).
+jsTestLog("set initiated without configsvr, restarted adding --configsvr cmd line");
+var rst = new ReplSetTest({name: "configrs7",
+                           nodes: 1,
+                           nodeOptions: {journal: "",
+                                         storageEngine: "wiredTiger"}});
+
+rst.startSet();
+var conf = rst.getReplSetConfig();
+assert.commandWorked(rst.nodes[0].adminCommand({replSetInitiate: conf}));
+
+rst.getMaster();
+expectState(rst, rst.PRIMARY);
+assert.throws(function() {
+                  rst.restart(0, {configsvr: ""});
+              });
+
+rst.stopSet();
+})();
+
+(function() {
+// Test that a set initialized with --configsvr but then restarted without --configsvr will fail to
+// start up.
+jsTestLog("set initiated with configsvr, restarted without --configsvr cmd line");
+var rst = new ReplSetTest({name: "configrs8",
+                           nodes: 1,
+                           nodeOptions: {configsvr: "",
+                                         journal: "",
+                                         storageEngine: "wiredTiger"}});
+
+rst.startSet();
+var conf = rst.getReplSetConfig();
+conf.configsvr = true;
+assert.commandWorked(rst.nodes[0].adminCommand({replSetInitiate: conf}));
+
+rst.getMaster();
+expectState(rst, rst.PRIMARY);
+
+var node = rst.nodes[0];
+var options = node.savedOptions;
+delete options.configsvr;
+options.noCleanData = true;
+
+MongoRunner.stopMongod(node);
+var conn = MongoRunner.runMongod(options);
+assert.eq(null, conn, "Mongod should have failed to start, but didn't");
+
 rst.stopSet();
 })();
 
diff --git a/src/mongo/db/repl/replica_set_config.cpp b/src/mongo/db/repl/replica_set_config.cpp
index ff3ce971717..106e8329473 100644
--- a/src/mongo/db/repl/replica_set_config.cpp
+++ b/src/mongo/db/repl/replica_set_config.cpp
@@ -79,6 +79,14 @@ const std::string kGetLastErrorModesFieldName = "getLastErrorModes";
 }  // namespace
 
 Status ReplicaSetConfig::initialize(const BSONObj& cfg, bool usePV1ByDefault) {
+    return _initialize(cfg, false, usePV1ByDefault);
+}
+
+Status ReplicaSetConfig::initializeForInitiate(const BSONObj& cfg, bool usePV1ByDefault) {
+    return _initialize(cfg, true, usePV1ByDefault);
+}
+
+Status ReplicaSetConfig::_initialize(const BSONObj& cfg, bool forInitiate, bool usePV1ByDefault) {
     _isInitialized = false;
     _members.clear();
     Status status =
@@ -127,8 +135,10 @@ Status ReplicaSetConfig::initialize(const BSONObj& cfg, bool usePV1ByDefault) {
     //
     // Parse configServer
     //
-    status = bsonExtractBooleanFieldWithDefault(
-        cfg, kConfigServerFieldName, serverGlobalParams.configsvr, &_configServer);
+    status = bsonExtractBooleanFieldWithDefault(cfg,
+                                                kConfigServerFieldName,
+                                                forInitiate ? serverGlobalParams.configsvr : false,
+                                                &_configServer);
     if (!status.isOK()) {
         return status;
     }
diff --git a/src/mongo/db/repl/replica_set_config.h b/src/mongo/db/repl/replica_set_config.h
index 67ac3cfe85d..b02a3feba68 100644
--- a/src/mongo/db/repl/replica_set_config.h
+++ b/src/mongo/db/repl/replica_set_config.h
@@ -72,6 +72,12 @@ public:
     Status initialize(const BSONObj& cfg, bool usePV1ByDefault = false);
 
     /**
+     * Same as the generic initialize() above except will default "configsvr" setting to the value
+     * of serverGlobalParams.configsvr.
+     */
+    Status initializeForInitiate(const BSONObj& cfg, bool usePV1ByDefault = false);
+
+    /**
      * Returns true if this object has been successfully initialized or copied from
      * an initialized object.
      */
@@ -306,6 +312,8 @@ private:
      */
     void _addInternalWriteConcernModes();
 
+    Status _initialize(const BSONObj& cfg, bool forInitiate, bool usePV1ByDefault);
+
     bool _isInitialized = false;
     long long _version = 1;
     std::string _replSetName;
diff --git a/src/mongo/db/repl/replica_set_config_checks_test.cpp b/src/mongo/db/repl/replica_set_config_checks_test.cpp
index 0c52bb8cf8d..e88f69f34f6 100644
--- a/src/mongo/db/repl/replica_set_config_checks_test.cpp
+++ b/src/mongo/db/repl/replica_set_config_checks_test.cpp
@@ -46,26 +46,26 @@ TEST(ValidateConfigForInitiate, VersionMustBe1) {
     rses.addSelf(HostAndPort("h1"));
 
     ReplicaSetConfig config;
-    ASSERT_OK(config.initialize(BSON("_id"
-                                     << "rs0"
-                                     << "version" << 2 << "members"
-                                     << BSON_ARRAY(BSON("_id" << 1 << "host"
-                                                              << "h1")))));
+    ASSERT_OK(config.initializeForInitiate(BSON("_id"
+                                                << "rs0"
+                                                << "version" << 2 << "members"
+                                                << BSON_ARRAY(BSON("_id" << 1 << "host"
+                                                                         << "h1")))));
     ASSERT_EQUALS(ErrorCodes::NewReplicaSetConfigurationIncompatible,
                   validateConfigForInitiate(&rses, config).getStatus());
 }
 
 TEST(ValidateConfigForInitiate, MustFindSelf) {
     ReplicaSetConfig config;
-    ASSERT_OK(
-        config.initialize(BSON("_id"
-                               << "rs0"
-                               << "version" << 1 << "members"
-                               << BSON_ARRAY(BSON("_id" << 1 << "host"
-                                                        << "h1")
-                                             << BSON("_id" << 2 << "host"
-                                                           << "h2") << BSON("_id" << 3 << "host"
-                                                                                  << "h3")))));
+    ASSERT_OK(config.initializeForInitiate(BSON("_id"
+                                                << "rs0"
+                                                << "version" << 1 << "members"
+                                                << BSON_ARRAY(BSON("_id" << 1 << "host"
+                                                                         << "h1")
+                                                              << BSON("_id" << 2 << "host"
+                                                                            << "h2")
+                                                              << BSON("_id" << 3 << "host"
+                                                                            << "h3")))));
     ReplicationCoordinatorExternalStateMock notPresentExternalState;
     ReplicationCoordinatorExternalStateMock presentOnceExternalState;
     presentOnceExternalState.addSelf(HostAndPort("h2"));
@@ -83,16 +83,16 @@ TEST(ValidateConfigForInitiate, MustFindSelf) {
 
 TEST(ValidateConfigForInitiate, SelfMustBeElectable) {
     ReplicaSetConfig config;
-    ASSERT_OK(config.initialize(BSON("_id"
-                                     << "rs0"
-                                     << "version" << 1 << "members"
-                                     << BSON_ARRAY(BSON("_id" << 1 << "host"
-                                                              << "h1")
-                                                   << BSON("_id" << 2 << "host"
-                                                                 << "h2"
-                                                                 << "priority" << 0)
-                                                   << BSON("_id" << 3 << "host"
-                                                                 << "h3")))));
+    ASSERT_OK(config.initializeForInitiate(BSON("_id"
+                                                << "rs0"
+                                                << "version" << 1 << "members"
+                                                << BSON_ARRAY(BSON("_id" << 1 << "host"
+                                                                         << "h1")
+                                                              << BSON("_id" << 2 << "host"
+                                                                            << "h2"
+                                                                            << "priority" << 0)
+                                                              << BSON("_id" << 3 << "host"
+                                                                            << "h3")))));
     ReplicationCoordinatorExternalStateMock presentOnceExternalState;
     presentOnceExternalState.addSelf(HostAndPort("h2"));
 
@@ -488,13 +488,13 @@ TEST(ValidateConfigForInitiate, NewConfigInvalid) {
     // config is invalid, validateConfigForInitiate will return a status indicating what is
     // wrong with the new config.
     ReplicaSetConfig newConfig;
-    ASSERT_OK(newConfig.initialize(BSON("_id"
-                                        << "rs0"
-                                        << "version" << 2 << "members"
-                                        << BSON_ARRAY(BSON("_id" << 0 << "host"
-                                                                 << "h2")
-                                                      << BSON("_id" << 0 << "host"
-                                                                    << "h3")))));
+    ASSERT_OK(newConfig.initializeForInitiate(BSON("_id"
+                                                   << "rs0"
+                                                   << "version" << 2 << "members"
+                                                   << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                                                            << "h2")
+                                                                 << BSON("_id" << 0 << "host"
+                                                                               << "h3")))));
 
     ReplicationCoordinatorExternalStateMock presentOnceExternalState;
     presentOnceExternalState.addSelf(HostAndPort("h2"));
diff --git a/src/mongo/db/repl/replica_set_config_test.cpp b/src/mongo/db/repl/replica_set_config_test.cpp
index b48eff7300e..5ec17940e65 100644
--- a/src/mongo/db/repl/replica_set_config_test.cpp
+++ b/src/mongo/db/repl/replica_set_config_test.cpp
@@ -32,6 +32,7 @@
 #include "mongo/db/jsobj.h"
 #include "mongo/db/repl/replica_set_config.h"
 #include "mongo/db/server_options.h"
+#include "mongo/util/scopeguard.h"
 #include "mongo/unittest/unittest.h"
 
 namespace mongo {
@@ -714,6 +715,8 @@ TEST(ReplicaSetConfig, ConfigServerField) {
 
     // Configs in which configsvr is not the same as the --configsvr flag are invalid.
     serverGlobalParams.configsvr = true;
+    ON_BLOCK_EXIT([&] { serverGlobalParams.configsvr = false; });
+
     ASSERT_OK(config.validate());
     ASSERT_EQUALS(ErrorCodes::BadValue, config2.validate());
 
@@ -722,6 +725,47 @@ TEST(ReplicaSetConfig, ConfigServerField) {
     ASSERT_OK(config2.validate());
 }
 
+TEST(ReplicaSetConfig, ConfigServerFieldDefaults) {
+    serverGlobalParams.configsvr = false;
+
+    ReplicaSetConfig config;
+    ASSERT_OK(config.initialize(BSON("_id"
+                                     << "rs0"
+                                     << "protocolVersion" << 1 << "version" << 1 << "members"
+                                     << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                                              << "localhost:12345")))));
+    ASSERT_FALSE(config.isConfigServer());
+
+    ReplicaSetConfig config2;
+    ASSERT_OK(
+        config2.initializeForInitiate(BSON("_id"
+                                           << "rs0"
+                                           << "protocolVersion" << 1 << "version" << 1 << "members"
+                                           << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                                                    << "localhost:12345")))));
+    ASSERT_FALSE(config2.isConfigServer());
+
+    serverGlobalParams.configsvr = true;
+    ON_BLOCK_EXIT([&] { serverGlobalParams.configsvr = false; });
+
+    ReplicaSetConfig config3;
+    ASSERT_OK(config3.initialize(BSON("_id"
+                                      << "rs0"
+                                      << "protocolVersion" << 1 << "version" << 1 << "members"
+                                      << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                                               << "localhost:12345")))));
+    ASSERT_FALSE(config3.isConfigServer());
+
+    ReplicaSetConfig config4;
+    ASSERT_OK(
+        config4.initializeForInitiate(BSON("_id"
+                                           << "rs0"
+                                           << "protocolVersion" << 1 << "version" << 1 << "members"
+                                           << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                                                    << "localhost:12345")))));
+    ASSERT_TRUE(config4.isConfigServer());
+}
+
 TEST(ReplicaSetConfig, HeartbeatIntervalField) {
     ReplicaSetConfig config;
     ASSERT_OK(config.initialize(BSON("_id"
diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index e9f307ede48..096dae4dbf5 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -2125,7 +2125,7 @@ Status ReplicationCoordinatorImpl::processReplSetInitiate(OperationContext* txn,
     }
 
     ReplicaSetConfig newConfig;
-    Status status = newConfig.initialize(configObj, true);
+    Status status = newConfig.initializeForInitiate(configObj, true);
     if (!status.isOK()) {
         error() << "replSet initiate got " << status << " while parsing " << configObj;
         return Status(ErrorCodes::InvalidReplicaSetConfig, status.reason());