Repository: aurora Updated Branches: refs/heads/master 545e8396d -> 4b117395a
Prevent job updates from allowing unbounded instance events Bugs closed: AURORA-1096 Reviewed at https://reviews.apache.org/r/36436/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/4b117395 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/4b117395 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/4b117395 Branch: refs/heads/master Commit: 4b117395ab4b04f507f10b7a6ad4e6be0387a023 Parents: 545e839 Author: Joe Smith <yasumo...@gmail.com> Authored: Wed Jul 15 15:02:24 2015 -0700 Committer: Bill Farner <wfar...@apache.org> Committed: Wed Jul 15 15:02:24 2015 -0700 ---------------------------------------------------------------------- .../thrift/SchedulerThriftInterface.java | 26 ++++++++++++++-- .../thrift/SchedulerThriftInterfaceTest.java | 31 +++++++++++++++++++- 2 files changed, 53 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/4b117395/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java index dc0cd2d..22786de 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java @@ -161,9 +161,20 @@ import static org.apache.aurora.scheduler.thrift.Responses.ok; */ @DecoratedThrift class SchedulerThriftInterface implements AnnotatedAuroraAdmin { + private static final int DEFAULT_MAX_TASKS_PER_JOB = 4000; + private static final int DEFAULT_MAX_UPDATE_INSTANCE_FAILURES = + DEFAULT_MAX_TASKS_PER_JOB * 5; + @Positive @CmdLine(name = "max_tasks_per_job", help = "Maximum number of allowed tasks in a single job.") - public static final Arg<Integer> MAX_TASKS_PER_JOB = Arg.create(4000); + public static final Arg<Integer> MAX_TASKS_PER_JOB = Arg.create(DEFAULT_MAX_TASKS_PER_JOB); + + @Positive + @CmdLine(name = "max_update_instance_failures", help = "Upper limit on the number of " + + "failures allowed during a job update. This helps cap potentially unbounded entries into " + + "storage.") + public static final Arg<Integer> MAX_UPDATE_INSTANCE_FAILURES = Arg.create( + DEFAULT_MAX_UPDATE_INSTANCE_FAILURES); // This number is derived from the maximum file name length limit on most UNIX systems, less // the number of characters we've observed being added by mesos for the executor ID, prefix, and @@ -1113,6 +1124,11 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { return invalidRequest(INVALID_MAX_FAILED_INSTANCES); } + if (settings.getMaxPerInstanceFailures() * mutableRequest.getInstanceCount() + > MAX_UPDATE_INSTANCE_FAILURES.get()) { + return invalidRequest(TOO_MANY_POTENTIAL_FAILED_INSTANCES); + } + if (settings.getMinWaitInInstanceRunningMs() < 0) { return invalidRequest(INVALID_MIN_WAIT_TO_RUNNING); } @@ -1376,8 +1392,12 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { static final String INVALID_MAX_FAILED_INSTANCES = "maxFailedInstances must be non-negative."; @VisibleForTesting - static final String INVALID_MAX_INSTANCE_FAILURES = - "maxPerInstanceFailures must be non-negative."; + static final String TOO_MANY_POTENTIAL_FAILED_INSTANCES = "Your update allows too many failures " + + "to occur, consider decreasing the per-instance failures or maxFailedInstances."; + + @VisibleForTesting + static final String INVALID_MAX_INSTANCE_FAILURES + = "maxPerInstanceFailures must be non-negative."; @VisibleForTesting static final String INVALID_MIN_WAIT_TO_RUNNING = http://git-wip-us.apache.org/repos/asf/aurora/blob/4b117395/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java index d28baba..94ea0c8 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java @@ -53,6 +53,7 @@ import org.apache.aurora.gen.JobKey; import org.apache.aurora.gen.JobUpdate; import org.apache.aurora.gen.JobUpdateInstructions; import org.apache.aurora.gen.JobUpdatePulseStatus; +import org.apache.aurora.gen.JobUpdateQuery; import org.apache.aurora.gen.JobUpdateRequest; import org.apache.aurora.gen.JobUpdateSettings; import org.apache.aurora.gen.JobUpdateSummary; @@ -1932,6 +1933,20 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { } @Test + public void testStartUpdateFailsTooManyPerInstanceFailures() throws Exception { + control.replay(); + + JobUpdateRequest updateRequest = buildServiceJobUpdateRequest(); + updateRequest.getSettings() + .setMaxPerInstanceFailures(SchedulerThriftInterface.MAX_UPDATE_INSTANCE_FAILURES.get() + + 10); + + assertEquals( + invalidResponse(SchedulerThriftInterface.TOO_MANY_POTENTIAL_FAILED_INSTANCES), + thrift.startJobUpdate(updateRequest, AUDIT_MESSAGE, SESSION)); + } + + @Test public void testStartUpdateFailsInvalidMaxFailedInstances() throws Exception { control.replay(); @@ -2329,7 +2344,8 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); assertEquals( - okResponse(Result.pulseJobUpdateResult(new PulseJobUpdateResult(JobUpdatePulseStatus.OK))), + okResponse(Result.pulseJobUpdateResult( + new PulseJobUpdateResult(JobUpdatePulseStatus.OK))), thrift.pulseJobUpdate(UPDATE_KEY.newBuilder(), SESSION)); } @@ -2366,6 +2382,19 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { assertResponse(AUTH_FAILED, thrift.pulseJobUpdate(UPDATE_KEY.newBuilder(), SESSION)); } + @Test + public void testGetJobUpdateSummaries() throws Exception { + Response updateSummary = Responses.empty() + .setResponseCode(OK) + .setDetails(ImmutableList.of(new ResponseDetail("summary"))); + + expect(readOnlyScheduler.getJobUpdateSummaries( + anyObject(JobUpdateQuery.class))).andReturn(updateSummary); + control.replay(); + + assertEquals(updateSummary, thrift.getJobUpdateSummaries(new JobUpdateQuery())); + } + private static final String AUTH_DENIED_MESSAGE = "Denied!"; private IExpectationSetters<?> expectAuth(Set<String> roles, boolean allowed)
