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

[JENKINS-23792] Do not terminate stopped EC2 nodes that were just booted #632

Merged
merged 8 commits into from Jun 18, 2021

Conversation

unicolet
Copy link
Contributor

@unicolet unicolet commented Jun 14, 2021

when a node just booted it is not connected, and simultaneously its idle start time could still be set to the previous time the node was booted. If the instance is checked by the plugin while in this phase, the plugin will erroneously decide to terminate it again. The change in this PR adds a safety check to ensure that idle time is calculated from the greatest of idle time as returned by jenkins or the node uptime, as returned by EC2.

Fixes: https://issues.jenkins.io/browse/JENKINS-23792

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

…again

when a node just booted it is not connected, and simultaneously its idle start time
could still be set to the last time the node was booted. If the instance is checked by the plugin while in this phase, the plugin will erroneously decide to terminate it again. The change in this PR adds a safety check to ensure that
idle time is calculated from the greatest of idle time as returned by jenkins or the node uptime, as returned by EC2.

Fixes: https://issues.jenkins.io/browse/JENKINS-23792
@@ -86,6 +86,7 @@ private EC2Computer computerWithIdleTime(final int minutes, final int seconds) t
* the computer returns the value established.
*/
private EC2Computer computerWithIdleTime(final int minutes, final int seconds, final Boolean isOffline, final Boolean isConnecting) throws Exception {
idleTimeoutCalled.set(false);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

required, since the state is otherwise never reset across tests. Admittedly, this could be fixed better, but I wanted to keep the PR scope as small as possible to make it easy to review :-)

@unicolet
Copy link
Contributor Author

unicolet commented Jun 14, 2021

@res0nance @MRamonLeon could you please take a look? This issue is affecting our jenkins setup a lot :-( I'd be more than happy to timely clarify any question you'll send our way. Thanks!

Copy link
Contributor

@res0nance res0nance left a comment

Choose a reason for hiding this comment

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

From my understanding this happens only if the stop option is used?

@unicolet
Copy link
Contributor Author

From my understanding this happens only if the stop option is used?

@res0nance yes, that's my understanding too

@unicolet
Copy link
Contributor Author

@res0nance please take a look again. Thanks for the prompt reply ❤️

@unicolet unicolet requested a review from res0nance June 14, 2021 13:05
@unicolet unicolet requested a review from res0nance June 16, 2021 13:31
@unicolet
Copy link
Contributor Author

@res0nance thanks for the review ❤️ How does the process for merging and releasing this plugin looks like? in other words, when will the plugin be avail for upgrade? 😅

@res0nance res0nance changed the title [JENKINS-23792] Regression: do not terminate stopped EC2 nodes that were just booted [JENKINS-23792] Do not terminate stopped EC2 nodes that were just booted Jun 18, 2021
@res0nance res0nance merged commit 64ab73b into jenkinsci:master Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants