From 486218ac728b413ebadfecdb5bbb79239d9fdb01 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 13 Dec 2018 15:21:35 +0100 Subject: [PATCH] [JENKINS-55138] Don't close kubernetes client upon cache removal Added a SaveableListener to invalidate cache when Jenkins object is being saved. Instead of blindly closing the KubernetesClient when evicted from the cache, put it in a list to monitor. Then every minute, PurgeExpiredKubernetesClients will check if there are still http connections and close it when it becomes unused. --- .../kubernetes/KubernetesClientProvider.java | 172 +++++++++++++----- 1 file changed, 131 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesClientProvider.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesClientProvider.java index 30c8ec6a19..b94aa051c2 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesClientProvider.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesClientProvider.java @@ -1,70 +1,88 @@ package org.csanchez.jenkins.plugins.kubernetes; -import com.google.common.base.Objects; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.RemovalListener; - -import io.fabric8.kubernetes.client.KubernetesClient; - import java.io.IOException; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.UnrecoverableKeyException; import java.security.cert.CertificateEncodingException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.logging.Level; +import java.util.logging.Logger; + +import com.google.common.base.Objects; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import io.fabric8.kubernetes.client.HttpClientAware; +import io.fabric8.kubernetes.client.KubernetesClient; +import okhttp3.Dispatcher; +import okhttp3.OkHttpClient; + +import hudson.Extension; +import hudson.XmlFile; +import hudson.model.AsyncPeriodicWork; +import hudson.model.Saveable; +import hudson.model.TaskListener; +import hudson.model.listeners.SaveableListener; +import jenkins.model.Jenkins; /** * Manages the Kubernetes client creation per cloud */ final class KubernetesClientProvider { - private static final Cache clients; - - private static final Integer CACHE_SIZE; - private static final Integer CACHE_TTL; - - static { - CACHE_SIZE = Integer.getInteger("org.csanchez.jenkins.plugins.kubernetes.clients.cacheSize", 10); - CACHE_TTL = Integer.getInteger("org.csanchez.jenkins.plugins.kubernetes.clients.cacheTtl", 60); - clients = CacheBuilder.newBuilder() - .maximumSize(CACHE_SIZE) - .expireAfterWrite(CACHE_TTL, TimeUnit.MINUTES) - .removalListener((RemovalListener) removalNotification -> { - // https://google.github.io/guava/releases/23.0/api/docs/com/google/common/cache/RemovalNotification.html - // A notification of the removal of a single entry. The key and/or value may be null if they were already garbage collected. - if (removalNotification.getValue() != null) { - removalNotification.getValue().getClient().close(); + private static final Logger LOGGER = Logger.getLogger(KubernetesClientProvider.class.getName()); + + private static final Integer CACHE_SIZE = Integer.getInteger("org.csanchez.jenkins.plugins.kubernetes.clients.cacheSize", 10); + + private static final List expiredClients = Collections.synchronizedList(new ArrayList()); + + private static final Cache clients = CacheBuilder + .newBuilder() + .maximumSize(CACHE_SIZE) + .removalListener(rl -> { + LOGGER.log(Level.FINE, "{0} cache : Removing entry for {1}", new Object[] {KubernetesClient.class.getSimpleName(), rl.getKey()}); + KubernetesClient client = ((Client) rl.getValue()).getClient(); + if (client != null) { + if (client instanceof HttpClientAware) { + if (!gracefulClose(client, ((HttpClientAware) client).getHttpClient())) { + expiredClients.add(client); + } + } else { + LOGGER.log(Level.WARNING, "{0} is not {1}, forcing close", new Object[] {client.toString(), HttpClientAware.class.getSimpleName()}); + client.close(); } - }) - .build(); - } + } + + }) + .build(); private KubernetesClientProvider() { } static KubernetesClient createClient(KubernetesCloud cloud) throws NoSuchAlgorithmException, UnrecoverableKeyException, KeyStoreException, IOException, CertificateEncodingException { - - final int validity = Objects.hashCode(cloud.getServerUrl(), cloud.getNamespace(), cloud.getServerCertificate(), - cloud.getCredentialsId(), cloud.isSkipTlsVerify(), cloud.getConnectTimeout(), cloud.getReadTimeout(), - cloud.getMaxRequestsPerHostStr()); - final Client c = clients.getIfPresent(cloud.getDisplayName()); - - if (c != null && validity == c.getValidity()) { - return c.getClient(); - } else { - // expire tha cache if any of these config options have changed - if (c != null) { - c.client.close(); - } + String displayName = cloud.getDisplayName(); + final Client c = clients.getIfPresent(displayName); + if (c == null) { KubernetesClient client = new KubernetesFactoryAdapter(cloud.getServerUrl(), cloud.getNamespace(), cloud.getServerCertificate(), cloud.getCredentialsId(), cloud.isSkipTlsVerify(), cloud.getConnectTimeout(), cloud.getReadTimeout(), cloud.getMaxRequestsPerHost()).createClient(); - clients.put(cloud.getDisplayName(), new Client(validity, client)); - + clients.put(displayName, new Client(getValidity(cloud), client)); return client; } + return c.getClient(); + } + + private static int getValidity(KubernetesCloud cloud) { + return Objects.hashCode(cloud.getServerUrl(), cloud.getNamespace(), cloud.getServerCertificate(), + cloud.getCredentialsId(), cloud.isSkipTlsVerify(), cloud.getConnectTimeout(), cloud.getReadTimeout(), + cloud.getMaxRequestsPerHostStr()); } private static class Client { @@ -85,4 +103,76 @@ public int getValidity() { } } + @Extension + public static class PurgeExpiredKubernetesClients extends AsyncPeriodicWork { + + public PurgeExpiredKubernetesClients() { + super("Purge expired KubernetesClients"); + } + + @Override + public long getRecurrencePeriod() { + return TimeUnit.MINUTES.toMillis(1); + } + + @Override + protected Level getNormalLoggingLevel() { + return Level.FINE; + } + + @Override + protected void execute(TaskListener listener) { + for (Iterator it = expiredClients.iterator(); it.hasNext();) { + KubernetesClient client = it.next(); + if (client instanceof HttpClientAware) { + if (gracefulClose(client, ((HttpClientAware) client).getHttpClient())) { + it.remove(); + } + } else { + LOGGER.log(Level.WARNING, "{0} is not {1}, forcing close", new Object[] {client.toString(), HttpClientAware.class.getSimpleName()}); + client.close(); + it.remove(); + } + } + } + } + + private static boolean gracefulClose(KubernetesClient client, OkHttpClient httpClient) { + Dispatcher dispatcher = httpClient.dispatcher(); + // Remove the client if there are no more users + int runningCallsCount = dispatcher.runningCallsCount(); + int queuedCallsCount = dispatcher.queuedCallsCount(); + if (runningCallsCount == 0 && queuedCallsCount == 0) { + LOGGER.log(Level.INFO, "Closing {0}", client.toString()); + client.close(); + return true; + } else { + LOGGER.log(Level.INFO, "Not closing {0}: there are still running ({1}) or queued ({2}) calls", new Object[] {client.toString(), runningCallsCount, queuedCallsCount}); + return false; + } + } + + @Extension + public static class SaveableListenerImpl extends SaveableListener { + @Override + public void onChange(Saveable o, XmlFile file) { + if (o instanceof Jenkins) { + Jenkins jenkins = (Jenkins) o; + Set cloudDisplayNames = new HashSet<>(clients.asMap().keySet()); + for (KubernetesCloud cloud : jenkins.clouds.getAll(KubernetesCloud.class)) { + String displayName = cloud.getDisplayName(); + Client client = clients.getIfPresent(displayName); + if (client != null && client.getValidity() == getValidity(cloud)) { + cloudDisplayNames.remove(displayName); + } + } + // Remove missing / invalid clients + for (String displayName : cloudDisplayNames) { + clients.invalidate(displayName); + } + } + super.onChange(o, file); + } + } + }