Skip to content

Commit

Permalink
Refactor the code to address comments
Browse files Browse the repository at this point in the history
* Use native logic that takes into account state of the nodes
  • Loading branch information
nkorabli committed May 28, 2021
1 parent 1cbedb6 commit 3fe45d5
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 156 deletions.
57 changes: 44 additions & 13 deletions src/main/java/org/jenkinsci/plugins/vSphereCloud.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import org.jenkinsci.plugins.folder.FolderVSphereCloudProperty;
import org.jenkinsci.plugins.vsphere.VSphereConnectionConfig;
import org.jenkinsci.plugins.vsphere.tools.*;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.Stapler;
Expand Down Expand Up @@ -217,8 +215,7 @@ public List<? extends vSphereCloudSlaveTemplate> getTemplates() {
return this.templates;
}

@Restricted(NoExternalUse.class)
vSphereCloudSlaveTemplate getTemplateForVM(final String vmName) {
private vSphereCloudSlaveTemplate getTemplateForVM(final String vmName) {
if (this.templates == null || vmName == null)
return null;
for (final vSphereCloudSlaveTemplate t : this.templates) {
Expand Down Expand Up @@ -355,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;
final CloudProvisioningRecord whatWeShouldSpinUp = CloudProvisioningAlgorithm.findTemplateWithMostFreeCapacity(whatWeCouldUse);
if (whatWeShouldSpinUp == null) {
break; // out of capacity due to template instance cap
Expand All @@ -390,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
20 changes: 0 additions & 20 deletions src/main/java/org/jenkinsci/plugins/vSphereCloudSlaveComputer.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,8 @@

import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.List;

import javax.annotation.Nonnull;
import org.jenkinsci.plugins.vsphere.tools.VSphere;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

import com.vmware.vim25.VirtualHardware;
import com.vmware.vim25.VirtualMachineConfigInfo;
Expand All @@ -19,10 +14,8 @@
import com.vmware.vim25.mo.ManagedEntity;
import com.vmware.vim25.mo.VirtualMachine;

import hudson.model.Computer;
import hudson.slaves.AbstractCloudComputer;
import hudson.slaves.AbstractCloudSlave;
import jenkins.model.Jenkins;

public class vSphereCloudSlaveComputer extends AbstractCloudComputer {
private final vSphereCloudSlave vSlave;
Expand Down Expand Up @@ -84,19 +77,6 @@ public String getVmInformationError() {
return getVMInformation().errorEncounteredWhenDataWasRead;
}

/**
* Get all vsphere computers.
*/
@Restricted(NoExternalUse.class)
static @Nonnull List<vSphereCloudSlaveComputer> getAll() {
ArrayList<vSphereCloudSlaveComputer> out = new ArrayList<>();
for (final Computer c : Jenkins.get().getComputers()) {
if (!(c instanceof vSphereCloudSlaveComputer)) continue;
out.add((vSphereCloudSlaveComputer) c);
}
return out;
}

/** 10 seconds */
private static final long NANOSECONDS_TO_CACHE_VMINFORMATION = 10L * 1000000000L;

Expand Down
31 changes: 0 additions & 31 deletions src/main/java/org/jenkinsci/plugins/vSphereCloudSlaveTemplate.java
Original file line number Diff line number Diff line change
Expand Up @@ -325,37 +325,6 @@ public RetentionStrategy<?> getRetentionStrategy() {
return this.retentionStrategy;
}

/**
* Return a list of running nodes provisioned using this template.
*/
@Restricted(NoExternalUse.class)
public List<vSphereCloudSlaveComputer> getOnlineNodes() {
return getNodes(false);
}

/**
* Return a list of idle nodes provisioned using this template.
*/
@Restricted(NoExternalUse.class)
public List<vSphereCloudSlaveComputer> getIdleNodes() {
return getNodes(true);
}

private List<vSphereCloudSlaveComputer> getNodes(boolean idle) {
List<vSphereCloudSlaveComputer> nodes = new ArrayList<>();
for (vSphereCloudSlaveComputer node : vSphereCloudSlaveComputer.getAll()) {
if (!node.isOnline()) continue;
if (idle && !node.isIdle()) continue;
String vmName = node.getName();
vSphereCloudSlaveTemplate nodeTemplate = getParent().getTemplateForVM(vmName);
// Filter out nodes from other clouds: nodeTemplate is null for these.
if (nodeTemplate == null) continue;
if (getLabelString() != nodeTemplate.getLabelString()) continue;
nodes.add(node);
}
return nodes;
}

protected Object readResolve() {
this.labelSet = Label.parse(labelString);
if(this.templateInstanceCap == 0) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
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;
for (vSphereCloudSlaveTemplate template : ((vSphereCloud) cloud).getTemplates()) {
if (template.getInstancesMin() > 0) {
LOGGER.log(Level.INFO, "Check if template (label=" + template.getLabelString() + ") has enough active nodes to meet instances Min value");
((vSphereCloud) cloud).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;
}

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 @@ -8,5 +8,5 @@
<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>
<dt>Pre-provisoned VMs will be deleted based on the retention policy.</dt>
</div>

0 comments on commit 3fe45d5

Please sign in to comment.