Merge pull request #340 from juhtie01/JENKINS-50932

[JENKINS-50932] Combine parent pod template ports with children
jenkinsci · Oct 31, 2018 · 4f4c69b · 4f4c69b
2 parents b128989 + f619fe5
commit 4f4c69b
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 10 deletions.
diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java
@@ -94,6 +94,9 @@ public static ContainerTemplate combine(@CheckForNull ContainerTemplate parent,
         String resourceRequestMemory = Strings.isNullOrEmpty(template.getResourceRequestMemory()) ? parent.getResourceRequestMemory() : template.getResourceRequestMemory();
         String resourceLimitCpu = Strings.isNullOrEmpty(template.getResourceLimitCpu()) ? parent.getResourceLimitCpu() : template.getResourceLimitCpu();
         String resourceLimitMemory = Strings.isNullOrEmpty(template.getResourceLimitMemory()) ? parent.getResourceLimitMemory() : template.getResourceLimitMemory();
+        Map<String, PortMapping> ports = parent.getPorts().stream()
+                .collect(Collectors.toMap(PortMapping::getName, Function.identity()));
+        template.getPorts().stream().forEach(p -> ports.put(p.getName(), p));
 
         ContainerTemplate combined = new ContainerTemplate(image);
         combined.setName(name);
@@ -109,6 +112,7 @@ public static ContainerTemplate combine(@CheckForNull ContainerTemplate parent,
         combined.setWorkingDir(workingDir);
         combined.setPrivileged(privileged);
         combined.setEnvVars(combineEnvVars(parent, template));
+        combined.setPorts(new ArrayList<>(ports.values()));
         return combined;
     }
 
@@ -142,7 +146,7 @@ public static Container combine(@CheckForNull Container parent, @Nonnull Contain
         Boolean tty = template.getTty() != null ? template.getTty() : parent.getTty();
         Map<String, Quantity> requests = combineResources(parent, template, ResourceRequirements::getRequests);
         Map<String, Quantity> limits = combineResources(parent, template, ResourceRequirements::getLimits);
-        
+
         Map<String, VolumeMount> volumeMounts = parent.getVolumeMounts().stream()
                 .collect(Collectors.toMap(VolumeMount::getMountPath, Function.identity()));
         template.getVolumeMounts().stream().forEach(vm -> volumeMounts.put(vm.getMountPath(), vm));

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PortMapping.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PortMapping.java
@@ -67,7 +67,7 @@ public ContainerPort toPort() {
     }
 
     public String toString() {
-        return String.format("%s,%d", name, containerPort);
+        return String.format("%s,%d,%d", name, containerPort, hostPort);
     }
 
     @Override

diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java
@@ -36,20 +36,15 @@
 import java.util.List;
 import java.util.Map;
 
-import hudson.model.Node;
-import hudson.slaves.NodeProperty;
-import hudson.tools.ToolLocationNodeProperty;
-
-import org.apache.commons.compress.utils.IOUtils;
 import org.csanchez.jenkins.plugins.kubernetes.model.KeyValueEnvVar;
 import org.csanchez.jenkins.plugins.kubernetes.model.SecretEnvVar;
 import org.csanchez.jenkins.plugins.kubernetes.volumes.HostPathVolume;
-import org.jenkinsci.plugins.pipeline.modeldefinition.shaded.com.google.common.collect.Lists;
 import org.junit.Test;
 
 import com.google.common.collect.ImmutableMap;
 
 import hudson.model.Node;
+import hudson.tools.ToolLocationNodeProperty;
 import io.fabric8.kubernetes.api.model.ConfigMapEnvSource;
 import io.fabric8.kubernetes.api.model.Container;
 import io.fabric8.kubernetes.api.model.ContainerBuilder;
@@ -60,7 +55,6 @@
 import io.fabric8.kubernetes.api.model.PodFluent.SpecNested;
 import io.fabric8.kubernetes.api.model.PodSpec;
 import io.fabric8.kubernetes.api.model.Quantity;
-import io.fabric8.kubernetes.api.model.QuantityBuilder;
 import io.fabric8.kubernetes.api.model.ResourceRequirementsBuilder;
 import io.fabric8.kubernetes.api.model.SecretEnvSource;
 import io.fabric8.kubernetes.api.model.Toleration;
@@ -519,6 +513,23 @@ public void shouldCombineAllTolerations() {
     }
 
     @Test
+    public void shouldCombineAllPorts() {
+        ContainerTemplate template1 = new ContainerTemplate("name-1", "image-1");
+        PortMapping port1 = new PortMapping("port-1", 1000, 1000);
+        template1.setPorts(Arrays.asList(port1));
+
+        ContainerTemplate template2 = new ContainerTemplate("name-2", "image-2");
+
+        assertThat(combine(template1, template2).getPorts(), contains(port1));
+
+        PortMapping port2 = new PortMapping("port-2", 2000, 2000);
+        template2.setPorts(Arrays.asList(port2));
+        assertThat(combine(template1, template2).getPorts(), containsInAnyOrder(port1, port2));
+
+        port2.setName("port-1");
+        assertThat(combine(template1, template2).getPorts(), contains(port2));
+    }
+
     public void shouldCombineAllResources() {
         Container container1 = new Container();
         container1.setResources(new ResourceRequirementsBuilder() //

diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java
@@ -427,5 +427,4 @@ public void runInPodWithRetention() throws Exception {
         r.assertBuildStatusSuccess(r.waitForCompletion(b));
         assertTrue(deletePods(cloud.connect(), getLabels(this), true));
     }
-
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -427,5 +427,4 @@ public void runInPodWithRetention() throws Exception { @@
             r.assertBuildStatusSuccess(r.waitForCompletion(b));
             assertTrue(deletePods(cloud.connect(), getLabels(this), true));
         }
     }