Skip to content
This repository has been archived by the owner on Sep 29, 2021. It is now read-only.

Commit

Permalink
Adding global flag --strict-start to wildcard-based commands.
Browse files Browse the repository at this point in the history
This forces job names to be matched from the string start. By default,
some commands will match on any jobs that contain the input name as a
substring, this option forces the match to only happen if the string
starts with the input name. Affects the subcommands: remove, inspect,
rolling-update, undeploy, deploy and stop.

The motivation behind this is production usages returning
"JOB_AMBIGUOUS_REFERENCE" in cases where a job's name is a substring of
another (eg: foo-bar and bar-foo-bar, as the first a substring of the
second).
  • Loading branch information
hstefan authored and Stefan Puhlmann committed Apr 9, 2019
1 parent c9000bc commit 48398bd
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ private static String formatRolloutOptions(final RolloutOptions options) {
}

public JobInspectCommand(final Subparser parser) {
super(parser);
super(parser, false);
parser.help("print the configuration of a job");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class JobRemoveCommand extends WildcardJobCommand {
private final Argument yesArg;

public JobRemoveCommand(Subparser parser) {
super(parser);
super(parser, false);

parser.help("remove a job");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class JobStartCommand extends WildcardJobCommand {
private final Argument tokenArg;

public JobStartCommand(Subparser parser) {
super(parser);
super(parser, false);

parser.help("start a stopped job");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class JobStopCommand extends WildcardJobCommand {
private final Argument tokenArg;

public JobStopCommand(Subparser parser) {
super(parser);
super(parser, false);

parser.help("stop a running job without undeploying it");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class JobUndeployCommand extends WildcardJobCommand {
private final Argument yesArg;

public JobUndeployCommand(final Subparser parser) {
super(parser);
super(parser, false);

parser.help("undeploy a job from hosts");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
abstract class WildcardJobCommand extends ControlCommand {

private final Argument jobArg;
private final Argument strictStartArg;

public WildcardJobCommand(final Subparser parser) {
this(parser, false);
Expand All @@ -51,16 +52,31 @@ public WildcardJobCommand(final Subparser parser, final boolean shortCircuit) {

jobArg = parser.addArgument("job")
.help("Job id.");
strictStartArg = parser.addArgument("--strict-start")
.type(Boolean.class)
.setDefault(false)
.help("Forces job names to be matched from the string start. "
+ "By default, some commands will match on any jobs that contain the input name as a"
+ " substring, this option forces the match to only happen if the string starts with"
+ " the input name. Affects the subcommands: remove, inspect, rolling-update,"
+ " undeploy, deploy and stop.");
}

@Override
int run(final Namespace options, final HeliosClient client, final PrintStream out,
final boolean json, final BufferedReader stdin)
throws ExecutionException, InterruptedException, IOException {

final String jobIdString = options.getString(jobArg.getDest());
final Map<JobId, Job> jobs = client.jobs(jobIdString).get();

if (options.getBoolean(strictStartArg.getDest())) {
for (final Map.Entry<JobId, Job> entry : jobs.entrySet()) {
if (!entry.getKey().toShortString().startsWith(jobIdString)) {
jobs.remove(entry.getKey());
}
}
}

if (jobs.size() == 0) {
if (!json) {
out.printf("Unknown job: %s%n", jobIdString);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static com.spotify.helios.common.descriptors.DeploymentGroup.RollingUpdateReason.MANUAL;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.mockito.Matchers.any;
Expand Down Expand Up @@ -648,6 +649,37 @@ public void testTimeoutDuringRolloutOutput() throws Exception {
assertThat(baos.toString(), containsString(expectedSubstring));
}

@Test
public void testStrictStartJobName() throws Exception {
final JobId jobId1 = JobId.parse("foo-bar:cafe");
Job job1 = Job.newBuilder()
.setName(jobId1.getName())
.setHash(jobId1.getHash())
.setRolloutOptions(jobOptions)
.build();
final JobId jobId2 = JobId.parse("bar:beef");
Job job2 = Job.newBuilder()
.setName(jobId2.getName())
.setHash(jobId2.getHash())
.setRolloutOptions(jobOptions)
.build();
final Map<JobId, Job> jobs = ImmutableMap.of(
jobId1, job1,
jobId2, job2
);
when(client.jobs(anyString())).thenReturn(immediateFuture(jobs));

when(options.getBoolean("strict-start")).thenReturn(true);

when(options.getString("job")).thenReturn("bar");

final int ret = command.run(options, client, out, false, null);

assertEquals(ret, 0);

assertThat(out.toString(), not(containsString("Ambiguous job reference")));
}

private static class TimeUtil implements RollingUpdateCommand.SleepFunction, Supplier<Long> {

private long curentTimeMillis = 0;
Expand Down

0 comments on commit 48398bd

Please sign in to comment.