Repository: aurora Updated Branches: refs/heads/master 9a93955a1 -> 11d5a72ae
Remove AddInstancesConfig parameter from addInstances RPC. Bugs closed: AURORA-1595 Reviewed at https://reviews.apache.org/r/45725/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/11d5a72a Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/11d5a72a Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/11d5a72a Branch: refs/heads/master Commit: 11d5a72ae3af2e612857f7169b08f2bd9956577f Parents: 9a93955 Author: Bill Farner <[email protected]> Authored: Wed Apr 6 17:20:29 2016 -0700 Committer: Bill Farner <[email protected]> Committed: Wed Apr 6 17:20:29 2016 -0700 ---------------------------------------------------------------------- RELEASE-NOTES.md | 1 + .../thrift/org/apache/aurora/gen/api.thrift | 13 +-- .../ShiroAuthorizingParamInterceptor.java | 8 -- .../thrift/SchedulerThriftInterface.java | 61 +++++--------- .../thrift/aop/AnnotatedAuroraAdmin.java | 2 - .../python/apache/aurora/client/api/__init__.py | 2 +- .../thrift/SchedulerThriftInterfaceTest.java | 88 ++------------------ src/test/python/apache/aurora/api_util.py | 2 +- .../python/apache/aurora/client/api/test_api.py | 1 - .../aurora/client/api/test_scheduler_client.py | 7 +- 10 files changed, 37 insertions(+), 148 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/11d5a72a/RELEASE-NOTES.md ---------------------------------------------------------------------- diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 0e2e04b..46fa2d4 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -34,6 +34,7 @@ - `TaskConfig.environment` - `TaskConfig.jobName` - `TaskQuery.owner` +- Removed deprecated `AddInstancesConfig` parameter to `addInstances` RPC. - Removed deprecated executor argument `-announcer-enable`, which was a no-op in 0.12.0. - Removed deprecated API constructs related to Locks: - removed RPCs that managed locks http://git-wip-us.apache.org/repos/asf/aurora/blob/11d5a72a/api/src/main/thrift/org/apache/aurora/gen/api.thrift ---------------------------------------------------------------------- diff --git a/api/src/main/thrift/org/apache/aurora/gen/api.thrift b/api/src/main/thrift/org/apache/aurora/gen/api.thrift index 47ab500..fc038d7 100644 --- a/api/src/main/thrift/org/apache/aurora/gen/api.thrift +++ b/api/src/main/thrift/org/apache/aurora/gen/api.thrift @@ -299,13 +299,6 @@ struct JobSummary { 3: optional i64 nextCronRunMs } -/** A request to add the following instances to an existing job. Used by addInstances. */ -struct AddInstancesConfig { - 1: JobKey key - 2: TaskConfig taskConfig - 3: set<i32> instanceIds -} - /** Closed range of integers. */ struct Range { 1: i32 first @@ -996,12 +989,8 @@ service AuroraSchedulerManager extends ReadOnlyScheduler { /** * Adds new instances with the TaskConfig of the existing instance pointed by the key. - * TODO(maxim): remove AddInstancesConfig in AURORA-1595. */ - Response addInstances( - 1: AddInstancesConfig config, - 3: InstanceKey key, - 4: i32 count) + Response addInstances(3: InstanceKey key, 4: i32 count) // TODO(maxim): reevaluate if it's still needed when client updater is gone (AURORA-785). /** http://git-wip-us.apache.org/repos/asf/aurora/blob/11d5a72a/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java index b078ef0..474a403 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java +++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java @@ -38,7 +38,6 @@ import com.google.common.collect.Maps; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.apache.aurora.common.stats.StatsProvider; -import org.apache.aurora.gen.AddInstancesConfig; import org.apache.aurora.gen.InstanceKey; import org.apache.aurora.gen.JobConfiguration; import org.apache.aurora.gen.JobKey; @@ -116,12 +115,6 @@ class ShiroAuthorizingParamInterceptor implements MethodInterceptor { private static final FieldGetter<JobUpdateKey, JobKey> JOB_UPDATE_KEY_GETTER = new ThriftFieldGetter<>(JobUpdateKey.class, JobUpdateKey._Fields.JOB, JobKey.class); - private static final FieldGetter<AddInstancesConfig, JobKey> ADD_INSTANCES_CONFIG_GETTER = - new ThriftFieldGetter<>( - AddInstancesConfig.class, - AddInstancesConfig._Fields.KEY, - JobKey.class); - private static final FieldGetter<InstanceKey, JobKey> INSTANCE_KEY_GETTER = new ThriftFieldGetter<>(InstanceKey.class, InstanceKey._Fields.JOB_KEY, JobKey.class); @@ -134,7 +127,6 @@ class ShiroAuthorizingParamInterceptor implements MethodInterceptor { FieldGetters.compose(LOCK_GETTER, LOCK_KEY_GETTER), LOCK_KEY_GETTER, JOB_UPDATE_KEY_GETTER, - ADD_INSTANCES_CONFIG_GETTER, INSTANCE_KEY_GETTER, new IdentityFieldGetter<>(JobKey.class)); http://git-wip-us.apache.org/repos/asf/aurora/blob/11d5a72a/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 86088d9..14abebb 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java @@ -35,7 +35,6 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; import com.google.common.collect.Range; -import org.apache.aurora.gen.AddInstancesConfig; import org.apache.aurora.gen.ConfigRewrite; import org.apache.aurora.gen.DrainHostsResult; import org.apache.aurora.gen.EndMaintenanceResult; @@ -710,13 +709,8 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { } @Override - public Response addInstances( - @Nullable AddInstancesConfig config, - @Nullable InstanceKey key, - int count) { - - IJobKey jobKey = - JobKeys.assertValid(IJobKey.build(config == null ? key.getJobKey() : config.getKey())); + public Response addInstances(InstanceKey key, int count) { + IJobKey jobKey = JobKeys.assertValid(IJobKey.build(key.getJobKey())); Response response = empty(); return storage.write(storeProvider -> { @@ -730,39 +724,28 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { FluentIterable<IScheduledTask> currentTasks = FluentIterable.from( storeProvider.getTaskStore().fetchTasks(Query.jobScoped(jobKey).active())); - ITaskConfig task; - Set<Integer> instanceIds; - if (config == null) { - if (count <= 0) { - return invalidRequest(INVALID_INSTANCE_COUNT); - } - - Optional<IScheduledTask> templateTask = Iterables.tryFind( - currentTasks, - e -> e.getAssignedTask().getInstanceId() == key.getInstanceId()); - if (!templateTask.isPresent()) { - return invalidRequest(INVALID_INSTANCE_ID); - } - - task = templateTask.get().getAssignedTask().getTask(); - int lastId = currentTasks - .transform(e -> e.getAssignedTask().getInstanceId()) - .toList() - .stream() - .max(Comparator.naturalOrder()).get(); - - instanceIds = ContiguousSet.create( - Range.openClosed(lastId, lastId + count), - DiscreteDomain.integers()); - } else { - checkNotBlank(config.getInstanceIds()); - addMessage(response, "The AddInstancesConfig field is deprecated."); + if (count <= 0) { + return invalidRequest(INVALID_INSTANCE_COUNT); + } - task = configurationManager.validateAndPopulate( - ITaskConfig.build(config.getTaskConfig())); - instanceIds = ImmutableSet.copyOf(config.getInstanceIds()); + Optional<IScheduledTask> templateTask = Iterables.tryFind( + currentTasks, + e -> e.getAssignedTask().getInstanceId() == key.getInstanceId()); + if (!templateTask.isPresent()) { + return invalidRequest(INVALID_INSTANCE_ID); } + int lastId = currentTasks + .transform(e -> e.getAssignedTask().getInstanceId()) + .toList() + .stream() + .max(Comparator.naturalOrder()).get(); + + Set<Integer> instanceIds = ContiguousSet.create( + Range.openClosed(lastId, lastId + count), + DiscreteDomain.integers()); + + ITaskConfig task = templateTask.get().getAssignedTask().getTask(); validateTaskLimits( task, Iterables.size(currentTasks) + instanceIds.size(), @@ -773,7 +756,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { return response.setResponseCode(OK); } catch (LockException e) { return error(LOCK_ERROR, e); - } catch (TaskDescriptionException | TaskValidationException | IllegalArgumentException e) { + } catch (TaskValidationException | IllegalArgumentException e) { return error(INVALID_REQUEST, e); } }); http://git-wip-us.apache.org/repos/asf/aurora/blob/11d5a72a/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java b/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java index 73a7128..9243c92 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java @@ -17,7 +17,6 @@ import java.util.Set; import javax.annotation.Nullable; -import org.apache.aurora.gen.AddInstancesConfig; import org.apache.aurora.gen.AuroraAdmin; import org.apache.aurora.gen.InstanceKey; import org.apache.aurora.gen.JobConfiguration; @@ -66,7 +65,6 @@ public interface AnnotatedAuroraAdmin extends AuroraAdmin.Iface { @Override Response addInstances( - @AuthorizingParam @Nullable AddInstancesConfig config, @AuthorizingParam @Nullable InstanceKey key, int count) throws TException; http://git-wip-us.apache.org/repos/asf/aurora/blob/11d5a72a/src/main/python/apache/aurora/client/api/__init__.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/client/api/__init__.py b/src/main/python/apache/aurora/client/api/__init__.py index 294c590..bbbe67b 100644 --- a/src/main/python/apache/aurora/client/api/__init__.py +++ b/src/main/python/apache/aurora/client/api/__init__.py @@ -101,7 +101,7 @@ class AuroraClientAPI(object): key = InstanceKey(jobKey=job_key.to_thrift(), instanceId=instance_id) log.info("Adding %s instances to %s using the task config of instance %s" % (count, job_key, instance_id)) - return self._scheduler_proxy.addInstances(None, key, count) + return self._scheduler_proxy.addInstances(key, count) def kill_job(self, job_key, instances=None): log.info("Killing tasks for job: %s" % job_key) http://git-wip-us.apache.org/repos/asf/aurora/blob/11d5a72a/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 3facb13..e6e79a0 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java @@ -28,7 +28,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import org.apache.aurora.common.testing.easymock.EasyMockTest; -import org.apache.aurora.gen.AddInstancesConfig; import org.apache.aurora.gen.AssignedTask; import org.apache.aurora.gen.AuroraAdmin; import org.apache.aurora.gen.ConfigRewrite; @@ -1237,55 +1236,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { } } - private static AddInstancesConfig createInstanceConfig(TaskConfig task) { - return new AddInstancesConfig() - .setTaskConfig(task) - .setInstanceIds(ImmutableSet.of(0)) - .setKey(JOB_KEY.newBuilder()); - } - - @Test - public void testAddInstances() throws Exception { - ITaskConfig populatedTask = ITaskConfig.build(populatedTask()); - expectNoCronJob(); - lockManager.assertNotLocked(LOCK_KEY); - storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active()); - expect(taskIdGenerator.generate(populatedTask, 1)) - .andReturn(TASK_ID); - expectInstanceQuotaCheck(populatedTask, ENOUGH_QUOTA); - stateManager.insertPendingTasks( - storageUtil.mutableStoreProvider, - populatedTask, - ImmutableSet.of(0)); - - control.replay(); - - AddInstancesConfig config = createInstanceConfig(populatedTask.newBuilder()); - assertOkResponse(deprecatedAddInstances(config)); - } - - @Test - public void testAddInstancesWithNullLock() throws Exception { - ITaskConfig populatedTask = ITaskConfig.build(populatedTask()); - AddInstancesConfig config = createInstanceConfig(populatedTask.newBuilder()); - expectNoCronJob(); - lockManager.assertNotLocked(LOCK_KEY); - storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active()); - expect(taskIdGenerator.generate(populatedTask, 1)) - .andReturn(TASK_ID); - expectInstanceQuotaCheck(populatedTask, ENOUGH_QUOTA); - stateManager.insertPendingTasks( - storageUtil.mutableStoreProvider, - populatedTask, - ImmutableSet.of(0)); - - control.replay(); - - Response response = deprecatedAddInstances(config); - assertOkResponse(response); - assertMessageMatches(response, "The AddInstancesConfig field is deprecated."); - } - @Test public void testAddInstancesWithInstanceKey() throws Exception { expectNoCronJob(); @@ -1305,7 +1255,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); - assertOkResponse(newAddInstances(INSTANCE_KEY, 2)); + assertOkResponse(thrift.addInstances(INSTANCE_KEY, 2)); } @Test @@ -1318,7 +1268,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { assertEquals( invalidResponse(SchedulerThriftInterface.INVALID_INSTANCE_ID), - newAddInstances(INSTANCE_KEY, 2)); + thrift.addInstances(INSTANCE_KEY, 2)); } @Test @@ -1331,19 +1281,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { assertEquals( invalidResponse(SchedulerThriftInterface.INVALID_INSTANCE_COUNT), - newAddInstances(INSTANCE_KEY, 0)); - } - - @Test - public void testAddInstancesFailsConfigCheck() throws Exception { - AddInstancesConfig config = createInstanceConfig(INVALID_TASK_CONFIG); - expectNoCronJob(); - storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active()); - lockManager.assertNotLocked(LOCK_KEY); - - control.replay(); - - assertResponse(INVALID_REQUEST, deprecatedAddInstances(config)); + thrift.addInstances(INSTANCE_KEY, 0)); } @Test @@ -1352,7 +1290,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); - assertResponse(INVALID_REQUEST, newAddInstances(INSTANCE_KEY, 1)); + assertResponse(INVALID_REQUEST, thrift.addInstances(INSTANCE_KEY, 1)); } @Test(expected = StorageException.class) @@ -1361,7 +1299,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); - newAddInstances(INSTANCE_KEY, 1); + thrift.addInstances(INSTANCE_KEY, 1); } @Test @@ -1372,7 +1310,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); - assertResponse(LOCK_ERROR, newAddInstances(INSTANCE_KEY, 1)); + assertResponse(LOCK_ERROR, thrift.addInstances(INSTANCE_KEY, 1)); } @Test @@ -1391,7 +1329,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); - assertResponse(INVALID_REQUEST, newAddInstances(INSTANCE_KEY, 1)); + assertResponse(INVALID_REQUEST, thrift.addInstances(INSTANCE_KEY, 1)); } @Test @@ -1407,7 +1345,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); - assertResponse(INVALID_REQUEST, newAddInstances(INSTANCE_KEY, 1)); + assertResponse(INVALID_REQUEST, thrift.addInstances(INSTANCE_KEY, 1)); } @Test @@ -1428,15 +1366,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); - assertResponse(INVALID_REQUEST, newAddInstances(INSTANCE_KEY, 1)); - } - - private Response newAddInstances(InstanceKey key, int count) throws Exception { - return thrift.addInstances(null, key, count); - } - - private Response deprecatedAddInstances(AddInstancesConfig config) throws Exception { - return thrift.addInstances(config, null, 0); + assertResponse(INVALID_REQUEST, thrift.addInstances(INSTANCE_KEY, 1)); } @Test http://git-wip-us.apache.org/repos/asf/aurora/blob/11d5a72a/src/test/python/apache/aurora/api_util.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/api_util.py b/src/test/python/apache/aurora/api_util.py index 4fde4ba..983cde4 100644 --- a/src/test/python/apache/aurora/api_util.py +++ b/src/test/python/apache/aurora/api_util.py @@ -88,7 +88,7 @@ class SchedulerThriftApiSpec(ReadOnlyScheduler.Iface): def killTasks(self, jobKey, instances): pass - def addInstances(self, config, key, count): + def addInstances(self, key, count): pass def replaceCronTemplate(self, config): http://git-wip-us.apache.org/repos/asf/aurora/blob/11d5a72a/src/test/python/apache/aurora/client/api/test_api.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/client/api/test_api.py b/src/test/python/apache/aurora/client/api/test_api.py index 0dd4b41..9e2e2fa 100644 --- a/src/test/python/apache/aurora/client/api/test_api.py +++ b/src/test/python/apache/aurora/client/api/test_api.py @@ -118,7 +118,6 @@ class TestJobUpdateApis(unittest.TestCase): api.add_instances(job_key, 1, 10) mock_proxy.addInstances.assert_called_once_with( - None, InstanceKey(jobKey=job_key.to_thrift(), instanceId=1), 10) http://git-wip-us.apache.org/repos/asf/aurora/blob/11d5a72a/src/test/python/apache/aurora/client/api/test_scheduler_client.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/client/api/test_scheduler_client.py b/src/test/python/apache/aurora/client/api/test_scheduler_client.py index bd7c834..afbd385 100644 --- a/src/test/python/apache/aurora/client/api/test_scheduler_client.py +++ b/src/test/python/apache/aurora/client/api/test_scheduler_client.py @@ -147,12 +147,9 @@ class TestSchedulerProxyInjection(unittest.TestCase): self.make_scheduler_proxy().getQuota(ROLE) def test_addInstances(self): - self.mock_thrift_client.addInstances( - IsA(JobKey), - IgnoreArg(), - IsA(Lock)).AndReturn(DEFAULT_RESPONSE) + self.mock_thrift_client.addInstances(IsA(JobKey), 1).AndReturn(DEFAULT_RESPONSE) self.mox.ReplayAll() - self.make_scheduler_proxy().addInstances(JobKey(), {}, Lock()) + self.make_scheduler_proxy().addInstances(JobKey(), 1) def test_getJobUpdateSummaries(self): self.mock_thrift_client.getJobUpdateSummaries(IsA(JobUpdateQuery)).AndReturn(DEFAULT_RESPONSE)
