SERVER-29653 Drop admin.system.keys on CSRS downgrade from 3.6 fcv to 3.4 fcv

6 files changed, 120 insertions, 17 deletions

diff --git a/jstests/multiVersion/clear_and_reinstate_keys.js b/jstests/multiVersion/clear_and_reinstate_keys.js
new file mode 100644
index 00000000000..be32e064594
--- /dev/null
+++ b/jstests/multiVersion/clear_and_reinstate_keys.js
@@ -0,0 +1,82 @@
+// This tests that the keys collection is correctly handled on downgrade and re-upgrade.
+//
+// In fcv 3.6, the CSRS generates keys for causal consistency in admin.system.keys. Replication only
+// replicates system collections that are in its whitelist. So if a new node were added to the CSRS
+// after downgrade, the keys collection would not replicate to the new node because it has a v3.4
+// whitelist without admin.system.keys. On re-upgrade, that node would be missing the keys.
+// Therefore, here we test that the collection is dropped as expected on downgrade and avoids the
+// re-upgrade inconsistencies.
+//
+// TODO: this test should be removed in v3.7 when it's no longer relevant.
+
+(function() {
+    "use strict";
+
+    load("jstests/multiVersion/libs/multi_cluster.js");
+    load("jstests/replsets/rslib.js");
+
+    let st = new ShardingTest({
+        shards: {},
+        mongos: 1,
+        mongosWaitsForKeys: true,
+        other: {
+            // Set catchUpTimeoutMillis for compatibility with v3.4.
+            configReplSetTestOptions: {settings: {catchUpTimeoutMillis: 2000}},
+            rsOptions: {settings: {catchUpTimeoutMillis: 2000}}
+        }
+    });
+    let keysCollectionName = "admin.system.keys";
+
+    jsTestLog("Verify the admin.system.keys collection after startup.");
+    st.configRS.awaitReplication();
+    let keyCount = st.configRS.getPrimary().getCollection(keysCollectionName).find().count();
+    assert.gte(keyCount, 2, "There should be admin.system.keys entries!! Found: " + keyCount);
+
+    jsTestLog("Set feature compatibility version to 3.4.");
+    assert.commandWorked(st.s0.adminCommand({setFeatureCompatibilityVersion: "3.4"}));
+
+    jsTestLog("Verify the admin.system.keys collection is gone.");
+    keyCount = st.configRS.getPrimary().getCollection('admin.system.keys').find().count();
+    assert.eq(keyCount, 0, "Expected no keys on downgrade to fcv 3.4");
+
+    jsTest.log("Downgrading mongos servers to binary v3.4.");
+    st.upgradeCluster("last-stable", {upgradeConfigs: false, upgradeShards: false});
+    st.restartMongoses();
+
+    jsTest.log("Downgrading shard servers to binary v3.4.");
+    st.upgradeCluster("last-stable", {upgradeConfigs: false, upgradeMongos: false});
+    st.restartMongoses();
+
+    jsTest.log("Downgrading config servers to binary v3.4.");
+    st.upgradeCluster("last-stable", {upgradeMongos: false, upgradeShards: false});
+    st.configRS.awaitNodesAgreeOnPrimary();
+    st.restartMongoses();
+
+    jsTest.log("Add new node to CSRS.");
+    let newConfigNode =
+        st.configRS.add({binVersion: 'last-stable', configsvr: "", storageEngine: "wiredTiger"});
+    st.configRS.reInitiate();
+    st.configRS.awaitSecondaryNodes();
+    st.configRS.awaitNodesAgreeOnConfigVersion();
+
+    jsTest.log("Upgrade cluster to binary v3.6.");
+    st.upgradeCluster("latest");
+
+    jsTestLog("Set feature compatibility version to 3.6.");
+    assert.commandWorked(st.s0.adminCommand({setFeatureCompatibilityVersion: "3.6"}));
+    st.restartMongoses();
+
+    jsTestLog("Verify the admin.system.keys collection on the newer CSRS node.");
+    let node = new Mongo(newConfigNode.host);
+    node.setSlaveOk(true);
+    let newNodeKeyCount = node.getCollection(keysCollectionName).find().count();
+
+    assert.gte(newNodeKeyCount,
+               2,
+               "There should be admin.system.keys entries on upgrade to fcv 3.6! Found: " +
+                   newNodeKeyCount);
+
+    jsTest.log('DONE!');
+    st.stop();
+
+})();
diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp
index 97ca07c6c6d..e5c50486f5c 100644
--- a/src/mongo/db/catalog/database_impl.cpp
+++ b/src/mongo/db/catalog/database_impl.cpp
@@ -453,7 +453,8 @@ Status DatabaseImpl::dropCollection(OperationContext* opCtx,
                     return Status(ErrorCodes::IllegalOperation,
                                   "turn off profiling before dropping system.profile collection");
             } else if (!(nss.isSystemDotViews() || nss.isHealthlog() ||
-                         nss == SessionsCollection::kSessionsNamespaceString)) {
+                         nss == SessionsCollection::kSessionsNamespaceString ||
+                         nss == NamespaceString::kSystemKeysCollectionName)) {
                 return Status(ErrorCodes::IllegalOperation,
                               str::stream() << "can't drop system collection " << fullns);
             }
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 9dfb4985ae4..ba418c0aac7 100644
--- a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
+++ b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
@@ -37,6 +37,8 @@
 #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/dbdirectclient.h"
+#include "mongo/db/keys_collection_document.h"
 #include "mongo/db/logical_time_validator.h"
 #include "mongo/db/repl/replication_coordinator.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
@@ -123,6 +125,8 @@ public:
 
             FeatureCompatibilityVersion::setTargetUpgrade(opCtx);
 
+            // First put UUIDs in the storage layer metadata. UUIDs will be generated for unsharded
+            // collections; shards will query the config server for sharded collection UUIDs.
             // Remove after 3.4 -> 3.6 upgrade.
             updateUUIDSchemaVersion(opCtx, /*upgrade*/ true);
 
@@ -137,18 +141,18 @@ public:
                         opCtx, requestedVersion));
             }
 
+            if (ShardingState::get(opCtx)->enabled()) {
+                // Ensure we try reading the keys for signing clusterTime immediately on upgrade.
+                // Remove after 3.4 -> 3.6 upgrade.
+                LogicalTimeValidator::get(opCtx)->forceKeyRefreshNow(opCtx);
+            }
+
             // Fail after adding UUIDs but before updating the FCV document.
             if (MONGO_FAIL_POINT(featureCompatibilityUpgrade)) {
                 exitCleanly(EXIT_CLEAN);
             }
 
             FeatureCompatibilityVersion::unsetTargetUpgradeOrDowngrade(opCtx, requestedVersion);
-
-            if (ShardingState::get(opCtx)->enabled()) {
-                // Ensure we try reading the keys for signing clusterTime immediately on upgrade.
-                // Remove after 3.4 -> 3.6 upgrade.
-                LogicalTimeValidator::get(opCtx)->forceKeyRefreshNow(opCtx);
-            }
         } else {
             invariant(requestedVersion == FeatureCompatibilityVersionCommandParser::kVersion34);
 
@@ -165,11 +169,31 @@ public:
                 exitCleanly(EXIT_CLEAN);
             }
 
-            // If config server, downgrade shards *before* downgrading self.
+            // If config server, downgrade shards *before* downgrading self; and clear the keys
+            // collection to ensure all shards will have the same key information on re-upgrade.
             if (serverGlobalParams.clusterRole == ClusterRole::ConfigServer) {
                 uassertStatusOK(
                     ShardingCatalogManager::get(opCtx)->setFeatureCompatibilityVersionOnShards(
                         opCtx, requestedVersion));
+
+                // Stop the background key generator thread from running before trying to drop the
+                // collection so we know the key won't just be recreated.
+                //
+                // Note: no need to restart the key generator if downgrade doesn't succeed at once
+                // because downgrade must be completed before upgrade is allowed; and clusterTime
+                // auth is inactive unless fully upgraded.
+                LogicalTimeValidator::get(opCtx)->enableKeyGenerator(opCtx, false);
+
+                DBDirectClient client(opCtx);
+                BSONObj result;
+                if (!client.dropCollection(NamespaceString::kSystemKeysCollectionName.toString(),
+                                           ShardingCatalogClient::kMajorityWriteConcern,
+                                           &result)) {
+                    Status status = getStatusFromCommandResult(result);
+                    if (status != ErrorCodes::NamespaceNotFound) {
+                        uassertStatusOK(status);
+                    }
+                }
             }
 
             // Remove after 3.6 -> 3.4 downgrade.
@@ -181,7 +205,6 @@ public:
         return true;
     }
 
-
 } setFeatureCompatibilityVersionCommand;
 
 }  // namespace
diff --git a/src/mongo/db/namespace_string.cpp b/src/mongo/db/namespace_string.cpp
index 035228b65ad..970b0be8e2a 100644
--- a/src/mongo/db/namespace_string.cpp
+++ b/src/mongo/db/namespace_string.cpp
@@ -70,7 +70,6 @@ const string escapeTable[256] = {
     ".252", ".253", ".254", ".255"};
 
 const char kServerConfiguration[] = "admin.system.version";
-const char kLogicalTimeKeysCollection[] = "admin.system.keys";
 
 constexpr auto listCollectionsCursorCol = "$cmd.listCollections"_sd;
 constexpr auto listIndexesCursorNSPrefix = "$cmd.listIndexes."_sd;
@@ -84,6 +83,7 @@ constexpr StringData NamespaceString::kLocalDb;
 constexpr StringData NamespaceString::kConfigDb;
 constexpr StringData NamespaceString::kSystemDotViewsCollectionName;
 constexpr StringData NamespaceString::kShardConfigCollectionsCollectionName;
+constexpr StringData NamespaceString::kSystemKeysCollectionName;
 
 const NamespaceString NamespaceString::kServerConfigurationNamespace(kServerConfiguration);
 const NamespaceString NamespaceString::kSessionTransactionsTableNamespace(
@@ -109,7 +109,7 @@ bool NamespaceString::isLegalClientSystemNS() const {
             return true;
         if (ns() == kServerConfiguration)
             return true;
-        if (ns() == kLogicalTimeKeysCollection)
+        if (ns() == kSystemKeysCollectionName)
             return true;
         if (ns() == "admin.system.new_users")
             return true;
diff --git a/src/mongo/db/namespace_string.h b/src/mongo/db/namespace_string.h
index 1f8a1187857..f110489916e 100644
--- a/src/mongo/db/namespace_string.h
+++ b/src/mongo/db/namespace_string.h
@@ -68,6 +68,9 @@ public:
     static constexpr StringData kShardConfigCollectionsCollectionName =
         "config.cache.collections"_sd;
 
+    // Name for causal consistency's key collection.
+    static constexpr StringData kSystemKeysCollectionName = "admin.system.keys"_sd;
+
     // Namespace for storing configuration data, which needs to be replicated if the server is
     // running as a replica set. Documents in this collection should represent some configuration
     // state of the server, which needs to be recovered/consulted at startup. Each document in this
diff --git a/src/mongo/shell/shardingtest.js b/src/mongo/shell/shardingtest.js
index 91c3d7f0d14..ed6bb36b3d9 100644
--- a/src/mongo/shell/shardingtest.js
+++ b/src/mongo/shell/shardingtest.js
@@ -1108,12 +1108,6 @@ var ShardingTest = function(params) {
 
     var pathOpts = {testName: testName};
 
-    for (var k in otherParams) {
-        if (k.startsWith("rs") && otherParams[k] != undefined) {
-            break;
-        }
-    }
-
     this._connections = [];
     this._rs = [];
     this._rsObjects = [];