You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by ma...@apache.org on 2014/10/22 18:01:49 UTC
git commit: Adding quota check into scheduleCronJob RPC.
Repository: incubator-aurora
Updated Branches:
refs/heads/master dde0a7267 -> 4269acac0
Adding quota check into scheduleCronJob RPC.
Bugs closed: AURORA-824
Reviewed at https://reviews.apache.org/r/26997/
Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/4269acac
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/4269acac
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/4269acac
Branch: refs/heads/master
Commit: 4269acac0a84592ac3c25ab8ade47b8470755ca2
Parents: dde0a72
Author: Maxim Khutornenko <ma...@apache.org>
Authored: Wed Oct 22 09:01:30 2014 -0700
Committer: Maxim Khutornenko <ma...@apache.org>
Committed: Wed Oct 22 09:01:30 2014 -0700
----------------------------------------------------------------------
build.gradle | 2 +-
.../aurora/scheduler/cron/SanitizedCronJob.java | 25 ++++
.../thrift/SchedulerThriftInterface.java | 38 +++----
.../thrift/SchedulerThriftInterfaceTest.java | 113 ++++++++++++++++++-
4 files changed, 156 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4269acac/build.gradle
----------------------------------------------------------------------
diff --git a/build.gradle b/build.gradle
index 459c79d..62d1492 100644
--- a/build.gradle
+++ b/build.gradle
@@ -506,7 +506,7 @@ test.finalizedBy jacocoTestReport
task analyzeReport(type: CoverageReportCheck) {
coverageReportFile = "$reportPath/jacocoTestReport.xml"
- minInstructionCoverage = 0.87
+ minInstructionCoverage = 0.88
minBranchCoverage = 0.82
legacyClassesWithoutCoverage = file('config/legacy_untested_classes.txt').readLines()
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4269acac/src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java b/src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java
index 48b403e..babbf42 100644
--- a/src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java
+++ b/src/main/java/org/apache/aurora/scheduler/cron/SanitizedCronJob.java
@@ -15,6 +15,7 @@ package org.apache.aurora.scheduler.cron;
import javax.annotation.Nullable;
+import com.google.common.base.Objects;
import com.google.common.base.Optional;
import org.apache.aurora.gen.CronCollisionPolicy;
@@ -126,4 +127,28 @@ public final class SanitizedCronJob {
public SanitizedConfiguration getSanitizedConfig() {
return config;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (!(o instanceof SanitizedCronJob)) {
+ return false;
+ }
+
+ SanitizedCronJob other = (SanitizedCronJob) o;
+
+ return Objects.equal(config, other.config) && Objects.equal(crontabEntry, other.crontabEntry);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(config, crontabEntry);
+ }
+
+ @Override
+ public String toString() {
+ return Objects.toStringHelper(this)
+ .add("config", config)
+ .add("crontabEntry", crontabEntry)
+ .toString();
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4269acac/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 e792d23..743189b 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
@@ -308,13 +308,10 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
return invalidResponse("Job already exists: " + JobKeys.canonicalString(job.getKey()));
}
- ITaskConfig taskConfig = sanitized.getJobConfig().getTaskConfig();
- int instanceCount = sanitized.getInstanceIds().size();
+ ITaskConfig template = sanitized.getJobConfig().getTaskConfig();
+ int count = sanitized.getJobConfig().getInstanceCount();
- validateTaskLimits(
- taskConfig,
- instanceCount,
- quotaManager.checkInstanceAddition(taskConfig, instanceCount));
+ validateTaskLimits(template, count, quotaManager.checkInstanceAddition(template, count));
// TODO(mchucarroll): deprecate cron as a part of create/kill job.(AURORA-454)
if (sanitized.isCron()) {
@@ -322,8 +319,8 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
+ " with cron via createJob (AURORA_454)");
cronJobManager.createJob(SanitizedCronJob.from(sanitized));
} else {
- LOG.info("Launching " + instanceCount + " tasks.");
- stateManager.insertPendingTasks(taskConfig, sanitized.getInstanceIds());
+ LOG.info("Launching " + count + " tasks.");
+ stateManager.insertPendingTasks(template, sanitized.getInstanceIds());
}
return okEmptyResponse();
} catch (LockException e) {
@@ -362,21 +359,24 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
LOG.info("Invalid attempt to schedule non-cron job " + jobKey + " with cron.");
return invalidResponse("Job " + jobKey + " has no cron schedule");
}
- try {
- // TODO(mchucarroll): Merge CronJobManager.createJob/updateJob
- if (cronJobManager.hasJob(sanitized.getJobConfig().getKey())) {
- // The job already has a schedule: so update it.
- cronJobManager.updateJob(SanitizedCronJob.from(sanitized));
- } else {
- cronJobManager.createJob(SanitizedCronJob.from(sanitized));
- }
- } catch (CronException e) {
- return errorResponse(INVALID_REQUEST, e);
+
+ ITaskConfig template = sanitized.getJobConfig().getTaskConfig();
+ int count = sanitized.getJobConfig().getInstanceCount();
+
+ validateTaskLimits(template, count, quotaManager.checkInstanceAddition(template, count));
+
+ // TODO(mchucarroll): Merge CronJobManager.createJob/updateJob
+ if (cronJobManager.hasJob(sanitized.getJobConfig().getKey())) {
+ // The job already has a schedule: so update it.
+ cronJobManager.updateJob(SanitizedCronJob.from(sanitized));
+ } else {
+ cronJobManager.createJob(SanitizedCronJob.from(sanitized));
}
+
return okEmptyResponse();
} catch (LockException e) {
return errorResponse(LOCK_ERROR, e);
- } catch (TaskDescriptionException e) {
+ } catch (TaskDescriptionException | TaskValidationException | CronException e) {
return errorResponse(INVALID_REQUEST, e);
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4269acac/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 f36a88f..3fde3f7 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
@@ -1084,6 +1084,16 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
}
@Test
+ public void testReplaceCronTemplateFailedLockValidation() throws Exception {
+ expectAuth(ROLE, true);
+ lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
+ expectLastCall().andThrow(new LockException("Failed lock."));
+ control.replay();
+
+ assertResponse(LOCK_ERROR, thrift.replaceCronTemplate(CRON_JOB, DEFAULT_LOCK, SESSION));
+ }
+
+ @Test
public void testReplaceCronTemplateDoesNotExist() throws Exception {
expectAuth(ROLE, true);
lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
@@ -1096,16 +1106,106 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
}
@Test
- public void testUpdateScheduledCronJob() throws Exception {
+ public void testStartCronJob() throws Exception {
+ expectAuth(ROLE, true);
+ cronJobManager.startJobNow(JOB_KEY);
+ control.replay();
+ assertResponse(OK, thrift.startCronJob(JOB_KEY.newBuilder(), SESSION));
+ }
+
+ @Test
+ public void testStartCronJobFailsAuth() throws Exception {
+ expectAuth(ROLE, false);
+ control.replay();
+ assertResponse(AUTH_FAILED, thrift.startCronJob(JOB_KEY.newBuilder(), SESSION));
+ }
+
+ @Test
+ public void testStartCronJobFailsInCronManager() throws Exception {
+ expectAuth(ROLE, true);
+ cronJobManager.startJobNow(JOB_KEY);
+ expectLastCall().andThrow(new CronException("failed"));
+ control.replay();
+ assertResponse(INVALID_REQUEST, thrift.startCronJob(JOB_KEY.newBuilder(), SESSION));
+ }
+
+ @Test
+ public void testScheduleCronCreatesJob() throws Exception {
expectAuth(ROLE, true);
lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
+ SanitizedConfiguration sanitized =
+ SanitizedConfiguration.fromUnsanitized(IJobConfiguration.build(CRON_JOB));
+
+ expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
+ .andReturn(TASK_ID);
+ expect(quotaManager.checkInstanceAddition(sanitized.getJobConfig().getTaskConfig(), 1))
+ .andReturn(ENOUGH_QUOTA);
+
+ expect(cronJobManager.hasJob(JOB_KEY)).andReturn(false);
+ cronJobManager.createJob(SanitizedCronJob.from(sanitized));
+ control.replay();
+ assertResponse(OK, thrift.scheduleCronJob(CRON_JOB, DEFAULT_LOCK, SESSION));
+ }
+
+ @Test
+ public void testScheduleCronUpdatesJob() throws Exception {
+ expectAuth(ROLE, true);
+ lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
+ SanitizedConfiguration sanitized =
+ SanitizedConfiguration.fromUnsanitized(IJobConfiguration.build(CRON_JOB));
+
+ expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
+ .andReturn(TASK_ID);
+ expect(quotaManager.checkInstanceAddition(sanitized.getJobConfig().getTaskConfig(), 1))
+ .andReturn(ENOUGH_QUOTA);
+
expect(cronJobManager.hasJob(JOB_KEY)).andReturn(true);
- cronJobManager.updateJob(anyObject(SanitizedCronJob.class));
+ cronJobManager.updateJob(SanitizedCronJob.from(sanitized));
control.replay();
assertResponse(OK, thrift.scheduleCronJob(CRON_JOB, DEFAULT_LOCK, SESSION));
}
@Test
+ public void testUpdateScheduledCronJobFailedAuth() throws Exception {
+ expectAuth(ROLE, false);
+ control.replay();
+ assertResponse(AUTH_FAILED, thrift.scheduleCronJob(CRON_JOB, DEFAULT_LOCK, SESSION));
+ }
+
+ @Test
+ public void testScheduleCronJobFailsLockValidation() throws Exception {
+ expectAuth(ROLE, true);
+ lockManager.validateIfLocked(LOCK_KEY, Optional.of(LOCK));
+ expectLastCall().andThrow(new LockException("Failed lock"));
+ control.replay();
+ assertResponse(LOCK_ERROR, thrift.scheduleCronJob(CRON_JOB, LOCK.newBuilder(), SESSION));
+ }
+
+ @Test
+ public void testScheduleCronJobFailsWithNoCronSchedule() throws Exception {
+ expectAuth(ROLE, true);
+ lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
+ control.replay();
+ assertResponse(INVALID_REQUEST, thrift.scheduleCronJob(makeJob(), DEFAULT_LOCK, SESSION));
+ }
+
+ @Test
+ public void testScheduleCronFailsQuotaCheck() throws Exception {
+ expectAuth(ROLE, true);
+ lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
+ SanitizedConfiguration sanitized =
+ SanitizedConfiguration.fromUnsanitized(IJobConfiguration.build(CRON_JOB));
+
+ expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
+ .andReturn(TASK_ID);
+ expect(quotaManager.checkInstanceAddition(sanitized.getJobConfig().getTaskConfig(), 1))
+ .andReturn(NOT_ENOUGH_QUOTA);
+
+ control.replay();
+ assertResponse(INVALID_REQUEST, thrift.scheduleCronJob(CRON_JOB, DEFAULT_LOCK, SESSION));
+ }
+
+ @Test
public void testDescheduleCronJob() throws Exception {
expectAuth(ROLE, true);
lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
@@ -1123,6 +1223,15 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
}
@Test
+ public void testDescheduleCronJobFailsLockValidation() throws Exception {
+ expectAuth(ROLE, true);
+ lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
+ expectLastCall().andThrow(new LockException("Failed lock"));
+ control.replay();
+ assertResponse(LOCK_ERROR, thrift.descheduleCronJob(CRON_JOB.getKey(), DEFAULT_LOCK, SESSION));
+ }
+
+ @Test
public void testDescheduleCronJobWithError() throws Exception {
expectAuth(ROLE, true);
lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());