Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overrides for run now requests #1650

Merged
merged 27 commits into from
Dec 7, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
4d1815c
Support extra artifacts in models.
baconmania Nov 3, 2017
edab7aa
Download additional artifacts as well.
baconmania Nov 3, 2017
27ddc68
Update tests.
baconmania Nov 3, 2017
3634f26
Default extraArtifacts to an empty list instead of using Optional<>.
baconmania Nov 7, 2017
4bf55d2
Explicit type not required here.
baconmania Nov 7, 2017
cf38987
Don't use an immutable list here.
baconmania Nov 7, 2017
7db038c
Safest way of doing this.
baconmania Nov 7, 2017
f9d59e5
Download extra artifacts regardless of whether we're using the defaul…
baconmania Nov 7, 2017
42fe3b6
Allow clients to set an output file path.
baconmania Nov 7, 2017
6c0c836
Failing test case for environment variable overrides in run now requests
Nov 14, 2017
4b1c9fa
Passing of environment variable overrides through mesos task builder
Nov 14, 2017
d81a6a9
CR changes
Nov 14, 2017
f3012b7
Added environment variable overrides into pending tasks
Nov 15, 2017
a049250
Added environment variable overrides into pending tasks
Nov 15, 2017
5a2e951
CR Changes
Nov 15, 2017
19267fb
Added environment variable overrides to pending requests
Nov 15, 2017
03b9972
Indentation fix
Nov 21, 2017
3110897
Merge branch 'run-now-with-additional-artifacts' into run_now_overrides
baconmania Nov 21, 2017
38554c0
Don't break compatibility.
baconmania Nov 21, 2017
547b834
Log warning when pending type cannot be determine
Nov 21, 2017
19b385e
Support user overrides in Run-Now requests.
baconmania Dec 1, 2017
30a441e
Unnecessary method.
baconmania Dec 1, 2017
28e9b53
Fix isPresent() check.
baconmania Dec 4, 2017
097469d
Merge pull request #1668 from HubSpot/run-now-user-override
baconmania Dec 4, 2017
bc231b2
Merge branch 'master' into run_now_overrides
baconmania Dec 6, 2017
15c44da
Fix test.
baconmania Dec 6, 2017
265f042
fix merge conflicts in task builder
ssalinas Dec 7, 2017
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,6 +1,8 @@
package com.hubspot.singularity;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import javax.annotation.Nonnull;
Expand All @@ -20,6 +22,7 @@ public class SingularityPendingTask {
private final Optional<Boolean> skipHealthchecks;
private final Optional<String> message;
private final Optional<Resources> resources;
private final Map<String, String> envOverrides;
private final Optional<String> actionId;

public static Predicate<SingularityPendingTask> matchingRequest(final String requestId) {
Expand All @@ -45,16 +48,24 @@ public boolean apply(@Nonnull SingularityPendingTask input) {
}

@JsonCreator
public SingularityPendingTask(@JsonProperty("pendingTaskId") SingularityPendingTaskId pendingTaskId, @JsonProperty("cmdLineArgsList") Optional<List<String>> cmdLineArgsList,
@JsonProperty("user") Optional<String> user, @JsonProperty("runId") Optional<String> runId, @JsonProperty("skipHealthchecks") Optional<Boolean> skipHealthchecks,
@JsonProperty("message") Optional<String> message, @JsonProperty("resources") Optional<Resources> resources, @JsonProperty("actionId") Optional<String> actionId) {
public SingularityPendingTask(
@JsonProperty("pendingTaskId") SingularityPendingTaskId pendingTaskId,
@JsonProperty("cmdLineArgsList") Optional<List<String>> cmdLineArgsList,
@JsonProperty("user") Optional<String> user,
@JsonProperty("runId") Optional<String> runId,
@JsonProperty("skipHealthchecks") Optional<Boolean> skipHealthchecks,
@JsonProperty("message") Optional<String> message,
@JsonProperty("resources") Optional<Resources> resources,
@JsonProperty("envOverrides") Map<String, String> envOverrides,
@JsonProperty("actionId") Optional<String> actionId) {
this.pendingTaskId = pendingTaskId;
this.user = user;
this.message = message;
this.cmdLineArgsList = cmdLineArgsList;
this.runId = runId;
this.skipHealthchecks = skipHealthchecks;
this.resources = resources;
this.envOverrides = envOverrides == null ? Collections.emptyMap() : envOverrides;
this.actionId = actionId;
}

Expand Down Expand Up @@ -103,6 +114,8 @@ public Optional<Resources> getResources() {
return resources;
}

public Map<String, String> getEnvOverrides() { return envOverrides; }

public Optional<String> getActionId() {
return actionId;
}
Expand All @@ -117,6 +130,7 @@ public String toString() {
", skipHealthchecks=" + skipHealthchecks +
", message=" + message +
", resources=" + resources +
", envOverrides=" + envOverrides +
", actionId=" + actionId +
'}';
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package com.hubspot.singularity;

import java.util.List;
import java.util.Map;

import com.google.common.base.Optional;
import com.hubspot.mesos.Resources;

public class SingularityPendingTaskBuilder {

private SingularityPendingTaskId pendingTaskId;
private Optional<List<String>> cmdLineArgsList;
private Optional<String> user;
private Optional<String> runId;
private Optional<Boolean> skipHealthchecks;
private Optional<String> message;
private Optional<Resources> resources;
private Map<String, String> envOverrides;
private Optional<String> actionId;

public SingularityPendingTaskBuilder() {
this.pendingTaskId = null;
this.cmdLineArgsList = Optional.absent();
this.user = Optional.absent();
this.runId = Optional.absent();
this.skipHealthchecks = Optional.absent();
this.message = Optional.absent();
this.resources = Optional.absent();
this.envOverrides = null;
this.actionId = Optional.absent();
}

public SingularityPendingTaskBuilder setPendingTaskId(SingularityPendingTaskId pendingTaskId) {
this.pendingTaskId = pendingTaskId;
return this;
}


public SingularityPendingTaskBuilder setCmdLineArgsList(List<String> cmdLineArgsList) {
this.cmdLineArgsList = Optional.of(cmdLineArgsList);
return this;
}

public SingularityPendingTaskBuilder setCmdLineArgsList(Optional<List<String>> cmdLineArgsList) {
this.cmdLineArgsList = cmdLineArgsList;
return this;
}

public SingularityPendingTaskBuilder setUser(String user) {
this.user = Optional.of(user);
return this;
}


public SingularityPendingTaskBuilder setRunId(String runId) {
this.runId = Optional.of(runId);
return this;
}

public SingularityPendingTaskBuilder setSkipHealthchecks(Boolean skipHealthchecks) {
this.skipHealthchecks = Optional.of(skipHealthchecks);
return this;
}

public SingularityPendingTaskBuilder setMessage(String message) {
this.message = Optional.of(message);
return this;
}

public SingularityPendingTaskBuilder setResources(Resources resources) {
this.resources = Optional.of(resources);
return this;
}

public SingularityPendingTaskBuilder setEnvOverrides(Map<String, String> envOverrides) {
this.envOverrides = envOverrides;
return this;
}
public SingularityPendingTaskBuilder setActionId(String actionId) {
this.actionId = Optional.of(actionId);
return this;
}

public SingularityPendingTask build() {
return new SingularityPendingTask(
pendingTaskId, cmdLineArgsList, user, runId, skipHealthchecks, message, resources, envOverrides, actionId
);
}

Copy link
Member

Choose a reason for hiding this comment

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

Good to add a toString on these. Ends up being helpful for debugging later if we ever have to log it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to know :)

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package com.hubspot.singularity;

import java.util.List;
import java.util.Map;

import com.google.common.base.Optional;
import com.hubspot.mesos.Resources;
import com.hubspot.singularity.api.SingularityRunNowRequest;

public class SingularityRunNowRequestBuilder {
private Optional<String> message;
private Optional<String> runId;
private Optional<List<String>> commandLineArgs;
private Optional<Boolean> skipHealthchecks;
private Optional<Resources> resources;
private Map<String, String> envOverrides;
private Optional<Long> runAt;

public SingularityRunNowRequestBuilder()
{
this.message = Optional.absent();
this.runId = Optional.absent();
this.commandLineArgs = Optional.absent();
this.skipHealthchecks = Optional.absent();
this.resources = Optional.absent();
this.envOverrides = null;
Copy link
Member

Choose a reason for hiding this comment

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

empty map instead of null here?

this.runAt = Optional.absent();
}

public SingularityRunNowRequestBuilder setMessage(String message) {
this.message = Optional.of(message);
return this;
}

public SingularityRunNowRequestBuilder setRunId(String runId) {
this.runId = Optional.of(runId);
return this;
}

public SingularityRunNowRequestBuilder setCommandLineArgs(List<String> commandLineArgs) {
this.commandLineArgs = Optional.of(commandLineArgs);
return this;
}

public SingularityRunNowRequestBuilder setSkipHealthchecks(Boolean skipHealthchecks) {
this.skipHealthchecks = Optional.of(skipHealthchecks);
return this;
}

public SingularityRunNowRequestBuilder setResources(Resources resources) {
this.resources = Optional.of(resources);
return this;
}

public SingularityRunNowRequestBuilder setEnvOverrides(Map<String, String> environmentVariables) {
this.envOverrides = environmentVariables;
return this;
}

public SingularityRunNowRequestBuilder setRunAt(Long runAt) {
this.runAt = Optional.of(runAt);
return this;
}

public SingularityRunNowRequest build() {
return new SingularityRunNowRequest(
message, skipHealthchecks, runId, commandLineArgs, resources, envOverrides, runAt);
}

Copy link
Member

Choose a reason for hiding this comment

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

same toString comment for this one

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.hubspot.singularity.api;

import java.util.Collections;
import java.util.List;
import java.util.Map;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
Expand All @@ -15,34 +17,38 @@ public class SingularityRunNowRequest {
private final Optional<List<String>> commandLineArgs;
private final Optional<Boolean> skipHealthchecks;
private final Optional<Resources> resources;
private final Map<String, String> envOverrides;
private final Optional<Long> runAt;

public SingularityRunNowRequest(
Optional<String> message,
Optional<Boolean> skipHealthchecks,
Optional<String> runId,
Optional<List<String>> commandLineArgs,
Optional<Resources> resources
Optional<Resources> resources,
Map<String, String> envOverrides
Copy link
Contributor

Choose a reason for hiding this comment

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

Quick note here, we don't want to modify existing constructor signatures for classes in a Base module of a project (or more generally, for any classes that are intended for use outside the project itself), because it'd break other projects. Adding "proxy" constructors is one way around this.

) {
this(message, skipHealthchecks, runId, commandLineArgs, resources, Optional.<Long>absent());
this(message, skipHealthchecks, runId, commandLineArgs, resources, envOverrides, Optional.<Long>absent());
}

@JsonCreator
public SingularityRunNowRequest(@JsonProperty("message") Optional<String> message,
@JsonProperty("skipHealthchecks") Optional<Boolean> skipHealthchecks,
@JsonProperty("runId") Optional<String> runId,
@JsonProperty("commandLineArgs") Optional<List<String>> commandLineArgs,
public SingularityRunNowRequest(@JsonProperty("setMessage") Optional<String> message,
Copy link
Member

Choose a reason for hiding this comment

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

the @JsonProperty on these needs to stay as it was before. This is one of the pieces jackson uses to determine the field name in the json. e.g. @JsonProperty("message") becomes {"message": {}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm...think that was my improper use of Intellij's refactoring tools. Going to need to be more careful of that in the future.

@JsonProperty("setSkipHealthchecks") Optional<Boolean> skipHealthchecks,
@JsonProperty("setRunId") Optional<String> runId,
@JsonProperty("setCommandLineArgs") Optional<List<String>> commandLineArgs,
@JsonProperty("resources") Optional<Resources> resources,
@JsonProperty("runAt") Optional<Long> runAt) {
@JsonProperty("setEnvOverrides") Map<String, String> envOverrides,
@JsonProperty("setRunAt") Optional<Long> runAt) {
this.message = message;
this.commandLineArgs = commandLineArgs;
this.runId = runId;
this.skipHealthchecks = skipHealthchecks;
this.resources = resources;
this.envOverrides = envOverrides == null ? Collections.emptyMap() : envOverrides;
this.runAt = runAt;
}

@ApiModelProperty(required=false, value="A message to show to users about why this action was taken")
@ApiModelProperty(required=false, value="A setMessage to show to users about why this action was taken")
public Optional<String> getMessage() {
return message;
}
Expand All @@ -62,11 +68,16 @@ public Optional<Boolean> getSkipHealthchecks() {
return skipHealthchecks;
}

@ApiModelProperty(required=false, value="Override the resources from the active deploy for this run")
@ApiModelProperty(required=false, value="Override the setResources from the active deploy for this run")
Copy link
Member

Choose a reason for hiding this comment

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

similar comment here, don't want to rename these to set...

public Optional<Resources> getResources() {
return resources;
}

@ApiModelProperty(required=false, value="Override the environment variables for launched tasks")
public Map<String, String> getEnvOverrides() {
return envOverrides;
}

@ApiModelProperty(required=false, value="Schedule this task to run at a specified time")
public Optional<Long> getRunAt() {
return runAt;
Expand All @@ -80,6 +91,7 @@ public String toString() {
", commandLineArgs=" + commandLineArgs +
", skipHealthchecks=" + skipHealthchecks +
", resources=" + resources +
", envOverrides=" + envOverrides +
", runAt=" + runAt +
'}';
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ private SingularityRunNowRequest fillRunNowRequest(Optional<SingularityRunNowReq
Optional.of(getRunId(request.getRunId())),
request.getCommandLineArgs(),
request.getResources(),
request.getEnvOverrides(),
request.getRunAt());
} else {
return new SingularityRunNowRequest(
Expand All @@ -456,7 +457,7 @@ private SingularityRunNowRequest fillRunNowRequest(Optional<SingularityRunNowReq
Optional.of(getRunId(Optional.absent())),
Optional.absent(),
Optional.absent(),
Optional.absent());
null);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@
import com.google.common.base.Optional;
import com.google.common.base.Throwables;
import com.google.inject.Inject;
import com.hubspot.mesos.Resources;
import com.hubspot.singularity.SingularityCreateResult;
import com.hubspot.singularity.SingularityPendingRequest;
import com.hubspot.singularity.SingularityPendingRequest.PendingType;
import com.hubspot.singularity.SingularityPendingTask;
import com.hubspot.singularity.SingularityPendingTaskBuilder;
import com.hubspot.singularity.SingularityPendingTaskId;
import com.hubspot.singularity.data.TaskManager;
import com.hubspot.singularity.data.transcoders.StringTranscoder;
Expand Down Expand Up @@ -142,8 +141,12 @@ private void checkPendingTasks() {
for (SingularityPendingTaskId pendingTaskId : taskManager.getPendingTaskIds()) {
Optional<String> cmdLineArgs = getCmdLineArgs(pendingTaskId);

SingularityCreateResult result = taskManager.savePendingTask(new SingularityPendingTask(pendingTaskId, getCmdLineArgs(cmdLineArgs), Optional.<String> absent(),
Optional.<String> absent(), Optional.<Boolean> absent(), Optional.<String> absent(), Optional.<Resources>absent(), Optional.<String>absent()));
SingularityCreateResult result = taskManager.savePendingTask(
new SingularityPendingTaskBuilder()
.setPendingTaskId(pendingTaskId)
.setCmdLineArgsList(getCmdLineArgs(cmdLineArgs))
.build()
);

LOG.info("Saving {} ({}) {}", pendingTaskId, cmdLineArgs, result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@
import com.google.common.base.Throwables;
import com.google.inject.Inject;
import com.hubspot.mesos.JavaUtils;
import com.hubspot.mesos.Resources;
import com.hubspot.singularity.InvalidSingularityTaskIdException;
import com.hubspot.singularity.SingularityPendingRequest.PendingType;
import com.hubspot.singularity.SingularityPendingTask;
import com.hubspot.singularity.SingularityPendingTaskBuilder;
import com.hubspot.singularity.SingularityPendingTaskId;
import com.hubspot.singularity.data.TaskManager;
import com.hubspot.singularity.data.transcoders.StringTranscoder;
Expand Down Expand Up @@ -59,8 +58,11 @@ public void applyMigration() {

Optional<String> cmdLineArgs = getCmdLineArgs(pendingTaskId);

taskManager.savePendingTask(new SingularityPendingTask(newPendingTaskId, cmdLineArgs.isPresent() ? Optional.of(Collections.singletonList(cmdLineArgs.get())) :
Optional.<List<String>> absent(), Optional.<String> absent(), Optional.<String> absent(), Optional.<Boolean> absent(), Optional.<String> absent(), Optional.<Resources>absent(), Optional.<String>absent()));
taskManager.savePendingTask(
new SingularityPendingTaskBuilder()
.setPendingTaskId(newPendingTaskId)
.setCmdLineArgsList(cmdLineArgs.isPresent() ? Optional.of(Collections.singletonList(cmdLineArgs.get())) : Optional.<List<String>> absent())
.build());

curator.delete().forPath(ZKPaths.makePath(PENDING_TASKS_ROOT, pendingTaskId));
}
Expand Down
Loading