Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.hubspot.singularity.api;

import java.util.UUID;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.Optional;
Expand All @@ -18,6 +20,10 @@ public SingularityBounceRequest(@JsonProperty("incremental") Optional<Boolean> i
this.skipHealthchecks = skipHealthchecks;
}

public static SingularityBounceRequest defaultRequest() {
return new SingularityBounceRequest(Optional.<Boolean>absent(), Optional.<Boolean>absent(), Optional.<Long>absent(), Optional.of(UUID.randomUUID().toString()), Optional.<String>absent());
}

@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<Boolean> getIncremental() {
return incremental;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ public class SingularityConfiguration extends Configuration {
@JsonProperty("database")
private DataSourceFactory databaseConfiguration;

@Min(value = 1, message = "Must be positive and non-zero")
private int defaultBounceExpirationMinutes = 60;

@NotNull
private SlavePlacement defaultSlavePlacement = SlavePlacement.GREEDY;

Expand Down Expand Up @@ -339,6 +342,14 @@ public Optional<DataSourceFactory> getDatabaseConfiguration() {
return Optional.fromNullable(databaseConfiguration);
}

public int getDefaultBounceExpirationMinutes() {
return defaultBounceExpirationMinutes;
}

public void setDefaultBounceExpirationMinutes(int defaultBounceExpirationMinutes) {
this.defaultBounceExpirationMinutes = defaultBounceExpirationMinutes;
}

public SlavePlacement getDefaultSlavePlacement() {
return defaultSlavePlacement;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,10 @@ public SingularityRequestParent bounce(@ApiParam("The request ID to bounce") @Pa
if (bounceRequest.isPresent()) {
actionId = bounceRequest.get().getActionId();
message = bounceRequest.get().getMessage();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic was changed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll check against staging to see what changed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, now that I look again - are we now saying it's impossible to have unlimited bounce?

If so, what will you do if someone enters a negative value for this configuration? Need to have validation on it or allow people to set it to 0/negative to indicate that they don't want to use this feature.

In which case, we'd only set actionId if there was a duration set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should still create an expiring object either way? One thing I realized in writing this is that if we don't create the expiring object, there isn't actually a way to cancel the bounce (without doing a janky pause/unpause)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, maybe just setup the config to validate that this must be non-zero, positive, then it's not an issue. I think you can do that with annotations. Someone could set it really high.

if (bounceRequest.get().getDurationMillis().isPresent() && !actionId.isPresent()) {
actionId = Optional.of(UUID.randomUUID().toString());
}
if (!actionId.isPresent()) {
actionId = Optional.of(UUID.randomUUID().toString());
}

final String deployId = getAndCheckDeployId(requestId);
Expand All @@ -195,10 +195,8 @@ public SingularityRequestParent bounce(@ApiParam("The request ID to bounce") @Pa

requestManager.bounce(requestWithState.getRequest(), System.currentTimeMillis(), JavaUtils.getUserEmail(user), message);

if (bounceRequest.isPresent() && bounceRequest.get().getDurationMillis().isPresent()) {
requestManager.saveExpiringObject(new SingularityExpiringBounce(requestId, deployId, JavaUtils.getUserEmail(user),
System.currentTimeMillis(), bounceRequest.get(), actionId.get()));
}
requestManager.saveExpiringObject(new SingularityExpiringBounce(requestId, deployId, JavaUtils.getUserEmail(user),
System.currentTimeMillis(), bounceRequest.or(SingularityBounceRequest.defaultRequest()), actionId.get()));

return fillEntireRequest(requestWithState);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public class SingularityExpiringUserActionPoller extends SingularityLeaderOnlyPo
private final SingularityMailer mailer;
private final RequestHelper requestHelper;
private final List<SingularityExpiringUserActionHandler<?>> handlers;
private final SingularityConfiguration configuration;

@Inject
SingularityExpiringUserActionPoller(SingularityConfiguration configuration, RequestManager requestManager, TaskManager taskManager,
Expand All @@ -56,6 +57,7 @@ public class SingularityExpiringUserActionPoller extends SingularityLeaderOnlyPo
this.requestHelper = requestHelper;
this.mailer = mailer;
this.taskManager = taskManager;
this.configuration = configuration;

List<SingularityExpiringUserActionHandler<?>> tempHandlers = Lists.newArrayList();
tempHandlers.add(new SingularityExpiringBounceHandler());
Expand Down Expand Up @@ -85,18 +87,22 @@ private boolean isExpiringDue(T expiringObject) {
final long now = System.currentTimeMillis();
final long duration = now - expiringObject.getStartMillis();

return duration > expiringObject.getExpiringAPIRequestObject().getDurationMillis().get();
return duration > getDurationMillis(expiringObject);
}

protected String getMessage(T expiringObject) {
String msg = String.format("%s expired after %s", getActionName(),
JavaUtils.durationFromMillis(expiringObject.getExpiringAPIRequestObject().getDurationMillis().get()));
JavaUtils.durationFromMillis(getDurationMillis(expiringObject)));
if (expiringObject.getExpiringAPIRequestObject().getMessage().isPresent() && expiringObject.getExpiringAPIRequestObject().getMessage().get().length() > 0) {
msg = String.format("%s (%s)", msg, expiringObject.getExpiringAPIRequestObject().getMessage().get());
}
return msg;
}

protected long getDurationMillis(T expiringObject) {
return expiringObject.getExpiringAPIRequestObject().getDurationMillis().get();
}

protected void checkExpiringObjects() {
for (T expiringObject : requestManager.getExpiringObjects(clazz)) {
if (isExpiringDue(expiringObject)) {
Expand Down Expand Up @@ -130,6 +136,11 @@ protected String getActionName() {
return "Bounce";
}

@Override
protected long getDurationMillis(SingularityExpiringBounce expiringBounce) {
return expiringBounce.getExpiringAPIRequestObject().getDurationMillis().or(TimeUnit.MINUTES.toMillis(configuration.getDefaultBounceExpirationMinutes()));
}

@Override
protected void handleExpiringObject(SingularityExpiringBounce expiringObject, SingularityRequestWithState requestWithState, String message) {
for (SingularityTaskCleanup taskCleanup : taskManager.getCleanupTasks()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class IndexView extends View {
private final Integer slaveHttpPort;
private final Integer slaveHttpsPort;

private final int defaultBounceExpirationMinutes;
private final long defaultHealthcheckIntervalSeconds;
private final long defaultHealthcheckTimeoutSeconds;
private final long defaultDeployHealthTimeoutSeconds;
Expand Down Expand Up @@ -77,6 +78,7 @@ public IndexView(String singularityUriBase, String appRoot, SingularityConfigura

this.navColor = configuration.getUiConfiguration().getNavColor();

this.defaultBounceExpirationMinutes = configuration.getDefaultBounceExpirationMinutes();
this.defaultHealthcheckIntervalSeconds = configuration.getHealthcheckIntervalSeconds();
this.defaultHealthcheckTimeoutSeconds = configuration.getHealthcheckTimeoutSeconds();
this.defaultDeployHealthTimeoutSeconds = configuration.getDeployHealthyBySeconds();
Expand Down Expand Up @@ -154,6 +156,10 @@ public Boolean getLoadBalancingEnabled() {
return loadBalancingEnabled;
}

public int getDefaultBounceExpirationMinutes() {
return defaultBounceExpirationMinutes;
}

public long getDefaultHealthcheckIntervalSeconds() {
return defaultHealthcheckIntervalSeconds;
}
Expand Down Expand Up @@ -213,6 +219,7 @@ public String toString() {
", title='" + title + '\'' +
", slaveHttpPort=" + slaveHttpPort +
", slaveHttpsPort=" + slaveHttpsPort +
", defaultBounceExpirationMinutes=" + defaultBounceExpirationMinutes +
", defaultHealthcheckIntervalSeconds=" + defaultHealthcheckIntervalSeconds +
", defaultHealthcheckTimeoutSeconds=" + defaultHealthcheckTimeoutSeconds +
", defaultDeployHealthTimeoutSeconds=" + defaultDeployHealthTimeoutSeconds +
Expand Down
1 change: 1 addition & 0 deletions SingularityUI/app/assets/_index.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
loadBalancingEnabled: {{{loadBalancingEnabled}}},
defaultCpus: {{{defaultCpus}}},
defaultMemory: {{{defaultMemory}}},
defaultBounceExpirationMinutes: {{{defaultBounceExpirationMinutes}}},
defaultHealthcheckIntervalSeconds: {{{defaultHealthcheckIntervalSeconds}}},
defaultHealthcheckTimeoutSeconds: {{{defaultHealthcheckTimeoutSeconds}}},
defaultDeployHealthTimeoutSeconds: {{{defaultDeployHealthTimeoutSeconds}}},
Expand Down
4 changes: 3 additions & 1 deletion SingularityUI/app/models/Request.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,9 @@ class Request extends Model

promptBounce: (callback) =>
vex.dialog.confirm
message: bounceTemplate id: @get "id"
message: bounceTemplate
id: @get "id"
config: config
input: """
<input name="message" id="bounce-message" type="text" placeholder="Message (optional)" />
"""
Expand Down
2 changes: 1 addition & 1 deletion SingularityUI/app/templates/vex/requestBounce.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
<input type="checkbox" name="skip-healthchecks" id="skip-healthchecks"> Skip healthchecks during bounce
</label>
<div class="vex-dialog-input expiration-input">
<input name="duration" id="bounce-expiration" type="text" placeholder="Expiration (optional)" />
<input name="duration" id="bounce-expiration" type="text" placeholder="Expiration (optional - default {{ config.defaultBounceExpirationMinutes }}m)" />
</div>
<span class="help">If an expiration duration is specified, this bounce will be aborted if not finished. Accepts any english time duration. (Days, Hr, Min...)</span>
23 changes: 14 additions & 9 deletions SingularityUI/app/views/requestActionExpirations.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,20 @@ class requestActionExpirations extends View
revertParam: request.expiringScale.revertToInstances
message: request.expiringScale.expiringAPIRequestObject.message

if request.expiringBounce and (request.expiringBounce.startMillis + request.expiringBounce.expiringAPIRequestObject.durationMillis) > new Date().getTime()
expirations.push
action: 'Bounce'
user: if request.expiringBounce.user then request.expiringBounce.user.split('@')[0] else ""
endMillis: request.expiringBounce.startMillis + request.expiringBounce.expiringAPIRequestObject.durationMillis
canRevert: false
cancelText: 'Cancel'
cancelAction: 'cancelBounce'
message: request.expiringBounce.expiringAPIRequestObject.message
if request.expiringBounce
if request.expiringBounce.expiringAPIRequestObject.durationMillis
endMillis = request.expiringBounce.startMillis + request.expiringBounce.expiringAPIRequestObject.durationMillis
else
endMillis = request.expiringBounce.startMillis + (config.defaultBounceExpirationMinutes * 60 * 1000)
if endMillis > new Date().getTime()
expirations.push
action: 'Bounce'
user: if request.expiringBounce.user then request.expiringBounce.user.split('@')[0] else ""
endMillis: endMillis
canRevert: false
cancelText: 'Cancel'
cancelAction: 'cancelBounce'
message: request.expiringBounce.expiringAPIRequestObject.message

if request.expiringPause and (request.expiringPause.startMillis + request.expiringPause.expiringAPIRequestObject.durationMillis) > new Date().getTime()
expirations.push
Expand Down
1 change: 1 addition & 0 deletions SingularityUI/config.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ exports.config =
navColor: process.env.SINGULARITY_NAV_COLOR
defaultCpus: process.env.SINGUALRITY_DEFAULT_CPUS ? 1
defaultMemory: process.env.SINGULARITY_DEFAULT_MEMORY ? 128
defaultBounceExpirationMinutes: process.env.SINGULARITY_DEFAULT_BOUNCE_EXPIRATION_MINUTES ? 60
defaultHealthcheckIntervalSeconds: process.env.SINGULARITY_DEFAULT_HEALTHCHECK_INTERVAL_SECONDS ? 5
defaultHealthcheckTimeoutSeconds: process.env.SINGULARITY_HEALTHCHECK_TIMEOUT_SECONDS ? 5
defaultDeployHealthTimeoutSeconds: process.env.SINGULARITY_DEPLOY_HEALTH_TIMEOUT_SECONDS ? 120
Expand Down