Skip to content

Commit

Permalink
Merge pull request #279 from Vlatombe/JENKINS-49166
Browse files Browse the repository at this point in the history
 [JENKINS-49166] Store definition of dynamic templates to a separate configuration than kubernetes cloud
  • Loading branch information
carlossg committed Jan 29, 2018
1 parent 9453d84 commit 42ccc75
Show file tree
Hide file tree
Showing 6 changed files with 246 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -229,4 +229,25 @@ public List<? extends Descriptor> getEnvVarsDescriptors() {
return DescriptorVisibilityFilter.apply(null, Jenkins.getInstance().getDescriptorList(TemplateEnvVar.class));
}
}

@Override
public String toString() {
return "ContainerTemplate{" +
(name == null ? "" : "name='" + name + '\'') +
(image == null ? "" : ", image='" + image + '\'') +
(!privileged ? "" : ", privileged=" + privileged) +
(!alwaysPullImage ? "" : ", alwaysPullImage=" + alwaysPullImage) +
(workingDir == null ? "" : ", workingDir='" + workingDir + '\'') +
(command == null ? "" : ", command='" + command + '\'') +
(args == null ? "" : ", args='" + args + '\'') +
(!ttyEnabled ? "" : ", ttyEnabled=" + ttyEnabled) +
(resourceRequestCpu == null ? "" : ", resourceRequestCpu='" + resourceRequestCpu + '\'') +
(resourceRequestMemory == null ? "" : ", resourceRequestMemory='" + resourceRequestMemory + '\'') +
(resourceLimitCpu == null ? "" : ", resourceLimitCpu='" + resourceLimitCpu + '\'') +
(resourceLimitMemory == null ? "" : ", resourceLimitMemory='" + resourceLimitMemory + '\'') +
(envVars == null || envVars.isEmpty() ? "" : ", envVars=" + envVars) +
(ports == null || ports.isEmpty() ? "" : ", ports=" + ports) +
(livenessProbe == null ? "" : ", livenessProbe=" + livenessProbe) +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@

import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.csanchez.jenkins.plugins.kubernetes.pipeline.PodTemplateMap;

import hudson.Extension;
import hudson.Util;
import hudson.model.Computer;
Expand Down Expand Up @@ -85,7 +87,8 @@ public class KubernetesCloud extends Cloud {

private String defaultsProviderTemplate;

private List<PodTemplate> templates = new ArrayList<PodTemplate>();
@Nonnull
private List<PodTemplate> templates = new ArrayList<>();
private String serverUrl;
@CheckForNull
private String serverCertificate;
Expand Down Expand Up @@ -170,13 +173,25 @@ public void setDefaultsProviderTemplate(String defaultsProviderTemplate) {
this.defaultsProviderTemplate = defaultsProviderTemplate;
}

@Nonnull
public List<PodTemplate> getTemplates() {
return templates;
}

/**
* Returns all pod templates for this cloud including the dynamic ones.
* @return all pod templates for this cloud including the dynamic ones.
*/
@Nonnull
public List<PodTemplate> getAllTemplates() {
List<PodTemplate> podTemplates = new ArrayList<>(PodTemplateMap.get().getTemplates(this));
podTemplates.addAll(templates);
return Collections.unmodifiableList(podTemplates);
}

@DataBoundSetter
public void setTemplates(@Nonnull List<PodTemplate> templates) {
this.templates = templates;
this.templates = new ArrayList<>(templates);
}

public String getServerUrl() {
Expand Down Expand Up @@ -465,7 +480,7 @@ public boolean canProvision(@CheckForNull Label label) {
* @return the template
*/
public PodTemplate getTemplate(@CheckForNull Label label) {
return PodTemplateUtils.getTemplateByLabel(label, templates);
return PodTemplateUtils.getTemplateByLabel(label, getAllTemplates());
}

/**
Expand All @@ -475,7 +490,8 @@ public PodTemplate getTemplate(@CheckForNull Label label) {
*/
public ArrayList<PodTemplate> getMatchingTemplates(@CheckForNull Label label) {
ArrayList<PodTemplate> podList = new ArrayList<PodTemplate>();
for (PodTemplate t : templates) {
List<PodTemplate> podTemplates = getAllTemplates();
for (PodTemplate t : podTemplates) {
if ((label == null && t.getNodeUsageMode() == Node.Mode.NORMAL) || (label != null && label.matches(t.getLabelSet()))) {
podList.add(t);
}
Expand All @@ -501,6 +517,22 @@ public void removeTemplate(PodTemplate t) {
this.templates.remove(t);
}

/**
* Add a dynamic pod template. Won't be displayed in UI, and persisted separately from the cloud instance.
* @param t the template to add
*/
public void addDynamicTemplate(PodTemplate t) {
PodTemplateMap.get().addTemplate(this, t);
}

/**
* Remove a dynamic pod template.
* @param t the template to remove
*/
public void removeDynamicTemplate(PodTemplate t) {
PodTemplateMap.get().removeTemplate(this, t);
}

@Extension
public static class DescriptorImpl extends Descriptor<Cloud> {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -601,4 +601,39 @@ public List<? extends Descriptor> getEnvVarsDescriptors() {
return DescriptorVisibilityFilter.apply(null, Jenkins.getInstance().getDescriptorList(TemplateEnvVar.class));
}
}

@Override
public String toString() {
return "PodTemplate{" +
(inheritFrom == null ? "" : "inheritFrom='" + inheritFrom + '\'') +
(name == null ? "" : ", name='" + name + '\'') +
(namespace == null ? "" : ", namespace='" + namespace + '\'') +
(image == null ? "" : ", image='" + image + '\'') +
(!privileged ? "" : ", privileged=" + privileged) +
(!alwaysPullImage ? "" : ", alwaysPullImage=" + alwaysPullImage) +
(command == null ? "" : ", command='" + command + '\'') +
(args == null ? "" : ", args='" + args + '\'') +
(remoteFs == null ? "" : ", remoteFs='" + remoteFs + '\'') +
(instanceCap == Integer.MAX_VALUE ? "" : ", instanceCap=" + instanceCap) +
(slaveConnectTimeout == DEFAULT_SLAVE_JENKINS_CONNECTION_TIMEOUT ? "" : ", slaveConnectTimeout=" + slaveConnectTimeout) +
(idleMinutes == 0 ? "" : ", idleMinutes=" + idleMinutes) +
(activeDeadlineSeconds == 0 ? "" : ", activeDeadlineSeconds=" + activeDeadlineSeconds) +
(label == null ? "" : ", label='" + label + '\'') +
(serviceAccount == null ? "" : ", serviceAccount='" + serviceAccount + '\'') +
(nodeSelector == null ? "" : ", nodeSelector='" + nodeSelector + '\'') +
(nodeUsageMode == null ? "" : ", nodeUsageMode=" + nodeUsageMode) +
(resourceRequestCpu == null ? "" : ", resourceRequestCpu='" + resourceRequestCpu + '\'') +
(resourceRequestMemory == null ? "" : ", resourceRequestMemory='" + resourceRequestMemory + '\'') +
(resourceLimitCpu == null ? "" : ", resourceLimitCpu='" + resourceLimitCpu + '\'') +
(resourceLimitMemory == null ? "" : ", resourceLimitMemory='" + resourceLimitMemory + '\'') +
(!customWorkspaceVolumeEnabled ? "" : ", customWorkspaceVolumeEnabled=" + customWorkspaceVolumeEnabled) +
(workspaceVolume == null ? "" : ", workspaceVolume=" + workspaceVolume) +
(volumes == null || volumes.isEmpty() ? "" : ", volumes=" + volumes) +
(containers == null || containers.isEmpty() ? "" : ", containers=" + containers) +
(envVars == null || envVars.isEmpty() ? "" : ", envVars=" + envVars) +
(annotations == null || annotations.isEmpty() ? "" : ", annotations=" + annotations) +
(imagePullSecrets == null || imagePullSecrets.isEmpty() ? "" : ", imagePullSecrets=" + imagePullSecrets) +
(nodeProperties == null || nodeProperties.isEmpty() ? "" : ", nodeProperties=" + nodeProperties) +
'}';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
package org.csanchez.jenkins.plugins.kubernetes.pipeline;

import static java.util.stream.Collectors.toSet;

import java.io.File;
import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.logging.Level;
import java.util.logging.Logger;

import javax.annotation.Nonnull;

import org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud;
import org.csanchez.jenkins.plugins.kubernetes.PodTemplate;

import hudson.BulkChange;
import hudson.Extension;
import hudson.ExtensionList;
import hudson.XmlFile;
import hudson.model.Saveable;
import hudson.model.listeners.SaveableListener;
import hudson.util.CopyOnWriteMap;
import jenkins.model.Jenkins;

/**
* A persisted map of {@link KubernetesCloud} -&gt; List of {@link PodTemplate} instances. Used to persist {@link PodTemplate} instances created by {@link PodTemplateStepExecution}.
*/
@Extension
public class PodTemplateMap implements Saveable {
private static final Logger LOGGER = Logger.getLogger(PodTemplateMap.class.getName());

public static PodTemplateMap get() {
// TODO Replace with lookupSingleton post 2.87
return ExtensionList.lookup(PodTemplateMap.class).get(PodTemplateMap.class);
}

/**
* List of Pod Templates indexed by cloud name
*/
private Map<String, List<PodTemplate>> map;

public PodTemplateMap() {
load();
if (map == null) {
map = new CopyOnWriteMap.Hash<>();
}
check();
}

private void check() {
List<KubernetesCloud> clouds = Jenkins.getInstance().clouds.getAll(KubernetesCloud.class);
Set<String> names = clouds.stream().map(cloud -> cloud.name).collect(toSet());
Set<String> cloudNames = new HashSet<>(map.keySet());

// Remove entries for non-existent clouds
for (String cloudName : cloudNames) {
if (!names.contains(cloudName)) {
map.remove(cloudName);
}
}
}

/**
* Returns a read-only view of the templates available for the corresponding cloud instance.
* @param cloud The kubernetes cloud instance for which templates are needed
* @return a read-only view of the templates available for the corresponding cloud instance.
*/
@Nonnull
public List<PodTemplate> getTemplates(@Nonnull KubernetesCloud cloud) {
return Collections.unmodifiableList(getOrCreateTemplateList(cloud));
}

private List<PodTemplate> getOrCreateTemplateList(@Nonnull KubernetesCloud cloud) {
List<PodTemplate> podTemplates = map.get(cloud.name);
return podTemplates == null ? new CopyOnWriteArrayList<>() : podTemplates;
}

/**
* Adds a template for the corresponding cloud instance.
* @param cloud The cloud instance.
* @param podTemplate The pod template to add.
*/
public void addTemplate(@Nonnull KubernetesCloud cloud, @Nonnull PodTemplate podTemplate) {
List<PodTemplate> list = getOrCreateTemplateList(cloud);
list.add(podTemplate);
map.put(cloud.name, list);
save();
}

public void removeTemplate(@Nonnull KubernetesCloud cloud, @Nonnull PodTemplate podTemplate) {
List<PodTemplate> list = map.get(cloud.name);
if (list != null) {
if (list.remove(podTemplate)) {
save();
}
}
}

/**
* Saves the configuration info to the disk.
*/
public synchronized void save() {
if(BulkChange.contains(this)) return;
try {
getConfigFile().write(this);
SaveableListener.fireOnChange(this, getConfigFile());
} catch (IOException e) {
LOGGER.log(Level.WARNING, "Failed to save "+getConfigFile(),e);
}
}

/**
* Loads the data from the disk into this object.
*
* <p>
* The constructor of the derived class must call this method.
* (If we do that in the base class, the derived class won't
* get a chance to set default values.)
*/
public synchronized void load() {
XmlFile file = getConfigFile();
if(!file.exists())
return;

try {
file.unmarshal(this);
} catch (IOException e) {
LOGGER.log(Level.WARNING, "Failed to load "+file, e);
}
}

protected XmlFile getConfigFile() {
return new XmlFile(new File(Jenkins.getInstance().getRootDir(),"kubernetes-podtemplates.xml"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public boolean start() throws Exception {
newTemplate.setActiveDeadlineSeconds(step.getActiveDeadlineSeconds());
}

kubernetesCloud.addTemplate(newTemplate);
kubernetesCloud.addDynamicTemplate(newTemplate);
getContext().newBodyInvoker().withContext(step).withCallback(new PodTemplateCallback(newTemplate)).start();

PodTemplateAction.push(run, name);
Expand Down Expand Up @@ -141,7 +141,7 @@ protected void finished(StepContext context) throws Exception {
LOGGER.log(Level.INFO, "Removing pod template and deleting pod {1} from cloud {0}",
new Object[] { cloud.name, podTemplate.getName() });
KubernetesCloud kubernetesCloud = (KubernetesCloud) cloud;
kubernetesCloud.removeTemplate(podTemplate);
kubernetesCloud.removeDynamicTemplate(podTemplate);
KubernetesClient client = kubernetesCloud.connect();
Boolean deleted = client.pods().withName(podTemplate.getName()).delete();
if (!Boolean.TRUE.equals(deleted)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud;
import org.csanchez.jenkins.plugins.kubernetes.PodTemplate;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
Expand Down Expand Up @@ -62,10 +61,10 @@ public void runInPod() throws Exception {
p.setDefinition(new CpsFlowDefinition(loadPipelineScript("runInPod.groovy"), true));
WorkflowRun b = p.scheduleBuild2(0).waitForStart();
assertNotNull(b);
List<PodTemplate> templates = cloud.getTemplates();
while (templates.isEmpty()) {
List<PodTemplate> templates = cloud.getAllTemplates();
while (hasPodTemplateWithLabel("mypod",templates)) {
LOGGER.log(Level.INFO, "Waiting for template to be created");
templates = cloud.getTemplates();
templates = cloud.getAllTemplates();
Thread.sleep(1000);
}
assertFalse(templates.isEmpty());
Expand All @@ -77,6 +76,13 @@ public void runInPod() throws Exception {
deletePods(cloud.connect(), getLabels(this), true));
}

private boolean hasPodTemplateWithLabel(String label, List<PodTemplate> templates) {
return templates != null
&& templates.stream()
.map(PodTemplate::getLabel)
.anyMatch(label::equals);
}

@Test
public void runInPodWithMultipleContainers() throws Exception {
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
Expand Down Expand Up @@ -258,10 +264,10 @@ public void runWithActiveDeadlineSeconds() throws Exception {

r.waitForMessage("podTemplate", b);

PodTemplate deadlineTemplate = cloud.getTemplates().stream().filter(x -> x.getLabel() == "deadline").findAny().get();
PodTemplate deadlineTemplate = cloud.getAllTemplates().stream().filter(x -> x.getLabel() == "deadline").findAny().orElse(null);

assertEquals(10, deadlineTemplate.getActiveDeadlineSeconds());
assertNotNull(deadlineTemplate);
assertEquals(10, deadlineTemplate.getActiveDeadlineSeconds());
r.assertLogNotContains("Hello from container!", b);
}

Expand Down

0 comments on commit 42ccc75

Please sign in to comment.