SERVER-49986 Convert isMaster command to hello and keep isMaster, ismaster aliases

(cherry picked from commit 9a4be902441496be7ef40e5404a91ac30dc81f77)

13 files changed, 145 insertions, 92 deletions

diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
index 4e8b3dbce46..258dfcf1971 100644
--- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
+++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
@@ -122,6 +122,9 @@ selector:
   - jstests/sharding/mr_merge_to_existing.js
   # Disabled due to missing functionality
   - jstests/sharding/ssv_after_restart_of_shards_and_mongos_workarround.js
+  # Enable the following two tests once SERVER-49986 and SERVER-49988 are backported to 4.0.
+  - jstests/sharding/database_and_shard_versioning_all_commands.js
+  - jstests/sharding/ismaster.js
 executor:
   config:
     shell_options:
diff --git a/jstests/core/ismaster.js b/jstests/core/ismaster.js
index 22b19ac734e..985eeb3ed8d 100644
--- a/jstests/core/ismaster.js
+++ b/jstests/core/ismaster.js
@@ -1,44 +1,60 @@
+/**
+ * This test ensures that the hello command and its aliases, ismaster and isMaster, are all
+ * accepted.
+ */
 "use strict";
-var res = db.isMaster();
-// check that the fields that should be there are there and have proper values
-assert(res.maxBsonObjectSize && isNumber(res.maxBsonObjectSize) && res.maxBsonObjectSize > 0,
-       "maxBsonObjectSize possibly missing:" + tojson(res));
-assert(res.maxMessageSizeBytes && isNumber(res.maxMessageSizeBytes) && res.maxBsonObjectSize > 0,
-       "maxMessageSizeBytes possibly missing:" + tojson(res));
-assert(res.maxWriteBatchSize && isNumber(res.maxWriteBatchSize) && res.maxWriteBatchSize > 0,
-       "maxWriteBatchSize possibly missing:" + tojson(res));
-assert.eq("boolean", typeof res.ismaster, "ismaster field is not a boolean" + tojson(res));
-assert(res.ismaster === true, "ismaster field is false" + tojson(res));
-assert(res.localTime, "localTime possibly missing:" + tojson(res));
 
-if (!testingReplication) {
-    var badFields = [];
-    var unwantedReplSetFields = [
-        "setName",
-        "setVersion",
-        "secondary",
-        "hosts",
-        "passives",
-        "arbiters",
-        "primary",
-        "aribterOnly",
-        "passive",
-        "slaveDelay",
-        "hidden",
-        "tags",
-        "buildIndexes",
-        "me"
-    ];
-    var field;
-    // check that the fields that shouldn't be there are not there
-    for (field in res) {
-        if (!res.hasOwnProperty(field)) {
-            continue;
-        }
-        if (Array.contains(unwantedReplSetFields, field)) {
-            badFields.push(field);
+function checkResponseFields(commandString) {
+    var res = db.runCommand(commandString);
+    // check that the fields that should be there are there and have proper values
+    assert(res.maxBsonObjectSize && isNumber(res.maxBsonObjectSize) && res.maxBsonObjectSize > 0,
+           "maxBsonObjectSize possibly missing:" + tojson(res));
+    assert(
+        res.maxMessageSizeBytes && isNumber(res.maxMessageSizeBytes) && res.maxBsonObjectSize > 0,
+        "maxMessageSizeBytes possibly missing:" + tojson(res));
+    assert(res.maxWriteBatchSize && isNumber(res.maxWriteBatchSize) && res.maxWriteBatchSize > 0,
+           "maxWriteBatchSize possibly missing:" + tojson(res));
+    assert.eq("boolean", typeof res.ismaster, "ismaster field is not a boolean" + tojson(res));
+
+    // TODO SERVER-49988: Check for res.isWritablePrimary instead of res.ismaster if a hello command
+    // was executed.
+    assert(res.ismaster === true, "ismaster field is false" + tojson(res));
+    assert(res.localTime, "localTime possibly missing:" + tojson(res));
+
+    if (!testingReplication) {
+        var badFields = [];
+        var unwantedReplSetFields = [
+            "setName",
+            "setVersion",
+            "secondary",
+            "hosts",
+            "passives",
+            "arbiters",
+            "primary",
+            "aribterOnly",
+            "passive",
+            "slaveDelay",
+            "hidden",
+            "tags",
+            "buildIndexes",
+            "me"
+        ];
+        var field;
+        // check that the fields that shouldn't be there are not there
+        for (field in res) {
+            if (!res.hasOwnProperty(field)) {
+                continue;
+            }
+            if (Array.contains(unwantedReplSetFields, field)) {
+                badFields.push(field);
+            }
         }
+        assert(
+            badFields.length === 0,
+            "\nthe result:\n" + tojson(res) + "\ncontained fields it shouldn't have: " + badFields);
     }
-    assert(badFields.length === 0,
-           "\nthe result:\n" + tojson(res) + "\ncontained fields it shouldn't have: " + badFields);
 }
+
+checkResponseFields("hello");
+checkResponseFields("ismaster");
+checkResponseFields("isMaster");
diff --git a/jstests/core/list_commands.js b/jstests/core/list_commands.js
index 94e85511a9b..2310d7d9a0a 100644
--- a/jstests/core/list_commands.js
+++ b/jstests/core/list_commands.js
@@ -21,7 +21,7 @@
 
     // Test that result contains basic commands.
     assert(commands.hasOwnProperty("commands"));
-    assert(commands["commands"].hasOwnProperty("isMaster"));
+    assert(commands["commands"].hasOwnProperty("hello"));
     assert(commands["commands"].hasOwnProperty("insert"));
     assert(commands["commands"].hasOwnProperty("ping"));
 })();
diff --git a/jstests/core/views/views_all_commands.js b/jstests/core/views/views_all_commands.js
index b86b33dbeb0..56e2eb59bde 100644
--- a/jstests/core/views/views_all_commands.js
+++ b/jstests/core/views/views_all_commands.js
@@ -333,12 +333,12 @@
             command: {group: {ns: "test.view", key: "x", $reduce: function() {}, initial: {}}},
         },
         handshake: {skip: isUnrelated},
+        hello: {skip: isUnrelated},
         hostInfo: {skip: isUnrelated},
         insert: {command: {insert: "view", documents: [{x: 1}]}, expectFailure: true},
         invalidateUserCache: {skip: isUnrelated},
         invalidateViewCatalog: {command: {invalidateViewCatalog: 1}},
         isdbgrid: {skip: isUnrelated},
-        isMaster: {skip: isUnrelated},
         journalLatencyTest: {skip: isUnrelated},
         killCursors: {
             setup: function(conn) {
diff --git a/jstests/replsets/db_reads_while_recovering_all_commands.js b/jstests/replsets/db_reads_while_recovering_all_commands.js
index 0cfddb1ed61..32d915c0eb7 100644
--- a/jstests/replsets/db_reads_while_recovering_all_commands.js
+++ b/jstests/replsets/db_reads_while_recovering_all_commands.js
@@ -201,13 +201,13 @@
             expectFailure: true,
             expectedErrorCode: ErrorCodes.NotMasterOrSecondary
         },
+        hello: {skip: isNotAUserDataRead},
         hostInfo: {skip: isNotAUserDataRead},
         httpClientRequest: {skip: isNotAUserDataRead},
         insert: {skip: isPrimaryOnly},
         internalRenameIfOptionsAndIndexesMatch: {skip: isAnInternalCommand},
         invalidateUserCache: {skip: isNotAUserDataRead},
         invalidateViewCatalog: {skip: isNotAUserDataRead},
-        isMaster: {skip: isNotAUserDataRead},
         journalLatencyTest: {skip: isNotAUserDataRead},
         killAllSessions: {skip: isNotAUserDataRead},
         killAllSessionsByPattern: {skip: isNotAUserDataRead},
diff --git a/jstests/sharding/database_and_shard_versioning_all_commands.js b/jstests/sharding/database_and_shard_versioning_all_commands.js
index d91d00ae53f..6e03e4d3363 100644
--- a/jstests/sharding/database_and_shard_versioning_all_commands.js
+++ b/jstests/sharding/database_and_shard_versioning_all_commands.js
@@ -264,6 +264,7 @@
                     {ns: collName, key: {x: 1}, $reduce: function(curr, result) {}, initial: {}}
             },
         },
+        hello: {skip: "executes locally on mongos (not sent to any remote node)"},
         hostInfo: {skip: "executes locally on mongos (not sent to any remote node)"},
         insert: {
             sendsDbVersion: false,
@@ -276,7 +277,6 @@
         },
         invalidateUserCache: {skip: "executes locally on mongos (not sent to any remote node)"},
         isdbgrid: {skip: "executes locally on mongos (not sent to any remote node)"},
-        isMaster: {skip: "executes locally on mongos (not sent to any remote node)"},
         killCursors: {skip: "requires a previously established cursor"},
         killAllSessions: {skip: "always broadcast to all hosts in the cluster"},
         killAllSessionsByPattern: {skip: "always broadcast to all hosts in the cluster"},
diff --git a/jstests/sharding/ismaster.js b/jstests/sharding/ismaster.js
index 2d5cea7b586..a582f677241 100644
--- a/jstests/sharding/ismaster.js
+++ b/jstests/sharding/ismaster.js
@@ -1,42 +1,62 @@
+/**
+ * This test ensures that the hello command and its aliases, ismaster and isMaster, are all
+ * accepted by mongos.
+ */
 "use strict";
 var st = new ShardingTest({shards: 1, mongos: 1});
-var res = st.s0.getDB("admin").runCommand("ismaster");
-// check that the fields that should be there are there and have proper values
-assert(res.maxBsonObjectSize && isNumber(res.maxBsonObjectSize) && res.maxBsonObjectSize > 0,
-       "maxBsonObjectSize possibly missing:" + tojson(res));
-assert(res.maxMessageSizeBytes && isNumber(res.maxMessageSizeBytes) && res.maxBsonObjectSize > 0,
-       "maxMessageSizeBytes possibly missing:" + tojson(res));
-assert.eq("boolean", typeof res.ismaster, "ismaster field is not a boolean" + tojson(res));
-assert(res.ismaster === true, "ismaster field is false" + tojson(res));
-assert(res.localTime, "localTime possibly missing:" + tojson(res));
-assert(res.msg && res.msg == "isdbgrid", "msg possibly missing or wrong:" + tojson(res));
-var unwantedFields = [
-    "setName",
-    "setVersion",
-    "secondary",
-    "hosts",
-    "passives",
-    "arbiters",
-    "primary",
-    "aribterOnly",
-    "passive",
-    "slaveDelay",
-    "hidden",
-    "tags",
-    "buildIndexes",
-    "me"
-];
-// check that the fields that shouldn't be there are not there
-var badFields = [];
-var field;
-for (field in res) {
-    if (!res.hasOwnProperty(field)) {
-        continue;
-    }
-    if (Array.contains(unwantedFields, field)) {
-        badFields.push(field);
+
+function checkResponseFields(commandString) {
+    jsTestLog("Running the " + commandString + " command");
+    var res = st.s0.getDB("admin").runCommand(commandString);
+
+    // check that the fields that should be there are there and have proper values
+    assert(res.maxBsonObjectSize && isNumber(res.maxBsonObjectSize) && res.maxBsonObjectSize > 0,
+           "maxBsonObjectSize possibly missing:" + tojson(res));
+    assert(
+        res.maxMessageSizeBytes && isNumber(res.maxMessageSizeBytes) && res.maxBsonObjectSize > 0,
+        "maxMessageSizeBytes possibly missing:" + tojson(res));
+
+    // TODO SERVER-49988: Check for res.isWritablePrimary instead of res.ismaster if a hello command
+    // was executed.
+    assert.eq("boolean", typeof res.ismaster, "ismaster field is not a boolean" + tojson(res));
+    assert(res.ismaster === true, "ismaster field is false" + tojson(res));
+
+    assert(res.localTime, "localTime possibly missing:" + tojson(res));
+    assert(res.msg && res.msg == "isdbgrid", "msg possibly missing or wrong:" + tojson(res));
+
+    var unwantedFields = [
+        "setName",
+        "setVersion",
+        "secondary",
+        "hosts",
+        "passives",
+        "arbiters",
+        "primary",
+        "aribterOnly",
+        "passive",
+        "slaveDelay",
+        "hidden",
+        "tags",
+        "buildIndexes",
+        "me"
+    ];
+    // check that the fields that shouldn't be there are not there
+    var badFields = [];
+    var field;
+    for (field in res) {
+        if (!res.hasOwnProperty(field)) {
+            continue;
+        }
+        if (Array.contains(unwantedFields, field)) {
+            badFields.push(field);
+        }
     }
+    assert(badFields.length === 0,
+           "\nthe result:\n" + tojson(res) + "\ncontained fields it shouldn't have: " + badFields);
 }
-assert(badFields.length === 0,
-       "\nthe result:\n" + tojson(res) + "\ncontained fields it shouldn't have: " + badFields);
+
+checkResponseFields("hello");
+checkResponseFields("ismaster");
+checkResponseFields("isMaster");
+
 st.stop();
diff --git a/jstests/sharding/safe_secondary_reads_drop_recreate.js b/jstests/sharding/safe_secondary_reads_drop_recreate.js
index 4690a61e704..5bd78819f84 100644
--- a/jstests/sharding/safe_secondary_reads_drop_recreate.js
+++ b/jstests/sharding/safe_secondary_reads_drop_recreate.js
@@ -216,11 +216,11 @@
             behavior: "unshardedOnly"
         },
         handshake: {skip: "does not return user data"},
+        hello: {skip: "does not return user data"},
         hostInfo: {skip: "does not return user data"},
         insert: {skip: "primary only"},
         invalidateUserCache: {skip: "does not return user data"},
         isdbgrid: {skip: "does not return user data"},
-        isMaster: {skip: "does not return user data"},
         journalLatencyTest: {skip: "does not return user data"},
         killAllSessions: {skip: "does not return user data"},
         killAllSessionsByPattern: {skip: "does not return user data"},
diff --git a/jstests/sharding/safe_secondary_reads_single_migration_suspend_range_deletion.js b/jstests/sharding/safe_secondary_reads_single_migration_suspend_range_deletion.js
index 99c651b1712..e67b228c002 100644
--- a/jstests/sharding/safe_secondary_reads_single_migration_suspend_range_deletion.js
+++ b/jstests/sharding/safe_secondary_reads_single_migration_suspend_range_deletion.js
@@ -260,11 +260,11 @@
             behavior: "unshardedOnly"
         },
         handshake: {skip: "does not return user data"},
+        hello: {skip: "does not return user data"},
         hostInfo: {skip: "does not return user data"},
         insert: {skip: "primary only"},
         invalidateUserCache: {skip: "does not return user data"},
         isdbgrid: {skip: "does not return user data"},
-        isMaster: {skip: "does not return user data"},
         journalLatencyTest: {skip: "does not return user data"},
         killCursors: {skip: "does not return user data"},
         killAllSessions: {skip: "does not return user data"},
diff --git a/jstests/sharding/safe_secondary_reads_single_migration_waitForDelete.js b/jstests/sharding/safe_secondary_reads_single_migration_waitForDelete.js
index 375494b5e7c..2908eed678c 100644
--- a/jstests/sharding/safe_secondary_reads_single_migration_waitForDelete.js
+++ b/jstests/sharding/safe_secondary_reads_single_migration_waitForDelete.js
@@ -220,11 +220,11 @@
             behavior: "unshardedOnly"
         },
         handshake: {skip: "does not return user data"},
+        hello: {skip: "does not return user data"},
         hostInfo: {skip: "does not return user data"},
         insert: {skip: "primary only"},
         invalidateUserCache: {skip: "does not return user data"},
         isdbgrid: {skip: "does not return user data"},
-        isMaster: {skip: "does not return user data"},
         journalLatencyTest: {skip: "does not return user data"},
         killAllSessions: {skip: "does not return user data"},
         killAllSessionsByPattern: {skip: "does not return user data"},
diff --git a/src/mongo/db/repl/replication_info.cpp b/src/mongo/db/repl/replication_info.cpp
index 3e1a8d87acc..4c919971447 100644
--- a/src/mongo/db/repl/replication_info.cpp
+++ b/src/mongo/db/repl/replication_info.cpp
@@ -33,6 +33,7 @@
 #include <list>
 #include <vector>
 
+#include "mongo/base/string_data.h"
 #include "mongo/client/connpool.h"
 #include "mongo/db/auth/sasl_mechanism_registry.h"
 #include "mongo/db/client.h"
@@ -74,6 +75,11 @@ namespace {
 
 MONGO_FAIL_POINT_DEFINE(impersonateFullyUpgradedFutureVersion);
 
+constexpr auto kHelloString = "hello"_sd;
+// Aliases for the hello command in order to provide backwards compatibility.
+constexpr auto kCamelCaseIsMasterString = "isMaster"_sd;
+constexpr auto kLowerCaseIsMasterString = "ismaster"_sd;
+
 void appendReplicationInfo(OperationContext* opCtx, BSONObjBuilder& result, int level) {
     ReplicationCoordinator* replCoord = ReplicationCoordinator::get(opCtx);
     if (replCoord->getSettings().usingReplSets()) {
@@ -207,8 +213,10 @@ public:
     }
 } oplogInfoServerStatus;
 
-class CmdIsMaster : public BasicCommand {
+class CmdHello : public BasicCommand {
 public:
+    CmdHello() : BasicCommand(kHelloString, {kCamelCaseIsMasterString, kLowerCaseIsMasterString}) {}
+
     bool requiresAuth() const override {
         return false;
     }
@@ -225,7 +233,6 @@ public:
     virtual void addRequiredPrivileges(const std::string& dbname,
                                        const BSONObj& cmdObj,
                                        std::vector<Privilege>* out) const {}  // No auth required
-    CmdIsMaster() : BasicCommand("isMaster", "ismaster") {}
     virtual bool run(OperationContext* opCtx,
                      const string&,
                      const BSONObj& cmdObj,
@@ -391,7 +398,7 @@ public:
 
         return true;
     }
-} cmdismaster;
+} cmdhello;
 
 OpCounterServerStatusSection replOpCounterServerStatusSection("opcountersRepl", &replOpCounters);
 
diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp
index a50743e0f1d..32fd94b7e40 100644
--- a/src/mongo/db/service_entry_point_common.cpp
+++ b/src/mongo/db/service_entry_point_common.cpp
@@ -134,8 +134,8 @@ const StringMap<int> sessionCheckoutWhitelist = {{"abortTransaction", 1},
 bool shouldActivateFailCommandFailPoint(const BSONObj& data,
                                         const CommandInvocation* invocation,
                                         Client* client) {
-    auto cmdName = invocation->definition()->getName();
-    if (cmdName == "configureFailPoint"_sd)  // Banned even if in failCommands.
+    const Command* cmd = invocation->definition();
+    if (cmd->getName() == "configureFailPoint"_sd)  // Banned even if in failCommands.
         return false;
 
     if (client->session() && (client->session()->getTags() & transport::Session::kInternalClient)) {
@@ -149,7 +149,7 @@ bool shouldActivateFailCommandFailPoint(const BSONObj& data,
     }
 
     for (auto&& failCommand : data.getObjectField("failCommands")) {
-        if (failCommand.type() == String && failCommand.valueStringData() == cmdName) {
+        if (failCommand.type() == String && cmd->hasAlias(failCommand.valueStringData())) {
             return true;
         }
     }
diff --git a/src/mongo/s/commands/cluster_is_master_cmd.cpp b/src/mongo/s/commands/cluster_is_master_cmd.cpp
index f92f1415da3..a8321dabad6 100644
--- a/src/mongo/s/commands/cluster_is_master_cmd.cpp
+++ b/src/mongo/s/commands/cluster_is_master_cmd.cpp
@@ -30,6 +30,7 @@
 
 #include "mongo/platform/basic.h"
 
+#include "mongo/base/string_data.h"
 #include "mongo/db/auth/sasl_mechanism_registry.h"
 #include "mongo/db/client.h"
 #include "mongo/db/commands.h"
@@ -48,9 +49,15 @@
 namespace mongo {
 namespace {
 
-class CmdIsMaster : public BasicCommand {
+constexpr auto kHelloString = "hello"_sd;
+// Aliases for the hello command in order to provide backwards compatibility.
+constexpr auto kCamelCaseIsMasterString = "isMaster"_sd;
+constexpr auto kLowerCaseIsMasterString = "ismaster"_sd;
+
+
+class CmdHello : public BasicCommand {
 public:
-    CmdIsMaster() : BasicCommand("isMaster", "ismaster") {}
+    CmdHello() : BasicCommand(kHelloString, {kCamelCaseIsMasterString, kLowerCaseIsMasterString}) {}
 
     bool supportsWriteConcern(const BSONObj& cmd) const override {
         return false;
@@ -135,7 +142,7 @@ public:
         return true;
     }
 
-} isMaster;
+} hello;
 
 }  // namespace
 }  // namespace mongo