Skip to content

Commit

Permalink
Remove buildx cache. Do not delete builder instances after goal. Use …
Browse files Browse the repository at this point in the history
…builder instance to cache artifacts (#1579)

closes #1576

Co-authored-by: Rohan Kumar <[email protected]>
  • Loading branch information
chonton and rohanKanojia authored Jul 9, 2022
1 parent db7a69b commit df4d7d0
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 88 deletions.
17 changes: 9 additions & 8 deletions src/main/asciidoc/inc/build/_buildx.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,19 @@ The `<buildx>` element within `<build>` defines how to build multi-architecture
| Element | Description

| *builderName*
| Name of builder to use with buildx. If not supplied, a builder instance is dynamically created before each build and destroyed
after completion of build.

| *cache*
| Cache directory name, which is `cache` by default. This directory is used to cache image layers. If the directory name is relative,
the cache directory is resolved relative to `outputDirectory`. If the cache name starts with `~/`, the cache directory is relative
to the user's home directory.
| Name of builder to use with buildx. If not supplied, the builder is named `maven`. The builder is created as necessary.
The builder manages the build cache.

| *configFile*
| Configuration file for builder. Non-absolute files are relative to the maven project directory. If the cache name starts with
| Configuration file for builder. Non-absolute files are relative to the maven project directory. If configFile starts with
`~/`, the configuration file is relative to the user's home directory.

| *dockerStateDir*
| State directory for docker builder. This directory holds docker builder configurations and context state. Sharing a state
directory across builds will share the cache and will decrease pull times.
Non-absolute files are relative to the maven project directory. If dockerConfigDir starts with `~/`, the configuration directory
is relative to the user's home directory.

| *platforms*
| A list of `<platform>` elements specifying platform to build. A platform has syntax of `OS/architecture` (e.g. linux/amd64, linux/arm64, darwin/amd64).

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,17 @@ public class BuildXConfiguration implements Serializable {
*/
@Parameter
private String builderName;

/**
* Location of docker cache
* Configuration file to create builder
*/
@Parameter
private String cache;
private String configFile;

/**
* Configuration file to create builder
* Location of docker state, including builder configurations
*/
@Parameter
private String configFile;
private String dockerStateDir;

/**
* List of platforms for multi-architecture build
Expand All @@ -42,14 +41,14 @@ public String getBuilderName() {
return builderName;
}

public String getCache() {
return cache;
}

public String getConfigFile() {
return configFile;
}

public String getDockerStateDir() {
return dockerStateDir;
}

public boolean isBuildX() {
return !getPlatforms().isEmpty();
}
Expand All @@ -72,17 +71,17 @@ public Builder builderName(String builderName) {
return this;
}

public Builder cache(String cache) {
config.cache = cache;
if (cache != null) {
public Builder configFile(String configFile) {
config.configFile = configFile;
if (configFile != null) {
isEmpty = false;
}
return this;
}

public Builder configFile(String configFile) {
config.configFile = configFile;
if (configFile != null) {
public Builder dockerStateDir(String dockerStateDir) {
config.dockerStateDir = dockerStateDir;
if (dockerStateDir != null) {
isEmpty = false;
}
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ public enum ConfigKey {
BUILD_OPTIONS,
BUILDX_PLATFORMS("buildx.platforms", ValueCombinePolicy.Merge),
BUILDX_BUILDERNAME("buildx.builderName"),
BUILDX_CACHE("buildx.cache"),
BUILDX_CONFIGFILE("buildx.configFile"),
BUILDX_DOCKERSTATEDIR("buildx.dockerStateDir"),
CAP_ADD,
CAP_DROP,
SYSCTLS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@ private BuildXConfiguration extractBuildx(BuildXConfiguration config, ValueProvi

return new BuildXConfiguration.Builder()
.builderName(valueProvider.getString(BUILDX_BUILDERNAME, config.getBuilderName()))
.cache(valueProvider.getString(BUILDX_CACHE, config.getCache()))
.configFile(valueProvider.getString(BUILDX_CONFIGFILE, config.getConfigFile()))
.dockerStateDir(valueProvider.getString(BUILDX_DOCKERSTATEDIR, config.getDockerStateDir()))
.platforms(valueProvider.getList(BUILDX_PLATFORMS, config.getPlatforms()))
.build();
}
Expand Down
73 changes: 28 additions & 45 deletions src/main/java/io/fabric8/maven/docker/service/BuildXService.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -49,33 +49,28 @@ public BuildXService(DockerAccess dockerAccess, DockerAssemblyManager dockerAsse
}

public void build(ProjectPaths projectPaths, ImageConfiguration imageConfig, AuthConfig authConfig, File buildArchive) throws MojoExecutionException {
createAndRemoveBuilder(projectPaths, imageConfig, authConfig, buildArchive, this::buildMultiPlatform);
useBuilder(projectPaths, imageConfig, authConfig, buildArchive, this::buildMultiPlatform);
}

public void push(ProjectPaths projectPaths, ImageConfiguration imageConfig, AuthConfig authConfig) throws MojoExecutionException {
BuildDirs buildDirs = new BuildDirs(projectPaths, imageConfig.getName());
File archive = new File(buildDirs.getTemporaryRootDirectory(), "docker-build.tar");
createAndRemoveBuilder(projectPaths, imageConfig, authConfig, archive, this::pushMultiPlatform);
useBuilder(projectPaths, imageConfig, authConfig, archive, this::pushMultiPlatform);
}

private <C> void createAndRemoveBuilder(ProjectPaths projectPaths, ImageConfiguration imageConfig, AuthConfig authConfig, C context, Builder<C> builder) throws MojoExecutionException {
private <C> void useBuilder(ProjectPaths projectPaths, ImageConfiguration imageConfig, AuthConfig authConfig, C context, Builder<C> builder) throws MojoExecutionException {
BuildDirs buildDirs = new BuildDirs(projectPaths, imageConfig.getName());

Path configPath = buildDirs.getBuildPath("docker");
createDirectory(configPath);
Path configPath = getDockerStateDir(imageConfig.getBuildConfiguration(), buildDirs);
List<String> buildX = Arrays.asList("docker", "--config", configPath.toString(), "buildx");

Map.Entry<String, Boolean> builderName = createBuilder(buildX, imageConfig, buildDirs);
String builderName = createBuilder(configPath, buildX, imageConfig, buildDirs);
Path configJson = configPath.resolve("config.json");
try {
Path configJson = configPath.resolve("config.json");
try {
createConfigJson(configJson, authConfig);
builder.useBuilder(buildX, builderName.getKey(), buildDirs, imageConfig, context);
} finally {
removeConfigJson(configJson);
}
createConfigJson(configJson, authConfig);
builder.useBuilder(buildX, builderName, buildDirs, imageConfig, context);
} finally {
removeBuilder(buildX, builderName);
removeConfigJson(configJson);
}
}

Expand Down Expand Up @@ -144,10 +139,6 @@ private void buildX(List<String> buildX, String builderName, BuildDirs buildDirs
cmdLine.add(extraParam);
}

String cacheDir = getCacheDir(buildConfiguration, buildDirs);
cmdLine.add("--cache-to=type=local,dest=" + cacheDir);
cmdLine.add("--cache-from=type=local,src=" + cacheDir);

String[] ctxCmds;
File contextDir = buildConfiguration.getContextDir();
if (contextDir != null) {
Expand Down Expand Up @@ -180,11 +171,11 @@ private Path getContextPath(File buildArchive, boolean extract) throws MojoExecu
return destinationPath;
}

private String getCacheDir(BuildImageConfiguration buildConfiguration, BuildDirs buildDirs) {
String cache = buildConfiguration.getBuildX().getCache();
Path cachePath = buildDirs.getBuildPath(cache != null ? EnvUtil.resolveHomeReference(cache) : "cache");
createDirectory(cachePath);
return cachePath.toString();
private Path getDockerStateDir(BuildImageConfiguration buildConfiguration, BuildDirs buildDirs) {
String stateDir = buildConfiguration.getBuildX().getDockerStateDir();
Path dockerStatePath = buildDirs.getBuildPath(stateDir != null ? EnvUtil.resolveHomeReference(stateDir) : "docker");
createDirectory(dockerStatePath);
return dockerStatePath;
}

private void createDirectory(Path cachePath) {
Expand All @@ -195,33 +186,25 @@ private void createDirectory(Path cachePath) {
}
}

private Map.Entry<String, Boolean> createBuilder(List<String> buildX, ImageConfiguration imageConfig, BuildDirs buildDirs) throws MojoExecutionException {
private String createBuilder(Path configPath, List<String> buildX, ImageConfiguration imageConfig, BuildDirs buildDirs) throws MojoExecutionException {
BuildXConfiguration buildXConfiguration = imageConfig.getBuildConfiguration().getBuildX();
String configuredBuilder = buildXConfiguration.getBuilderName();
if (configuredBuilder != null) {
return new AbstractMap.SimpleEntry<>(configuredBuilder, false);
String builderName = buildXConfiguration.getBuilderName();
if (builderName == null) {
builderName= "maven";
}

String builderName = "dmp_" + buildDirs.getBuildTopDir().replace(File.separatorChar, '_');
String buildConfig = buildXConfiguration.getConfigFile();
int rc = exec.process(buildX, buildConfig == null
? new String[] { "create", "--driver", "docker-container", "--name", builderName }
: new String[] { "create", "--driver", "docker-container", "--name", builderName, "--config",
Path builderPath = configPath.resolve(Paths.get("buildx", "instances", builderName));
if(Files.notExists(builderPath)) {
String buildConfig = buildXConfiguration.getConfigFile();
int rc = exec.process(buildX, buildConfig == null
? new String[] { "create", "--driver", "docker-container", "--name", builderName }
: new String[] { "create", "--driver", "docker-container", "--name", builderName, "--config",
buildDirs.getProjectPath(EnvUtil.resolveHomeReference(buildConfig)).toString() }
);
if (rc != 0) {
throw new MojoExecutionException("Error status (" + rc + ") while creating builder " + builderName);
}
return new AbstractMap.SimpleEntry<>(builderName, true);
}

private void removeBuilder(List<String> buildX, Map.Entry<String, Boolean> builderName) throws MojoExecutionException {
if (Boolean.TRUE.equals(builderName.getValue())) {
int rc = exec.process(buildX, "rm", builderName.getKey());
);
if (rc != 0) {
logger.warn("Ignoring non-zero status (" + rc + ") while removing builder " + builderName);
throw new MojoExecutionException("Error status (" + rc + ") while creating builder " + builderName);
}
}
return builderName;
}

interface Builder<C> {
Expand Down
12 changes: 3 additions & 9 deletions src/test/java/io/fabric8/maven/docker/BuildMojoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,9 @@ private void verifyBuild(int wantedNumberOfInvocations) throws DockerAccessExcep
private void thenBuildxRun(String relativeConfigFile, String contextDir, boolean nativePlatformIncluded) throws MojoExecutionException {
Path buildPath = projectBaseDirectory.toPath().resolve("target/docker/example/latest");
String config = getOsDependentBuild(buildPath, "docker");
String cacheDir = getOsDependentBuild(buildPath, "cache");
String buildDir = getOsDependentBuild(buildPath, "build");
String configFile = relativeConfigFile != null ? getOsDependentBuild(projectBaseDirectory.toPath(), relativeConfigFile) : null;
String builderName = "dmp_example_latest";
String builderName = "maven";

String[] cfgCmdLine = configFile == null
? new String[] { "create", "--driver", "docker-container", "--name", builderName }
Expand All @@ -174,19 +173,14 @@ private void thenBuildxRun(String relativeConfigFile, String contextDir, boolean
String platforms = nativePlatformIncluded ? NATIVE_PLATFORM + "," + NON_NATIVE_PLATFORM : NON_NATIVE_PLATFORM;
Mockito.verify(exec).process(Arrays.asList("docker", "--config", config, "buildx",
"build", "--progress=plain", "--builder", builderName,
"--platform", platforms, "--tag", "example:latest", "--build-arg", "foo=bar",
"--cache-to=type=local,dest=" + cacheDir, "--cache-from=type=local,src=" + cacheDir), ctxCmdLine);
"--platform", platforms, "--tag", "example:latest", "--build-arg", "foo=bar"), ctxCmdLine);

if (nativePlatformIncluded) {
Mockito.verify(exec).process(Arrays.asList("docker", "--config", config, "buildx",
"build", "--progress=plain", "--builder", builderName,
"--platform", NATIVE_PLATFORM, "--tag", "example:latest", "--build-arg", "foo=bar",
"--load",
"--cache-to=type=local,dest=" + cacheDir, "--cache-from=type=local,src=" + cacheDir), ctxCmdLine);
"--load"), ctxCmdLine);
}

Mockito.verify(exec).process(Arrays.asList("docker", "--config", config, "buildx"),
"rm", builderName);
}

private void givenPackaging(String packaging) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,8 @@ private void thenImageHasBeenPushed() throws DockerAccessException {
private void thenBuildxImageHasBeenPushed(String providedBuilder, String relativeDockerfile, boolean tag) throws MojoExecutionException {
Path buildPath = projectBaseDir.toPath().resolve("target/docker").resolve("user/test/1.0.1");
String config = getOsDependentBuild(buildPath, "docker");
String cacheDir = getOsDependentBuild(buildPath, "cache");
String buildDir = getOsDependentBuild(buildPath, "build");
String builderName = providedBuilder != null ? providedBuilder : "dmp_user_test_1.0.1";
String builderName = providedBuilder != null ? providedBuilder : "maven";

if (providedBuilder == null) {
Mockito.verify(exec).process(Arrays.asList("docker", "--config", config, "buildx"),
Expand All @@ -370,8 +369,7 @@ private void thenBuildxImageHasBeenPushed(String providedBuilder, String relativ
if (tag) {
args.addAll(Arrays.asList("--tag", "user/test:perri-air"));
}
args.addAll(Arrays.asList("--tag", "user/test:1.0.1", "--push",
"--cache-to=type=local,dest=" + cacheDir, "--cache-from=type=local,src=" + cacheDir));
args.addAll(Arrays.asList("--tag", "user/test:1.0.1", "--push"));

String[] cmds;
if (relativeDockerfile != null) {
Expand All @@ -382,11 +380,6 @@ private void thenBuildxImageHasBeenPushed(String providedBuilder, String relativ
}

Mockito.verify(exec).process(args, cmds);

if (providedBuilder == null) {
Mockito.verify(exec).process(Arrays.asList("docker", "--config", config, "buildx"),
"rm", builderName);
}
}

private void thenImageHasBeenTagged() throws DockerAccessException {
Expand Down

0 comments on commit df4d7d0

Please sign in to comment.