Throw ContainerNotFoundException when container is missing

2 files changed, 46 insertions, 20 deletions

diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
index 93a95cd45cf..00753f59461 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.dockerapi;
 
 import com.github.dockerjava.api.DockerClient;
 import com.github.dockerjava.api.command.ExecCreateCmdResponse;
-import com.github.dockerjava.api.command.ExecStartCmd;
 import com.github.dockerjava.api.command.InspectContainerResponse;
 import com.github.dockerjava.api.command.InspectExecResponse;
 import com.github.dockerjava.api.command.InspectImageResponse;
@@ -22,6 +21,7 @@ import com.github.dockerjava.core.command.PullImageResultCallback;
 import com.github.dockerjava.jaxrs.JerseyDockerCmdExecFactory;
 import com.google.inject.Inject;
 import com.yahoo.log.LogLevel;
+import com.yahoo.vespa.hosted.dockerapi.exception.ContainerNotFoundException;
 import com.yahoo.vespa.hosted.dockerapi.exception.DockerException;
 import com.yahoo.vespa.hosted.dockerapi.exception.DockerExecTimeoutException;
 import com.yahoo.vespa.hosted.dockerapi.metrics.CounterWrapper;
@@ -192,6 +192,8 @@ public class DockerImpl implements Docker {
             dockerClient.connectToNetworkCmd()
                     .withContainerId(containerName.asString())
                     .withNetworkId(networkName).exec();
+        } catch (NotFoundException e) {
+            throw new ContainerNotFoundException(containerName);
         } catch (RuntimeException e) {
             numberOfDockerDaemonFails.add();
             throw new DockerException("Failed to connect container '" + containerName.asString() +
@@ -219,33 +221,27 @@ public class DockerImpl implements Docker {
      */
     private ProcessResult executeInContainerAsUser(ContainerName containerName, String user, Optional<Long> timeoutSeconds, String... command) {
         try {
-            final ExecCreateCmdResponse response = dockerClient.execCreateCmd(containerName.asString())
-                    .withCmd(command)
-                    .withAttachStdout(true)
-                    .withAttachStderr(true)
-                    .withUser(user)
-                    .exec();
+            ExecCreateCmdResponse response = execCreateCmd(containerName, user, command);
 
             ByteArrayOutputStream output = new ByteArrayOutputStream();
             ByteArrayOutputStream errors = new ByteArrayOutputStream();
-            ExecStartCmd execStartCmd = dockerClient.execStartCmd(response.getId());
-            ExecStartResultCallback callback = execStartCmd.exec(new ExecStartResultCallback(output, errors));
+            ExecStartResultCallback callback = dockerClient.execStartCmd(response.getId())
+                    .exec(new ExecStartResultCallback(output, errors));
 
             if (timeoutSeconds.isPresent()) {
-                if (!callback.awaitCompletion(timeoutSeconds.get(), TimeUnit.SECONDS)) {
-                    throw new DockerExecTimeoutException(String.format("Command '%s' did not finish within %s seconds.", command[0], timeoutSeconds));
-                }
+                if (!callback.awaitCompletion(timeoutSeconds.get(), TimeUnit.SECONDS))
+                    throw new DockerExecTimeoutException(String.format(
+                            "Command '%s' did not finish within %s seconds.", command[0], timeoutSeconds));
             } else {
                 // Wait for completion no timeout
                 callback.awaitCompletion();
             }
 
-            final InspectExecResponse state = dockerClient.inspectExecCmd(execStartCmd.getExecId()).exec();
-            assert !state.isRunning();
-            Integer exitCode = state.getExitCode();
-            assert exitCode != null;
+            InspectExecResponse state = dockerClient.inspectExecCmd(response.getId()).exec();
+            if (state.isRunning())
+                throw new DockerException("Command '%s' did not finish within %s seconds.");
 
-            return new ProcessResult(exitCode, new String(output.toByteArray()), new String(errors.toByteArray()));
+            return new ProcessResult(state.getExitCode(), new String(output.toByteArray()), new String(errors.toByteArray()));
         } catch (RuntimeException | InterruptedException e) {
             numberOfDockerDaemonFails.add();
             throw new DockerException("Container '" + containerName.asString()
@@ -253,6 +249,19 @@ public class DockerImpl implements Docker {
         }
     }
 
+    private ExecCreateCmdResponse execCreateCmd(ContainerName containerName, String user, String... command) {
+        try {
+            return dockerClient.execCreateCmd(containerName.asString())
+                    .withCmd(command)
+                    .withAttachStdout(true)
+                    .withAttachStderr(true)
+                    .withUser(user)
+                    .exec();
+        } catch (NotFoundException e) {
+            throw new ContainerNotFoundException(containerName);
+        }
+    }
+
     private Optional<InspectContainerResponse> inspectContainerCmd(String container) {
         try {
             return Optional.of(dockerClient.inspectContainerCmd(container).exec());
@@ -284,6 +293,8 @@ public class DockerImpl implements Docker {
     public void startContainer(ContainerName containerName) {
         try {
             dockerClient.startContainerCmd(containerName.asString()).exec();
+        } catch (NotFoundException e) {
+            throw new ContainerNotFoundException(containerName);
         } catch (NotModifiedException ignored) {
             // If is already started, ignore
         } catch (RuntimeException e) {
@@ -296,6 +307,8 @@ public class DockerImpl implements Docker {
     public void stopContainer(ContainerName containerName) {
         try {
             dockerClient.stopContainerCmd(containerName.asString()).withTimeout(secondsToWaitBeforeKilling).exec();
+        } catch (NotFoundException e) {
+            throw new ContainerNotFoundException(containerName);
         } catch (NotModifiedException ignored) {
             // If is already stopped, ignore
         } catch (RuntimeException e) {
@@ -313,8 +326,8 @@ public class DockerImpl implements Docker {
             });
 
             dockerClient.removeContainerCmd(containerName.asString()).exec();
-        } catch (NotFoundException ignored) {
-            // If container doesn't exist ignore
+        } catch (NotFoundException e) {
+            throw new ContainerNotFoundException(containerName);
         } catch (RuntimeException e) {
             numberOfDockerDaemonFails.add();
             throw new DockerException("Failed to delete container '" + containerName.asString() + "'", e);
@@ -351,7 +364,7 @@ public class DockerImpl implements Docker {
                 .orElse(Stream.empty());
     }
 
-    private boolean isManagedBy(final com.github.dockerjava.api.model.Container container, String manager) {
+    private boolean isManagedBy(com.github.dockerjava.api.model.Container container, String manager) {
         final Map<String, String> labels = container.getLabels();
         return labels != null && manager.equals(labels.get(LABEL_NAME_MANAGEDBY));
     }
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/ContainerNotFoundException.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/ContainerNotFoundException.java
new file mode 100644
index 00000000000..b237228ee8e
--- /dev/null
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/exception/ContainerNotFoundException.java
@@ -0,0 +1,13 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.dockerapi.exception;
+
+import com.yahoo.vespa.hosted.dockerapi.ContainerName;
+
+/**
+ * @author freva
+ */
+public class ContainerNotFoundException extends DockerException {
+    public ContainerNotFoundException(ContainerName containerName) {
+        super("No such container: " + containerName.asString());
+    }
+}