Forbid health checks longer than kill time

SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java

@@ -83,6 +83,7 @@ public class SingularityValidator {
   private final int maxMemoryMbPerRequest;
   private final int maxMemoryMbPerInstance;
   private final Optional<Integer> maxTotalHealthcheckTimeoutSeconds;
+  private final long defaultKillAfterNotHealthySeconds;
   private final int defaultHealthcheckIntervalSeconds;
   private final int defaultHealthcheckStartupTimeooutSeconds;
   private final int defaultHealthcehckMaxRetries;
@@ -128,6 +129,7 @@ public SingularityValidator(SingularityConfiguration configuration, DeployHistor
     this.allowBounceToSameHost = configuration.isAllowBounceToSameHost();
 
     this.maxTotalHealthcheckTimeoutSeconds = configuration.getHealthcheckMaxTotalTimeoutSeconds();
+    this.defaultKillAfterNotHealthySeconds = configuration.getKillAfterTasksDoNotRunDefaultSeconds();
     this.defaultHealthcheckIntervalSeconds = configuration.getHealthcheckIntervalSeconds();
     this.defaultHealthcheckStartupTimeooutSeconds = configuration.getStartupTimeoutSeconds();
     this.defaultHealthcehckMaxRetries = configuration.getHealthcheckMaxRetries().or(0);
@@ -302,10 +304,18 @@ public SingularityDeploy checkDeploy(SingularityRequest request, SingularityDepl
       int httpTimeoutSeconds = options.getResponseTimeoutSeconds().or(defaultHealthcheckResponseTimeoutSeconds);
       int startupTime = options.getStartupTimeoutSeconds().or(defaultHealthcheckStartupTimeooutSeconds);
       int attempts = options.getMaxRetries().or(defaultHealthcehckMaxRetries) + 1;
+
       checkBadRequest((startupTime + ((httpTimeoutSeconds + intervalSeconds) * attempts)) > maxTotalHealthcheckTimeoutSeconds.get(),
         String.format("Max healthcheck time cannot be greater than %s, (was startup timeout: %s, interval: %s, attempts: %s)", maxTotalHealthcheckTimeoutSeconds.get(), startupTime, intervalSeconds, attempts));
     }
 
+    if (deploy.getHealthcheck().isPresent() && deploy.getHealthcheck().get().getStartupDelaySeconds().isPresent()) {
+      int startUpDelay = deploy.getHealthcheck().get().getStartupDelaySeconds().get();
+
+      checkBadRequest(startUpDelay < defaultKillAfterNotHealthySeconds,
+          String.format("Health check startup delay time must be less than %s (was %s)", defaultKillAfterNotHealthySeconds, startUpDelay));
+    }
+
     checkBadRequest(deploy.getCommand().isPresent() && !deploy.getExecutorData().isPresent() ||
             deploy.getExecutorData().isPresent() && deploy.getCustomExecutorCmd().isPresent() && !deploy.getCommand().isPresent() ||
             deploy.getContainerInfo().isPresent(),

SingularityService/src/test/java/com/hubspot/singularity/data/ValidatorTest.java

@@ -1,11 +1,17 @@
 package com.hubspot.singularity.data;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.catchThrowable;
+
 import javax.ws.rs.WebApplicationException;
 
 import org.junit.Assert;
 import org.junit.Test;
 
+import com.google.common.base.Optional;
 import com.google.inject.Inject;
+import com.hubspot.deploy.HealthcheckOptions;
+import com.hubspot.deploy.HealthcheckOptionsBuilder;
 import com.hubspot.singularity.RequestType;
 import com.hubspot.singularity.SingularityDeploy;
 import com.hubspot.singularity.SingularityRequest;
@@ -44,14 +50,34 @@ public void itForbidsBracketCharactersInDeployIds() throws Exception {
     validator.checkDeploy(singularityRequest, singularityDeploy);
   }
 
-  @Test(expected = WebApplicationException.class)
+  @Test
   public void itForbidsQuotesInDeployIds() throws Exception {
     final String badDeployId = "deployKey'";
 
     SingularityDeploy singularityDeploy = SingularityDeploy.newBuilder(badDeployId, badDeployId).build();
     SingularityRequest singularityRequest = new SingularityRequestBuilder(badDeployId, RequestType.SERVICE).build();
 
-    validator.checkDeploy(singularityRequest, singularityDeploy);
+    WebApplicationException exn = (WebApplicationException) catchThrowable(() -> validator.checkDeploy(singularityRequest, singularityDeploy));
+    assertThat((String) exn.getResponse().getEntity())
+        .contains("[a-zA-Z0-9_]");
+  }
+
+  @Test
+  public void itForbidsHealthCheckStartupDelaysLongerThanKillWait() {
+    // Default kill wait time is 10 minutes (600 seconds)
+    HealthcheckOptions healthCheck = new HealthcheckOptionsBuilder("/")
+        .setPortNumber(Optional.of(8080L))
+        .setStartupDelaySeconds(Optional.of(10000))
+        .build();
+    SingularityDeploy deploy = SingularityDeploy
+        .newBuilder("1234567", "1234567")
+        .setHealthcheck(Optional.of(healthCheck))
+        .build();
+    SingularityRequest request = new SingularityRequestBuilder("1234567", RequestType.SERVICE).build();
+
+    WebApplicationException exn = (WebApplicationException) catchThrowable(() -> validator.checkDeploy(request, deploy));
+    assertThat((String) exn.getResponse().getEntity())
+        .contains("Health check startup delay");
   }
 
 }

SingularityUI/app/components/newDeployForm/NewDeployForm.jsx

@@ -1334,7 +1334,7 @@ class NewDeployForm extends Component {
         id="healthcheck-startup-timeout"
         onChange={event => this.updateField('startupTimeoutSeconds', event.target.value)}
         value={this.props.form.startupTimeoutSeconds}
-        label="HC startup delay"
+        label="HC startup timeout"
         placeholder="default: 30"
         feedback={this.formFieldFeedback(INDEXED_FIELDS.startupTimeoutSeconds, this.props.form.startupTimeoutSeconds)}
       />