From 77cb958b25caf6c4ef2fd86b49e66a1e536616c9 Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Tue, 9 Aug 2016 12:13:39 -0400 Subject: [PATCH 01/13] Guarantee durationMillis is present in getExpiringBounce response --- .../singularity/data/RequestManager.java | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java index c54528bc25..a9eca4189e 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java @@ -3,6 +3,7 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.utils.ZKPaths; @@ -31,6 +32,7 @@ import com.hubspot.singularity.SingularityRequestHistory.RequestHistoryType; import com.hubspot.singularity.SingularityRequestLbCleanup; import com.hubspot.singularity.SingularityRequestWithState; +import com.hubspot.singularity.api.SingularityBounceRequest; import com.hubspot.singularity.api.SingularityExpiringRequestParent; import com.hubspot.singularity.config.SingularityConfiguration; import com.hubspot.singularity.data.transcoders.Transcoder; @@ -53,6 +55,7 @@ public class RequestManager extends CuratorAsyncManager { private final Transcoder requestLbCleanupTranscoder; private final SingularityEventListener singularityEventListener; + private final SingularityConfiguration singularityConfiguration; private static final String REQUEST_ROOT = "/requests"; @@ -80,7 +83,8 @@ public class RequestManager extends CuratorAsyncManager { public RequestManager(CuratorFramework curator, SingularityConfiguration configuration, MetricRegistry metricRegistry, SingularityEventListener singularityEventListener, Transcoder requestCleanupTranscoder, Transcoder requestTranscoder, Transcoder requestLbCleanupTranscoder, Transcoder pendingRequestTranscoder, Transcoder requestHistoryTranscoder, Transcoder expiringBounceTranscoder, - Transcoder expiringScaleTranscoder, Transcoder expiringPauseTranscoder, Transcoder expiringSkipHealthchecksTranscoder) { + Transcoder expiringScaleTranscoder, Transcoder expiringPauseTranscoder, Transcoder expiringSkipHealthchecksTranscoder, + SingularityConfiguration singularityConfiguration) { super(curator, configuration, metricRegistry); this.requestTranscoder = requestTranscoder; this.requestCleanupTranscoder = requestCleanupTranscoder; @@ -88,6 +92,7 @@ public RequestManager(CuratorFramework curator, SingularityConfiguration configu this.requestHistoryTranscoder = requestHistoryTranscoder; this.singularityEventListener = singularityEventListener; this.requestLbCleanupTranscoder = requestLbCleanupTranscoder; + this.singularityConfiguration = singularityConfiguration; this.expiringTranscoderMap = ImmutableMap.of( SingularityExpiringBounce.class, expiringBounceTranscoder, @@ -385,7 +390,31 @@ public getExpiringBounce(String requestId) { - return getExpiringObject(SingularityExpiringBounce.class, requestId); + final Optional optionalBounce = getExpiringObject(SingularityExpiringBounce.class, requestId); + if (!optionalBounce.isPresent()) { + return optionalBounce; + } + final SingularityExpiringBounce bounce = optionalBounce.get(); + if (bounce.getExpiringAPIRequestObject().getDurationMillis().isPresent()) { + return optionalBounce; + } + final Long durationMillis = TimeUnit.MINUTES.toMillis(singularityConfiguration.getDefaultBounceExpirationMinutes()); + return Optional.of( + new SingularityExpiringBounce( + bounce.getRequestId(), + bounce.getDeployId(), + bounce.getUser(), + bounce.getStartMillis(), + new SingularityBounceRequest( + bounce.getExpiringAPIRequestObject().getIncremental(), + bounce.getExpiringAPIRequestObject().getSkipHealthchecks(), + Optional.of(durationMillis), + bounce.getExpiringAPIRequestObject().getActionId(), + bounce.getExpiringAPIRequestObject().getMessage() + ), + bounce.getActionId() + ) + ); } public Optional getExpiringPause(String requestId) { From 5afce17632ad24d15259a8b6395ba020f9a8c879 Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Tue, 9 Aug 2016 12:49:31 -0400 Subject: [PATCH 02/13] Rename optionalBounce -> maybeExpiringBounce --- .../com/hubspot/singularity/data/RequestManager.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java index a9eca4189e..3c60537075 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java @@ -390,13 +390,13 @@ public getExpiringBounce(String requestId) { - final Optional optionalBounce = getExpiringObject(SingularityExpiringBounce.class, requestId); - if (!optionalBounce.isPresent()) { - return optionalBounce; + final Optional maybeExpiringBounce = getExpiringObject(SingularityExpiringBounce.class, requestId); + if (!maybeExpiringBounce.isPresent()) { + return maybeExpiringBounce; } - final SingularityExpiringBounce bounce = optionalBounce.get(); + final SingularityExpiringBounce bounce = maybeExpiringBounce.get(); if (bounce.getExpiringAPIRequestObject().getDurationMillis().isPresent()) { - return optionalBounce; + return maybeExpiringBounce; } final Long durationMillis = TimeUnit.MINUTES.toMillis(singularityConfiguration.getDefaultBounceExpirationMinutes()); return Optional.of( From 298f71cdd735e296526a0e03f8997caefb5ad97c Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Tue, 9 Aug 2016 13:57:27 -0400 Subject: [PATCH 03/13] Move ExpiringBounce defaultMillis logic out of RequestManager --- .../expiring/SingularityExpiringBounce.java | 28 ++++++++++++++++++ .../singularity/data/RequestManager.java | 29 ++----------------- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/SingularityBase/src/main/java/com/hubspot/singularity/expiring/SingularityExpiringBounce.java b/SingularityBase/src/main/java/com/hubspot/singularity/expiring/SingularityExpiringBounce.java index 2b708d0c98..d188e2d057 100644 --- a/SingularityBase/src/main/java/com/hubspot/singularity/expiring/SingularityExpiringBounce.java +++ b/SingularityBase/src/main/java/com/hubspot/singularity/expiring/SingularityExpiringBounce.java @@ -20,6 +20,34 @@ public String getDeployId() { return deployId; } + public static Optional withDefaultExpiringMillis( + Optional maybeExpiringBounce, + Long durationMillis) { + if (!maybeExpiringBounce.isPresent()) { + return maybeExpiringBounce; + } + final SingularityExpiringBounce bounce = maybeExpiringBounce.get(); + if (bounce.getExpiringAPIRequestObject().getDurationMillis().isPresent()) { + return maybeExpiringBounce; + } + return Optional.of( + new SingularityExpiringBounce( + bounce.getRequestId(), + bounce.getDeployId(), + bounce.getUser(), + bounce.getStartMillis(), + new SingularityBounceRequest( + bounce.getExpiringAPIRequestObject().getIncremental(), + bounce.getExpiringAPIRequestObject().getSkipHealthchecks(), + Optional.of(durationMillis), + bounce.getExpiringAPIRequestObject().getActionId(), + bounce.getExpiringAPIRequestObject().getMessage() + ), + bounce.getActionId() + ) + ); + } + @Override public String toString() { return "SingularityExpiringBounce [toString()=" + super.toString() + "]"; diff --git a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java index 3c60537075..f9bc78d325 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java @@ -32,7 +32,6 @@ import com.hubspot.singularity.SingularityRequestHistory.RequestHistoryType; import com.hubspot.singularity.SingularityRequestLbCleanup; import com.hubspot.singularity.SingularityRequestWithState; -import com.hubspot.singularity.api.SingularityBounceRequest; import com.hubspot.singularity.api.SingularityExpiringRequestParent; import com.hubspot.singularity.config.SingularityConfiguration; import com.hubspot.singularity.data.transcoders.Transcoder; @@ -390,31 +389,9 @@ public getExpiringBounce(String requestId) { - final Optional maybeExpiringBounce = getExpiringObject(SingularityExpiringBounce.class, requestId); - if (!maybeExpiringBounce.isPresent()) { - return maybeExpiringBounce; - } - final SingularityExpiringBounce bounce = maybeExpiringBounce.get(); - if (bounce.getExpiringAPIRequestObject().getDurationMillis().isPresent()) { - return maybeExpiringBounce; - } - final Long durationMillis = TimeUnit.MINUTES.toMillis(singularityConfiguration.getDefaultBounceExpirationMinutes()); - return Optional.of( - new SingularityExpiringBounce( - bounce.getRequestId(), - bounce.getDeployId(), - bounce.getUser(), - bounce.getStartMillis(), - new SingularityBounceRequest( - bounce.getExpiringAPIRequestObject().getIncremental(), - bounce.getExpiringAPIRequestObject().getSkipHealthchecks(), - Optional.of(durationMillis), - bounce.getExpiringAPIRequestObject().getActionId(), - bounce.getExpiringAPIRequestObject().getMessage() - ), - bounce.getActionId() - ) - ); + return SingularityExpiringBounce.withDefaultExpiringMillis( + getExpiringObject(SingularityExpiringBounce.class, requestId), + TimeUnit.MINUTES.toMillis(singularityConfiguration.getDefaultBounceExpirationMinutes())); } public Optional getExpiringPause(String requestId) { From 008eaa255478dd57b08b11064f9affaba4da1364 Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Tue, 9 Aug 2016 16:11:52 -0400 Subject: [PATCH 04/13] Implement BounceRequestBuilder, update durationMillis of /bounce response value --- .../api/SingularityBounceRequest.java | 9 ++ .../api/SingularityBounceRequestBuilder.java | 82 +++++++++++++++++++ .../resources/RequestResource.java | 12 ++- 3 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequestBuilder.java diff --git a/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequest.java b/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequest.java index d5f62fca11..954c067d69 100644 --- a/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequest.java +++ b/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequest.java @@ -24,6 +24,15 @@ public static SingularityBounceRequest defaultRequest() { return new SingularityBounceRequest(Optional.absent(), Optional.absent(), Optional.absent(), Optional.of(UUID.randomUUID().toString()), Optional.absent()); } + public SingularityBounceRequestBuilder toBuilder() { + return new SingularityBounceRequestBuilder() + .setIncremental(incremental) + .setSkipHealthchecks(skipHealthchecks) + .setDurationMillis(getDurationMillis()) + .setActionId(getActionId()) + .setMessage(getMessage()); + } + @ApiModelProperty(required=false, value="If present and set to true, old tasks will be killed as soon as replacement tasks are available, instead of waiting for all replacement tasks to be healthy") public Optional getIncremental() { return incremental; diff --git a/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequestBuilder.java b/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequestBuilder.java new file mode 100644 index 0000000000..b7e9f92ee4 --- /dev/null +++ b/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequestBuilder.java @@ -0,0 +1,82 @@ +package com.hubspot.singularity.api; + +import com.google.common.base.Optional; + +public class SingularityBounceRequestBuilder { + + private Optional incremental; + private Optional skipHealthchecks; + + private Optional durationMillis; + private Optional actionId; + private Optional message; + + public SingularityBounceRequestBuilder() { + this.incremental = Optional.absent(); + this.skipHealthchecks = Optional.absent(); + + this.durationMillis = Optional.absent(); + this.actionId = Optional.absent(); + this.message = Optional.absent(); + } + + public SingularityBounceRequest build() { + return new SingularityBounceRequest(incremental, skipHealthchecks, durationMillis, actionId, message); + } + + public Optional getDurationMillis() { + return durationMillis; + } + + public SingularityBounceRequestBuilder setDurationMillis(Optional durationMillis) { + this.durationMillis = durationMillis; + return this; + } + + public Optional getActionId() { + return actionId; + } + + public SingularityBounceRequestBuilder setActionId(Optional actionId) { + this.actionId = actionId; + return this; + } + + public Optional getMessage() { + return message; + } + + public SingularityBounceRequestBuilder setMessage(Optional message) { + this.message = message; + return this; + } + + public Optional getIncremental() { + return incremental; + } + + public SingularityBounceRequestBuilder setIncremental(Optional incremental) { + this.incremental = incremental; + return this; + } + + public Optional getSkipHealthchecks() { + return skipHealthchecks; + } + + public SingularityBounceRequestBuilder setSkipHealthchecks(Optional skipHealthchecks) { + this.skipHealthchecks = skipHealthchecks; + return this; + } + + @Override + public String toString() { + return "SingularityBounceRequestBuilder{" + + "durationMillis='" + durationMillis + '\'' + + ", actionId='" + actionId + '\'' + + ", message=" + message + '\'' + + ", incremental=" + incremental + '\'' + + ", skipHealthchecks=" + skipHealthchecks + '\'' + + "}"; + } +} diff --git a/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java b/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java index 11b9b7bacd..815b329fd7 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java @@ -8,6 +8,7 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.TimeUnit; import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; @@ -201,8 +202,17 @@ public SingularityRequestParent bounce(@ApiParam("The request ID to bounce") @Pa requestManager.bounce(requestWithState.getRequest(), System.currentTimeMillis(), JavaUtils.getUserEmail(user), message); + SingularityBounceRequest defaultBounceRequest = bounceRequest.or(SingularityBounceRequest.defaultRequest()); + if (!defaultBounceRequest.getDurationMillis().isPresent()) { + final Long durationMillis = TimeUnit.MINUTES.toMillis(configuration.getDefaultBounceExpirationMinutes()); + defaultBounceRequest = defaultBounceRequest + .toBuilder() + .setDurationMillis(Optional.of(durationMillis)) + .build(); + } + requestManager.saveExpiringObject(new SingularityExpiringBounce(requestId, deployId, JavaUtils.getUserEmail(user), - System.currentTimeMillis(), bounceRequest.or(SingularityBounceRequest.defaultRequest()), actionId.get())); + System.currentTimeMillis(), defaultBounceRequest, actionId.get())); return fillEntireRequest(requestWithState); } From a469d15bb07838f8852698633c2ba5dedf993cee Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Tue, 9 Aug 2016 16:13:29 -0400 Subject: [PATCH 05/13] Remove old ExpiringBounce durationMillis fix --- .../expiring/SingularityExpiringBounce.java | 28 ------------------- .../singularity/data/RequestManager.java | 5 +--- 2 files changed, 1 insertion(+), 32 deletions(-) diff --git a/SingularityBase/src/main/java/com/hubspot/singularity/expiring/SingularityExpiringBounce.java b/SingularityBase/src/main/java/com/hubspot/singularity/expiring/SingularityExpiringBounce.java index d188e2d057..2b708d0c98 100644 --- a/SingularityBase/src/main/java/com/hubspot/singularity/expiring/SingularityExpiringBounce.java +++ b/SingularityBase/src/main/java/com/hubspot/singularity/expiring/SingularityExpiringBounce.java @@ -20,34 +20,6 @@ public String getDeployId() { return deployId; } - public static Optional withDefaultExpiringMillis( - Optional maybeExpiringBounce, - Long durationMillis) { - if (!maybeExpiringBounce.isPresent()) { - return maybeExpiringBounce; - } - final SingularityExpiringBounce bounce = maybeExpiringBounce.get(); - if (bounce.getExpiringAPIRequestObject().getDurationMillis().isPresent()) { - return maybeExpiringBounce; - } - return Optional.of( - new SingularityExpiringBounce( - bounce.getRequestId(), - bounce.getDeployId(), - bounce.getUser(), - bounce.getStartMillis(), - new SingularityBounceRequest( - bounce.getExpiringAPIRequestObject().getIncremental(), - bounce.getExpiringAPIRequestObject().getSkipHealthchecks(), - Optional.of(durationMillis), - bounce.getExpiringAPIRequestObject().getActionId(), - bounce.getExpiringAPIRequestObject().getMessage() - ), - bounce.getActionId() - ) - ); - } - @Override public String toString() { return "SingularityExpiringBounce [toString()=" + super.toString() + "]"; diff --git a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java index f9bc78d325..a5433abcc6 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java @@ -3,7 +3,6 @@ import java.util.Collection; import java.util.List; import java.util.Map; -import java.util.concurrent.TimeUnit; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.utils.ZKPaths; @@ -389,9 +388,7 @@ public getExpiringBounce(String requestId) { - return SingularityExpiringBounce.withDefaultExpiringMillis( - getExpiringObject(SingularityExpiringBounce.class, requestId), - TimeUnit.MINUTES.toMillis(singularityConfiguration.getDefaultBounceExpirationMinutes())); + return getExpiringObject(SingularityExpiringBounce.class, requestId); } public Optional getExpiringPause(String requestId) { From 0845b8087533f40758b477ead34e0307ce34dff7 Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Wed, 10 Aug 2016 14:00:30 -0400 Subject: [PATCH 06/13] Moved durationMillis fix logic into validator --- .../singularity/data/SingularityValidator.java | 17 ++++++++++++++++- .../singularity/resources/RequestResource.java | 12 ++---------- 2 files changed, 18 insertions(+), 11 deletions(-) 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 c63d03b5ce..fd796458cb 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java @@ -11,6 +11,7 @@ import java.util.List; import java.util.TimeZone; import java.util.UUID; +import java.util.concurrent.TimeUnit; import java.util.regex.Pattern; import javax.inject.Singleton; @@ -30,7 +31,6 @@ import com.hubspot.mesos.SingularityContainerInfo; import com.hubspot.mesos.SingularityContainerType; import com.hubspot.mesos.SingularityDockerInfo; -import com.hubspot.mesos.SingularityDockerNetworkType; import com.hubspot.mesos.SingularityDockerPortMapping; import com.hubspot.mesos.SingularityPortMappingType; import com.hubspot.mesos.SingularityVolume; @@ -39,6 +39,7 @@ import com.hubspot.singularity.SingularityDeployBuilder; import com.hubspot.singularity.SingularityRequest; import com.hubspot.singularity.SingularityWebhook; +import com.hubspot.singularity.api.SingularityBounceRequest; import com.hubspot.singularity.config.SingularityConfiguration; import com.hubspot.singularity.data.history.DeployHistoryHelper; @@ -48,6 +49,7 @@ public class SingularityValidator { private static final List DEPLOY_ID_ILLEGAL_CHARACTERS = Arrays.asList('@', '-', '\\', '/', '*', '?', '%', ' ', '[', ']', '#', '$'); // Characters that make Mesos or URL bars sad private static final List REQUEST_ID_ILLEGAL_CHARACTERS = Arrays.asList('@', '\\', '/', '*', '?', '%', ' ', '[', ']', '#', '$'); // Characters that make Mesos or URL bars sad + private final SingularityConfiguration configuration; private final int maxDeployIdSize; private final int maxRequestIdSize; private final int maxCpusPerRequest; @@ -66,6 +68,8 @@ public class SingularityValidator { @Inject public SingularityValidator(SingularityConfiguration configuration, DeployHistoryHelper deployHistoryHelper, RequestManager requestManager) { + this.configuration = configuration; + this.maxDeployIdSize = configuration.getMaxDeployIdSize(); this.maxRequestIdSize = configuration.getMaxRequestIdSize(); this.allowRequestsWithoutOwners = configuration.isAllowRequestsWithoutOwners(); @@ -436,4 +440,15 @@ private boolean isValidInteger(String strValue) { return false; } } + + public SingularityBounceRequest withDurationMillis(SingularityBounceRequest defaultBounceRequest) { + if (defaultBounceRequest.getDurationMillis().isPresent()) { + return defaultBounceRequest; + } + final Long durationMillis = TimeUnit.MINUTES.toMillis(configuration.getDefaultBounceExpirationMinutes()); + return defaultBounceRequest + .toBuilder() + .setDurationMillis(Optional.of(durationMillis)) + .build(); + } } diff --git a/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java b/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java index 815b329fd7..2d35eefe4b 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java @@ -8,7 +8,6 @@ import java.util.List; import java.util.Map; import java.util.UUID; -import java.util.concurrent.TimeUnit; import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; @@ -202,17 +201,10 @@ public SingularityRequestParent bounce(@ApiParam("The request ID to bounce") @Pa requestManager.bounce(requestWithState.getRequest(), System.currentTimeMillis(), JavaUtils.getUserEmail(user), message); - SingularityBounceRequest defaultBounceRequest = bounceRequest.or(SingularityBounceRequest.defaultRequest()); - if (!defaultBounceRequest.getDurationMillis().isPresent()) { - final Long durationMillis = TimeUnit.MINUTES.toMillis(configuration.getDefaultBounceExpirationMinutes()); - defaultBounceRequest = defaultBounceRequest - .toBuilder() - .setDurationMillis(Optional.of(durationMillis)) - .build(); - } + final SingularityBounceRequest bounceRequestWithDurationMillis = validator.withDurationMillis(bounceRequest.or(SingularityBounceRequest.defaultRequest())); requestManager.saveExpiringObject(new SingularityExpiringBounce(requestId, deployId, JavaUtils.getUserEmail(user), - System.currentTimeMillis(), defaultBounceRequest, actionId.get())); + System.currentTimeMillis(), bounceRequestWithDurationMillis, actionId.get())); return fillEntireRequest(requestWithState); } From e1ee388b942c5dad5d841ea64e2bb507bcb671b4 Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Wed, 10 Aug 2016 14:01:00 -0400 Subject: [PATCH 07/13] Change Long to long in durationMillis fix --- .../java/com/hubspot/singularity/data/SingularityValidator.java | 2 +- 1 file changed, 1 insertion(+), 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 index fd796458cb..f5969afc94 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java @@ -445,7 +445,7 @@ public SingularityBounceRequest withDurationMillis(SingularityBounceRequest defa if (defaultBounceRequest.getDurationMillis().isPresent()) { return defaultBounceRequest; } - final Long durationMillis = TimeUnit.MINUTES.toMillis(configuration.getDefaultBounceExpirationMinutes()); + final long durationMillis = TimeUnit.MINUTES.toMillis(configuration.getDefaultBounceExpirationMinutes()); return defaultBounceRequest .toBuilder() .setDurationMillis(Optional.of(durationMillis)) From cb5cd7f3bbffdb69750bb8e16d196ce3d22eeec0 Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Wed, 10 Aug 2016 14:04:27 -0400 Subject: [PATCH 08/13] Make durationMillis validator method name more appropriate --- .../com/hubspot/singularity/data/SingularityValidator.java | 2 +- .../com/hubspot/singularity/resources/RequestResource.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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 f5969afc94..12d874266b 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java @@ -441,7 +441,7 @@ private boolean isValidInteger(String strValue) { } } - public SingularityBounceRequest withDurationMillis(SingularityBounceRequest defaultBounceRequest) { + public SingularityBounceRequest validateBounceRequest(SingularityBounceRequest defaultBounceRequest) { if (defaultBounceRequest.getDurationMillis().isPresent()) { return defaultBounceRequest; } diff --git a/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java b/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java index 2d35eefe4b..72f5a2e2fc 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java @@ -201,10 +201,10 @@ public SingularityRequestParent bounce(@ApiParam("The request ID to bounce") @Pa requestManager.bounce(requestWithState.getRequest(), System.currentTimeMillis(), JavaUtils.getUserEmail(user), message); - final SingularityBounceRequest bounceRequestWithDurationMillis = validator.withDurationMillis(bounceRequest.or(SingularityBounceRequest.defaultRequest())); + final SingularityBounceRequest validatedBounceRequest = validator.validateBounceRequest(bounceRequest.or(SingularityBounceRequest.defaultRequest())); requestManager.saveExpiringObject(new SingularityExpiringBounce(requestId, deployId, JavaUtils.getUserEmail(user), - System.currentTimeMillis(), bounceRequestWithDurationMillis, actionId.get())); + System.currentTimeMillis(), validatedBounceRequest, actionId.get())); return fillEntireRequest(requestWithState); } From f7d52110042ef93dc88ef0e4644986785568387f Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Wed, 10 Aug 2016 14:11:06 -0400 Subject: [PATCH 09/13] Remove now-unnecessary SingularityConfiguration attribute --- .../main/java/com/hubspot/singularity/data/RequestManager.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java index a5433abcc6..5e9a4bc21e 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java @@ -53,7 +53,6 @@ public class RequestManager extends CuratorAsyncManager { private final Transcoder requestLbCleanupTranscoder; private final SingularityEventListener singularityEventListener; - private final SingularityConfiguration singularityConfiguration; private static final String REQUEST_ROOT = "/requests"; @@ -90,7 +89,6 @@ public RequestManager(CuratorFramework curator, SingularityConfiguration configu this.requestHistoryTranscoder = requestHistoryTranscoder; this.singularityEventListener = singularityEventListener; this.requestLbCleanupTranscoder = requestLbCleanupTranscoder; - this.singularityConfiguration = singularityConfiguration; this.expiringTranscoderMap = ImmutableMap.of( SingularityExpiringBounce.class, expiringBounceTranscoder, From 0d767d268be23e2e96cd48a6d6dc1fbe790ac4f5 Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Wed, 10 Aug 2016 14:15:30 -0400 Subject: [PATCH 10/13] Rename validateBounceRequest -> checkBounceRequest --- .../java/com/hubspot/singularity/data/SingularityValidator.java | 2 +- .../java/com/hubspot/singularity/resources/RequestResource.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 12d874266b..05c3b7eec1 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java @@ -441,7 +441,7 @@ private boolean isValidInteger(String strValue) { } } - public SingularityBounceRequest validateBounceRequest(SingularityBounceRequest defaultBounceRequest) { + public SingularityBounceRequest checkBounceRequest(SingularityBounceRequest defaultBounceRequest) { if (defaultBounceRequest.getDurationMillis().isPresent()) { return defaultBounceRequest; } diff --git a/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java b/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java index 72f5a2e2fc..1c3fe18178 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/resources/RequestResource.java @@ -201,7 +201,7 @@ public SingularityRequestParent bounce(@ApiParam("The request ID to bounce") @Pa requestManager.bounce(requestWithState.getRequest(), System.currentTimeMillis(), JavaUtils.getUserEmail(user), message); - final SingularityBounceRequest validatedBounceRequest = validator.validateBounceRequest(bounceRequest.or(SingularityBounceRequest.defaultRequest())); + final SingularityBounceRequest validatedBounceRequest = validator.checkBounceRequest(bounceRequest.or(SingularityBounceRequest.defaultRequest())); requestManager.saveExpiringObject(new SingularityExpiringBounce(requestId, deployId, JavaUtils.getUserEmail(user), System.currentTimeMillis(), validatedBounceRequest, actionId.get())); From df9182f11f3bb205d59f50c250f23b91635253e0 Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Wed, 24 Aug 2016 18:01:45 -0400 Subject: [PATCH 11/13] Remove redundant SingularityConfiguration param --- .../main/java/com/hubspot/singularity/data/RequestManager.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java index 5e9a4bc21e..c54528bc25 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/RequestManager.java @@ -80,8 +80,7 @@ public class RequestManager extends CuratorAsyncManager { public RequestManager(CuratorFramework curator, SingularityConfiguration configuration, MetricRegistry metricRegistry, SingularityEventListener singularityEventListener, Transcoder requestCleanupTranscoder, Transcoder requestTranscoder, Transcoder requestLbCleanupTranscoder, Transcoder pendingRequestTranscoder, Transcoder requestHistoryTranscoder, Transcoder expiringBounceTranscoder, - Transcoder expiringScaleTranscoder, Transcoder expiringPauseTranscoder, Transcoder expiringSkipHealthchecksTranscoder, - SingularityConfiguration singularityConfiguration) { + Transcoder expiringScaleTranscoder, Transcoder expiringPauseTranscoder, Transcoder expiringSkipHealthchecksTranscoder) { super(curator, configuration, metricRegistry); this.requestTranscoder = requestTranscoder; this.requestCleanupTranscoder = requestCleanupTranscoder; From 2036fb6822ce309fe33a1b8e4b644d23b009c39c Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Wed, 24 Aug 2016 18:05:36 -0400 Subject: [PATCH 12/13] Remove SConfiguration property, replace with defaultBounceExpirationMinutes --- .../com/hubspot/singularity/data/SingularityValidator.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 05c3b7eec1..08909bcf3d 100644 --- a/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java +++ b/SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java @@ -49,7 +49,6 @@ public class SingularityValidator { private static final List DEPLOY_ID_ILLEGAL_CHARACTERS = Arrays.asList('@', '-', '\\', '/', '*', '?', '%', ' ', '[', ']', '#', '$'); // Characters that make Mesos or URL bars sad private static final List REQUEST_ID_ILLEGAL_CHARACTERS = Arrays.asList('@', '\\', '/', '*', '?', '%', ' ', '[', ']', '#', '$'); // Characters that make Mesos or URL bars sad - private final SingularityConfiguration configuration; private final int maxDeployIdSize; private final int maxRequestIdSize; private final int maxCpusPerRequest; @@ -59,6 +58,7 @@ public class SingularityValidator { private final int defaultCpus; private final int defaultMemoryMb; private final int defaultDiskMb; + private final int defaultBounceExpirationMinutes; private final int maxMemoryMbPerInstance; private final boolean allowRequestsWithoutOwners; private final boolean createDeployIds; @@ -68,8 +68,6 @@ public class SingularityValidator { @Inject public SingularityValidator(SingularityConfiguration configuration, DeployHistoryHelper deployHistoryHelper, RequestManager requestManager) { - this.configuration = configuration; - this.maxDeployIdSize = configuration.getMaxDeployIdSize(); this.maxRequestIdSize = configuration.getMaxRequestIdSize(); this.allowRequestsWithoutOwners = configuration.isAllowRequestsWithoutOwners(); @@ -80,6 +78,7 @@ public SingularityValidator(SingularityConfiguration configuration, DeployHistor this.defaultCpus = configuration.getMesosConfiguration().getDefaultCpus(); this.defaultMemoryMb = configuration.getMesosConfiguration().getDefaultMemory(); this.defaultDiskMb = configuration.getMesosConfiguration().getDefaultDisk(); + this.defaultBounceExpirationMinutes = configuration.getDefaultBounceExpirationMinutes(); defaultResources = new Resources(defaultCpus, defaultMemoryMb, 0, defaultDiskMb); @@ -445,7 +444,7 @@ public SingularityBounceRequest checkBounceRequest(SingularityBounceRequest defa if (defaultBounceRequest.getDurationMillis().isPresent()) { return defaultBounceRequest; } - final long durationMillis = TimeUnit.MINUTES.toMillis(configuration.getDefaultBounceExpirationMinutes()); + final long durationMillis = TimeUnit.MINUTES.toMillis(defaultBounceExpirationMinutes); return defaultBounceRequest .toBuilder() .setDurationMillis(Optional.of(durationMillis)) From 591e238ebf777fd71945c17b6e9f26e2762a055f Mon Sep 17 00:00:00 2001 From: Matthew Cotton Date: Thu, 25 Aug 2016 11:34:24 -0400 Subject: [PATCH 13/13] Fix missing apostrophes --- .../singularity/api/SingularityBounceRequestBuilder.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequestBuilder.java b/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequestBuilder.java index b7e9f92ee4..19a111e9ba 100644 --- a/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequestBuilder.java +++ b/SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityBounceRequestBuilder.java @@ -74,9 +74,9 @@ public String toString() { return "SingularityBounceRequestBuilder{" + "durationMillis='" + durationMillis + '\'' + ", actionId='" + actionId + '\'' + - ", message=" + message + '\'' + - ", incremental=" + incremental + '\'' + - ", skipHealthchecks=" + skipHealthchecks + '\'' + + ", message='" + message + '\'' + + ", incremental='" + incremental + '\'' + + ", skipHealthchecks='" + skipHealthchecks + '\'' + "}"; } }