Skip to content

Commit

Permalink
JENKINS-60088: move label key to non-unique value; breaking change
Browse files Browse the repository at this point in the history 
making labels require compliant format - no / or other characters
allowed

Change-Id: I39bdfde4a03abaacd0aa87222a190935ab5f45c4
  • Loading branch information
@MattLud
MattLud committed Dec 12, 2019
1 parent c655ef4 commit d990ac6
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ public Map<String, String> getLabelsMap() {
ImmutableMap.Builder<String, String> builder = ImmutableMap.<String, String> builder();
if (!labelSet.isEmpty()) {
for (LabelAtom label : labelSet) {
builder.put(label == null ? DEFAULT_ID : "jenkins/" + label.getName(), "true");
builder.put("jenkins/label",label == null ? DEFAULT_ID : label.getName());
}
}
return builder.build();
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,12 @@ public void runInPod() throws Exception {
}
assertTrue(foundBuildUrl);
assertEquals(Integer.MAX_VALUE, template.getInstanceCap());
assertThat(template.getLabelsMap(), hasEntry("jenkins/" + name.getMethodName(), "true"));
assertThat(template.getLabelsMap(), hasEntry("jenkins/label", name.getMethodName()));

Pod pod = pods.getItems().get(0);
LOGGER.log(Level.INFO, "One pod found: {0}", pod);
assertThat(pod.getMetadata().getLabels(), hasEntry("jenkins", "slave"));
assertThat("Pod labels are wrong: " + pod, pod.getMetadata().getLabels(), hasEntry("jenkins/" + name.getMethodName(), "true"));
assertThat("Pod labels are wrong: " + pod, pod.getMetadata().getLabels(), hasEntry("jenkins/label", name.getMethodName()));

r.assertBuildStatusSuccess(r.waitForCompletion(b));
r.assertLogContains("script file contents: ", b);
Expand All @@ -167,10 +167,10 @@ public void runIn2Pods() throws Exception {
PodTemplate template1 = podTemplatesWithLabel(label1, cloud.getAllTemplates()).get(0);
SemaphoreStep.success("podTemplate1/1", null);
assertEquals(Integer.MAX_VALUE, template1.getInstanceCap());
assertThat(template1.getLabelsMap(), hasEntry("jenkins/" + label1, "true"));
assertThat(template1.getLabelsMap(), hasEntry("jenkins/label", label1));
SemaphoreStep.waitForStart("pod1/1", b);
Map<String, String> labels1 = getLabels(cloud, this, name);
labels1.put("jenkins/"+label1, "true");
labels1.put("jenkins/label",label1);
PodList pods = cloud.connect().pods().withLabels(labels1).list();
assertTrue(!pods.getItems().isEmpty());
SemaphoreStep.success("pod1/1", null);
Expand All @@ -180,11 +180,11 @@ public void runIn2Pods() throws Exception {
PodTemplate template2 = podTemplatesWithLabel(label2, cloud.getAllTemplates()).get(0);
SemaphoreStep.success("podTemplate2/1", null);
assertEquals(Integer.MAX_VALUE, template2.getInstanceCap());
assertThat(template2.getLabelsMap(), hasEntry("jenkins/" + label2, "true"));
assertThat(template2.getLabelsMap(), hasEntry("jenkins/label", label2));
assertNull(label2 + " should not inherit from anything", template2.getInheritFrom());
SemaphoreStep.waitForStart("pod2/1", b);
Map<String, String> labels2 = getLabels(cloud, this, name);
labels1.put("jenkins/" + label2, "true");
labels1.put("jenkins/label", label2);
PodList pods2 = cloud.connect().pods().withLabels(labels2).list();
assertTrue(!pods2.getItems().isEmpty());
SemaphoreStep.success("pod2/1", null);
Expand Down

3 comments on commit d990ac6

@guipal
Copy link

@guipal guipal commented on d990ac6 Dec 18, 2019 •
edited by Vlatombe
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello.

This change is breaking the compatibility with multiple labels on a slave.

java.lang.IllegalArgumentException: duplicate key: jenkins/label
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:115)
	at com.google.common.collect.RegularImmutableMap.<init>(RegularImmutableMap.java:72)
	at com.google.common.collect.ImmutableMap$Builder.fromEntryList(ImmutableMap.java:245)
	at com.google.common.collect.ImmutableMap$Builder.build(ImmutableMap.java:231)
	at org.csanchez.jenkins.plugins.kubernetes.PodTemplate.getLabelsMap(PodTemplate.java:387)
	at org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud.addProvisionedSlave(KubernetesCloud.java:599)
	at org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud.provision(KubernetesCloud.java:542)
	at hudson.slaves.NodeProvisioner$StandardStrategyImpl.apply(NodeProvisioner.java:729)
	at hudson.slaves.NodeProvisioner.update(NodeProvisioner.java:334)
	at hudson.slaves.NodeProvisioner.access$900(NodeProvisioner.java:64)
	at hudson.slaves.NodeProvisioner$NodeProvisionerInvoker.doRun(NodeProvisioner.java:823)
	at hudson.triggers.SafeTimerTask.run(SafeTimerTask.java:70)
	at jenkins.security.ImpersonatingScheduledExecutorService$1.run(ImpersonatingScheduledExecutorService.java:58)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

@MattLud
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide an example pipeline?

@guipal
Copy link

@guipal guipal commented on d990ac6 Dec 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not related with pipelines.

We are definind the podTemplates on the UI. If you define one there with several Labels, then you can execute a simpre pipeline like:

node('label'){
sh 'echo Hello World!'
}

That will trigger the error.

Please sign in to comment.