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

Use builder image's objcopy when building with quarkus.native.container-build=true and quarkus.native.debug.enabled=true #13963

Merged
merged 5 commits into from
Apr 22, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ public class NativeConfig {
public boolean dumpProxies;

/**
* If this build should be done using a container runtime. If this is set docker will be used by default,
* unless container-runtime is also set.
* If this build should be done using a container runtime. Unless container-runtime is also set, docker will be
* used by default. If docker is not available or is an alias to podman, podman will be used instead as the default.
*/
@ConfigItem
public boolean containerBuild;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public NativeImageBuildContainerRunner(NativeConfig nativeConfig, Path outputDir
containerRuntime = nativeConfig.containerRuntime.orElseGet(NativeImageBuildContainerRunner::detectContainerRuntime);
log.infof("Using %s to run the native image builder", containerRuntime.getExecutableName());

this.baseContainerRuntimeArgs = new String[] { "--env", "LANG=C" };
this.baseContainerRuntimeArgs = new String[] { "--env", "LANG=C", "--rm" };

outputPath = outputDir == null ? null : outputDir.toAbsolutePath().toString();

Expand Down Expand Up @@ -70,6 +70,17 @@ protected String[] getBuildCommand(List<String> args) {
return buildCommand("run", getContainerRuntimeBuildArgs(), args);
}

@Override
protected void objcopy(String... args) {
final List<String> containerRuntimeBuildArgs = getContainerRuntimeBuildArgs();
Collections.addAll(containerRuntimeBuildArgs, "--entrypoint", "/bin/bash");
final ArrayList<String> objcopyCommand = new ArrayList<>(2);
objcopyCommand.add("-c");
objcopyCommand.add("objcopy " + String.join(" ", args));
final String[] command = buildCommand("run", containerRuntimeBuildArgs, objcopyCommand);
runCommand(command, null, null);
}

protected List<String> getContainerRuntimeBuildArgs() {
List<String> containerRuntimeArgs = new ArrayList<>();
nativeConfig.containerRuntimeOptions.ifPresent(containerRuntimeArgs::addAll);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ protected List<String> getContainerRuntimeBuildArgs() {
}
}

Collections.addAll(containerRuntimeArgs, "--rm", "-v",
Collections.addAll(containerRuntimeArgs, "-v",
volumeOutputPath + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH + ":z");
return containerRuntimeArgs;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
package io.quarkus.deployment.pkg.steps;

import java.io.File;
import java.util.List;
import java.util.stream.Stream;

public class NativeImageBuildLocalRunner extends NativeImageBuildRunner {

private final String nativeImageExecutable;
private final File workingDirectory;

public NativeImageBuildLocalRunner(String nativeImageExecutable) {
public NativeImageBuildLocalRunner(String nativeImageExecutable, File workingDirectory) {
this.nativeImageExecutable = nativeImageExecutable;
this.workingDirectory = workingDirectory;
}

@Override
Expand All @@ -21,6 +24,34 @@ protected String[] getBuildCommand(List<String> args) {
return buildCommand(args);
}

@Override
protected void objcopy(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);
}

@Override
protected boolean objcopyExists() {
// System path
String systemPath = System.getenv("PATH");
if (systemPath != null) {
String[] pathDirs = systemPath.split(File.pathSeparator);
for (String pathDir : pathDirs) {
File dir = new File(pathDir);
if (dir.isDirectory()) {
File file = new File(dir, "objcopy");
if (file.exists()) {
return true;
}
}
}
}

return false;
}

private String[] buildCommand(List<String> args) {
return Stream.concat(Stream.of(nativeImageExecutable), args.stream()).toArray(String[]::new);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import org.jboss.logging.Logger;
Expand All @@ -13,52 +15,83 @@
public class NativeImageBuildRemoteContainerRunner extends NativeImageBuildContainerRunner {

private static final Logger log = Logger.getLogger(NativeImageBuildRemoteContainerRunner.class);
// Use a predefined volume name and implicitly create the volume with `podman create`, instead of explicitly
// generating a unique volume with `podman volume create`, to work around Podman's 3.0.x
// issue https://github.com/containers/podman/issues/9608
private static final String CONTAINER_BUILD_VOLUME_NAME = "quarkus-native-builder-image-project-volume";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally I would prefer a dynamically generated unique volume name, but I am not sure it's worth the effort.
We could also do something like fixed-name-YYYY-MM-DD-HHMM to avoid potential conflicts with existing volumes.
WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's straightforward to let docker generate the unique volume id, I quickly tested it locally:

final String[] createVolumeCommand = new String[] { containerRuntime.getExecutableName(), "volume", "create" };
volumeId = runCommandAndReadOutput(createVolumeCommand, "Failed to create temp volume.");

Using this as a first step in preBuild and replacing all CONTAINER_BUILD_VOLUME_NAME with volumeId.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it's not that simple. If you manually create the volume before starting the container, the volume (when mount) is owned by root:root and native-image fails to write to it in subsequent steps.

Creating the volume explicitly

$ podman volume create test-volume                         
test-volume
$ podman create --name temp -v test-volume:/project quay.io/quarkus/ubi-quarkus-native-image:21.0.0-java11                         
bfa2406d5fa0a003af99c91f077ed989006eb2441004135773429c425f9ce997
$ podman run --rm --entrypoint /bin/bash -it -v test-volume:/project quay.io/quarkus/ubi-quarkus-native-image:21.0.0-java11 -c "ls -la"
total 0
drwxr-xr-x.  2 root root 6 Mar  3 11:40 .
drwxr-xr-x. 19 root root 6 Mar  3 11:41 ..

Creating the volume implicitly

$ podman create --name temp -v test-volume:/project quay.io/quarkus/ubi-quarkus-native-image:21.0.0-java11
e30ffb76aa320448a3e645d7030c4dcf76b0efb83b68e3bc58a2e655c683d1de
$ podman run --rm --entrypoint /bin/bash -it -v test-volume:/project quay.io/quarkus/ubi-quarkus-native-image:21.0.0-java11 -c "ls -la"
total 0
drwxr-xr-x.  2 quarkus quarkus 6 Mar  3 11:42 .
drwxr-xr-x. 19 root    root    6 Mar  3 11:42 ..

I am not a containers expert though so I'll be happy to be corrected :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! I checked the following:

  • docker 19.03.13 on Ubuntu 20.10
  • docker 20.10.2 on Ubuntu 20.10 in WSL
  • podman 2.0.6 on Ubuntu 20.10
  • podman 1.6.4 on CentOS 7.9
  • podman 3.0.1 on CentOS 7.9

in all but the last case, the /project folder was owned by the quarkus user also when creating the volume explicitly. So something seems to have changed for podman 3. I'm guessing you're running on podman 3 as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed I am using podman 3.0.1 on Fedora 33.

Thanks for the additional info @jonathan-meier, I created containers/podman#9608.


private final String nativeImageName;
private final String resultingExecutableName;
private String containerId;

public NativeImageBuildRemoteContainerRunner(NativeConfig nativeConfig, Path outputDir, String resultingExecutableName) {
public NativeImageBuildRemoteContainerRunner(NativeConfig nativeConfig, Path outputDir,
String nativeImageName, String resultingExecutableName) {
super(nativeConfig, outputDir);
this.nativeImageName = nativeImageName;
this.resultingExecutableName = resultingExecutableName;
}

@Override
protected void preBuild(List<String> buildArgs) throws InterruptedException, IOException {
List<String> containerRuntimeArgs = getContainerRuntimeBuildArgs();
String[] createContainerCommand = buildCommand("create", containerRuntimeArgs, buildArgs);
log.info(String.join(" ", createContainerCommand).replace("$", "\\$"));
Process createContainerProcess = new ProcessBuilder(createContainerCommand).start();
if (createContainerProcess.waitFor() != 0) {
throw new RuntimeException("Failed to create builder container.");
}
try (BufferedReader reader = new BufferedReader(new InputStreamReader(createContainerProcess.getInputStream()))) {
containerId = reader.readLine();
}
// docker volume rm <volumeID>
rmVolume(null);
// docker create -v <volumeID>:/project <image-name>
final List<String> containerRuntimeArgs = Arrays.asList("-v",
CONTAINER_BUILD_VOLUME_NAME + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH);
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 + "/.",
containerId + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH };
runCommand(copyCommand, "Failed to copy source-jar and libs from host to builder container", null);
super.preBuild(buildArgs);
}

@Override
protected String[] getBuildCommand(List<String> args) {
return new String[] { containerRuntime.getExecutableName(), "start", "--attach", containerId };
private String runCommandAndReadOutput(String[] command, String errorMsg) throws IOException, InterruptedException {
log.info(String.join(" ", command).replace("$", "\\$"));
Process process = new ProcessBuilder(command).start();
if (process.waitFor() != 0) {
throw new RuntimeException(errorMsg);
}
try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()))) {
return reader.readLine();
}
}

@Override
protected void postBuild() throws InterruptedException, IOException {
copyFromBuilder(resultingExecutableName, "Failed to copy native executable from container back to the host.");
protected void postBuild() {
copyFromContainerVolume(resultingExecutableName, "Failed to copy native image from container volume back to the host.");
if (nativeConfig.debug.enabled) {
copyFromBuilder("sources", "Failed to copy sources from container back to the host.");
copyFromContainerVolume("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.");
}
String[] removeCommand = new String[] { containerRuntime.getExecutableName(), "container", "rm", "--volumes",
// docker container rm <containerID>
final String[] rmTempContainerCommand = new String[] { containerRuntime.getExecutableName(), "container", "rm",
containerId };
runCommand(removeCommand, "Failed to remove container: " + containerId, null);
runCommand(rmTempContainerCommand, "Failed to remove container: " + containerId, null);
// docker volume rm <volumeID>
rmVolume("Failed to remove volume: " + CONTAINER_BUILD_VOLUME_NAME);
}

private void copyFromBuilder(String path, String errorMsg) throws IOException, InterruptedException {
private void rmVolume(String errorMsg) {
final String[] rmVolumeCommand = new String[] { containerRuntime.getExecutableName(), "volume", "rm",
CONTAINER_BUILD_VOLUME_NAME };
runCommand(rmVolumeCommand, errorMsg, null);
}

private void copyFromContainerVolume(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 };
runCommand(copyCommand, errorMsg, null);
}

@Override
protected List<String> getContainerRuntimeBuildArgs() {
List<String> containerRuntimeArgs = super.getContainerRuntimeBuildArgs();
Collections.addAll(containerRuntimeArgs, "-v",
CONTAINER_BUILD_VOLUME_NAME + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH);
return containerRuntimeArgs;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ public GraalVM.Version getGraalVMVersion() {
public void setup(boolean processInheritIODisabled) {
}

public int build(List<String> args, Path outputDir, boolean processInheritIODisabled)
public int build(List<String> args, String nativeImageName, String resultingExecutableName, Path outputDir,
boolean debugEnabled, boolean processInheritIODisabled)
throws InterruptedException, IOException {
preBuild(args);
try {
Expand All @@ -57,16 +58,44 @@ public int build(List<String> args, Path outputDir, boolean processInheritIODisa
errorReportLatch));
executor.shutdown();
errorReportLatch.await();
return process.waitFor();
int exitCode = process.waitFor();
if (exitCode != 0) {
return exitCode;
}

if (objcopyExists()) {
if (debugEnabled) {
splitDebugSymbols(nativeImageName, resultingExecutableName);
} else {
// Strip debug symbols regardless, because the underlying JDK might contain them
objcopy("--strip-debug", resultingExecutableName);
}
} else {
log.warn("objcopy executable not found in PATH. Debug symbols will not be separated from executable.");
log.warn("That will result in a larger native image with debug symbols embedded in it.");
}
return 0;
} finally {
postBuild();
}
}

private void splitDebugSymbols(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);
}

protected abstract String[] getGraalVMVersionCommand(List<String> args);

protected abstract String[] getBuildCommand(List<String> args);

protected boolean objcopyExists() {
return true;
}

protected abstract void objcopy(String... args);

protected void preBuild(List<String> buildArgs) throws IOException, InterruptedException {
}

Expand All @@ -81,7 +110,7 @@ protected void postBuild() throws InterruptedException, IOException {
* If {@code null} the failure is ignored, but logged.
* @param workingDirectory The directory in which to run the command
*/
void runCommand(String[] command, String errorMsg, File workingDirectory) {
static void runCommand(String[] command, String errorMsg, File workingDirectory) {
log.info(String.join(" ", command).replace("$", "\\$"));
Process process = null;
try {
Expand Down
Loading