Take into account "fallback to container" when determining where nati…

…ve-image gets executed Sometimes we need to do things differently based on whether the native build happens within a container or not. Before this patch, we used to determine that based exclusively on explicit configuration. After this patch, we correctly take into account that we sometimes need to "fall back" to containers even though the configuration didn't mention anything about containers, simply because native-image isn't installed. The previous behavior used to lead to at least one bug: when debugging GraalVM's JVM, we determined the address to bind to based on whether we're running in a container or not, and [here we used to make the wrong choice](https://github.com/quarkusio/quarkus/blob/635a848bb8022e4ca6e945af7e62edc32a288588/core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildStep.java#L780-L784), resulting in the JVM debug agent being inaccessible from the Docker host. See also https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/quarkus.2Enative.2Edebug-build-process/near/345399379 Co-Authored-By: Foivos Zakkak <fzakkak@redhat.com>
quarkusio · Apr 3, 2023 · 6ea71f8 · 6ea71f8
1 parent 4fca0d7
commit 6ea71f8
Show file tree

Hide file tree

Showing 14 changed files with 181 additions and 100 deletions.
diff --git a/...ent/src/main/java/io/quarkus/deployment/builditem/nativeimage/UnsupportedOSBuildItem.java b/...ent/src/main/java/io/quarkus/deployment/builditem/nativeimage/UnsupportedOSBuildItem.java
@@ -5,7 +5,6 @@
 import static io.quarkus.dev.console.QuarkusConsole.IS_WINDOWS;
 
 import io.quarkus.builder.item.MultiBuildItem;
-import io.quarkus.deployment.pkg.NativeConfig;
 
 /**
  * Native-image might not be supported for a particular
@@ -32,14 +31,14 @@ public UnsupportedOSBuildItem(Os os, String error) {
         this.error = error;
     }
 
-    public boolean triggerError(NativeConfig nativeConfig) {
+    public boolean triggerError(boolean isContainerBuild) {
         return
         // When the host OS is unsupported, it could have helped to
         // run in a Linux builder image (e.g. an extension unsupported on Windows).
-        (os.active && !nativeConfig.isContainerBuild()) ||
+        (os.active && !isContainerBuild) ||
         // If Linux is the OS the extension does not support,
         // it fails in a container build regardless the host OS,
         // because we have only Linux based builder images.
-                (nativeConfig.isContainerBuild() && os == Os.LINUX);
+                (isContainerBuild && os == Os.LINUX);
     }
 }
diff --git a/core/deployment/src/main/java/io/quarkus/deployment/pkg/NativeConfig.java b/core/deployment/src/main/java/io/quarkus/deployment/pkg/NativeConfig.java
@@ -205,7 +205,7 @@ public class NativeConfig {
     @ConfigItem
     public boolean remoteContainerBuild;
 
-    public boolean isContainerBuild() {
+    public boolean isExplicitContainerBuild() {
         return containerBuild.orElse(containerRuntime.isPresent() || remoteContainerBuild);
     }
 

diff --git a/...loyment/src/main/java/io/quarkus/deployment/pkg/builditem/NativeImageRunnerBuildItem.java b/...loyment/src/main/java/io/quarkus/deployment/pkg/builditem/NativeImageRunnerBuildItem.java
@@ -0,0 +1,23 @@
+package io.quarkus.deployment.pkg.builditem;
+
+import io.quarkus.builder.item.SimpleBuildItem;
+import io.quarkus.deployment.pkg.steps.NativeImageBuildRunner;
+
+/**
+ * The resolved factory for the native image runner.
+ */
+public final class NativeImageRunnerBuildItem extends SimpleBuildItem {
+    private final NativeImageBuildRunner buildRunner;
+
+    public NativeImageRunnerBuildItem(NativeImageBuildRunner buildRunner) {
+        this.buildRunner = buildRunner;
+    }
+
+    public NativeImageBuildRunner getBuildRunner() {
+        return buildRunner;
+    }
+
+    public boolean isContainerBuild() {
+        return buildRunner.isContainer();
+    }
+}
diff --git a/...oyment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildContainerRunner.java b/...oyment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildContainerRunner.java
@@ -24,20 +24,23 @@ public abstract class NativeImageBuildContainerRunner extends NativeImageBuildRu
     final NativeConfig nativeConfig;
     protected final ContainerRuntimeUtil.ContainerRuntime containerRuntime;
     String[] baseContainerRuntimeArgs;
-    protected final String outputPath;
     private final String containerName;
 
-    public NativeImageBuildContainerRunner(NativeConfig nativeConfig, Path outputDir) {
+    protected NativeImageBuildContainerRunner(NativeConfig nativeConfig) {
         this.nativeConfig = nativeConfig;
         containerRuntime = nativeConfig.containerRuntime.orElseGet(ContainerRuntimeUtil::detectContainerRuntime);
         log.infof("Using %s to run the native image builder", containerRuntime.getExecutableName());
 
         this.baseContainerRuntimeArgs = new String[] { "--env", "LANG=C", "--rm" };
 
-        outputPath = outputDir == null ? null : outputDir.toAbsolutePath().toString();
         containerName = "build-native-" + RandomStringUtils.random(5, true, false);
     }
 
+    @Override
+    public boolean isContainer() {
+        return true;
+    }
+
     @Override
     public void setup(boolean processInheritIODisabled) {
         if (containerRuntime == ContainerRuntimeUtil.ContainerRuntime.DOCKER
@@ -69,8 +72,8 @@ protected String[] getGraalVMVersionCommand(List<String> args) {
     }
 
     @Override
-    protected String[] getBuildCommand(List<String> args) {
-        List<String> containerRuntimeBuildArgs = getContainerRuntimeBuildArgs();
+    protected String[] getBuildCommand(Path outputDir, List<String> args) {
+        List<String> containerRuntimeBuildArgs = getContainerRuntimeBuildArgs(outputDir);
         List<String> effectiveContainerRuntimeBuildArgs = new ArrayList<>(containerRuntimeBuildArgs.size() + 2);
         effectiveContainerRuntimeBuildArgs.addAll(containerRuntimeBuildArgs);
         effectiveContainerRuntimeBuildArgs.add("--name");
@@ -79,8 +82,8 @@ protected String[] getBuildCommand(List<String> args) {
     }
 
     @Override
-    protected void objcopy(String... args) {
-        final List<String> containerRuntimeBuildArgs = getContainerRuntimeBuildArgs();
+    protected void objcopy(Path outputDir, String... args) {
+        final List<String> containerRuntimeBuildArgs = getContainerRuntimeBuildArgs(outputDir);
         Collections.addAll(containerRuntimeBuildArgs, "--entrypoint", "/bin/bash");
         final ArrayList<String> objcopyCommand = new ArrayList<>(2);
         objcopyCommand.add("-c");
@@ -107,7 +110,7 @@ public void addShutdownHook(Process process) {
         }));
     }
 
-    protected List<String> getContainerRuntimeBuildArgs() {
+    protected List<String> getContainerRuntimeBuildArgs(Path outputDir) {
         List<String> containerRuntimeArgs = new ArrayList<>();
         nativeConfig.containerRuntimeOptions.ifPresent(containerRuntimeArgs::addAll);
         if (nativeConfig.debugBuildProcess && nativeConfig.publishDebugBuildProcessPort) {

diff --git a/...t/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildLocalContainerRunner.java b/...t/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildLocalContainerRunner.java
@@ -17,8 +17,8 @@
 
 public class NativeImageBuildLocalContainerRunner extends NativeImageBuildContainerRunner {
 
-    public NativeImageBuildLocalContainerRunner(NativeConfig nativeConfig, Path outputDir) {
-        super(nativeConfig, outputDir);
+    public NativeImageBuildLocalContainerRunner(NativeConfig nativeConfig) {
+        super(nativeConfig);
         if (SystemUtils.IS_OS_LINUX) {
             final ArrayList<String> containerRuntimeArgs = new ArrayList<>(Arrays.asList(baseContainerRuntimeArgs));
             if (containerRuntime == DOCKER && containerRuntime.isRootless()) {
@@ -39,13 +39,11 @@ public NativeImageBuildLocalContainerRunner(NativeConfig nativeConfig, Path outp
     }
 
     @Override
-    protected List<String> getContainerRuntimeBuildArgs() {
-        final List<String> containerRuntimeArgs = super.getContainerRuntimeBuildArgs();
-        final String volumeOutputPath;
+    protected List<String> getContainerRuntimeBuildArgs(Path outputDir) {
+        final List<String> containerRuntimeArgs = super.getContainerRuntimeBuildArgs(outputDir);
+        String volumeOutputPath = outputDir.toAbsolutePath().toString();
         if (SystemUtils.IS_OS_WINDOWS) {
-            volumeOutputPath = FileUtil.translateToVolumePath(outputPath);
-        } else {
-            volumeOutputPath = outputPath;
+            volumeOutputPath = FileUtil.translateToVolumePath(volumeOutputPath);
         }
 
         final String selinuxBindOption;

diff --git a/...deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildLocalRunner.java b/...deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildLocalRunner.java
@@ -1,6 +1,7 @@
 package io.quarkus.deployment.pkg.steps;
 
 import java.io.File;
+import java.nio.file.Path;
 import java.util.List;
 import java.util.stream.Stream;
 
@@ -9,11 +10,14 @@
 public class NativeImageBuildLocalRunner extends NativeImageBuildRunner {
 
     private final String nativeImageExecutable;
-    private final File workingDirectory;
 
-    public NativeImageBuildLocalRunner(String nativeImageExecutable, File workingDirectory) {
+    public NativeImageBuildLocalRunner(String nativeImageExecutable) {
         this.nativeImageExecutable = nativeImageExecutable;
-        this.workingDirectory = workingDirectory;
+    }
+
+    @Override
+    public boolean isContainer() {
+        return false;
     }
 
     @Override
@@ -22,16 +26,16 @@ protected String[] getGraalVMVersionCommand(List<String> args) {
     }
 
     @Override
-    protected String[] getBuildCommand(List<String> args) {
+    protected String[] getBuildCommand(Path outputDir, List<String> args) {
         return buildCommand(args);
     }
 
     @Override
-    protected void objcopy(String... args) {
+    protected void objcopy(Path outputDir, String... args) {
         final String[] command = new String[args.length + 1];
         command[0] = "objcopy";
         System.arraycopy(args, 0, command, 1, args.length);
-        runCommand(command, null, workingDirectory);
+        runCommand(command, null, outputDir.toFile());
     }
 
     @Override

diff --git a/.../src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildRemoteContainerRunner.java b/.../src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildRemoteContainerRunner.java
@@ -20,19 +20,14 @@ public class NativeImageBuildRemoteContainerRunner extends NativeImageBuildConta
     // issue https://github.com/containers/podman/issues/9608
     private static final String CONTAINER_BUILD_VOLUME_NAME = "quarkus-native-builder-image-project-volume";
 
-    private final String nativeImageName;
-    private final String resultingExecutableName;
     private String containerId;
 
-    public NativeImageBuildRemoteContainerRunner(NativeConfig nativeConfig, Path outputDir,
-            String nativeImageName, String resultingExecutableName) {
-        super(nativeConfig, outputDir);
-        this.nativeImageName = nativeImageName;
-        this.resultingExecutableName = resultingExecutableName;
+    protected NativeImageBuildRemoteContainerRunner(NativeConfig nativeConfig) {
+        super(nativeConfig);
     }
 
     @Override
-    protected void preBuild(List<String> buildArgs) throws InterruptedException, IOException {
+    protected void preBuild(Path outputDir, List<String> buildArgs) throws InterruptedException, IOException {
         // docker volume rm <volumeID>
         rmVolume(null);
         // docker create -v <volumeID>:/project <image-name>
@@ -41,10 +36,10 @@ protected void preBuild(List<String> buildArgs) throws InterruptedException, IOE
         final String[] createTempContainerCommand = buildCommand("create", containerRuntimeArgs, Collections.emptyList());
         containerId = runCommandAndReadOutput(createTempContainerCommand, "Failed to create temp container.");
         // docker cp <files> <containerID>:/project
-        String[] copyCommand = new String[] { containerRuntime.getExecutableName(), "cp", outputPath + "/.",
+        String[] copyCommand = new String[] { containerRuntime.getExecutableName(), "cp", outputDir.toAbsolutePath() + "/.",
                 containerId + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH };
         runCommand(copyCommand, "Failed to copy source-jar and libs from host to builder container", null);
-        super.preBuild(buildArgs);
+        super.preBuild(outputDir, buildArgs);
     }
 
     private String runCommandAndReadOutput(String[] command, String errorMsg) throws IOException, InterruptedException {
@@ -59,12 +54,13 @@ private String runCommandAndReadOutput(String[] command, String errorMsg) throws
     }
 
     @Override
-    protected void postBuild() {
-        copyFromContainerVolume(resultingExecutableName, "Failed to copy native image from container volume back to the host.");
+    protected void postBuild(Path outputDir, String nativeImageName, String resultingExecutableName) {
+        copyFromContainerVolume(outputDir, resultingExecutableName,
+                "Failed to copy native image from container volume back to the host.");
         if (nativeConfig.debug.enabled) {
-            copyFromContainerVolume("sources", "Failed to copy sources from container volume back to the host.");
+            copyFromContainerVolume(outputDir, "sources", "Failed to copy sources from container volume back to the host.");
             String symbols = String.format("%s.debug", nativeImageName);
-            copyFromContainerVolume(symbols, "Failed to copy debug symbols from container volume back to the host.");
+            copyFromContainerVolume(outputDir, symbols, "Failed to copy debug symbols from container volume back to the host.");
         }
         // docker container rm <containerID>
         final String[] rmTempContainerCommand = new String[] { containerRuntime.getExecutableName(), "container", "rm",
@@ -80,16 +76,17 @@ private void rmVolume(String errorMsg) {
         runCommand(rmVolumeCommand, errorMsg, null);
     }
 
-    private void copyFromContainerVolume(String path, String errorMsg) {
+    private void copyFromContainerVolume(Path outputDir, String path, String errorMsg) {
         // docker cp <containerID>:/project/<path> <dest>
         String[] copyCommand = new String[] { containerRuntime.getExecutableName(), "cp",
-                containerId + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH + "/" + path, outputPath };
+                containerId + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH + "/" + path,
+                outputDir.toAbsolutePath().toString() };
         runCommand(copyCommand, errorMsg, null);
     }
 
     @Override
-    protected List<String> getContainerRuntimeBuildArgs() {
-        List<String> containerRuntimeArgs = super.getContainerRuntimeBuildArgs();
+    protected List<String> getContainerRuntimeBuildArgs(Path outputDir) {
+        List<String> containerRuntimeArgs = super.getContainerRuntimeBuildArgs(outputDir);
         Collections.addAll(containerRuntimeArgs, "-v",
                 CONTAINER_BUILD_VOLUME_NAME + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH);
         return containerRuntimeArgs;

diff --git a/core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildRunner.java b/core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildRunner.java
@@ -40,6 +40,8 @@ public GraalVM.Version getGraalVMVersion() {
         return graalVMVersion;
     }
 
+    public abstract boolean isContainer();
+
     public void setup(boolean processInheritIODisabled) {
     }
 
@@ -49,10 +51,10 @@ public void addShutdownHook(Process buildNativeProcess) {
     public Result build(List<String> args, String nativeImageName, String resultingExecutableName, Path outputDir,
             GraalVM.Version graalVMVersion, boolean debugSymbolsEnabled, boolean processInheritIODisabled)
             throws InterruptedException, IOException {
-        preBuild(args);
+        preBuild(outputDir, args);
         try {
             CountDownLatch errorReportLatch = new CountDownLatch(1);
-            final String[] buildCommand = getBuildCommand(args);
+            final String[] buildCommand = getBuildCommand(outputDir, args);
             final ProcessBuilder processBuilder = new ProcessBuilder(buildCommand)
                     .directory(outputDir.toFile());
             log.info(String.join(" ", buildCommand).replace("$", "\\$"));
@@ -71,15 +73,15 @@ public Result build(List<String> args, String nativeImageName, String resultingE
 
             if (debugSymbolsEnabled && graalVMVersion.compareTo(GraalVM.Version.VERSION_23_0_0) < 0 && objcopyExists) {
                 // Need to explicitly split debug symbols prior to GraalVM/Mandrel 23.0
-                splitDebugSymbols(nativeImageName, resultingExecutableName);
+                splitDebugSymbols(outputDir, nativeImageName, resultingExecutableName);
             }
             if (!(debugSymbolsEnabled && graalVMVersion.compareTo(GraalVM.Version.VERSION_23_0_0) >= 0)) {
                 // Strip debug symbols even if not generated by GraalVM/Mandrel, because the underlying JDK might
                 // contain them. Note, however, that starting with GraalVM/Mandrel 23.0 this is done by default when
                 // generating debug info, so we don't want to do it twice and print twice a warning if objcopy is not
                 // available.
                 if (objcopyExists) {
-                    objcopy("--strip-debug", resultingExecutableName);
+                    objcopy(outputDir, "--strip-debug", resultingExecutableName);
                 } else if (SystemUtils.IS_OS_LINUX) {
                     log.warn(
                             "objcopy executable not found in PATH. Debug symbols will therefore not be separated from the executable.");
@@ -88,30 +90,31 @@ public Result build(List<String> args, String nativeImageName, String resultingE
             }
             return new Result(0);
         } finally {
-            postBuild();
+            postBuild(outputDir, nativeImageName, resultingExecutableName);
         }
     }
 
-    private void splitDebugSymbols(String nativeImageName, String resultingExecutableName) {
+    private void splitDebugSymbols(Path outputDir, String nativeImageName, String resultingExecutableName) {
         String symbols = String.format("%s.debug", nativeImageName);
-        objcopy("--only-keep-debug", resultingExecutableName, symbols);
-        objcopy(String.format("--add-gnu-debuglink=%s", symbols), resultingExecutableName);
+        objcopy(outputDir, "--only-keep-debug", resultingExecutableName, symbols);
+        objcopy(outputDir, String.format("--add-gnu-debuglink=%s", symbols), resultingExecutableName);
     }
 
     protected abstract String[] getGraalVMVersionCommand(List<String> args);
 
-    protected abstract String[] getBuildCommand(List<String> args);
+    protected abstract String[] getBuildCommand(Path outputDir, List<String> args);
 
     protected boolean objcopyExists() {
         return true;
     }
 
-    protected abstract void objcopy(String... args);
+    protected abstract void objcopy(Path outputDir, String... args);
 
-    protected void preBuild(List<String> buildArgs) throws IOException, InterruptedException {
+    protected void preBuild(Path outputDir, List<String> buildArgs) throws IOException, InterruptedException {
     }
 
-    protected void postBuild() throws InterruptedException, IOException {
+    protected void postBuild(Path outputDir, String nativeImageName, String resultingExecutableName)
+            throws InterruptedException, IOException {
     }
 
     /**

diff --git a/...deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildRunnerError.java b/...deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildRunnerError.java
@@ -0,0 +1,33 @@
+package io.quarkus.deployment.pkg.steps;
+
+import java.nio.file.Path;
+import java.util.List;
+
+public class NativeImageBuildRunnerError extends NativeImageBuildRunner {
+
+    private String message;
+
+    public NativeImageBuildRunnerError(String message) {
+        this.message = message;
+    }
+
+    @Override
+    public boolean isContainer() {
+        return false;
+    }
+
+    @Override
+    protected String[] getGraalVMVersionCommand(List<String> args) {
+        throw new RuntimeException(message);
+    }
+
+    @Override
+    protected String[] getBuildCommand(Path outputDir, List<String> args) {
+        throw new RuntimeException(message);
+    }
+
+    @Override
+    protected void objcopy(Path outputDir, String... args) {
+        throw new RuntimeException(message);
+    }
+}