Merge pull request fabric8io#914 from isavin/post_start_exit_code_check

Fail the build if postStart/preStop commands return a non-zero exit code
transamericamoon · Jan 18, 2018 · 4f9a721 · 4f9a721
2 parents 15f8c94 + ee033d5
commit 4f9a721
Show file tree

Hide file tree

Showing 22 changed files with 232 additions and 92 deletions.
diff --git a/doc/changelog.md b/doc/changelog.md
@@ -8,8 +8,10 @@
   - Fix for hanging wait on log (#904)
   - Fix for credential helper which do not return a version (#896)
   - Also remove tagged images when calling `docker:remove` (#193)
-  - Introduce a `removeMode` for selecting the images to remove
-
+  - Introduced a `removeMode` for selecting the images to remove
+  - Introduced a `breakOnError`` for the `postStart` and `preStop` hooks in the 
+    wait configuration (#914)
+
 Please note that `autoPullMode` is deprecated now and the behaviour of the `autoPullMode == always` has been changed slightly so that now, it really always pulls the image from the registry. Also `removeAll` for `docker:remove` is deprecated in favor of `removeMode` (and the default mode has changed slightly). Please refer to the documentation for more information.
 
 * **0.23.0** (2017-11-04)

diff --git a/src/main/asciidoc/inc/external/_property_configuration.adoc b/src/main/asciidoc/inc/external/_property_configuration.adoc
@@ -249,6 +249,9 @@ when a `docker.from` or a `docker.fromExt` is set.
 | *docker.wait.exec.preStop*
 | Command to execute before command stops.
 
+| *docker.wait.exec.breakOnError*
+| If set to "true" then stop the build if the a `postStart` or `preStop` command failed
+
 | *docker.wait.shutdown*
 | Time in milliseconds to wait between stopping a container and removing it.
 

diff --git a/src/main/asciidoc/inc/start/_wait.adoc b/src/main/asciidoc/inc/start/_wait.adoc
@@ -34,6 +34,7 @@ a| Commands to execute during specified lifecycle of the container. It knows the
 
 * *postStart* Command to run after the above wait criteria has been met
 * *preStop* Command to run before the container is stopped.
+* *breakOnError* If set to `true` then break the build if a `postStart` or `preStop` command exits with an return code other than 0, otherwise only print an error message.
 
 | *tcp*
 a| TCP port check which periodically polls given tcp ports. It knows the following sub-elements:

diff --git a/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java b/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java
@@ -7,6 +7,7 @@
 
 import io.fabric8.maven.docker.access.DockerAccess;
 import io.fabric8.maven.docker.access.DockerAccessException;
+import io.fabric8.maven.docker.access.ExecException;
 import io.fabric8.maven.docker.config.ConfigHelper;
 import io.fabric8.maven.docker.config.DockerMachineConfiguration;
 import io.fabric8.maven.docker.config.ImageConfiguration;
@@ -218,7 +219,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
                 }
                 ServiceHub serviceHub = serviceHubFactory.createServiceHub(project, session, access, log, logSpecFactory);
                 executeInternal(serviceHub);
-            } catch (DockerAccessException exp) {
+            } catch (DockerAccessException | ExecException exp) {
                 logException(exp);
                 throw new MojoExecutionException(log.errorMessage(exp.getMessage()), exp);
             } catch (MojoExecutionException exp) {
@@ -340,7 +341,7 @@ protected boolean isDockerAccessRequired() {
      * @param serviceHub context for accessing backends
      */
     protected abstract void executeInternal(ServiceHub serviceHub)
-        throws DockerAccessException, MojoExecutionException;
+        throws DockerAccessException, ExecException, MojoExecutionException;
 
     // =============================================================================================
 

diff --git a/src/main/java/io/fabric8/maven/docker/StartMojo.java b/src/main/java/io/fabric8/maven/docker/StartMojo.java
@@ -14,6 +14,7 @@
 
 import com.google.common.util.concurrent.MoreExecutors;
 import io.fabric8.maven.docker.access.DockerAccessException;
+import io.fabric8.maven.docker.access.ExecException;
 import io.fabric8.maven.docker.access.PortMapping;
 import io.fabric8.maven.docker.config.*;
 import io.fabric8.maven.docker.log.LogDispatcher;
@@ -91,6 +92,7 @@ private StartedContainer(ImageConfiguration imageConfig, String containerId) {
      */
     @Override
     public synchronized void executeInternal(final ServiceHub hub) throws DockerAccessException,
+                                                                          ExecException,
                                                                           MojoExecutionException {
         if (skipRun) {
             return;
@@ -203,12 +205,14 @@ private void shutdownExecutorService(ExecutorService executorService) {
         }
     }
 
-    private void rethrowCause(ExecutionException e) throws IOException, InterruptedException {
+    private void rethrowCause(ExecutionException e) throws IOException, InterruptedException, ExecException {
         Throwable cause = e.getCause();
         if (cause instanceof RuntimeException) {
             throw (RuntimeException) cause;
         } else if (cause instanceof IOException) {
             throw (IOException) cause;
+        } else if (cause instanceof ExecException) {
+            throw (ExecException) cause;
         } else if (cause instanceof InterruptedException) {
                 throw (InterruptedException) cause;
         } else {
@@ -254,7 +258,15 @@ public StartedContainer call() throws Exception {
                 hub.getWaitService().wait(image, projProperties, containerId);
                 WaitConfiguration waitConfig = runConfig.getWaitConfiguration();
                 if (waitConfig != null && waitConfig.getExec() != null && waitConfig.getExec().getPostStart() != null) {
-                    runService.execInContainer(containerId, waitConfig.getExec().getPostStart(), image);
+                    try {
+                        runService.execInContainer(containerId, waitConfig.getExec().getPostStart(), image);
+                    } catch (ExecException exp) {
+                        if (waitConfig.getExec().isBreakOnError()) {
+                            throw exp;
+                        } else {
+                            log.warn("Cannot run postStart: %s", exp.getMessage());
+                        }
+                    }
                 }
 
                 return new StartedContainer(image, containerId);

diff --git a/src/main/java/io/fabric8/maven/docker/StopMojo.java b/src/main/java/io/fabric8/maven/docker/StopMojo.java
@@ -8,6 +8,7 @@
 import java.util.Set;
 
 import io.fabric8.maven.docker.access.DockerAccessException;
+import io.fabric8.maven.docker.access.ExecException;
 import io.fabric8.maven.docker.config.ImageConfiguration;
 import io.fabric8.maven.docker.config.NetworkConfig;
 import io.fabric8.maven.docker.config.RunImageConfiguration;
@@ -55,9 +56,8 @@ public class StopMojo extends AbstractDockerMojo {
     @Parameter( property = "docker.sledgeHammer", defaultValue = "false" )
     private boolean sledgeHammer;
 
-
     @Override
-    protected void executeInternal(ServiceHub hub) throws MojoExecutionException, DockerAccessException {
+    protected void executeInternal(ServiceHub hub) throws MojoExecutionException, DockerAccessException, ExecException {
         QueryService queryService = hub.getQueryService();
         RunService runService = hub.getRunService();
 
@@ -76,7 +76,7 @@ protected void executeInternal(ServiceHub hub) throws MojoExecutionException, Do
         dispatcher.untrackAllContainerLogs();
     }
 
-    private void stopContainers(QueryService queryService, RunService runService, PomLabel pomLabel) throws DockerAccessException {
+    private void stopContainers(QueryService queryService, RunService runService, PomLabel pomLabel) throws DockerAccessException, ExecException {
         Collection<Network> networksToRemove = getNetworksToRemove(queryService, pomLabel);
         for (ImageConfiguration image : getResolvedImages()) {
             for (Container container : getContainersToStop(queryService, image)) {

diff --git a/src/main/java/io/fabric8/maven/docker/access/DockerAccess.java b/src/main/java/io/fabric8/maven/docker/access/DockerAccess.java
@@ -9,7 +9,8 @@
 import io.fabric8.maven.docker.config.Arguments;
 import io.fabric8.maven.docker.log.LogOutputSpec;
 import io.fabric8.maven.docker.model.Container;
-import io.fabric8.maven.docker.model.InspectedContainer;
+import io.fabric8.maven.docker.model.ContainerDetails;
+import io.fabric8.maven.docker.model.ExecDetails;
 import io.fabric8.maven.docker.model.Network;
 
 /**
@@ -36,7 +37,16 @@ public interface DockerAccess {
      * @return <code>ContainerDetails<code> representing the container or null if none could be found
      * @throws DockerAccessException if the container could not be inspected
      */
-    InspectedContainer getContainer(String containerIdOrName) throws DockerAccessException;
+    ContainerDetails getContainer(String containerIdOrName) throws DockerAccessException;
+
+    /**
+     * Get an exec container which is the result of executing a command in a running container.
+     *
+     * @param containerIdOrName exec container id or name
+     * @return <code>ExecDetails<code> representing the container or null if none could be found
+     * @throws DockerAccessException if the container could not be inspected
+     */
+    ExecDetails getExecContainer(String containerIdOrName) throws DockerAccessException;
 
     /**
      * Check whether the given name exists as image at the docker daemon
@@ -65,8 +75,9 @@ public interface DockerAccess {
     List<Container> getContainersForImage(String image) throws DockerAccessException;
 
     /**
-     * Starts a previously set up exec instance id.
-     * this API sets up an interactive session with the exec command. Output is streamed to the log.
+     * Starts a previously set up exec instance (via {@link #createExecContainer(String, Arguments)} container
+     * this API sets up a session with the exec command. Output is streamed to the log. This methods
+     * returns only when the exec command has finished (i.e this method calls the command in a non-detached mode).
      *
      * @param containerId id of the exec container
      * @param outputSpec how to print out the output of the command

diff --git a/src/main/java/io/fabric8/maven/docker/access/ExecException.java b/src/main/java/io/fabric8/maven/docker/access/ExecException.java
@@ -0,0 +1,25 @@
+package io.fabric8.maven.docker.access;
+
+import java.util.Arrays;
+
+import io.fabric8.maven.docker.model.ContainerDetails;
+import io.fabric8.maven.docker.model.ExecDetails;
+
+/**
+ * Exception thrown when the execution of an exec container failed
+ *
+ * @author roland
+ * @since 18.01.18
+ */
+public class ExecException extends Exception {
+    public ExecException(ExecDetails details, ContainerDetails container) {
+        super(String.format(
+                "Executing '%s' with args '%s' inside container '%s' [%s](%s) resulted in a non-zero exit code: %d",
+                details.getEntryPoint(),
+                Arrays.toString(details.getArguments()),
+                container.getName(),
+                container.getImage(),
+                container.getId(),
+                details.getExitCode()));
+    }
+}
diff --git a/src/main/java/io/fabric8/maven/docker/access/UrlBuilder.java b/src/main/java/io/fabric8/maven/docker/access/UrlBuilder.java
@@ -75,6 +75,11 @@ public String inspectContainer(String containerId) {
                 .build();
     }
 
+    public String inspectExecContainer(String containerId) {
+        return u("exec/%s/json", containerId)
+                .build();
+    }
+
     public String listContainers(String ... filter) {
         Builder builder = u("containers/json");
         addFilters(builder, filter);

diff --git a/src/main/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClient.java b/src/main/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClient.java
@@ -276,7 +276,7 @@ public List<Container> getContainersForImage(String image) throws DockerAccessEx
     }
 
     @Override
-    public InspectedContainer getContainer(String containerIdOrName) throws DockerAccessException {
+    public ContainerDetails getContainer(String containerIdOrName) throws DockerAccessException {
         HttpBodyAndStatus response = inspectContainer(containerIdOrName);
         if (response.getStatusCode() == HTTP_NOT_FOUND) {
             return null;
@@ -285,6 +285,16 @@ public InspectedContainer getContainer(String containerIdOrName) throws DockerAc
         }
     }
 
+    @Override
+    public ExecDetails getExecContainer(String containerIdOrName) throws DockerAccessException {
+        HttpBodyAndStatus response = inspectExecContainer(containerIdOrName);
+        if (response.getStatusCode() == HTTP_NOT_FOUND) {
+            return null;
+        } else {
+            return new ExecDetails(new JSONObject(response.getBody()));
+        }
+    }
+
     private HttpBodyAndStatus inspectContainer(String containerIdOrName) throws DockerAccessException {
         try {
             String url = urlBuilder.inspectContainer(containerIdOrName);
@@ -294,6 +304,15 @@ private HttpBodyAndStatus inspectContainer(String containerIdOrName) throws Dock
         }
     }
 
+    private HttpBodyAndStatus inspectExecContainer(String containerIdOrName) throws DockerAccessException {
+        try {
+            String url = urlBuilder.inspectExecContainer(containerIdOrName);
+            return delegate.get(url, new BodyAndStatusResponseHandler(), HTTP_OK, HTTP_NOT_FOUND);
+        } catch (IOException e) {
+            throw new DockerAccessException(e, "Unable to retrieve container name for [%s]", containerIdOrName);
+        }
+    }
+
     @Override
     public boolean hasImage(String name) throws DockerAccessException {
         String url = urlBuilder.inspectImage(name);

diff --git a/src/main/java/io/fabric8/maven/docker/config/WaitConfiguration.java b/src/main/java/io/fabric8/maven/docker/config/WaitConfiguration.java
@@ -119,6 +119,7 @@ public static class Builder {
         private String tcpHost;
         private TcpConfigMode tcpMode;
         private Integer exit;
+        private Boolean breakOnError = false;
 
         public Builder time(int time) {
             this.time = time;
@@ -185,7 +186,7 @@ public Builder tcpMode(String tcpMode) {
 
         public WaitConfiguration build() {
             return new WaitConfiguration(time,
-                                         postStart != null || preStop != null ? new ExecConfiguration(postStart, preStop) : null,
+                                         postStart != null || preStop != null ? new ExecConfiguration(postStart, preStop, breakOnError != null ? breakOnError : false) : null,
                                          url != null ? new HttpConfiguration(url,method,status) : null,
                                          tcpPorts != null ? new TcpConfiguration(tcpMode, tcpHost, tcpPorts) : null,
                                          healthy,
@@ -204,6 +205,11 @@ public Builder postStart(String command) {
             this.postStart = command;
             return this;
         }
+
+        public Builder breakOnError(Boolean stop) {
+            this.breakOnError = stop;
+            return this;
+        }
     }
 
     public static class ExecConfiguration implements Serializable {
@@ -213,11 +219,15 @@ public static class ExecConfiguration implements Serializable {
         @Parameter
         private String preStop;
 
+        @Parameter
+        private boolean breakOnError;
+
         public ExecConfiguration() {}
 
-        public ExecConfiguration(String postStart, String preStop) {
+        public ExecConfiguration(String postStart, String preStop, boolean breakOnError) {
             this.postStart = postStart;
             this.preStop = preStop;
+            this.breakOnError = breakOnError;
         }
 
         public String getPostStart() {
@@ -227,6 +237,10 @@ public String getPostStart() {
         public String getPreStop() {
             return preStop;
         }
+
+        public boolean isBreakOnError() {
+            return breakOnError;
+        }
     }
 
     public static class HttpConfiguration implements Serializable {

diff --git a/src/main/java/io/fabric8/maven/docker/config/handler/property/ConfigKey.java b/src/main/java/io/fabric8/maven/docker/config/handler/property/ConfigKey.java
@@ -86,8 +86,6 @@ public enum ConfigKey {
     NETWORK_ALIAS("network.alias"),
     PORT_PROPERTY_FILE,
     PORTS,
-    POST_START("wait.exec.postStart"),
-    PRE_STOP("wait.exec.preStop"),
     PRIVILEGED,
     REGISTRY,
     RESTART_POLICY_NAME("restartPolicy.name"),
@@ -110,6 +108,9 @@ public enum ConfigKey {
     WAIT_HTTP_METHOD("wait.http.method"),
     WAIT_HTTP_STATUS("wait.http.status"),
     WAIT_KILL("wait.kill"),
+    WAIT_EXEC_POST_START("wait.exec.postStart"),
+    WAIT_EXEC_PRE_STOP("wait.exec.preStop"),
+    WAIT_EXEC_BREAK_ON_ERROR("wait.exec.breakOnError"),
     WAIT_EXIT("wait.exit"),
     WAIT_SHUTDOWN("wait.shutdown"),
     WAIT_TCP_MODE("wait.tcp.mode"),

diff --git a/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java b/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java
@@ -245,8 +245,9 @@ private WaitConfiguration extractWaitConfig(String prefix, Properties properties
         return new WaitConfiguration.Builder()
                 .time(asInt(withPrefix(prefix, WAIT_TIME,properties)))
                 .url(url)
-                .preStop(withPrefix(prefix, PRE_STOP, properties))
-                .postStart(withPrefix(prefix, POST_START, properties))
+                .preStop(withPrefix(prefix, WAIT_EXEC_PRE_STOP, properties))
+                .postStart(withPrefix(prefix, WAIT_EXEC_POST_START, properties))
+                .breakOnError(booleanWithPrefix(prefix, WAIT_EXEC_BREAK_ON_ERROR, properties))
                 .method(withPrefix(prefix, WAIT_HTTP_METHOD, properties))
                 .status(withPrefix(prefix, WAIT_HTTP_STATUS, properties))
                 .log(withPrefix(prefix, WAIT_LOG, properties))

diff --git a/src/main/java/io/fabric8/maven/docker/model/ContainerDetails.java b/src/main/java/io/fabric8/maven/docker/model/ContainerDetails.java
@@ -7,7 +7,7 @@
 import java.util.*;
 
 
-public class ContainerDetails implements InspectedContainer {
+public class ContainerDetails implements Container {
 
     static final String CONFIG = "Config";
     static final String CREATED = "Created";
@@ -136,14 +136,12 @@ public Integer getExitCode() {
         return state.getInt(EXIT_CODE);
     }
 
-    @Override
     public boolean isHealthy() {
         final JSONObject state = json.getJSONObject(STATE);
         // always indicate healthy for docker hosts that do not support health checks.
         return !state.has(HEALTH) || HEALTH_STATUS_HEALTHY.equals(state.getJSONObject(HEALTH).getString(STATUS));
     }
 
-    @Override
     public String getHealthcheck() {
         if (!json.getJSONObject(CONFIG).has(HEALTHCHECK) ||
             !json.getJSONObject(CONFIG).getJSONObject(HEALTHCHECK).has(TEST)) {