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.

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

Terminate the EC2 on receiving an exception #952

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BushraManga
Copy link

@BushraManga BushraManga commented Mar 19, 2024

Problem:
Currently, for our infrastructure we are executing jobs on our EC2 nodes. In the node initialisation script we we will fetch one of the mandate auth file. If the file is not present or if it is empty we are using exit 1 to raise an exception.
Ideally, on receiving exit 1 the container should be gracefully terminated but we have noticed that as per script EC2UnixLauncher.java it will just log a warning message & continue. No termination action is taken.

Proposed Solution:
This PR makes a simple change to the EC2 termination logic so that it takes account of exception raised.
We would like the EC2 node be terminated gracefully whenever it encounters any IOException at the same time log a warning message.

Testing done
None yet - EC2UnixLauncher.java seems like the right place to add a test case for this specific scenario.

Right now this has been causing potential impact for our end users but the frequency is minimal. We want to at least get the proposed solution be analysed & reviewed for discussion.

@BushraManga
Copy link
Author

@lemeurherve Can you please take a look at this PR or ask some one else to do so?

@@ -210,9 +210,9 @@ protected void launchScript(EC2Computer computer, TaskListener listener) throws
int exitStatus = waitCompletion(sess);
if (exitStatus != 0) {
logWarning(computer, listener, "init script failed: exit code=" + exitStatus);
return;
sess.close();
Copy link
Contributor

Choose a reason for hiding this comment

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

This will clearly not close the sess object if the script is successful, you probably should hoist this above the if.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants