From b30bfb0d80500026dba962e6fbce4e502a1f4b90 Mon Sep 17 00:00:00 2001 From: dancc Date: Mon, 5 Mar 2018 22:04:07 -0500 Subject: [PATCH] Cache dockerClient so that we're not creating a new one for every task 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. --- ...adContextClassLoaderIntegrationTest.groovy | 19 --- .../DockerThreadContextClassLoader.groovy | 116 ++++++++++-------- 2 files changed, 63 insertions(+), 72 deletions(-) diff --git a/src/integTest/groovy/com/bmuschko/gradle/docker/utils/DockerThreadContextClassLoaderIntegrationTest.groovy b/src/integTest/groovy/com/bmuschko/gradle/docker/utils/DockerThreadContextClassLoaderIntegrationTest.groovy index c5aad9c0a..7e0f79d20 100644 --- a/src/integTest/groovy/com/bmuschko/gradle/docker/utils/DockerThreadContextClassLoaderIntegrationTest.groovy +++ b/src/integTest/groovy/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 index 5c7aff05f..c7779f66a 100644 --- a/src/main/groovy/com/bmuschko/gradle/docker/utils/DockerThreadContextClassLoader.groovy +++ b/src/main/groovy/com/bmuschko/gradle/docker/utils/DockerThreadContextClassLoader.groovy @@ -19,6 +19,7 @@ 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 @@ -26,6 +27,7 @@ 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,8 +42,9 @@ 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 } @@ -49,22 +52,61 @@ class DockerThreadContextClassLoader implements ThreadContextClassLoader { * {@inheritDoc} */ @Override - void withClasspath(Set classpath, DockerClientConfiguration dockerClientConfiguration, Closure closure) { - ClassLoader originalClassLoader = getClass().classLoader + void withClasspath(final Set 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 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 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 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} */