Skip to content

Commit

Permalink
Merge pull request #305 from jenkinsci/JENKINS-50533
Browse files Browse the repository at this point in the history 
[JENKINS-50533] Merge default jnlp container options
  • Loading branch information
@carlossg
carlossg authored Apr 10, 2018
2 parents 6ae4965 + 679c9c2 commit 097922e
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 18 deletions.
38 changes: 28 additions & 10 deletions src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Function;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.apache.commons.lang.StringUtils;
import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar;
Expand All @@ -51,6 +52,7 @@
import org.kohsuke.accmod.restrictions.NoExternalUse;

import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;

import edu.umd.cs.findbugs.annotations.CheckForNull;
Expand Down Expand Up @@ -90,8 +92,6 @@ public class PodTemplateBuilder {

private static final String WORKSPACE_VOLUME_NAME = "workspace-volume";

private static final String DEFAULT_JNLP_ARGUMENTS = "${computer.jnlpmac} ${computer.name}";

private static final String DEFAULT_JNLP_IMAGE = System
.getProperty(PodTemplateStepExecution.class.getName() + ".defaultImage", "jenkins/jnlp-slave:alpine");

Expand Down Expand Up @@ -225,12 +225,23 @@ public Pod build() {
}

// default jnlp container
if (pod.getSpec().getContainers().stream().noneMatch(c -> JNLP_NAME.equals(c.getName()))) {
ContainerTemplate containerTemplate = new ContainerTemplate(DEFAULT_JNLP_IMAGE);
containerTemplate.setName(JNLP_NAME);
containerTemplate.setArgs(DEFAULT_JNLP_ARGUMENTS);
pod.getSpec().getContainers().add(createContainer(containerTemplate, template.getEnvVars(), volumeMounts.values()));
Optional<Container> jnlpOpt = pod.getSpec().getContainers().stream().filter(c -> JNLP_NAME.equals(c.getName()))
.findFirst();
Container jnlp = jnlpOpt.orElse(new ContainerBuilder().withName(JNLP_NAME).build());
if (!jnlpOpt.isPresent()) {
pod.getSpec().getContainers().add(jnlp);
}
if (StringUtils.isBlank(jnlp.getImage())) {
jnlp.setImage(DEFAULT_JNLP_IMAGE);
}
if (jnlp.getArgs().isEmpty() && slave != null) {
jnlp.setArgs(ImmutableList.of(slave.getComputer().getJnlpMac(), slave.getComputer().getName()));
}
Map<String, EnvVar> envVars = defaultEnvVars(slave,
jnlp.getWorkingDir() != null ? jnlp.getWorkingDir() : ContainerTemplate.DEFAULT_WORKING_DIR,
template.getEnvVars());
envVars.putAll(jnlp.getEnv().stream().collect(Collectors.toMap(EnvVar::getName, Function.identity())));
jnlp.setEnv(new ArrayList<>(envVars.values()));

// default workspace volume, add an empty volume to share the workspace across the pod
if (pod.getSpec().getVolumes().stream().noneMatch(v -> WORKSPACE_VOLUME_NAME.equals(v.getName()))) {
Expand All @@ -248,7 +259,8 @@ public Pod build() {
return pod;
}

private Container createContainer(ContainerTemplate containerTemplate, Collection<TemplateEnvVar> globalEnvVars, Collection<VolumeMount> volumeMounts) {
private Map<String, EnvVar> defaultEnvVars(KubernetesSlave slave, String workingDir,
Collection<TemplateEnvVar> globalEnvVars) {
// Last-write wins map of environment variable names to values
HashMap<String, String> env = new HashMap<>();

Expand All @@ -270,7 +282,7 @@ private Container createContainer(ContainerTemplate containerTemplate, Collectio
// Running on OpenShift Enterprise, security concerns force use of arbitrary user ID
// As a result, container is running without a home set for user, resulting into using `/` for some tools,
// and `?` for java build tools. So we force HOME to a safe location.
env.put("HOME", containerTemplate.getWorkingDir());
env.put("HOME", workingDir);

Map<String, EnvVar> envVarsMap = new HashMap<>();

Expand All @@ -283,6 +295,12 @@ private Container createContainer(ContainerTemplate containerTemplate, Collectio
envVarsMap.put(item.getKey(), item.buildEnvVar())
);
}
return envVarsMap;
}

private Container createContainer(ContainerTemplate containerTemplate, Collection<TemplateEnvVar> globalEnvVars,
Collection<VolumeMount> volumeMounts) {
Map<String, EnvVar> envVarsMap = defaultEnvVars(slave, containerTemplate.getWorkingDir(), globalEnvVars);

if (containerTemplate.getEnvVars() != null) {
containerTemplate.getEnvVars().forEach(item ->
Expand Down
38 changes: 30 additions & 8 deletions src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package org.csanchez.jenkins.plugins.kubernetes;

import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateBuilder.*;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import static org.mockito.Mockito.*;

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
Expand All @@ -22,14 +22,20 @@

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;

import io.fabric8.kubernetes.api.model.Container;
import io.fabric8.kubernetes.api.model.EnvVar;
import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.api.model.Volume;
import io.fabric8.kubernetes.api.model.VolumeMount;

public class PodTemplateBuilderTest {

private static final String AGENT_NAME = "jenkins-agent";
private static final String AGENT_SECRET = "xxx";
private static final String JENKINS_URL = "http://jenkins.example.com";

@Rule
public MockitoRule mockitoRule = MockitoJUnit.rule();

Expand Down Expand Up @@ -69,6 +75,7 @@ public void testParseLivenessProbe() {

@Test
public void testBuildWithoutSlave() throws Exception {
slave = null;
PodTemplate template = new PodTemplate();
template.setYaml(new String(IOUtils.toByteArray(getClass().getResourceAsStream("pod-busybox.yaml"))));
Pod pod = new PodTemplateBuilder(template).build();
Expand All @@ -85,9 +92,9 @@ public void testBuildFromYaml() throws Exception {
}

private void setupStubs() {
when(cloud.getJenkinsUrlOrDie()).thenReturn("http://jenkins.example.com");
when(computer.getName()).thenReturn("jenkins-agent");
when(computer.getJnlpMac()).thenReturn("xxx");
when(cloud.getJenkinsUrlOrDie()).thenReturn(JENKINS_URL);
when(computer.getName()).thenReturn(AGENT_NAME);
when(computer.getJnlpMac()).thenReturn(AGENT_SECRET);
when(slave.getComputer()).thenReturn(computer);
when(slave.getKubernetesCloud()).thenReturn(cloud);
}
Expand Down Expand Up @@ -116,8 +123,24 @@ private void validatePod(Pod pod) {
assertEquals(new VolumeMount("/container/data", "host-volume", null, null), mounts.get(0));
assertEquals(new VolumeMount("/home/jenkins", "workspace-volume", false, null), mounts.get(1));

mounts = containers.get("jnlp").getVolumeMounts();
assertEquals(1, mounts.size());
validateJnlpContainer(containers.get("jnlp"), slave);
}

private void validateJnlpContainer(Container jnlp, KubernetesSlave slave) {
assertEquals("Wrong number of volume mounts: " + jnlp.getVolumeMounts(), 1, jnlp.getVolumeMounts().size());
assertThat(jnlp.getCommand(), empty());
List<EnvVar> envVars = Lists.newArrayList( //
new EnvVar("HOME", "/home/jenkins", null) //
);
if (slave != null) {
assertThat(jnlp.getArgs(), contains(AGENT_SECRET, AGENT_NAME));
envVars.add(new EnvVar("JENKINS_URL", JENKINS_URL, null));
envVars.add(new EnvVar("JENKINS_SECRET", AGENT_SECRET, null));
envVars.add(new EnvVar("JENKINS_NAME", AGENT_NAME, null));
} else {
assertThat(jnlp.getArgs(), empty());
}
assertThat(jnlp.getEnv(), hasItems(envVars.toArray(new EnvVar[envVars.size()])));
}

@Test
Expand All @@ -130,8 +153,7 @@ public void testOverridesFromYaml() throws Exception {
Map<String, Container> containers = pod.getSpec().getContainers().stream()
.collect(Collectors.toMap(Container::getName, Function.identity()));
assertEquals(1, containers.size());
Container jnlp = containers.get("jnlp");
assertEquals("Wrong number of volume mounts: " + jnlp.getVolumeMounts(), 1, jnlp.getVolumeMounts().size());
validateJnlpContainer(containers.get("jnlp"), slave);
}

}

0 comments on commit 097922e

Please sign in to comment.