SERVER-56919: Add validation for memberIndex to reconfigToPSASet() shell helper

jstests/noPassthrough/reconfig_for_psa_set_shell.js

@@ -19,24 +19,106 @@ rst.initiateWithHighElectionTimeout();
 const primary = rst.getPrimary();
 assert.eq(primary, rst.nodes[0], "the primary should be the node at index 0");
 
-// Verify that a reconfig that directly gives the secondary 'votes: 1' and 'priority: 1' will fail.
-const config = rst.getReplSetConfigFromNode();
+// Store the old config so that we can reset to it after every successful reconfig.
+const originalConfig = rst.getReplSetConfigFromNode();
+
+// Verify that directly calling the standard 'rs.reconfig()' function to give the secondary 'votes:
+// 1' and 'priority: 1' will fail.
+let config = rst.getReplSetConfigFromNode();
 config.members[1].votes = 1;
 config.members[1].priority = 1;
 
-let reconfigScript = `assert.commandFailedWithCode(rs.reconfig(${
+jsTestLog("Testing standard rs.reconfig() function");
+const reconfigScript = `assert.commandFailedWithCode(rs.reconfig(${
     tojson(config)}), ErrorCodes.NewReplicaSetConfigurationIncompatible)`;
-let result = runMongoProgram('mongo', '--port', primary.port, '--eval', reconfigScript);
+const result = runMongoProgram('mongo', '--port', primary.port, '--eval', reconfigScript);
 assert.eq(0, result, `reconfig did not fail with expected error code`);
 
-// Verify that calling 'reconfigForPSASet()' will succeed.
-reconfigScript = `assert.commandWorked(rs.reconfigForPSASet(1, ${tojson(config)}))`;
-result = runMongoProgram('mongo', '--port', primary.port, '--eval', reconfigScript);
-assert.eq(0, result, `reconfig did not succeed as expected`);
+const runReconfigForPSASet = (memberIndex, config, shouldSucceed, endPriority = 1) => {
+    jsTestLog(`Testing with memberIndex ${memberIndex} and config ${tojson(config)}`);
 
-const replSetGetConfig = assert.commandWorked(primary.adminCommand({replSetGetConfig: 1})).config;
-assert.eq(1, replSetGetConfig.members[1].votes);
-assert.eq(1, replSetGetConfig.members[1].priority);
+    const reconfigScript =
+        `assert.commandWorked(rs.reconfigForPSASet(${memberIndex}, ${tojson(config)}))`;
+    const result = runMongoProgram('mongo', '--port', primary.port, '--eval', reconfigScript);
+    if (shouldSucceed) {
+        assert.eq(0, result, 'expected reconfigToPSASet to succeed, but it failed');
+
+        const replSetGetConfig =
+            assert.commandWorked(primary.adminCommand({replSetGetConfig: 1})).config;
+        assert.eq(1, replSetGetConfig.members[1].votes);
+        assert.eq(endPriority, replSetGetConfig.members[1].priority);
+
+        // Reset the config back to the original config.
+        originalConfig.members[memberIndex].votes = 0;
+        originalConfig.members[memberIndex].priority = 0;
+        const reconfigToOriginalConfig =
+            `assert.commandWorked(rs.reconfig(${tojson(originalConfig)}))`;
+        assert.eq(
+            0,
+            runMongoProgram('mongo', '--port', primary.port, '--eval', reconfigToOriginalConfig));
+    } else {
+        assert.neq(0, result, 'expected reconfigToPSASet to fail, but it succeeded');
+    }
+};
+
+// Succeed with a reconfig to a standard PSA set, where the secondary has 'votes: 1' and 'priority:
+// 1'.
+jsTestLog("Testing reconfigForPSASet() succeeded: standard PSA set");
+runReconfigForPSASet(1, config, true /* shouldSucceed */);
+
+// Fail when 'memberIndex' does not refer to a node in the new config.
+jsTestLog("Testing reconfigForPSASet() failed: memberIndex out of bounds");
+runReconfigForPSASet(3, config, false /* shouldSucceed */);
+
+// Fail when the node in the new config at 'memberIndex' is not a voter.
+jsTestLog("Testing reconfigForPSASet() failed: node at memberIndex is not a voter");
+config.members[1].votes = 0;
+config.members[1].priority = 0;
+runReconfigForPSASet(1, config, false /* shouldSucceed */);
+
+// Test that reconfigToPSASet() will succeed when we are adding a new node at a specific
+// 'memberIndex'.
+jsTestLog("Testing reconfigForPSASet() succeeded: adding new node");
+
+// First remove the node at index 1 to simulate a two-node replica set.
+config = rst.getReplSetConfigFromNode();
+const filteredMembers = config.members.filter(member => member._id !== 1);
+config.members = filteredMembers;
+config.version += 1;
+assert.commandWorked(primary.adminCommand({replSetReconfig: config}));
+
+// Attempt to add a node and assert that the reconfigToPSASet() call is successful.
+config.members = originalConfig.members;
+config.members[1].votes = 1;
+config.members[1].priority = 1;
+config.version += 1;
+runReconfigForPSASet(1, config, true /* shouldSucceed */);
+
+// Test that reconfigToPSASet() will succeed even if the priority of the node is 0 in the final
+// config.
+jsTestLog("Testing reconfigForPSASet() succeeded: priority of node is 0 in final config");
+config = rst.getReplSetConfigFromNode();
+config.members[1].votes = 1;
+config.members[1].priority = 0;
+runReconfigForPSASet(1, config, true /* shouldSucceed */, 0 /* endPriority */);
+
+// Test that reconfigToPSASet() will fail if the node at 'memberIndex' exists in the old config but
+// was a voter. It should fail because this is not the use case of the helper function.
+jsTestLog("Testing reconfigForPSASet() failed: node at memberIndex was a voter in old config");
+
+// First turn the secondary into a voting node but with priority 0.
+config = rst.getReplSetConfigFromNode();
+config.members[1].votes = 1;
+config.members[1].priority = 0;
+config.version += 1;
+assert.commandWorked(primary.adminCommand({replSetReconfig: config}));
+
+// Now attempt a reconfig to give the secondary priority 1 via the reconfigForPSASet() function.
+// This should fail because this function should not be used if the secondary is a voter in the old
+// config.
+config.members[1].priority = 1;
+config.version += 1;
+runReconfigForPSASet(1, config, false /* shouldSucceed */);
 
 rst.stopSet();
 })();

src/mongo/shell/utils.js

@@ -1568,13 +1568,38 @@ rs.reconfig = function(cfg, options) {
     }
     return this._runCmd(cmd);
 };
+
+_validateMemberIndex = function(memberIndex, newConfig) {
+    const newMemberConfig = newConfig.members[memberIndex];
+    assert(newMemberConfig, `Node at index ${memberIndex} does not exist in the new config`);
+    assert.eq(1,
+              newMemberConfig.votes,
+              `Node at index ${memberIndex} must have {votes: 1} in the new config`);
+
+    // Use memberId to compare nodes across configs.
+    const memberId = newMemberConfig._id;
+    const oldConfig = rs.conf();
+    const oldMemberConfig = oldConfig.members.find(member => member._id === memberId);
+
+    // If the node doesn't exist in the old config, we are adding it as a new node. Skip validating
+    // the node in the old config.
+    if (!oldMemberConfig) {
+        return;
+    }
+
+    assert(!oldMemberConfig.votes,
+           `Node at index ${memberIndex} must have {votes: 0} in the old config`);
+};
+
 rs.reconfigForPSASet = function(memberIndex, cfg, options) {
+    _validateMemberIndex(memberIndex, cfg);
+
     const memberPriority = cfg.members[memberIndex].priority;
     print(
         `Running first reconfig to give member at index ${memberIndex} { votes: 1, priority: 0 }`);
     cfg.members[memberIndex].votes = 1;
     cfg.members[memberIndex].priority = 0;
-    const res = rs.reconfig(cfg, options);
+    let res = rs.reconfig(cfg, options);
     if (!res.ok) {
         return res;
     }
@@ -1582,7 +1607,15 @@ rs.reconfigForPSASet = function(memberIndex, cfg, options) {
     print(`Running second reconfig to give member at index ${memberIndex} { priority: ${
         memberPriority} }`);
     cfg.members[memberIndex].priority = memberPriority;
-    return rs.reconfig(cfg, options);
+
+    // If the first reconfig added a new node, the second config will not succeed until the
+    // automatic reconfig to remove the 'newlyAdded' field is completed. Retry the second reconfig
+    // until it succeeds in that case.
+    assert.soon(() => {
+        res = rs.reconfig(cfg, options);
+        return res.ok;
+    });
+    return res;
 };
 rs.add = function(hostport, arb) {
     let res;