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

Copy sources from remote container when quarkus.native.debug.enabled=true #15288

Merged
merged 4 commits into from
Mar 2, 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
@@ -1,7 +1,5 @@
package io.quarkus.deployment.pkg.steps;

import static io.quarkus.deployment.pkg.steps.LinuxIDUtil.getLinuxID;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
Expand All @@ -12,7 +10,6 @@
import java.util.function.Function;
import java.util.stream.Stream;

import org.apache.commons.lang3.SystemUtils;
import org.jboss.logging.Logger;

import io.quarkus.deployment.pkg.NativeConfig;
Expand All @@ -23,7 +20,7 @@ public abstract class NativeImageBuildContainerRunner extends NativeImageBuildRu

private static final Logger log = Logger.getLogger(NativeImageBuildContainerRunner.class);

private final NativeConfig nativeConfig;
final NativeConfig nativeConfig;
protected final NativeConfig.ContainerRuntime containerRuntime;
private final String[] baseContainerRuntimeArgs;
protected final String outputPath;
Expand All @@ -33,23 +30,10 @@ public NativeImageBuildContainerRunner(NativeConfig nativeConfig, Path outputDir
containerRuntime = nativeConfig.containerRuntime.orElseGet(NativeImageBuildContainerRunner::detectContainerRuntime);
log.infof("Using %s to run the native image builder", containerRuntime.getExecutableName());

List<String> containerRuntimeArgs = new ArrayList<>();
Collections.addAll(containerRuntimeArgs, "--env", "LANG=C");
this.baseContainerRuntimeArgs = new String[] { "--env", "LANG=C" };

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

if (SystemUtils.IS_OS_LINUX) {
String uid = getLinuxID("-ur");
String gid = getLinuxID("-gr");
if (uid != null && gid != null && !uid.isEmpty() && !gid.isEmpty()) {
Collections.addAll(containerRuntimeArgs, "--user", uid + ":" + gid);
if (containerRuntime == NativeConfig.ContainerRuntime.PODMAN) {
// Needed to avoid AccessDeniedExceptions
containerRuntimeArgs.add("--userns=keep-id");
}
}
}
this.baseContainerRuntimeArgs = containerRuntimeArgs.toArray(new String[0]);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.quarkus.deployment.pkg.steps;

import static io.quarkus.deployment.pkg.steps.LinuxIDUtil.getLinuxID;

import java.nio.file.Path;
import java.util.Collections;
import java.util.List;
Expand All @@ -21,7 +23,18 @@ protected List<String> getContainerRuntimeBuildArgs() {
String volumeOutputPath = outputPath;
if (SystemUtils.IS_OS_WINDOWS) {
volumeOutputPath = FileUtil.translateToVolumePath(volumeOutputPath);
} else if (SystemUtils.IS_OS_LINUX) {
String uid = getLinuxID("-ur");
String gid = getLinuxID("-gr");
if (uid != null && gid != null && !uid.isEmpty() && !gid.isEmpty()) {
Collections.addAll(containerRuntimeArgs, "--user", uid + ":" + gid);
if (containerRuntime == NativeConfig.ContainerRuntime.PODMAN) {
// Needed to avoid AccessDeniedExceptions
containerRuntimeArgs.add("--userns=keep-id");
}
}
}

Collections.addAll(containerRuntimeArgs, "--rm", "-v",
volumeOutputPath + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH + ":z");
return containerRuntimeArgs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
import java.util.List;
import java.util.stream.Stream;

import io.quarkus.deployment.util.ProcessUtil;

public class NativeImageBuildLocalRunner extends NativeImageBuildRunner {

private final String nativeImageExecutable;
Expand All @@ -16,11 +14,9 @@ public NativeImageBuildLocalRunner(String nativeImageExecutable) {
}

@Override
public void cleanupServer(File outputDir, boolean processInheritIODisabled) throws InterruptedException, IOException {
final ProcessBuilder pb = new ProcessBuilder(nativeImageExecutable, "--server-shutdown");
pb.directory(outputDir);
final Process process = ProcessUtil.launchProcess(pb, processInheritIODisabled);
process.waitFor();
public void cleanupServer(File outputDir) throws InterruptedException, IOException {
final String[] cleanupCommand = { nativeImageExecutable, "--server-shutdown" };
runCommand(cleanupCommand, null, outputDir);
zakkak marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@
import java.nio.file.Path;
import java.util.List;

import org.jboss.logging.Logger;

import io.quarkus.deployment.pkg.NativeConfig;

public class NativeImageBuildRemoteContainerRunner extends NativeImageBuildContainerRunner {

private static final Logger log = Logger.getLogger(NativeImageBuildRemoteContainerRunner.class);

private final String nativeImageName;
private String containerId;

Expand All @@ -22,15 +26,17 @@ public NativeImageBuildRemoteContainerRunner(NativeConfig nativeConfig, Path out
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();
createContainerProcess.waitFor();
if (createContainerProcess.waitFor() != 0) {
throw new RuntimeException("Failed to create builder container.");
}
try (BufferedReader reader = new BufferedReader(new InputStreamReader(createContainerProcess.getInputStream()))) {
containerId = reader.readLine();
}
String[] copyCommand = new String[] { containerRuntime.getExecutableName(), "cp", outputPath + "/.",
containerId + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH };
Process copyProcess = new ProcessBuilder(copyCommand).start();
copyProcess.waitFor();
runCommand(copyCommand, "Failed to copy source-jar and libs from host to builder container", null);
super.preBuild(buildArgs);
}

Expand All @@ -41,13 +47,18 @@ protected String[] getBuildCommand(List<String> args) {

@Override
protected void postBuild() throws InterruptedException, IOException {
String[] copyCommand = new String[] { containerRuntime.getExecutableName(), "cp",
containerId + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH + "/" + nativeImageName, outputPath };
Process copyProcess = new ProcessBuilder(copyCommand).start();
copyProcess.waitFor();
copyFromBuilder(nativeImageName, "Failed to copy native image from container back to the host.");
if (nativeConfig.debug.enabled) {
copyFromBuilder("sources", "Failed to copy sources from container back to the host.");
}
String[] removeCommand = new String[] { containerRuntime.getExecutableName(), "container", "rm", "--volumes",
containerId };
Process removeProcess = new ProcessBuilder(removeCommand).start();
removeProcess.waitFor();
runCommand(removeCommand, "Failed to remove container: " + containerId, null);
}

private void copyFromBuilder(String path, String errorMsg) throws IOException, InterruptedException {
String[] copyCommand = new String[] { containerRuntime.getExecutableName(), "cp",
containerId + ":" + NativeImageBuildStep.CONTAINER_BUILD_VOLUME_PATH + "/" + path, outputPath };
runCommand(copyCommand, errorMsg, null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,20 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

import org.jboss.logging.Logger;

import io.quarkus.deployment.pkg.steps.NativeImageBuildStep.GraalVM;
import io.quarkus.deployment.util.ProcessUtil;

public abstract class NativeImageBuildRunner {

private static final Logger log = Logger.getLogger(NativeImageBuildRunner.class);

public GraalVM.Version getGraalVMVersion() {
final GraalVM.Version graalVMVersion;
try {
String[] versionCommand = getGraalVMVersionCommand(Collections.singletonList("--version"));
log.debugf(String.join(" ", versionCommand).replace("$", "\\$"));
Process versionProcess = new ProcessBuilder(versionCommand)
.redirectErrorStream(true)
.start();
Expand All @@ -38,16 +43,18 @@ public GraalVM.Version getGraalVMVersion() {
public void setup(boolean processInheritIODisabled) {
}

public void cleanupServer(File outputDir, boolean processInheritIODisabled) throws InterruptedException, IOException {
public void cleanupServer(File outputDir) throws InterruptedException, IOException {
}

public int build(List<String> args, Path outputDir, boolean processInheritIODisabled)
throws InterruptedException, IOException {
preBuild(args);
try {
CountDownLatch errorReportLatch = new CountDownLatch(1);
final ProcessBuilder processBuilder = new ProcessBuilder(getBuildCommand(args))
final String[] buildCommand = getBuildCommand(args);
final ProcessBuilder processBuilder = new ProcessBuilder(buildCommand)
.directory(outputDir.toFile());
log.info(String.join(" ", buildCommand).replace("$", "\\$"));
final Process process = ProcessUtil.launchProcessStreamStdOut(processBuilder, processInheritIODisabled);
ExecutorService executor = Executors.newSingleThreadExecutor();
executor.submit(new ErrorReplacingProcessReader(process.getErrorStream(), outputDir.resolve("reports").toFile(),
Expand All @@ -70,4 +77,41 @@ protected void preBuild(List<String> buildArgs) throws IOException, InterruptedE
protected void postBuild() throws InterruptedException, IOException {
}

/**
* Run {@code command} in {@code workingDirectory} and log error if {@code errorMsg} is not null.
*
* @param command The command to run
* @param errorMsg The error message to be printed in case of failure.
* 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) {
log.info(String.join(" ", command).replace("$", "\\$"));
Process process = null;
try {
final ProcessBuilder processBuilder = new ProcessBuilder(command);
if (workingDirectory != null) {
processBuilder.directory(workingDirectory);
}
process = processBuilder.start();
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you are you are not redirecting the output streams like ProcessUtil#launchProcess was doing

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, but that's how things where before (except for cleanupServer).

runCommand is used to run native-image --shutdown-server, docker rm container-name and docker cp src dest. IMO it's OK to not redirect output for these commands (as discussed in https://github.com/quarkusio/quarkus/pull/15288/files#r582745324). WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks George

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing this

final int exitCode = process.waitFor();
if (exitCode != 0) {
if (errorMsg != null) {
log.error(errorMsg);
} else {
log.debugf("Command: " + String.join(" ", command) + " failed with exit code " + exitCode);
}
}
} catch (IOException | InterruptedException e) {
if (errorMsg != null) {
log.error(errorMsg);
} else {
log.debugf(e, "Command: " + String.join(" ", command) + " failed.");
}
} finally {
if (process != null) {
process.destroy();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public NativeImageBuildItem build(NativeConfig nativeConfig, NativeImageSourceJa

try {
if (nativeConfig.cleanupServer && !graalVMVersion.isMandrel()) {
buildRunner.cleanupServer(outputDir.toFile(), processInheritIODisabled.isPresent());
buildRunner.cleanupServer(outputDir.toFile());
}

NativeImageInvokerInfo commandAndExecutable = new NativeImageInvokerInfo.Builder()
Expand All @@ -175,7 +175,6 @@ public NativeImageBuildItem build(NativeConfig nativeConfig, NativeImageSourceJa

List<String> nativeImageArgs = commandAndExecutable.args;

log.info(String.join(" ", nativeImageArgs).replace("$", "\\$"));
int exitCode = buildRunner.build(nativeImageArgs, outputDir, processInheritIODisabled.isPresent());
if (exitCode != 0) {
throw imageGenerationFailed(exitCode, nativeImageArgs);
Expand Down