Skip to content

Commit

Permalink
fix (buildx) : Remove hardcoded --node value while creating buildx …
Browse files Browse the repository at this point in the history
…builder

Instead of using a hardcoded value, node should be configurable by new
configuration option buildx > nodeName

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
  • Loading branch information
rohanKanojia committed Aug 13, 2023
1 parent ff65263 commit a339536
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 9 deletions.
1 change: 1 addition & 0 deletions doc/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

* **0.44-SNAPSHOT** :
- Only add `--config` to buildx command string when authentication credentials are coming from outside sources
- Remove hardcoded `--node` value while creating buildx builder

* **0.43.2** (2023-07-29):
- Make `--config` from buildx command string generation optional ([1673](https://github.com/fabric8io/docker-maven-plugin/pull/1673)) @robfrank
Expand Down
3 changes: 3 additions & 0 deletions src/main/asciidoc/inc/build/_buildx.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ The `<buildx>` element within `<build>` defines how to build multi-architecture
| 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.

| *nodeName*
| Specify the name of the node to be created or modified. If none is specified, it is the name of the builder it belongs to, with an index `0` number suffix.

| *configFile*
| 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ public class BuildXConfiguration implements Serializable {
@Parameter
private AttestationConfiguration attestations;

@Parameter
private String nodeName;

public String getBuilderName() {
return builderName;
}
Expand All @@ -51,6 +54,10 @@ public String getDockerStateDir() {
return dockerStateDir;
}

public String getNodeName() {
return nodeName;
}

public boolean isBuildX() {
return !getPlatforms().isEmpty();
}
Expand Down Expand Up @@ -81,6 +88,14 @@ public Builder builderName(String builderName) {
return this;
}

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

public Builder configFile(String configFile) {
config.configFile = configFile;
if (configFile != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public enum ConfigKey {
BUILD_OPTIONS,
BUILDX_PLATFORMS("buildx.platforms", ValueCombinePolicy.Merge),
BUILDX_BUILDERNAME("buildx.builderName"),
BUILDX_NODENAME("buildx.nodeName"),
BUILDX_CONFIGFILE("buildx.configFile"),
BUILDX_DOCKERSTATEDIR("buildx.dockerStateDir"),
BUILDX_ATTESTATION_PROVENANCE("buildx.attestations.provenance"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ private BuildXConfiguration extractBuildx(BuildXConfiguration config, ValueProvi

return new BuildXConfiguration.Builder()
.builderName(valueProvider.getString(BUILDX_BUILDERNAME, config.getBuilderName()))
.nodeName(valueProvider.getString(BUILDX_NODENAME, config.getNodeName()))
.configFile(valueProvider.getString(BUILDX_CONFIGFILE, config.getConfigFile()))
.dockerStateDir(valueProvider.getString(BUILDX_DOCKERSTATEDIR, config.getDockerStateDir()))
.platforms(valueProvider.getList(BUILDX_PLATFORMS, config.getPlatforms()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;

import static io.fabric8.maven.docker.util.AuthConfigFactory.hasAuthForRegistryInDockerConfig;
Expand Down Expand Up @@ -217,14 +218,12 @@ protected void createDirectory(Path cachePath) {

protected String createBuilder(Path configPath, List<String> buildX, ImageConfiguration imageConfig, BuildDirs buildDirs) throws MojoExecutionException {
BuildXConfiguration buildXConfiguration = imageConfig.getBuildConfiguration().getBuildX();
String builderName = buildXConfiguration.getBuilderName();
if (builderName == null) {
builderName= "maven";
}
String builderName = Optional.ofNullable(buildXConfiguration.getBuilderName()).orElse("maven");
String nodeName = Optional.ofNullable(buildXConfiguration.getNodeName()).orElse(builderName + "0");
Path builderPath = configPath.resolve(Paths.get("buildx", "instances", builderName));
if(Files.notExists(builderPath)) {
List<String> cmds = new ArrayList<>(buildX);
append(cmds, "create", "--driver", "docker-container", "--name", builderName, "--node", builderName + "0");
append(cmds, "create", "--driver", "docker-container", "--name", builderName, "--node", nodeName);
String buildConfig = buildXConfiguration.getConfigFile();
if(buildConfig != null) {
append(cmds, "--config",
Expand Down
9 changes: 5 additions & 4 deletions src/test/java/io/fabric8/maven/docker/PushMojoBuildXTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ void execute_whenNoAuthConfig_thenRunBuildXCommandWithAddedConfig() throws MojoE
assertEquals(1, defaultExecMockedConstruction.constructed().size());
BuildXService.DefaultExec defaultExec = defaultExecMockedConstruction.constructed().get(0);
verify(defaultExec).process(Arrays.asList("docker", "buildx", "create",
"--driver", "docker-container", "--name", "testbuilder", "--node", "testbuilder0"));
"--driver", "docker-container", "--name", "testbuilder", "--node", "testnode"));
verify(defaultExec).process(Arrays.asList("docker", "buildx", "build",
"--progress=plain", "--builder", "testbuilder", "--platform", "linux/amd64,linux/arm64",
"--tag", "test.example.org/testuser/sample-test-image:latest",
Expand All @@ -137,7 +137,7 @@ void execute_whenAuthConfigFromLocalDockerConfig_thenDoNotAddConfigToDockerBuild
assertEquals(1, defaultExecMockedConstruction.constructed().size());
BuildXService.DefaultExec defaultExec = defaultExecMockedConstruction.constructed().get(0);
verify(defaultExec).process(Arrays.asList("docker", "buildx", "create",
"--driver", "docker-container", "--name", "testbuilder", "--node", "testbuilder0"));
"--driver", "docker-container", "--name", "testbuilder", "--node", "testnode"));
verify(defaultExec).process(Arrays.asList("docker", "buildx", "build",
"--progress=plain", "--builder", "testbuilder", "--platform", "linux/amd64,linux/arm64",
"--tag", "test.example.org/testuser/sample-test-image:latest",
Expand All @@ -162,7 +162,7 @@ void execute_whenAuthConfigFromMavenSettings_thenAddConfigToDockerBuildXCommand(
assertEquals(1, defaultExecMockedConstruction.constructed().size());
BuildXService.DefaultExec defaultExec = defaultExecMockedConstruction.constructed().get(0);
verify(defaultExec).process(Arrays.asList("docker", "--config", expectedDockerStateConfigDir.getAbsolutePath(), "buildx", "create",
"--driver", "docker-container", "--name", "testbuilder", "--node", "testbuilder0"));
"--driver", "docker-container", "--name", "testbuilder", "--node", "testnode"));
verify(defaultExec).process(Arrays.asList("docker", "--config", expectedDockerStateConfigDir.getAbsolutePath(), "buildx", "build",
"--progress=plain", "--builder", "testbuilder", "--platform", "linux/amd64,linux/arm64",
"--tag", "test.example.org/testuser/sample-test-image:latest",
Expand All @@ -184,7 +184,7 @@ void execute_whenAuthConfigFromProperties_thenAddConfigOptionToBuildXCommand() t
assertEquals(1, defaultExecMockedConstruction.constructed().size());
BuildXService.DefaultExec defaultExec = defaultExecMockedConstruction.constructed().get(0);
verify(defaultExec).process(Arrays.asList("docker", "--config", expectedDockerStateConfigDir.getAbsolutePath(), "buildx", "create",
"--driver", "docker-container", "--name", "testbuilder", "--node", "testbuilder0"));
"--driver", "docker-container", "--name", "testbuilder", "--node", "testnode"));
verify(defaultExec).process(Arrays.asList("docker", "--config", expectedDockerStateConfigDir.getAbsolutePath(), "buildx", "build",
"--progress=plain", "--builder", "testbuilder", "--platform", "linux/amd64,linux/arm64",
"--tag", "test.example.org/testuser/sample-test-image:latest",
Expand All @@ -203,6 +203,7 @@ private ImageConfiguration createImageConfiguration() {
.buildx(new BuildXConfiguration.Builder()
.platforms(Arrays.asList("linux/amd64", "linux/arm64"))
.builderName("testbuilder")
.nodeName("testnode")
.build())
.build())
.build();
Expand Down

0 comments on commit a339536

Please sign in to comment.