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

Fix unnecessary decrement of maxTotalUses and add clarity to logs around terminations triggered by plugin #375

Merged
merged 5 commits into from
Jun 23, 2023

Conversation

pdk27
Copy link
Collaborator

@pdk27 pdk27 commented Jun 21, 2023

Issue

#363

Changes

  • add and track EC2AgentTerminationReason for cases when plugin triggers termination
  • [fix] remove unnecessary decrement which could lead to a positive maxTotalUses eventually decrement to -1.
    '-1' has special meaning, means unlimited uses which is the opposite of the maxTotalUses.
  • [fix] remove misleading log to prevent logs like Agent xxx is still in use by more than one (1) executors.
  • [fix] remove redundant computer.setAcceptingTasks(false)
  • rename
    • EC2TerminationCause to EC2ExecutorInterruptionCause for clarity
    • rename force to overrideOtherSettings in scheduleToTerminate for clarity

Testing done

Tested all changes with a snapshot version of the plugin.

Extra logs look like

[INFO  ][com.amazon.jenkins.ec2fleet.EC2FleetCloud info] EC2FleetCloud1 [spot-workers] Scheduling instance 'i-xxx' for termination on cloud com.amazon.jenkins.ec2fleet.EC2FleetCloud@xxx because of reason: MaxTotalUses exhausted for agent  

[FINE  ][com.amazon.jenkins.ec2fleet.EC2FleetCloud fine] EC2FleetCloud1 [spot-workers] InstanceIdsToTerminate: {i-xxx=MaxTotalUses exhausted for agent}

[FINE  ][com.amazon.jenkins.ec2fleet.EC2FleetCloud fine] SpotFleetCloud1 [spot-workers-2] InstanceIdsToTerminate: {i-yyy=Agent idle for too long} 

@pdk27 pdk27 added the do not merge Don't merge this (at least not yet) label Jun 21, 2023
@pdk27 pdk27 marked this pull request as ready for review June 22, 2023 15:42
@pdk27 pdk27 requested a review from cjerad June 22, 2023 18:23
@pdk27 pdk27 merged commit 9450fa8 into jenkinsci:master Jun 23, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Don't merge this (at least not yet)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants