fix issues

SingularityBase/src/main/java/com/hubspot/singularity/SingularityPendingRequest.java

@@ -1,6 +1,5 @@
 package com.hubspot.singularity;
 
-import java.util.Collections;
 import java.util.List;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
@@ -18,19 +17,19 @@ public enum PendingType {
   private final long timestamp;
   private final PendingType pendingType;
   private final Optional<String> user;
-  private final List<String> cmdLineArgsList;
+  private final Optional<List<String>> cmdLineArgsList;
   private final Optional<String> runId;
   private final Optional<Boolean> skipHealthchecks;
   private final Optional<String> message;
   private final Optional<String> actionId;
 
   public SingularityPendingRequest(String requestId, String deployId, long timestamp, Optional<String> user, PendingType pendingType, Optional<Boolean> skipHealthchecks, Optional<String> message) {
-    this(requestId, deployId, timestamp, user, pendingType, Collections.<String> emptyList(), Optional.<String> absent(), skipHealthchecks, message, Optional.<String> absent());
+    this(requestId, deployId, timestamp, user, pendingType, Optional.<List<String>> absent(), Optional.<String> absent(), skipHealthchecks, message, Optional.<String> absent());
   }
 
   @JsonCreator
   public SingularityPendingRequest(@JsonProperty("requestId") String requestId, @JsonProperty("deployId") String deployId, @JsonProperty("timestamp") long timestamp,
-      @JsonProperty("user") Optional<String> user, @JsonProperty("pendingType") PendingType pendingType, @JsonProperty("cmdLineArgsList") List<String> cmdLineArgsList,
+      @JsonProperty("user") Optional<String> user, @JsonProperty("pendingType") PendingType pendingType, @JsonProperty("cmdLineArgsList") Optional<List<String>> cmdLineArgsList,
       @JsonProperty("runId") Optional<String> runId, @JsonProperty("skipHealthchecks") Optional<Boolean> skipHealthchecks, @JsonProperty("message") Optional<String> message,
       @JsonProperty("actionId") Optional<String> actionId) {
     this.requestId = requestId;
@@ -73,7 +72,7 @@ public PendingType getPendingType() {
     return pendingType;
   }
 
-  public List<String> getCmdLineArgsList() {
+  public Optional<List<String>> getCmdLineArgsList() {
     return cmdLineArgsList;
   }
 

SingularityBase/src/main/java/com/hubspot/singularity/SingularityPendingTask.java

@@ -9,12 +9,11 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
-import com.hubspot.mesos.JavaUtils;
 
 public class SingularityPendingTask {
 
   private final SingularityPendingTaskId pendingTaskId;
-  private final List<String> cmdLineArgsList;
+  private final Optional<List<String>> cmdLineArgsList;
   private final Optional<String> user;
   private final Optional<String> runId;
   private final Optional<Boolean> skipHealthchecks;
@@ -42,11 +41,11 @@ public boolean apply(@Nonnull SingularityPendingTask input) {
   }
 
   @JsonCreator
-  public SingularityPendingTask(@JsonProperty("pendingTaskId") SingularityPendingTaskId pendingTaskId, @JsonProperty("cmdLineArgsList") List<String> cmdLineArgsList,
+  public SingularityPendingTask(@JsonProperty("pendingTaskId") SingularityPendingTaskId pendingTaskId, @JsonProperty("cmdLineArgsList") Optional<List<String>> cmdLineArgsList,
       @JsonProperty("user") Optional<String> user, @JsonProperty("runId") Optional<String> runId, @JsonProperty("skipHealthchecks") Optional<Boolean> skipHealthchecks) {
     this.pendingTaskId = pendingTaskId;
     this.user = user;
-    this.cmdLineArgsList = JavaUtils.nonNullImmutable(cmdLineArgsList);
+    this.cmdLineArgsList = cmdLineArgsList;
     this.runId = runId;
     this.skipHealthchecks = skipHealthchecks;
   }
@@ -79,7 +78,7 @@ public SingularityPendingTaskId getPendingTaskId() {
     return pendingTaskId;
   }
 
-  public List<String> getCmdLineArgsList() {
+  public Optional<List<String>> getCmdLineArgsList() {
     return cmdLineArgsList;
   }
 

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/shells/SingularityExecutorShellCommandRunner.java

@@ -139,10 +139,19 @@ public boolean apply(SingularityExecutorShellCommandDescriptor input) {
         int pid;
         Path pidFilePath = MesosUtils.getTaskDirectoryPath(getTask().getTaskId()).resolve(executorConfiguration.getShellCommandPidFile());
         if (Files.exists(pidFilePath)) {
+          Scanner scanner = null;
           try {
-            pid = Integer.parseInt(new Scanner(pidFilePath).useDelimiter("\\Z").next());
+            scanner = new Scanner(pidFilePath);
+            scanner.useDelimiter("\\Z");
+            pid = Integer.parseInt(scanner.next());
           } catch (Exception e) {
             throw new InvalidShellCommandException(String.format("No PID found due to exception reading pid file: %s", e.getMessage()));
+          } finally {
+            if (scanner != null) {
+              try {
+                scanner.close();
+              } catch (Throwable t) {}
+            }
           }
         } else if (isDocker) {
           pid = 1;

SingularityOOMKiller/src/main/java/com/hubspot/singularity/oomkiller/config/SingularityOOMKillerConfiguration.java

@@ -1,7 +1,6 @@
 package com.hubspot.singularity.oomkiller.config;
 
 import javax.validation.constraints.Min;
-import javax.validation.constraints.NotNull;
 
 import org.hibernate.validator.constraints.NotEmpty;
 
@@ -79,11 +78,11 @@ public void setCgroupProcsPathFormat(String cgroupProcsPathFormat) {
   @Override
   public String toString() {
     return "SingularityOOMKillerConfiguration[" +
-            "requestKillThresholdRatio=" + requestKillThresholdRatio +
-            ", killProcessDirectlyThresholdRatio=" + killProcessDirectlyThresholdRatio +
-            ", checkForOOMEveryMillis=" + checkForOOMEveryMillis +
-            ", slaveHostname='" + slaveHostname + '\'' +
-            ", cgroupProcsPathFormat='" + cgroupProcsPathFormat + '\'' +
-            ']';
+        "requestKillThresholdRatio=" + requestKillThresholdRatio +
+        ", killProcessDirectlyThresholdRatio=" + killProcessDirectlyThresholdRatio +
+        ", checkForOOMEveryMillis=" + checkForOOMEveryMillis +
+        ", slaveHostname='" + slaveHostname + '\'' +
+        ", cgroupProcsPathFormat='" + cgroupProcsPathFormat + '\'' +
+        ']';
   }
 }

SingularityS3Uploader/src/main/java/com/hubspot/singularity/s3uploader/SingularityS3UploaderDriver.java

@@ -29,7 +29,6 @@
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Optional;
-import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;

SingularityService/src/main/java/com/hubspot/singularity/data/zkmigrations/SingularityCmdLineArgsMigration.java

@@ -150,8 +150,8 @@ private void checkPendingTasks() {
     }
   }
 
-  private List<String> getCmdLineArgs(Optional<String> cmdLineArgs) {
-    return cmdLineArgs.isPresent() ? Collections.singletonList(cmdLineArgs.get()) : Collections.<String> emptyList();
+  private Optional<List<String>> getCmdLineArgs(Optional<String> cmdLineArgs) {
+    return cmdLineArgs.isPresent() ? Optional.of(Collections.singletonList(cmdLineArgs.get())) : Optional.<List<String>> absent();
   }
 
   private Optional<String> getCmdLineArgs(SingularityPendingTaskId pendingTaskId) throws Exception {

SingularityService/src/main/java/com/hubspot/singularity/data/zkmigrations/SingularityPendingTaskIdMigration.java

@@ -1,6 +1,7 @@
 package com.hubspot.singularity.data.zkmigrations;
 
 import java.util.Collections;
+import java.util.List;
 
 import javax.inject.Singleton;
 
@@ -57,8 +58,8 @@ public void applyMigration() {
 
           Optional<String> cmdLineArgs = getCmdLineArgs(pendingTaskId);
 
-          taskManager.savePendingTask(new SingularityPendingTask(newPendingTaskId, cmdLineArgs.isPresent() ? Collections.singletonList(cmdLineArgs.get()) :
-            Collections.<String> emptyList(), Optional.<String> absent(), Optional.<String> absent(), Optional.<Boolean> absent()));
+          taskManager.savePendingTask(new SingularityPendingTask(newPendingTaskId, cmdLineArgs.isPresent() ? Optional.of(Collections.singletonList(cmdLineArgs.get())) :
+            Optional.<List<String>> absent(), Optional.<String> absent(), Optional.<String> absent(), Optional.<Boolean> absent()));
 
           curator.delete().forPath(ZKPaths.makePath(PENDING_TASKS_ROOT, pendingTaskId));
         }

SingularityService/src/main/java/com/hubspot/singularity/mesos/SingularityMesosTaskBuilder.java

@@ -19,7 +19,6 @@
 import org.apache.mesos.Protos.Label;
 import org.apache.mesos.Protos.Labels;
 import org.apache.mesos.Protos.Labels.Builder;
-import org.apache.mesos.Protos.LabelsOrBuilder;
 import org.apache.mesos.Protos.Offer;
 import org.apache.mesos.Protos.Resource;
 import org.apache.mesos.Protos.TaskID;
@@ -197,12 +196,12 @@ private Optional<DockerInfo.PortMapping> buildPortMapping(final SingularityDocke
   private String fillInTaskIdValues(String string, Offer offer, SingularityTaskId taskId) {
     if (!Strings.isNullOrEmpty(string)) {
       string = string.replace("${TASK_REQUEST_ID}", taskId.getRequestId())
-              .replace("${TASK_DEPLOY_ID}", taskId.getDeployId())
-              .replace("${TASK_STARTED_AT}", Long.toString(taskId.getStartedAt()))
-              .replace("${TASK_INSTANCE_NO}", Integer.toString(taskId.getInstanceNo()))
-              .replace("${TASK_HOST}", offer.getHostname())
-              .replace("${TASK_RACK_ID}", slaveAndRackHelper.getRackIdOrDefault(offer))
-              .replace("${TASK_ID}", taskId.getId());
+          .replace("${TASK_DEPLOY_ID}", taskId.getDeployId())
+          .replace("${TASK_STARTED_AT}", Long.toString(taskId.getStartedAt()))
+          .replace("${TASK_INSTANCE_NO}", Integer.toString(taskId.getInstanceNo()))
+          .replace("${TASK_HOST}", offer.getHostname())
+          .replace("${TASK_RACK_ID}", slaveAndRackHelper.getRackIdOrDefault(offer))
+          .replace("${TASK_ID}", taskId.getId());
     }
 
     return string;
@@ -281,11 +280,11 @@ private void prepareCustomExecutor(final TaskInfo.Builder bldr, final Singularit
     }
 
     bldr.setExecutor(ExecutorInfo.newBuilder()
-            .setCommand(commandBuilder.build())
-            .setExecutorId(ExecutorID.newBuilder().setValue(task.getDeploy().getCustomExecutorId().or(idGenerator.getNextExecutorId())))
-            .setSource(task.getDeploy().getCustomExecutorSource().or(task.getPendingTask().getPendingTaskId().getId()))
-            .addAllResources(buildMesosResources(desiredExecutorResources))
-            .build()
+        .setCommand(commandBuilder.build())
+        .setExecutorId(ExecutorID.newBuilder().setValue(task.getDeploy().getCustomExecutorId().or(idGenerator.getNextExecutorId())))
+        .setSource(task.getDeploy().getCustomExecutorSource().or(task.getPendingTask().getPendingTaskId().getId()))
+        .addAllResources(buildMesosResources(desiredExecutorResources))
+        .build()
         );
 
     if (task.getDeploy().getExecutorData().isPresent()) {
@@ -299,14 +298,14 @@ private void prepareCustomExecutor(final TaskInfo.Builder bldr, final Singularit
         }
       }
 
-      if (!task.getPendingTask().getCmdLineArgsList().isEmpty()) {
+      if (task.getPendingTask().getCmdLineArgsList().isPresent() && !task.getPendingTask().getCmdLineArgsList().get().isEmpty()) {
         LOG.trace("Adding cmd line args {} to task {} executorData", task.getPendingTask().getCmdLineArgsList(), taskId.getId());
 
         final ImmutableList.Builder<String> extraCmdLineArgsBuilder = ImmutableList.builder();
         if (executorDataBldr.getExtraCmdLineArgs() != null && !executorDataBldr.getExtraCmdLineArgs().isEmpty()) {
           extraCmdLineArgsBuilder.addAll(executorDataBldr.getExtraCmdLineArgs());
         }
-        extraCmdLineArgsBuilder.addAll(task.getPendingTask().getCmdLineArgsList());
+        extraCmdLineArgsBuilder.addAll(task.getPendingTask().getCmdLineArgsList().get());
         executorDataBldr.setExtraCmdLineArgs(extraCmdLineArgsBuilder.build());
       }
 
@@ -336,7 +335,9 @@ private void prepareCommand(final TaskInfo.Builder bldr, final SingularityTaskId
       commandBldr.addAllArguments(task.getDeploy().getArguments().get());
     }
 
-    commandBldr.addAllArguments(task.getPendingTask().getCmdLineArgsList());
+    if (task.getPendingTask().getCmdLineArgsList().isPresent()) {
+      commandBldr.addAllArguments(task.getPendingTask().getCmdLineArgsList().get());
+    }
 
     if (task.getDeploy().getArguments().isPresent() ||
         // Hopefully temporary workaround for

SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java

@@ -204,7 +204,6 @@ public SingularityRequestParent bounce(@ApiParam("The request ID to bounce") @Pa
   })
   public SingularityPendingRequestParent scheduleImmediately(@ApiParam("The request ID to run") @PathParam("requestId") String requestId,
       @ApiParam("Run ID to associate with this task. If not specified, one will be generated") @QueryParam("runId") Optional<String> runId,
-      @ApiParam("Additional command line arguments to append to the task") List<String> commandLineArgs,
       Optional<SingularityRunNowRequest> runNowRequest) {
     SingularityRequestWithState requestWithState = fetchRequestWithState(requestId);
 
@@ -225,14 +224,13 @@ public SingularityPendingRequestParent scheduleImmediately(@ApiParam("The reques
 
     Optional<String> message = Optional.absent();
     Optional<Boolean> skipHealthchecks = Optional.absent();
+    Optional<List<String>> commandLineArgs = Optional.absent();
 
     if (runNowRequest.isPresent()) {
       message = runNowRequest.get().getMessage();
       runId = runNowRequest.get().getRunId();
       skipHealthchecks = runNowRequest.get().getSkipHealthchecks();
-      if (runNowRequest.get().getCommandLineArgs().isPresent()) {
-        commandLineArgs = runNowRequest.get().getCommandLineArgs().get();
-      }
+      commandLineArgs = runNowRequest.get().getCommandLineArgs();
     }
 
     if (!runId.isPresent()) {

SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityCleaner.java

@@ -300,7 +300,7 @@ private void bounce(SingularityRequestCleanup requestCleanup, final List<Singula
     }
 
     requestManager.addToPendingQueue(new SingularityPendingRequest(requestCleanup.getRequestId(), requestCleanup.getDeployId().get(), requestCleanup.getTimestamp(),
-        requestCleanup.getUser(), PendingType.BOUNCE, Collections.<String> emptyList(), Optional.<String> absent(), requestCleanup.getSkipHealthchecks(), requestCleanup.getMessage(), requestCleanup.getActionId()));
+        requestCleanup.getUser(), PendingType.BOUNCE, Optional.<List<String>> absent(), Optional.<String> absent(), requestCleanup.getSkipHealthchecks(), requestCleanup.getMessage(), requestCleanup.getActionId()));
 
     LOG.info("Added {} tasks for request {} to cleanup bounce queue in {}", matchingTaskIds.size(), requestCleanup.getRequestId(), JavaUtils.duration(now));
   }

SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityExpiringUserActionPoller.java

@@ -1,6 +1,5 @@
 package com.hubspot.singularity.scheduler;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
@@ -146,7 +145,7 @@ protected void handleExpiringObject(SingularityExpiringBounce expiringObject, Si
       }
 
       requestManager.addToPendingQueue(new SingularityPendingRequest(expiringObject.getRequestId(), expiringObject.getDeployId(), System.currentTimeMillis(), expiringObject.getUser(),
-          PendingType.CANCEL_BOUNCE, Collections.<String> emptyList(), Optional.<String> absent(), Optional.<Boolean> absent(), Optional.of(message), Optional.of(expiringObject.getActionId())));
+          PendingType.CANCEL_BOUNCE, Optional.<List<String>> absent(), Optional.<String> absent(), Optional.<Boolean> absent(), Optional.of(message), Optional.of(expiringObject.getActionId())));
     }
 
   }

SingularityService/src/test/java/com/hubspot/singularity/SingularityHistoryPurgerTest.java

@@ -176,7 +176,7 @@ public void testRunId() {
 
     String runId = "my-run-id";
 
-    SingularityPendingRequestParent parent = requestResource.scheduleImmediately(requestId, Optional.of(runId), Collections.<String> emptyList(), Optional.<SingularityRunNowRequest> absent());
+    SingularityPendingRequestParent parent = requestResource.scheduleImmediately(requestId, Optional.of(runId),  Optional.<SingularityRunNowRequest> absent());
 
     Assert.assertEquals(runId, parent.getPendingRequest().getRunId().get());
 
@@ -193,7 +193,7 @@ public void testRunId() {
     Assert.assertEquals(runId, historyManager.getTaskHistory(taskId.getId()).get().getTask().getTaskRequest().getPendingTask().getRunId().get());
     Assert.assertEquals(runId, historyManager.getTaskHistoryForRequest(requestId, 0, 10).get(0).getRunId().get());
 
-    parent = requestResource.scheduleImmediately(requestId, Optional.<String> absent(), Collections.<String> emptyList(), Optional.<SingularityRunNowRequest> absent());
+    parent = requestResource.scheduleImmediately(requestId, Optional.<String> absent(),  Optional.<SingularityRunNowRequest> absent());
 
     Assert.assertTrue(parent.getPendingRequest().getRunId().isPresent());
   }
@@ -206,7 +206,7 @@ public void testPersisterRaceCondition() {
     initScheduledRequest();
     initFirstDeploy();
 
-    requestResource.scheduleImmediately(requestId, Optional.<String>absent(), Collections.<String>emptyList(), Optional.<SingularityRunNowRequest> absent());
+    requestResource.scheduleImmediately(requestId, Optional.<String>absent(), Optional.<SingularityRunNowRequest> absent());
 
     resourceOffers();
 

SingularityService/src/test/java/com/hubspot/singularity/SingularitySchedulerTestBase.java

@@ -218,7 +218,7 @@ protected SingularityTask launchTask(SingularityRequest request, SingularityDepl
 
   protected SingularityPendingTask buildPendingTask(SingularityRequest request, SingularityDeploy deploy, long launchTime, int instanceNo) {
     SingularityPendingTaskId pendingTaskId = new SingularityPendingTaskId(request.getId(), deploy.getId(), launchTime, instanceNo, PendingType.IMMEDIATE, launchTime);
-    SingularityPendingTask pendingTask = new SingularityPendingTask(pendingTaskId, Collections.<String> emptyList(), Optional.<String> absent(), Optional.<String> absent(), Optional.<Boolean> absent());
+    SingularityPendingTask pendingTask = new SingularityPendingTask(pendingTaskId, Optional.<List<String>> absent(), Optional.<String> absent(), Optional.<String> absent(), Optional.<Boolean> absent());
 
     return pendingTask;
   }
@@ -464,7 +464,7 @@ protected SingularityPendingTask createAndSchedulePendingTask(String deployId) {
     SingularityPendingTaskId pendingTaskId = new SingularityPendingTaskId(requestId, deployId,
         System.currentTimeMillis() + TimeUnit.DAYS.toMillis(random.nextInt(3)), random.nextInt(10), PendingType.IMMEDIATE, System.currentTimeMillis());
 
-    SingularityPendingTask pendingTask = new SingularityPendingTask(pendingTaskId, Collections.<String> emptyList(), Optional.<String> absent(), Optional.<String> absent(), Optional.<Boolean> absent());
+    SingularityPendingTask pendingTask = new SingularityPendingTask(pendingTaskId, Optional.<List<String>> absent(), Optional.<String> absent(), Optional.<String> absent(), Optional.<Boolean> absent());
 
     taskManager.savePendingTask(pendingTask);
 

SingularityService/src/test/java/com/hubspot/singularity/data/zkmigrations/ZkMigrationTest.java

@@ -86,9 +86,9 @@ public void testCmdLineArgsMigration() throws Exception {
 
     migrationRunner.checkMigrations();
 
-    Assert.assertTrue(taskManager.getPendingTask(pt1).get().getCmdLineArgsList().isEmpty());
-    Assert.assertTrue(taskManager.getPendingTask(pt2).get().getCmdLineArgsList().get(0).equals("cmd line args"));
-    Assert.assertTrue(taskManager.getPendingTask(pt2).get().getCmdLineArgsList().size() == 1);
+    Assert.assertTrue(!taskManager.getPendingTask(pt1).get().getCmdLineArgsList().isPresent());
+    Assert.assertTrue(taskManager.getPendingTask(pt2).get().getCmdLineArgsList().get().get(0).equals("cmd line args"));
+    Assert.assertTrue(taskManager.getPendingTask(pt2).get().getCmdLineArgsList().get().size() == 1);
 
     Assert.assertTrue(taskManager.getPendingTaskIds().contains(pt1));
     Assert.assertTrue(taskManager.getPendingTaskIds().contains(pt2));
@@ -100,14 +100,14 @@ public void testCmdLineArgsMigration() throws Exception {
         Assert.assertEquals(r.getDeployId(), p1.getDeployId());
         Assert.assertEquals(r.getTimestamp(), p1.getTimestamp());
         Assert.assertEquals(r.getPendingType(), p1.getPendingType());
-        Assert.assertTrue(r.getCmdLineArgsList().isEmpty());
+        Assert.assertTrue(!r.getCmdLineArgsList().isPresent());
         Assert.assertEquals(r.getUser(), p1.getUser());
       } else {
         Assert.assertEquals(r.getDeployId(), p2.getDeployId());
         Assert.assertEquals(r.getTimestamp(), p2.getTimestamp());
         Assert.assertEquals(r.getPendingType(), p2.getPendingType());
-        Assert.assertTrue(r.getCmdLineArgsList().size() == 1);
-        Assert.assertTrue(r.getCmdLineArgsList().get(0).equals("cmd line args"));
+        Assert.assertTrue(r.getCmdLineArgsList().get().size() == 1);
+        Assert.assertTrue(r.getCmdLineArgsList().get().get(0).equals("cmd line args"));
         Assert.assertEquals(r.getUser(), p2.getUser());
       }
     }