Skip to content

Commit

Permalink
JENKINS-26797 - Zombie instance created by the EC2 plugins
Browse files Browse the repository at this point in the history
* fixing error code for provisionOndemand - InvalidInstanceID.NotFound
* refactoring updateRemoteTags - no code duplication
* no exception throwing if remote tagging fails - only logging
** otherwise we got zombie instances which are not shown
* adding unit testing
  • Loading branch information
cbek committed Mar 19, 2015
1 parent ba77bb7 commit 141236b
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 29 deletions.
6 changes: 6 additions & 0 deletions pom.xml
Expand Up @@ -52,6 +52,12 @@ THE SOFTWARE.
</scm>

<dependencies>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.12</version>
<scope>test</scope>
</dependency>
<dependency>
<!-- we only use this to handle key fingerprint. should be able to replace this with trilead -->
<groupId>bouncycastle</groupId>
Expand Down
54 changes: 25 additions & 29 deletions src/main/java/hudson/plugins/ec2/SlaveTemplate.java
Expand Up @@ -42,6 +42,7 @@
import java.io.PrintStream;
import java.net.URL;
import java.util.*;
import java.util.logging.Level;
import java.util.logging.Logger;

import javax.servlet.ServletException;
Expand Down Expand Up @@ -421,18 +422,7 @@ private EC2AbstractSlave provisionOndemand(TaskListener listener) throws AmazonC

/* Now that we have our instance, we can set tags on it */
if (inst_tags != null) {
for (int i = 0; i < 5; i++) {
try {
updateRemoteTags(ec2, inst_tags, inst.getInstanceId());
break;
} catch (AmazonServiceException e) {
if (e.getErrorCode().equals("InvalidInstanceRequestID.NotFound")) {
Thread.sleep(5000);
continue;
}
throw e;
}
}
updateRemoteTags(ec2, inst_tags, "InvalidInstanceID.NotFound", inst.getInstanceId());

// That was a remote request - we should also update our local instance data.
inst.setTags(inst_tags);
Expand Down Expand Up @@ -676,18 +666,7 @@ private EC2AbstractSlave provisionSpot(TaskListener listener) throws AmazonClien

/* Now that we have our Spot request, we can set tags on it */
if (inst_tags != null) {
for (int i = 0; i < 5; i++) {
try {
updateRemoteTags(ec2, inst_tags, spotInstReq.getSpotInstanceRequestId());
break;
} catch (AmazonServiceException e) {
if (e.getErrorCode().equals("InvalidSpotInstanceRequestID.NotFound")) {
Thread.sleep(5000);
continue;
}
throw e;
}
}
updateRemoteTags(ec2, inst_tags, "InvalidSpotInstanceRequestID.NotFound", spotInstReq.getSpotInstanceRequestId());

// That was a remote request - we should also update our local instance data.
spotInstReq.setTags(inst_tags);
Expand Down Expand Up @@ -724,12 +703,29 @@ private KeyPair getKeyPair(AmazonEC2 ec2) throws IOException, AmazonClientExcept
}

/**
* Update the tags stored in EC2 with the specified information
* Update the tags stored in EC2 with the specified information.
* Re-try 5 times if instances isn't up by catchErrorCode - e.g. InvalidSpotInstanceRequestID.NotFound or InvalidInstanceRequestID.NotFound
* @param ec2
* @param inst_tags
* @param catchErrorCode
* @param params
* @throws InterruptedException
*/
private void updateRemoteTags(AmazonEC2 ec2, Collection<Tag> inst_tags, String... params) {
CreateTagsRequest tag_request = new CreateTagsRequest();
tag_request.withResources(params).setTags(inst_tags);
ec2.createTags(tag_request);
protected void updateRemoteTags(AmazonEC2 ec2, Collection<Tag> inst_tags, String catchErrorCode, String... params) throws InterruptedException {
for (int i = 0; i < 5; i++) {
try {
CreateTagsRequest tag_request = new CreateTagsRequest();
tag_request.withResources(params).setTags(inst_tags);
ec2.createTags(tag_request);
break;
} catch (AmazonServiceException e) {
if (e.getErrorCode().equals(catchErrorCode)) {
Thread.sleep(5000);
continue;
}
LOGGER.log(Level.SEVERE, e.getErrorMessage(), e);
}
}
}

/**
Expand Down
139 changes: 139 additions & 0 deletions src/test/java/hudson/plugins/ec2/SlaveTemplateUnitTest.java
@@ -0,0 +1,139 @@
package hudson.plugins.ec2;

import com.amazonaws.AmazonServiceException;
import com.amazonaws.services.ec2.AmazonEC2;
import com.amazonaws.services.ec2.AmazonEC2Client;
import com.amazonaws.services.ec2.model.InstanceType;
import com.amazonaws.services.ec2.model.Tag;
import hudson.model.Node;
import junit.framework.Assert;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import java.util.*;
import java.util.logging.Handler;
import java.util.logging.LogRecord;
import java.util.logging.Logger;

/**
* Created by christophbeckmann on 3/17/15.
*/
public class SlaveTemplateUnitTest {

Logger logger;
TestHandler handler;

@Before
public void setUp() throws Exception {
AmazonEC2Cloud.testMode = true;

handler = new TestHandler();
logger = Logger.getLogger(SlaveTemplate.class.getName());
logger.addHandler(handler);
}

@After
public void tearDown() throws Exception {
AmazonEC2Cloud.testMode = false;
}

@Test
public void testUpdateRemoteTags() throws Exception {
AmazonEC2 ec2 = new AmazonEC2Client() {
@Override
public void createTags(com.amazonaws.services.ec2.model.CreateTagsRequest createTagsRequest) {

}
};

String ami = "ami1";
String description = "foo ami";

EC2Tag tag1 = new EC2Tag("name1", "value1");
EC2Tag tag2 = new EC2Tag("name2", "value2");
List<EC2Tag> tags = new ArrayList<EC2Tag>();
tags.add(tag1);
tags.add(tag2);
String instanceId = "123";

SlaveTemplate orig = new SlaveTemplate(ami, EC2AbstractSlave.TEST_ZONE, null, "default", "foo", InstanceType.M1Large, "ttt", Node.Mode.NORMAL, description, "bar", "bbb", "aaa", "10", "fff", null, "-Xmx1g", false, "subnet 456", tags, null, false, null, "", true, false, "", false, "") {
@Override
protected Object readResolve() {
return null;
}
};

ArrayList<Tag> awsTags = new ArrayList<Tag>();
awsTags.add(new Tag(EC2Tag.TAG_NAME_JENKINS_SLAVE_TYPE, "value1"));
awsTags.add(new Tag(EC2Tag.TAG_NAME_JENKINS_SLAVE_TYPE, "value2"));

orig.updateRemoteTags(ec2, awsTags, "InvalidInstanceRequestID.NotFound", instanceId);
Assert.assertEquals(0, handler.getRecords().size());
}

@Test
public void testUpdateRemoteTagsInstanceNotFound() throws Exception {
AmazonEC2 ec2 = new AmazonEC2Client() {
@Override
public void createTags(com.amazonaws.services.ec2.model.CreateTagsRequest createTagsRequest) {
AmazonServiceException e = new AmazonServiceException("Instance not found - InvalidInstanceRequestID.NotFound");
e.setErrorCode("InvalidInstanceRequestID.NotFound");
throw e;
}
};

String ami = "ami1";
String description = "foo ami";

EC2Tag tag1 = new EC2Tag("name1", "value1");
EC2Tag tag2 = new EC2Tag("name2", "value2");
List<EC2Tag> tags = new ArrayList<EC2Tag>();
tags.add(tag1);
tags.add(tag2);
String instanceId = "123";

SlaveTemplate orig = new SlaveTemplate(ami, EC2AbstractSlave.TEST_ZONE, null, "default", "foo", InstanceType.M1Large, "ttt", Node.Mode.NORMAL, description, "bar", "bbb", "aaa", "10", "fff", null, "-Xmx1g", false, "subnet 456", tags, null, false, null, "", true, false, "", false, "") {
@Override
protected Object readResolve() {
return null;
}
};

ArrayList<Tag> awsTags = new ArrayList<Tag>();
awsTags.add(new Tag(EC2Tag.TAG_NAME_JENKINS_SLAVE_TYPE, "value1"));
awsTags.add(new Tag(EC2Tag.TAG_NAME_JENKINS_SLAVE_TYPE, "value2"));

orig.updateRemoteTags(ec2, awsTags, "InvalidSpotInstanceRequestID.NotFound", instanceId);
Assert.assertEquals(5, handler.getRecords().size());

Iterator<LogRecord> logs = handler.getRecords().iterator();
while (logs.hasNext()) {
String log = logs.next().getMessage();
Assert.assertTrue(log.contains("Instance not found - InvalidInstanceRequestID.NotFound"));
}

}
}

class TestHandler extends Handler {
private List<LogRecord> records = new LinkedList<LogRecord>();

@Override
public void close() throws SecurityException {
}

@Override
public void flush() {
}

@Override
public void publish(LogRecord record) {
records.add(record);
}

public List<LogRecord> getRecords() {
return records;
}
}

0 comments on commit 141236b

Please sign in to comment.