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.

Sign up for GitHub

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

Build docker commands better #441

Merged
merged 1 commit into from
Feb 23, 2024
Merged

Build docker commands better #441

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
125 changes: 87 additions & 38 deletions src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1267,21 +1267,30 @@ private void buildContainerImage(File tempContainerfile, File userContainerfile,
// Name rules: may contain lowercase letters, digits and a period, one or two underscores, or one or more dashes. Cannot start with dash.
imageName = imageName.replaceAll("[^a-zA-Z0-9]", "-").replaceAll("^[\\-]+", "").toLowerCase();

StringBuilder sb = new StringBuilder();
sb.append(getContainerCommandPrefix());
sb.append(" build ");
// Use List<String> for building the command. This will be converted to a String[] when passed to ProcessBuilder.
// The ProcessBuilder will handle quoting of paths (for blanks) as needed for various OS.
List<String> commandElements = new ArrayList<String>();
commandElements.add(getContainerCommandPrefix().trim());
commandElements.add("build");

if (pullParentImage) {
sb.append("--pull ");
commandElements.add("--pull");
}
sb.append("-f " + tempContainerfile + " -t " + imageName + " " + buildContext.getAbsolutePath());
String buildCmd = sb.toString();
info(buildCmd);
commandElements.add("-f");
commandElements.add(tempContainerfile.getAbsolutePath());
commandElements.add("-t");
commandElements.add(imageName);
commandElements.add(buildContext.getAbsolutePath());

String[] newCommand = commandElements.toArray(new String[commandElements.size()]);
info("Container command: "+String.join(" ", newCommand));

if (hasFeaturesSh.get()) {
info("The RUN features.sh command is detected in the Containerfile and extra time may be necessary when installing features.");
}
long startTime = System.currentTimeMillis();

execContainerCmdAndLog(getRunProcess(buildCmd), containerBuildTimeout, false);
execContainerCmdAndLog(getRunProcess(newCommand), containerBuildTimeout, false);

checkContainerBuildTime(startTime, buildContext);
info("Completed building container image.");
Expand Down Expand Up @@ -1337,8 +1346,8 @@ private void startContainer() throws PluginExecutionException {
new File(buildDirectory, DEVC_HIDDEN_FOLDER + "/dropins").mkdirs();

info("Starting container...");
String startContainerCommand = getContainerCommand();
info(startContainerCommand);
String[] startContainerCommand = getContainerCommand();
// Use String[] instead for command parameters. It will handle quoting of paths for various OS.
containerRunProcess = getRunProcess(startContainerCommand);
execContainerCmdAndLog(containerRunProcess, 0, true);
} catch (IOException e) {
Expand All @@ -1359,9 +1368,11 @@ private void startContainer() throws PluginExecutionException {
}
}

private Process getRunProcess(String command) throws IOException {
// Pass String[] containing command and all of its individual arguments.
// This ensures proper escaping/quoting for arguments that might contain spaces or special characters.
private Process getRunProcess(String[] command) throws IOException {
ProcessBuilder processBuilder = new ProcessBuilder();
processBuilder.command(getCommandTokens(command));
processBuilder.command(command);
Map<String, String> env = processBuilder.environment();
if (!OSUtil.isLinux()){
if (!env.keySet().contains("DOCKER_BUILDKIT")) { // don't touch if already set
Expand Down Expand Up @@ -1569,8 +1580,14 @@ protected static File getLongestCommonDir(File dir1, File dir2) {
* the CMD attribute of the Open Liberty docker image.
* @return the command string to use to start the container
*/
private String getContainerCommand() throws IOException, PluginExecutionException {
StringBuilder command = new StringBuilder(getContainerCommandPrefix() + "run --rm");
private String[] getContainerCommand() throws IOException, PluginExecutionException {
// Use List<String> for building the command. This will be converted to a String[] when passed to ProcessBuilder.
// The ProcessBuilder will handle quoting of paths (for blanks) as needed for various OS.
List<String> commandElements = new ArrayList<String>();
commandElements.add(getContainerCommandPrefix().trim());
commandElements.add("run");
commandElements.add("--rm");

if (!skipDefaultPorts) {
int httpPortToUse, httpsPortToUse;
try {
Expand All @@ -1581,8 +1598,11 @@ private String getContainerCommand() throws IOException, PluginExecutionExceptio
httpPortToUse = LIBERTY_DEFAULT_HTTP_PORT;
httpsPortToUse = LIBERTY_DEFAULT_HTTPS_PORT;
}
command.append(" -p ").append(httpPortToUse).append(":").append(LIBERTY_DEFAULT_HTTP_PORT);
command.append(" -p ").append(httpsPortToUse).append(":").append(LIBERTY_DEFAULT_HTTPS_PORT);
commandElements.add("-p");
commandElements.add(httpPortToUse+":"+LIBERTY_DEFAULT_HTTP_PORT);

commandElements.add("-p");
commandElements.add(httpsPortToUse+":"+LIBERTY_DEFAULT_HTTPS_PORT);
}

if (libertyDebug) {
Expand All @@ -1600,34 +1620,46 @@ private String getContainerCommand() throws IOException, PluginExecutionExceptio
} catch (IOException x) {
containerDebugPort = hostDebugPort = libertyDebugPort;
}
command.append(" -p " + hostDebugPort + ":" + containerDebugPort);
commandElements.add("-p");
commandElements.add(hostDebugPort+":"+containerDebugPort);
// set environment variables in the container to ensure debug mode does not suspend the server, and to enable a custom debug port to be used
// and to allow remote debugging into the container
command.append(" -e WLP_DEBUG_SUSPEND=n -e WLP_DEBUG_ADDRESS=" + containerDebugPort + " -e WLP_DEBUG_REMOTE=y");
commandElements.add("-e");
commandElements.add("WLP_DEBUG_SUSPEND=n");
commandElements.add("-e");
commandElements.add("WLP_DEBUG_ADDRESS=" + containerDebugPort);
commandElements.add("-e");
commandElements.add("WLP_DEBUG_REMOTE=y");
}

// mount potential directories containing .war.xml from devc specific folder - override /config/apps and /config/dropins
File tempApps = new File(buildDirectory, DEVC_HIDDEN_FOLDER + "/apps");
File tempDropins = new File(buildDirectory, DEVC_HIDDEN_FOLDER + "/dropins");
command.append(" -v " + tempApps + ":/config/apps");
command.append(" -v " + tempDropins + ":/config/dropins");
commandElements.add("-v");
commandElements.add(tempApps + ":/config/apps");

commandElements.add("-v");
commandElements.add(tempDropins + ":/config/dropins");

// mount the loose application resources in the container using the appropriate project root
File looseApplicationProjectRoot = getLooseAppProjectRoot(projectDirectory, multiModuleProjectDirectory);
command.append(" -v " + looseApplicationProjectRoot.getAbsolutePath() + ":" + DEVMODE_DIR_NAME);
commandElements.add("-v");
commandElements.add(looseApplicationProjectRoot.getAbsolutePath() + ":" + DEVMODE_DIR_NAME);

// mount the server logs directory over the /logs used by the open liberty container as defined by the LOG_DIR env. var.
File logsDir = new File(serverDirectory.getAbsolutePath(), "logs");
command.append(" -v " + logsDir + ":/logs");
commandElements.add("-v");
commandElements.add(logsDir + ":/logs");

// mount the Maven .m2 cache directory for featureUtility to use. For now, featureUtility does not support Gradle cache.
command.append(" -v " + mavenCacheLocation + ":/devmode-maven-cache");
commandElements.add("-v");
commandElements.add(mavenCacheLocation + ":/devmode-maven-cache");

// mount all files from COPY commands in the Containerfile to allow for hot deployment
command.append(getCopiedFiles());
addCopiedFiles(commandElements);

// Add a --user option when running Linux
command.append(getUserId());
addUserId(commandElements);

// Do not generate a name if the user has specified a name
String name = getContainerOption("--name");
Expand All @@ -1637,26 +1669,44 @@ private String getContainerCommand() throws IOException, PluginExecutionExceptio
// now generate a name so that the container errors make some sense to the user.
}
containerName = generateNewContainerName();
command.append(" --name " + containerName);
commandElements.add("--name");
commandElements.add(containerName);
} else {
containerName = name;
}
debug("containerName: " + containerName + ".");

// Allow the user to add their own options to this command via a system property.
if (containerRunOpts != null) {
command.append(" "+containerRunOpts);
// If something in containerRunOpts contains spaces and needs quoting/escaping, this getCommandTokens method won't handle it correctly.
// But what was here before would not either.
String[] runOpts = getCommandTokens(containerRunOpts);
for (int i=0; i < runOpts.length; i++) {
commandElements.add(runOpts[i]);
}
}

// Options must precede this in any order. Image name and command code follows.
command.append(" " + imageName);
commandElements.add(imageName);

// Command to start the server
command.append(" server" + ((libertyDebug) ? " debug " : " run ") + "defaultServer");
commandElements.add("server");
if (libertyDebug) {
commandElements.add("debug");
} else {
commandElements.add("run");
}
commandElements.add("defaultServer");

// All the Liberty variable definitions must appear after the -- option.
// Important: other Liberty options must appear before --
command.append(" -- --"+DEVMODE_PROJECT_ROOT+"="+DEVMODE_DIR_NAME);
commandElements.add("--");
commandElements.add("--"+DEVMODE_PROJECT_ROOT+"="+DEVMODE_DIR_NAME);

return command.toString();
//return command.toString();
String[] newCommand = commandElements.toArray(new String[commandElements.size()]);
info("Container command: "+String.join(" ", newCommand));
return newCommand;
}

/**
Expand Down Expand Up @@ -1769,33 +1819,32 @@ protected static String removeSurroundingQuotes(String str) {
}

// Read all the files from the array list.
private String getCopiedFiles() {
StringBuilder param = new StringBuilder(256); // estimate of size needed
private void addCopiedFiles(List<String> commandElements) {
for (int i=0; i < srcMount.size(); i++) {
if (new File(srcMount.get(i)).exists()) { // only Files are in this list
param.append(" -v ").append(srcMount.get(i)).append(":").append(destMount.get(i));
commandElements.add("-v");
commandElements.add(srcMount.get(i)+":"+destMount.get(i));
} else {
error("A file referenced by the Containerfile is not found: " + srcMount.get(i) +
". Update the Containerfile or ensure the file is in the correct location.");
}
}
return param.toString();
}

private String getUserId() {
private void addUserId(List<String> commandElements) {
if (OSUtil.isLinux() || !isDocker) {
try {
String id = runCmd("id -u");
if (id != null) {
return " --user " + id;
commandElements.add("--user");
commandElements.add(id);
}
} catch (IOException e) {
// can't get user id. runCmd has printed an error message.
} catch (InterruptedException e) {
// can't get user id. runCmd has printed an error message.
}
}
return "";
}

public abstract void libertyCreate() throws PluginExecutionException;
Expand Down
Loading