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-58405] Backward compatibility for yaml field merge #538

Merged
merged 5 commits into from
Jul 10, 2019
Merged
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -141,6 +141,7 @@ Either way it provides access to the following fields:
* **namespace** The namespace of the pod.
* **label** The label of the pod. Set a unique value to avoid conflicts across builds
* **yaml** [yaml representation of the Pod](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.10/#pod-v1-core), to allow setting any values not supported as fields
* **yamlMergeStrategy** `merge()` or `override()`. Controls whether the yaml definition overrides or is merged with the yaml definition inherited from pod templates declared with `inheritFrom`. Defaults to `override()`.
* **containers** The container templates that are use to create the containers of the pod *(see below)*.
* **serviceAccount** The service account of the pod.
* **nodeSelector** The node selector of the pod.
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@
import org.apache.commons.lang.StringUtils;
import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar;
import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention;
import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.YamlMergeStrategy;
import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume;
import org.csanchez.jenkins.plugins.kubernetes.volumes.workspace.WorkspaceVolume;
import org.kohsuke.accmod.Restricted;
@@ -129,6 +130,21 @@ public class PodTemplate extends AbstractDescribableImpl<PodTemplate> implements

private List<String> yamls = new ArrayList<>();

public YamlMergeStrategy getYamlMergeStrategy() {
return yamlMergeStrategy;
}

@DataBoundSetter
public void setYamlMergeStrategy(YamlMergeStrategy yamlMergeStrategy) {
this.yamlMergeStrategy = yamlMergeStrategy;
}

private YamlMergeStrategy yamlMergeStrategy = YamlMergeStrategy.defaultStrategy();

public Pod getYamlsPod() {
return yamlMergeStrategy.merge(getYamls());
}

private Boolean showRawYaml;

@CheckForNull
@@ -718,6 +734,10 @@ protected Object readResolve() {
showRawYaml = Boolean.TRUE;
}

if (yamlMergeStrategy == null) {
yamlMergeStrategy = YamlMergeStrategy.defaultStrategy();
}

return this;
}

@@ -801,7 +821,8 @@ public List<? extends Descriptor> getEnvVarsDescriptors() {
return DescriptorVisibilityFilter.apply(null, Jenkins.getInstance().getDescriptorList(TemplateEnvVar.class));
}

@SuppressWarnings("rawtypes")
@SuppressWarnings("unused") // Used by jelly
@Restricted(DoNotUse.class) // Used by jelly
public Descriptor getDefaultPodRetention() {
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins == null) {
@@ -810,6 +831,11 @@ public Descriptor getDefaultPodRetention() {
return jenkins.getDescriptor(PodRetention.getPodTemplateDefault().getClass());
}

@SuppressWarnings("unused") // Used by jelly
@Restricted(DoNotUse.class) // Used by jelly
public YamlMergeStrategy getDefaultYamlMergeStrategy() {
return YamlMergeStrategy.defaultStrategy();
}
}

@Override
Original file line number Diff line number Diff line change
@@ -198,8 +198,7 @@ public Pod build() {
builder.withContainers(containers.values().toArray(new Container[containers.size()]));

// merge with the yaml fragments
Pod yamlPods = combine(template.getYamls().stream().map(PodTemplateUtils::parseFromYaml).collect(Collectors.toList()));
Pod pod = combine(yamlPods, builder.endSpec().build());
Pod pod = combine(template.getYamlsPod(), builder.endSpec().build());

// Apply defaults

Original file line number Diff line number Diff line change
@@ -351,6 +351,7 @@ public static PodTemplate combine(PodTemplate parent, PodTemplate template) {
podTemplate.setAnnotations(new ArrayList<>(podAnnotations));
podTemplate.setNodeProperties(toolLocationNodeProperties);
podTemplate.setNodeUsageMode(nodeUsageMode);
podTemplate.setYamlMergeStrategy(template.getYamlMergeStrategy());
podTemplate.setInheritFrom(!Strings.isNullOrEmpty(template.getInheritFrom()) ?
template.getInheritFrom() : parent.getInheritFrom());

Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@
import org.csanchez.jenkins.plugins.kubernetes.PodTemplate;
import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar;
import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention;
import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.YamlMergeStrategy;
import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume;
import org.csanchez.jenkins.plugins.kubernetes.volumes.workspace.WorkspaceVolume;
import org.jenkinsci.plugins.workflow.steps.Step;
@@ -58,6 +59,7 @@ public class PodTemplateStep extends Step implements Serializable {
private String workingDir = ContainerTemplate.DEFAULT_WORKING_DIR;

private String yaml;
private YamlMergeStrategy yamlMergeStrategy = YamlMergeStrategy.defaultStrategy();
private PodRetention podRetention;

@DataBoundConstructor
@@ -122,6 +124,15 @@ public void setEnvVars(List<TemplateEnvVar> envVars) {
}
}

public YamlMergeStrategy getYamlMergeStrategy() {
return yamlMergeStrategy;
}

@DataBoundSetter
public void setYamlMergeStrategy(YamlMergeStrategy yamlMergeStrategy) {
this.yamlMergeStrategy = yamlMergeStrategy;
}

public List<PodVolume> getVolumes() {
return volumes;
}
Original file line number Diff line number Diff line change
@@ -101,6 +101,7 @@ public boolean start() throws Exception {
newTemplate.setNodeUsageMode(step.getNodeUsageMode());
newTemplate.setServiceAccount(step.getServiceAccount());
newTemplate.setAnnotations(step.getAnnotations());
newTemplate.setYamlMergeStrategy(step.getYamlMergeStrategy());
if(run!=null) {
newTemplate.getAnnotations().add(new PodAnnotation("buildUrl", ((KubernetesCloud)cloud).getJenkinsUrlOrDie()+run.getUrl()));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package org.csanchez.jenkins.plugins.kubernetes.pod.yaml;

import hudson.Extension;
import hudson.model.Descriptor;
import io.fabric8.kubernetes.api.model.Pod;
import org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.annotation.Nonnull;
import java.util.List;
import java.util.stream.Collectors;

import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.combine;

public class Merge extends YamlMergeStrategy {
private static final long serialVersionUID = 6562610892063268131L;

@DataBoundConstructor
public Merge() {
}

@Override
public Pod merge(List<String> yamls) {
return combine(yamls.stream().map(PodTemplateUtils::parseFromYaml).collect(Collectors.toList()));
}

@Extension
@Symbol("merge")
public static class DescriptorImpl extends Descriptor<YamlMergeStrategy> {
@Nonnull
@Override
public String getDisplayName() {
return "Merge";
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package org.csanchez.jenkins.plugins.kubernetes.pod.yaml;

import hudson.Extension;
import hudson.model.Descriptor;
import io.fabric8.kubernetes.api.model.Pod;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.annotation.Nonnull;
import java.util.List;

import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.parseFromYaml;

public class Overrides extends YamlMergeStrategy {
private static final long serialVersionUID = 4510341864619338164L;

@DataBoundConstructor
public Overrides() {
}

@Override
public Pod merge(List<String> yamls) {
if (yamls.isEmpty()) {
return null;
} else {
return parseFromYaml(yamls.get(yamls.size()-1));
}
}

@Extension
@Symbol("override")
public static class DescriptorImpl extends Descriptor<YamlMergeStrategy> {
@Nonnull
@Override
public String getDisplayName() {
return "Override";
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package org.csanchez.jenkins.plugins.kubernetes.pod.yaml;

import hudson.ExtensionPoint;
import hudson.model.AbstractDescribableImpl;
import io.fabric8.kubernetes.api.model.Pod;

import javax.annotation.Nonnull;
import java.io.Serializable;
import java.util.List;

public abstract class YamlMergeStrategy extends AbstractDescribableImpl<YamlMergeStrategy> implements ExtensionPoint, Serializable {
jglick marked this conversation as resolved.
Show resolved Hide resolved
public static YamlMergeStrategy defaultStrategy() {
return new Overrides();
}

public abstract Pod merge(@Nonnull List<String> yamls);
}
Original file line number Diff line number Diff line change
@@ -71,6 +71,8 @@
<f:textarea/>
</f:entry>

<f:dropdownDescriptorSelector title="${%Yaml merge strategy}" field="yamlMergeStrategy" default="${descriptor.defaultYamlMergeStrategy}" />

<f:entry field="showRawYaml" title="${%Show raw yaml in console}" >
<f:checkbox default="true"/>
</f:entry>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Defines how the raw yaml field gets merged with yaml definitions from inherited pod templates.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout"
xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<f:description>The yaml definitions from inherited pod templates are merged with the current template yaml fragment.</f:description>
</j:jelly>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout"
xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<f:description>The yaml definitions from inherited pod templates are completely overridden by the current yaml field.</f:description>
</j:jelly>
Original file line number Diff line number Diff line change
@@ -20,6 +20,8 @@
import org.apache.commons.compress.utils.IOUtils;
import org.csanchez.jenkins.plugins.kubernetes.model.KeyValueEnvVar;
import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar;
import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.Merge;
import org.csanchez.jenkins.plugins.kubernetes.pod.yaml.YamlMergeStrategy;
import org.csanchez.jenkins.plugins.kubernetes.volumes.EmptyDirVolume;
import org.csanchez.jenkins.plugins.kubernetes.volumes.HostPathVolume;
import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume;
@@ -331,6 +333,7 @@ public void yamlMergeContainers() throws Exception {
" - cat\n" +
" tty: true\n"
);
child.setYamlMergeStrategy(merge());
child.setInheritFrom("parent");
setupStubs();
PodTemplate result = combine(parent, child);
@@ -372,6 +375,7 @@ public void yamlOverrideContainer() throws Exception {
" tty: true\n"
);
child.setInheritFrom("parent");
child.setYamlMergeStrategy(merge());
setupStubs();
PodTemplate result = combine(parent, child);
Pod pod = new PodTemplateBuilder(result).withSlave(slave).build();
@@ -396,6 +400,7 @@ public void yamlOverrideContainerEnvvar() throws Exception {
" - name: VAR2\n" +
" value: \"1\"\n");
PodTemplate child = new PodTemplate();
child.setYamlMergeStrategy(merge());
child.setYaml("kind: Pod\n" +
"spec:\n" +
" containers:\n" +
@@ -439,6 +444,7 @@ public void yamlOverrideVolume() throws Exception {
" path: /host/data2\n"
);
child.setInheritFrom("parent");
child.setYamlMergeStrategy(merge());
setupStubs();
PodTemplate result = combine(parent, child);
Pod pod = new PodTemplateBuilder(result).withSlave(slave).build();
@@ -474,6 +480,7 @@ public void yamlMergeVolumes() throws Exception {
" path: /host/data2\n"
);
child.setInheritFrom("parent");
child.setYamlMergeStrategy(merge());
setupStubs();
PodTemplate result = combine(parent, child);
Pod pod = new PodTemplateBuilder(result).withSlave(slave).build();
@@ -511,4 +518,8 @@ private Map<String, Container> toContainerMap(Pod pod) {
private String loadYamlFile(String s) throws IOException {
return new String(IOUtils.toByteArray(getClass().getResourceAsStream(s)));
}

private YamlMergeStrategy merge() {
return new Merge();
}
}
Original file line number Diff line number Diff line change
@@ -345,4 +345,16 @@ public void runInPodWithRetention() throws Exception {
r.assertBuildStatusSuccess(r.waitForCompletion(b));
assertTrue(deletePods(cloud.connect(), getLabels(this, name), true));
}

@Test
@Issue("JENKINS-58405")
public void overrideYaml() throws Exception {
r.assertBuildStatusSuccess(r.waitForCompletion(b));
}

@Test
@Issue("JENKINS-58405")
public void mergeYaml() throws Exception {
r.assertBuildStatusSuccess(r.waitForCompletion(b));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
podTemplate(label: 'mergeYaml-parent', yaml: """
jglick marked this conversation as resolved.
Show resolved Hide resolved
spec:
containers:
- name: jnlp
env:
- name: VAR1
value: 1
""") {
podTemplate(label: 'mergeYaml-child',
yamlMergeStrategy: merge(),
yaml: """
containers:
- name: jnlp
env:
- name: VAR2
value: 1
""") {
node('mergeYaml-child'){
sh '["$VAR1" == "$VAR2"]'
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
podTemplate(label: 'mergeYaml-parent', yaml: """
spec:
containers:
- name: jnlp
env:
- name: VAR1
value: 1
""") {
podTemplate(label: 'mergeYaml-child',
yaml: """
containers:
- name: jnlp
env:
- name: VAR2
value: 1
""") {
node('mergeYaml-child'){
sh '["$VAR1" != "$VAR2"]'
}
}
}