Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<Pod> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,28 @@
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;
import java.util.LinkedHashMap;
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;
Expand Down Expand Up @@ -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<String, SharedIndexInformer<Pod>> getInformersMap(KubernetesCloud cloud) throws Exception {
Field field = KubernetesCloud.class.getDeclaredField("informers");
field.setAccessible(true);
return (Map<String, SharedIndexInformer<Pod>>) field.get(cloud);
}

@Test
public void unregisterPodInformer_closesAndRemoves() throws Exception {
KubernetesCloud cloud = new KubernetesCloud("kubernetes");
Map<String, SharedIndexInformer<Pod>> informers = getInformersMap(cloud);

@SuppressWarnings("unchecked")
SharedIndexInformer<Pod> 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<String, SharedIndexInformer<Pod>> informers = getInformersMap(cloud);

@SuppressWarnings("unchecked")
SharedIndexInformer<Pod> 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<String, SharedIndexInformer<Pod>> informers = getInformersMap(cloud);
@SuppressWarnings("unchecked")
SharedIndexInformer<Pod> 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<String, SharedIndexInformer<Pod>> informers = getInformersMap(cloud);
@SuppressWarnings("unchecked")
SharedIndexInformer<Pod> 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<String, SharedIndexInformer<Pod>> informersA = getInformersMap(cloudA);
@SuppressWarnings("unchecked")
SharedIndexInformer<Pod> informerA = mock(SharedIndexInformer.class);
informersA.put(sharedNs, informerA);

Map<String, SharedIndexInformer<Pod>> informersB = getInformersMap(cloudB);
@SuppressWarnings("unchecked")
SharedIndexInformer<Pod> 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());
}
}