From ce7951568e7291c5c0c20b8564940ce5b251b219 Mon Sep 17 00:00:00 2001 From: Zhao Xiaojie Date: Sat, 8 Dec 2018 10:47:53 +0800 Subject: [PATCH 1/6] Fixes the declarative agent can't set the namespaces --- .../kubernetes/KubernetesLauncher.java | 4 ++ .../KubernetesDeclarativeAgentTest.java | 14 +++++ .../declarativeWithNamespaceFromYaml.groovy | 52 +++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativeWithNamespaceFromYaml.groovy diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java index 8cc4c5b8a4..1c7436d4d4 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -100,6 +100,10 @@ public void launch(SlaveComputer computer, TaskListener listener) { String podId = pod.getMetadata().getName(); String namespace = StringUtils.defaultIfBlank(slave.getNamespace(), client.getNamespace()); + // JENKINS-51610 the namespace must keep same or will cause an exception + if(pod.getMetadata() != null && StringUtils.isNotBlank(pod.getMetadata().getNamespace())) { + namespace = pod.getMetadata().getNamespace(); + } LOGGER.log(Level.FINE, "Creating Pod: {0} in namespace {1}", new Object[]{podId, namespace}); pod = client.pods().inNamespace(namespace).create(pod); diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgentTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgentTest.java index ff82a963ad..beb939d782 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgentTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgentTest.java @@ -67,6 +67,20 @@ public void declarativeFromYaml() throws Exception { r.assertLogContains("BUSYBOX_CONTAINER_ENV_VAR = busybox\n", b); } + @Issue("JENKINS-51610") + @Test + public void declarativeFromYamlWithNamespace() throws Exception { + WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "job with dir"); + p.setDefinition(new CpsFlowDefinition(loadPipelineScript("declarativeWithNamespaceFromYaml.groovy"), true)); + WorkflowRun b = p.scheduleBuild2(0).waitForStart(); + assertNotNull(b); + r.assertBuildStatusSuccess(r.waitForCompletion(b)); + r.assertLogContains("Apache Maven 3.3.9", b); + r.assertLogContains("OUTSIDE_CONTAINER_ENV_VAR = jnlp\n", b); + r.assertLogContains("MAVEN_CONTAINER_ENV_VAR = maven\n", b); + r.assertLogContains("BUSYBOX_CONTAINER_ENV_VAR = busybox\n", b); + } + @Issue("JENKINS-52259") @Test public void declarativeFromYamlFile() throws Exception { diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativeWithNamespaceFromYaml.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativeWithNamespaceFromYaml.groovy new file mode 100644 index 0000000000..b62a8828e7 --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativeWithNamespaceFromYaml.groovy @@ -0,0 +1,52 @@ +pipeline { + agent { + kubernetes { + label 'declarativefromyaml-pod' + yaml """ +metadata: + namespace: test + labels: + some-label: some-label-value + class: KubernetesDeclarativeAgentTest +spec: + containers: + - name: jnlp + env: + - name: CONTAINER_ENV_VAR + value: jnlp + - name: maven + image: maven:3.3.9-jdk-8-alpine + command: + - cat + tty: true + env: + - name: CONTAINER_ENV_VAR + value: maven + - name: busybox + image: busybox + command: + - cat + tty: true + env: + - name: CONTAINER_ENV_VAR + value: busybox +""" + } + } + stages { + stage('Run maven') { + steps { + sh 'set' + sh "echo OUTSIDE_CONTAINER_ENV_VAR = ${CONTAINER_ENV_VAR}" + container('maven') { + sh 'echo MAVEN_CONTAINER_ENV_VAR = ${CONTAINER_ENV_VAR}' + sh 'mvn -version' + } + container('busybox') { + sh 'echo BUSYBOX_CONTAINER_ENV_VAR = ${CONTAINER_ENV_VAR}' + sh '/bin/busybox' + } + } + } + } +} From 8ad36e1793e7f3387a94142648d5d84c60c9cc6f Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Wed, 19 Dec 2018 16:41:25 +0100 Subject: [PATCH 2/6] Use kubernetes-plugin-test-overridden-namespace for test --- .../kubernetes/pipeline/declarativeWithNamespaceFromYaml.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativeWithNamespaceFromYaml.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativeWithNamespaceFromYaml.groovy index b62a8828e7..496fe0047c 100644 --- a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativeWithNamespaceFromYaml.groovy +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativeWithNamespaceFromYaml.groovy @@ -4,7 +4,7 @@ pipeline { label 'declarativefromyaml-pod' yaml """ metadata: - namespace: test + namespace: kubernetes-plugin-test-overridden-namespace labels: some-label: some-label-value class: KubernetesDeclarativeAgentTest From b4d3e9b89f25b57e79161620e8701554b87be08d Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Thu, 20 Dec 2018 10:54:24 +0100 Subject: [PATCH 3/6] Set the slave namespace after it is started --- .../kubernetes/KubernetesLauncher.java | 12 +++++++----- .../plugins/kubernetes/KubernetesSlave.java | 19 +++++++++---------- .../pipeline/ContainerExecDecorator.java | 2 ++ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java index 74b575a517..d2b1edf6b2 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -29,6 +29,7 @@ import java.io.IOException; import java.io.PrintStream; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.logging.Level; @@ -99,11 +100,12 @@ public void launch(SlaveComputer computer, TaskListener listener) { Pod pod = getPodTemplate(client, slave, unwrappedTemplate); String podId = pod.getMetadata().getName(); - String namespace = StringUtils.defaultIfBlank(slave.getNamespace(), client.getNamespace()); - // JENKINS-51610 the namespace must keep same or will cause an exception - if(pod.getMetadata() != null && StringUtils.isNotBlank(pod.getMetadata().getNamespace())) { - namespace = pod.getMetadata().getNamespace(); - } + + String namespace = Arrays.asList( // + pod.getMetadata() != null ? pod.getMetadata().getNamespace() : null, + unwrappedTemplate.getNamespace(), client.getNamespace()) // + .stream().filter(s -> StringUtils.isNotBlank(s)).findFirst().orElse(null); + slave.setNamespace(namespace); LOGGER.log(Level.FINE, "Creating Pod: {0} in namespace {1}", new Object[]{podId, namespace}); pod = client.pods().inNamespace(namespace).create(pod); 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 db49c5aa98..92acd26f24 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java @@ -67,7 +67,7 @@ public class KubernetesSlave extends AbstractCloudSlave { private static final ResourceBundleHolder HOLDER = ResourceBundleHolder.get(Messages.class); private final String cloudName; - private final String namespace; + private String namespace; private final PodTemplate template; private transient Set executables = new HashSet<>(); @@ -129,23 +129,22 @@ protected KubernetesSlave(String name, PodTemplate template, String nodeDescript template.getNodeProperties()); this.cloudName = cloudName; - 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; } + public void setNamespace(@Nonnull String namespace) { + this.namespace = namespace; + } + + @Nonnull public String getNamespace() { + if (namespace == null) { + throw new IllegalStateException("Namespace has not been set yet"); + } return namespace; } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java index bfdee811c3..c51038cb47 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java @@ -420,6 +420,8 @@ private void setupEnvironmentVariable(EnvVars vars, ExecWatch watch) throws IOEx } private void waitUntilContainerIsReady() throws IOException { + LOGGER.log(Level.FINEST, "Waiting until container is ready: {0}/{1}", + new String[] { namespace, podName }); try { Pod pod = client.pods().inNamespace(namespace).withName(podName) .waitUntilReady(CONTAINER_READY_TIMEOUT, TimeUnit.MINUTES); From 33ae9fb3f8fae653cb332b48c6d19a57b5140efc Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Thu, 20 Dec 2018 11:17:44 +0100 Subject: [PATCH 4/6] Findbugs fixes --- .../jenkins/plugins/kubernetes/KubernetesSlave.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 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 92acd26f24..e9deb39c4a 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java @@ -293,24 +293,23 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted } private void deleteSlavePod(TaskListener listener, KubernetesClient client) throws IOException { - String actualNamespace = getNamespace() == null ? client.getNamespace() : getNamespace(); try { - Boolean deleted = client.pods().inNamespace(actualNamespace).withName(name).delete(); + Boolean deleted = client.pods().inNamespace(getNamespace()).withName(name).delete(); if (!Boolean.TRUE.equals(deleted)) { - String msg = String.format("Failed to delete pod for agent %s/%s: not found", actualNamespace, name); + String msg = String.format("Failed to delete pod for agent %s/%s: not found", getNamespace(), name); LOGGER.log(Level.WARNING, msg); listener.error(msg); return; } } catch (KubernetesClientException e) { - String msg = String.format("Failed to delete pod for agent %s/%s: %s", actualNamespace, name, + String msg = String.format("Failed to delete pod for agent %s/%s: %s", getNamespace(), name, e.getMessage()); LOGGER.log(Level.WARNING, msg, e); listener.error(msg); return; } - String msg = String.format("Terminated Kubernetes instance for agent %s/%s", actualNamespace, name); + String msg = String.format("Terminated Kubernetes instance for agent %s/%s", getNamespace(), name); LOGGER.log(Level.INFO, msg); listener.getLogger().println(msg); } From 48a5ea83737485659d8e42a735fd0a5f060de015 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Sun, 23 Dec 2018 17:41:21 +0100 Subject: [PATCH 5/6] Can't throw exception or hashCode fails Check for null in toComputer --- .../jenkins/plugins/kubernetes/DefaultInProvisioning.java | 8 +++++--- .../jenkins/plugins/kubernetes/KubernetesSlave.java | 3 --- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/DefaultInProvisioning.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/DefaultInProvisioning.java index 041343ac74..8c127cae93 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/DefaultInProvisioning.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/DefaultInProvisioning.java @@ -8,6 +8,7 @@ import javax.annotation.CheckForNull; import hudson.Extension; +import hudson.model.Computer; import hudson.model.Label; import hudson.model.Node; @@ -16,9 +17,10 @@ public class DefaultInProvisioning extends InProvisioning { private static final Logger LOGGER = Logger.getLogger(DefaultInProvisioning.class.getName()); private static boolean isNotAcceptingTasks(Node n) { - return n.toComputer().isLaunchSupported() // Launcher hasn't been called yet - || !n.isAcceptingTasks() // node is not ready yet - ; + Computer computer = n.toComputer(); + return computer != null && (computer.isLaunchSupported() // Launcher hasn't been called yet + || !n.isAcceptingTasks()) // node is not ready yet + ; } @Override 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 e9deb39c4a..291e48c473 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java @@ -142,9 +142,6 @@ public void setNamespace(@Nonnull String namespace) { @Nonnull public String getNamespace() { - if (namespace == null) { - throw new IllegalStateException("Namespace has not been set yet"); - } return namespace; } From aea91d4c55b44095d2af5fb6566a1fd234f69540 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Sun, 23 Dec 2018 18:28:43 +0100 Subject: [PATCH 6/6] Do not use namespace in equals and hashcode --- .../csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java | 2 -- 1 file changed, 2 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 291e48c473..7d11b1a4d8 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java @@ -325,7 +325,6 @@ public boolean equals(Object o) { KubernetesSlave that = (KubernetesSlave) o; if (cloudName != null ? !cloudName.equals(that.cloudName) : that.cloudName != null) return false; - if (namespace != null ? !namespace.equals(that.namespace) : that.namespace != null) return false; return template != null ? template.equals(that.template) : that.template == null; } @@ -333,7 +332,6 @@ public boolean equals(Object o) { public int hashCode() { int result = super.hashCode(); result = 31 * result + (cloudName != null ? cloudName.hashCode() : 0); - result = 31 * result + (namespace != null ? namespace.hashCode() : 0); result = 31 * result + (template != null ? template.hashCode() : 0); return result; }