Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support fail fast for node removal #998

Closed
wants to merge 5 commits into from
Closed

support fail fast for node removal #998

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
81617a6
Use AbstractCloudSlave and AbstractCloudComputer
car-roll Oct 28, 2024
bbcfe07
cleanup
car-roll Oct 29, 2024
753e04a
address comments
car-roll Oct 31, 2024
8b95a32
use Functions.printStackTrace
car-roll Nov 1, 2024
b032ed0
update checkRetentionStrategy
car-roll Nov 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/main/java/hudson/plugins/ec2/EC2OndemandSlave.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted
Jenkins.get().removeNode(this);
LOGGER.info("Removed EC2 instance from jenkins controller: " + getInstanceId());
} catch (AmazonClientException | IOException e) {
listener.error("Failed to terminate EC2 instance: " + getInstanceId());
LOGGER.log(Level.WARNING, "Failed to terminate EC2 instance: " + getInstanceId(), e);
e.printStackTrace(listener.error("Failed to terminate EC2 instance: " + getInstanceId()));
car-roll marked this conversation as resolved.
Show resolved Hide resolved
} finally {
synchronized(terminateScheduled) {
terminateScheduled.countDown();
Expand Down
9 changes: 3 additions & 6 deletions src/main/java/hudson/plugins/ec2/EC2SpotSlave.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted
LOGGER.info("Cancelled Spot request: " + spotInstanceRequestId);
} catch (AmazonClientException e) {
// Spot request is no longer valid
listener.error("Failed to cancel Spot request: " + spotInstanceRequestId);
LOGGER.log(Level.WARNING, "Failed to cancel Spot request: " + spotInstanceRequestId, e);
e.printStackTrace(listener.error("Failed to cancel Spot request: " + spotInstanceRequestId));
car-roll marked this conversation as resolved.
Show resolved Hide resolved
}

// Terminate the agent if it is running
Expand All @@ -102,8 +101,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted
LOGGER.info("Terminated EC2 instance (terminated): " + instanceId);
} catch (AmazonClientException e) {
// Spot request is no longer valid
listener.error("Failed to terminate the Spot instance: " + instanceId);
LOGGER.log(Level.WARNING, "Failed to terminate the Spot instance: " + instanceId, e);
e.printStackTrace(listener.error("Failed to terminate the Spot instance: " + instanceId));
car-roll marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand All @@ -117,8 +115,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted
try {
Jenkins.get().removeNode(this);
} catch (IOException e) {
listener.error("Failed to remove agent");
LOGGER.log(Level.WARNING, "Failed to remove agent: " + name, e);
e.printStackTrace(listener.error("Failed to remove agent"));
car-roll marked this conversation as resolved.
Show resolved Hide resolved
}
synchronized(terminateScheduled) {
terminateScheduled.countDown();
Expand Down
2 changes: 0 additions & 2 deletions src/test/java/hudson/plugins/ec2/EC2AbstractSlaveTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ public void testGetLaunchTimeoutInMillisShouldNotOverflow() throws Exception {

@Override
protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
return;
}

@Override
Expand All @@ -59,7 +58,6 @@ public void testMaxUsesBackwardCompat() throws Exception {
EC2AbstractSlave slave = new EC2AbstractSlave("name", "", description, "fs", 1, null, "label", null, null, "init", "tmpDir", new ArrayList<NodeProperty<?>>(), "root", "jvm", false, "idle", null, cloudName, false, Integer.MAX_VALUE, new UnixData("remote", null, null, "22", null), ConnectionStrategy.PRIVATE_IP, 0) {
@Override
protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
return;
}

@Override
Expand Down
12 changes: 1 addition & 11 deletions src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing in CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this one is mystifying me. I'm still trying to figure out why

Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,17 @@
import hudson.plugins.ec2.util.SSHCredentialHelper;
import hudson.security.ACL;
import hudson.security.AccessControlled;
import hudson.security.Permission;import hudson.model.Executor;
import hudson.security.Permission;
import hudson.slaves.NodeProperty;
import hudson.slaves.OfflineCause;
import jenkins.util.NonLocalizable;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.acegisecurity.Authentication;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;
import org.testcontainers.shaded.org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.jvnet.hudson.test.LoggerRule;

import java.security.Security;
import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
Expand All @@ -48,18 +45,13 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.logging.Level;
import java.util.stream.Collectors;
import jenkins.util.NonLocalizable;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasItem;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.LoggerRule;

public class EC2RetentionStrategyTest {

Expand Down Expand Up @@ -212,7 +204,6 @@ private EC2Computer computerWithIdleTime(final int minutes, final int seconds, f
final EC2AbstractSlave slave = new EC2AbstractSlave("name", "id", "description", "fs", 1, null, "label", null, null, "init", "tmpDir", new ArrayList<NodeProperty<?>>(), "remote", "jvm", false, "idle", null, "cloud", false, Integer.MAX_VALUE, null, ConnectionStrategy.PRIVATE_IP, -1) {
@Override
protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
return;
}

@Override
Expand Down Expand Up @@ -282,7 +273,6 @@ private EC2Computer computerWithUpTime(final int minutes, final int seconds, fin
final EC2AbstractSlave slave = new EC2AbstractSlave("name", "id", "description", "fs", 1, null, "label", null, null, "init", "tmpDir", new ArrayList<NodeProperty<?>>(), "remote", "jvm", false, "idle", null, "cloud", false, Integer.MAX_VALUE, null, ConnectionStrategy.PRIVATE_IP, -1) {
@Override
protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
return;
}

@Override
Expand Down
1 change: 0 additions & 1 deletion src/test/java/hudson/plugins/ec2/ssh/verifiers/SshHostKeyVerificationStrategyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ private static MockEC2Computer createComputer(String suffix) throws Exception {
final EC2AbstractSlave slave = new EC2AbstractSlave(COMPUTER_NAME + suffix, "id" + suffix, "description" + suffix, "fs", 1, null, "label", null, null, "init", "tmpDir", new ArrayList<NodeProperty<?>>(), "remote", "jvm", false, "idle", null, "cloud", false, Integer.MAX_VALUE, null, ConnectionStrategy.PRIVATE_IP, -1) {
@Override
protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
return;
}

@Override
Expand Down