keep track of instances being provisioned; use this count when determining total/AMI instance caps #25

Merged
merged 1 commit into from Feb 7, 2013

Conversation

Projects
None yet
4 participants
Contributor

zzzeek commented Oct 10, 2012

This is regarding https://issues.jenkins-ci.org/browse/JENKINS-6691 .

This change adds a static map to the EC2Cloud class, which stores string AMI names mapped to an integer count of how many instances of this AMI are undergoing provisioning. As EC2Cloud.provision() adds PlannedNode instances, it checks a new method addProvisionedSlave(), which replaces the previously inlined checks against Amazon's reported numbers. addProvisionedSlave() checks not just Amazon's reporting of running EC2 instances, but also adds the number of nodes currently under "provisioning". addProvisionedSlave() and it's complementing decrementAmiSlaveProvision() method both synchronize access to the essentially global map as the counts are read, increased, and decreased.

I've been testing this for several hours, both by starting lots of jobs manually and also watching as a cluster of jobs are started at once due to an event; I now get the correct number of EC2 nodes spawned, and consistent messaging as to the status when there are already too many nodes present.

@zzzeek zzzeek - add a new mechanism to help count the total number of EC2 instances…
… for a particular AMI. As an EC2Slave

is being provisioned, a temporary count is placed in a HashMap, which is used in addition to the
count reported by Amazon itself for a particular ami.   This is so that the count of total
nodes provisioned takes into account those which Amazon doesn't report yet.  The count returned
may be too high, if amazon reports it in addition to the "provision" count, but better to err on the
side of not spawning a node too soon; it will get spawned on the next go-around.
Tries to fix [JENKINS-6691].
8d0a9fa

Jenkins » ec2-plugin #32 SUCCESS
This pull request looks good
(what's this?)

Member

francisu commented Feb 7, 2013

This appears to be superseded by your more recent pull request, but since it can be automatically merged, I'm just going to take it now. Please confirm that you are OK with the Jenkins-MIT license for this contribution.

@francisu francisu added a commit that referenced this pull request Feb 7, 2013

@francisu francisu Merge pull request #25 from zzzeek/master
keep track of instances being provisioned; use this count when determining total/AMI instance caps
de72f67

@francisu francisu merged commit de72f67 into jenkinsci:master Feb 7, 2013

Contributor

kpfleming commented Feb 7, 2013

I've got another patch on the way shortly that completely revamps instance cap management, and avoids reliance on the Amazon instance list; it's not reliable, and it will break when the plugin (finally) properly supports multiple cloud definitions that share the same AWS user account. It will require a Jenkins core API change (so that the plugin can track PlannedNode instances), but it completely solves this problem for good. Stay tuned!

Contributor

zzzeek commented Feb 7, 2013

hm OK, does this mean I have to re-format/ re-submit my other pullreq #33 a third time?

Member

francisu commented Feb 8, 2013

Mike, your pull request is now in, so Kevin will adjust his patch to
account for what you did. You are done. Thanks for your contribution.

On Thu, Feb 7, 2013 at 2:36 PM, mike bayer notifications@github.com wrote:

hm OK, does this mean I have to re-format/ re-submit my other pullreq #33https://github.com/jenkinsci/ec2-plugin/issues/33a third time?


Reply to this email directly or view it on GitHubhttps://github.com/jenkinsci/ec2-plugin/pull/25#issuecomment-13266201.

Cell 510 432 1589

Contributor

zzzeek commented Feb 8, 2013

just saw that, thanks much !

@kpfleming kpfleming added a commit to kpfleming/jenkins-ec2-plugin that referenced this pull request Feb 12, 2013

@kpfleming kpfleming Revert "Merge pull request #25 from zzzeek/master"
This reverts commit de72f67, reversing
changes made to 082f3e5.
51306d1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment