SERVER-21082 Add more basic tests for addShard and removeShard

5 files changed, 71 insertions, 16 deletions

diff --git a/jstests/sharding/add_invalid_shard.js b/jstests/sharding/add_invalid_shard.js
new file mode 100644
index 00000000000..7dfa6d0f819
--- /dev/null
+++ b/jstests/sharding/add_invalid_shard.js
@@ -0,0 +1,49 @@
+/**
+ * Test that adding invalid or duplicate shards will fail.
+ */
+(function() {
+
+"use strict";
+
+var st = new ShardingTest({ shards: 1 });
+
+var configDB = st.s.getDB('config');
+var shardDoc = configDB.shards.findOne();
+
+// Can't add mongos as shard.
+assert.commandFailed(st.admin.runCommand({ addshard: st.s.host }));
+
+// Can't add config servers as shard.
+assert.commandFailed(st.admin.runCommand({ addshard: st._configDB }));
+
+var replTest = new ReplSetTest({ nodes: 2 });
+replTest.startSet({ oplogSize: 10 });
+replTest.initiate();
+
+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 }));
+
+assert.commandWorked(st.admin.runCommand({ addshard: rsConnStr, name: 'dummyRS' }));
+
+// Cannot add the same replSet shard host twice.
+assert.commandFailed(st.admin.runCommand({ addshard: rsConnStr }));
+
+// Cannot add the same replSet shard host twice even when using a unique shard name.
+assert.commandFailed(st.admin.runCommand({ addshard: rsConnStr, name: 'dupRS' }));
+
+// Cannot add the same replSet shard host twice even when using an valid variant of the replSet
+// connection string.
+var truncatedRSConnStr = rsConnStr.substring(0, rsConnStr.indexOf(','));
+assert.commandFailed(st.admin.runCommand({ addshard: truncatedRSConnStr, name: 'dupRS' }));
+
+// Cannot add the same stand alone shard host twice.
+assert.commandFailed(st.admin.runCommand({ addshard: shardDoc.host }));
+
+// Cannot add the same stand alone shard host twice even with a unique shard name.
+assert.commandFailed(st.admin.runCommand({ addshard: shardDoc.host, name: 'dupShard' }));
+
+replTest.stopSet();
+st.stop();
+
+})();
diff --git a/jstests/sharding/addshard1.js b/jstests/sharding/addshard1.js
index 5ae6f706687..1c4c1cfeca9 100644
--- a/jstests/sharding/addshard1.js
+++ b/jstests/sharding/addshard1.js
@@ -13,9 +13,19 @@ for (i=0; i<3; i++){
     numObjs++;
 }
 
+var configDB = s.s.getDB('config');
+assert.eq(null, configDB.databases.findOne({ _id: 'testDB' }));
+
 newShard = "myShard";
-assert( s.admin.runCommand( { addshard: "localhost:" + conn1.port , name: newShard } ).ok,
-        "did not accept non-duplicated shard" );
+assert.commandWorked(s.admin.runCommand({ addshard: "localhost:" + conn1.port,
+                                          name: newShard,
+                                          maxSize: 1024
+                                        }));
+
+assert.neq(null, configDB.databases.findOne({ _id: 'testDB' }));
+
+var newShardDoc = configDB.shards.findOne({ _id: newShard });
+assert.eq(1024, newShardDoc.maxSize);
 
 // a mongod with an existing database name should not be allowed to become a shard
 conn2 = MongoRunner.runMongod({});
diff --git a/jstests/sharding/addshard2.js b/jstests/sharding/addshard2.js
index e06c1bb990f..2ec634cae9a 100644
--- a/jstests/sharding/addshard2.js
+++ b/jstests/sharding/addshard2.js
@@ -42,9 +42,10 @@ conf.configsvr = true;
 rs5.initiate(conf);
 
 
-// step 1. name given
-assert(s.admin.runCommand({"addshard" : getHostName()+":" + conn1.port, "name" : "bar"}).ok,
-       "failed to add shard in step 1");
+// step 1. name given. maxSize zero means no limit. Make sure it is allowed.
+assert.commandWorked(s.admin.runCommand({ addshard: getHostName() + ":" + conn1.port,
+                                          name: "bar",
+                                          maxSize: 0 }));
 var shard = s.getDB("config").shards.findOne({"_id" : {"$nin" : ["shard0000"]}});
 assert(shard, "shard wasn't found");
 assert.eq("bar", shard._id, "shard has incorrect name");
@@ -88,6 +89,9 @@ assert(!s.admin.runCommand({
        }).ok,
        "accepted bad hostname in step 6");
 
+// Cannot add invalid stand alone host.
+assert.commandFailed(s.admin.runCommand({ addshard: 'dummy:12345' }));
+
 //
 // SERVER-17231 Adding replica set w/ set name = 'config'
 //
diff --git a/jstests/sharding/disallow_mongos_add_as_shard.js b/jstests/sharding/disallow_mongos_add_as_shard.js
deleted file mode 100644
index 524715bde25..00000000000
--- a/jstests/sharding/disallow_mongos_add_as_shard.js
+++ /dev/null
@@ -1,11 +0,0 @@
-(function() {
-
-var st = new ShardingTest({ name: "add_shard3", shards: 1 });
-
-var result = st.admin.runCommand({addshard: st.s.host});
-
-printjson(result);
-
-assert.eq(result.ok, 0, "don't add mongos as a shard");
-
-})();
diff --git a/jstests/sharding/remove1.js b/jstests/sharding/remove1.js
index 0143e49dc1b..22443aae938 100644
--- a/jstests/sharding/remove1.js
+++ b/jstests/sharding/remove1.js
@@ -8,6 +8,9 @@ assert.writeOK(s.config.databases.insert({ _id: 'needToMove',
                                            partitioned: false,
                                            primary: 'shard0000'}));
 
+// Returns an error when trying to remove a shard that doesn't exist.
+assert.commandFailed(s.admin.runCommand({ removeshard: "shardz" }));
+
 // first remove puts in draining mode, the second tells me a db needs to move, the third actually removes
 assert( s.admin.runCommand( { removeshard: "shard0000" } ).ok , "failed to start draining shard" );
 assert( !s.admin.runCommand( { removeshard: "shard0001" } ).ok , "allowed two draining shards" );