Repository: aurora Updated Branches: refs/heads/master 1024a8579 -> 97520ba4d
Prevent quota from being set below current production reservation. Having quota below current production reservation is illogical and the scheduler should reject requests that would result in this. Bugs closed: AURORA-1375 Reviewed at https://reviews.apache.org/r/35954/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/97520ba4 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/97520ba4 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/97520ba4 Branch: refs/heads/master Commit: 97520ba4db2390f0e1fbdafbd1c776d18f9fa021 Parents: 1024a85 Author: Zameer Manji <zma...@apache.org> Authored: Wed Jul 1 11:15:59 2015 -0700 Committer: zma...@apache.org <zma...@twitter.com> Committed: Wed Jul 1 11:15:59 2015 -0700 ---------------------------------------------------------------------- .../aurora/scheduler/quota/QuotaManager.java | 55 ++++++++------------ .../thrift/SchedulerThriftInterface.java | 2 +- .../scheduler/quota/QuotaManagerImplTest.java | 31 +++++++++-- .../thrift/SchedulerThriftInterfaceTest.java | 4 +- .../aurora/scheduler/thrift/ThriftIT.java | 5 +- 5 files changed, 57 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/97520ba4/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java b/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java index 7453680..e30b229 100644 --- a/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java +++ b/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java @@ -36,7 +36,7 @@ import org.apache.aurora.scheduler.base.JobKeys; import org.apache.aurora.scheduler.base.Query; import org.apache.aurora.scheduler.base.ResourceAggregates; import org.apache.aurora.scheduler.storage.JobUpdateStore; -import org.apache.aurora.scheduler.storage.QuotaStore; +import org.apache.aurora.scheduler.storage.Storage.MutableStoreProvider; import org.apache.aurora.scheduler.storage.Storage.StoreProvider; import org.apache.aurora.scheduler.storage.entities.IAssignedTask; import org.apache.aurora.scheduler.storage.entities.IInstanceTaskConfig; @@ -71,11 +71,13 @@ public interface QuotaManager { * * @param role Quota owner. * @param quota Quota to save. - * @param quoteStore A quota store. + * @param storeProvider A store provider to access quota and other data. * @throws QuotaException If provided quota specification is invalid. */ - void saveQuota(String role, IResourceAggregate quota, QuotaStore.Mutable quoteStore) - throws QuotaException; + void saveQuota( + String role, + IResourceAggregate quota, + MutableStoreProvider storeProvider) throws QuotaException; /** * Gets {@code QuotaInfo} for the specified role. @@ -139,7 +141,7 @@ public interface QuotaManager { public void saveQuota( final String ownerRole, final IResourceAggregate quota, - QuotaStore.Mutable quoteStore) throws QuotaException { + MutableStoreProvider storeProvider) throws QuotaException { if (!quota.isSetNumCpus() || !quota.isSetRamMb() || !quota.isSetDiskMb()) { throw new QuotaException("Missing quota specification(s) in: " + quota.toString()); @@ -149,7 +151,18 @@ public interface QuotaManager { throw new QuotaException("Negative values in: " + quota.toString()); } - quoteStore.saveQuota(ownerRole, quota); + QuotaInfo info = getQuotaInfo(ownerRole, Optional.absent(), storeProvider); + IResourceAggregate prodConsumption = info.getProdConsumption(); + if (quota.getNumCpus() < prodConsumption.getNumCpus() + || quota.getRamMb() < prodConsumption.getRamMb() + || quota.getDiskMb() < prodConsumption.getDiskMb()) { + throw new QuotaException(String.format( + "Quota: %s is less then current prod reservation: %s", + quota.toString(), + prodConsumption.toString())); + } + + storeProvider.getQuotaStore().saveQuota(ownerRole, quota); } @Override @@ -265,14 +278,6 @@ public interface QuotaManager { return new QuotaInfo(quota, prodConsumed, nonProdConsumed); } - private static final Function<IJobConfiguration, ITaskConfig> JOB_TO_TASK = - new Function<IJobConfiguration, ITaskConfig>() { - @Override - public ITaskConfig apply(IJobConfiguration job) { - return job.getTaskConfig(); - } - }; - private IResourceAggregate getConsumption( FluentIterable<IAssignedTask> tasks, Map<IJobKey, IJobUpdateInstructions> updatesByKey, @@ -296,7 +301,7 @@ public interface QuotaManager { IResourceAggregate cronConsumption = getCronConsumption( Iterables.filter( cronTemplatesByKey.values(), - Predicates.compose(prodFilter, JOB_TO_TASK)), + Predicates.compose(prodFilter, IJobConfiguration::getTaskConfig)), filteredTasks.transform(ASSIGNED_TO_INFO)); return add(nonCronConsumption, cronConsumption); @@ -323,7 +328,7 @@ public interface QuotaManager { .transform(ASSIGNED_TO_INFO)); final Predicate<IInstanceTaskConfig> instanceFilter = - Predicates.compose(configFilter, INSTANCE_CONFIG); + Predicates.compose(configFilter, IInstanceTaskConfig::getTask); IResourceAggregate updateConsumption = addAll(Iterables.transform(updatesByKey.values(), updateResources(instanceFilter))); @@ -380,14 +385,6 @@ public interface QuotaManager { }; } - private static final Function<IJobUpdate, IJobUpdateInstructions> UPDATE_TO_INSTRUCTIONS = - new Function<IJobUpdate, IJobUpdateInstructions>() { - @Override - public IJobUpdateInstructions apply(IJobUpdate update) { - return update.getInstructions(); - } - }; - private static Map<IJobKey, IJobUpdateInstructions> fetchActiveJobUpdates( final JobUpdateStore jobUpdateStore, String role) { @@ -404,7 +401,7 @@ public interface QuotaManager { FluentIterable.from(jobUpdateStore.fetchJobUpdateSummaries(updateQuery(role))) .transform(fetchUpdate) .uniqueIndex(UPDATE_TO_JOB_KEY), - UPDATE_TO_INSTRUCTIONS); + IJobUpdate::getInstructions); } @VisibleForTesting @@ -414,14 +411,6 @@ public interface QuotaManager { .setUpdateStatuses(Updates.ACTIVE_JOB_UPDATE_STATES)); } - private static final Function<IInstanceTaskConfig, ITaskConfig> INSTANCE_CONFIG = - new Function<IInstanceTaskConfig, ITaskConfig>() { - @Override - public ITaskConfig apply(IInstanceTaskConfig config) { - return config.getTask(); - } - }; - private static final Function<ITaskConfig, IResourceAggregate> CONFIG_RESOURCES = new Function<ITaskConfig, IResourceAggregate>() { @Override http://git-wip-us.apache.org/repos/asf/aurora/blob/97520ba4/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 9af379c..dea8107 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java @@ -659,7 +659,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { quotaManager.saveQuota( ownerRole, IResourceAggregate.build(resourceAggregate), - store.getQuotaStore()); + store); } }); return ok(); http://git-wip-us.apache.org/repos/asf/aurora/blob/97520ba4/src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java b/src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java index 58ee226..4cc0ee5 100644 --- a/src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java +++ b/src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java @@ -669,10 +669,19 @@ public class QuotaManagerImplTest extends EasyMockTest { @Test public void testSaveQuotaPasses() throws Exception { + expectNoJobUpdates(); + expectNoCronJobs(); + IScheduledTask prodTask = createProdTask("foo", 1, 1, 1); + expectTasks(prodTask); + expectQuota(IResourceAggregate.build(new ResourceAggregate(1, 1, 1))); + storageUtil.quotaStore.saveQuota(ROLE, QUOTA); control.replay(); - quotaManager.saveQuota(ROLE, QUOTA, storageUtil.mutableStoreProvider.getQuotaStore()); + quotaManager.saveQuota( + ROLE, + QUOTA, + storageUtil.mutableStoreProvider); } @Test(expected = QuotaException.class) @@ -681,7 +690,7 @@ public class QuotaManagerImplTest extends EasyMockTest { quotaManager.saveQuota( ROLE, IResourceAggregate.build(new ResourceAggregate()), - storageUtil.mutableStoreProvider.getQuotaStore()); + storageUtil.mutableStoreProvider); } @Test(expected = QuotaException.class) @@ -690,7 +699,23 @@ public class QuotaManagerImplTest extends EasyMockTest { quotaManager.saveQuota( ROLE, IResourceAggregate.build(new ResourceAggregate(-2.0, 4, 5)), - storageUtil.mutableStoreProvider.getQuotaStore()); + storageUtil.mutableStoreProvider); + } + + @Test(expected = QuotaException.class) + public void testSaveQuotaFailsWhenBelowCurrentReservation() throws Exception { + expectNoJobUpdates(); + expectNoCronJobs(); + IScheduledTask prodTask = createProdTask("foo", 10, 100, 100); + expectTasks(prodTask); + expectQuota(IResourceAggregate.build(new ResourceAggregate(20, 200, 200))); + + control.replay(); + + quotaManager.saveQuota( + ROLE, + IResourceAggregate.build(new ResourceAggregate(1, 1, 1)), + storageUtil.mutableStoreProvider); } @Test http://git-wip-us.apache.org/repos/asf/aurora/blob/97520ba4/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 ee8f542..be9a64f 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java @@ -844,7 +844,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { quotaManager.saveQuota( ROLE, IResourceAggregate.build(resourceAggregate), - storageUtil.quotaStore); + storageUtil.mutableStoreProvider); control.replay(); @@ -860,7 +860,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { quotaManager.saveQuota( ROLE, IResourceAggregate.build(resourceAggregate), - storageUtil.quotaStore); + storageUtil.mutableStoreProvider); expectLastCall().andThrow(new QuotaManager.QuotaException("fail")); http://git-wip-us.apache.org/repos/asf/aurora/blob/97520ba4/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java b/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java index 2a2b499..2f8dd3a 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java @@ -188,7 +188,10 @@ public class ThriftIT extends EasyMockTest { @Test public void testProvisionAccess() throws Exception { storageTestUtil.expectOperations(); - quotaManager.saveQuota(USER, QUOTA, storageTestUtil.quotaStore); + quotaManager.saveQuota( + USER, + QUOTA, + storageTestUtil.mutableStoreProvider); expectLastCall().times(2); control.replay();
