improve OPatch error troubleshooting (#180)

* improve error message when duplicate patches are specified on the command line

* Deprecated --type argument for AddPatchEntry (#179)

* print the OPatch log to stdout upon failure

* moved --type deprecation message to i18n

Author: ddsharpe

imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/cache/AddPatchEntry.java

@@ -31,7 +31,7 @@ public class AddPatchEntry extends CacheOperation {
     public CommandResponse call() throws Exception {
 
         if (type != null) {
-            logger.warning("[[cyan: --type]] is [[brightred: DEPRECATED]] and will be removed in an upcoming release.");
+            logger.warning("IMG-0078");
         }
 
         if (patchId != null && !patchId.isEmpty()

imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/menu/CommonOptions.java

@@ -6,6 +6,7 @@
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -112,7 +113,8 @@ DockerBuildCommand getInitialBuildCmd(String contextFolder) {
         logger.entering();
         DockerBuildCommand cmdBuilder = new DockerBuildCommand(contextFolder);
 
-        cmdBuilder.tag(imageTag)
+        cmdBuilder.forceRm(!skipcleanup)
+            .tag(imageTag)
             .network(buildNetwork)
             .pull(buildPull)
             .buildArg("http_proxy", httpProxyUrl)
@@ -229,13 +231,12 @@ void handlePatchFiles() throws Exception {
      * @param previousInventory existing inventory found in the "from" image
      * @throws Exception in case of error
      */
-    void handlePatchFiles(String previousInventory, String existingPsuVersion) throws Exception {
-        logger.entering(existingPsuVersion);
+    void handlePatchFiles(String previousInventory, String psuVersion) throws Exception {
+        logger.entering(psuVersion);
         if (!applyingPatches()) {
             return;
         }
 
-        String psuVersion = existingPsuVersion;
         String toPatchesPath = createPatchesTempDirectory().toAbsolutePath().toString();
 
         List<PatchFile> patchFiles = new ArrayList<>();
@@ -287,7 +288,11 @@ void handlePatchFiles(String previousInventory, String existingPsuVersion) throw
             String patchLocation = patch.resolve(cache());
             if (patchLocation != null && !Utils.isEmptyString(patchLocation)) {
                 File patchFile = new File(patchLocation);
-                Files.copy(Paths.get(patchLocation), Paths.get(toPatchesPath, patchFile.getName()));
+                try {
+                    Files.copy(Paths.get(patchLocation), Paths.get(toPatchesPath, patchFile.getName()));
+                } catch (FileAlreadyExistsException ee) {
+                    logger.warning("IMG-0077", patch.getKey());
+                }
             } else {
                 logger.severe("IMG-0024", patch.getKey());
             }
@@ -389,8 +394,7 @@ String getPassword() {
 
     @Option(
         names = {"--skipcleanup"},
-        description = "Do no delete Docker context folder or intermediate images.",
-        hidden = true
+        description = "Do no delete Docker context folder, intermediate images, and failed build container."
     )
     boolean skipcleanup = false;
 

imagetool/src/main/java/com/oracle/weblogic/imagetool/util/DockerBuildCommand.java

@@ -36,8 +36,7 @@ public class DockerBuildCommand {
     public DockerBuildCommand(String contextFolder) {
         Objects.requireNonNull(contextFolder);
         buildArgs = new ArrayList<>();
-        command = Stream.of("docker", "build",
-            "--force-rm=true", "--no-cache").collect(Collectors.toList());
+        command = Stream.of("docker", "build", "--no-cache").collect(Collectors.toList());
         context = contextFolder;
     }
 
@@ -59,6 +58,19 @@ public DockerBuildCommand tag(String value) {
         return this;
     }
 
+    /**
+     * Always remove intermediate containers if set to true.
+     * By default, Docker leaves intermediate containers when the build fails which is not ideal for CI/CD servers.
+     * @param value true to enable --force-rm on docker build.
+     * @return this
+     */
+    public DockerBuildCommand forceRm(boolean value) {
+        if (value) {
+            command.add("--force-rm");
+        }
+        return this;
+    }
+
     /**
      * Add a --build-arg to the Docker build command.
      * Conceal is defaulted to false.
@@ -137,7 +149,7 @@ public DockerBuildCommand run(Path dockerLog)
         logger.finer("Starting docker process...");
         final Process process = processBuilder.start();
         logger.finer("Docker process started");
-        writeFromInputToOutputStreams(process.getInputStream(), outputStreams.toArray(new OutputStream[0]));
+        writeFromInputToOutputStreams(process.getInputStream(), outputStreams);
         logger.finer("Waiting for Docker to finish");
         if (process.waitFor() != 0) {
             Utils.processError(process);
@@ -175,7 +187,7 @@ private Path createFile(Path filePath) {
         return logFilePath;
     }
 
-    private void writeFromInputToOutputStreams(InputStream inputStream, OutputStream... outputStreams) {
+    private void writeFromInputToOutputStreams(InputStream inputStream, List<OutputStream> outputStreams) {
         Thread readerThread = new Thread(() -> {
             try (
                 BufferedReader processReader = new BufferedReader(new InputStreamReader(inputStream));
@@ -196,12 +208,10 @@ private void writeFromInputToOutputStreams(InputStream inputStream, OutputStream
         readerThread.start();
     }
 
-    private CloseableList<PrintWriter> createPrintWriters(OutputStream... outputStreams) {
+    private CloseableList<PrintWriter> createPrintWriters(List<OutputStream> outputStreams) {
         CloseableList<PrintWriter> retVal = new CloseableList<>();
-        if (outputStreams != null) {
-            for (OutputStream outputStream : outputStreams) {
-                retVal.add(new PrintWriter(new OutputStreamWriter(outputStream), true));
-            }
+        for (OutputStream outputStream : outputStreams) {
+            retVal.add(new PrintWriter(new OutputStreamWriter(outputStream), true));
         }
         return retVal;
     }

imagetool/src/main/resources/ImageTool.properties

@@ -75,3 +75,5 @@ IMG-0073=Invalid file {0} listed for Verrazzano model
 IMG-0074=OPatch will not be updated, fromImage has version {0}, available version is {1}
 IMG-0075=Added patch entry [[cyan: {0}]]={1}
 IMG-0076=Cache key {0} already exists, remove it first
+IMG-0077=Skipping duplicate patch {0}. Patch file already exists in the build context folder. Did you accidentally list the patch twice?
+IMG-0078=[[cyan: --type]] is [[brightred: DEPRECATED]] and will be removed in an upcoming release.

imagetool/src/main/resources/docker-files/Create_Image.mustache

@@ -114,7 +114,9 @@ RUN cd {{{tempDir}}}/opatch \
 
 {{#isPatchingEnabled}}
 RUN {{{oracle_home}}}/OPatch/opatch napply -silent -oh {{{oracle_home}}} -phBaseDir {{{tempDir}}}/patches \
- && {{{oracle_home}}}/OPatch/opatch util cleanup -silent -oh {{{oracle_home}}}
+ && test $? -eq 0 \
+ && {{{oracle_home}}}/OPatch/opatch util cleanup -silent -oh {{{oracle_home}}} \
+ || (cat {{{oracle_home}}}/cfgtoollogs/opatch/opatch*.log && exit 1)
 {{/isPatchingEnabled}}
 
 {{#afterFmwInstall}}