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

Add nodes pre-provisioning #127

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 43 additions & 9 deletions src/main/java/org/jenkinsci/plugins/vSphereCloud.java
Original file line number Diff line number Diff line change
Expand Up @@ -352,22 +352,15 @@ public Collection<PlannedNode> provision(final Label label, int excessWorkload)
final List<PlannedNode> plannedNodes = new ArrayList<PlannedNode>();
synchronized (templateState) {
templateState.pruneUnwantedRecords();
Integer maxSlavesToProvisionBeforeCloudCapHit = calculateMaxAdditionalSlavesPermitted();
if (maxSlavesToProvisionBeforeCloudCapHit != null && maxSlavesToProvisionBeforeCloudCapHit <= 0) {
if (!cloudHasCapacity()) {
return Collections.emptySet(); // no capacity due to cloud instance cap
}
final List<vSphereCloudSlaveTemplate> templates = getTemplates(label);
final List<CloudProvisioningRecord> whatWeCouldUse = templateState.calculateProvisionableTemplates(templates);
VSLOG.log(Level.INFO, methodCallDescription + ": " + numberOfvSphereCloudSlaves + " existing slaves (="
+ numberOfvSphereCloudSlaveExecutors + " executors), templates available are " + whatWeCouldUse);
while (excessWorkloadSoFar > 0) {
if (maxSlavesToProvisionBeforeCloudCapHit != null) {
final int intValue = maxSlavesToProvisionBeforeCloudCapHit.intValue();
if (intValue <= 0) {
break; // out of capacity due to cloud instance cap
}
maxSlavesToProvisionBeforeCloudCapHit = Integer.valueOf(intValue - 1);
}
if (!cloudHasCapacity()) break;
Copy link
Author

@korablin korablin May 28, 2021

Choose a reason for hiding this comment

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

I am not subtracting 1 here since maxSlavesToProvisionBeforeCloudCapHit is re-calculated on every call to cloudHasCapacity()

Copy link
Member

Choose a reason for hiding this comment

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

Understood - done like this (re-calculating every time) is not as CPU-efficient as it was, but I guess the additional workload is insigificant compared to everything else.

final CloudProvisioningRecord whatWeShouldSpinUp = CloudProvisioningAlgorithm.findTemplateWithMostFreeCapacity(whatWeCouldUse);
if (whatWeShouldSpinUp == null) {
break; // out of capacity due to template instance cap
Expand All @@ -387,6 +380,47 @@ public Collection<PlannedNode> provision(final Label label, int excessWorkload)
}
}

/**
* Pre-provisions nodes per template to save time on a VM boot.
*
* @param template
*/
public void preProvisionNodes(vSphereCloudSlaveTemplate template) {
final String methodCallDescription = "preProvisionNodesForTemplate(" + template.getLabelString() + ")";
try {
synchronized (this) {
ensureLists();
}
retryVMdeletionIfNecessary(template.getInstancesMin());
synchronized (templateState) {
templateState.pruneUnwantedRecords();
final CloudProvisioningRecord provisionable = templateState.getOrCreateRecord(template);
int nodesToProvision = CloudProvisioningAlgorithm.shouldPreProvisionNodes(provisionable);
VSLOG.log(Level.INFO, methodCallDescription + ": should pre-provision " + nodesToProvision + " nodes");
while (nodesToProvision > 0) {
if (!cloudHasCapacity()) break;
final String nodeName = CloudProvisioningAlgorithm.findUnusedName(provisionable);
VSpherePlannedNode.createInstance(templateState, nodeName, provisionable);
nodesToProvision -= 1;
}
}
} catch (Exception ex) {
VSLOG.log(Level.WARNING, methodCallDescription + ": Failed.", ex);
}
}

/**
* Check if at least one additional node can be provisioned.
*/
private boolean cloudHasCapacity(){
Integer maxSlavesToProvisionBeforeCloudCapHit = calculateMaxAdditionalSlavesPermitted();
if (maxSlavesToProvisionBeforeCloudCapHit != null && maxSlavesToProvisionBeforeCloudCapHit <= 0) {
VSLOG.info("The cloud is at max capacity. Can not provison more nodes.");
return false;
}
return true;
}

/**
* Has another go at deleting VMs we failed to delete earlier. It's possible
* that we were unable to talk to vSphere (or some other failure happened)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;

import javax.annotation.Nonnull;

Expand Down Expand Up @@ -114,6 +113,7 @@ public class vSphereCloudSlaveTemplate implements Describable<vSphereCloudSlaveT
private final boolean saveFailure;
private final String targetResourcePool;
private final String targetHost;
private final int instancesMin;
/**
* Credentials from old configuration format. Credentials are now in the
* {@link #launcher} configuration
Expand Down Expand Up @@ -152,6 +152,7 @@ public vSphereCloudSlaveTemplate(final String cloneNamePrefix,
final boolean saveFailure,
final String targetResourcePool,
final String targetHost,
final int instancesMin,
final String credentialsId /*deprecated*/,
final ComputerLauncher launcher,
final RetentionStrategy<?> retentionStrategy,
Expand Down Expand Up @@ -181,6 +182,7 @@ public vSphereCloudSlaveTemplate(final String cloneNamePrefix,
this.saveFailure = saveFailure;
this.targetResourcePool = targetResourcePool;
this.targetHost = targetHost;
this.instancesMin = instancesMin;
this.credentialsId = credentialsId;
this.nodeProperties = Util.fixNull(nodeProperties);
this.guestInfoProperties = Util.fixNull(guestInfoProperties);
Expand Down Expand Up @@ -272,6 +274,10 @@ public int getLimitedRunCount() {
return this.limitedRunCount;
}

public int getInstancesMin() {
return this.instancesMin;
}

public boolean getSaveFailure() {
return this.saveFailure;
}
Expand Down Expand Up @@ -550,6 +556,10 @@ public FormValidation doCheckLimitedRunCount(@QueryParameter String limitedRunCo
return FormValidation.validateNonNegativeInteger(limitedRunCount);
}

public FormValidation doCheckInstancesMin(@QueryParameter String instancesMin) {
return FormValidation.validateNonNegativeInteger(instancesMin);
}

public FormValidation doCheckTemplateInstanceCap(@QueryParameter String templateInstanceCap) {
return FormValidation.validateNonNegativeInteger(templateInstanceCap);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package org.jenkinsci.plugins.vsphere;

import hudson.Extension;
import hudson.Functions;
import hudson.model.TaskListener;
import hudson.model.AsyncPeriodicWork;
import hudson.slaves.Cloud;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.vSphereCloud;
import org.jenkinsci.plugins.vSphereCloudSlaveTemplate;

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 pre-provisions nodes to meet insntanceMin template value.
* <p>
* The async work will check the number of active nodes
* and provision additional ones to meet template values.
*
* The check is happening every 2 minutes.
*/
@Extension
@Restricted(NoExternalUse.class)
public final class VSpherePreProvisonWork extends AsyncPeriodicWork {
private static final Logger LOGGER = Logger.getLogger(VSpherePreProvisonWork.class.getName());

public VSpherePreProvisonWork() {
super("Vsphere pre-provision check");
}

@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;
vSphereCloud vsCloud = (vSphereCloud) cloud;
for (vSphereCloudSlaveTemplate template : vsCloud.getTemplates()) {
if (template.getInstancesMin() > 0) {
LOGGER.log(Level.INFO, "Check if template (label=" + template.getLabelString() + ") has enough active nodes to meet instances Min value");
vsCloud.preProvisionNodes(template);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,23 @@ public static String findUnusedName(CloudProvisioningRecord record) {
+ ", even after " + maxAttempts + " attempts.");
}

/**
* Compares sum of provisioned and planned nodes for the template.
*
* If the sum is less than instanceMin template value we should provision more nodes,
* otherwise the value is satisfied and we should not add any more nodes yet.
*
* @param record
* Our record regarding the template the agent will be created
* from.
* @return A number of nodes to be provisioned.
*/
public static int shouldPreProvisionNodes(CloudProvisioningRecord record) {
int provisionedNodes = record.getCurrentlyProvisioned().size() + record.getCurrentlyPlanned().size();
int requiredPreProvisionedNodes = record.getTemplate().getInstancesMin();
return requiredPreProvisionedNodes - provisionedNodes;
}
Copy link
Member

Choose a reason for hiding this comment

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

We're going to need a unit test for this new method.


private static String calcSequentialSuffix(final int attempt) {
final int slaveNumber = attempt + 1;
final String suffix = Integer.toString(slaveNumber);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@
<f:textbox clazz="required number" default="0"/>
</f:entry>

<f:entry title="${%Instances Min}" field="instancesMin">
<f:textbox clazz="required number" default="0"/>
</f:entry>

<f:entry title="${%# of Executors}" field="numberOfExecutors">
<f:textbox clazz="required positive-number" default="1"/>
</f:entry>
Expand Down
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>Pre-provisoned VMs will be deleted based on the retention policy.</dt>
</div>
109 changes: 109 additions & 0 deletions src/test/java/org/jenkinsci/plugins/NodePreProvisionTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package org.jenkinsci.plugins;

import static org.junit.Assert.*;
import static org.hamcrest.CoreMatchers.*;


import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;

import org.jenkinsci.plugins.vSphereCloud;
import org.jenkinsci.plugins.vSphereCloudSlaveTemplate;
import org.jenkinsci.plugins.vSphereCloud.VSpherePlannedNode;
import org.jenkinsci.plugins.vsphere.VSphereConnectionConfig;
import org.jenkinsci.plugins.vsphere.tools.CloudProvisioningAlgorithm;
import org.jenkinsci.plugins.vsphere.tools.CloudProvisioningRecord;
import org.jenkinsci.plugins.vsphere.tools.CloudProvisioningState;

import hudson.model.Label;
import hudson.slaves.JNLPLauncher;
import hudson.slaves.RetentionStrategy;
import hudson.slaves.NodeProvisioner.PlannedNode;

import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;

public class NodePreProvisionTest {
/** Used when faking up test data */
private static List<vSphereCloudSlaveTemplate> stubVSphereCloudTemplates;
private static VSphereConnectionConfig vsConnectionConfig;
private static vSphereCloud stubVSphereCloud;
private static CloudProvisioningState stubVSphereTemplateState;
private Logger testLogger;
private List<LogRecord> loggedMessages;

@BeforeClass
public static void setupClass() {
stubVSphereCloudTemplates = new ArrayList<vSphereCloudSlaveTemplate>();
vsConnectionConfig = new VSphereConnectionConfig("vsHost", false, "credentialsId");
stubVSphereCloud = new vSphereCloud(vsConnectionConfig, "vsDescription", 100, 100, stubVSphereCloudTemplates);
stubVSphereTemplateState = new CloudProvisioningState(stubVSphereCloud);
}

@Before
public void setup() {
stubVSphereCloudTemplates.clear();
loggedMessages = new ArrayList<LogRecord>();
// Get vSphereCloud logger
Logger logger = Logger.getLogger("vsphere-cloud");
logger.setLevel(Level.ALL);
// final Handler[] handlers = logger.getHandlers();
// for (final Handler handler : handlers) {
// logger.removeHandler(handler);
// }
final Handler testHandler = new Handler() {
@Override
public void publish(LogRecord record) {
loggedMessages.add(record);
}

@Override
public void flush() {
}

@Override
public void close() {
}
};
logger.addHandler(testHandler);
testLogger = logger;
}

@Test
public void shouldPreProvisionNodesWhenNotEnough() {
vSphereCloudSlaveTemplate template = createTemplate(10, 2);
provisionNode(template);
assertThat(stubVSphereTemplateState.countNodes(), equalTo(1));

// Here it says that there still should be provisioned 2 nodes, despite the fact there is 1 active already
stubVSphereCloud.preProvisionNodes(template);
Copy link
Author

@korablin korablin Jun 1, 2021

Choose a reason for hiding this comment

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

Logs:

Jun 01, 2021 5:54:23 PM org.jenkinsci.plugins.vSphereCloud InternalLog
INFO: STARTING VSPHERE CLOUD

Jun 01, 2021 5:54:23 PM org.jenkinsci.plugins.vsphere.tools.CloudProvisioningState logStateChange
WARNING: Marking templateCapacity10instanceMin21 as active : wasPreviouslyPlanned!=true

Jun 01, 2021 5:54:24 PM org.jenkinsci.plugins.vSphereCloud preProvisionNodes
INFO: preProvisionNodesForTemplate(null): should pre-provision 2 nodes

Jun 01, 2021 5:54:24 PM org.jenkinsci.plugins.vSphereCloud calculateMaxAdditionalSlavesPermitted
INFO: There are 0 VMs in this cloud. The instance cap for the cloud is 100, so we have room for more

Jun 01, 2021 5:54:24 PM org.jenkinsci.plugins.vSphereCloud calculateMaxAdditionalSlavesPermitted
INFO: There are 1 VMs in this cloud. The instance cap for the cloud is 100, so we have room for more

Copy link
Author

@korablin korablin Jun 2, 2021

Choose a reason for hiding this comment

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

Note, that when preProvision is invoked it actually sees that "There are 1 VMs in this cloud", which was "provisioned" by the method just a moment ago.
So, I would love to achieve the same state without calling preProvision(); I've tried marking the associated record as provisionedSlaveNowActive or provisioningStarted, but that doesn't do the trick, neither calling VSpherePlannedNode.createInstance(stubVSphereTemplateState, nodeName, provisionable);


// Below is a draft line
//assertThat(loggedMessages.get(1).getMessage(), equalTo("should pre-provision 1 node"));

}

private vSphereCloudSlaveTemplate createTemplate(int templateCapacity, int instanceMin){
return stubTemplate("templateCapacity" + templateCapacity + "instanceMin" + instanceMin, templateCapacity, instanceMin);
}

private void provisionNode(vSphereCloudSlaveTemplate template) {
CloudProvisioningRecord provisionable = stubVSphereTemplateState.getOrCreateRecord(template);
final String nodeName = CloudProvisioningAlgorithm.findUnusedName(provisionable);
stubVSphereTemplateState.provisionedSlaveNowActive(provisionable, nodeName);
// Below doesn't work either
//VSpherePlannedNode.createInstance(stubVSphereTemplateState, nodeName, provisionable);
}

private static vSphereCloudSlaveTemplate stubTemplate(String prefix, int templateInstanceCap, int instanceMin) {
return new vSphereCloudSlaveTemplate(prefix, "", null, null, false, null, null, null, null, null, null, templateInstanceCap, 1,
null, null, null, false, false, 0, 0, false, null, null, instanceMin, null, new JNLPLauncher(),
RetentionStrategy.NOOP, null, null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,21 @@ public void findUnusedNameGivenUncappedInstancesThenReturnsUniqueNames() {
assertThat(actuals, everyItem(startsWith(prefix)));
}

@Test
public void shouldPreProvisionNodesWhenNotEnough() {
// we don't care about cap here
// Given
int provisioned = 3;
int planned = 2;
final CloudProvisioningRecord record = createInstance(10, provisioned, planned);

// When
int instanceMin = record.getTemplate().getInstancesMin();

// Then
assertThat(CloudProvisioningAlgorithm.shouldPreProvisionNodes(record), equalTo(instanceMin - (provisioned + planned)));
}

private CloudProvisioningRecord createInstance(int capacity, int provisioned, int planned) {
final int iNum = ++instanceNumber;
final vSphereCloudSlaveTemplate template = stubTemplate(iNum + "cap" + capacity, capacity);
Expand All @@ -316,7 +331,7 @@ private CloudProvisioningRecord createInstance(int capacity, int provisioned, in

private static vSphereCloudSlaveTemplate stubTemplate(String prefix, int templateInstanceCap) {
return new vSphereCloudSlaveTemplate(prefix, "", null, null, false, null, null, null, null, null, null, templateInstanceCap, 1,
null, null, null, false, false, 0, 0, false, null, null, null, new JNLPLauncher(),
null, null, null, false, false, 0, 0, false, null, null, 2, null, new JNLPLauncher(),
RetentionStrategy.NOOP, null, null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ private CloudProvisioningRecord createRecord(CloudProvisioningState instance) {
final String cloneNamePrefix = "prefix" + recordNumber;
final vSphereCloudSlaveTemplate template = new vSphereCloudSlaveTemplate(cloneNamePrefix, "masterImageName",
null, "snapshotName", false, "cluster", "resourcePool", "datastore", "folder", "customizationSpec", "templateDescription", 0, 1, "remoteFS",
"", Mode.NORMAL, false, false, 0, 0, false, "targetResourcePool", "targetHost", null,
"", Mode.NORMAL, false, false, 0, 0, false, "targetResourcePool", "targetHost", 0, null,
new JNLPLauncher(), RetentionStrategy.NOOP, Collections.<NodeProperty<?>> emptyList(),
Collections.<VSphereGuestInfoProperty> emptyList());
stubVSphereCloudTemplates.add(template);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public void should_support_configuration_as_code() {
assertThat(template.getTemplateInstanceCap(), is(5));
assertThat(template.getUseSnapshot(), is(true));
assertThat(template.getWaitForVMTools(), is(true));
assertThat(template.getInstancesMin(), is(3));
List<? extends VSphereGuestInfoProperty> guestInfoProperties = template.getGuestInfoProperties();
assertThat(guestInfoProperties, hasSize(1));
VSphereGuestInfoProperty guestInfoProperty = guestInfoProperties.get(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jenkins:
# ^ escapes the secret
- name: "JENKINS_URL"
value: "^${JENKINS_URL}"
instancesMin: 3
labelString: "windows vsphere"
launchDelay: 60
launcher:
Expand Down
Loading