From c120f9dcad0ecee01e51736a5beb363538de0289 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Mon, 22 Oct 2018 11:00:11 +0200 Subject: [PATCH 1/4] [JENKINS-53297] Use data we have from KubernetesSlave instead of introspecting --- .../pipeline/KubernetesNodeContext.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java index ea91efc36e..7ba183fa18 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java @@ -16,43 +16,41 @@ package org.csanchez.jenkins.plugins.kubernetes.pipeline; -import hudson.AbortException; -import hudson.FilePath; -import hudson.model.Node; -import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.KubernetesClient; -import org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud; import org.csanchez.jenkins.plugins.kubernetes.KubernetesSlave; +import org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils; import org.jenkinsci.plugins.workflow.steps.StepContext; +import hudson.AbortException; +import hudson.model.Node; + /** * helper class for steps running in a kubernetes `node` context */ class KubernetesNodeContext { - private static final transient String HOSTNAME_FILE = "/etc/hostname"; private StepContext context; - private FilePath workspace; KubernetesNodeContext(StepContext context) throws Exception { this.context = context; - workspace = context.get(FilePath.class); } String getPodName() throws Exception { - return workspace.child(HOSTNAME_FILE).readToString().trim(); + return PodTemplateUtils.substituteEnv(getKubernetesSlave().getNodeName()); } public String getNamespace() throws Exception { - return workspace.child(Config.KUBERNETES_NAMESPACE_PATH).readToString().trim(); + return getKubernetesSlave().getNamespace(); } KubernetesClient connectToCloud() throws Exception { + return getKubernetesSlave().getKubernetesCloud().connect(); + } + + private KubernetesSlave getKubernetesSlave() throws java.io.IOException, InterruptedException { Node node = context.get(Node.class); if (! (node instanceof KubernetesSlave)) { throw new AbortException(String.format("Node is not a Kubernetes node: %s", node != null ? node.getNodeName() : null)); } - KubernetesSlave slave = (KubernetesSlave) node; - KubernetesCloud cloud = slave.getKubernetesCloud(); - return cloud.connect(); + return (KubernetesSlave) node; } } From 8b7b627179d0ccc8e89d8a9bb6e9087eda6a4a89 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Mon, 22 Oct 2018 17:02:05 +0200 Subject: [PATCH 2/4] Namespace should default to client namespace --- .../plugins/kubernetes/pipeline/KubernetesNodeContext.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java index 7ba183fa18..1f8ce9acdb 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java @@ -39,7 +39,8 @@ String getPodName() throws Exception { } public String getNamespace() throws Exception { - return getKubernetesSlave().getNamespace(); + String namespace = getKubernetesSlave().getNamespace(); + return namespace != null ? namespace : connectToCloud().getNamespace(); } KubernetesClient connectToCloud() throws Exception { From 0657fd10beb2bafc040a70a72d4a6aa8f717737c Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Tue, 23 Oct 2018 10:49:11 +0200 Subject: [PATCH 3/4] Clean up --- .../plugins/kubernetes/KubernetesSlave.java | 25 +++++++++++++++---- .../kubernetes/PodTemplateBuilder.java | 2 +- .../pipeline/KubernetesNodeContext.java | 6 ++--- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java index 47bf009376..ba93788f09 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java @@ -129,10 +129,18 @@ protected KubernetesSlave(String name, PodTemplate template, String nodeDescript template.getNodeProperties()); this.cloudName = cloudName; - this.namespace = Util.fixEmpty(template.getNamespace()); + this.namespace = determineNamespace(getKubernetesCloud(cloudName), Util.fixEmpty(template.getNamespace())); this.template = template; } + private static String determineNamespace(KubernetesCloud cloud, String namespace) throws IOException { + try { + return namespace == null ? cloud.connect().getNamespace() : namespace; + } catch (UnrecoverableKeyException|NoSuchAlgorithmException|KeyStoreException|CertificateEncodingException e) { + throw new IOException(e); + } + } + public String getCloudName() { return cloudName; } @@ -141,6 +149,10 @@ public String getNamespace() { return namespace; } + public String getPodName() { + return PodTemplateUtils.substituteEnv(getNodeName()); + } + /** * @deprecated Please use the strongly typed getKubernetesCloud() instead. @@ -157,11 +169,15 @@ public Cloud getCloud() { */ @Nonnull public KubernetesCloud getKubernetesCloud() { - Cloud cloud = Jenkins.getInstance().getCloud(getCloudName()); + return getKubernetesCloud(getCloudName()); + } + + private static KubernetesCloud getKubernetesCloud(String cloudName) { + Cloud cloud = Jenkins.get().getCloud(cloudName); if (cloud instanceof KubernetesCloud) { return (KubernetesCloud) cloud; } else { - throw new IllegalStateException(getClass().getName() + " can be launched only by instances of " + KubernetesCloud.class.getName()); + throw new IllegalStateException(KubernetesSlave.class.getName() + " can be launched only by instances of " + KubernetesCloud.class.getName()); } } @@ -229,8 +245,7 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted // Prior to termination, determine if we should delete the slave pod based on // the slave pod's current state and the pod retention policy. // Healthy slave pods should still have a JNLP agent running at this point. - String actualNamespace = getNamespace() == null ? client.getNamespace() : getNamespace(); - Pod pod = client.pods().inNamespace(actualNamespace).withName(name).get(); + Pod pod = client.pods().inNamespace(getNamespace()).withName(name).get(); boolean deletePod = getPodRetention(cloud).shouldDeletePod(cloud, pod); Computer computer = toComputer(); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java index 957957865d..f1f06509e4 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java @@ -153,7 +153,7 @@ public Pod build() { MetadataNested metadataBuilder = new PodBuilder().withNewMetadata(); if (slave != null) { - metadataBuilder.withName(substituteEnv(slave.getNodeName())); + metadataBuilder.withName(slave.getPodName()); } Map labels = new HashMap<>(); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java index 1f8ce9acdb..0afd95a9e8 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesNodeContext.java @@ -18,7 +18,6 @@ import io.fabric8.kubernetes.client.KubernetesClient; import org.csanchez.jenkins.plugins.kubernetes.KubernetesSlave; -import org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils; import org.jenkinsci.plugins.workflow.steps.StepContext; import hudson.AbortException; @@ -35,12 +34,11 @@ class KubernetesNodeContext { } String getPodName() throws Exception { - return PodTemplateUtils.substituteEnv(getKubernetesSlave().getNodeName()); + return getKubernetesSlave().getPodName(); } public String getNamespace() throws Exception { - String namespace = getKubernetesSlave().getNamespace(); - return namespace != null ? namespace : connectToCloud().getNamespace(); + return getKubernetesSlave().getNamespace(); } KubernetesClient connectToCloud() throws Exception { From 749ff066c227718258f4f07b0a9f9fa3d89a8978 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Tue, 23 Oct 2018 11:06:21 +0200 Subject: [PATCH 4/4] Fix wrong test --- .../jenkins/plugins/kubernetes/KubernetesSlaveTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlaveTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlaveTest.java index 5087db963c..b76d8d7e25 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlaveTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlaveTest.java @@ -89,8 +89,9 @@ public void testGetPodRetention() { createPodRetentionTestCase(new Always(), new OnFailure(), new OnFailure()), createPodRetentionTestCase(new Always(), new Never(), new Never()) ); + KubernetesCloud cloud = new KubernetesCloud("test"); + r.jenkins.clouds.add(cloud); for (KubernetesSlaveTestCase testCase : cases) { - KubernetesCloud cloud = new KubernetesCloud("test"); cloud.setPodRetention(testCase.getCloudPodRetention()); KubernetesSlave testSlave = testCase.buildSubject(cloud); assertEquals(testCase.getExpectedResult(), testSlave.getPodRetention(cloud));