Fix duplicate listener checks

Only the protocol, port and address of the listener were used to check if
a listener exists. The check should also use the name of the listener to
be sure that each name is unique.

Expanded tests to check that the creation of duplicate listeners is
detected. Did minor improvements to related test code.

include/maxscale/service.h

@@ -247,7 +247,7 @@ bool serviceHasBackend(SERVICE *service, SERVER *server);
  * @param port Listener port
  * @return True if service has the listener
  */
-bool serviceHasListener(SERVICE *service, const char *protocol,
+bool serviceHasListener(SERVICE* service, const char* name, const char* protocol,
                         const char* address, unsigned short port);
 
 /**

maxctrl/test/cluster.js

@@ -145,6 +145,7 @@ describe('Cluster Command Internals', function() {
         }
 
         cluster.haveExtraServices(a, b, 'test1', 'test2').should.be.rejected
+        cluster.haveExtraServices(b, a, 'test2', 'test1').should.be.rejected
         expect(cluster.haveExtraServices(a, a, 'test1', 'test2')).to.equal(undefined)
         expect(cluster.haveExtraServices(b, b, 'test1', 'test2')).to.equal(undefined)
     })
@@ -212,12 +213,17 @@ describe('Cluster Sync', function() {
 
     // As the listeners cannot be truly deleted, since there's no code for actually closing a socket at runtime,
     // we do the listener tests last
-    it('sync listener creation', function() {
+    it('sync listener creation/deletion', function() {
         return doCommand('create listener RW-Split-Router my-listener-2 5999 --hosts 127.0.0.1:8990')
         // As both MaxScales are on the same machine, both can't listen on the same port. The sync should fail due to this
             .then(() => doCommand('cluster sync 127.0.0.1:8990 --hosts 127.0.0.1:8989').should.be.rejected)
         // Create the listener on the second MaxScale to avoid it being synced later on
             .then(() => doCommand('create listener RW-Split-Router my-listener-2 5998 --hosts 127.0.0.1:8989'))
+        // Sync after creation should succeed
+            .then(() => doCommand('cluster sync 127.0.0.1:8990 --hosts 127.0.0.1:8989'))
+        // Destroy the created server, should succeed
+            .then(() => doCommand('destroy listener RW-Split-Router my-listener-2'))
+            .then(() => doCommand('cluster sync 127.0.0.1:8990 --hosts 127.0.0.1:8989'))
     })
 
     after(stopDoubleMaxScale)

maxctrl/test/createdestroy.js

@@ -110,11 +110,26 @@ describe("Create/Destroy Commands", function() {
             .should.be.fulfilled
     })
 
+    it('create already existing listener', function() {
+        return doCommand('create listener RW-Split-Router my-listener 7890')
+            .should.be.rejected
+    })
+
+    it('create listener with already used port', function() {
+        return doCommand('create listener RW-Split-Router my-listener2 4567')
+            .should.be.rejected
+    })
+
     it('destroy listener', function() {
         return doCommand('destroy listener RW-Split-Router my-listener')
             .should.be.fulfilled
     })
 
+    it('destroy static listener', function() {
+        return doCommand('destroy listener RW-Split-Router RW-Split-Listener')
+            .should.be.rejected
+    })
+
     it('create user', function() {
         return verifyCommand('create user testuser test',
                              'users/inet/testuser')

server/core/config.cc

@@ -3283,7 +3283,7 @@ int create_new_listener(CONFIG_CONTEXT *obj)
             SSL_LISTENER *ssl_info = make_ssl_structure(obj, true, &error_count);
             if (socket)
             {
-                if (serviceHasListener(service, protocol, address, 0))
+                if (serviceHasListener(service, obj->object, protocol, address, 0))
                 {
                     MXS_ERROR("Listener '%s' for service '%s' already has a socket at '%s.",
                               obj->object, service_name, socket);
@@ -3298,7 +3298,7 @@ int create_new_listener(CONFIG_CONTEXT *obj)
 
             if (port)
             {
-                if (serviceHasListener(service, protocol, address, atoi(port)))
+                if (serviceHasListener(service, obj->object, protocol, address, atoi(port)))
                 {
                     MXS_ERROR("Listener '%s', for service '%s', already have port %s.",
                               obj->object,

server/core/config_runtime.cc

@@ -743,7 +743,7 @@ bool runtime_create_listener(SERVICE *service, const char *name, const char *add
 
     spinlock_acquire(&crt_lock);
 
-    if (!serviceHasListener(service, proto, addr, u_port))
+    if (!serviceHasListener(service, name, proto, addr, u_port))
     {
         SSL_LISTENER *ssl = NULL;
 
@@ -807,10 +807,8 @@ bool runtime_destroy_listener(SERVICE *service, const char *name)
         }
         else
         {
-            rval = false;
+            runtime_error("Listener '%s' was not created at runtime. Remove the listener "
-            MXS_WARNING("Listener '%s' was not created at runtime. Remove the "
+                          "manually from the correct configuration file.", name);
-                        "listener manually from the correct configuration file.",
-                        name);
         }
     }
     else

server/core/service.cc

@@ -776,7 +776,7 @@ SERV_LISTENER* serviceCreateListener(SERVICE *service, const char *name, const c
  * @param port          The port to listen on
  * @return      True if the protocol/port is already part of the service
  */
-bool serviceHasListener(SERVICE *service, const char *protocol,
+bool serviceHasListener(SERVICE* service, const char* name, const char* protocol,
                         const char* address, unsigned short port)
 {
     LISTENER_ITERATOR iter;
@@ -785,9 +785,12 @@ bool serviceHasListener(SERVICE *service, const char *protocol,
          listener; listener = listener_iterator_next(&iter))
     {
         if (listener_is_active(listener) &&
-            strcmp(listener->protocol, protocol) == 0 && listener->port == port &&
+            // Listener with same name exists
+            (strcmp(listener->name, name) == 0 ||
+             // Listener listening on the same interface and port exists
+            ((strcmp(listener->protocol, protocol) == 0 && listener->port == port &&
             ((address && listener->address && strcmp(listener->address, address) == 0) ||
-             (address == NULL && listener->address == NULL)))
+             (address == NULL && listener->address == NULL))))))
         {
             return true;
         }

server/core/test/rest-api/test/service.js

@@ -63,28 +63,48 @@ describe("Service", function() {
             })
     });
 
-    it("create a listener", function() {
+    const listener = {
-        var listener = {
+        "links": {
-            "links": {
+            "self": "http://localhost:8989/v1/services/RW-Split-Router/listeners"
-                "self": "http://localhost:8989/v1/services/RW-Split-Router/listeners"
+        },
+        "data": {
+            "attributes": {
+                "parameters": {
+                    "port": 4012,
+                    "protocol": "MySQLClient",
+                    "authenticator": "MySQLAuth",
+                    "address": "127.0.0.1"
+                }
             },
-            "data": {
+            "id": "RW-Split-Listener-2",
-                "attributes": {
+            "type": "listeners"
-                    "parameters": {
-                        "port": 4012,
-                        "protocol": "MySQLClient",
-                        "authenticator": "MySQLAuth",
-                        "address": "127.0.0.1"
-                    }
-                },
-                "id": "RW-Split-Listener-2",
-                "type": "listeners"
-            }
         }
+    }
 
+    it("create a listener", function() {
         return request.post(base_url + "/services/RW-Split-Router/listeners", {json: listener})
             .should.be.fulfilled
     });
 
+    it("create an already existing listener", function() {
+        return request.post(base_url + "/services/RW-Split-Router/listeners", {json: listener})
+            .should.be.rejected
+    });
+
+    it("destroy a listener", function() {
+        return request.delete(base_url + "/services/RW-Split-Router/listeners/RW-Split-Listener-2")
+            .should.be.fulfilled
+    });
+
+    it("destroy a nonexistent listener", function() {
+        return request.delete(base_url + "/services/RW-Split-Router/listeners/I-bet-this-listener-exists")
+            .should.be.rejected
+    });
+
+    it("destroy a static listener", function() {
+        return request.delete(base_url + "/services/RW-Split-Router/listeners/RW-Split-Listener")
+            .should.be.rejected
+    });
+
     after(stopMaxScale)
 });

server/core/test/testservice.cc

@@ -73,7 +73,7 @@ test1()
     ss_info_dassert(serviceCreateListener(service, "TestProtocol", "testprotocol",
                                           "localhost", 9876, "MySQLAuth", NULL, NULL),
                     "Add Protocol should succeed");
-    ss_info_dassert(0 != serviceHasListener(service, "testprotocol", "localhost", 9876),
+    ss_info_dassert(0 != serviceHasListener(service, "TestProtocol", "testprotocol", "localhost", 9876),
                     "Service should have new protocol as requested");
 
     return 0;