fix for bug 1162887

Users no longer need to disable the plugin to connect to
a different gearman server.

Along with this commit I changed the 'launchWorker' plugin config name to
'enablePlugin' because enabling plugin is distinct from running gearman workers.

Change-Id: Ia81d78da3dbdc83fd46dd7f5d40ccb9aca3af97f

src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java

@@ -47,7 +47,7 @@ public class ComputerListenerImpl extends ComputerListener {
                 + " onConfigurationChange");
 
         // update functions only when gearman-plugin is enabled
-        if (!GearmanPluginConfig.get().launchWorker()) {
+        if (!GearmanPluginConfig.get().enablePlugin()) {
             return;
         }
 
@@ -63,7 +63,7 @@ public class ComputerListenerImpl extends ComputerListener {
                 + " onOffline");
 
         // update functions only when gearman-plugin is enabled
-        if (!GearmanPluginConfig.get().launchWorker()) {
+        if (!GearmanPluginConfig.get().enablePlugin()) {
             return;
         }
 
@@ -92,7 +92,7 @@ public class ComputerListenerImpl extends ComputerListener {
                 + " onOnline");
 
         // update functions only when gearman-plugin is enabled
-        if (!GearmanPluginConfig.get().launchWorker()) {
+        if (!GearmanPluginConfig.get().enablePlugin()) {
             return;
         }
 

src/main/java/hudson/plugins/gearman/Constants.java

@@ -24,7 +24,7 @@ package hudson.plugins.gearman;
 public interface Constants {
     /* Defines. */
 
-    public static final boolean GEARMAN_DEFAULT_LAUNCH_WORKER = false;
+    public static final boolean GEARMAN_DEFAULT_ENABLE_PLUGIN = false;
     public static final String GEARMAN_DEFAULT_TCP_HOST = "127.0.0.1";
     public static final int GEARMAN_DEFAULT_TCP_PORT = 4730;
 

src/main/java/hudson/plugins/gearman/GearmanPluginConfig.java

@@ -48,7 +48,7 @@ public class GearmanPluginConfig extends GlobalConfiguration {
 
     private static final Logger logger = LoggerFactory
             .getLogger(Constants.PLUGIN_LOGGER_NAME);
-    private boolean launchWorker; // enable/disable plugin
+    private boolean enablePlugin; // config to enable and disable plugin
     private String host; // gearman server host
     private int port; // gearman server port
 
@@ -56,8 +56,6 @@ public class GearmanPluginConfig extends GlobalConfiguration {
      * Constructor.
      */
     public GearmanPluginConfig() {
-        logger.info("---- GearmanPluginConfig Constructor ---");
-
         load();
     }
 
@@ -65,6 +63,7 @@ public class GearmanPluginConfig extends GlobalConfiguration {
         return GlobalConfiguration.all().get(GearmanPluginConfig.class);
     }
 
+
     /*
      * This method runs when user clicks Test Connection button.
      *
@@ -89,28 +88,50 @@ public class GearmanPluginConfig extends GlobalConfiguration {
     public boolean configure(StaplerRequest req, JSONObject json)
             throws Descriptor.FormException {
 
-        // set the gearman config from user entered values in jenkins config
-        // page
-        launchWorker = json.getBoolean("launchWorker");
+        // save current plugin config so we can compare to new user settings
+        String prevHost = this.host;
+        int prevPort = this.port;
+        boolean prevEnablePlugin = this.enablePlugin;
+
+        // get the new gearman plugin configs from jenkins config page settings
+        enablePlugin = json.getBoolean("enablePlugin");
         host = json.getString("host");
         port = json.getInt("port");
 
-        if (launchWorker) {
+        if (!enablePlugin && prevEnablePlugin) {  // gearman-plugin goes from ON to OFF state
+            GearmanProxy.getInstance().stopAll();
 
-            // check for a valid connection to gearman server
-            logger.info("---- Check connection to Gearman Server " + host + ":"
-                    + port);
+        } else if (enablePlugin && !prevEnablePlugin) { // gearman-plugin goes from OFF to ON state
+            // check for a valid connection to server
             if (!connectionIsAvailable(host, port, 5000)) {
-                launchWorker = false;
+                enablePlugin = false;
                 throw new FormException("Unable to connect to Gearman server. "
-                        + "Please check the server connection settings and retry.",
-                        "host");
+                            + "Please check the server connection settings and retry.",
+                            "host");
             }
 
+            // run workers
             GearmanProxy.getInstance().initWorkers();
 
-        } else {
-            GearmanProxy.getInstance().stopAll();
+        } else if (enablePlugin && prevEnablePlugin) { // gearman-plugin stays in the ON state
+            // update connection for a plugin config change
+            if (!host.equals(prevHost) || port != prevPort) {
+
+                // stop the workers on the current connected
+                GearmanProxy.getInstance().stopAll();
+
+                // check for a valid connection to server
+                if (!connectionIsAvailable(host, port, 5000)) {
+                    enablePlugin = false;
+                    throw new FormException("Unable to connect to Gearman server. "
+                                + "Please check the server connection settings and retry.",
+                                "host");
+                }
+
+                // run workers with new connection
+                GearmanProxy.getInstance().initWorkers();
+            }
+
         }
 
         req.bindJSON(this, json);
@@ -118,12 +139,13 @@ public class GearmanPluginConfig extends GlobalConfiguration {
         return true;
     }
 
+
     /**
      * This method returns true if the global configuration says we should
-     * launch worker.
+     * enable the plugin.
      */
-    public boolean launchWorker() {
-        return Objects.firstNonNull(launchWorker, Constants.GEARMAN_DEFAULT_LAUNCH_WORKER);
+    public boolean enablePlugin() {
+        return Objects.firstNonNull(enablePlugin, Constants.GEARMAN_DEFAULT_ENABLE_PLUGIN);
     }
 
     /**
@@ -146,7 +168,7 @@ public class GearmanPluginConfig extends GlobalConfiguration {
     }
 
     /*
-     * This method checks whether a connection open and available
+     * This method checks whether a connection is open and available
      * on $host:$port
      *
      * @param host

src/main/java/hudson/plugins/gearman/GearmanProxy.java

@@ -82,50 +82,48 @@ public class GearmanProxy {
          * executor' then use the gearman worker to execute builds on that
          * jenkins nodes
          */
-        if (getNumExecutors() == 0) {
 
-            /*
-             * Spawn management executor worker. This worker does not need any
-             * executors. It only needs to work with gearman.
-             */
-            createManagementWorker();
+        /*
+         * Spawn management executor worker. This worker does not need any
+         * executors. It only needs to work with gearman.
+         */
+        createManagementWorker();
 
-            /*
-             * Spawn executors for the jenkins master Need to treat the master
-             * differently than slaves because the master is not the same as a
-             * slave
-             */
-            // first make sure master is enabled (or has executors)
-            Node masterNode = null;
-            try {
-                masterNode = Computer.currentComputer().getNode();
-            } catch (NullPointerException npe) {
-                logger.info("---- Master is offline");
-            } catch (Exception e) {
-                logger.info("---- Can't get Master");
-                e.printStackTrace();
+        /*
+         * Spawn executors for the jenkins master Need to treat the master
+         * differently than slaves because the master is not the same as a
+         * slave
+         */
+        // first make sure master is enabled (or has executors)
+        Node masterNode = null;
+        try {
+            masterNode = Computer.currentComputer().getNode();
+        } catch (NullPointerException npe) {
+            logger.info("---- Master is offline");
+        } catch (Exception e) {
+            logger.info("---- Can't get Master");
+            e.printStackTrace();
+        }
+
+        if (masterNode != null) {
+            Computer computer = masterNode.toComputer();
+            if (computer != null) {
+                createExecutorWorkersOnNode(computer);
             }
+        }
 
-            if (masterNode != null) {
-                Computer computer = masterNode.toComputer();
+        /*
+         * Spawn executors for the jenkins slaves
+         */
+        List<Node> nodes = Jenkins.getInstance().getNodes();
+        if (!nodes.isEmpty()) {
+            for (Node node : nodes) {
+                Computer computer = node.toComputer();
                 if (computer != null) {
+                    // create a gearman worker for every executor on the slave
                     createExecutorWorkersOnNode(computer);
                 }
             }
-
-            /*
-             * Spawn executors for the jenkins slaves
-             */
-            List<Node> nodes = Jenkins.getInstance().getNodes();
-            if (!nodes.isEmpty()) {
-                for (Node node : nodes) {
-                    Computer computer = node.toComputer();
-                    if (computer != null) {
-                        // create a gearman worker for every executor on the slave
-                        createExecutorWorkersOnNode(computer);
-                    }
-                }
-            }
         }
 
         logger.info("---- Num of executors running = " + getNumExecutors());

src/main/java/hudson/plugins/gearman/SaveableListenerImpl.java

@@ -54,7 +54,7 @@ public class SaveableListenerImpl extends SaveableListener {
                 + " onChange");
 
         // update functions only when gearman-plugin is enabled
-        if (!GearmanPluginConfig.get().launchWorker()) {
+        if (!GearmanPluginConfig.get().enablePlugin()) {
             return;
         }
 

src/main/resources/hudson/plugins/gearman/GearmanPluginConfig/config.jelly

@@ -9,9 +9,9 @@
     <f:validateButton
          title="${%Test Connection}" progress="${%Testing...}"
          method="testConnection" with="host,port"/>
-    <f:entry title="Launch Worker" field="launchWorker"
-      description="Select to run Gearman workers, Unselect to stop">
-      <f:checkbox checked="${descriptor.launchWorker()}"/>
+    <f:entry title="Enable Gearman" field="enablePlugin"
+      description="Select to enable Gearman plugin, Unselect to disable">
+      <f:checkbox checked="${descriptor.enablePlugin()}"/>
     </f:entry>
   </f:section>
 </j:jelly>

src/test/java/hudson/plugins/gearman/GearmanPluginConfigTest.java

@@ -69,7 +69,7 @@ public class GearmanPluginConfigTest {
 
     @Test
     public void testDefaultLaunchWorker() {
-        assertEquals(Constants.GEARMAN_DEFAULT_LAUNCH_WORKER,
-                gpc.launchWorker());
+        assertEquals(Constants.GEARMAN_DEFAULT_ENABLE_PLUGIN,
+                gpc.enablePlugin());
     }
 }