From a339536df08246483ccc56de59638783e77ab142 Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Sun, 13 Aug 2023 19:33:46 +0530 Subject: [PATCH] fix (buildx) : Remove hardcoded `--node` value while creating buildx builder Instead of using a hardcoded value, node should be configurable by new configuration option buildx > nodeName Signed-off-by: Rohan Kumar --- doc/changelog.md | 1 + src/main/asciidoc/inc/build/_buildx.adoc | 3 +++ .../maven/docker/config/BuildXConfiguration.java | 15 +++++++++++++++ .../docker/config/handler/property/ConfigKey.java | 1 + .../handler/property/PropertyConfigHandler.java | 1 + .../maven/docker/service/BuildXService.java | 9 ++++----- .../fabric8/maven/docker/PushMojoBuildXTest.java | 9 +++++---- 7 files changed, 30 insertions(+), 9 deletions(-) diff --git a/doc/changelog.md b/doc/changelog.md index 6279ee790..c302971b5 100644 --- a/doc/changelog.md +++ b/doc/changelog.md @@ -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 diff --git a/src/main/asciidoc/inc/build/_buildx.adoc b/src/main/asciidoc/inc/build/_buildx.adoc index 480d3ec9f..3e3a90477 100644 --- a/src/main/asciidoc/inc/build/_buildx.adoc +++ b/src/main/asciidoc/inc/build/_buildx.adoc @@ -31,6 +31,9 @@ The `` element within `` 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. diff --git a/src/main/java/io/fabric8/maven/docker/config/BuildXConfiguration.java b/src/main/java/io/fabric8/maven/docker/config/BuildXConfiguration.java index e746b436c..9d52376f4 100644 --- a/src/main/java/io/fabric8/maven/docker/config/BuildXConfiguration.java +++ b/src/main/java/io/fabric8/maven/docker/config/BuildXConfiguration.java @@ -39,6 +39,9 @@ public class BuildXConfiguration implements Serializable { @Parameter private AttestationConfiguration attestations; + @Parameter + private String nodeName; + public String getBuilderName() { return builderName; } @@ -51,6 +54,10 @@ public String getDockerStateDir() { return dockerStateDir; } + public String getNodeName() { + return nodeName; + } + public boolean isBuildX() { return !getPlatforms().isEmpty(); } @@ -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) { diff --git a/src/main/java/io/fabric8/maven/docker/config/handler/property/ConfigKey.java b/src/main/java/io/fabric8/maven/docker/config/handler/property/ConfigKey.java index 0810a94ac..08ab6a179 100644 --- a/src/main/java/io/fabric8/maven/docker/config/handler/property/ConfigKey.java +++ b/src/main/java/io/fabric8/maven/docker/config/handler/property/ConfigKey.java @@ -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"), diff --git a/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java b/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java index af247c1c1..4922bd966 100644 --- a/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java +++ b/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java @@ -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())) diff --git a/src/main/java/io/fabric8/maven/docker/service/BuildXService.java b/src/main/java/io/fabric8/maven/docker/service/BuildXService.java index 880ec6255..07e397bd8 100644 --- a/src/main/java/io/fabric8/maven/docker/service/BuildXService.java +++ b/src/main/java/io/fabric8/maven/docker/service/BuildXService.java @@ -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; @@ -217,14 +218,12 @@ protected void createDirectory(Path cachePath) { protected String createBuilder(Path configPath, List 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 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", diff --git a/src/test/java/io/fabric8/maven/docker/PushMojoBuildXTest.java b/src/test/java/io/fabric8/maven/docker/PushMojoBuildXTest.java index a954d93c7..32a3ebb6f 100644 --- a/src/test/java/io/fabric8/maven/docker/PushMojoBuildXTest.java +++ b/src/test/java/io/fabric8/maven/docker/PushMojoBuildXTest.java @@ -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", @@ -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", @@ -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", @@ -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", @@ -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();