From 3192123cbc96d4c8121bd9849e70fd8b79eb37a9 Mon Sep 17 00:00:00 2001 From: XueruiFa Date: Thu, 13 May 2021 20:39:09 +0000 Subject: [PATCH] SERVER-56919: Add validation for memberIndex to reconfigToPSASet() shell helper --- .../reconfig_for_psa_set_shell.js | 104 ++++++++++++++++-- src/mongo/shell/utils.js | 37 ++++++- 2 files changed, 128 insertions(+), 13 deletions(-) diff --git a/jstests/noPassthrough/reconfig_for_psa_set_shell.js b/jstests/noPassthrough/reconfig_for_psa_set_shell.js index 2b0ba125432..79e149d1156 100644 --- a/jstests/noPassthrough/reconfig_for_psa_set_shell.js +++ b/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(); })(); diff --git a/src/mongo/shell/utils.js b/src/mongo/shell/utils.js index c115bb58313..d06c96b2989 100644 --- a/src/mongo/shell/utils.js +++ b/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;