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

[JENKINS-50533] Merge default jnlp container options #305

Merged
merged 1 commit into from
Apr 10, 2018
Merged
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
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
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);
}

}