aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject git commit: Adding quota check into scheduleCronJob RPC.
Date Wed, 22 Oct 2014 16:01:49 GMT
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 <maxim@apache.org>
Authored: Wed Oct 22 09:01:30 2014 -0700
Committer: Maxim Khutornenko <maxim@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());


Mime
View raw message