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 89157e27a..6e2d199da 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java @@ -1448,6 +1448,25 @@ public void registerPodInformer(KubernetesSlave node) { }); } + /** + * Close and remove the informer associated with the given namespace. + * Should be called when no more pods managed by this cloud exist in the namespace + * (typically from {@link KubernetesSlave#_terminate}). + * + * @param namespace the namespace whose informer should be closed + */ + public void unregisterPodInformer(String namespace) { + if (informers == null) { + return; + } + SharedIndexInformer informer = informers.remove(namespace); + if (informer != null) { + informer.close(); + LOGGER.info(() -> String.format( + "Closed informer for namespace [%s] on cloud [%s]", namespace, name)); + } + } + @Extension public static class PodTemplateSourceImpl extends PodTemplateSource { @NonNull 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 601514b73..2b420ee5f 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java @@ -442,6 +442,24 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted name, getPodRetention(cloud) }); } + + // Close the namespace informer if no other pod managed by this cloud remains. + // Without this cleanup, informers for ephemeral namespaces (HNC, etc.) survive + // indefinitely after the namespace is deleted, leaking threads and flooding + // logs with 403 Forbidden errors. + String ns = getNamespace(); + if (ns != null) { + boolean namespaceStillInUse = Jenkins.get().getNodes().stream() + .filter(KubernetesSlave.class::isInstance) + .map(KubernetesSlave.class::cast) + .filter(s -> s != this) + .filter(s -> getCloudName().equals(s.getCloudName())) + .anyMatch(s -> ns.equals(s.getNamespace())); + if (!namespaceStillInUse) { + cloud.unregisterPodInformer(ns); + } + } + String msg = String.format("Disconnected computer %s", name); LOGGER.log(Level.INFO, msg); listener.getLogger().println(msg); diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudTest.java index 1b3f4db59..af7870557 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudTest.java @@ -7,12 +7,20 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.model.Node; import hudson.model.User; import hudson.security.ACL; import hudson.security.ACLContext; import hudson.security.AccessDeniedException3; +import hudson.slaves.RetentionStrategy; +import io.fabric8.kubernetes.api.model.Pod; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -20,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Level; import java.util.logging.Logger; import jenkins.model.Jenkins; @@ -364,4 +373,190 @@ private static void assertAccessDenied(ThrowingRunnable throwingRunnable, String private static @NonNull ACLContext asUser(String admin) { return ACL.as2(User.get(admin, true, Map.of()).impersonate2()); } + + @SuppressWarnings("unchecked") + private Map> getInformersMap(KubernetesCloud cloud) throws Exception { + Field field = KubernetesCloud.class.getDeclaredField("informers"); + field.setAccessible(true); + return (Map>) field.get(cloud); + } + + @Test + public void unregisterPodInformer_closesAndRemoves() throws Exception { + KubernetesCloud cloud = new KubernetesCloud("kubernetes"); + Map> informers = getInformersMap(cloud); + + @SuppressWarnings("unchecked") + SharedIndexInformer informer = mock(SharedIndexInformer.class); + informers.put("my-namespace", informer); + + cloud.unregisterPodInformer("my-namespace"); + + verify(informer).close(); + assertTrue("Informer should be removed from the map", informers.isEmpty()); + } + + @Test + public void unregisterPodInformer_noopOnUnknownNamespace() throws Exception { + KubernetesCloud cloud = new KubernetesCloud("kubernetes"); + Map> informers = getInformersMap(cloud); + + @SuppressWarnings("unchecked") + SharedIndexInformer informer = mock(SharedIndexInformer.class); + informers.put("other-namespace", informer); + + cloud.unregisterPodInformer("unknown-namespace"); + + verify(informer, never()).close(); + assertEquals("Existing informer should not be affected", 1, informers.size()); + } + + /** + * When two pods share the same namespace, terminating the first must NOT + * close the informer — the second pod still needs it. + * Replicates the guard logic from {@code KubernetesSlave._terminate()}. + */ + @Test + public void informerKeptWhileOtherPodsShareNamespace() throws Exception { + String cloudName = "test-cloud"; + String sharedNs = "shared-ns"; + + KubernetesCloud cloud = new KubernetesCloud(cloudName); + j.jenkins.clouds.add(cloud); + + Map> informers = getInformersMap(cloud); + @SuppressWarnings("unchecked") + SharedIndexInformer informer = mock(SharedIndexInformer.class); + informers.put(sharedNs, informer); + + PodTemplate template = new PodTemplate(); + template.setName("tpl"); + KubernetesSlave slave1 = new KubernetesSlave( + template, "slave1", cloudName, "", RetentionStrategy.NOOP); + slave1.setNamespace(sharedNs); + KubernetesSlave slave2 = new KubernetesSlave( + template, "slave2", cloudName, "", RetentionStrategy.NOOP); + slave2.setNamespace(sharedNs); + + j.jenkins.addNode(slave1); + j.jenkins.addNode(slave2); + + // Simulate the guard from _terminate() for slave1: slave2 still exists + boolean namespaceStillInUse = j.jenkins.getNodes().stream() + .filter(KubernetesSlave.class::isInstance) + .map(KubernetesSlave.class::cast) + .filter(s -> s != slave1) + .filter(s -> cloudName.equals(s.getCloudName())) + .anyMatch(s -> sharedNs.equals(s.getNamespace())); + + assertTrue("Namespace should still be in use by slave2", namespaceStillInUse); + if (!namespaceStillInUse) { + cloud.unregisterPodInformer(sharedNs); + } + verify(informer, never()).close(); + assertEquals("Informer must remain in the map", 1, informers.size()); + } + + /** + * When the last pod in a namespace terminates, the informer MUST be closed. + * Replicates the guard logic from {@code KubernetesSlave._terminate()}. + */ + @Test + public void informerClosedWhenLastPodInNamespaceTerminates() throws Exception { + String cloudName = "test-cloud"; + String sharedNs = "shared-ns"; + + KubernetesCloud cloud = new KubernetesCloud(cloudName); + j.jenkins.clouds.add(cloud); + + Map> informers = getInformersMap(cloud); + @SuppressWarnings("unchecked") + SharedIndexInformer informer = mock(SharedIndexInformer.class); + informers.put(sharedNs, informer); + + PodTemplate template = new PodTemplate(); + template.setName("tpl"); + KubernetesSlave slave1 = new KubernetesSlave( + template, "slave1", cloudName, "", RetentionStrategy.NOOP); + slave1.setNamespace(sharedNs); + KubernetesSlave slave2 = new KubernetesSlave( + template, "slave2", cloudName, "", RetentionStrategy.NOOP); + slave2.setNamespace(sharedNs); + + j.jenkins.addNode(slave1); + j.jenkins.addNode(slave2); + + // slave1 terminated and removed from Jenkins + j.jenkins.removeNode(slave1); + + // Now simulate the guard from _terminate() for slave2 (last pod) + boolean namespaceStillInUse = j.jenkins.getNodes().stream() + .filter(KubernetesSlave.class::isInstance) + .map(KubernetesSlave.class::cast) + .filter(s -> s != slave2) + .filter(s -> cloudName.equals(s.getCloudName())) + .anyMatch(s -> sharedNs.equals(s.getNamespace())); + + if (!namespaceStillInUse) { + cloud.unregisterPodInformer(sharedNs); + } + + verify(informer).close(); + assertTrue("Informer must be removed from the map", informers.isEmpty()); + } + + /** + * Informers for a different cloud must not be affected when a pod terminates. + */ + @Test + public void informerNotAffectedByOtherCloud() throws Exception { + String sharedNs = "shared-ns"; + + KubernetesCloud cloudA = new KubernetesCloud("cloud-a"); + KubernetesCloud cloudB = new KubernetesCloud("cloud-b"); + j.jenkins.clouds.add(cloudA); + j.jenkins.clouds.add(cloudB); + + Map> informersA = getInformersMap(cloudA); + @SuppressWarnings("unchecked") + SharedIndexInformer informerA = mock(SharedIndexInformer.class); + informersA.put(sharedNs, informerA); + + Map> informersB = getInformersMap(cloudB); + @SuppressWarnings("unchecked") + SharedIndexInformer informerB = mock(SharedIndexInformer.class); + informersB.put(sharedNs, informerB); + + PodTemplate template = new PodTemplate(); + template.setName("tpl"); + // One pod on cloud-a, one pod on cloud-b, same namespace + KubernetesSlave slaveA = new KubernetesSlave( + template, "slaveA", "cloud-a", "", RetentionStrategy.NOOP); + slaveA.setNamespace(sharedNs); + KubernetesSlave slaveB = new KubernetesSlave( + template, "slaveB", "cloud-b", "", RetentionStrategy.NOOP); + slaveB.setNamespace(sharedNs); + + j.jenkins.addNode(slaveA); + j.jenkins.addNode(slaveB); + + // Simulate _terminate guard for slaveA (last pod on cloud-a) + boolean namespaceStillInUse = j.jenkins.getNodes().stream() + .filter(KubernetesSlave.class::isInstance) + .map(KubernetesSlave.class::cast) + .filter(s -> s != slaveA) + .filter(s -> "cloud-a".equals(s.getCloudName())) + .anyMatch(s -> sharedNs.equals(s.getNamespace())); + + if (!namespaceStillInUse) { + cloudA.unregisterPodInformer(sharedNs); + } + + // cloud-a informer closed (no other cloud-a pods in that namespace) + verify(informerA).close(); + assertTrue("cloud-a informer must be removed", informersA.isEmpty()); + // cloud-b informer untouched + verify(informerB, never()).close(); + assertEquals("cloud-b informer must remain", 1, informersB.size()); + } }