SERVER-56227 Add user-facing command for permitMigrations using DDL coordinator

8 files changed, 64 insertions, 34 deletions

diff --git a/jstests/core/views/views_all_commands.js b/jstests/core/views/views_all_commands.js
index 260eaef9b39..b3c9a356bc6 100644
--- a/jstests/core/views/views_all_commands.js
+++ b/jstests/core/views/views_all_commands.js
@@ -123,6 +123,7 @@ let viewsCommandTests = {
     _shardsvrCloneCatalogData: {skip: isAnInternalCommand},
     _shardsvrMovePrimary: {skip: isAnInternalCommand},
     _shardsvrRenameCollection: {skip: isAnInternalCommand},
+    _shardsvrSetAllowMigrations: {skip: isAnInternalCommand},
     _shardsvrShardCollection: {skip: isAnInternalCommand},
     _transferMods: {skip: isAnInternalCommand},
     abortTransaction: {skip: isUnrelated},
@@ -500,6 +501,16 @@ let viewsCommandTests = {
     saslContinue: {skip: isUnrelated},
     saslStart: {skip: isUnrelated},
     serverStatus: {command: {serverStatus: 1}, skip: isUnrelated},
+    setAllowMigrations: {
+        command: {setAllowMigrations: "test.view", allowMigrations: false},
+        setup: function(conn) {
+            assert.commandWorked(conn.adminCommand({enableSharding: "test"}));
+        },
+        expectedErrorCode: ErrorCodes.NamespaceNotSharded,
+        skipStandalone: true,
+        expectFailure: true,
+        isAdminCommand: true
+    },
     setIndexCommitQuorum: {skip: isUnrelated},
     setCommittedSnapshot: {skip: isAnInternalCommand},
     setDefaultRWConcern: {skip: isUnrelated},
diff --git a/jstests/replsets/db_reads_while_recovering_all_commands.js b/jstests/replsets/db_reads_while_recovering_all_commands.js
index 7bcd62f3de3..c994bed47f2 100644
--- a/jstests/replsets/db_reads_while_recovering_all_commands.js
+++ b/jstests/replsets/db_reads_while_recovering_all_commands.js
@@ -65,6 +65,7 @@ const allCommands = {
     _recvChunkStatus: {skip: isPrimaryOnly},
     _shardsvrCloneCatalogData: {skip: isPrimaryOnly},
     _shardsvrMovePrimary: {skip: isPrimaryOnly},
+    _shardsvrSetAllowMigrations: {skip: isPrimaryOnly},
     _shardsvrShardCollection: {skip: isPrimaryOnly},
     _transferMods: {skip: isPrimaryOnly},
     abortTransaction: {skip: isPrimaryOnly},
diff --git a/jstests/sharding/database_versioning_all_commands.js b/jstests/sharding/database_versioning_all_commands.js
index 2121fdcc632..7700739c9ec 100644
--- a/jstests/sharding/database_versioning_all_commands.js
+++ b/jstests/sharding/database_versioning_all_commands.js
@@ -607,6 +607,7 @@ let testCases = {
     saslContinue: {skip: "not on a user database"},
     saslStart: {skip: "not on a user database"},
     serverStatus: {skip: "executes locally on mongos (not sent to any remote node)"},
+    setAllowMigrations: {skip: "not on a user database"},
     setDefaultRWConcern: {skip: "always targets the config server"},
     setIndexCommitQuorum: {
         run: {
diff --git a/jstests/sharding/move_chunk_permitMigrations.js b/jstests/sharding/move_chunk_permitMigrations.js
index 849f0d3ed2a..bb58f3012dc 100644
--- a/jstests/sharding/move_chunk_permitMigrations.js
+++ b/jstests/sharding/move_chunk_permitMigrations.js
@@ -3,7 +3,8 @@
  * moveChunk and disables the balancer.
  *
  * @tags: [
- *   multiversion_incompatible
+ *   multiversion_incompatible,
+ *   does_not_support_stepdowns,
  * ]
  */
 (function() {
@@ -11,11 +12,10 @@
 
 load('jstests/libs/fail_point_util.js');
 load('jstests/libs/parallel_shell_helpers.js');
-load("jstests/sharding/libs/shard_versioning_util.js");
 
 const st = new ShardingTest({shards: 2});
 const configDB = st.s.getDB("config");
-const dbName = 'PermitMigrations';
+const dbName = 'AllowMigrations';
 
 // Resets database dbName and enables sharding and establishes shard0 as primary, test case agnostic
 const setUpDb = function setUpDatabaseAndEnableSharding() {
@@ -24,15 +24,13 @@ const setUpDb = function setUpDatabaseAndEnableSharding() {
         st.s.adminCommand({enableSharding: dbName, primaryShard: st.shard0.shardName}));
 };
 
-const setPermitMigrations = function(ns, permit) {
-    // For now update the flag manually, a user-facing command will be implemented with
-    // SERVER-56227.
-    assert.commandWorked(configDB.collections.updateOne(
-        {_id: ns}, {$set: {permitMigrations: permit}}, {writeConcern: {w: "majority"}}));
+// Use the setAllowMigrations command to set the permitMigrations flag in the collection.
+const setAllowMigrationsCmd = function(ns, allow) {
+    assert.commandWorked(st.s.adminCommand({setAllowMigrations: ns, allowMigrations: allow}));
 };
 
-// Tests that moveChunk does not succeed when {permitMigrations: false}
-(function testPermitMigrationsFalsePreventsMoveChunk() {
+// Tests that moveChunk does not succeed when setAllowMigrations is called with a false value.
+(function testSetAllowMigrationsFalsePreventsMoveChunk() {
     setUpDb();
 
     const collName = "collA";
@@ -42,31 +40,19 @@ const setPermitMigrations = function(ns, permit) {
     assert.commandWorked(st.s.getDB(dbName).getCollection(collName).insert({_id: 1}));
     assert.commandWorked(st.s.adminCommand({shardCollection: ns, key: {_id: 1}}));
 
-    // Confirm that an inProgress moveChunk fails once {permitMigrations: false}
-    const fp = configureFailPoint(st.shard0, "moveChunkHangAtStep4");
-    const awaitResult = startParallelShell(
-        funWithArgs(function(ns, toShardName) {
-            assert.commandFailedWithCode(
-                db.adminCommand({moveChunk: ns, find: {_id: 0}, to: toShardName}),
-                ErrorCodes.ConflictingOperationInProgress);
-        }, ns, st.shard1.shardName), st.s.port);
-    fp.wait();
-    setPermitMigrations(ns, false);
-    fp.off();
-    awaitResult();
-
-    // {permitMigrations: false} is set, sending a new moveChunk command should also fail.
+    setAllowMigrationsCmd(ns, false);
+
+    // setAllowMigrations was called, sending a new moveChunk command should fail.
     assert.commandFailedWithCode(
         st.s.adminCommand({moveChunk: ns, find: {_id: 0}, to: st.shard1.shardName}),
         ErrorCodes.ConflictingOperationInProgress);
 })();
 
-// Tests {permitMigrations: false} disables balancing for collB and does not interfere with
+// Tests setAllowMigrations disables balancing for collB and does not interfere with
 // balancing for collA.
 //
 // collBSetParams specify the field(s) that will be set on the collB in config.collections.
-const testBalancer = function testPermitMigrationsFalseDisablesBalancer(permitMigrations,
-                                                                        collBSetNoBalanceParam) {
+const testBalancer = function(setAllowMigrations, collBSetNoBalanceParam) {
     setUpDb();
 
     const collAName = "collA";
@@ -99,11 +85,12 @@ const testBalancer = function testPermitMigrationsFalseDisablesBalancer(permitMi
             configDB.chunks.countDocuments({ns: coll.getFullName(), shard: st.shard0.shardName}));
     }
 
-    jsTestLog(`Disabling balancing of ${collB.getFullName()} with permitMigrations ${
-        permitMigrations} and parameters ${tojson(collBSetNoBalanceParam)}`);
+    jsTestLog(`Disabling balancing of ${collB.getFullName()} with setAllowMigrations ${
+        setAllowMigrations} and parameters ${tojson(collBSetNoBalanceParam)}`);
     assert.commandWorked(
         configDB.collections.update({_id: collB.getFullName()}, {$set: collBSetNoBalanceParam}));
-    setPermitMigrations(collB.getFullName(), permitMigrations);
+
+    setAllowMigrationsCmd(collB.getFullName(), setAllowMigrations);
 
     st.startBalancer();
     assert.soon(() => {
@@ -127,13 +114,38 @@ const testBalancer = function testPermitMigrationsFalseDisablesBalancer(permitMi
     assert.eq(collBBalanceStatus.balancerCompliant, false);
     assert.eq(collBBalanceStatus.firstComplianceViolation, 'chunksImbalance');
     assert.eq(
-        4, configDB.chunks.countDocuments({ns: collB.getFullName(), shard: st.shard0.shardName}));
+        4, configDB.chunks.countDocuments({ns: collA.getFullName(), shard: st.shard0.shardName}));
+};
+
+const testSetAllowMigrationsCommand = function() {
+    setUpDb();
+
+    const collName = "foo";
+    const ns = dbName + "." + collName;
+
+    assert.commandWorked(st.s.adminCommand({shardCollection: ns, key: {x: 1}}));
+
+    // Use setAllowMigrations to forbid migrations from happening
+    setAllowMigrationsCmd(ns, false);
+
+    // Check that allowMigrations has been set to 'false' on the configsvr config.collections.
+    assert.eq(false, configDB.collections.findOne({_id: ns}).permitMigrations);
+
+    // Use setAllowMigrations to allow migrations to happen
+    setAllowMigrationsCmd(ns, true);
+
+    // Check that permitMigrations has been unset (that implies migrations are allowed) on the
+    // configsvr config.collections.
+    assert.eq(undefined, configDB.collections.findOne({_id: ns}).permitMigrations);
 };
 
 // Test cases that should disable the balancer.
-testBalancer(false /* permitMigrations */, {});
-testBalancer(false /* permitMigrations */, {noBalance: false});
-testBalancer(false /* permitMigrations */, {noBalance: true});
+testBalancer(false /* setAllowMigrations */, {});
+testBalancer(false /* setAllowMigrations */, {noBalance: false});
+testBalancer(false /* setAllowMigrations */, {noBalance: true});
+
+// Test the setAllowMigrations command.
+testSetAllowMigrationsCommand();
 
 st.stop();
 })();
diff --git a/jstests/sharding/read_write_concern_defaults_application.js b/jstests/sharding/read_write_concern_defaults_application.js
index b1afbbadd8b..c88217fd41a 100644
--- a/jstests/sharding/read_write_concern_defaults_application.js
+++ b/jstests/sharding/read_write_concern_defaults_application.js
@@ -116,6 +116,7 @@ let testCases = {
     _shardsvrCloneCatalogData: {skip: "internal command"},
     _shardsvrMovePrimary: {skip: "internal command"},
     _shardsvrRenameCollection: {skip: "internal command"},
+    _shardsvrSetAllowMigrations: {skip: "internal command"},
     _shardsvrShardCollection: {skip: "internal command"},
     _transferMods: {skip: "internal command"},
     abortTransaction: {
@@ -593,6 +594,7 @@ let testCases = {
     saslContinue: {skip: "does not accept read or write concern"},
     saslStart: {skip: "does not accept read or write concern"},
     serverStatus: {skip: "does not accept read or write concern"},
+    setAllowMigrations: {skip: "does not accept read or write concern"},
     setCommittedSnapshot: {skip: "internal command"},
     setDefaultRWConcern: {skip: "special case (must run after all other commands)"},
     setFeatureCompatibilityVersion: {skip: "does not accept read or write concern"},
diff --git a/jstests/sharding/safe_secondary_reads_drop_recreate.js b/jstests/sharding/safe_secondary_reads_drop_recreate.js
index ae88a62b0a7..28e1a82fb87 100644
--- a/jstests/sharding/safe_secondary_reads_drop_recreate.js
+++ b/jstests/sharding/safe_secondary_reads_drop_recreate.js
@@ -286,6 +286,7 @@ let testCases = {
     saslContinue: {skip: "primary only"},
     saslStart: {skip: "primary only"},
     serverStatus: {skip: "does not return user data"},
+    setAllowMigrations: {skip: "primary only"},
     setCommittedSnapshot: {skip: "does not return user data"},
     setDefaultRWConcern: {skip: "primary only"},
     setIndexCommitQuorum: {skip: "primary only"},
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 20e21459b80..c09ac7f2627 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
@@ -358,6 +358,7 @@ let testCases = {
     saslContinue: {skip: "primary only"},
     saslStart: {skip: "primary only"},
     serverStatus: {skip: "does not return user data"},
+    setAllowMigrations: {skip: "primary only"},
     setCommittedSnapshot: {skip: "does not return user data"},
     setDefaultRWConcern: {skip: "primary only"},
     setIndexCommitQuorum: {skip: "primary only"},
diff --git a/jstests/sharding/safe_secondary_reads_single_migration_waitForDelete.js b/jstests/sharding/safe_secondary_reads_single_migration_waitForDelete.js
index 6b2d4f6f90d..d5d33c89cce 100644
--- a/jstests/sharding/safe_secondary_reads_single_migration_waitForDelete.js
+++ b/jstests/sharding/safe_secondary_reads_single_migration_waitForDelete.js
@@ -293,6 +293,7 @@ let testCases = {
     saslContinue: {skip: "primary only"},
     saslStart: {skip: "primary only"},
     serverStatus: {skip: "does not return user data"},
+    setAllowMigrations: {skip: "primary only"},
     setCommittedSnapshot: {skip: "does not return user data"},
     setDefaultRWConcern: {skip: "primary only"},
     setIndexCommitQuorum: {skip: "primary only"},