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

Remove buildx cache. Do not delete builder instances after goal. Us… #1579

Merged
merged 2 commits into from
Jul 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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