x
/
gearman-plugin
Code Issues Proposed changes

fix executor names, clean up, remove code duplication 

src/main/java/hudson/plugins/gearman/AbstractWorkerThread.java
    Removed Id field, it was initially added because I thought it was the plugin's
    responsibility to cancel jobs that are on the gearman queue.  We've decided that
    it will be the client (zuul or otherwise) responsibility to cancel jobs from the gearman
    queue.  The gearman plugin will cancel jobs that have already been put on the
    jenkins queue.

src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java
src/main/java/hudson/plugins/gearman/ExecutorWorkerThread.java
src/main/java/hudson/plugins/gearman/GearmanPluginConfig.java
src/main/java/hudson/plugins/gearman/GearmanProxy.java
src/main/java/hudson/plugins/gearman/ManagementWorkerThread.java
src/main/java/hudson/plugins/gearman/StartJobWorker.java
    Refactor to reduce code duplication. Consolidated creation of management worker and
    executor workers.  Added a fix so that executors spawned on master node
    is named 'master-manager' for the manager and 'master-exec-#' for executors

src/test/java/hudson/plugins/gearman/ManagementWorkerThreadTest.java
    Added test to make sure worker name is set correctly

src/main/java/hudson/plugins/gearman/GearmanPluginUtil.java
src/test/java/hudson/plugins/gearman/GearmanPluginUtilTest.java
    Useful utils for the gearman plugin with tests

Change-Id: I96e097dc0dbf5cd78e5e82af584976085aee61b3
This commit is contained in:
zaro0508 2013-03-22 15:09:20 -07:00
parent 307a674a12
commit 9c9acdbdba
10 changed files with 191 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/main/java/hudson/plugins/gearman/AbstractWorkerThread.java
View File

@ -19,7 +19,6 @@
package hudson.plugins.gearman;
import java.util.Date;
import java.util.UUID;
import org.gearman.common.GearmanNIOJobServerConnection;
import org.gearman.worker.GearmanWorker;
@ -47,7 +46,6 @@ public abstract class AbstractWorkerThread implements Runnable {
protected GearmanWorker worker;
private final GearmanNIOJobServerConnection conn;
private Thread thread;
private UUID id;
public AbstractWorkerThread(String host, int port) {
this(host, port, DEFAULT_EXECUTOR_NAME);
@ -57,7 +55,6 @@ public abstract class AbstractWorkerThread implements Runnable {
setHost(host);
setPort(port);
setName(name);
setId(UUID.randomUUID());
worker = new GearmanWorkerImpl();
conn = new GearmanNIOJobServerConnection(host, port);
}
@ -86,15 +83,6 @@ public abstract class AbstractWorkerThread implements Runnable {
this.name = name;
}
public UUID getId() {
return id;
}
public void setId(UUID id) {
this.id = id;
}
/*
* Register jobs with the gearman worker.
* This method should be overriden.
@ -122,8 +110,7 @@ public abstract class AbstractWorkerThread implements Runnable {
if (worker.isRunning()) {
try {
logger.info("---- Stopping " + getName() + ":" + getId().toString() +
" (" + new Date().toString() + ")");
logger.info("---- Stopping " + getName() +" (" + new Date().toString() + ")");
worker.unregisterAll();
worker.shutdown();
} catch (Exception e) {
@ -154,8 +141,7 @@ public abstract class AbstractWorkerThread implements Runnable {
public void run() {
if (!worker.isRunning()) {
logger.info("---- Starting Worker "+ getName() + ":" + getId().toString() +
" ("+new Date().toString()+")");
logger.info("---- Starting Worker "+ getName() +" ("+new Date().toString()+")");
worker.setWorkerID(name);
worker.setJobUniqueIdRequired(true);
worker.addServer(conn);

35
src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java
View File

@ -20,7 +20,6 @@ package hudson.plugins.gearman;
import hudson.Extension;
import hudson.model.TaskListener;
import hudson.model.Computer;
import hudson.model.Node;
import hudson.slaves.ComputerListener;
import java.io.IOException;
@ -104,49 +103,21 @@ public class ComputerListenerImpl extends ComputerListener {
* Spawn management executor worker. This worker does not need any
* executors. It only needs to work with gearman.
*/
String host = GearmanPluginConfig.get().getHost();
int port = GearmanPluginConfig.get().getPort();
ManagementWorkerThread mwt = new ManagementWorkerThread(host, port, host);
mwt.registerJobs();
mwt.start();
GearmanProxy.getGmwtHandles().add(mwt);
GearmanProxy.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
*/
Node masterNode = c.getNode();
int executors = c.getExecutors().size();
for (int i = 0; i < executors; i++) {
// create a gearman worker for every executor on the master
ExecutorWorkerThread ewt = new ExecutorWorkerThread(GearmanPluginConfig.get().getHost(),
GearmanPluginConfig.get().getPort(),
"master-exec"+ Integer.toString(i),
masterNode);
ewt.registerJobs();
ewt.start();
GearmanProxy.getGewtHandles().add(ewt);
}
GearmanProxy.createExecutorWorkersOnNode(c);
}
// on creation of new slave
if (Computer.currentComputer() != c
&& !GearmanProxy.getGewtHandles().contains(c)) {
Node node = c.getNode();
int slaveExecutors = c.getExecutors().size();
// create a gearman worker for every executor on the slave
for (int i = 0; i < slaveExecutors; i++) {
ExecutorWorkerThread gwt = new ExecutorWorkerThread(
GearmanPluginConfig.get().getHost(),
GearmanPluginConfig.get().getPort(), node.getNodeName()
+ "-exec" + Integer.toString(i), node);
gwt.registerJobs();
gwt.start();
GearmanProxy.getGewtHandles().add(gwt);
}
GearmanProxy.createExecutorWorkersOnNode(c);
}
}
}

5
src/main/java/hudson/plugins/gearman/ExecutorWorkerThread.java
View File

@ -48,11 +48,6 @@ public class ExecutorWorkerThread extends AbstractWorkerThread{
private final Node node;
// constructor
public ExecutorWorkerThread(String host, int port, Node node) {
super(host, port);
this.node = node;
}
public ExecutorWorkerThread(String host, int port, String name, Node node) {
super(host, port, name);
this.node = node;

2
src/main/java/hudson/plugins/gearman/GearmanPluginConfig.java
View File

@ -109,7 +109,7 @@ public class GearmanPluginConfig extends GlobalConfiguration {
"host");
}
gearmanProxy.init_worker(host, port);
gearmanProxy.init_worker();
} else {
gearmanProxy.stop_all();

58
src/main/java/hudson/plugins/gearman/GearmanPluginUtil.java Normal file
View File

@ -0,0 +1,58 @@
/*
*
* Copyright 2013 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.model.Computer;
import hudson.model.Node;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* This class contains some useful utilities for this plugin
*
* @author Khai Do
*/
public class GearmanPluginUtil {
private static final Logger logger = LoggerFactory
.getLogger(Constants.PLUGIN_LOGGER_NAME);
/*
* This method returns the real node name. Master node
* by default has an empty string for the name. But you
* need to use "master" to tell jenkins to do stuff,
* namely like schedule a build.
*
* @param Node
* The node to lookup
*
* @return
* "master" for the master node or assigned name of the slave node
*/
public static String getRealName(Node node) {
if (Computer.currentComputer() == node.toComputer()) {
return "master";
} else {
return node.getNodeName();
}
}
}

65
src/main/java/hudson/plugins/gearman/GearmanProxy.java
View File

@ -62,7 +62,7 @@ public class GearmanProxy {
* @param port the host port
*
*/
public void init_worker(String host, int port) {
public void init_worker() {
/*
* Purpose here is to create a 1:1 mapping of 'gearman worker':'jenkins
@ -75,10 +75,7 @@ public class GearmanProxy {
* Spawn management executor worker. This worker does not need any
* executors. It only needs to work with gearman.
*/
ManagementWorkerThread gwt = new ManagementWorkerThread(host, port, host);
gwt.registerJobs();
gwt.start();
gmwtHandles.add(gwt);
createManagementWorker();
/*
* Spawn executors for the jenkins master Need to treat the master
@ -99,15 +96,7 @@ public class GearmanProxy {
if (masterNode != null) {
Computer computer = masterNode.toComputer();
if (computer != null) {
int executors = computer.getExecutors().size();
for (int i = 0; i < executors; i++) {
// create a gearman worker for every executor on the master
ExecutorWorkerThread ewt = new ExecutorWorkerThread(host, port, "master-exec"
+ Integer.toString(i), masterNode);
ewt.registerJobs();
ewt.start();
gewtHandles.add(ewt);
}
createExecutorWorkersOnNode(computer);
}
}
@ -120,15 +109,7 @@ public class GearmanProxy {
Computer computer = node.toComputer();
if (computer != null) {
// create a gearman worker for every executor on the slave
int slaveExecutors = computer.getExecutors().size();
for (int i = 0; i < slaveExecutors; i++) {
ExecutorWorkerThread ewt = new ExecutorWorkerThread(host, port,
node.getNodeName() + "-exec"
+ Integer.toString(i), node);
ewt.registerJobs();
ewt.start();
gewtHandles.add(ewt);
}
createExecutorWorkersOnNode(computer);
}
}
}
@ -137,6 +118,44 @@ public class GearmanProxy {
logger.info("---- Num of executors running = " + getNumExecutors());
}
/*
* Spawn management executor worker. This worker does not need any
* executors. It only needs to work with gearman.
*/
public static void createManagementWorker() {
ManagementWorkerThread gwt = new ManagementWorkerThread(
GearmanPluginConfig.get().getHost(),
GearmanPluginConfig.get().getPort(),
"master-manager");
gwt.registerJobs();
gwt.start();
gmwtHandles.add(gwt);
}
/*
* Spawn workers for each executor on a node.
*/
public static void createExecutorWorkersOnNode(Computer computer) {
Node node = computer.getNode();
int executors = computer.getExecutors().size();
for (int i = 0; i < executors; i++) {
ExecutorWorkerThread ewt;
ewt = new ExecutorWorkerThread(GearmanPluginConfig.get().getHost(),
GearmanPluginConfig.get().getPort(),
GearmanPluginUtil.getRealName(node)+"-exec-"+Integer.toString(i),
node);
ewt.registerJobs();
ewt.start();
gewtHandles.add(ewt);
}
}
/*
* This method stops all gearman workers

6
src/main/java/hudson/plugins/gearman/ManagementWorkerThread.java
View File

@ -31,8 +31,6 @@ import org.slf4j.LoggerFactory;
public class ManagementWorkerThread extends AbstractWorkerThread{
// private static final Logger logger = LoggerFactory
// .getLogger(AbstractWorkerThread.class);
private static final Logger logger = LoggerFactory
.getLogger(Constants.PLUGIN_LOGGER_NAME);
@ -50,11 +48,9 @@ public class ManagementWorkerThread extends AbstractWorkerThread{
@Override
public void registerJobs(){
String jobFunctionName = "stop:"+host;
logger.info("Registering job "+jobFunctionName+" on "+host);
logger.info("---- Registering job "+jobFunctionName+" on "+host);
worker.registerFunctionFactory(new DefaultGearmanFunctionFactory(jobFunctionName,
StopJobWorker.class.getName()));
}
}

7
src/main/java/hudson/plugins/gearman/StartJobWorker.java
View File

@ -111,12 +111,7 @@ public class StartJobWorker extends AbstractGearmanFunction {
/*
* make this node build this project with unique id and build params from the client
*/
// set the name of the node to execute build on
String runNodeName = node.getNodeName();
if (runNodeName.isEmpty()) { // master node name is ""
runNodeName = "master";
}
String runNodeName = GearmanPluginUtil.getRealName(node);
// create action to run on a specified node
Action runNode = new NodeAssignmentAction(runNodeName);

75
src/test/java/hudson/plugins/gearman/GearmanPluginUtilTest.java Normal file
View File

@ -0,0 +1,75 @@
/*
*
* Copyright 2013 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 static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import hudson.model.Computer;
import hudson.slaves.DumbSlave;
import org.gearman.common.GearmanNIOJobServerConnection;
import org.gearman.worker.GearmanWorker;
import org.gearman.worker.GearmanWorkerImpl;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.jvnet.hudson.test.HudsonTestCase;
import org.powermock.core.classloader.annotations.PrepareForTest;
/**
* Test for the {@link GearmanPluginUtil} class.
*
* @author Khai Do
*/
@PrepareForTest(GearmanWorkerImpl.class)
public class GearmanPluginUtilTest extends HudsonTestCase {
/**
*/
@Before
public void setUpTest() {
GearmanWorker gearmanWorker = mock(GearmanWorker.class);
GearmanNIOJobServerConnection conn = new GearmanNIOJobServerConnection("localhost", 4730);
doNothing().when(gearmanWorker).work();
when(gearmanWorker.addServer(conn)).thenReturn(true);
}
@After
public void tearDownTest() throws Exception {
}
@Test
public void testGetRealNameSlave() throws Exception {
DumbSlave slave = createSlave();
slave.setNodeName("oneiric-10");
hudson.removeNode(slave);
assertEquals("oneiric-10", GearmanPluginUtil.getRealName(slave));
}
@Test
public void testGetRealNameMaster() throws Exception {
assertEquals("master", GearmanPluginUtil.getRealName(Computer.currentComputer().getNode()));
}
}

9
src/test/java/hudson/plugins/gearman/ManagementWorkerThreadTest.java
View File

@ -56,9 +56,16 @@ public class ManagementWorkerThreadTest {
@Test
public void testRegisterJobs() {
AbstractWorkerThread manager = new ManagementWorkerThread("GearmanServer", 4730, "MyWorker" );
AbstractWorkerThread manager = new ManagementWorkerThread("GearmanServer", 4730, "manager");
manager.registerJobs();
Set<String> functions = manager.worker.getRegisteredFunctions();
assertEquals("stop:GearmanServer", functions.toArray()[0]);
}
@Test
public void testManagerId() {
AbstractWorkerThread manager = new ManagementWorkerThread("GearmanServer", 4730, "manager");
assertEquals("manager", manager.getName());
}
}