Polish #287

* No color if logging to a file * Append to file (user must rotate on its own) * Add documentation * Refactoring (the map holding the streams is not needed) * Add a property `docker.logStdout` to overwrite any configured file (useful when doing a `docker:log`)
fabric8io · Sep 23, 2015 · d7a4ca8 · d7a4ca8
1 parent 12ec3e3
commit d7a4ca8
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 67 deletions.
diff --git a/doc/changelog.md b/doc/changelog.md
@@ -3,6 +3,7 @@
 * **0.13.5**
   - Improvements for `docker:watch` (#288)
   - Add parameter `kill` to `<watch>` configuration for waiting before sending SIGKILL when stopping containers (#293)
+  - Add `file` for `<log>` to store the logout put in a file. Use `docker.logStdout` to show logs nevertheless to stdout (#287)
 
 * **0.13.4**
   - Support explicit exec arguments for `start.cmd` and `start.entrypoint`. ([#253](https://github.com/rhuss/docker-maven-plugin/issues/253)) 

diff --git a/doc/manual.md b/doc/manual.md
@@ -130,15 +130,17 @@ parentheses.
   to be always pulled. This is true for any base image during build and for any image 
   during run which has no `<build>` section. Valid values are `on|off|always`.
 * **authConfig** holds the authentication information when pulling from
-  or pushing to Docker registry. There is a dedicated
-  [section](#authentication) for how doing security.
+  or pushing to Docker registry. There is a dedicated section 
+  [Authentication](#authentication) for how doing security.
 * **logDate** (`docker.logDate`) specifies the date format which is used for printing out
   container logs. This configuration can be overwritten by individual
   run configurations and described below. The format is described in
-  the [section](#log-configuration) below. 
+  [Log configuration](#log-configuration) below. 
+* **logStdout** (`docker.logStdout`) if set, do all container logging to standard output, 
+  regardless whether a `file` for log output is specified. See also [Log configuration](#log-configuration)
 * **portPropertyFile** if given, specifies a global file into which the
   mapped properties should be written to. The format of this file and
-  its purpose are also described [below](#port-mapping).
+  its purpose are also described in [Port Mapping](#port-mapping).
 * **sourceDirectory** (`docker.source.dir`) specifies the default directory that contains
   the assembly descriptor(s) used by the plugin. The default value is `src/main/docker`. This
   option is only relevant for the `docker:build` goal.
@@ -900,6 +902,8 @@ configuring the log output:
   colors are `YELLOW`, `CYAN`, `MAGENTA`, `GREEN`, `RED`, `BLUE`. If
   coloring is enabled and now color is provided a color is picked for
   you. 
+* **file** Path to a file to which the log output is written. This file is overwritten 
+  for every run and colors are switched off. 
 
 Example (values can be case insensitive, too) :
 

diff --git a/src/main/java/org/jolokia/docker/maven/AbstractDockerMojo.java b/src/main/java/org/jolokia/docker/maven/AbstractDockerMojo.java
@@ -115,6 +115,10 @@ public abstract class AbstractDockerMojo extends AbstractMojo implements Context
     /** @parameter property = "docker.logDate" */
     private String logDate;
 
+    // Log to stdout regardless if log files are configured or not
+    /** @parameter property = "docker.logStdout" default-value = "false" */
+    private boolean logStdout;
+
     // Whether to skip docker altogether
     /** @parameter property = "docker.skip" default-value = "false" */
     private boolean skip;
@@ -297,7 +301,7 @@ protected AuthConfig prepareAuthConfig(String image, String configuredRegistry)
     protected LogDispatcher getLogDispatcher(DockerAccess docker) {
         LogDispatcher dispatcher = (LogDispatcher) getPluginContext().get(CONTEXT_KEY_LOG_DISPATCHER);
         if (dispatcher == null) {
-            dispatcher = new LogDispatcher(docker, useColor);
+            dispatcher = new LogDispatcher(docker, useColor, logStdout);
             getPluginContext().put(CONTEXT_KEY_LOG_DISPATCHER, dispatcher);
         }
         return dispatcher;
@@ -309,10 +313,9 @@ protected ContainerLogOutputSpec getContainerLogSpec(String containerId, ImageCo
 
         addLogFormat(builder, logConfig);
         addPrefix(builder, logConfig.getPrefix(), imageConfiguration.getAlias(), containerId);
-        addLogFile(builder, logConfig.getFileLocation());
-
-        builder.containerId(containerId)
-                .color(logConfig.getColor());
+        builder.file(logConfig.getFileLocation())
+               .containerId(containerId)
+               .color(logConfig.getColor());
 
         return builder.build();
     }
@@ -338,11 +341,6 @@ private void addLogFormat(ContainerLogOutputSpec.Builder builder, LogConfigurati
         }
     }
 
-    private void addLogFile(ContainerLogOutputSpec.Builder builder, String logFile) {
-        String file = logFile;
-        builder.file(file);
-    }
-
     private LogConfiguration extractLogConfiguration(ImageConfiguration imageConfiguration) {
         RunImageConfiguration runConfig = imageConfiguration.getRunConfiguration();
         LogConfiguration logConfig = null;

diff --git a/src/main/java/org/jolokia/docker/maven/LogsMojo.java b/src/main/java/org/jolokia/docker/maven/LogsMojo.java
@@ -1,9 +1,12 @@
 package org.jolokia.docker.maven;
 
+import java.io.FileNotFoundException;
+
 import org.apache.maven.plugin.MojoExecutionException;
 import org.jolokia.docker.maven.access.DockerAccess;
 import org.jolokia.docker.maven.access.DockerAccessException;
 import org.jolokia.docker.maven.config.ImageConfiguration;
+import org.jolokia.docker.maven.log.ContainerLogOutputSpec;
 import org.jolokia.docker.maven.log.LogDispatcher;
 import org.jolokia.docker.maven.model.Container;
 import org.jolokia.docker.maven.service.QueryService;
@@ -57,11 +60,16 @@ protected void executeInternal(DockerAccess access) throws MojoExecutionExceptio
         }
     }
 
-    private void doLogging(LogDispatcher logDispatcher, ImageConfiguration image, String container) {
-        if (follow) {
-            logDispatcher.trackContainerLog(container, getContainerLogSpec(container, image));
-        } else {
-            logDispatcher.fetchContainerLog(container, getContainerLogSpec(container, image));
+    private void doLogging(LogDispatcher logDispatcher, ImageConfiguration image, String container) throws MojoExecutionException {
+        ContainerLogOutputSpec spec = getContainerLogSpec(container, image);
+        try {
+            if (follow) {
+                logDispatcher.trackContainerLog(container, spec);
+            } else {
+                logDispatcher.fetchContainerLog(container, spec);
+            }
+        } catch (FileNotFoundException e) {
+            throw new MojoExecutionException("Can not log to file " + spec.getFile());
         }
     }
 

diff --git a/src/main/java/org/jolokia/docker/maven/log/LogDispatcher.java b/src/main/java/org/jolokia/docker/maven/log/LogDispatcher.java
@@ -32,89 +32,63 @@
 public class LogDispatcher {
 
     private final boolean withColor;
+    private final boolean logStdout;
     private Map<String,ContainerLogOutputSpec> outputSpecs;
     private Map<String,LogGetHandle> logHandles;
 
-    private HashMap<String, PrintStream> printStreamsMap;
-
     private DockerAccess dockerAccess;
 
-    public LogDispatcher(DockerAccess dockerAccess,boolean withColor) {
+    public LogDispatcher(DockerAccess dockerAccess, boolean withColor, boolean logStdout) {
         this.dockerAccess = dockerAccess;
         this.withColor = withColor;
+        this.logStdout = logStdout;
         outputSpecs = new HashMap<>();
-        printStreamsMap = new HashMap<String, PrintStream>();
         logHandles = new HashMap<>();
     }
 
-    public void addLogOutputStreams(String id, ContainerLogOutputSpec spec){
-        String logFile = spec.getFile();
-        try {
-            if (logFile == null) {
-                printStreamsMap.put(id, System.out);
-            } else {
-                printStreamsMap.put(id, new PrintStream(new FileOutputStream(logFile), true));
-            }
-        }catch (FileNotFoundException e) {
-            e.printStackTrace();
-        }
-    }
-
-    public synchronized void trackContainerLog(String id, ContainerLogOutputSpec spec) {
-        addLogOutputStreams(id, spec);
+    public synchronized void trackContainerLog(String id, ContainerLogOutputSpec spec) throws FileNotFoundException {
         outputSpecs.put(id, spec);
-
-        LogGetHandle handle = dockerAccess.getLogAsync(id, createLogCallBack(id));
+        LogGetHandle handle = dockerAccess.getLogAsync(id, createLogCallBack(id, spec.getFile()));
         logHandles.put(id, handle);
     }
 
-    public synchronized void fetchContainerLog(String id, ContainerLogOutputSpec spec) {
-        addLogOutputStreams(id, spec);
+    public synchronized void fetchContainerLog(String id, ContainerLogOutputSpec spec) throws FileNotFoundException {
         outputSpecs.put(id, spec);
-        dockerAccess.getLogSync(id,createLogCallBack(id));
+        dockerAccess.getLogSync(id,createLogCallBack(id,spec.getFile()));
     }
 
-    private LogCallback createLogCallBack(final String id) {
+    // =======================================================================================
+
+    private PrintStream getPrintStream(String file) throws FileNotFoundException {
+        return !logStdout && file != null ? new PrintStream(new FileOutputStream(file), true) : System.out;
+    }
+
+    private LogCallback createLogCallBack(final String id, final String file) throws FileNotFoundException {
+        final PrintStream ps = getPrintStream(file);
         return new LogCallback() {
             @Override
             public void log(int type, Timestamp timestamp, String txt) {
-                addLogEntry(new LogEntry(id, type, timestamp, txt));
+                addLogEntry(ps, new LogEntry(id, type, timestamp, txt, withColor && (file == null || logStdout)));
             }
 
             @Override
             public void error(String error) {
-                printError(id, error);
+                ps.println(error);
             }
         };
     }
 
-    private void addLogEntry(LogEntry logEntry) {
+    private void addLogEntry(PrintStream ps, LogEntry logEntry) {
         // TODO: Add the entry to a queue, and let the queue be picked up with a small delay from an extra
         // thread which then can sort the entries by time before printing it out in order to avoid race conditions.
 
-        ContainerLogOutputSpec spec = outputSpecs.get(logEntry.getContainerId());
+        String id = logEntry.getContainerId();
+        ContainerLogOutputSpec spec = outputSpecs.get(id);
         if (spec == null) {
             spec = ContainerLogOutputSpec.DEFAULT;
         }
 
-        // FIX me according to spec
-        print(spec.getContainerId(), spec.getPrompt(withColor,logEntry.getTimestamp()) + logEntry.getText());
-    }
-
-    private void printError(String id, String e) {
-        for (String containerId : printStreamsMap.keySet() ){
-            if (containerId == id){
-                printStreamsMap.get(id).println(e);
-            }
-        }
-    }
-
-    private void print(String id, String line) {
-        for (String containerId : printStreamsMap.keySet() ){
-            if (containerId == id) {
-                printStreamsMap.get(id).println(line);
-            }
-        }
+        ps.println(spec.getPrompt(logEntry.isWithColor(),logEntry.getTimestamp()) + logEntry.getText());
     }
 
     public synchronized void untrackAllContainerLogs() {
@@ -132,11 +106,14 @@ private class LogEntry implements Comparable<LogEntry> {
         private final Timestamp timestamp;
         private final String text;
 
-        public LogEntry(String containerId, int type, Timestamp timestamp, String text) {
+        private final boolean withColor;
+
+        public LogEntry(String containerId, int type, Timestamp timestamp, String text, boolean withColor) {
             this.containerId = containerId;
             this.type = type;
             this.timestamp = timestamp;
             this.text = text;
+            this.withColor = withColor;
         }
 
         public String getContainerId() {
@@ -155,6 +132,10 @@ public String getText() {
             return text;
         }
 
+        public boolean isWithColor() {
+            return withColor;
+        }
+
         @Override
         public int compareTo(LogEntry entry) {
             return timestamp.compareTo(entry.timestamp);