[JENKINS-56839] Allow disabling T2/T3 Unlimited Mode for T3 instances #375
Conversation
Previously, it was only possible to either explicitly enable Unlimited
Mode, or use the AWS default ("unlimited" for T3 and T3a instances, and
"standard"/limited for T2 instances).
|
It seems like the Windows node used to run checks for pull requests is unreliable:
Is there any way for me to trigger a re-build? I logged into |
|
Close and open PR should trigger a rebuild |
|
I would prefer to use a simple solution, that is to set the value all the time and not to change the interface |
|
@thoulen, all right, I can do that. |
|
@thoulen: While implementing your suggestion, testing showed that AWS allows specifying a CPU credit option (i. e. the Unlimited Mode option) only for T2/T3 instances. Specifying any CPU credit option for other instance types, no matter the value of the option, leads to AWS rejecting the API request: The only solution is to not specify any CPU credit option at all in this case. So it looks like we do need the "Use Instance Type Default" option after all so that people can properly use instance types other than T2/T3. @res0nance: I'm not familar with JCasC, but shouldn't things continue working as-is as long as |
|
Thanks for the try |
|
The build is failing because https://repo.azure.jenkins.io/ seems to be experiencing issues. :( |
- Deprecate old constructor instead of modifying it (and use the old one where appropriate). - Resolve burstableUnlimitedMode in readResolve().
| @@ -175,7 +181,8 @@ public SlaveTemplate(String ami, String zone, SpotConfiguration spotConfig, Stri | |||
| String instanceCapStr, String iamInstanceProfile, boolean deleteRootOnTermination, | |||
| boolean useEphemeralDevices, boolean useDedicatedTenancy, String launchTimeoutStr, boolean associatePublicIp, | |||
| String customDeviceMapping, boolean connectBySSHProcess, boolean monitoring, | |||
| boolean t2Unlimited, ConnectionStrategy connectionStrategy, int maxTotalUses) { | |||
| boolean t2Unlimited, BurstableUnlimitedMode burstableUnlimitedMode, ConnectionStrategy connectionStrategy, | |||
There was a problem hiding this comment.
I wonder whether t2Unlimited should maybe be removed completely from this constructor and be made into a (deprecated) DataBoundSetter instead (to make it clear that t2Unlimited is deprecated; its presence in the DataBoundConstructor signature might be confusing).
There was a problem hiding this comment.
I just realized that this would break the migration of t2Unlimited, unless we move that from readResolve() back to e. g. getBurstableUnlimitedMode()... So let's leave things like they are for now.
There was a problem hiding this comment.
You can do it this way, it removes the Deprecated field from the ctor to make it less confusing.
There was a problem hiding this comment.
Ah, right. I will look into that.
|
@res0nance: I addressed your suggestions; please have a look. |
Quoting the JIRA issue:
This pull request implements the latter suggestion. Instead of a simple checkbox for Unlimited Mode, there now is a drop-down, allowing the administrator to choose whether to explicitly enable/disable Unlimited Mode, or whether to use the AWS default for the chosen instance type (by not specifying any preference for Unlimited Mode when creating instances).
The old boolean setting is migrated to preserve existing configurations.