From 0de925f6d8454e73a9aa33c6262ea56852766132 Mon Sep 17 00:00:00 2001 From: zaro0508 Date: Mon, 29 Apr 2013 14:00:25 -0700 Subject: [PATCH] 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 --- .../plugins/gearman/ComputerListenerImpl.java | 6 +- .../hudson/plugins/gearman/Constants.java | 2 +- .../plugins/gearman/GearmanPluginConfig.java | 60 +++++++++++----- .../hudson/plugins/gearman/GearmanProxy.java | 70 +++++++++---------- .../plugins/gearman/SaveableListenerImpl.java | 2 +- .../gearman/GearmanPluginConfig/config.jelly | 6 +- ...unchWorker.html => help-enablePlugin.html} | 0 .../gearman/GearmanPluginConfigTest.java | 4 +- 8 files changed, 85 insertions(+), 65 deletions(-) rename src/main/resources/hudson/plugins/gearman/GearmanPluginConfig/{help-launchWorker.html => help-enablePlugin.html} (100%) diff --git a/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java b/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java index f3da05b..4766186 100644 --- a/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java +++ b/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; } diff --git a/src/main/java/hudson/plugins/gearman/Constants.java b/src/main/java/hudson/plugins/gearman/Constants.java index 50347f8..97e0e06 100644 --- a/src/main/java/hudson/plugins/gearman/Constants.java +++ b/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; diff --git a/src/main/java/hudson/plugins/gearman/GearmanPluginConfig.java b/src/main/java/hudson/plugins/gearman/GearmanPluginConfig.java index a0e5feb..e2a6cbd 100644 --- a/src/main/java/hudson/plugins/gearman/GearmanPluginConfig.java +++ b/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 diff --git a/src/main/java/hudson/plugins/gearman/GearmanProxy.java b/src/main/java/hudson/plugins/gearman/GearmanProxy.java index cf59e7f..8d979c7 100644 --- a/src/main/java/hudson/plugins/gearman/GearmanProxy.java +++ b/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 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 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()); diff --git a/src/main/java/hudson/plugins/gearman/SaveableListenerImpl.java b/src/main/java/hudson/plugins/gearman/SaveableListenerImpl.java index 4f789be..1e10f82 100644 --- a/src/main/java/hudson/plugins/gearman/SaveableListenerImpl.java +++ b/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; } diff --git a/src/main/resources/hudson/plugins/gearman/GearmanPluginConfig/config.jelly b/src/main/resources/hudson/plugins/gearman/GearmanPluginConfig/config.jelly index 0ac4b1d..1b45669 100644 --- a/src/main/resources/hudson/plugins/gearman/GearmanPluginConfig/config.jelly +++ b/src/main/resources/hudson/plugins/gearman/GearmanPluginConfig/config.jelly @@ -9,9 +9,9 @@ - - + + \ No newline at end of file diff --git a/src/main/resources/hudson/plugins/gearman/GearmanPluginConfig/help-launchWorker.html b/src/main/resources/hudson/plugins/gearman/GearmanPluginConfig/help-enablePlugin.html similarity index 100% rename from src/main/resources/hudson/plugins/gearman/GearmanPluginConfig/help-launchWorker.html rename to src/main/resources/hudson/plugins/gearman/GearmanPluginConfig/help-enablePlugin.html diff --git a/src/test/java/hudson/plugins/gearman/GearmanPluginConfigTest.java b/src/test/java/hudson/plugins/gearman/GearmanPluginConfigTest.java index d8497cd..7382b10 100644 --- a/src/test/java/hudson/plugins/gearman/GearmanPluginConfigTest.java +++ b/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()); } }