-
Notifications
You must be signed in to change notification settings - Fork 105
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
Add nodes pre-provisioning #127
base: master
Are you sure you want to change the base?
Changes from 6 commits
3116d76
b9bab52
1656dfc
2a5abc1
256eaf4
335181d
1cbedb6
3fe45d5
d27517a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,6 +133,12 @@ public Integer getLimitedTestRunCount() { | |
return LimitedTestRunCount; | ||
} | ||
|
||
@Restricted(NoExternalUse.class) | ||
public vSphereCloudSlaveTemplate getTemplate() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this need to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not convinced that this method is used. I'm looking using my browser on github, and it says that "getTemplate(" only occurs here, where it's declared, and nowhere else in this PR. |
||
vSphereCloud cloud = findOurVsInstance(); | ||
return cloud.getTemplateForVM(getVmName()); | ||
} | ||
|
||
public boolean isLaunchSupportForced() { | ||
return ((vSphereCloudLauncher) getLauncher()).getOverrideLaunchSupported() == Boolean.TRUE; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
package org.jenkinsci.plugins.vsphere; | ||
|
||
import hudson.Extension; | ||
import hudson.Functions; | ||
import hudson.model.TaskListener; | ||
import hudson.model.AsyncPeriodicWork; | ||
import hudson.slaves.AbstractCloudSlave; | ||
import hudson.slaves.Cloud; | ||
import hudson.model.Label; | ||
import jenkins.model.Jenkins; | ||
import org.jenkinsci.plugins.vSphereCloud; | ||
import org.jenkinsci.plugins.vSphereCloudSlaveComputer; | ||
import org.jenkinsci.plugins.vSphereCloudSlaveTemplate; | ||
|
||
import java.io.IOException; | ||
import java.util.List; | ||
import java.util.logging.Level; | ||
import java.util.logging.Logger; | ||
|
||
import org.kohsuke.accmod.Restricted; | ||
import org.kohsuke.accmod.restrictions.NoExternalUse; | ||
|
||
/** | ||
* A {@link AsyncPeriodicWork} that reconciles nodes to meet template values. | ||
* <p> | ||
* The async work will check the number of deployed nodes and provision (or | ||
* delete) additional ones to meet template values. The check is happening every | ||
* 2 minutes. | ||
*/ | ||
@Extension | ||
@Restricted(NoExternalUse.class) | ||
public final class VSphereNodeReconcileWork extends AsyncPeriodicWork { | ||
private static final Logger LOGGER = Logger.getLogger(VSphereNodeReconcileWork.class.getName()); | ||
|
||
public VSphereNodeReconcileWork() { | ||
super("Vsphere nodes reconciliation"); | ||
} | ||
|
||
@Override | ||
public long getRecurrencePeriod() { | ||
return Functions.getIsUnitTest() ? Long.MAX_VALUE : MIN * 2; | ||
} | ||
|
||
@Override | ||
public void execute(TaskListener listener) { | ||
for (Cloud cloud : Jenkins.getActiveInstance().clouds) { | ||
if (!(cloud instanceof vSphereCloud)) continue; | ||
for (vSphereCloudSlaveTemplate template : ((vSphereCloud) cloud).getTemplates()) { | ||
String templateLabel = template.getLabelString(); | ||
Label label = Label.get(templateLabel); | ||
|
||
int instancesMin = template.getInstancesMin(); | ||
List<vSphereCloudSlaveComputer> idleNodes = template.getIdleNodes(); | ||
List<vSphereCloudSlaveComputer> runningNodes = template.getOnlineNodes(); | ||
// Get max number of nodes that could be provisioned | ||
int globalMaxNodes = ((vSphereCloud) cloud).getInstanceCap(); | ||
int templateMaxNodes = template.getTemplateInstanceCap(); | ||
int maxNodes = Math.min(globalMaxNodes, templateMaxNodes); | ||
|
||
// if maxNumber is lower than instancesMin, we have to ignore instancesMin | ||
int toProvision = Math.min(instancesMin - idleNodes.size(), maxNodes - runningNodes.size()); | ||
korablin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (toProvision > 0) { | ||
// provision desired number of nodes for this label | ||
LOGGER.log(Level.INFO, "Pre-creating {0} instance(s) for template {1} in cloud {3}", | ||
new Object[] { toProvision, templateLabel, cloud.name }); | ||
try { | ||
cloud.provision(label, toProvision); | ||
korablin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} catch (Throwable ex) { | ||
LOGGER.log(Level.SEVERE, "Failed to pre-create instance from template {0}. Exception: {1}", | ||
new Object[] { templateLabel, ex }); | ||
} | ||
} else if (toProvision < 0) { | ||
korablin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
int toDelete = Math.min(idleNodes.size(), Math.abs(toProvision)); | ||
for (int i = 0; i < toDelete; i++) { | ||
AbstractCloudSlave node = idleNodes.get(i).getNode(); | ||
korablin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (node == null) continue; | ||
LOGGER.log(Level.INFO, "Found excessive instance. Terminating {0} node {1}.", | ||
new Object[] { idleNodes.get(i).getName(), node }); | ||
try { | ||
node.terminate(); | ||
} catch (InterruptedException | IOException e) { | ||
LOGGER.log(Level.WARNING, e.getMessage()); | ||
// try to delete it later | ||
continue; | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<div> | ||
The number of VMs to be provisioned beforehand.<br> | ||
This allows to speed up CI runs by starting them immediately without waiting for a VM to get booted.<br> | ||
<dt>If the number is set to 0:</dt> | ||
<dd>No VMs provisioned in advance.</dd> | ||
<dt>If the number is bigger than 0:</dt> | ||
<dd>The plugin provisions new VMs to meet the value.</dd> | ||
<dt>If instances Min is bigger than instance Cap:</dt> | ||
<dd>The plugin provisions max number of VMs specified in instance Cap (the smallest of cloud and template options).</dd> | ||
The plugin checks the number of running VMs once in 2 minutes. | ||
<dt>The plugin respects retention policies.</dt> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be
public
?Could we get away with just default (package-level) visibility instead, i.e.
vSphereCloudSlaveTemplate getTemplateForVM(final String vmName) {
rather thanpublic vSphereCloudSlaveTemplate getTemplateForVM(final String vmName) {
?(we'll need the
@Restricted
as it's not private but package-level, orprotected
, would be preferred overpublic
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I think that) This method is only called from elsewhere within this class and the neighbouring vSphereCloudSlaveTemplate class; they're both in the same package.
I believe, therefore, that it doesn't need to be public and that it can be set to default visibility.
i.e.