Skip to content
Permalink
Browse files

JENKINS-29136 fix NPE in getRetentionTime()

I documented the bug here: https://issues.jenkins-ci.org/browse/JENKINS-29136

this NPE was preventing jenkins workers from being deleted when they should have been.

I added a defaults class that will be used when parent cloud cannot be resolved
due to whatever reason.
  • Loading branch information
Alexey Skorokhodov committed Jun 30, 2015
1 parent 20c6ca8 commit e4f8cdcc8b54c897e626a5aa3f0b179d42b52d93
@@ -0,0 +1,8 @@
package jenkins.plugins.jclouds.compute;

/**
* Holds default values for Cloud Instances, like default retention time.
*/
final class CloudInstanceDefaults {
public static final int DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES = 30;
}
@@ -60,6 +60,8 @@
import shaded.com.google.common.collect.Iterables;
import shaded.com.google.common.io.Closeables;

import static jenkins.plugins.jclouds.compute.CloudInstanceDefaults.DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES;

/**
* The JClouds version of the Jenkins Cloud.
*
@@ -128,10 +130,11 @@ protected Object readResolve() {
}

/**
* Get the retention time, defaulting to 30 minutes.
* Get the retention time in minutes or default value from CloudInstanceDefaults if it is zero.
* @see CloudInstanceDefaults#DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES
*/
public int getRetentionTime() {
return retentionTime == 0 ? 30 : retentionTime;
return retentionTime == 0 ? DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES : retentionTime;
}

static final Iterable<Module> MODULES = ImmutableSet.<Module>of(new SshjSshClientModule(), new JDKLoggingModule() {
@@ -20,6 +20,8 @@
import org.jclouds.domain.LoginCredentials;
import org.kohsuke.stapler.DataBoundConstructor;

import static jenkins.plugins.jclouds.compute.CloudInstanceDefaults.DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES;

/**
* Jenkins Slave node - managed by JClouds.
*
@@ -134,15 +136,19 @@ public LoginCredentials getCredentials() {

/**
* Get the retention time for this slave, defaulting to the parent cloud's if not set.
* Sometime parent cloud cannot be determined (returns Null as I see), in which case this method will
* return default value set in CloudInstanceDefaults.
*
* @return overrideTime
* @see CloudInstanceDefaults#DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES
*/
public int getRetentionTime() {
if (overrideRetentionTime > 0) {
return overrideRetentionTime;
} else {
return JCloudsCloud.getByName(cloudName).getRetentionTime();
}

JCloudsCloud cloud = JCloudsCloud.getByName(cloudName);
return cloud == null ? DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES : cloud.getRetentionTime();
}

/**
@@ -9,6 +9,8 @@
import org.jclouds.ssh.SshKeys;
import org.jvnet.hudson.test.HudsonTestCase;

import static jenkins.plugins.jclouds.compute.CloudInstanceDefaults.DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES;

public class JCloudsCloudInsideJenkinsLiveTest extends HudsonTestCase {

private ComputeTestFixture fixture;
@@ -24,7 +26,7 @@ public void setUp() throws Exception {

// TODO: this may need to vary per test
cloud = new JCloudsCloud(fixture.getProvider() + "-profile", fixture.getProvider(), fixture.getIdentity(), fixture.getCredential(),
generatedKeys.get("private"), generatedKeys.get("public"), fixture.getEndpoint(), 1, 30, 600 * 1000, 600 * 1000, null,
generatedKeys.get("private"), generatedKeys.get("public"), fixture.getEndpoint(), 1, DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES, 600 * 1000, 600 * 1000, null,
Collections.<JCloudsSlaveTemplate>emptyList());
}

@@ -10,6 +10,8 @@

import org.jclouds.ssh.SshKeys;

import static jenkins.plugins.jclouds.compute.CloudInstanceDefaults.DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES;

public class JCloudsCloudLiveTest extends TestCase {

private ComputeTestFixture fixture;
@@ -25,7 +27,7 @@ public void setUp() throws Exception {

// TODO: this may need to vary per test
cloud = new JCloudsCloud(fixture.getProvider() + "-profile", fixture.getProvider(), fixture.getIdentity(), fixture.getCredential(),
generatedKeys.get("private"), generatedKeys.get("public"), fixture.getEndpoint(), 1, 30, 600 * 1000, 600 * 1000, null,
generatedKeys.get("private"), generatedKeys.get("public"), fixture.getEndpoint(), 1, DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES, 600 * 1000, 600 * 1000, null,
Collections.<JCloudsSlaveTemplate>emptyList());
}

@@ -1,5 +1,6 @@
package jenkins.plugins.jclouds.compute;

import static jenkins.plugins.jclouds.compute.CloudInstanceDefaults.DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

@@ -56,7 +57,7 @@ public void testConfigurationUI() throws Exception {
@Test
public void testConfigRoundtrip() throws Exception {

JCloudsCloud original = new JCloudsCloud("aws-profile", "aws-ec2", "identity", "credential", "privateKey", "publicKey", "endPointUrl", 1, 30,
JCloudsCloud original = new JCloudsCloud("aws-profile", "aws-ec2", "identity", "credential", "privateKey", "publicKey", "endPointUrl", 1, DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES,
600 * 1000, 600 * 1000, null, Collections.<JCloudsSlaveTemplate>emptyList());

j.getInstance().clouds.add(original);
@@ -5,6 +5,8 @@

import org.jvnet.hudson.test.HudsonTestCase;

import static jenkins.plugins.jclouds.compute.CloudInstanceDefaults.DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES;

/**
* @author Vijay Kiran
*/
@@ -20,7 +22,7 @@ public void testConfigRoundtrip() throws Exception {
List<JCloudsSlaveTemplate> templates = new ArrayList<JCloudsSlaveTemplate>();
templates.add(originalTemplate);

JCloudsCloud originalCloud = new JCloudsCloud("aws-profile", "aws-ec2", "identity", "credential", "privateKey", "publicKey", "endPointUrl", 1, 30,
JCloudsCloud originalCloud = new JCloudsCloud("aws-profile", "aws-ec2", "identity", "credential", "privateKey", "publicKey", "endPointUrl", 1, DEFAULT_INSTANCE_RETENTION_TIME_IN_MINUTES,
600 * 1000, 600 * 1000, null, templates);

hudson.clouds.add(originalCloud);

0 comments on commit e4f8cdc

Please sign in to comment.