Cache dockerClient so that we're not creating a new one for every tas…

…k executed and ensure as part of shutdown we close said client. Added documentation. Refactor methods around to better show the flow of the file itself.
bmuschko · Mar 9, 2018 · b30bfb0 · b30bfb0
1 parent fd785b7
commit b30bfb0
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 72 deletions.
diff --git a/...ovy/com/bmuschko/gradle/docker/utils/DockerThreadContextClassLoaderIntegrationTest.groovy b/...ovy/com/bmuschko/gradle/docker/utils/DockerThreadContextClassLoaderIntegrationTest.groovy
@@ -370,25 +370,6 @@ class DockerThreadContextClassLoaderIntegrationTest extends AbstractIntegrationT
         instance
     }
 
-    @Unroll
-    def "Supports Docker host URL in format #config.url"(DockerClientConfiguration config, String expectedScheme) {
-
-        when:
-        threadContextClassLoader.withClasspath(project.configurations.dockerJava.files, config) { dockerClient ->
-            assert dockerClient.dockerClientConfig.dockerHost.scheme == expectedScheme
-        }
-
-        then:
-        noExceptionThrown()
-
-        where:
-        config                                                            | expectedScheme
-        new DockerClientConfiguration(url: 'tcp://localhost:2375')        | 'tcp'
-        new DockerClientConfiguration(url: 'http://localhost:2375')       | 'tcp'
-        new DockerClientConfiguration(url: 'https://localhost:2375')      | 'tcp'
-        new DockerClientConfiguration(url: 'unix:///var/run/docker.sock') | 'unix'
-    }
-
     private DockerRegistryCredentials createCredentials() {
         DockerRegistryCredentials credentials = new DockerRegistryCredentials()
 

diff --git a/src/main/groovy/com/bmuschko/gradle/docker/utils/DockerThreadContextClassLoader.groovy b/src/main/groovy/com/bmuschko/gradle/docker/utils/DockerThreadContextClassLoader.groovy
@@ -19,13 +19,15 @@ import com.bmuschko.gradle.docker.DockerExtension
 import com.bmuschko.gradle.docker.DockerRegistryCredentials
 import com.bmuschko.gradle.docker.tasks.DockerClientConfiguration
 import com.bmuschko.gradle.docker.tasks.container.DockerCreateContainer
+import groovy.transform.Synchronized
 import org.gradle.api.logging.Logger
 
 import java.lang.reflect.Array
 import java.lang.reflect.Constructor
 import java.lang.reflect.InvocationTargetException
 import java.lang.reflect.Method
 
+@SuppressWarnings(['FieldTypeRequired', 'UnnecessaryDefInFieldDeclaration', 'Indentation'])
 class DockerThreadContextClassLoader implements ThreadContextClassLoader {
     public static final String CORE_PACKAGE = 'com.github.dockerjava.core'
     public static final String MODEL_PACKAGE = 'com.github.dockerjava.api.model'
@@ -40,31 +42,71 @@ class DockerThreadContextClassLoader implements ThreadContextClassLoader {
     private static final String ON_NEXT_METHOD_NAME = 'onNext'
 
     private final DockerExtension dockerExtension
+    private def dockerClient
 
-    DockerThreadContextClassLoader(DockerExtension dockerExtension) {
+    DockerThreadContextClassLoader(final DockerExtension dockerExtension) {
         this.dockerExtension = dockerExtension
     }
 
     /**
      * {@inheritDoc}
      */
     @Override
-    void withClasspath(Set<File> classpath, DockerClientConfiguration dockerClientConfiguration, Closure closure) {
-        ClassLoader originalClassLoader = getClass().classLoader
+    void withClasspath(final Set<File> classpath, final DockerClientConfiguration dockerClientConfiguration, final Closure closure) {
+        closure.resolveStrategy = Closure.DELEGATE_FIRST
+        closure.delegate = this
+        closure(getDockerClient(dockerClientConfiguration,
+                classpath ?: dockerExtension.classpath?.files))
+    }
+
+    /**
+     * Get, and possibly create, DockerClient.
+     *
+     * @param dockerClientConfiguration Docker client configuration
+     * @param classpathFiles set of files containing DockerClient jars
+     * @return DockerClient instance
+     */
+    @Synchronized
+    private def getDockerClient(DockerClientConfiguration dockerClientConfiguration, Set<File> classpathFiles) {
+        if (!dockerClient) {
+            loadClasses(classpathFiles, this.class.classLoader)
 
-        try {
             String dockerUrl = getDockerHostUrl(dockerClientConfiguration)
-            File certPath = dockerClientConfiguration.certPath ?: dockerExtension.certPath
+            File dockerCertPath = dockerClientConfiguration.certPath ?: dockerExtension.certPath
             String apiVersion = dockerClientConfiguration.apiVersion ?: dockerExtension.apiVersion
 
-            Thread.currentThread().contextClassLoader = createClassLoader(classpath ?: dockerExtension.classpath?.files)
-            closure.resolveStrategy = Closure.DELEGATE_FIRST
-            closure.delegate = this
-            closure(getDockerClient(dockerUrl, certPath, apiVersion))
-        }
-        finally {
-            Thread.currentThread().contextClassLoader = originalClassLoader
+            // Create configuration
+            Class dockerClientConfigClass = loadClass("${CORE_PACKAGE}.DockerClientConfig")
+            Class dockerClientConfigClassImpl = loadClass("${CORE_PACKAGE}.DefaultDockerClientConfig")
+            Method dockerClientConfigMethod = dockerClientConfigClassImpl.getMethod('createDefaultConfigBuilder')
+            def dockerClientConfigBuilder = dockerClientConfigMethod.invoke(null)
+            dockerClientConfigBuilder.withDockerHost(dockerUrl)
+
+            if (dockerCertPath) {
+                dockerClientConfigBuilder.withDockerTlsVerify(true)
+                dockerClientConfigBuilder.withDockerCertPath(dockerCertPath.canonicalPath)
+            } else {
+                dockerClientConfigBuilder.withDockerTlsVerify(false)
+            }
+
+            if (apiVersion) {
+                dockerClientConfigBuilder.withApiVersion(apiVersion)
+            }
+
+            def dockerClientConfig = dockerClientConfigBuilder.build()
+
+            // Create client
+            Class dockerClientBuilderClass = loadClass("${CORE_PACKAGE}.DockerClientBuilder")
+            Method method = dockerClientBuilderClass.getMethod('getInstance', dockerClientConfigClass)
+            def dockerClientBuilder = method.invoke(null, dockerClientConfig)
+            dockerClient = dockerClientBuilder.build()
+
+            // register shutdown-hook to close kubernetes client.
+            addShutdownHook {
+                dockerClient.close()
+            }
         }
+        dockerClient
     }
 
     /**
@@ -80,13 +122,15 @@ class DockerThreadContextClassLoader implements ThreadContextClassLoader {
     }
 
     /**
-     * Creates the classloader with the given classpath files.
-     *
-     * @param classpathFiles Classpath files
-     * @return URL classloader
-     */
-    private URLClassLoader createClassLoader(Set<File> classpathFiles) {
-        new URLClassLoader(toURLArray(classpathFiles), ClassLoader.systemClassLoader.parent)
+     * Load set of files into an arbitrary ClassLoader.
+     * 
+     * @param classpathFiles set of files to load
+     * @param loader ClassLoader to load files into
+     */ 
+    private loadClasses(final Set<File> classpathFiles, final ClassLoader loader) {
+        toURLArray(classpathFiles).each { url ->
+            loader.addURL(url)
+        }
     }
 
     /**
@@ -99,40 +143,6 @@ class DockerThreadContextClassLoader implements ThreadContextClassLoader {
         files.collect { file -> file.toURI().toURL() } as URL[]
     }
 
-    /**
-     * Creates DockerClient from ClassLoader.
-     *
-     * @param dockerClientConfiguration Docker client configuration
-     * @return DockerClient instance
-     */
-    private getDockerClient(String dockerUrl, File dockerCertPath, String apiVersion) {
-        // Create configuration
-        Class dockerClientConfigClass = loadClass("${CORE_PACKAGE}.DockerClientConfig")
-        Class dockerClientConfigClassImpl = loadClass("${CORE_PACKAGE}.DefaultDockerClientConfig")
-        Method dockerClientConfigMethod = dockerClientConfigClassImpl.getMethod('createDefaultConfigBuilder')
-        def dockerClientConfigBuilder = dockerClientConfigMethod.invoke(null)
-        dockerClientConfigBuilder.withDockerHost(dockerUrl)
-
-        if (dockerCertPath) {
-            dockerClientConfigBuilder.withDockerTlsVerify(true)
-            dockerClientConfigBuilder.withDockerCertPath(dockerCertPath.canonicalPath)
-        } else {
-            dockerClientConfigBuilder.withDockerTlsVerify(false)
-        }
-
-        if (apiVersion) {
-            dockerClientConfigBuilder.withApiVersion(apiVersion)
-        }
-
-        def dockerClientConfig = dockerClientConfigBuilder.build()
-
-        // Create client
-        Class dockerClientBuilderClass = loadClass("${CORE_PACKAGE}.DockerClientBuilder")
-        Method method = dockerClientBuilderClass.getMethod('getInstance', dockerClientConfigClass)
-        def dockerClientBuilder = method.invoke(null, dockerClientConfig)
-        dockerClientBuilder.build()
-    }
-
     /**
      * {@inheritDoc}
      */