Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make DockerRemoteApi plugin configuration cache compatible for Gradle 8.1+ #1193

Merged
merged 8 commits into from
Sep 20, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ abstract class AbstractFunctionalTest extends Specification {
File buildFile
File settingsFile
Map<String, String> envVars = new HashMap<>()
String version

def setup() {
projectDir = temporaryFolder
Expand All @@ -42,6 +43,14 @@ abstract class AbstractFunctionalTest extends Specification {
envVars.put(variable, value)
}

/**
* Set a specific Gradle version for gradle runner
* @param version version string
*/
protected void useGradleVersion(String version) {
this.version = version
}

protected BuildResult build(String... arguments) {
createAndConfigureGradleRunner(arguments).build()
}
Expand All @@ -51,11 +60,15 @@ abstract class AbstractFunctionalTest extends Specification {
}

private GradleRunner createAndConfigureGradleRunner(String... arguments) {
GradleRunner.create()
def gradleRunner = GradleRunner.create()
.withProjectDir(projectDir)
.withArguments(arguments + '-s' + '--configuration-cache' as List<String>)
.withPluginClasspath()
.withEnvironment(envVars)
if (version) {
gradleRunner.withGradleVersion(version)
}
return gradleRunner
}

protected File file(String relativePath) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.bmuschko.gradle.docker

import org.gradle.testkit.runner.BuildResult
import spock.lang.Ignore
import spock.lang.IgnoreIf
import spock.lang.Requires

import static com.bmuschko.gradle.docker.fixtures.DockerConventionPluginFixture.groovySettingsFile
Expand Down Expand Up @@ -114,7 +115,7 @@ class DockerRemoteApiPluginFunctionalTest extends AbstractGroovyDslFunctionalTes
import com.bmuschko.gradle.docker.tasks.image.DockerPullImage
import com.bmuschko.gradle.docker.tasks.image.DockerRemoveImage
import com.bmuschko.gradle.docker.tasks.image.Dockerfile

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto formatting, is this OK to remove @bmuschko?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. Seem like a tab or tabs ended up here.

task dockerfile(type: Dockerfile) {
from '$TEST_IMAGE_WITH_TAG'
runCommand("echo ${UUID.randomUUID()}")
Expand Down Expand Up @@ -261,6 +262,38 @@ class DockerRemoteApiPluginFunctionalTest extends AbstractGroovyDslFunctionalTes
build('convert')
}

@IgnoreIf({ os.windows })
def "configuration cache compatible when docker state changes on disk for OS #osName"() {
Jocce-Nilsson marked this conversation as resolved.
Show resolved Hide resolved
given:
useGradleVersion("8.3")

and:
// Force use of user.home and define a variable holding the file
def home = new File(temporaryFolder, "home").tap{it.mkdirs()}
def dockerSockDirectory = new File(home, "/.docker/run/").tap{it.mkdirs()}
String[] arguments = [
"-Dcom.bmuschko.gradle.docker.internal.DefaultDockerUrlValueSource.skipCheckOfVarRun=true",
Jocce-Nilsson marked this conversation as resolved.
Show resolved Hide resolved
"-Duser.home=${home.absolutePath}",
"-Dos.name=$osName",
"help"
].toArray()
Jocce-Nilsson marked this conversation as resolved.
Show resolved Hide resolved

when: "First run will save configuration cache state, including traversed files. Create a file to be checked by plugin."
def dockerSockFile = new File(dockerSockDirectory, "docker.sock")
dockerSockFile.createNewFile()
build(arguments)

and: "Second run is investigating cache input, including traversed files, in order to determine if configuration cache is reused"
dockerSockFile.delete()
def output = build(arguments).output

then:
output.contains("Configuration cache entry reused")

where:
osName << ['Mac OS X', 'Linux']
}

@Ignore
@Requires({ TestPrecondition.HARBOR_CREDENTIALS_AVAILABLE })
def "can push image to Harbor"() {
Expand All @@ -272,11 +305,11 @@ class DockerRemoteApiPluginFunctionalTest extends AbstractGroovyDslFunctionalTes
import com.bmuschko.gradle.docker.tasks.image.DockerBuildImage
import com.bmuschko.gradle.docker.tasks.image.DockerPushImage
import com.bmuschko.gradle.docker.tasks.image.DockerRemoveImage

task pullImage(type: DockerPullImage) {
image = '$AbstractFunctionalTest.TEST_IMAGE_WITH_TAG'
}

task dockerfile(type: Dockerfile) {
dependsOn pullImage
from '$AbstractFunctionalTest.TEST_IMAGE_WITH_TAG'
Expand All @@ -286,7 +319,7 @@ class DockerRemoteApiPluginFunctionalTest extends AbstractGroovyDslFunctionalTes
dependsOn dockerfile
images = ['demo.goharbor.io/gradle-docker-plugin/$AbstractFunctionalTest.TEST_IMAGE_WITH_TAG']
}

task removeImage(type: DockerRemoveImage) {
targetImageId buildImage.imageId
force = true
Expand All @@ -296,7 +329,7 @@ class DockerRemoteApiPluginFunctionalTest extends AbstractGroovyDslFunctionalTes
dependsOn buildImage
finalizedBy removeImage
images = buildImage.images

registryCredentials {
url = 'https://demo.goharbor.io/v2/'
username = '$credentials.username'
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/com/bmuschko/gradle/docker/DockerExtension.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package com.bmuschko.gradle.docker;

import com.bmuschko.gradle.docker.internal.DefaultDockerConfigResolver;
import com.bmuschko.gradle.docker.internal.DefaultDockerUrlValueSource;
import com.bmuschko.gradle.docker.internal.DockerConfigResolver;
import org.gradle.api.Action;
import org.gradle.api.XmlProvider;
import org.gradle.api.file.DirectoryProperty;
import org.gradle.api.model.ObjectFactory;
import org.gradle.api.provider.Property;
import org.gradle.api.provider.ProviderFactory;
import org.gradle.api.provider.ValueSourceParameters;

import java.io.File;

Expand Down Expand Up @@ -63,11 +67,11 @@ public final DockerRegistryCredentials getRegistryCredentials() {

private final DockerRegistryCredentials registryCredentials;

public DockerExtension(ObjectFactory objectFactory) {
public DockerExtension(ObjectFactory objectFactory, ProviderFactory providerFactory) {
DockerConfigResolver dockerConfigResolver = new DefaultDockerConfigResolver();

url = objectFactory.property(String.class);
url.convention(dockerConfigResolver.getDefaultDockerUrl());
url.convention(providerFactory.of(DefaultDockerUrlValueSource.class, noneValueSourceSpec -> {}));
certPath = objectFactory.directoryProperty();

File defaultDockerCert = dockerConfigResolver.getDefaultDockerCert();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class DockerRemoteApiPlugin implements Plugin<Project> {

@Override
public void apply(Project project) {
final DockerExtension dockerExtension = project.getExtensions().create(EXTENSION_NAME, DockerExtension.class, project.getObjects());
final DockerExtension dockerExtension = project.getExtensions().create(EXTENSION_NAME, DockerExtension.class, project.getObjects(), project.getProviders());
configureRegistryCredentialsAwareTasks(project, dockerExtension.getRegistryCredentials());

final Provider<DockerClientService> serviceProvider = project.getGradle().getSharedServices().registerIfAbsent("docker", DockerClientService.class, new Action<BuildServiceSpec<DockerClientService.Params>>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,6 @@
import java.io.File;

public class DefaultDockerConfigResolver implements DockerConfigResolver {

private static final Logger logger = Logging.getLogger(DefaultDockerConfigResolver.class);

@Override
public String getDefaultDockerUrl() {
String dockerUrl = getEnv("DOCKER_HOST");
if (dockerUrl == null) {
if (OsUtils.isWindows()) {
if (isFileExists("\\\\.\\pipe\\docker_engine")) {
dockerUrl = "npipe:////./pipe/docker_engine";
}
} else {
// macOS or Linux
if (isFileExists("/var/run/docker.sock")) {
dockerUrl = "unix:///var/run/docker.sock";
} else if (isFileExists(System.getProperty("user.home") + "/.docker/run/docker.sock")) {
dockerUrl = "unix://" + System.getProperty("user.home") + "/.docker/run/docker.sock";
}
}


if (dockerUrl == null) {
dockerUrl = "tcp://127.0.0.1:2375";
}
}

logger.info("Default docker.url set to " + dockerUrl);
return dockerUrl;
}

@Nullable
@Override
public File getDefaultDockerCert() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package com.bmuschko.gradle.docker.internal;

import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.api.provider.ValueSource;
import org.gradle.api.provider.ValueSourceParameters;

import javax.annotation.Nullable;
import java.io.File;

public abstract class DefaultDockerUrlValueSource implements ValueSource<String, ValueSourceParameters.None> {
Jocce-Nilsson marked this conversation as resolved.
Show resolved Hide resolved

private static final Logger logger = Logging.getLogger(DefaultDockerUrlValueSource.class);

@Override
public String obtain() {
String dockerUrl = getEnv("DOCKER_HOST");
if (dockerUrl == null) {
if (OsUtils.isWindows()) {
if (isFileExists("\\\\.\\pipe\\docker_engine")) {
dockerUrl = "npipe:////./pipe/docker_engine";
}
} else {
//Added for possibility to run integration tests, also makes it possible to override use of /var/run/
boolean skipVarRun = Boolean.valueOf(System.getProperty("com.bmuschko.gradle.docker.internal.DefaultDockerUrlValueSource.skipCheckOfVarRun", "false"));
// macOS or Linux
if (isFileExists("/var/run/docker.sock") && !skipVarRun) {
dockerUrl = "unix:///var/run/docker.sock";
} else if (isFileExists(System.getProperty("user.home") + "/.docker/run/docker.sock")) {
dockerUrl = "unix://" + System.getProperty("user.home") + "/.docker/run/docker.sock";
}
}

Jocce-Nilsson marked this conversation as resolved.
Show resolved Hide resolved

if (dockerUrl == null) {
dockerUrl = "tcp://127.0.0.1:2375";
}
}

logger.info("Default docker.url set to " + dockerUrl);
return dockerUrl;
}

@Nullable
String getEnv(String name) {
return System.getenv(name);
}

boolean isFileExists(String path) {
return new File(path).exists();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,5 @@
import java.io.File;

public interface DockerConfigResolver {
String getDefaultDockerUrl();

File getDefaultDockerCert();
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,82 +13,6 @@ class DefaultDockerConfigResolverTest extends Specification {
@TempDir
Path temporaryFolder

def "returns DOCKER_HOST value if env is set"() {
given:
def dockerHostEnv = 'unix:///var/run/test-docker.sock'

DefaultDockerConfigResolver dockerConfigResolver = Spy(DefaultDockerConfigResolver.class)
dockerConfigResolver.getEnv('DOCKER_HOST') >> dockerHostEnv

expect:
dockerConfigResolver.defaultDockerUrl == dockerHostEnv
}

@RestoreSystemProperties
@Unroll
def "returns default fallback docker url for all operation systems: #osName"() {
given:
System.setProperty('user.home', temporaryFolder.resolve('home').toAbsolutePath().toString())
System.setProperty('os.name', osName)

DefaultDockerConfigResolver dockerConfigResolver = Spy(DefaultDockerConfigResolver.class)
dockerConfigResolver.getEnv('DOCKER_HOST') >> null
dockerConfigResolver.isFileExists(_) >> false

expect:
dockerConfigResolver.defaultDockerUrl == 'tcp://127.0.0.1:2375'

where:
osName << ['Windows 10', 'Mac OS X', 'Linux']
}

@RestoreSystemProperties
def "returns npipe:////./pipe/docker_engine if file exists on Windows"() {
given:
System.setProperty('os.name', 'Windows 10')

DefaultDockerConfigResolver dockerConfigResolver = Spy(DefaultDockerConfigResolver.class)
dockerConfigResolver.getEnv('DOCKER_HOST') >> null
dockerConfigResolver.isFileExists('\\\\.\\pipe\\docker_engine') >> true

expect:
dockerConfigResolver.getDefaultDockerUrl() == 'npipe:////./pipe/docker_engine'
}

@RestoreSystemProperties
def "returns unix:///var/run/docker.sock url if file exists on #osName"() {
given:
System.setProperty('os.name', osName)

DefaultDockerConfigResolver dockerConfigResolver = Spy(DefaultDockerConfigResolver.class)
dockerConfigResolver.getEnv('DOCKER_HOST') >> null
dockerConfigResolver.isFileExists('/var/run/docker.sock') >> true

expect:
dockerConfigResolver.getDefaultDockerUrl() == 'unix:///var/run/docker.sock'

where:
osName << ['Mac OS X', 'Linux']
}

@RestoreSystemProperties
def "returns unix://{user.home}/.docker/run/docker.sock url if file exists on #osName"() {
given:
System.setProperty('os.name', osName)
System.setProperty('user.home', '/home/test')

DefaultDockerConfigResolver dockerConfigResolver = Spy(DefaultDockerConfigResolver.class)
dockerConfigResolver.getEnv('DOCKER_HOST') >> null
dockerConfigResolver.isFileExists('/var/run/docker.sock') >> false
dockerConfigResolver.isFileExists('/home/test/.docker/run/docker.sock') >> true

expect:
dockerConfigResolver.getDefaultDockerUrl() == "unix:///home/test/.docker/run/docker.sock"

where:
osName << ['Mac OS X', 'Linux']
}

def "returns null if DOCKER_CERT_PATH is not set"() {
given:
DefaultDockerConfigResolver dockerConfigResolver = Spy(DefaultDockerConfigResolver.class)
Expand Down
Loading
Loading