From 6307ba2225bbc9e8d4f41062bd6b944e40d7fa7a Mon Sep 17 00:00:00 2001 From: Khai Do Date: Thu, 2 Oct 2014 12:44:49 -0700 Subject: [PATCH] update to work with Jenkins LTS ver 1.565.3 This change updates the gearman-plugin to build against Jenkins LTS ver 1.565.3. This is required to support later versions of Jenkins because there have been changes to Jenkins events in core. This change adds the ItemListener to re-register gearman functions on changes to jenkins projects. Using the ItemListener is better because it provides more details on the item that's been changed and the events are more grandular. There was also a change to ComputerListener events (onTemporaryOffline and onTemporaryOnline) that needed to be handled seperately in this change. The SaveableListener is still needed due to a bug in ItemListener. The most notable change to function registration is that the gearman plugin will no longer register functions containing a node's self label. For example: Assume you have the following setup: a node: named 'trusty-slave1' and labeled 'trusty' a job: named 'my-job' that targets the 'trusty' label The gearman plugin used to register the following functions: 'build:my-job', 'build:my-job:trusty' and 'build:my-job:trusty-slave1' With this update the gearman plugin will only register 'build:my-job' and 'build:my-job:trusty'. It will no longer register functions containing the implicit node name (trusty-slave1). If your gearman client has been using explicit labels to execute builds then this change will not affect your workflow. Closes-Bug: #1353891 Change-Id: I9e57ec9f24bf303989a4df1fc4f1a0c4b6d001bc --- pom.xml | 4 +- .../plugins/gearman/ComputerListenerImpl.java | 56 ++++++++++--- .../plugins/gearman/ItemListenerImpl.java | 83 +++++++++++++++++++ .../plugins/gearman/SaveableListenerImpl.java | 23 ++--- .../gearman/ExecutorWorkerThreadTest.java | 22 ----- 5 files changed, 139 insertions(+), 49 deletions(-) create mode 100644 src/main/java/hudson/plugins/gearman/ItemListenerImpl.java diff --git a/pom.xml b/pom.xml index e2f03e6..2f34cbc 100644 --- a/pom.xml +++ b/pom.xml @@ -28,7 +28,7 @@ org.jenkins-ci.plugins plugin - 1.502 + 1.565.3 gearman-plugin @@ -228,9 +228,7 @@ maven-surefire-plugin ${maven-surefire-plugin.version} - methods true - 4 diff --git a/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java b/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java index 568d1a7..08e07a9 100644 --- a/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java +++ b/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java @@ -1,6 +1,6 @@ /* * - * Copyright 2013 Hewlett-Packard Development Company, L.P. + * Copyright 2014 Hewlett-Packard Development Company, L.P. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,7 @@ import hudson.Extension; import hudson.model.TaskListener; import hudson.model.Computer; import hudson.slaves.ComputerListener; +import hudson.slaves.OfflineCause; import java.io.IOException; @@ -28,7 +29,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Update gearman workers when node changes + * Update gearman workers on node state and configuration changes */ @Extension public class ComputerListenerImpl extends ComputerListener { @@ -36,12 +37,10 @@ public class ComputerListenerImpl extends ComputerListener { private static final Logger logger = LoggerFactory .getLogger(Constants.PLUGIN_LOGGER_NAME); - @Override public void onConfigurationChange() { - // Bug: Configuration save occurs after this function is called - // gets called on any configuration change - // includes new slave and delete slave + // only fired on nodes configuration changes like a label or + // name change. Not fired on state changes, like offline or online. logger.info("---- " + ComputerListenerImpl.class.getName() + ":" + " onConfigurationChange"); @@ -50,6 +49,10 @@ public class ComputerListenerImpl extends ComputerListener { return; } + // re-register gearman functions on node configuration changes, + // specifically node label changes + GearmanProxy.getInstance().registerJobs(); + // TODO: adjust for an update to executors. Method does not provide the // computer to know which thread to remove or add } @@ -84,10 +87,6 @@ public class ComputerListenerImpl extends ComputerListener { return; } - /* - * Need to treat the master differently than slaves because - * the master is not the same as a slave - */ GearmanProxy gp = GearmanProxy.getInstance(); /* * Spawn management executor worker if one doesn't exist yet. @@ -99,4 +98,41 @@ public class ComputerListenerImpl extends ComputerListener { // on creation of new slave gp.createExecutorWorkersOnNode(c); } + + @Override + public void onTemporarilyOffline(Computer c, OfflineCause cause) { + // fired when master or slave goes into temporary offline state + logger.info("---- " + ComputerListenerImpl.class.getName() + ":" + + " onTemporarilyOffline computer " + c); + // update functions only when gearman-plugin is enabled + if (!GearmanPluginConfig.get().enablePlugin()) { + return; + } + + // stop worker when jenkins slave is set to offline + GearmanProxy.getInstance().stop(c); + } + + @Override + public void onTemporarilyOnline(Computer c) { + // fired when master or slave goes into temporary online state + logger.info("---- " + ComputerListenerImpl.class.getName() + ":" + + " onTemporarilyOnline computer " + c); + // update functions only when gearman-plugin is enabled + if (!GearmanPluginConfig.get().enablePlugin()) { + return; + } + + GearmanProxy gp = GearmanProxy.getInstance(); + /* + * Spawn management executor worker if one doesn't exist yet. + * This worker does not need any executors. It only needs + * to work with gearman. + */ + gp.createManagementWorker(); + + // on brining a slave back online + gp.createExecutorWorkersOnNode(c); + } + } diff --git a/src/main/java/hudson/plugins/gearman/ItemListenerImpl.java b/src/main/java/hudson/plugins/gearman/ItemListenerImpl.java new file mode 100644 index 0000000..42ca024 --- /dev/null +++ b/src/main/java/hudson/plugins/gearman/ItemListenerImpl.java @@ -0,0 +1,83 @@ +/* + * + * Copyright 2014 Hewlett-Packard Development Company, L.P. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package hudson.plugins.gearman; + +import hudson.Extension; +import hudson.model.Item; +import hudson.model.listeners.ItemListener; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/* + * This handles events for generic items in Jenkins. We also extended + * the SaveableListener to catch any events that this one misses. + */ +@Extension +public class ItemListenerImpl extends ItemListener { + + private static final Logger logger = LoggerFactory + .getLogger(Constants.PLUGIN_LOGGER_NAME); + + @Override + public void onCopied(Item src, Item item) { + // Called after a new job is created by copying from an existing job + registerJobs(); + } + + @Override + public void onRenamed(Item item, String oldName, String newName) { + // Called after a job is renamed + registerJobs(); + } + + @Override + public void onLoaded() { + registerJobs(); + } + + @Override + public void onCreated(Item item) { + registerJobs(); + } + + @Override + public void onUpdated(Item item) { + registerJobs(); + } + + @Override + public void onDeleted(Item item) { + registerJobs(); + } + + @Override + public void onLocationChanged(Item item, String oldFullName, String newFullName) { + registerJobs(); + } + + // register gearman functions + private void registerJobs() { + // update functions only when gearman-plugin is enabled + if (!GearmanPluginConfig.get().enablePlugin()) { + return; + } + GearmanProxy.getInstance().registerJobs(); + } +} diff --git a/src/main/java/hudson/plugins/gearman/SaveableListenerImpl.java b/src/main/java/hudson/plugins/gearman/SaveableListenerImpl.java index 9ecdab0..b1fecd3 100644 --- a/src/main/java/hudson/plugins/gearman/SaveableListenerImpl.java +++ b/src/main/java/hudson/plugins/gearman/SaveableListenerImpl.java @@ -22,18 +22,16 @@ import hudson.Extension; import hudson.XmlFile; import hudson.model.Saveable; import hudson.model.AbstractProject; -import hudson.model.Node; import hudson.model.listeners.SaveableListener; -import java.util.List; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * Using the SaveableListener is required as a work around because - * updating gearman function in ComputerListener.onConfigurationChange - * doesn't work. + * the itemListener.onUpdate event does not fire on changes to + * project updates using the Jenkins REST API + * Bug: https://issues.jenkins-ci.org/browse/JENKINS-25175 */ @Extension public class SaveableListenerImpl extends SaveableListener { @@ -41,22 +39,19 @@ public class SaveableListenerImpl extends SaveableListener { private static final Logger logger = LoggerFactory .getLogger(Constants.PLUGIN_LOGGER_NAME); - /* - * TODO: this works, but is NOT GOOD! - * This listener fires for every change to an object. So if you - * have 3 nodes in Jenkins it will fire 3 times even though - * the change was only applied to one of the nodes. - * - */ @Override + // This works but is NOT good because this event is a catch all + // for just about any change that happens in Jenkins. This event + // also doesn't provide much detail on what has changed. public void onChange(Saveable o, XmlFile file) { // update functions only when gearman-plugin is enabled if (!GearmanPluginConfig.get().enablePlugin()) { return; } - // update for when any changes are applied to a project or node - if (o instanceof Node || o instanceof AbstractProject) { + // only look for changes to projects, specifically for project + // label changes. Node changes are handled in ComputerListenerImpl + if (o instanceof AbstractProject) { GearmanProxy.getInstance().registerJobs(); } } diff --git a/src/test/java/hudson/plugins/gearman/ExecutorWorkerThreadTest.java b/src/test/java/hudson/plugins/gearman/ExecutorWorkerThreadTest.java index 38ab140..80bb6fd 100644 --- a/src/test/java/hudson/plugins/gearman/ExecutorWorkerThreadTest.java +++ b/src/test/java/hudson/plugins/gearman/ExecutorWorkerThreadTest.java @@ -68,28 +68,6 @@ public class ExecutorWorkerThreadTest extends HudsonTestCase { super.tearDown(); } - /* - * This test verifies that gearman functions are correctly registered for a - * project that contains a label matching a slave node's self label - */ - @Test - public void testRegisterJobs_NodeSelfLabel() throws Exception { - - - Project apple = createFreeStyleProject("apple"); - apple.setAssignedLabel(new LabelAtom("oneiric-10")); - - AbstractWorkerThread oneiric = new ExecutorWorkerThread("GearmanServer", 4730, "MyWorker", slave.toComputer(), "master", new NoopAvailabilityMonitor()); - oneiric.testInitWorker(); - oneiric.registerJobs(); - Set functions = oneiric.worker.getRegisteredFunctions(); - - assertEquals(2, functions.size()); - assertTrue(functions.contains("build:apple")); - assertTrue(functions.contains("build:apple:oneiric-10")); - - } - /* * This test verifies that gearman functions are correctly registered for a * project that contains a single label matching a label on the slave node