SERVER-35903 Free monitoring disable without enabling does not work

8 files changed, 81 insertions, 34 deletions

diff --git a/jstests/free_mon/free_mon_disable.js b/jstests/free_mon/free_mon_disable.js
new file mode 100644
index 00000000000..2de9de1c651
--- /dev/null
+++ b/jstests/free_mon/free_mon_disable.js
@@ -0,0 +1,35 @@
+// Validate disable works
+//
+load("jstests/free_mon/libs/free_mon.js");
+
+(function() {
+    'use strict';
+
+    let mock_web = new FreeMonWebServer();
+
+    mock_web.start();
+
+    let options = {
+        setParameter: "cloudFreeMonitoringEndpointURL=" + mock_web.getURL(),
+        freeMonitoringTag: "foo",
+        verbose: 1,
+    };
+
+    const conn = MongoRunner.runMongod(options);
+    assert.neq(null, conn, 'mongod was unable to start up');
+
+    assert.commandWorked(conn.adminCommand({setFreeMonitoring: 1, action: "disable"}));
+
+    const stats = mock_web.queryStats();
+    print(tojson(stats));
+
+    assert.eq(stats.registers, 0);
+
+    assert.eq(FreeMonGetStatus(conn).state, "disabled");
+
+    assert.eq(FreeMonGetServerStatus(conn).state, "disabled");
+
+    MongoRunner.stopMongod(conn);
+
+    mock_web.stop();
+})();
diff --git a/jstests/free_mon/free_mon_rs_delete.js b/jstests/free_mon/free_mon_rs_delete.js
index 6f14d17d1ad..335d93cfef4 100644
--- a/jstests/free_mon/free_mon_rs_delete.js
+++ b/jstests/free_mon/free_mon_rs_delete.js
@@ -24,8 +24,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     mock_web.waitRegisters(2);
 
-    assert.eq(FreeMonGetStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetStatus(rst.getSecondary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
 
     const qs1 = mock_web.queryStats();
 
@@ -42,8 +42,8 @@ load("jstests/free_mon/libs/free_mon.js");
     assert.eq(qs1.registers, qs2.registers);
 
     // Make sure we are back to the initial state.
-    assert.eq(FreeMonGetStatus(rst.getPrimary()).state, 'undecided');
-    assert.eq(FreeMonGetStatus(rst.getSecondary()).state, 'undecided');
+    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'undecided');
+    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'undecided');
 
     // Enable it again to be sure we can resume
     assert.commandWorked(rst.getPrimary().adminCommand({setFreeMonitoring: 1, action: "enable"}));
@@ -52,8 +52,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     sleep(20 * 1000);
 
-    assert.eq(FreeMonGetStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetStatus(rst.getSecondary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
 
     rst.stopSet();
 
diff --git a/jstests/free_mon/free_mon_rs_halt.js b/jstests/free_mon/free_mon_rs_halt.js
index 0694f504254..5a5b94accc2 100644
--- a/jstests/free_mon/free_mon_rs_halt.js
+++ b/jstests/free_mon/free_mon_rs_halt.js
@@ -23,8 +23,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     mock_web.waitRegisters(2);
 
-    assert.eq(FreeMonGetStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetStatus(rst.getSecondary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
 
     mock_web.enableFaults();
     mock_web.waitFaults(1);
@@ -40,8 +40,8 @@ load("jstests/free_mon/libs/free_mon.js");
     assert.eq(qs1.registers, qs2.registers);
 
     // Halt causes us to disable free monitoring, not return it to initial state.
-    assert.eq(FreeMonGetStatus(rst.getPrimary()).state, 'disabled');
-    assert.eq(FreeMonGetStatus(rst.getSecondary()).state, 'disabled');
+    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'disabled');
+    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'disabled');
 
     // Disable the fault so we can re-enable again
     mock_web.disableFaults();
@@ -53,8 +53,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     sleep(20 * 1000);
 
-    assert.eq(FreeMonGetStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetStatus(rst.getSecondary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
 
     rst.stopSet();
 
diff --git a/jstests/free_mon/free_mon_rs_perm_del.js b/jstests/free_mon/free_mon_rs_perm_del.js
index be8176ef8a5..3f15719062d 100644
--- a/jstests/free_mon/free_mon_rs_perm_del.js
+++ b/jstests/free_mon/free_mon_rs_perm_del.js
@@ -24,8 +24,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     mock_web.waitRegisters(2);
 
-    assert.eq(FreeMonGetStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetStatus(rst.getSecondary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
 
     mock_web.enableFaults();
     mock_web.waitFaults(1);
@@ -33,9 +33,9 @@ load("jstests/free_mon/libs/free_mon.js");
     sleep(20 * 1000);
 
     // Make sure we are back to the initial state.
-    assert.eq(FreeMonGetStatus(rst.getPrimary()).state, 'undecided');
+    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'undecided');
 
-    assert.eq(FreeMonGetStatus(rst.getSecondary()).state, 'undecided');
+    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'undecided');
 
     // Disable the fault so we can re-enable again
     mock_web.disableFaults();
@@ -47,8 +47,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     sleep(20 * 1000);
 
-    assert.eq(FreeMonGetStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetStatus(rst.getSecondary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
 
     rst.stopSet();
 
diff --git a/jstests/free_mon/free_mon_rs_register.js b/jstests/free_mon/free_mon_rs_register.js
index 2c988e299fa..1c518456a2f 100644
--- a/jstests/free_mon/free_mon_rs_register.js
+++ b/jstests/free_mon/free_mon_rs_register.js
@@ -27,8 +27,8 @@ load("jstests/free_mon/libs/free_mon.js");
 
     mock_web.waitRegisters(2);
 
-    assert.eq(FreeMonGetStatus(rst.getPrimary()).state, 'enabled');
-    assert.eq(FreeMonGetStatus(rst.getSecondary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getPrimary()).state, 'enabled');
+    assert.eq(FreeMonGetServerStatus(rst.getSecondary()).state, 'enabled');
 
     const last_register = mock_web.query("last_register");
     print(tojson(last_register));
diff --git a/jstests/free_mon/libs/free_mon.js b/jstests/free_mon/libs/free_mon.js
index 21d42fb3e66..3cb9f90e364 100644
--- a/jstests/free_mon/libs/free_mon.js
+++ b/jstests/free_mon/libs/free_mon.js
@@ -251,9 +251,21 @@ function FreeMonGetRegistration(conn) {
  *
  * @param {object} serverStatus.freeMonitoring section
  */
-function FreeMonGetStatus(conn) {
+function FreeMonGetServerStatus(conn) {
     'use strict';
 
     const admin = conn.getDB("admin");
     return assert.commandWorked(admin.runCommand({serverStatus: 1})).freeMonitoring;
 }
+
+/**
+ * Get current Free Monitoring Status via getFreeMonitoringStatus.
+ *
+ * @param {object} getFreeMonitoringStatus document
+ */
+function FreeMonGetStatus(conn) {
+    'use strict';
+
+    const admin = conn.getDB("admin");
+    return assert.commandWorked(admin.runCommand({getFreeMonitoringStatus: 1}));
+}
diff --git a/src/mongo/db/free_mon/free_mon_processor.cpp b/src/mongo/db/free_mon/free_mon_processor.cpp
index e476278e34b..6e73596b8a1 100644
--- a/src/mongo/db/free_mon/free_mon_processor.cpp
+++ b/src/mongo/db/free_mon/free_mon_processor.cpp
@@ -686,14 +686,13 @@ void FreeMonProcessor::doAsyncRegisterFail(
 void FreeMonProcessor::doCommandUnregister(
     Client* client, FreeMonWaitableMessageWithPayload<FreeMonMessageType::UnregisterCommand>* msg) {
     // Treat this request as idempotent
-    if (_state->getState() != StorageStateEnum::disabled) {
+    readState(client);
 
-        _state->setState(StorageStateEnum::disabled);
+    _state->setState(StorageStateEnum::disabled);
 
-        writeState(client);
+    writeState(client);
 
-        log() << "Free Monitoring is Disabled";
-    }
+    log() << "Free Monitoring is Disabled";
 
     msg->setStatus(Status::OK());
 }
diff --git a/src/mongo/shell/utils.js b/src/mongo/shell/utils.js
index ec437903154..3c0c357820a 100644
--- a/src/mongo/shell/utils.js
+++ b/src/mongo/shell/utils.js
@@ -1033,15 +1033,16 @@ shellHelper.show = function(what) {
                 } else if (freemonStatus.state === 'undecided') {
                     print(
                         "---\n" +
-                        "Enable MongoDB's free cloud-based monitoring service to collect and display\n" +
-                        "metrics about your deployment (disk utilization, CPU, operation statistics,\n" +
-                        "etc).\n" + "\n" +
-                        "The monitoring data will be available on a MongoDB website with a unique\n" +
-                        "URL created for you. Anyone you share the URL with will also be able to\n" +
-                        "view this page. MongoDB may use this information to make product\n" +
+                        "Enable MongoDB's free cloud-based monitoring service, which will then receive and display\n" +
+                        "metrics about your deployment (disk utilization, CPU, operation statistics, etc).\n" +
+                        "\n" +
+                        "The monitoring data will be available on a MongoDB website with a unique URL accessible to you\n" +
+                        "and anyone you share the URL with. MongoDB may use this information to make product\n" +
                         "improvements and to suggest MongoDB products and deployment options to you.\n" +
-                        "\n" + "To enable free monitoring, run the following command:\n" +
-                        "db.enableFreeMonitoring()\n" + "---\n");
+                        "\n" +
+                        "To enable free monitoring, run the following command: db.enableFreeMonitoring()\n" +
+                        "To permanently disable this reminder, run the following command: db.disableFreeMonitoring()\n" +
+                        "---\n");
                 }
             }