From f0a86cd48d5c3f78156d52a6c172b9e9fd09c769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Fri, 13 Dec 2024 13:32:53 +0100 Subject: [PATCH 01/12] [JENKINS-74992] Print relevant pod provisioning events in build logs --- .../kubernetes/KubernetesLauncher.java | 15 ++++ .../watch/PodStatusEventHandler.java | 85 +++++++++++++++++++ .../PodProvisioningStatusLogsTest.java | 38 +++++++++ .../pipeline/containerStatusErrorLogs.groovy | 26 ++++++ .../pipeline/podStatusErrorLogs.groovy | 28 ++++++ .../pipeline/podStatusNoErrorLogs.groovy | 26 ++++++ 6 files changed, 218 insertions(+) create mode 100644 src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java create mode 100644 src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodProvisioningStatusLogsTest.java create mode 100644 src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/containerStatusErrorLogs.groovy create mode 100644 src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/podStatusErrorLogs.groovy create mode 100644 src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/podStatusNoErrorLogs.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 93c2d34a2..8f9bd24ee 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -42,6 +42,7 @@ import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -49,6 +50,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -57,6 +59,7 @@ import org.apache.commons.lang.StringUtils; import org.csanchez.jenkins.plugins.kubernetes.pod.decorator.PodDecoratorException; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper; +import org.csanchez.jenkins.plugins.kubernetes.watch.PodStatusEventHandler; import org.kohsuke.stapler.DataBoundConstructor; /** @@ -73,6 +76,9 @@ public class KubernetesLauncher extends JNLPLauncher { private volatile boolean launched = false; + // namespace -> informer + private static final Map> informers = new ConcurrentHashMap<>(); + /** * Provisioning exception if any. */ @@ -145,6 +151,15 @@ public synchronized void launch(SlaveComputer computer, TaskListener listener) { .orElse(null); node.setNamespace(namespace); + // register a namespace informer (if not registered yet) show relevant pod events in build logs + if (informers.get(namespace) == null) { + SharedIndexInformer inform = client.pods() + .inNamespace(namespace) + .inform(new PodStatusEventHandler(), TimeUnit.SECONDS.toMillis(30)); + LOGGER.info("Registered informer to watch events on namespace: " + namespace); + informers.put(namespace, inform); + } + // if the controller was interrupted after creating the pod but before it connected back, then // the pod might already exist and the creating logic must be skipped. Pod existingPod = diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java new file mode 100644 index 000000000..c8afc3221 --- /dev/null +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java @@ -0,0 +1,85 @@ +package org.csanchez.jenkins.plugins.kubernetes.watch; + +import hudson.model.Node; +import hudson.model.TaskListener; +import hudson.slaves.SlaveComputer; +import io.fabric8.kubernetes.api.model.ContainerState; +import io.fabric8.kubernetes.api.model.ContainerStatus; +import io.fabric8.kubernetes.api.model.Pod; +import io.fabric8.kubernetes.api.model.PodCondition; +import io.fabric8.kubernetes.client.informers.ResourceEventHandler; +import java.util.Optional; +import java.util.logging.Logger; +import jenkins.model.Jenkins; +import org.csanchez.jenkins.plugins.kubernetes.KubernetesSlave; + +/** + * Process pod events and print relevant information in build logs. + * Registered as an informer in {@link org.csanchez.jenkins.plugins.kubernetes.KubernetesLauncher#launch(SlaveComputer, TaskListener)}). + */ +public class PodStatusEventHandler implements ResourceEventHandler { + + private static final Logger LOGGER = Logger.getLogger(PodStatusEventHandler.class.getName()); + + @Override + public void onUpdate(Pod unused, Pod pod) { + Optional found = Jenkins.get().getNodes().stream() + .filter(n -> n.getNodeName().equals(pod.getMetadata().getName())) + .findFirst(); + if (found.isPresent()) { + final StringBuilder sb = new StringBuilder(); + pod.getStatus().getContainerStatuses().forEach(s -> sb.append(formatContainerStatus(s))); + pod.getStatus().getConditions().forEach(c -> sb.append(formatPodStatus(c, pod.getStatus().getPhase()))); + if (!sb.toString().isEmpty()) { + ((KubernetesSlave) found.get()) + .getRunListener() + .getLogger() + .println("[PodInfo] " + pod.getMetadata().getName() + sb); + } + } else { + LOGGER.fine(() -> "Event received for non-existent node: [" + + pod.getMetadata().getName() + "]"); + } + } + + private String formatPodStatus(PodCondition c, String phase) { + if (c.getReason() == null) { + // not interesting + return ""; + } + return String.format("%n\tPod [%s][%s] %s", phase, c.getReason(), c.getMessage()); + } + + private String formatContainerStatus(ContainerStatus s) { + ContainerState state = s.getState(); + if (state.getRunning() != null) { + // don't care about running + return ""; + } + StringBuilder sb = new StringBuilder(); + sb.append(String.format("%n\tContainer [%s]", s.getName())); + if (state.getTerminated() != null) { + String message = state.getTerminated().getMessage(); + sb.append(String.format( + " terminated [%s] %s", + state.getTerminated().getReason(), message != null ? message : "No message")); + } + if (state.getWaiting() != null) { + String message = state.getWaiting().getMessage(); + sb.append(String.format( + " waiting [%s] %s", + state.getWaiting().getReason(), message != null ? message : "No message")); + } + return sb.toString(); + } + + @Override + public void onDelete(Pod pod, boolean deletedFinalStateUnknown) { + // no-op + } + + @Override + public void onAdd(Pod pod) { + // no-op + } +} diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodProvisioningStatusLogsTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodProvisioningStatusLogsTest.java new file mode 100644 index 000000000..c0454008b --- /dev/null +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodProvisioningStatusLogsTest.java @@ -0,0 +1,38 @@ +package org.csanchez.jenkins.plugins.kubernetes.pipeline; + +import hudson.model.Result; +import org.junit.Test; + +import static org.junit.Assert.assertNotNull; + +public class PodProvisioningStatusLogsTest extends AbstractKubernetesPipelineTest { + + @Test + public void podStatusErrorLogs() throws Exception { + assertNotNull(createJobThenScheduleRun()); + // pod not schedulable + // build never finishes, so just checking the message and killing + r.waitForMessage("Pod [Pending][Unschedulable] 0/1 nodes are available", b); + b.doKill(); + r.waitUntilNoActivity(); + } + + @Test + public void podStatusNoErrorLogs() throws Exception { + assertNotNull(createJobThenScheduleRun()); + r.assertBuildStatusSuccess(r.waitForCompletion(b)); + // regular logs when starting containers + r.assertLogContains("Container [jnlp] waiting [ContainerCreating]", b); + r.assertLogContains("Pod [Pending][ContainersNotReady] containers with unready status: [shell jnlp]", b); + } + + @Test + public void containerStatusErrorLogs() throws Exception { + assertNotNull(createJobThenScheduleRun()); + r.assertBuildStatus(Result.ABORTED, r.waitForCompletion(b)); + // error starting container + r.assertLogContains("Container [shell] terminated [StartError]", b); + r.assertLogContains("exec: \"oops\": executable file not found", b); + r.assertLogContains("Pod [Running][ContainersNotReady] containers with unready status: [shell]", b); + } +} diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/containerStatusErrorLogs.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/containerStatusErrorLogs.groovy new file mode 100644 index 000000000..fd723dbc2 --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/containerStatusErrorLogs.groovy @@ -0,0 +1,26 @@ +//noinspection GrPackage +pipeline { + agent { + kubernetes { + yaml ''' +apiVersion: v1 +kind: Pod +spec: + containers: + - name: shell + image: ubuntu + command: + - oops + args: + - infinity +''' + } + } + stages { + stage('Run') { + steps { + sh 'hostname' + } + } + } +} \ No newline at end of file diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/podStatusErrorLogs.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/podStatusErrorLogs.groovy new file mode 100644 index 000000000..061dcce9d --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/podStatusErrorLogs.groovy @@ -0,0 +1,28 @@ +//noinspection GrPackage +pipeline { + agent { + kubernetes { + yaml ''' +apiVersion: v1 +kind: Pod +spec: + containers: + - name: shell + image: ubuntu + command: + - sleep + args: + - infinity + nodeSelector: + disktype: ssd +''' + } + } + stages { + stage('Run') { + steps { + sh 'hostname' + } + } + } +} \ No newline at end of file diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/podStatusNoErrorLogs.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/podStatusNoErrorLogs.groovy new file mode 100644 index 000000000..a96bad012 --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/podStatusNoErrorLogs.groovy @@ -0,0 +1,26 @@ +//noinspection GrPackage +pipeline { + agent { + kubernetes { + yaml ''' +apiVersion: v1 +kind: Pod +spec: + containers: + - name: shell + image: ubuntu + command: + - sleep + args: + - infinity +''' + } + } + stages { + stage('Run') { + steps { + sh 'hostname' + } + } + } +} \ No newline at end of file From 2335dbffd85460cf32ec56b084af2d93580e735e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Fri, 13 Dec 2024 17:27:29 +0100 Subject: [PATCH 02/12] Informers are scoped to the KubernetesCloud and use label filters to select what to watch more specifically. --- .../plugins/kubernetes/KubernetesCloud.java | 14 ++++++++++++++ .../plugins/kubernetes/KubernetesLauncher.java | 14 ++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index d93e9c60a..1661e0910 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -29,6 +29,7 @@ import hudson.util.FormValidation; import hudson.util.ListBoxModel; import hudson.util.XStream2; +import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.VersionInfo; @@ -53,9 +54,12 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Level; import java.util.logging.Logger; import javax.servlet.ServletException; + +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; import jenkins.authentication.tokens.api.AuthenticationTokens; import jenkins.bouncycastle.api.PEMEncodable; import jenkins.metrics.api.Metrics; @@ -161,6 +165,12 @@ public class KubernetesCloud extends Cloud implements PodTemplateGroup { @CheckForNull private GarbageCollection garbageCollection; + /** + * namespace -> informer + * Use to watch pod events per namespace. + */ + private final Map> informers = new ConcurrentHashMap<>(); + @DataBoundConstructor public KubernetesCloud(String name) { super(name); @@ -922,6 +932,10 @@ public HttpResponse doCreate(StaplerRequest req, StaplerResponse rsp) return FormApply.success("templates"); } + public Map> getInformers() { + return informers; + } + @Extension public static class DescriptorImpl extends Descriptor { @Override 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 8f9bd24ee..a54f034ad 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -27,6 +27,7 @@ import static java.util.logging.Level.FINE; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; +import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.sanitizeLabel; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -48,6 +49,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -76,9 +78,6 @@ public class KubernetesLauncher extends JNLPLauncher { private volatile boolean launched = false; - // namespace -> informer - private static final Map> informers = new ConcurrentHashMap<>(); - /** * Provisioning exception if any. */ @@ -152,11 +151,18 @@ public synchronized void launch(SlaveComputer computer, TaskListener listener) { node.setNamespace(namespace); // register a namespace informer (if not registered yet) show relevant pod events in build logs + Map> informers = node.getKubernetesCloud().getInformers(); if (informers.get(namespace) == null) { + Map labelsFilter = new HashMap<>(node.getKubernetesCloud().getPodLabelsMap()); + String jenkinsUrlLabel = sanitizeLabel(cloud.getJenkinsUrlOrNull()); + if (jenkinsUrlLabel != null) { + labelsFilter.put(PodTemplateBuilder.LABEL_KUBERNETES_CONTROLLER, jenkinsUrlLabel); + } SharedIndexInformer inform = client.pods() .inNamespace(namespace) + .withLabels(labelsFilter) .inform(new PodStatusEventHandler(), TimeUnit.SECONDS.toMillis(30)); - LOGGER.info("Registered informer to watch events on namespace: " + namespace); + LOGGER.info(String.format("Registered informer to watch pod events on namespace [%s], with labels [%s]", namespace, labelsFilter)); informers.put(namespace, inform); } From fd80126e0b56cc7a156b475c5a325ad456b25944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Fri, 13 Dec 2024 17:28:50 +0100 Subject: [PATCH 03/12] Apply spotless --- .../jenkins/plugins/kubernetes/KubernetesCloud.java | 3 +-- .../plugins/kubernetes/KubernetesLauncher.java | 11 +++++++---- .../kubernetes/watch/PodStatusEventHandler.java | 7 ++++--- .../pipeline/PodProvisioningStatusLogsTest.java | 4 ++-- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index 1661e0910..a94969b74 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -33,6 +33,7 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.VersionInfo; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; import java.io.IOException; import java.io.StringReader; import java.net.ConnectException; @@ -58,8 +59,6 @@ import java.util.logging.Level; import java.util.logging.Logger; import javax.servlet.ServletException; - -import io.fabric8.kubernetes.client.informers.SharedIndexInformer; import jenkins.authentication.tokens.api.AuthenticationTokens; import jenkins.bouncycastle.api.PEMEncodable; import jenkins.metrics.api.Metrics; 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 a54f034ad..5a1316a5f 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -52,7 +52,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -151,9 +150,11 @@ public synchronized void launch(SlaveComputer computer, TaskListener listener) { node.setNamespace(namespace); // register a namespace informer (if not registered yet) show relevant pod events in build logs - Map> informers = node.getKubernetesCloud().getInformers(); + Map> informers = + node.getKubernetesCloud().getInformers(); if (informers.get(namespace) == null) { - Map labelsFilter = new HashMap<>(node.getKubernetesCloud().getPodLabelsMap()); + Map labelsFilter = + new HashMap<>(node.getKubernetesCloud().getPodLabelsMap()); String jenkinsUrlLabel = sanitizeLabel(cloud.getJenkinsUrlOrNull()); if (jenkinsUrlLabel != null) { labelsFilter.put(PodTemplateBuilder.LABEL_KUBERNETES_CONTROLLER, jenkinsUrlLabel); @@ -162,7 +163,9 @@ public synchronized void launch(SlaveComputer computer, TaskListener listener) { .inNamespace(namespace) .withLabels(labelsFilter) .inform(new PodStatusEventHandler(), TimeUnit.SECONDS.toMillis(30)); - LOGGER.info(String.format("Registered informer to watch pod events on namespace [%s], with labels [%s]", namespace, labelsFilter)); + LOGGER.info(String.format( + "Registered informer to watch pod events on namespace [%s], with labels [%s]", + namespace, labelsFilter)); informers.put(namespace, inform); } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java index c8afc3221..c7db6b29e 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java @@ -29,7 +29,9 @@ public void onUpdate(Pod unused, Pod pod) { if (found.isPresent()) { final StringBuilder sb = new StringBuilder(); pod.getStatus().getContainerStatuses().forEach(s -> sb.append(formatContainerStatus(s))); - pod.getStatus().getConditions().forEach(c -> sb.append(formatPodStatus(c, pod.getStatus().getPhase()))); + pod.getStatus() + .getConditions() + .forEach(c -> sb.append(formatPodStatus(c, pod.getStatus().getPhase()))); if (!sb.toString().isEmpty()) { ((KubernetesSlave) found.get()) .getRunListener() @@ -67,8 +69,7 @@ private String formatContainerStatus(ContainerStatus s) { if (state.getWaiting() != null) { String message = state.getWaiting().getMessage(); sb.append(String.format( - " waiting [%s] %s", - state.getWaiting().getReason(), message != null ? message : "No message")); + " waiting [%s] %s", state.getWaiting().getReason(), message != null ? message : "No message")); } return sb.toString(); } diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodProvisioningStatusLogsTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodProvisioningStatusLogsTest.java index c0454008b..6a4be2d3e 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodProvisioningStatusLogsTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodProvisioningStatusLogsTest.java @@ -1,10 +1,10 @@ package org.csanchez.jenkins.plugins.kubernetes.pipeline; +import static org.junit.Assert.assertNotNull; + import hudson.model.Result; import org.junit.Test; -import static org.junit.Assert.assertNotNull; - public class PodProvisioningStatusLogsTest extends AbstractKubernetesPipelineTest { @Test From d97638010b8bd5e653c11cf2fb32f90a741ba67c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Fri, 13 Dec 2024 19:14:13 +0100 Subject: [PATCH 04/12] No need to serialize informers + review comments fix --- .../csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java | 2 +- .../jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index a94969b74..abd7a06ce 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -168,7 +168,7 @@ public class KubernetesCloud extends Cloud implements PodTemplateGroup { * namespace -> informer * Use to watch pod events per namespace. */ - private final Map> informers = new ConcurrentHashMap<>(); + private transient final Map> informers = new ConcurrentHashMap<>(); @DataBoundConstructor public KubernetesCloud(String name) { diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java index c7db6b29e..255bae375 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java @@ -36,7 +36,7 @@ public void onUpdate(Pod unused, Pod pod) { ((KubernetesSlave) found.get()) .getRunListener() .getLogger() - .println("[PodInfo] " + pod.getMetadata().getName() + sb); + .println("[PodInfo] " + pod.getMetadata().getNamespace() + "/" + pod.getMetadata().getName() + sb); } } else { LOGGER.fine(() -> "Event received for non-existent node: [" From 1838ee487c602e2765e403f72f8d16df3595df50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Fri, 13 Dec 2024 19:14:46 +0100 Subject: [PATCH 05/12] Fix spotless --- .../csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java | 2 +- .../plugins/kubernetes/watch/PodStatusEventHandler.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index abd7a06ce..b695e09c8 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -168,7 +168,7 @@ public class KubernetesCloud extends Cloud implements PodTemplateGroup { * namespace -> informer * Use to watch pod events per namespace. */ - private transient final Map> informers = new ConcurrentHashMap<>(); + private final transient Map> informers = new ConcurrentHashMap<>(); @DataBoundConstructor public KubernetesCloud(String name) { diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java index 255bae375..67e7d1b75 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java @@ -36,7 +36,8 @@ public void onUpdate(Pod unused, Pod pod) { ((KubernetesSlave) found.get()) .getRunListener() .getLogger() - .println("[PodInfo] " + pod.getMetadata().getNamespace() + "/" + pod.getMetadata().getName() + sb); + .println("[PodInfo] " + pod.getMetadata().getNamespace() + "/" + + pod.getMetadata().getName() + sb); } } else { LOGGER.fine(() -> "Event received for non-existent node: [" From 5414ee3fac238886d697edabfa3077f15d21fa7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Fri, 13 Dec 2024 19:39:43 +0100 Subject: [PATCH 06/12] Field can be null after deserialization --- .../csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index b695e09c8..21dab08eb 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -168,7 +168,7 @@ public class KubernetesCloud extends Cloud implements PodTemplateGroup { * namespace -> informer * Use to watch pod events per namespace. */ - private final transient Map> informers = new ConcurrentHashMap<>(); + private transient Map> informers = new ConcurrentHashMap<>(); @DataBoundConstructor public KubernetesCloud(String name) { @@ -1306,6 +1306,9 @@ private Object readResolve() { if (containerCap != null && containerCap == 0) { containerCap = null; } + if (informers == null) { + informers = new ConcurrentHashMap<>(); + } return this; } From 66e538efbc67d76c450f5ae9b1b9c5e590378558 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Mon, 16 Dec 2024 13:10:57 +0100 Subject: [PATCH 07/12] Moving informer registration to `KubernetesCloud` and make it thread safe. --- .../plugins/kubernetes/KubernetesCloud.java | 33 ++++++++++++++++--- .../kubernetes/KubernetesLauncher.java | 21 ++---------- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index 21dab08eb..fec252971 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -2,6 +2,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.commons.lang.StringUtils.isEmpty; +import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.sanitizeLabel; import com.cloudbees.plugins.credentials.CredentialsMatchers; import com.cloudbees.plugins.credentials.common.StandardCredentials; @@ -51,11 +52,13 @@ import java.util.Base64; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; import javax.servlet.ServletException; @@ -72,6 +75,7 @@ import org.csanchez.jenkins.plugins.kubernetes.pipeline.PodTemplateMap; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Default; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention; +import org.csanchez.jenkins.plugins.kubernetes.watch.PodStatusEventHandler; import org.jenkinsci.plugins.kubernetes.auth.KubernetesAuth; import org.jenkinsci.plugins.kubernetes.auth.KubernetesAuthException; import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl; @@ -931,10 +935,6 @@ public HttpResponse doCreate(StaplerRequest req, StaplerResponse rsp) return FormApply.success("templates"); } - public Map> getInformers() { - return informers; - } - @Extension public static class DescriptorImpl extends Descriptor { @Override @@ -1320,6 +1320,31 @@ public Cloud reconfigure(@NonNull StaplerRequest req, JSONObject form) throws De return newInstance; } + public void registerPodInformer(KubernetesSlave node, KubernetesClient client, String namespace) { + if (informers.get(namespace) == null) { + synchronized (this) { + // sync recheck + if (informers.get(namespace) != null) { + return; + } + Map labelsFilter = + new HashMap<>(node.getKubernetesCloud().getPodLabelsMap()); + String jenkinsUrlLabel = sanitizeLabel(this.getJenkinsUrlOrNull()); + if (jenkinsUrlLabel != null) { + labelsFilter.put(PodTemplateBuilder.LABEL_KUBERNETES_CONTROLLER, jenkinsUrlLabel); + } + SharedIndexInformer inform = client.pods() + .inNamespace(namespace) + .withLabels(labelsFilter) + .inform(new PodStatusEventHandler(), TimeUnit.SECONDS.toMillis(30)); + LOGGER.info(String.format( + "Registered informer to watch pod events on namespace [%s], with labels [%s]", + namespace, labelsFilter)); + informers.put(namespace, inform); + } + } + } + @Extension public static class PodTemplateSourceImpl extends PodTemplateSource { @NonNull 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 5a1316a5f..8608ebdd4 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -149,25 +149,8 @@ public synchronized void launch(SlaveComputer computer, TaskListener listener) { .orElse(null); node.setNamespace(namespace); - // register a namespace informer (if not registered yet) show relevant pod events in build logs - Map> informers = - node.getKubernetesCloud().getInformers(); - if (informers.get(namespace) == null) { - Map labelsFilter = - new HashMap<>(node.getKubernetesCloud().getPodLabelsMap()); - String jenkinsUrlLabel = sanitizeLabel(cloud.getJenkinsUrlOrNull()); - if (jenkinsUrlLabel != null) { - labelsFilter.put(PodTemplateBuilder.LABEL_KUBERNETES_CONTROLLER, jenkinsUrlLabel); - } - SharedIndexInformer inform = client.pods() - .inNamespace(namespace) - .withLabels(labelsFilter) - .inform(new PodStatusEventHandler(), TimeUnit.SECONDS.toMillis(30)); - LOGGER.info(String.format( - "Registered informer to watch pod events on namespace [%s], with labels [%s]", - namespace, labelsFilter)); - informers.put(namespace, inform); - } + // register a namespace informer (if not registered yet) to show relevant pod events in build logs + cloud.registerPodInformer(node, client, namespace); // if the controller was interrupted after creating the pod but before it connected back, then // the pod might already exist and the creating logic must be skipped. From 5ccb41667839df4a7f2418c6d204e8fe93e88688 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Mon, 16 Dec 2024 16:46:53 +0100 Subject: [PATCH 08/12] Spotless --- .../jenkins/plugins/kubernetes/KubernetesLauncher.java | 4 ---- 1 file changed, 4 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 8608ebdd4..9e47610e0 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -27,7 +27,6 @@ import static java.util.logging.Level.FINE; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; -import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.sanitizeLabel; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -43,13 +42,11 @@ import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; -import io.fabric8.kubernetes.client.informers.SharedIndexInformer; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -60,7 +57,6 @@ import org.apache.commons.lang.StringUtils; import org.csanchez.jenkins.plugins.kubernetes.pod.decorator.PodDecoratorException; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper; -import org.csanchez.jenkins.plugins.kubernetes.watch.PodStatusEventHandler; import org.kohsuke.stapler.DataBoundConstructor; /** From 6cf63ce728ae0a69e0fc09cf269c3b6bc6af1c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Mon, 16 Dec 2024 18:12:46 +0100 Subject: [PATCH 09/12] As suggested by @VLatombe in a code review: * Informer register logic moved to Kubernetes cloud * Create a new client connection instead of reusing one * Log the cloud name --- .../plugins/kubernetes/KubernetesCloud.java | 45 ++++++++++--------- .../kubernetes/KubernetesLauncher.java | 2 +- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index fec252971..1ce8d8360 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -1320,29 +1320,30 @@ public Cloud reconfigure(@NonNull StaplerRequest req, JSONObject form) throws De return newInstance; } - public void registerPodInformer(KubernetesSlave node, KubernetesClient client, String namespace) { - if (informers.get(namespace) == null) { - synchronized (this) { - // sync recheck - if (informers.get(namespace) != null) { - return; - } - Map labelsFilter = - new HashMap<>(node.getKubernetesCloud().getPodLabelsMap()); - String jenkinsUrlLabel = sanitizeLabel(this.getJenkinsUrlOrNull()); - if (jenkinsUrlLabel != null) { - labelsFilter.put(PodTemplateBuilder.LABEL_KUBERNETES_CONTROLLER, jenkinsUrlLabel); - } - SharedIndexInformer inform = client.pods() - .inNamespace(namespace) - .withLabels(labelsFilter) - .inform(new PodStatusEventHandler(), TimeUnit.SECONDS.toMillis(30)); - LOGGER.info(String.format( - "Registered informer to watch pod events on namespace [%s], with labels [%s]", - namespace, labelsFilter)); - informers.put(namespace, inform); + public void registerPodInformer(KubernetesSlave node) { + informers.computeIfAbsent(namespace, (n) -> { + KubernetesClient client; + try { + client = connect(); + } catch (KubernetesAuthException | IOException e) { + LOGGER.log(Level.WARNING, "Cannot connect to K8s cloud. Pod events will not be available in build logs.", e); + return null; } - } + Map labelsFilter = + new HashMap<>(getPodLabelsMap()); + String jenkinsUrlLabel = sanitizeLabel(getJenkinsUrlOrNull()); + if (jenkinsUrlLabel != null) { + labelsFilter.put(PodTemplateBuilder.LABEL_KUBERNETES_CONTROLLER, jenkinsUrlLabel); + } + SharedIndexInformer inform = client.pods() + .inNamespace(node.getNamespace()) + .withLabels(labelsFilter) + .inform(new PodStatusEventHandler(), TimeUnit.SECONDS.toMillis(30)); + LOGGER.info(String.format( + "Registered informer to watch pod events on namespace [%s], with labels [%s] on cloud [%s]", + namespace, labelsFilter, name)); + return inform; + }); } @Extension 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 8608ebdd4..efd3f8cdd 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -150,7 +150,7 @@ public synchronized void launch(SlaveComputer computer, TaskListener listener) { node.setNamespace(namespace); // register a namespace informer (if not registered yet) to show relevant pod events in build logs - cloud.registerPodInformer(node, client, namespace); + cloud.registerPodInformer(node); // if the controller was interrupted after creating the pod but before it connected back, then // the pod might already exist and the creating logic must be skipped. From 7ce3dabe605eac08dfa06fab858924281ee38a2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Mon, 16 Dec 2024 18:15:24 +0100 Subject: [PATCH 10/12] Spotless --- .../jenkins/plugins/kubernetes/KubernetesCloud.java | 8 +++++--- .../jenkins/plugins/kubernetes/KubernetesLauncher.java | 4 ---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index 1ce8d8360..a7e0fe6fa 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -1326,11 +1326,13 @@ public void registerPodInformer(KubernetesSlave node) { try { client = connect(); } catch (KubernetesAuthException | IOException e) { - LOGGER.log(Level.WARNING, "Cannot connect to K8s cloud. Pod events will not be available in build logs.", e); + LOGGER.log( + Level.WARNING, + "Cannot connect to K8s cloud. Pod events will not be available in build logs.", + e); return null; } - Map labelsFilter = - new HashMap<>(getPodLabelsMap()); + Map labelsFilter = new HashMap<>(getPodLabelsMap()); String jenkinsUrlLabel = sanitizeLabel(getJenkinsUrlOrNull()); if (jenkinsUrlLabel != null) { labelsFilter.put(PodTemplateBuilder.LABEL_KUBERNETES_CONTROLLER, jenkinsUrlLabel); 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 efd3f8cdd..97cdc7d45 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -27,7 +27,6 @@ import static java.util.logging.Level.FINE; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; -import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.sanitizeLabel; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -43,13 +42,11 @@ import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; -import io.fabric8.kubernetes.client.informers.SharedIndexInformer; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -60,7 +57,6 @@ import org.apache.commons.lang.StringUtils; import org.csanchez.jenkins.plugins.kubernetes.pod.decorator.PodDecoratorException; import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper; -import org.csanchez.jenkins.plugins.kubernetes.watch.PodStatusEventHandler; import org.kohsuke.stapler.DataBoundConstructor; /** From 04f48a552c24f646d56623a0349a4aebc5b3047e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Tue, 17 Dec 2024 10:35:53 +0100 Subject: [PATCH 11/12] Fix NPE when KubernetesCloud does not define a namespace (uses default) --- .../csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java index a7e0fe6fa..037887e47 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -1321,7 +1321,7 @@ public Cloud reconfigure(@NonNull StaplerRequest req, JSONObject form) throws De } public void registerPodInformer(KubernetesSlave node) { - informers.computeIfAbsent(namespace, (n) -> { + informers.computeIfAbsent(node.getNamespace(), (n) -> { KubernetesClient client; try { client = connect(); From d009f7ff3a7ffe2e1a07200386f6360954ab29b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Tue, 17 Dec 2024 16:28:05 +0100 Subject: [PATCH 12/12] Workaround to not print events which differences are not relevant --- .../plugins/kubernetes/watch/PodStatusEventHandler.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java index 67e7d1b75..cac846b50 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/watch/PodStatusEventHandler.java @@ -31,7 +31,7 @@ public void onUpdate(Pod unused, Pod pod) { pod.getStatus().getContainerStatuses().forEach(s -> sb.append(formatContainerStatus(s))); pod.getStatus() .getConditions() - .forEach(c -> sb.append(formatPodStatus(c, pod.getStatus().getPhase()))); + .forEach(c -> sb.append(formatPodStatus(c, pod.getStatus().getPhase(), sb))); if (!sb.toString().isEmpty()) { ((KubernetesSlave) found.get()) .getRunListener() @@ -45,12 +45,13 @@ public void onUpdate(Pod unused, Pod pod) { } } - private String formatPodStatus(PodCondition c, String phase) { + private String formatPodStatus(PodCondition c, String phase, StringBuilder sb) { if (c.getReason() == null) { // not interesting return ""; } - return String.format("%n\tPod [%s][%s] %s", phase, c.getReason(), c.getMessage()); + String formatted = String.format("%n\tPod [%s][%s] %s", phase, c.getReason(), c.getMessage()); + return sb.indexOf(formatted) == -1 ? formatted : ""; } private String formatContainerStatus(ContainerStatus s) {