resolved merge conflicts

Docs/features/expiring-actions.md

@@ -116,7 +116,7 @@ This URL accepts a JSON object with this format:
 **NOTE:** The `user` field has been removed from this object.
 
 #### Disabling request healthchecks
-- `/requests/request/{requestId}/skipHealthchecks`
+- `/requests/request/{requestId}/skip-healthchecks`
 
 This URL accepts a JSON object with this format:
 
@@ -130,6 +130,6 @@ This URL accepts a JSON object with this format:
 ### New endpoints for cancelling actions
 These endpoints were added in order to support cancelling certain actions:
 - `DELETE /requests/request/{requestId}/scale` -- Cancel an expiring scale
-- `DELETE /requests/request/{requestId}/skipHealthchecks` -- Cancel an expiring skip healthchecks override
+- `DELETE /requests/request/{requestId}/skip-healthchecks` -- Cancel an expiring skip healthchecks override
 - `DELETE /request/{requestId}/pause` -- Cancel (unpause) an expiring pause
 - `DELETE /request/{requestId}/bounce` -- Cancel a bounce

Docs/reference/api.md

@@ -825,7 +825,7 @@ Unpause a Singularity Request, scheduling new tasks immediately
 
 
 - - -
-#### **PUT** `/api/requests/request/{requestId}/skipHealthchecks`
+#### **PUT** `/api/requests/request/{requestId}/skip-healthchecks`
 
 Update the skipHealthchecks field for the request, possibly temporarily
 
@@ -853,7 +853,7 @@ Update the skipHealthchecks field for the request, possibly temporarily
 
 
 - - -
-#### **DELETE** `/api/requests/request/{requestId}/skipHealthchecks`
+#### **DELETE** `/api/requests/request/{requestId}/skip-healthchecks`
 
 Delete/cancel the expiring skipHealthchecks. This makes the skipHealthchecks request permanent.
 

Docs/reference/apidocs/api-requests.md

@@ -29,7 +29,7 @@ Unpause a Singularity Request, scheduling new tasks immediately
 
 
 - - -
-#### **PUT** `/api/requests/request/{requestId}/skipHealthchecks`
+#### **PUT** `/api/requests/request/{requestId}/skip-healthchecks`
 
 Update the skipHealthchecks field for the request, possibly temporarily
 
@@ -57,7 +57,7 @@ Update the skipHealthchecks field for the request, possibly temporarily
 
 
 - - -
-#### **DELETE** `/api/requests/request/{requestId}/skipHealthchecks`
+#### **DELETE** `/api/requests/request/{requestId}/skip-healthchecks`
 
 Delete/cancel the expiring skipHealthchecks. This makes the skipHealthchecks request permanent.
 

SingularityExecutor/pom.xml

@@ -40,6 +40,11 @@
       <artifactId>guava</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>com.github.rholder</groupId>
+      <artifactId>guava-retrying</artifactId>
+    </dependency>
+
     <dependency>
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/config/SingularityExecutorConfiguration.java

@@ -218,6 +218,9 @@ public class SingularityExecutorConfiguration extends BaseRunnerConfiguration {
   @JsonProperty
   private int dockerClientConnectionPoolSize = 5;
 
+  @JsonProperty
+  private int maxDockerPullAttempts = 2;
+
   @JsonProperty
   private Optional<SingularityExecutorDockerAuthConfig> dockerAuthConfig = Optional.absent();
 
@@ -631,6 +634,14 @@ public void setDockerClientConnectionPoolSize(int dockerClientConnectionPoolSize
     this.dockerClientConnectionPoolSize = dockerClientConnectionPoolSize;
   }
 
+  public int getMaxDockerPullAttempts() {
+    return maxDockerPullAttempts;
+  }
+
+  public void setMaxDockerPullAttempts(int maxDockerPullAttempts) {
+    this.maxDockerPullAttempts = maxDockerPullAttempts;
+  }
+
   public Optional<SingularityExecutorDockerAuthConfig> getDockerAuthConfig() {
     return dockerAuthConfig;
   }

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/config/SingularityExecutorModule.java

@@ -11,6 +11,8 @@
 import com.google.inject.Singleton;
 import com.google.inject.name.Named;
 import com.hubspot.singularity.executor.handlebars.BashEscapedHelper;
+import com.hubspot.singularity.executor.handlebars.EscapedNewLinesHelper;
+import com.hubspot.singularity.executor.handlebars.IfHasNewLinesHelper;
 import com.hubspot.singularity.executor.handlebars.IfPresentHelper;
 import com.hubspot.singularity.runner.base.config.SingularityRunnerBaseLogging;
 import com.ning.http.client.AsyncHttpClient;
@@ -90,6 +92,8 @@ public Handlebars providesHandlebars() {
 
     handlebars.registerHelper(BashEscapedHelper.NAME, new BashEscapedHelper());
     handlebars.registerHelper(IfPresentHelper.NAME, new IfPresentHelper());
+    handlebars.registerHelper(IfHasNewLinesHelper.NAME, new IfHasNewLinesHelper());
+    handlebars.registerHelper(EscapedNewLinesHelper.NAME, new EscapedNewLinesHelper());
 
     return handlebars;
   }

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/config/SingularityExecutorS3UploaderAdditionalFile.java

@@ -9,21 +9,24 @@ public class SingularityExecutorS3UploaderAdditionalFile {
     private final Optional<String> s3UploaderBucket;
     private final Optional<String> s3UploaderKeyPattern;
     private final Optional<String> s3UploaderFilenameHint;
+    private final Optional<String> directory;
 
     @JsonCreator
     public static SingularityExecutorS3UploaderAdditionalFile fromString(String value) {
-        return new SingularityExecutorS3UploaderAdditionalFile(value, Optional.<String>absent(), Optional.<String>absent(), Optional.<String>absent());
+        return new SingularityExecutorS3UploaderAdditionalFile(value, Optional.<String>absent(), Optional.<String>absent(), Optional.<String>absent(), Optional.<String>absent());
     }
 
     @JsonCreator
     public SingularityExecutorS3UploaderAdditionalFile(@JsonProperty("filename") String filename,
         @JsonProperty("s3UploaderBucket") Optional<String> s3UploaderBucket,
         @JsonProperty("s3UploaderKeyPattern") Optional<String> s3UploaderKeyPattern,
-        @JsonProperty("s3UploaderFilenameHint") Optional<String> s3UploaderFilenameHint) {
+        @JsonProperty("s3UploaderFilenameHint") Optional<String> s3UploaderFilenameHint,
+        @JsonProperty("directory") Optional<String> directory) {
         this.filename = filename;
         this.s3UploaderBucket = s3UploaderBucket;
         this.s3UploaderKeyPattern = s3UploaderKeyPattern;
         this.s3UploaderFilenameHint = s3UploaderFilenameHint;
+        this.directory = directory;
     }
 
     public String getFilename() {
@@ -42,13 +45,18 @@ public Optional<String> getS3UploaderFilenameHint() {
         return s3UploaderFilenameHint;
     }
 
+    public Optional<String> getDirectory() {
+        return directory;
+    }
+
     @Override
     public String toString() {
         return "SingularityExecutorS3UploaderAdditionalFile[" +
             "filename='" + filename + '\'' +
             ", s3UploaderBucket=" + s3UploaderBucket +
             ", s3UploaderKeyPattern=" + s3UploaderKeyPattern +
             ", s3UploaderFilenameHint=" + s3UploaderFilenameHint +
+            ", directory=" + directory +
             ']';
     }
 }

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/handlebars/EscapedNewLinesHelper.java

@@ -0,0 +1,35 @@
+package com.hubspot.singularity.executor.handlebars;
+
+import java.io.IOException;
+
+import com.github.jknack.handlebars.Helper;
+import com.github.jknack.handlebars.Options;
+
+public class EscapedNewLinesHelper implements Helper<Object> {
+
+  public static final String NAME = "escapedNewLines";
+
+  @Override
+  public CharSequence apply(Object context, Options options) throws IOException {
+    if (context == null) {
+      return "\"\"";
+    }
+
+    final StringBuilder sb = new StringBuilder();
+
+    sb.append('"');
+
+    for (char c : context.toString().toCharArray()) {
+      if (c == '\n') {
+        sb.append('\\');
+        sb.append('n');
+      } else {
+        sb.append(c);
+      }
+    }
+
+    sb.append('"');
+
+    return sb.toString();
+  }
+}

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/handlebars/IfHasNewLinesHelper.java

@@ -0,0 +1,23 @@
+package com.hubspot.singularity.executor.handlebars;
+
+
+import java.io.IOException;
+
+import com.github.jknack.handlebars.Helper;
+import com.github.jknack.handlebars.Options;
+
+public class IfHasNewLinesHelper implements Helper<Object> {
+
+  public static final String NAME = "ifHasNewLines";
+
+  @SuppressWarnings("unchecked")
+  @Override
+  public CharSequence apply(Object context, Options options) throws IOException {
+    if (context.toString().contains("\n")) {
+      return options.fn();
+    } else {
+      return options.inverse();
+    }
+  }
+
+}

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/task/SingularityExecutorTaskCleanup.java

@@ -5,14 +5,17 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.List;
+import java.util.concurrent.ExecutionException;
 
 import org.slf4j.Logger;
 
 import com.google.common.collect.ImmutableList;
 import com.hubspot.singularity.executor.config.SingularityExecutorConfiguration;
 import com.hubspot.singularity.executor.utils.DockerUtils;
+import com.hubspot.singularity.runner.base.shared.ExceptionChainParser;
 import com.hubspot.singularity.runner.base.shared.SimpleProcessManager;
 import com.spotify.docker.client.ContainerNotFoundException;
+import com.spotify.docker.client.DockerException;
 import com.spotify.docker.client.messages.ContainerInfo;
 
 public class SingularityExecutorTaskCleanup {
@@ -43,8 +46,13 @@ public TaskCleanupResult cleanup(boolean cleanupTaskAppDirectory, boolean isDock
           dockerUtils.stopContainer(containerName, configuration.getDockerStopTimeout());
         }
         dockerUtils.removeContainer(containerName, true);
-      } catch (ContainerNotFoundException e) {
-        log.trace("Container for task {} was already removed", taskDefinition.getTaskId());
+      } catch (DockerException e) {
+        if (ExceptionChainParser.exceptionChainContains(e, ContainerNotFoundException.class)) {
+          log.trace("Container for task {} was already removed", taskDefinition.getTaskId());
+        } else {
+          log.error("Could not ensure removal of container", e);
+          dockerCleanSuccess = false;
+        }
       } catch (Exception e) {
         log.error("Could not ensure removal of container", e);
         dockerCleanSuccess = false;

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/task/SingularityExecutorTaskLogManager.java

@@ -64,7 +64,9 @@ private boolean writeS3MetadataFileForRotatedFiles(boolean finished) {
 
     int index = 1;
     for (SingularityExecutorS3UploaderAdditionalFile additionalFile : configuration.getS3UploaderAdditionalFiles()) {
-      result = result && writeS3MetadataFile(additionalFile.getS3UploaderFilenameHint().or(String.format("extra%d", index)), logrotateDirectory, String.format("%s*.gz*", additionalFile.getFilename()), additionalFile.getS3UploaderBucket(), additionalFile.getS3UploaderKeyPattern(), finished);
+      Path directory = additionalFile.getDirectory().isPresent() ? taskDefinition.getTaskDirectoryPath().resolve(additionalFile.getDirectory().get()) : taskDefinition.getTaskDirectoryPath();
+      String fileGlob = additionalFile.getFilename() != null && additionalFile.getFilename().contains("*") ? additionalFile.getFilename() : String.format("%s*.gz*", additionalFile.getFilename());
+      result = result && writeS3MetadataFile(additionalFile.getS3UploaderFilenameHint().or(String.format("extra%d", index)), directory, fileGlob, additionalFile.getS3UploaderBucket(), additionalFile.getS3UploaderKeyPattern(), finished);
       index++;
     }
 

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/utils/DockerUtils.java

@@ -4,9 +4,12 @@
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.FutureTask;
 import java.util.concurrent.TimeUnit;
 
+import com.github.rholder.retry.AttemptTimeLimiters;
+import com.github.rholder.retry.RetryerBuilder;
+import com.github.rholder.retry.StopStrategies;
+import com.google.common.base.Optional;
 import com.google.inject.Inject;
 import com.hubspot.singularity.executor.config.SingularityExecutorConfiguration;
 import com.spotify.docker.client.DockerClient;
@@ -41,7 +44,7 @@ public ContainerInfo inspectContainer(final String containerName) throws DockerE
     };
 
     try {
-      return callWithTimeout(callable);
+      return callWithRetriesAndTimeout(callable);
     } catch (Exception e) {
       throw new DockerException(e);
     }
@@ -56,7 +59,7 @@ public void pull(final String imageName) throws DockerException {
     };
 
     try {
-      callWithTimeout(callable);
+      callWithRetriesAndTimeout(callable, Optional.of(configuration.getMaxDockerPullAttempts()));
     } catch (Exception e) {
       throw new DockerException(e);
     }
@@ -70,7 +73,7 @@ public List<Container> listContainers() throws DockerException {
     };
 
     try {
-      return callWithTimeout(callable);
+      return callWithRetriesAndTimeout(callable);
     } catch (Exception e) {
       throw new DockerException(e);
     }
@@ -85,7 +88,7 @@ public void stopContainer(final String containerId, final int timeout) throws Do
     };
 
     try {
-      callWithTimeout(callable);
+      callWithRetriesAndTimeout(callable);
     } catch (Exception e) {
       throw new DockerException(e);
     }
@@ -100,15 +103,22 @@ public void removeContainer(final String containerId, final boolean removeRunnin
     };
 
     try {
-      callWithTimeout(callable);
+      callWithRetriesAndTimeout(callable);
     } catch (Exception e) {
       throw new DockerException(e);
     }
   }
 
-  private <T> T callWithTimeout(Callable<T> callable) throws Exception {
-    FutureTask<T> task = new FutureTask<T>(callable);
-    executor.execute(task);
-    return task.get(configuration.getDockerClientTimeLimitSeconds(), TimeUnit.SECONDS);
+  private <T> T callWithRetriesAndTimeout(Callable<T> callable) throws Exception {
+    return callWithRetriesAndTimeout(callable, Optional.<Integer>absent());
+  }
+
+  private <T> T callWithRetriesAndTimeout(Callable<T> callable, Optional<Integer> retryCount) throws Exception {
+    RetryerBuilder<T> retryerBuilder = RetryerBuilder.<T>newBuilder()
+      .withAttemptTimeLimiter(AttemptTimeLimiters.<T>fixedTimeLimit(configuration.getDockerClientTimeLimitSeconds(), TimeUnit.SECONDS, executor));
+    if (retryCount.isPresent()) {
+      retryerBuilder.withStopStrategy(StopStrategies.stopAfterAttempt(retryCount.get()));
+    }
+    return retryerBuilder.build().call(callable);
   }
 }