fix (buildx) : Change default value of buildx node to null (fabric8io…

…#1701) Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia · Aug 17, 2023 · bd554a2 · bd554a2
1 parent d730680
commit bd554a2
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 4 deletions.
diff --git a/src/main/java/io/fabric8/maven/docker/service/BuildXService.java b/src/main/java/io/fabric8/maven/docker/service/BuildXService.java
@@ -2,6 +2,7 @@
 
 import io.fabric8.maven.docker.access.AuthConfigList;
 import io.fabric8.maven.docker.access.DockerAccess;
+import io.fabric8.maven.docker.access.util.ExternalCommand;
 import io.fabric8.maven.docker.assembly.BuildDirs;
 import io.fabric8.maven.docker.assembly.DockerAssemblyManager;
 import io.fabric8.maven.docker.config.AttestationConfiguration;
@@ -12,6 +13,7 @@
 import io.fabric8.maven.docker.util.ImageName;
 import io.fabric8.maven.docker.util.Logger;
 import io.fabric8.maven.docker.util.ProjectPaths;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.maven.plugin.MojoExecutionException;
 
 import java.io.BufferedReader;
@@ -68,8 +70,7 @@ protected <C> void useBuilder(ProjectPaths projectPaths, ImageConfiguration imag
         Path configPath = getDockerStateDir(imageConfig.getBuildConfiguration(),  buildDirs);
         List<String> buildX = new ArrayList<>();
         buildX.add("docker");
-        if (authConfig != null && !authConfig.isEmpty() &&
-            !hasAuthForRegistryInDockerConfig(logger, configuredRegistry, authConfig)) {
+        if (isDockerCLINotLegacy() || shouldAddConfigInLegacyDockerCLI(authConfig, configuredRegistry)) {
             buildX.add("--config");
             buildX.add(configPath.toString());
         }
@@ -85,6 +86,42 @@ protected <C> void useBuilder(ProjectPaths projectPaths, ImageConfiguration imag
         }
     }
 
+    private boolean shouldAddConfigInLegacyDockerCLI(AuthConfigList authConfigList, String configuredRegistry) throws MojoExecutionException {
+        return authConfigList != null && !authConfigList.isEmpty() &&
+            !hasAuthForRegistryInDockerConfig(logger, configuredRegistry, authConfigList);
+    }
+
+    private boolean isDockerCLINotLegacy(){
+        StringBuilder outputBuilder = new StringBuilder();
+        ExternalCommand dockerVersionExternalCommand = new ExternalCommand(logger) {
+            @Override
+            protected String[] getArgs() {
+                return new String[] {"docker", "version", "--format", "'{{.Client.Version}}'"};
+            }
+            @Override
+            protected void processLine(String line) {
+                if (StringUtils.isNotBlank(line)) {
+                    outputBuilder.append(line);
+                }
+            }
+        };
+
+        try {
+            dockerVersionExternalCommand.execute();
+        } catch (IOException e) {
+            logger.info("Failure in getting docker CLI version", e);
+        }
+        String version = outputBuilder.toString();
+        String[] versionParts = version.split("\\.");
+        if (versionParts.length >= 3) {
+            logger.info("Using Docker CLI " + version);
+            int cliMajorVersion = Integer.parseInt(versionParts[0]);
+            int cliMinorVersion = Integer.parseInt(versionParts[1]);
+            return cliMajorVersion >= 23 || cliMinorVersion >= 0;
+        }
+        return false;
+    }
+
     protected void createConfigJson(Path configJson, AuthConfigList authConfig) throws MojoExecutionException {
         try (BufferedWriter bufferedWriter = Files.newBufferedWriter(configJson, StandardCharsets.UTF_8,
             StandardOpenOption.WRITE, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)
@@ -219,11 +256,14 @@ 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 = Optional.ofNullable(buildXConfiguration.getBuilderName()).orElse("maven");
-        String nodeName = Optional.ofNullable(buildXConfiguration.getNodeName()).orElse(builderName + "0");
+        String nodeName = buildXConfiguration.getNodeName();
         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", nodeName);
+            append(cmds, "create", "--driver", "docker-container", "--name", builderName);
+            if (nodeName != null) {
+                append(cmds, "--node", nodeName);
+            }
             String buildConfig = buildXConfiguration.getConfigFile();
             if(buildConfig != null) {
                 append(cmds, "--config",

diff --git a/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java b/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java
@@ -336,6 +336,7 @@ private void whenMojoExecutes() throws IOException, MojoExecutionException {
 
     private BuildXConfiguration.Builder getBuildXPlatforms(String... platforms) {
         return new BuildXConfiguration.Builder()
+            .nodeName("maven0")
             .platforms(Arrays.asList(platforms));
     }
 

diff --git a/src/test/java/io/fabric8/maven/docker/service/RegistryServiceTest.java b/src/test/java/io/fabric8/maven/docker/service/RegistryServiceTest.java
@@ -576,6 +576,7 @@ private void givenBuildxImageConfiguration(String imageName, String builderName,
         BuildXConfiguration buildx = new BuildXConfiguration.Builder()
             .platforms(Arrays.asList("linux/amd64", "linux/arm64"))
             .builderName(builderName)
+            .nodeName("maven0")
             .build();
         givenImageNameAndBuildX(imageName, buildx, dockerFile, tag);
     }