From 7a9bde53150ce93132de9a079ef12b593adcf2b3 Mon Sep 17 00:00:00 2001 From: Stephen Salinas Date: Thu, 6 Oct 2016 15:45:59 -0400 Subject: [PATCH] updates for PR comments --- .../hubspot/deploy/HealthcheckOptions.java | 28 ++++++++++--------- .../singularity/SingularityDeploy.java | 21 ++++++++++++++ .../singularity/SingularityDeployBuilder.java | 21 ++++++++++++++ .../data/SingularityValidator.java | 12 ++++---- .../SingularityHealthcheckAsyncHandler.java | 5 +--- 5 files changed, 65 insertions(+), 22 deletions(-) diff --git a/SingularityBase/src/main/java/com/hubspot/deploy/HealthcheckOptions.java b/SingularityBase/src/main/java/com/hubspot/deploy/HealthcheckOptions.java index 81631a9b52..29c7137476 100644 --- a/SingularityBase/src/main/java/com/hubspot/deploy/HealthcheckOptions.java +++ b/SingularityBase/src/main/java/com/hubspot/deploy/HealthcheckOptions.java @@ -42,12 +42,12 @@ public String getUri() { return uri; } - @ApiModelProperty(required = false, value="Perform healthcheck on this dynamically allocated port (e.g. 0 for first port), defaults to first port") + @ApiModelProperty(required=false, value="Perform healthcheck on this dynamically allocated port (e.g. 0 for first port), defaults to first port") public Optional getPortIndex() { return portIndex; } - @ApiModelProperty(required = false, value="Perform healthcheck on this port (portIndex cannot also be used when using this setting)") + @ApiModelProperty(required=false, value="Perform healthcheck on this port (portIndex cannot also be used when using this setting)") public Optional getPortNumber() { return portNumber; } @@ -95,21 +95,22 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) { return false; } - HealthcheckOptions that = (HealthcheckOptions) o; - return Objects.equal(uri, that.uri) && - Objects.equal(portIndex, that.portIndex) && - Objects.equal(portNumber, that.portNumber) && - Objects.equal(protocol, that.protocol) && - Objects.equal(startupTimeoutSeconds, that.startupTimeoutSeconds) && - Objects.equal(startupDelaySeconds, that.startupDelaySeconds) && - Objects.equal(intervalSeconds, that.intervalSeconds) && - Objects.equal(responseTimeoutSeconds, that.responseTimeoutSeconds) && - Objects.equal(maxRetries, that.maxRetries); + HealthcheckOptions options = (HealthcheckOptions) o; + return Objects.equal(uri, options.uri) && + Objects.equal(portIndex, options.portIndex) && + Objects.equal(portNumber, options.portNumber) && + Objects.equal(protocol, options.protocol) && + Objects.equal(startupTimeoutSeconds, options.startupTimeoutSeconds) && + Objects.equal(startupDelaySeconds, options.startupDelaySeconds) && + Objects.equal(startupIntervalSeconds, options.startupIntervalSeconds) && + Objects.equal(intervalSeconds, options.intervalSeconds) && + Objects.equal(responseTimeoutSeconds, options.responseTimeoutSeconds) && + Objects.equal(maxRetries, options.maxRetries); } @Override public int hashCode() { - return Objects.hashCode(uri, portIndex, portNumber, protocol, startupTimeoutSeconds, startupDelaySeconds, intervalSeconds, responseTimeoutSeconds, maxRetries); + return Objects.hashCode(uri, portIndex, portNumber, protocol, startupTimeoutSeconds, startupDelaySeconds, startupIntervalSeconds, intervalSeconds, responseTimeoutSeconds, maxRetries); } @Override @@ -121,6 +122,7 @@ public String toString() { .add("protocol", protocol) .add("startupTimeoutSeconds", startupTimeoutSeconds) .add("startupDelaySeconds", startupDelaySeconds) + .add("startupIntervalSeconds", startupIntervalSeconds) .add("intervalSeconds", intervalSeconds) .add("responseTimeoutSeconds", responseTimeoutSeconds) .add("maxRetries", maxRetries) diff --git a/SingularityBase/src/main/java/com/hubspot/singularity/SingularityDeploy.java b/SingularityBase/src/main/java/com/hubspot/singularity/SingularityDeploy.java index 2b6e56fe4e..cca519b3c0 100644 --- a/SingularityBase/src/main/java/com/hubspot/singularity/SingularityDeploy.java +++ b/SingularityBase/src/main/java/com/hubspot/singularity/SingularityDeploy.java @@ -50,18 +50,39 @@ public class SingularityDeploy { private final Optional>> mesosTaskLabels; private final Optional>> taskEnv; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private final Optional healthcheckUri; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private final Optional healthcheckIntervalSeconds; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private final Optional healthcheckTimeoutSeconds; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private final Optional healthcheckPortIndex; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private final Optional healthcheckProtocol; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private final Optional healthcheckMaxRetries; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private final Optional healthcheckMaxTotalTimeoutSeconds; diff --git a/SingularityBase/src/main/java/com/hubspot/singularity/SingularityDeployBuilder.java b/SingularityBase/src/main/java/com/hubspot/singularity/SingularityDeployBuilder.java index ccb3e1069a..61aca24ccb 100644 --- a/SingularityBase/src/main/java/com/hubspot/singularity/SingularityDeployBuilder.java +++ b/SingularityBase/src/main/java/com/hubspot/singularity/SingularityDeployBuilder.java @@ -41,18 +41,39 @@ public class SingularityDeployBuilder { private Optional>> taskLabels; private Optional>> mesosTaskLabels; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private Optional healthcheckUri; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private Optional healthcheckIntervalSeconds; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private Optional healthcheckTimeoutSeconds; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private Optional healthcheckPortIndex; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private Optional healthcheckProtocol; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private Optional healthcheckMaxRetries; + /** + * @deprecated use {@link #healthcheck} + */ @Deprecated private Optional healthcheckMaxTotalTimeoutSeconds; diff --git a/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java b/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java index dfc56ef7bb..78c66c6676 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java @@ -29,6 +29,7 @@ import com.google.common.collect.Lists; import com.google.common.hash.Hashing; import com.google.inject.Inject; +import com.hubspot.deploy.HealthcheckOptions; import com.hubspot.mesos.Resources; import com.hubspot.mesos.SingularityContainerInfo; import com.hubspot.mesos.SingularityContainerType; @@ -235,12 +236,13 @@ public SingularityDeploy checkDeploy(SingularityRequest request, SingularityDepl if (deploy.getResources().isPresent()) { if (deploy.getHealthcheck().isPresent()) { - checkBadRequest(!(deploy.getHealthcheck().get().getPortIndex().isPresent() && deploy.getHealthcheck().get().getPortNumber().isPresent()), + HealthcheckOptions healthcheck = deploy.getHealthcheck().get(); + checkBadRequest(!(healthcheck.getPortIndex().isPresent() && healthcheck.getPortNumber().isPresent()), "Can only specify one of portIndex or portNumber for healthchecks"); - if (deploy.getHealthcheck().get().getPortIndex().isPresent()) { - checkBadRequest(deploy.getHealthcheck().get().getPortIndex().get() >= 0, "healthcheckPortIndex must be greater than 0"); - checkBadRequest(deploy.getResources().get().getNumPorts() > deploy.getHealthcheck().get().getPortIndex().get(), String - .format("Must request %s ports for healthcheckPortIndex %s, only requested %s", deploy.getHealthcheck().get().getPortIndex().get() + 1, deploy.getHealthcheck().get().getPortIndex().get(), + if (healthcheck.getPortIndex().isPresent()) { + checkBadRequest(healthcheck.getPortIndex().get() >= 0, "healthcheckPortIndex must be greater than 0"); + checkBadRequest(deploy.getResources().get().getNumPorts() > healthcheck.getPortIndex().get(), String + .format("Must request %s ports for healthcheckPortIndex %s, only requested %s", healthcheck.getPortIndex().get() + 1, healthcheck.getPortIndex().get(), deploy.getResources().get().getNumPorts())); } } diff --git a/SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityHealthcheckAsyncHandler.java b/SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityHealthcheckAsyncHandler.java index 5d140ce36f..b1fe3ec0dc 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityHealthcheckAsyncHandler.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityHealthcheckAsyncHandler.java @@ -60,10 +60,7 @@ public void onThrowable(Throwable t) { } public void saveResult(Optional statusCode, Optional responseBody, Optional errorMessage, Optional throwable) { - boolean inStartup = false; - if (throwable.isPresent() && throwable.get() instanceof ConnectException) { - inStartup = true; - } + boolean inStartup = throwable.isPresent() && throwable.get() instanceof ConnectException; try { SingularityTaskHealthcheckResult result = new SingularityTaskHealthcheckResult(statusCode, Optional.of(System.currentTimeMillis() - startTime), startTime, responseBody,