From 163b3a2da53cc02d5645f2cf95307f6cc2230040 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus.makela@mariadb.com>
Date: Wed, 17 Apr 2019 00:00:24 +0300
Subject: [PATCH] Fix unhandled promise rejection in create monitor

If a monitor was created with an argument that was not a key-value type, a
promise would be rejected outside of the main maxctrl function. Added a
test case that covers this and fixed a few other test coverage problems
that were present.
---
 maxctrl/lib/create.js         | 11 ++++++-----
 maxctrl/test/createdestroy.js | 17 +++++++++++++++++
 maxctrl/test/drain.js         |  5 +++++
 maxctrl/test/unknown.js       |  3 ++-
 4 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/maxctrl/lib/create.js b/maxctrl/lib/create.js
index 45d869579..954c4cd92 100644
--- a/maxctrl/lib/create.js
+++ b/maxctrl/lib/create.js
@@ -159,12 +159,10 @@ exports.builder = function(yargs) {
                 }
             }
 
-            if (argv.params) {
-                var err = validateParams(argv, argv.params)
-                if (err) {
-                    return Promise.reject(err)
-                }
+            var err = false;
 
+            if (argv.params) {
+                err = validateParams(argv, argv.params)
                 monitor.data.attributes.parameters = argv.params.reduce(to_obj, {})
             }
 
@@ -182,6 +180,9 @@ exports.builder = function(yargs) {
             }
 
             maxctrl(argv, function(host) {
+                if (err) {
+                    return Promise.reject(err)
+                }
                 return doRequest(host, 'monitors', null, {method: 'POST', body: monitor})
             })
         })
diff --git a/maxctrl/test/createdestroy.js b/maxctrl/test/createdestroy.js
index bde20b181..59a76ff3d 100644
--- a/maxctrl/test/createdestroy.js
+++ b/maxctrl/test/createdestroy.js
@@ -18,6 +18,18 @@ describe("Create/Destroy Commands", function() {
             .should.be.rejected
     })
 
+    it('monitor without parameters fails due to missing user parameter', function() {
+        return verifyCommand('create monitor my-monitor mysqlmon', 'monitors/my-monitor')
+            .should.be.rejected
+    })
+
+    it('destroy monitor created without parameters', function() {
+        return doCommand('destroy monitor my-monitor')
+            .should.be.fulfilled
+            .then(() => doCommand('show monitor my-monitor'))
+            .should.be.rejected
+    })
+
     it('will not destroy the same monitor again', function() {
         return doCommand('destroy monitor my-monitor')
             .should.be.rejected
@@ -38,6 +50,11 @@ describe("Create/Destroy Commands", function() {
             .should.be.rejected
     })
 
+    it('will not create monitor with malformed parameters', function() {
+        return doCommand('create monitor my-monitor mariadbmon not-a-param')
+            .should.be.rejected
+    })
+
     it('create monitor with options', function() {
         return doCommand('unlink monitor MariaDB-Monitor server4')
             .then(() => verifyCommand('create monitor my-monitor mysqlmon --servers server4 --monitor-user maxuser --monitor-password maxpwd',
diff --git a/maxctrl/test/drain.js b/maxctrl/test/drain.js
index b4d5f6673..ec489b8d1 100644
--- a/maxctrl/test/drain.js
+++ b/maxctrl/test/drain.js
@@ -15,5 +15,10 @@ describe("Draining servers", function() {
             .should.eventually.have.string("Maintenance")
     })
 
+    it('does not drain non-existent server', function() {
+        return doCommand('drain server not-a-server')
+            .should.be.rejected
+    })
+
     after(stopMaxScale)
 });
diff --git a/maxctrl/test/unknown.js b/maxctrl/test/unknown.js
index 7f818095a..1d0a30470 100644
--- a/maxctrl/test/unknown.js
+++ b/maxctrl/test/unknown.js
@@ -19,7 +19,8 @@ describe("Unknown Commands", function() {
         'alter',
         'rotate',
         'call',
-        'cluster'
+        'cluster',
+        'drain'
     ]
 
     endpoints.forEach(function (i) {