From e9b52de1d2860dbb02743f52a209d4f591fea375 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Wed, 27 Feb 2019 12:54:18 +0100 Subject: [PATCH] [JENKINS-55392] Do not close Kubernetes client as it is being cached and reused --- .../kubernetes/KubernetesClientProvider.java | 25 +++++++++++++------ .../pipeline/ContainerLogStepExecution.java | 8 +----- .../pipeline/ContainerStepExecution.java | 2 +- 3 files changed, 19 insertions(+), 16 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 c6205b5422..b9dff06a1f 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesClientProvider.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesClientProvider.java @@ -38,19 +38,24 @@ final class KubernetesClientProvider { 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); + /** + * How many clouds can we connect to, default to 10 + */ + private static final Integer CACHE_SIZE = Integer + .getInteger(KubernetesClientProvider.class.getPackage().getName() + ".clients.cacheSize", 10); + /** * Client expiration in seconds, default to one day */ - private static final Integer CACHE_EXPIRATION = Integer - .getInteger("org.csanchez.jenkins.plugins.kubernetes.clients.cacheExpiration", 24 * 60 * 60); + private static final Integer CACHE_EXPIRATION = Integer.getInteger( + KubernetesClientProvider.class.getPackage().getName() + ".clients.cacheExpiration", 24 * 60 * 60); private static final List expiredClients = Collections.synchronizedList(new ArrayList()); private static final Cache clients = CacheBuilder - .newBuilder() - .maximumSize(CACHE_SIZE) - .expireAfterWrite(CACHE_EXPIRATION, TimeUnit.SECONDS) + .newBuilder() // + .maximumSize(CACHE_SIZE) // + .expireAfterWrite(CACHE_EXPIRATION, TimeUnit.SECONDS) // .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(); @@ -65,7 +70,7 @@ final class KubernetesClientProvider { } } - }) + }) // .build(); private KubernetesClientProvider() { @@ -80,6 +85,7 @@ static KubernetesClient createClient(KubernetesCloud cloud) throws NoSuchAlgorit cloud.getServerCertificate(), cloud.getCredentialsId(), cloud.isSkipTlsVerify(), cloud.getConnectTimeout(), cloud.getReadTimeout(), cloud.getMaxRequestsPerHost()).createClient(); clients.put(displayName, new Client(getValidity(cloud), client)); + LOGGER.log(Level.INFO, "Created new Kubernetes client: {0} {1}", new Object[] { displayName, client }); return client; } return c.getClient(); @@ -123,7 +129,7 @@ public long getRecurrencePeriod() { @Override protected Level getNormalLoggingLevel() { - return Level.FINE; + return Level.FINEST; } @Override @@ -170,6 +176,9 @@ public void onChange(Saveable o, XmlFile file) { Client client = clients.getIfPresent(displayName); if (client != null && client.getValidity() == getValidity(cloud)) { cloudDisplayNames.remove(displayName); + } else { + LOGGER.log(Level.INFO, "Invalidating Kubernetes client: {0} {1}", + new Object[] { displayName, client }); } } // Remove missing / invalid clients diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java index 96a1da8584..9d0cc2f760 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStepExecution.java @@ -105,18 +105,12 @@ protected String run() throws Exception { logger().println(message); LOGGER.log(Level.WARNING, message, e); return ""; - } finally { - closeQuietly(getContext(), client); } } @Override public void stop(Throwable cause) throws Exception { LOGGER.log(Level.FINE, "Stopping container log step."); - try { - super.stop(cause); - } finally { - closeQuietly(getContext(), client); - } + super.stop(cause); } } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java index 0b6267495a..5000b94b95 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStepExecution.java @@ -107,7 +107,7 @@ public boolean start() throws Exception { @Override public void stop(@Nonnull Throwable cause) throws Exception { LOGGER.log(Level.FINE, "Stopping container step."); - closeQuietly(getContext(), client, decorator); + closeQuietly(getContext(), decorator); } private static class ContainerExecCallback extends BodyExecutionCallback.TailCall {