SERVER-23428 Make setShardVersion trigger CSRS upgrade if it provides a CSRS connection string

7 files changed, 116 insertions, 15 deletions

diff --git a/jstests/libs/csrs_upgrade_util.js b/jstests/libs/csrs_upgrade_util.js
index d865b6fee54..411e8b290fa 100644
--- a/jstests/libs/csrs_upgrade_util.js
+++ b/jstests/libs/csrs_upgrade_util.js
@@ -68,6 +68,10 @@ var CSRSUpgradeCoordinator = function() {
         return st._mongos[n];
     };
 
+    this.getShard = function(n) {
+        return st['shard' + n];
+    };
+
     this.getShardName = function(n) {
         return shardConfigs[n]._id;
     };
diff --git a/jstests/sharding/csrs_upgrade/csrs_upgrade_set_shard_version.js b/jstests/sharding/csrs_upgrade/csrs_upgrade_set_shard_version.js
new file mode 100644
index 00000000000..cfcaf713b35
--- /dev/null
+++ b/jstests/sharding/csrs_upgrade/csrs_upgrade_set_shard_version.js
@@ -0,0 +1,77 @@
+/**
+ * This test performs an upgrade from SCCC config servers to CSRS config servers.
+ *
+ * After upgrading the config servers it performs a simple query and then verifies that the
+ * setShardVersion call that the query sent was sufficient to trigger the shard mongods
+ * to swap their in-memory catalog manager to CSRS mode.
+ *
+ * This test restarts nodes and expects the data to still be present.
+ * @tags: [requires_persistence]
+ */
+
+load("jstests/libs/csrs_upgrade_util.js");
+
+(function() {
+    "use strict";
+
+    var assertIsSCCCConnectionString = function(connStr) {
+        assert.eq(-1, connStr.indexOf('/'), connStr);
+        var hosts = connStr.split(',');
+        assert.eq(3, hosts.length, connStr);
+    };
+
+    var assertIsCSRSConnectionString = function(connStr) {
+        var setAndHosts = connStr.split('/');
+        assert.eq(2, setAndHosts.length, connStr);
+    };
+
+    var coordinator = new CSRSUpgradeCoordinator();
+    coordinator.setupSCCCCluster();
+
+    assert.commandWorked(coordinator.getMongos(0).adminCommand(
+        {split: coordinator.getShardedCollectionName(), middle: {_id: 0}}));
+
+    assert.commandWorked(coordinator.getMongos(0).adminCommand({
+        moveChunk: coordinator.getShardedCollectionName(),
+        find: {_id: 0},
+        to: coordinator.getShardName(1)
+    }));
+
+    jsTest.log("Inserting data into " + coordinator.getShardedCollectionName());
+    coordinator.getMongos(1)
+        .getCollection(coordinator.getShardedCollectionName())
+        .insert((function() {
+            var result = [];
+            var i;
+            for (i = -20; i < 20; ++i) {
+                result.push({_id: i});
+            }
+            return result;
+        }()));
+
+    coordinator.restartFirstConfigAsReplSet();
+    coordinator.startNewCSRSNodes();
+    coordinator.waitUntilConfigsCaughtUp();
+    coordinator.shutdownOneSCCCNode();
+    coordinator.allowAllCSRSNodesToVote();
+    coordinator.switchToCSRSMode();
+
+    assertIsSCCCConnectionString(
+        coordinator.getShard(0).adminCommand('serverStatus').sharding.configsvrConnectionString);
+    assertIsSCCCConnectionString(
+        coordinator.getShard(1).adminCommand('serverStatus').sharding.configsvrConnectionString);
+
+    assert.commandWorked(coordinator.getMongos(0).adminCommand('flushRouterConfig'));
+
+    assert.eq(40,
+              coordinator.getMongos(0)
+                  .getCollection(coordinator.getShardedCollectionName())
+                  .find()
+                  .itcount());
+
+    assertIsCSRSConnectionString(
+        coordinator.getShard(0).adminCommand('serverStatus').sharding.configsvrConnectionString);
+    assertIsCSRSConnectionString(
+        coordinator.getShard(1).adminCommand('serverStatus').sharding.configsvrConnectionString);
+
+}());
diff --git a/src/mongo/db/s/set_shard_version_command.cpp b/src/mongo/db/s/set_shard_version_command.cpp
index e0e691117fe..3d4c7e29e7c 100644
--- a/src/mongo/db/s/set_shard_version_command.cpp
+++ b/src/mongo/db/s/set_shard_version_command.cpp
@@ -370,7 +370,8 @@ private:
                       << givenConnStr
                       << " for the config server connection string, but has stored: "
                       << storedConnStr;
-                storedConnStr = givenConnStr;
+                grid.forwardingCatalogManager()->scheduleReplaceCatalogManagerIfNeeded(
+                    CatalogManager::ConfigServerMode::CSRS, givenConnStr);
 
                 return true;
             }
diff --git a/src/mongo/s/catalog/forwarding_catalog_manager.cpp b/src/mongo/s/catalog/forwarding_catalog_manager.cpp
index 63e75997d1e..9e0e7bb064d 100644
--- a/src/mongo/s/catalog/forwarding_catalog_manager.cpp
+++ b/src/mongo/s/catalog/forwarding_catalog_manager.cpp
@@ -216,7 +216,7 @@ Status ForwardingCatalogManager::ScopedDistLock::checkStatus() {
 }
 
 Status ForwardingCatalogManager::scheduleReplaceCatalogManagerIfNeeded(
-    ConfigServerMode desiredMode, StringData replSetName, const HostAndPort& knownServer) {
+    ConfigServerMode desiredMode, const ConnectionString& csrsConnStr) {
     stdx::lock_guard<stdx::mutex> lk(_observerMutex);
     const auto currentMode = _actual->getMode();
     if (currentMode == desiredMode) {
@@ -231,13 +231,14 @@ Status ForwardingCatalogManager::scheduleReplaceCatalogManagerIfNeeded(
     }
     invariant(desiredMode == ConfigServerMode::CSRS);
     if (_nextConfigChangeComplete.isValid()) {
-        if (_nextConfigConnectionString.getSetName() != replSetName) {
+        if (_nextConfigConnectionString.getSetName() != csrsConnStr.getSetName()) {
             severe() << "Conflicting new config server replica set names: "
-                     << _nextConfigConnectionString.getSetName() << " vs " << replSetName;
+                     << _nextConfigConnectionString.getSetName() << " vs "
+                     << csrsConnStr.getSetName();
             fassertFailed(28788);
         }
     } else {
-        _nextConfigConnectionString = ConnectionString::forReplicaSet(replSetName, {knownServer});
+        _nextConfigConnectionString = csrsConnStr;
         _nextConfigChangeComplete =
             fassertStatusOK(28789, _shardRegistry->getExecutor()->makeEvent());
         fassertStatusOK(
diff --git a/src/mongo/s/catalog/forwarding_catalog_manager.h b/src/mongo/s/catalog/forwarding_catalog_manager.h
index 15227b16c29..9da3adfd743 100644
--- a/src/mongo/s/catalog/forwarding_catalog_manager.h
+++ b/src/mongo/s/catalog/forwarding_catalog_manager.h
@@ -84,8 +84,7 @@ public:
      * Currently only supports going to CSRS mode from SCCC mode.
      */
     Status scheduleReplaceCatalogManagerIfNeeded(ConfigServerMode desiredMode,
-                                                 StringData replSetName,
-                                                 const HostAndPort& knownServer);
+                                                 const ConnectionString& csrsConnString);
 
     /**
      * Blocking method, which will waits for a previously scheduled catalog manager change to
diff --git a/src/mongo/s/client/sharding_connection_hook.cpp b/src/mongo/s/client/sharding_connection_hook.cpp
index 05bf53722a6..89396ede142 100644
--- a/src/mongo/s/client/sharding_connection_hook.cpp
+++ b/src/mongo/s/client/sharding_connection_hook.cpp
@@ -162,13 +162,23 @@ void ShardingConnectionHook::onCreate(DBClientBase* conn) {
                 str::stream() << "Unrecognized configsvr version number: " << configServerModeNumber
                               << ". Expected either 0 or 1",
                 configServerModeNumber == 0 || configServerModeNumber == 1);
+        auto configServerMode = configServerModeNumber == 0
+            ? CatalogManager::ConfigServerMode::SCCC
+            : CatalogManager::ConfigServerMode::CSRS;
+
+        // We still want to call scheduleReplaceCatalogManagerIfNeeded when configServerMode
+        // is SCCC to catch illegal downgrade attempts and return a useful error message.
+        // To enable that we use the default (invalid) ConnectionString when configServerMode
+        // is SCCC.
+        ConnectionString configConnString;
+        if (configServerMode == CatalogManager::ConfigServerMode::CSRS) {
+            configConnString = ConnectionString::forReplicaSet(
+                isMasterResponse["setName"].valueStringData(),
+                {static_cast<DBClientConnection*>(conn)->getServerHostAndPort()});
+        }
 
-        BSONElement setName = isMasterResponse["setName"];
         status = grid.forwardingCatalogManager()->scheduleReplaceCatalogManagerIfNeeded(
-            configServerModeNumber == 0 ? CatalogManager::ConfigServerMode::SCCC
-                                        : CatalogManager::ConfigServerMode::CSRS,
-            setName.type() == String ? setName.valueStringData() : StringData(),
-            static_cast<DBClientConnection*>(conn)->getServerHostAndPort());
+            configServerMode, configConnString);
         uassertStatusOK(status);
     }
 }
diff --git a/src/mongo/s/client/sharding_network_connection_hook.cpp b/src/mongo/s/client/sharding_network_connection_hook.cpp
index 1581a05617a..6562bc0f090 100644
--- a/src/mongo/s/client/sharding_network_connection_hook.cpp
+++ b/src/mongo/s/client/sharding_network_connection_hook.cpp
@@ -73,11 +73,20 @@ Status ShardingNetworkConnectionHook::validateHostImpl(
             const BSONElement setName = isMasterReply.data["setName"];
             auto configServerMode =
                 (configServerModeNumber == 0 ? ConfigServerMode::SCCC : ConfigServerMode::CSRS);
+
+            // We still want to call scheduleReplaceCatalogManagerIfNeeded when configServerMode
+            // is SCCC to catch illegal downgrade attempts and return a useful error message.
+            // To enable that we use the default (invalid) ConnectionString when configServerMode
+            // is SCCC.
+            ConnectionString configConnString;
+            if (configServerMode == ConfigServerMode::CSRS) {
+                configConnString =
+                    ConnectionString::forReplicaSet(setName.valueStringData(), {remoteHost});
+            }
+
             auto catalogSwapStatus =
                 grid.forwardingCatalogManager()->scheduleReplaceCatalogManagerIfNeeded(
-                    configServerMode,
-                    (setName.type() == String ? setName.valueStringData() : StringData()),
-                    remoteHost);
+                    configServerMode, configConnString);
             if (configServerMode == ConfigServerMode::CSRS && catalogSwapStatus.isOK() && forSCC) {
                 // Even though scheduleReplaceCatalogManagerIfNeeded didn't indicate that a catalog
                 // manager swap is needed, if this connection is part of a SyncClusterConnection,