SERVER-59716 Removed "disabled_due_to_server_58295" tag from add_invalid_shard

2 files changed, 82 insertions, 37 deletions

diff --git a/jstests/multiVersion/genericSetFCVUsage/add_invalid_shard.js b/jstests/multiVersion/genericSetFCVUsage/add_invalid_shard.js
index c706d6c22f6..df9a2b7a4fd 100644
--- a/jstests/multiVersion/genericSetFCVUsage/add_invalid_shard.js
+++ b/jstests/multiVersion/genericSetFCVUsage/add_invalid_shard.js
@@ -1,58 +1,104 @@
 /**
  * Test that adding invalid or duplicate shards will fail.
  *
- * @tags: [disabled_due_to_server_58295]
  */
 (function() {
 
 "use strict";
 
-var st = new ShardingTest({shards: 1});
+const st = new ShardingTest({shards: 1});
 
-var configDB = st.s.getDB('config');
-var shardDoc = configDB.shards.findOne();
+const configDB = st.s.getDB('config');
+const shardDoc = configDB.shards.findOne();
 
-// Can't add mongos as shard.
-assert.commandFailedWithCode(st.admin.runCommand({addshard: st.s.host}),
-                             ErrorCodes.IllegalOperation);
+/**
+ * @summary Starts a new replica set with the specified binVersion and replicaName.
+ * Tests that we can't add that replicaSet as a shard to our existing cluster. Once the assertion
+ * completes successfully, it will stop the replica set.
+ * @param {string} binVersion The binVersion to configure the replica set with.
+ * @param {string} replicaName  The name given to the replica set.
+ */
+const executeOldBinaryTest = (binVersion) => {
+    jsTest.log(`Executing binary test for binVersion ${binVersion}`);
+    const oldBinaryReplTest = new ReplSetTest({nodes: 2, nodeOptions: {binVersion, shardsvr: ""}});
+    oldBinaryReplTest.startSet();
+    oldBinaryReplTest.initiate();
 
-// Can't add a mongod with a lower binary version than our featureCompatibilityVersion.
-var lastLTSMongod = MongoRunner.runMongod({binVersion: "last-lts", shardsvr: ""});
-assert.commandFailedWithCode(st.admin.runCommand({addshard: lastLTSMongod.host}),
-                             ErrorCodes.IncompatibleServerVersion);
-MongoRunner.stopMongod(lastLTSMongod);
+    const connectionString = oldBinaryReplTest.getURL();
 
-// Can't add a mongod with a lower binary version than our featureCompatibilityVersion.
-var lastContinuousMongod = MongoRunner.runMongod({binVersion: "last-continuous", shardsvr: ""});
-assert.commandFailedWithCode(st.admin.runCommand({addshard: lastContinuousMongod.host}),
-                             ErrorCodes.IncompatibleServerVersion);
-MongoRunner.stopMongod(lastContinuousMongod);
+    assert.commandFailed(st.admin.runCommand({addshard: connectionString}));
+    oldBinaryReplTest.stopSet();
+};
 
-// Can't add config servers as shard.
-assert.commandFailed(st.admin.runCommand({addshard: st._configDB}));
+/**
+ *
+ * @callback TestCaseGenerator
+ * @param {String} connectionString - The host URL of the replica set.
+ * @returns {Array<{addshard: string, name: string, setup: any}>} - An array of test cases to
+ *     execute.
+ */
 
-var replTest = new ReplSetTest({nodes: 2, nodeOptions: {shardsvr: ""}});
-replTest.startSet({oplogSize: 10});
-replTest.initiate();
+/**
+ * @summary Receives a function that it can call to generate addshard command objects.
+ * It will then go through each command and assert that it will fail. After going through each
+ * command, it will stop the replica set.
+ * @param {TestCaseGenerator} createStandardTestCases
+ */
+const executeStandardTests = (createStandardTestCases) => {
+    jsTest.log("Starting to execute the standard test cases");
+    const replTest = new ReplSetTest({nodes: 2, nodeOptions: {shardsvr: ""}});
+    replTest.startSet({oplogSize: 10});
+    replTest.initiate();
+
+    const addShardCommands = createStandardTestCases(replTest.getURL());
 
-var rsConnStr = replTest.getURL();
-// Can't add replSet as shard if the name doesn't match the replSet config.
-assert.commandFailed(st.admin.runCommand({addshard: "prefix_" + rsConnStr}));
+    addShardCommands.forEach(({addshard, name, setup}) => {
+        jsTest.log(`About to run addshard command with value ${addshard} and name ${name}`);
+        if (setup) {
+            setup();
+        }
+        assert.commandFailed(st.admin.runCommand({addshard: addshard, name: name}));
+    });
 
-assert.commandWorked(st.admin.runCommand({addshard: rsConnStr, name: 'dummyRS'}));
+    replTest.stopSet();
+};
 
-// Cannot add the same replSet shard host twice when using a unique shard name.
-assert.commandFailed(st.admin.runCommand({addshard: rsConnStr, name: 'dupRS'}));
+// ---- TEST CASES ----
+
+// Can't add a mongod with a lower binary version than our featureCompatibilityVersion.
+executeOldBinaryTest("last-lts");
+executeOldBinaryTest("last-continuous");
 
-// Cannot add the same stand alone shard host twice with a unique shard name.
-assert.commandFailed(st.admin.runCommand({addshard: shardDoc.host, name: 'dupShard'}));
+executeStandardTests((replicaSetConnectionURL) => {
+    const truncatedRSConnStr =
+        replicaSetConnectionURL.substring(0, replicaSetConnectionURL.indexOf(','));
 
-// Cannot add a replica set connection string containing a member that isn't actually part of
-// the replica set.
-var truncatedRSConnStr = rsConnStr.substring(0, rsConnStr.indexOf(','));
-assert.commandFailed(
-    st.admin.runCommand({addshard: truncatedRSConnStr + 'fakehost', name: 'dummyRS'}));
+    return [
+        {
+            // Can't add replSet as shard if the name doesn't match the replSet config.
+            addshard: "prefix_" + replicaSetConnectionURL,
+        },
+        {
+            // Cannot add the same replSet shard host twice when using a unique shard name.
+            addshard: replicaSetConnectionURL,
+            name: 'dupRS',
+            setup: () => (assert.commandWorked(
+                st.admin.runCommand({addshard: replicaSetConnectionURL, name: 'dummyRS'})))
+        },
+        {
+            // Cannot add a replica set connection string containing a member that isn't actually
+            // part of the replica set.
+            addshard: truncatedRSConnStr + 'fakehost',
+            name: 'dummyRS'
+        },
+        {addshard: shardDoc.host, name: 'dupShard'},
+        {addshard: st._configDB},  // Can't add config servers as shard.
+    ];
+});
+
+// Can't add mongos as shard.
+assert.commandFailedWithCode(st.admin.runCommand({addshard: st.s.host}),
+                             ErrorCodes.IllegalOperation);
 
-replTest.stopSet();
 st.stop();
 })();
diff --git a/jstests/sharding/rewrite_state_change_errors.js b/jstests/sharding/rewrite_state_change_errors.js
index 831696181ad..da89f947f63 100644
--- a/jstests/sharding/rewrite_state_change_errors.js
+++ b/jstests/sharding/rewrite_state_change_errors.js
@@ -10,7 +10,6 @@
  * This behavior can be overridden by adding the bool `allowRewriteStateChange`
  * to the failpoint's configuration object.
  *
- * @tags: [disabled_due_to_server_58295]
  */
 
 (function() {