10 files changed, 260 insertions, 87 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 18f64521d3f..3b3d13313d5 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
@@ -41,6 +41,7 @@ selector:
   - jstests/sharding/retryable_writes.js
   - jstests/sharding/uuid_propagated_to_config_server_on_shardCollection.js
   - jstests/sharding/uuid_propagated_to_recipient_shard_on_recvChunkStart.js
+  - jstests/sharding/uuid_propagated_to_shards_on_setFCV_3_6.js
   # New feature in v3.6 mongo shell.
   - jstests/sharding/causal_consistency_shell_support.js
   - jstests/sharding/keys_rotation_interval_sec.js
diff --git a/jstests/sharding/addshard2.js b/jstests/sharding/addshard2.js
index 30b7d4365ff..e0fce8864ec 100644
--- a/jstests/sharding/addshard2.js
+++ b/jstests/sharding/addshard2.js
@@ -5,7 +5,6 @@
 (function() {
 
     var addShardRes;
-    var rst;
 
     var assertAddShardSucceeded = function(res, shardName) {
         assert.commandWorked(res);
@@ -63,30 +62,23 @@
     // Allocate a port that can be used to test adding invalid hosts.
     var portWithoutHostRunning = allocatePort();
 
-    // Enable the failpoint that prevents the config server from upserting a shardIdentity on new
-    // shards so that the same shard host can be re-used for multiple addShard calls without being
-    // restarted in between each addShard (the shardIdentity cannot be deleted while the shard host
-    // is running with --shardsvr).
-    st.configRS.getPrimary().adminCommand(
-        {configureFailPoint: "dontUpsertShardIdentityOnNewShards", mode: "alwaysOn"});
-
     // 1. Test adding a *standalone*
 
     // 1.a. with or without specifying the shardName.
 
-    var standalone = MongoRunner.runMongod({shardsvr: ''});
-
     jsTest.log("Adding a standalone *without* a specified shardName should succeed.");
-    addShardRes = st.s.adminCommand({addshard: standalone.name});
+    let standalone1 = MongoRunner.runMongod({shardsvr: ''});
+    addShardRes = st.s.adminCommand({addshard: standalone1.name});
     assertAddShardSucceeded(addShardRes);
     removeShardWithName(addShardRes.shardAdded);
+    MongoRunner.stopMongod(standalone1);
 
     jsTest.log("Adding a standalone *with* a specified shardName should succeed.");
-    addShardRes = st.s.adminCommand({addshard: standalone.name, name: "shardName"});
+    let standalone2 = MongoRunner.runMongod({shardsvr: ''});
+    addShardRes = st.s.adminCommand({addshard: standalone2.name, name: "shardName"});
     assertAddShardSucceeded(addShardRes, "shardName");
     removeShardWithName(addShardRes.shardAdded);
-
-    MongoRunner.stopMongod(standalone);
+    MongoRunner.stopMongod(standalone2);
 
     // 1.b. with an invalid hostname.
 
@@ -98,88 +90,90 @@
 
     // 2.a. with or without specifying the shardName.
 
-    rst = new ReplSetTest({nodes: 1});
-    rst.startSet({shardsvr: ''});
-    rst.initiate();
-
     jsTest.log("Adding a replica set without a specified shardName should succeed.");
-    addShardRes = st.s.adminCommand({addShard: rst.getURL()});
+    let rst1 = new ReplSetTest({nodes: 1});
+    rst1.startSet({shardsvr: ''});
+    rst1.initiate();
+    addShardRes = st.s.adminCommand({addShard: rst1.getURL()});
     assertAddShardSucceeded(addShardRes);
-    assert.eq(rst.name, addShardRes.shardAdded);
+    assert.eq(rst1.name, addShardRes.shardAdded);
     removeShardWithName(addShardRes.shardAdded);
+    rst1.stopSet();
 
     jsTest.log(
         "Adding a replica set with a specified shardName that matches the set's name should succeed.");
-    addShardRes = st.s.adminCommand({addShard: rst.getURL(), name: rst.name});
-    assertAddShardSucceeded(addShardRes, rst.name);
+    let rst2 = new ReplSetTest({nodes: 1});
+    rst2.startSet({shardsvr: ''});
+    rst2.initiate();
+    addShardRes = st.s.adminCommand({addShard: rst2.getURL(), name: rst2.name});
+    assertAddShardSucceeded(addShardRes, rst2.name);
     removeShardWithName(addShardRes.shardAdded);
+    rst2.stopSet();
+
+    let rst3 = new ReplSetTest({nodes: 1});
+    rst3.startSet({shardsvr: ''});
+    rst3.initiate();
 
     jsTest.log(
         "Adding a replica set with a specified shardName that differs from the set's name should succeed.");
-    addShardRes = st.s.adminCommand({addShard: rst.getURL(), name: "differentShardName"});
+    addShardRes = st.s.adminCommand({addShard: rst3.getURL(), name: "differentShardName"});
     assertAddShardSucceeded(addShardRes, "differentShardName");
     removeShardWithName(addShardRes.shardAdded);
 
     jsTest.log("Adding a replica with a specified shardName of 'config' should fail.");
-    addShardRes = st.s.adminCommand({addShard: rst.getURL(), name: "config"});
+    addShardRes = st.s.adminCommand({addShard: rst3.getURL(), name: "config"});
     assertAddShardFailed(addShardRes, "config");
 
     // 2.b. with invalid hostnames.
 
     jsTest.log("Adding a replica set with only non-existing hosts should fail.");
     addShardRes =
-        st.s.adminCommand({addShard: rst.name + "/NonExistingHost:" + portWithoutHostRunning});
+        st.s.adminCommand({addShard: rst3.name + "/NonExistingHost:" + portWithoutHostRunning});
     assertAddShardFailed(addShardRes);
 
     jsTest.log("Adding a replica set with mixed existing/non-existing hosts should fail.");
     addShardRes = st.s.adminCommand({
         addShard:
-            rst.name + "/" + rst.getPrimary().name + ",NonExistingHost:" + portWithoutHostRunning
+            rst3.name + "/" + rst3.getPrimary().name + ",NonExistingHost:" + portWithoutHostRunning
     });
     assertAddShardFailed(addShardRes);
 
-    rst.stopSet();
+    rst3.stopSet();
 
     // 3. Test adding a replica set whose *set name* is "config" with or without specifying the
     // shardName.
 
-    rst = new ReplSetTest({name: "config", nodes: 1});
-    rst.startSet({shardsvr: ''});
-    rst.initiate();
+    let rst4 = new ReplSetTest({name: "config", nodes: 1});
+    rst4.startSet({shardsvr: ''});
+    rst4.initiate();
 
     jsTest.log(
         "Adding a replica set whose setName is config without specifying shardName should fail.");
-    addShardRes = st.s.adminCommand({addShard: rst.getURL()});
+    addShardRes = st.s.adminCommand({addShard: rst4.getURL()});
     assertAddShardFailed(addShardRes);
 
     jsTest.log(
         "Adding a replica set whose setName is config with specified shardName 'config' should fail.");
-    addShardRes = st.s.adminCommand({addShard: rst.getURL(), name: rst.name});
-    assertAddShardFailed(addShardRes, rst.name);
+    addShardRes = st.s.adminCommand({addShard: rst4.getURL(), name: rst4.name});
+    assertAddShardFailed(addShardRes, rst4.name);
 
     jsTest.log(
         "Adding a replica set whose setName is config with a non-'config' shardName should succeed");
-    addShardRes = st.s.adminCommand({addShard: rst.getURL(), name: "nonConfig"});
+    addShardRes = st.s.adminCommand({addShard: rst4.getURL(), name: "nonConfig"});
     assertAddShardSucceeded(addShardRes, "nonConfig");
     removeShardWithName(addShardRes.shardAdded);
 
-    rst.stopSet();
+    rst4.stopSet();
 
     // 4. Test that a replica set whose *set name* is "admin" can be written to (SERVER-17232).
 
-    // Turn off the dontUpsertShardIdentityOnNewShards failpoint, since mongos will send
-    // setShardVersion when trying to do the write, and the setShardVersion will fail if the
-    // sharding state will not be enabled.
-    assert.commandWorked(st.configRS.getPrimary().adminCommand(
-        {configureFailPoint: "dontUpsertShardIdentityOnNewShards", mode: "off"}));
-
-    rst = new ReplSetTest({name: "admin", nodes: 1});
-    rst.startSet({shardsvr: ''});
-    rst.initiate();
+    let rst5 = new ReplSetTest({name: "admin", nodes: 1});
+    rst5.startSet({shardsvr: ''});
+    rst5.initiate();
 
     jsTest.log("A replica set whose set name is 'admin' should be able to be written to.");
 
-    addShardRes = st.s.adminCommand({addShard: rst.getURL()});
+    addShardRes = st.s.adminCommand({addShard: rst5.getURL()});
     assertAddShardSucceeded(addShardRes);
 
     // Ensure the write goes to the newly added shard.
@@ -191,13 +185,13 @@
     }
 
     assert.writeOK(st.s.getDB('test').foo.insert({x: 1}));
-    assert.neq(null, rst.getPrimary().getDB('test').foo.findOne());
+    assert.neq(null, rst5.getPrimary().getDB('test').foo.findOne());
 
     assert.commandWorked(st.s.getDB('test').runCommand({dropDatabase: 1}));
 
     removeShardWithName(addShardRes.shardAdded);
 
-    rst.stopSet();
+    rst5.stopSet();
 
     st.stop();
 
diff --git a/jstests/sharding/uuid_propagated_to_shards_on_setFCV_3_6.js b/jstests/sharding/uuid_propagated_to_shards_on_setFCV_3_6.js
new file mode 100644
index 00000000000..ff6deb551d3
--- /dev/null
+++ b/jstests/sharding/uuid_propagated_to_shards_on_setFCV_3_6.js
@@ -0,0 +1,63 @@
+/**
+ * Tests that after setFeatureCompatibilityVersion=3.6, for all sharded collections, the UUID
+ * reported by the shard in listCollections matches the UUID in config.collections.
+ *
+ * This should be true even if setFCV upgrade/downgrade is called repeatedly on the cluster, and
+ * even if drops, recreates, and shardCollections are called in between the upgrades/downgrades.
+ */
+(function() {
+    let st = new ShardingTest({shards: {rs0: {nodes: 1}}, other: {config: 3}});
+
+    // Start in fcv=3.4.
+    assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: "3.4"}));
+
+    // We will manipulate collections across two databases.
+    let db1 = "test1";
+    let db2 = "test2";
+    assert.commandWorked(st.s.adminCommand({enableSharding: db1}));
+    st.ensurePrimaryShard(db1, st.shard0.shardName);
+    assert.commandWorked(st.s.adminCommand({enableSharding: db2}));
+    st.ensurePrimaryShard(db2, st.shard0.shardName);
+
+    jsTest.log("shard some collections in each database");
+    for (let i = 0; i < 3; i++) {
+        let coll = "foo" + i;
+        let nss1 = db1 + "." + coll;
+        let nss2 = db2 + "." + coll;
+        assert.commandWorked(st.s.adminCommand({shardCollection: nss1, key: {_id: 1}}));
+        assert.commandWorked(st.s.adminCommand({shardCollection: nss2, key: {_id: 1}}));
+    }
+
+    jsTest.log("upgrade the cluster to fcv=3.6");
+    assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: "3.6"}));
+
+    st.checkUUIDsConsistentAcrossCluster();
+
+    // Drop some collections, shard some new collections, and drop and recreate some of the
+    // collections as sharded with the same name.
+    assert.commandWorked(st.s.getDB(db1).runCommand({drop: "foo0"}));
+    assert.commandWorked(st.s.adminCommand({shardCollection: db1 + ".bar0", key: {_id: 1}}));
+    assert.commandWorked(st.s.getDB(db1).runCommand({drop: "foo1"}));
+    assert.commandWorked(st.s.adminCommand({shardCollection: db1 + ".foo1", key: {_id: 1}}));
+
+    st.checkUUIDsConsistentAcrossCluster();
+
+    jsTest.log("downgrade the cluster to fcv=3.4");
+    assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: "3.4"}));
+
+    // Drop, recreate, and shard some collections again, now while downgraded.
+    assert.commandWorked(st.s.getDB(db2).runCommand({drop: "foo0"}));
+    assert.commandWorked(st.s.adminCommand({shardCollection: db2 + ".bar0", key: {_id: 1}}));
+    assert.commandWorked(st.s.getDB(db2).runCommand({drop: "foo1"}));
+    assert.commandWorked(st.s.adminCommand({shardCollection: db2 + ".foo1", key: {_id: 1}}));
+
+    // We do not check UUID consistency after downgrading back to fcv=3.4, because the UUIDs are
+    // deleted from shards on downgrade, but not from the config server's metadata.
+
+    jsTest.log("re-upgrade the cluster to fcv=3.6");
+    assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: "3.6"}));
+
+    st.checkUUIDsConsistentAcrossCluster();
+
+    st.stop();
+})();
diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp
index 4233c92f37a..7fec15938b5 100644
--- a/src/mongo/db/catalog/coll_mod.cpp
+++ b/src/mongo/db/catalog/coll_mod.cpp
@@ -26,6 +26,8 @@
  *    it in the license file.
  */
 
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kCommand
+
 #include "mongo/platform/basic.h"
 
 #include "mongo/db/catalog/coll_mod.h"
@@ -43,9 +45,15 @@
 #include "mongo/db/db_raii.h"
 #include "mongo/db/index/index_descriptor.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
+#include "mongo/db/s/sharding_state.h"
 #include "mongo/db/service_context.h"
 #include "mongo/db/storage/recovery_unit.h"
 #include "mongo/db/views/view_catalog.h"
+#include "mongo/s/catalog/type_collection.h"
+#include "mongo/s/client/shard_registry.h"
+#include "mongo/s/grid.h"
+#include "mongo/s/sharding_initialization.h"
+#include "mongo/util/log.h"
 
 namespace mongo {
 
@@ -415,6 +423,7 @@ Status _collModInternal(OperationContext* opCtx,
 
 void _updateDBSchemaVersion(OperationContext* opCtx,
                             const std::string& dbname,
+                            std::map<std::string, UUID>& collToUUID,
                             bool needUUIDAdded) {
     // Iterate through all collections of database dbname and make necessary UUID changes.
     std::vector<NamespaceString> collNamespaceStrings;
@@ -439,6 +448,7 @@ void _updateDBSchemaVersion(OperationContext* opCtx,
         if (collNSS.db() == "local" || collNSS.coll() == "system.profile") {
             continue;
         }
+
         AutoGetDb autoDb(opCtx, dbname, MODE_X);
         Database* const db = autoDb.getDb();
         Collection* coll = db ? db->getCollection(opCtx, collNSS) : nullptr;
@@ -449,9 +459,16 @@ void _updateDBSchemaVersion(OperationContext* opCtx,
         BSONObjBuilder collModObjBuilder;
         collModObjBuilder.append("collMod", coll->ns().coll());
         BSONObj collModObj = collModObjBuilder.done();
+
         OptionalCollectionUUID uuid = boost::none;
         if (needUUIDAdded) {
-            uuid = UUID::gen();
+            if (collToUUID.find(collNSS.coll().toString()) != collToUUID.end()) {
+                // This is a sharded collection. Use the UUID generated by the config server.
+                uuid = collToUUID[collNSS.coll().toString()];
+            } else {
+                // This is an unsharded collection. Generate a UUID.
+                uuid = UUID::gen();
+            }
         }
         if ((needUUIDAdded && !coll->uuid()) || (!needUUIDAdded && coll->uuid())) {
             uassertStatusOK(collModForUUIDUpgrade(opCtx, coll->ns(), collModObj, uuid));
@@ -549,6 +566,40 @@ void updateUUIDSchemaVersion(OperationContext* opCtx, bool upgrade) {
     if (!enableCollectionUUIDs) {
         return;
     }
+
+    // A map of the form { db1: { collB: UUID, collA: UUID, ... }, db2: { ... } }
+    std::map<std::string, std::map<std::string, UUID>> dbToCollToUUID;
+    if (upgrade && ShardingState::get(opCtx)->enabled()) {
+        log() << "obtaining UUIDs for pre-existing sharded collections from config server";
+
+        // Get UUIDs for all existing sharded collections from the config server. Since the sharded
+        // collections are not stored per-database in config.collections, it's more efficient to
+        // read all the collections at once than to read them by database.
+        auto shardedColls =
+            uassertStatusOK(
+                Grid::get(opCtx)->shardRegistry()->getConfigShard()->exhaustiveFindOnConfig(
+                    opCtx,
+                    ReadPreferenceSetting{ReadPreference::PrimaryOnly},
+                    repl::ReadConcernLevel::kMajorityReadConcern,
+                    NamespaceString(CollectionType::ConfigNS),
+                    BSON("dropped" << false),  // query
+                    BSONObj(),                 // sort
+                    boost::none                // limit
+                    ))
+                .docs;
+
+        for (const auto& coll : shardedColls) {
+            auto collType = uassertStatusOK(CollectionType::fromBSON(coll));
+            uassert(ErrorCodes::InternalError,
+                    str::stream() << "expected entry " << coll << " in config.collections for "
+                                  << collType.getNs().ns()
+                                  << " to have a UUID, but it did not",
+                    collType.getUUID());
+            dbToCollToUUID[collType.getNs().db().toString()][collType.getNs().coll().toString()] =
+                *collType.getUUID();
+        }
+    }
+
     // Update UUIDs on all collections of all databases.
     std::vector<std::string> dbNames;
     StorageEngine* storageEngine = opCtx->getServiceContext()->getGlobalStorageEngine();
@@ -559,7 +610,7 @@ void updateUUIDSchemaVersion(OperationContext* opCtx, bool upgrade) {
 
     for (auto it = dbNames.begin(); it != dbNames.end(); ++it) {
         auto dbName = *it;
-        _updateDBSchemaVersion(opCtx, dbName, upgrade);
+        _updateDBSchemaVersion(opCtx, dbName, dbToCollToUUID[dbName], upgrade);
     }
     const WriteConcernOptions writeConcern(WriteConcernOptions::kMajority,
                                            WriteConcernOptions::SyncMode::UNSET,
diff --git a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
index f552c87478c..a9d4be0b58a 100644
--- a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
+++ b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
@@ -37,8 +37,10 @@
 #include "mongo/db/commands/feature_compatibility_version_command_parser.h"
 #include "mongo/db/concurrency/d_concurrency.h"
 #include "mongo/db/db_raii.h"
+#include "mongo/db/logical_time_validator.h"
 #include "mongo/db/repl/replication_coordinator.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
+#include "mongo/db/s/sharding_state.h"
 #include "mongo/db/server_options.h"
 #include "mongo/rpc/get_status_from_command_result.h"
 #include "mongo/util/scopeguard.h"
@@ -132,6 +134,12 @@ public:
             updateUUIDSchemaVersion(opCtx, /*upgrade*/ false);
         }
 
+        // Ensure we try reading the keys for signing clusterTime immediately on upgrade to 3.6.
+        if (ShardingState::get(opCtx)->enabled() &&
+            version == FeatureCompatibilityVersionCommandParser::kVersion36) {
+            LogicalTimeValidator::get(opCtx)->forceKeyRefreshNow(opCtx);
+        }
+
         return true;
     }
 
diff --git a/src/mongo/db/logical_time_validator.cpp b/src/mongo/db/logical_time_validator.cpp
index 98e4f740ff6..9bbb9021ac7 100644
--- a/src/mongo/db/logical_time_validator.cpp
+++ b/src/mongo/db/logical_time_validator.cpp
@@ -186,4 +186,8 @@ bool LogicalTimeValidator::shouldGossipLogicalTime() {
     return _keyManager->hasSeenKeys();
 }
 
+void LogicalTimeValidator::forceKeyRefreshNow(OperationContext* opCtx) {
+    _keyManager->refreshNow(opCtx);
+}
+
 }  // namespace mongo
diff --git a/src/mongo/db/logical_time_validator.h b/src/mongo/db/logical_time_validator.h
index 7d11450bf1c..dd171c5de2f 100644
--- a/src/mongo/db/logical_time_validator.h
+++ b/src/mongo/db/logical_time_validator.h
@@ -101,6 +101,11 @@ public:
      */
     bool shouldGossipLogicalTime();
 
+    /**
+     * Makes the KeysCollectionManager refresh synchronously.
+     */
+    void forceKeyRefreshNow(OperationContext* opCtx);
+
 private:
     SignedLogicalTime _getProof(const KeysCollectionDocument& keyDoc, LogicalTime newTime);
 
diff --git a/src/mongo/s/catalog/sharding_catalog_add_shard_test.cpp b/src/mongo/s/catalog/sharding_catalog_add_shard_test.cpp
index e0aa011dd2c..f986f2b2f28 100644
--- a/src/mongo/s/catalog/sharding_catalog_add_shard_test.cpp
+++ b/src/mongo/s/catalog/sharding_catalog_add_shard_test.cpp
@@ -398,12 +398,12 @@ TEST_F(AddShardTest, StandaloneBasicSuccess) {
                              BSON("name" << discoveredDB1.getName() << "sizeOnDisk" << 2000),
                              BSON("name" << discoveredDB2.getName() << "sizeOnDisk" << 5000)});
 
-    // The shard receives the setFeatureCompatibilityVersion command.
-    expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1));
-
     // The shardIdentity doc inserted into the admin.system.version collection on the shard.
     expectShardIdentityUpsertReturnSuccess(shardTarget, expectedShardName);
 
+    // The shard receives the setFeatureCompatibilityVersion command.
+    expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1));
+
     // Wait for the addShard to complete before checking the config database
     future.timed_get(kLongFutureTimeout);
 
@@ -481,12 +481,12 @@ TEST_F(AddShardTest, StandaloneGenerateName) {
                              BSON("name" << discoveredDB1.getName() << "sizeOnDisk" << 2000),
                              BSON("name" << discoveredDB2.getName() << "sizeOnDisk" << 5000)});
 
-    // The shard receives the setFeatureCompatibilityVersion command.
-    expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1));
-
     // The shardIdentity doc inserted into the admin.system.version collection on the shard.
     expectShardIdentityUpsertReturnSuccess(shardTarget, expectedShardName);
 
+    // The shard receives the setFeatureCompatibilityVersion command.
+    expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1));
+
     // Wait for the addShard to complete before checking the config database
     future.timed_get(kLongFutureTimeout);
 
@@ -906,12 +906,12 @@ TEST_F(AddShardTest, SuccessfullyAddReplicaSet) {
     // Get databases list from new shard
     expectListDatabases(shardTarget, std::vector<BSONObj>{BSON("name" << discoveredDB.getName())});
 
-    // The shard receives the setFeatureCompatibilityVersion command.
-    expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1));
-
     // The shardIdentity doc inserted into the admin.system.version collection on the shard.
     expectShardIdentityUpsertReturnSuccess(shardTarget, expectedShardName);
 
+    // The shard receives the setFeatureCompatibilityVersion command.
+    expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1));
+
     // Wait for the addShard to complete before checking the config database
     future.timed_get(kLongFutureTimeout);
 
@@ -971,12 +971,12 @@ TEST_F(AddShardTest, ReplicaSetExtraHostsDiscovered) {
     // Get databases list from new shard
     expectListDatabases(shardTarget, std::vector<BSONObj>{BSON("name" << discoveredDB.getName())});
 
-    // The shard receives the setFeatureCompatibilityVersion command.
-    expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1));
-
     // The shardIdentity doc inserted into the admin.system.version collection on the shard.
     expectShardIdentityUpsertReturnSuccess(shardTarget, expectedShardName);
 
+    // The shard receives the setFeatureCompatibilityVersion command.
+    expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1));
+
     // Wait for the addShard to complete before checking the config database
     future.timed_get(kLongFutureTimeout);
 
@@ -1052,12 +1052,12 @@ TEST_F(AddShardTest, AddShardSucceedsEvenIfAddingDBsFromNewShardFails) {
                              BSON("name" << discoveredDB1.getName() << "sizeOnDisk" << 2000),
                              BSON("name" << discoveredDB2.getName() << "sizeOnDisk" << 5000)});
 
-    // The shard receives the setFeatureCompatibilityVersion command.
-    expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1));
-
     // The shardIdentity doc inserted into the admin.system.version collection on the shard.
     expectShardIdentityUpsertReturnSuccess(shardTarget, expectedShardName);
 
+    // The shard receives the setFeatureCompatibilityVersion command.
+    expectSetFeatureCompatibilityVersion(shardTarget, BSON("ok" << 1));
+
     // Wait for the addShard to complete before checking the config database
     future.timed_get(kLongFutureTimeout);
 
diff --git a/src/mongo/s/catalog/sharding_catalog_manager_shard_operations.cpp b/src/mongo/s/catalog/sharding_catalog_manager_shard_operations.cpp
index 42e99784c6c..93560936442 100644
--- a/src/mongo/s/catalog/sharding_catalog_manager_shard_operations.cpp
+++ b/src/mongo/s/catalog/sharding_catalog_manager_shard_operations.cpp
@@ -85,8 +85,6 @@ const Seconds kDefaultFindHostMaxWaitTime(20);
 const ReadPreferenceSetting kConfigReadSelector(ReadPreference::Nearest, TagSet{});
 const WriteConcernOptions kNoWaitWriteConcern(1, WriteConcernOptions::SyncMode::UNSET, Seconds(0));
 
-MONGO_FP_DECLARE(dontUpsertShardIdentityOnNewShards);
-
 /**
  * Generates a unique name to be given to a newly added shard.
  */
@@ -620,6 +618,22 @@ StatusWith<std::string> ShardingCatalogManager::addShard(
         shardType.setMaxSizeMB(maxSize);
     }
 
+    // Insert a shardIdentity document onto the shard. This also triggers sharding initialization on
+    // the shard.
+    LOG(2) << "going to insert shardIdentity document into shard: " << shardType;
+    auto commandRequest = createShardIdentityUpsertForAddShard(opCtx, shardType.getName());
+    auto swCommandResponse = _runCommandForAddShard(opCtx, targeter.get(), "admin", commandRequest);
+    if (!swCommandResponse.isOK()) {
+        return swCommandResponse.getStatus();
+    }
+    auto commandResponse = std::move(swCommandResponse.getValue());
+    BatchedCommandResponse batchResponse;
+    auto batchResponseStatus =
+        Shard::CommandResponse::processBatchWriteResponse(commandResponse, &batchResponse);
+    if (!batchResponseStatus.isOK()) {
+        return batchResponseStatus;
+    }
+
     // The featureCompatibilityVersion should be the same throughout the cluster.
     auto versionResponse = _runCommandForAddShard(
         opCtx,
@@ -635,27 +649,6 @@ StatusWith<std::string> ShardingCatalogManager::addShard(
         return versionResponse.getValue().commandStatus;
     }
 
-    if (!MONGO_FAIL_POINT(dontUpsertShardIdentityOnNewShards)) {
-        auto commandRequest = createShardIdentityUpsertForAddShard(opCtx, shardType.getName());
-
-        LOG(2) << "going to insert shardIdentity document into shard: " << shardType;
-
-        auto swCommandResponse =
-            _runCommandForAddShard(opCtx, targeter.get(), "admin", commandRequest);
-        if (!swCommandResponse.isOK()) {
-            return swCommandResponse.getStatus();
-        }
-
-        auto commandResponse = std::move(swCommandResponse.getValue());
-
-        BatchedCommandResponse batchResponse;
-        auto batchResponseStatus =
-            Shard::CommandResponse::processBatchWriteResponse(commandResponse, &batchResponse);
-        if (!batchResponseStatus.isOK()) {
-            return batchResponseStatus;
-        }
-    }
-
     log() << "going to insert new entry for shard into config.shards: " << shardType.toString();
 
     Status result = Grid::get(opCtx)->catalogClient()->insertConfigDocument(
diff --git a/src/mongo/shell/shardingtest.js b/src/mongo/shell/shardingtest.js
index ad286256004..585f5f11de2 100644
--- a/src/mongo/shell/shardingtest.js
+++ b/src/mongo/shell/shardingtest.js
@@ -934,6 +934,60 @@ var ShardingTest = function(params) {
         assert(res.ok || res.errmsg == "it is already the primary", tojson(res));
     };
 
+    this.checkUUIDsConsistentAcrossCluster = function() {
+        print("Checking if UUIDs are consistent across the cluster");
+
+        let parseNs = function(dbDotColl) {
+            assert.gt(dbDotColl.indexOf('.'), 0);
+            let dbName = dbDotColl.substring(0, dbDotColl.indexOf('.'));
+            let collName = dbDotColl.substring(dbDotColl.indexOf('.') + 1, dbDotColl.length);
+            return [dbName, collName];
+        };
+
+        // Read from config.collections, config.shards, and config.chunks to construct a picture of
+        // which shards own data for which collections, and what the UUID for those collections are.
+        let authoritativeCollMetadatas =
+            this.s.getDB("config")
+                .chunks
+                .aggregate([
+                    {
+                      $lookup: {
+                          from: "shards",
+                          localField: "shard",
+                          foreignField: "_id",
+                          as: "shardHost"
+                      }
+                    },
+                    {$unwind: "$shardHost"},
+                    {$group: {_id: "$ns", shards: {$addToSet: "$shardHost.host"}}},
+                    {
+                      $lookup: {
+                          from: "collections",
+                          localField: "_id",
+                          foreignField: "_id",
+                          as: "collInfo"
+                      }
+                    },
+                    {$unwind: "$collInfo"}
+                ])
+                .toArray();
+
+        for (authoritativeCollMetadata of authoritativeCollMetadatas) {
+            let [dbName, collName] = parseNs(authoritativeCollMetadata._id);
+            for (shard of authoritativeCollMetadata.shards) {
+                let shardConn = new Mongo(shard);
+                let actualCollMetadata =
+                    shardConn.getDB(dbName).getCollectionInfos({name: collName})[0];
+                assert.eq(authoritativeCollMetadata.collInfo.uuid,
+                          actualCollMetadata.info.uuid,
+                          "authoritative collection info on config server: " +
+                              tojson(authoritativeCollMetadata.collInfo) +
+                              ", actual collection info on shard " + shard + ": " +
+                              tojson(actualCollMetadata));
+            }
+        }
+    };
+
     /**
      * Returns whether any settings to ShardingTest or jsTestOptions indicate this is a multiversion
      * cluster.