Merge pull request #1572 from HubSpot/singularity-validator-bug

Fix bug when 'maxTotalHealthcheckTimeout' set
HubSpot · Jun 28, 2017 · c414433 · c414433
2 parents e01a2d2 + 2fa665c
commit c414433
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 1 deletion.
diff --git a/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java b/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java
@@ -318,7 +318,8 @@ public SingularityDeploy checkDeploy(SingularityRequest request,
       int startupTime = options.getStartupTimeoutSeconds().or(defaultHealthcheckStartupTimeoutSeconds);
       int attempts = options.getMaxRetries().or(defaultHealthcehckMaxRetries) + 1;
 
-      checkBadRequest((startupTime + ((httpTimeoutSeconds + intervalSeconds) * attempts)) > maxTotalHealthcheckTimeoutSeconds.get(),
+      int totalHealthCheckTime = startupTime + ((httpTimeoutSeconds + intervalSeconds) * attempts);
+      checkBadRequest(totalHealthCheckTime < 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));
     }
 

diff --git a/SingularityService/src/test/java/com/hubspot/singularity/data/ValidatorTest.java b/SingularityService/src/test/java/com/hubspot/singularity/data/ValidatorTest.java
@@ -10,6 +10,7 @@
 import javax.ws.rs.WebApplicationException;
 
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 
 import com.google.common.base.Optional;
@@ -24,14 +25,34 @@
 import com.hubspot.singularity.SingularityRequestBuilder;
 import com.hubspot.singularity.SingularityTaskId;
 import com.hubspot.singularity.SingularityTestBaseNoDb;
+import com.hubspot.singularity.config.SingularityConfiguration;
+import com.hubspot.singularity.config.UIConfiguration;
+import com.hubspot.singularity.data.history.DeployHistoryHelper;
 import com.hubspot.singularity.api.SingularityRunNowRequest;
 
 
 public class ValidatorTest extends SingularityTestBaseNoDb {
 
   @Inject
+  private SingularityConfiguration singularityConfiguration;
+  @Inject
+  private DeployHistoryHelper deployHistoryHelper;
+  @Inject
+  private PriorityManager priorityManager;
+  @Inject
+  private DisasterManager disasterManager;
+  @Inject
+  private SlaveManager slaveManager;
+  @Inject
+  private UIConfiguration uiConfiguration;
+
   private SingularityValidator validator;
 
+  @Before
+  public void createValidator() {
+    validator = new SingularityValidator(singularityConfiguration, deployHistoryHelper, priorityManager, disasterManager, slaveManager, uiConfiguration);
+  }
+
   /**
    * Standard cron: day of week (0 - 6) (0 to 6 are Sunday to Saturday, or use names; 7 is Sunday, the same as 0)
    * Quartz: 1-7 or SUN-SAT
@@ -271,4 +292,32 @@ public void itForbidsHealthCheckStartupDelaysLongerThanKillWait() {
         .contains("Health check startup delay");
   }
 
+  @Test
+  public void itForbidsHealthCheckGreaterThanMaxTotalHealthCheck() {
+    singularityConfiguration.setHealthcheckMaxTotalTimeoutSeconds(Optional.of(100));
+    createValidator();
+
+    // Total wait time on this request is (startup time) + ((interval) + (http timeout)) * (1 + retries)
+    // = 50 + (5 + 5) * (9 + 1)
+    // = 150
+    HealthcheckOptions healthCheck = new HealthcheckOptionsBuilder("/")
+        .setPortNumber(Optional.of(8080L))
+        .setStartupTimeoutSeconds(Optional.of(50))
+        .setIntervalSeconds(Optional.of(5))
+        .setResponseTimeoutSeconds(Optional.of(5))
+        .setMaxRetries(Optional.of(9))
+        .build();
+    SingularityDeploy deploy = SingularityDeploy
+        .newBuilder("1234567", "1234567")
+        .setHealthcheck(Optional.of(healthCheck))
+        .setCommand(Optional.of("sleep 100;"))
+        .build();
+    SingularityRequest request = new SingularityRequestBuilder("1234567", RequestType.SERVICE).build();
+
+    WebApplicationException exn = (WebApplicationException) catchThrowable(() -> validator.checkDeploy(request, deploy, Collections.emptyList(), Collections.emptyList()));
+    System.out.println(exn.getResponse().getEntity());
+    assertThat((String) exn.getResponse().getEntity())
+        .contains("Max healthcheck time");
+  }
+
 }