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 2015/10/14 02:40:48 UTC

aurora git commit: Adding getJobUpdateDiff thrift API.

Repository: aurora
Updated Branches:
  refs/heads/master f630bf705 -> 888f9a3cc


Adding getJobUpdateDiff thrift API.

Bugs closed: AURORA-1515

Reviewed at https://reviews.apache.org/r/39143/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/888f9a3c
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/888f9a3c
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/888f9a3c

Branch: refs/heads/master
Commit: 888f9a3ccf8ee84dbf30f9bb612469ac70c019f7
Parents: f630bf7
Author: Maxim Khutornenko <ma...@apache.org>
Authored: Tue Oct 13 17:36:21 2015 -0700
Committer: Maxim Khutornenko <ma...@apache.org>
Committed: Tue Oct 13 17:36:21 2015 -0700

----------------------------------------------------------------------
 .../thrift/org/apache/aurora/gen/api.thrift     |  18 +++
 .../scheduler/thrift/ReadOnlySchedulerImpl.java |  69 ++++++++-
 .../thrift/SchedulerThriftInterface.java        |   5 +
 .../aurora/scheduler/updater/JobDiff.java       |  17 ++-
 .../aurora/scheduler/thrift/Fixtures.java       |   2 +
 .../thrift/ReadOnlySchedulerImplTest.java       | 141 +++++++++++++++++++
 .../thrift/SchedulerThriftInterfaceTest.java    |   8 +-
 .../scheduler/thrift/aop/ForwardingThrift.java  |   5 +
 .../aurora/scheduler/updater/JobDiffTest.java   |  51 ++++---
 9 files changed, 280 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/888f9a3c/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 f56d7bd..f0d4ef8 100644
--- a/api/src/main/thrift/org/apache/aurora/gen/api.thrift
+++ b/api/src/main/thrift/org/apache/aurora/gen/api.thrift
@@ -908,6 +908,20 @@ struct PulseJobUpdateResult {
   1: JobUpdatePulseStatus status
 }
 
+struct GetJobUpdateDiffResult {
+  /** Instance addition diff details. */
+  1: set<ConfigGroup> add
+
+  /** Instance removal diff details. */
+  2: set<ConfigGroup> remove
+
+  /** Instance update diff details. */
+  3: set<ConfigGroup> update
+
+  /** Instances unchanged by the update. */
+  4: set<ConfigGroup> unchanged
+}
+
 /** Information about the scheduler. */
 struct ServerInfo {
   1: string clusterName
@@ -938,6 +952,7 @@ union Result {
   23: GetJobUpdateSummariesResult getJobUpdateSummariesResult
   24: GetJobUpdateDetailsResult getJobUpdateDetailsResult
   25: PulseJobUpdateResult pulseJobUpdateResult
+  26: GetJobUpdateDiffResult getJobUpdateDiffResult
 }
 
 struct ResponseDetail {
@@ -1002,6 +1017,9 @@ service ReadOnlyScheduler {
 
   /** Gets job update details. */
   Response getJobUpdateDetails(1: JobUpdateKey key)
+
+  /** Gets the diff between client (desired) and server (current) job states. */
+  Response getJobUpdateDiff(1: JobUpdateRequest request)
 }
 
 // Due to assumptions in the client all authenticated RPCs must have a SessionKey as their

http://git-wip-us.apache.org/repos/asf/aurora/blob/888f9a3c/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java b/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
index c09130b..ea43912 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
@@ -22,7 +22,9 @@ import java.util.Set;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
+import com.google.common.base.Functions;
 import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
@@ -43,6 +45,7 @@ import org.apache.aurora.gen.ConfigGroup;
 import org.apache.aurora.gen.ConfigSummary;
 import org.apache.aurora.gen.ConfigSummaryResult;
 import org.apache.aurora.gen.GetJobUpdateDetailsResult;
+import org.apache.aurora.gen.GetJobUpdateDiffResult;
 import org.apache.aurora.gen.GetJobUpdateSummariesResult;
 import org.apache.aurora.gen.GetJobsResult;
 import org.apache.aurora.gen.GetLocksResult;
@@ -54,6 +57,7 @@ import org.apache.aurora.gen.JobSummary;
 import org.apache.aurora.gen.JobSummaryResult;
 import org.apache.aurora.gen.JobUpdateKey;
 import org.apache.aurora.gen.JobUpdateQuery;
+import org.apache.aurora.gen.JobUpdateRequest;
 import org.apache.aurora.gen.PendingReason;
 import org.apache.aurora.gen.PopulateJobResult;
 import org.apache.aurora.gen.ReadOnlyScheduler;
@@ -89,11 +93,13 @@ import org.apache.aurora.scheduler.storage.entities.IJobKey;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateDetails;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateKey;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateQuery;
+import org.apache.aurora.scheduler.storage.entities.IJobUpdateRequest;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateSummary;
 import org.apache.aurora.scheduler.storage.entities.ILock;
 import org.apache.aurora.scheduler.storage.entities.IRange;
 import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
 import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
+import org.apache.aurora.scheduler.updater.JobDiff;
 import org.apache.thrift.TException;
 
 import static java.util.Objects.requireNonNull;
@@ -219,14 +225,10 @@ class ReadOnlySchedulerImpl implements ReadOnlyScheduler.Iface {
     Map<Integer, ITaskConfig> tasksByInstance = Maps.transformValues(
         Maps.uniqueIndex(assignedTasks, IAssignedTask::getInstanceId),
         IAssignedTask::getTask);
-    Multimap<ITaskConfig, Integer> instancesByDetails = Multimaps.invertFrom(
-        Multimaps.forMap(tasksByInstance),
-        HashMultimap.create());
-    Iterable<ConfigGroup> groups = Iterables.transform(
-        instancesByDetails.asMap().entrySet(), TO_GROUP);
+    Set<ConfigGroup> groups = instancesToConfigGroups(tasksByInstance);
 
-    ConfigSummary summary = new ConfigSummary(job, ImmutableSet.copyOf(groups));
-    return ok(Result.configSummaryResult(new ConfigSummaryResult().setSummary(summary)));
+    return ok(Result.configSummaryResult(
+        new ConfigSummaryResult().setSummary(new ConfigSummary(job, groups))));
   }
 
   @Override
@@ -353,6 +355,56 @@ class ReadOnlySchedulerImpl implements ReadOnlyScheduler.Iface {
     }
   }
 
+  @Override
+  public Response getJobUpdateDiff(JobUpdateRequest mutableRequest) {
+    final IJobUpdateRequest request = IJobUpdateRequest.build(requireNonNull(mutableRequest));
+    final IJobKey job = request.getTaskConfig().getJob();
+
+    return storage.read(new Quiet<Response>() {
+      @Override
+      public Response apply(StoreProvider storeProvider) throws RuntimeException {
+        if (storeProvider.getCronJobStore().fetchJob(job).isPresent()) {
+          return Responses.invalidRequest(NO_CRON);
+        }
+
+        JobDiff diff = JobDiff.compute(
+            storeProvider.getTaskStore(),
+            job,
+            JobDiff.asMap(request.getTaskConfig(), request.getInstanceCount()),
+            request.getSettings().getUpdateOnlyTheseInstances());
+
+        Map<Integer, ITaskConfig> replaced = diff.getReplacedInstances();
+        Map<Integer, ITaskConfig> replacements = Maps.asMap(
+            diff.getReplacementInstances(),
+            Functions.constant(request.getTaskConfig()));
+
+        Map<Integer, ITaskConfig> add = Maps.filterKeys(
+            replacements,
+            Predicates.in(Sets.difference(replacements.keySet(), replaced.keySet())));
+        Map<Integer, ITaskConfig> remove = Maps.filterKeys(
+            replaced,
+            Predicates.in(Sets.difference(replaced.keySet(), replacements.keySet())));
+        Map<Integer, ITaskConfig> update = Maps.filterKeys(
+            replaced,
+            Predicates.in(Sets.intersection(replaced.keySet(), replacements.keySet())));
+
+        return ok(Result.getJobUpdateDiffResult(new GetJobUpdateDiffResult()
+            .setAdd(instancesToConfigGroups(add))
+            .setRemove(instancesToConfigGroups(remove))
+            .setUpdate(instancesToConfigGroups(update))
+            .setUnchanged(instancesToConfigGroups(diff.getUnchangedInstances()))));
+      }
+    });
+  }
+
+  private static Set<ConfigGroup> instancesToConfigGroups(Map<Integer, ITaskConfig> tasks) {
+    Multimap<ITaskConfig, Integer> instancesByDetails = Multimaps.invertFrom(
+        Multimaps.forMap(tasks),
+        HashMultimap.create());
+    return ImmutableSet.copyOf(
+        Iterables.transform(instancesByDetails.asMap().entrySet(), TO_GROUP));
+  }
+
   private List<ScheduledTask> getTasks(TaskQuery query) {
     requireNonNull(query);
 
@@ -418,4 +470,7 @@ class ReadOnlySchedulerImpl implements ReadOnlyScheduler.Iface {
   private Multimap<IJobKey, IScheduledTask> getTasks(Query.Builder query) {
     return Tasks.byJobKey(Storage.Util.fetchTasks(storage, query));
   }
+
+  @VisibleForTesting
+  static final String NO_CRON = "Cron jobs are not supported.";
 }

http://git-wip-us.apache.org/repos/asf/aurora/blob/888f9a3c/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 4efcd2c..304437e 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
@@ -479,6 +479,11 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
     return readOnlyScheduler.getJobs(maybeNullRole);
   }
 
+  @Override
+  public Response getJobUpdateDiff(JobUpdateRequest request) throws TException {
+    return readOnlyScheduler.getJobUpdateDiff(request);
+  }
+
   private void validateLockForTasks(java.util.Optional<ILock> lock, Iterable<IScheduledTask> tasks)
       throws LockException {
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/888f9a3c/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java b/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java
index a17337f..ca25388 100644
--- a/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java
+++ b/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java
@@ -46,7 +46,7 @@ import static java.util.Objects.requireNonNull;
 public final class JobDiff {
   private final Map<Integer, ITaskConfig> replacedInstances;
   private final Set<Integer> replacementInstances;
-  private final Set<Integer> unchangedInstances;
+  private final Map<Integer, ITaskConfig> unchangedInstances;
 
   /**
    * Creates a job diff containing the instances to be replaced (original state), and instances
@@ -55,11 +55,12 @@ public final class JobDiff {
    *
    * @param replacedInstances Instances being replaced.
    * @param replacementInstances Instances to replace {@code replacedInstances}.
+   * @param unchangedInstances Instances unchanged by the update.
    */
   public JobDiff(
       Map<Integer, ITaskConfig> replacedInstances,
       Set<Integer> replacementInstances,
-      Set<Integer> unchangedInstances) {
+      Map<Integer, ITaskConfig> unchangedInstances) {
 
     this.replacedInstances = requireNonNull(replacedInstances);
     this.replacementInstances = requireNonNull(replacementInstances);
@@ -74,7 +75,7 @@ public final class JobDiff {
     return replacementInstances;
   }
 
-  public Set<Integer> getUnchangedInstances() {
+  public Map<Integer, ITaskConfig> getUnchangedInstances() {
     return unchangedInstances;
   }
 
@@ -88,7 +89,7 @@ public final class JobDiff {
     Set<Integer> allValidInstances = ImmutableSet.<Integer>builder()
         .addAll(getReplacedInstances().keySet())
         .addAll(getReplacementInstances())
-        .addAll(getUnchangedInstances())
+        .addAll(getUnchangedInstances().keySet())
         .build();
     return ImmutableSet.copyOf(Sets.difference(scope, allValidInstances));
   }
@@ -160,7 +161,7 @@ public final class JobDiff {
     return new JobDiff(
         removedInstances,
         addedInstances,
-        ImmutableSet.copyOf(diff.entriesInCommon().keySet()));
+        ImmutableMap.copyOf(diff.entriesInCommon()));
   }
 
   /**
@@ -194,10 +195,14 @@ public final class JobDiff {
           ImmutableMap.copyOf(Maps.filterKeys(diff.getReplacedInstances(), Predicates.in(limit)));
       Set<Integer> replacements =
           ImmutableSet.copyOf(Sets.intersection(diff.getReplacementInstances(), limit));
-      Set<Integer> unchanged = ImmutableSet.copyOf(
+
+      Set<Integer> unchangedIds = ImmutableSet.copyOf(
           Sets.difference(
               ImmutableSet.copyOf(Sets.difference(currentState.keySet(), replaced.keySet())),
               replacements));
+      Map<Integer, ITaskConfig> unchanged =
+          ImmutableMap.copyOf(Maps.filterKeys(currentState, Predicates.in(unchangedIds)));
+
       return new JobDiff(replaced, replacements, unchanged);
     }
   }

http://git-wip-us.apache.org/repos/asf/aurora/blob/888f9a3c/src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java b/src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java
index adcc02b..76b0476 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java
@@ -117,6 +117,8 @@ final class Fixtures {
         .setRequestedPorts(ImmutableSet.of())
         .setTaskLinks(ImmutableMap.of())
         .setMaxTaskFailures(1)
+        .setConstraints(ImmutableSet.of())
+        .setMetadata(ImmutableSet.of())
         .setContainer(Container.mesos(new MesosContainer()));
   }
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/888f9a3c/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
index 82e7330..6efe03f 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
@@ -33,6 +33,7 @@ import org.apache.aurora.gen.AssignedTask;
 import org.apache.aurora.gen.ConfigGroup;
 import org.apache.aurora.gen.ConfigSummary;
 import org.apache.aurora.gen.ConfigSummaryResult;
+import org.apache.aurora.gen.GetJobUpdateDiffResult;
 import org.apache.aurora.gen.GetQuotaResult;
 import org.apache.aurora.gen.Identity;
 import org.apache.aurora.gen.JobConfiguration;
@@ -43,11 +44,15 @@ import org.apache.aurora.gen.JobUpdate;
 import org.apache.aurora.gen.JobUpdateDetails;
 import org.apache.aurora.gen.JobUpdateKey;
 import org.apache.aurora.gen.JobUpdateQuery;
+import org.apache.aurora.gen.JobUpdateRequest;
+import org.apache.aurora.gen.JobUpdateSettings;
 import org.apache.aurora.gen.JobUpdateSummary;
 import org.apache.aurora.gen.PendingReason;
 import org.apache.aurora.gen.PopulateJobResult;
+import org.apache.aurora.gen.Range;
 import org.apache.aurora.gen.ReadOnlyScheduler;
 import org.apache.aurora.gen.Response;
+import org.apache.aurora.gen.ResponseDetail;
 import org.apache.aurora.gen.Result;
 import org.apache.aurora.gen.RoleSummary;
 import org.apache.aurora.gen.RoleSummaryResult;
@@ -88,7 +93,9 @@ import static org.apache.aurora.scheduler.ResourceAggregates.MEDIUM;
 import static org.apache.aurora.scheduler.ResourceAggregates.SMALL;
 import static org.apache.aurora.scheduler.ResourceAggregates.XLARGE;
 import static org.apache.aurora.scheduler.base.Numbers.convertRanges;
+import static org.apache.aurora.scheduler.base.Numbers.rangesToInstanceIds;
 import static org.apache.aurora.scheduler.base.Numbers.toRanges;
+import static org.apache.aurora.scheduler.thrift.Fixtures.CRON_JOB;
 import static org.apache.aurora.scheduler.thrift.Fixtures.CRON_SCHEDULE;
 import static org.apache.aurora.scheduler.thrift.Fixtures.JOB_KEY;
 import static org.apache.aurora.scheduler.thrift.Fixtures.LOCK;
@@ -104,6 +111,7 @@ import static org.apache.aurora.scheduler.thrift.Fixtures.jobSummaryResponse;
 import static org.apache.aurora.scheduler.thrift.Fixtures.makeDefaultScheduledTasks;
 import static org.apache.aurora.scheduler.thrift.Fixtures.makeJob;
 import static org.apache.aurora.scheduler.thrift.Fixtures.nonProductionTask;
+import static org.apache.aurora.scheduler.thrift.ReadOnlySchedulerImpl.NO_CRON;
 import static org.easymock.EasyMock.expect;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
@@ -700,4 +708,137 @@ public class ReadOnlySchedulerImplTest extends EasyMockTest {
 
     assertResponse(INVALID_REQUEST, thrift.getJobUpdateDetails(UPDATE_KEY.newBuilder()));
   }
+
+  @Test
+  public void testGetJobUpdateDiffWithUpdateAdd() throws Exception {
+    TaskConfig task1 = defaultTask(false).setNumCpus(1.0);
+    TaskConfig task2 = defaultTask(false).setNumCpus(2.0);
+    TaskConfig task3 = defaultTask(false).setNumCpus(3.0);
+    TaskConfig task4 = defaultTask(false).setNumCpus(4.0);
+    TaskConfig task5 = defaultTask(false).setNumCpus(5.0);
+
+    ImmutableSet.Builder<IScheduledTask> tasks = ImmutableSet.builder();
+    makeTasks(0, 10, task1, tasks);
+    makeTasks(10, 20, task2, tasks);
+    makeTasks(20, 30, task3, tasks);
+    makeTasks(30, 40, task4, tasks);
+    makeTasks(40, 50, task5, tasks);
+
+    expect(storageUtil.jobStore.fetchJob(JOB_KEY)).andReturn(Optional.absent());
+    storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active(), tasks.build());
+
+    control.replay();
+
+    TaskConfig newTask = defaultTask(false).setNumCpus(6.0);
+    JobUpdateRequest request = new JobUpdateRequest()
+        .setTaskConfig(newTask)
+        .setInstanceCount(60)
+        .setSettings(new JobUpdateSettings()
+            .setUpdateOnlyTheseInstances(ImmutableSet.of(new Range(10, 59))));
+
+    GetJobUpdateDiffResult expected = new GetJobUpdateDiffResult()
+        .setAdd(ImmutableSet.of(group(newTask, new Range(50, 59))))
+        .setUpdate(ImmutableSet.of(
+            group(task2, new Range(10, 19)),
+            group(task3, new Range(20, 29)),
+            group(task4, new Range(30, 39)),
+            group(task5, new Range(40, 49))))
+        .setUnchanged(ImmutableSet.of(group(task1, new Range(0, 9))))
+        .setRemove(ImmutableSet.of());
+
+    Response response = assertOkResponse(thrift.getJobUpdateDiff(request));
+    assertEquals(expected, response.getResult().getGetJobUpdateDiffResult());
+  }
+
+  @Test
+  public void testGetJobUpdateDiffWithUpdateRemove() throws Exception {
+    TaskConfig task1 = defaultTask(false).setNumCpus(1.0);
+    TaskConfig task2 = defaultTask(false).setNumCpus(2.0);
+    TaskConfig task3 = defaultTask(false).setNumCpus(3.0);
+
+    ImmutableSet.Builder<IScheduledTask> tasks = ImmutableSet.builder();
+    makeTasks(0, 10, task1, tasks);
+    makeTasks(10, 20, task2, tasks);
+    makeTasks(20, 30, task3, tasks);
+
+    expect(storageUtil.jobStore.fetchJob(JOB_KEY)).andReturn(Optional.absent());
+    storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active(), tasks.build());
+
+    control.replay();
+
+    JobUpdateRequest request = new JobUpdateRequest()
+        .setTaskConfig(defaultTask(false).setNumCpus(6.0))
+        .setInstanceCount(20)
+        .setSettings(new JobUpdateSettings());
+
+    GetJobUpdateDiffResult expected = new GetJobUpdateDiffResult()
+        .setRemove(ImmutableSet.of(group(task3, new Range(20, 29))))
+        .setUpdate(ImmutableSet.of(
+            group(task1, new Range(0, 9)),
+            group(task2, new Range(10, 19))))
+        .setAdd(ImmutableSet.of())
+        .setUnchanged(ImmutableSet.of());
+
+    Response response = assertOkResponse(thrift.getJobUpdateDiff(request));
+    assertEquals(expected, response.getResult().getGetJobUpdateDiffResult());
+  }
+
+  @Test
+  public void testGetJobUpdateDiffWithUnchanged() throws Exception {
+    expect(storageUtil.jobStore.fetchJob(JOB_KEY)).andReturn(Optional.absent());
+    storageUtil.expectTaskFetch(
+        Query.jobScoped(JOB_KEY).active(),
+        ImmutableSet.copyOf(makeDefaultScheduledTasks(10)));
+
+    control.replay();
+
+    JobUpdateRequest request = new JobUpdateRequest()
+        .setTaskConfig(defaultTask(true))
+        .setInstanceCount(10)
+        .setSettings(new JobUpdateSettings());
+
+    GetJobUpdateDiffResult expected = new GetJobUpdateDiffResult()
+        .setUnchanged(ImmutableSet.of(group(defaultTask(true), new Range(0, 9))))
+        .setRemove(ImmutableSet.of())
+        .setUpdate(ImmutableSet.of())
+        .setAdd(ImmutableSet.of());
+
+    Response response = assertOkResponse(thrift.getJobUpdateDiff(request));
+    assertEquals(expected, response.getResult().getGetJobUpdateDiffResult());
+  }
+
+  @Test
+  public void testGetJobUpdateDiffNoCron() throws Exception {
+    expect(storageUtil.jobStore.fetchJob(JOB_KEY))
+        .andReturn(Optional.of(IJobConfiguration.build(CRON_JOB)));
+
+    control.replay();
+
+    JobUpdateRequest request = new JobUpdateRequest().setTaskConfig(defaultTask(false));
+
+    Response expected = Responses.empty()
+        .setResponseCode(INVALID_REQUEST)
+        .setDetails(ImmutableList.of(new ResponseDetail(NO_CRON)));
+
+    assertEquals(expected, thrift.getJobUpdateDiff(request));
+  }
+
+  private static void makeTasks(
+      int start,
+      int end,
+      TaskConfig config,
+      ImmutableSet.Builder<IScheduledTask> builder) {
+
+    for (int i = start; i < end; i++) {
+      builder.add(IScheduledTask.build(new ScheduledTask()
+          .setAssignedTask(new AssignedTask().setTask(config).setInstanceId(i))));
+    }
+  }
+
+  private static ConfigGroup group(TaskConfig task, Range range) {
+    return new ConfigGroup()
+        .setConfig(task)
+        .setInstanceIds(rangesToInstanceIds(ImmutableSet.of(IRange.build(range))))
+        .setInstances(ImmutableSet.of(range));
+  }
 }

http://git-wip-us.apache.org/repos/asf/aurora/blob/888f9a3c/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 83f65a7..f537f7a 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
@@ -588,7 +588,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     control.replay();
 
     TaskConfig task = nonProductionTask();
-    task.addToConstraints(dedicatedConstraint(ImmutableSet.of("mesos")));
+    task.setConstraints(ImmutableSet.of(dedicatedConstraint(ImmutableSet.of("mesos"))));
     assertResponse(INVALID_REQUEST, thrift.createJob(makeJob(task), null, SESSION));
   }
 
@@ -599,7 +599,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     control.replay();
 
     TaskConfig task = nonProductionTask();
-    task.addToConstraints(dedicatedConstraint(1));
+    task.setConstraints(ImmutableSet.of(dedicatedConstraint(1)));
     assertResponse(INVALID_REQUEST, thrift.createJob(makeJob(task), null, SESSION));
   }
 
@@ -610,7 +610,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     control.replay();
 
     TaskConfig task = nonProductionTask();
-    task.addToConstraints(dedicatedConstraint(ImmutableSet.of("mesos", "test")));
+    task.setConstraints(ImmutableSet.of(dedicatedConstraint(ImmutableSet.of("mesos", "test"))));
     assertResponse(INVALID_REQUEST, thrift.createJob(makeJob(task), null, SESSION));
   }
 
@@ -1456,7 +1456,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     control.replay();
 
     TaskConfig task = nonProductionTask();
-    task.addToConstraints(dedicatedConstraint(ImmutableSet.of("mesos")));
+    task.setConstraints(ImmutableSet.of(dedicatedConstraint(ImmutableSet.of("mesos"))));
     assertResponse(INVALID_REQUEST, thrift.createJob(makeJob(task), null, SESSION));
   }
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/888f9a3c/src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java b/src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
index d25618c..a6769e6 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
@@ -301,4 +301,9 @@ abstract class ForwardingThrift implements AnnotatedAuroraAdmin {
   public Response getJobUpdateDetails(JobUpdateKey key) throws TException {
     return delegate.getJobUpdateDetails(key);
   }
+
+  @Override
+  public Response getJobUpdateDiff(JobUpdateRequest request) throws TException {
+    return delegate.getJobUpdateDiff(request);
+  }
 }

http://git-wip-us.apache.org/repos/asf/aurora/blob/888f9a3c/src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java b/src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java
index 64d4581..67fe14b 100644
--- a/src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java
@@ -44,8 +44,6 @@ import static org.junit.Assert.assertNotEquals;
 public class JobDiffTest extends EasyMockTest {
 
   private static final IJobKey JOB = JobKeys.from("role", "env", "job");
-  private static final JobDiff NO_DIFF =
-      new JobDiff(ImmutableMap.of(), ImmutableSet.of(), ImmutableSet.of());
   private static final Set<IRange> NO_SCOPE = ImmutableSet.of();
   private static final Set<IRange> CANARY_SCOPE = ImmutableSet.of(IRange.build(new Range(0, 0)));
 
@@ -65,10 +63,10 @@ public class JobDiffTest extends EasyMockTest {
     control.replay();
 
     assertEquals(
-        new JobDiff(ImmutableMap.of(), ImmutableSet.of(), ImmutableSet.of(0, 1)),
+        new JobDiff(ImmutableMap.of(), ImmutableSet.of(), ImmutableMap.of(0, task, 1, task)),
         JobDiff.compute(store, JOB, JobDiff.asMap(task, 2), NO_SCOPE));
     assertEquals(
-        new JobDiff(ImmutableMap.of(), ImmutableSet.of(), ImmutableSet.of(0, 1)),
+        new JobDiff(ImmutableMap.of(), ImmutableSet.of(), ImmutableMap.of(0, task, 1, task)),
         JobDiff.compute(store, JOB, JobDiff.asMap(task, 2), CANARY_SCOPE));
   }
 
@@ -81,10 +79,10 @@ public class JobDiffTest extends EasyMockTest {
     control.replay();
 
     assertEquals(
-        new JobDiff(ImmutableMap.of(), ImmutableSet.of(2, 3, 4), ImmutableSet.of(0, 1)),
+        new JobDiff(ImmutableMap.of(), ImmutableSet.of(2, 3, 4), ImmutableMap.of(0, task, 1, task)),
         JobDiff.compute(store, JOB, JobDiff.asMap(task, 5), NO_SCOPE));
     assertEquals(
-        new JobDiff(ImmutableMap.of(), ImmutableSet.of(), ImmutableSet.of(0, 1)),
+        new JobDiff(ImmutableMap.of(), ImmutableSet.of(), ImmutableMap.of(0, task, 1, task)),
         JobDiff.compute(store, JOB, JobDiff.asMap(task, 5), CANARY_SCOPE));
   }
 
@@ -97,10 +95,13 @@ public class JobDiffTest extends EasyMockTest {
     control.replay();
 
     assertEquals(
-        new JobDiff(ImmutableMap.of(1, task, 2, task), ImmutableSet.of(), ImmutableSet.of(0)),
+        new JobDiff(ImmutableMap.of(1, task, 2, task), ImmutableSet.of(), ImmutableMap.of(0, task)),
         JobDiff.compute(store, JOB, JobDiff.asMap(task, 1), NO_SCOPE));
     assertEquals(
-        new JobDiff(ImmutableMap.of(), ImmutableSet.of(), ImmutableSet.of(0, 1, 2)),
+        new JobDiff(
+            ImmutableMap.of(),
+            ImmutableSet.of(),
+            ImmutableMap.of(0, task, 1, task, 2, task)),
         JobDiff.compute(store, JOB, JobDiff.asMap(task, 1), CANARY_SCOPE));
   }
 
@@ -117,10 +118,13 @@ public class JobDiffTest extends EasyMockTest {
         new JobDiff(
             ImmutableMap.of(0, oldTask, 1, oldTask, 2, oldTask),
             ImmutableSet.of(0, 1, 2),
-            ImmutableSet.of()),
+            ImmutableMap.of()),
         JobDiff.compute(store, JOB, JobDiff.asMap(newTask, 3), NO_SCOPE));
     assertEquals(
-        new JobDiff(ImmutableMap.of(0, oldTask), ImmutableSet.of(0), ImmutableSet.of(1, 2)),
+        new JobDiff(
+            ImmutableMap.of(0, oldTask),
+            ImmutableSet.of(0),
+            ImmutableMap.of(1, oldTask, 2, oldTask)),
         JobDiff.compute(store, JOB, JobDiff.asMap(newTask, 3), CANARY_SCOPE));
   }
 
@@ -140,10 +144,13 @@ public class JobDiffTest extends EasyMockTest {
         new JobDiff(
             ImmutableMap.of(0, oldTask, 3, oldTask2, 4, oldTask),
             ImmutableSet.of(0, 2, 3, 4),
-            ImmutableSet.of(1)),
+            ImmutableMap.of(1, newTask)),
         JobDiff.compute(store, JOB, JobDiff.asMap(newTask, 5), NO_SCOPE));
     assertEquals(
-        new JobDiff(ImmutableMap.of(0, oldTask), ImmutableSet.of(0), ImmutableSet.of(1, 3, 4)),
+        new JobDiff(
+            ImmutableMap.of(0, oldTask),
+            ImmutableSet.of(0),
+            ImmutableMap.of(1, newTask, 3, oldTask2, 4, oldTask)),
         JobDiff.compute(store, JOB, JobDiff.asMap(newTask, 5), CANARY_SCOPE));
   }
 
@@ -160,13 +167,19 @@ public class JobDiffTest extends EasyMockTest {
         new JobDiff(
             ImmutableMap.of(0, oldTask, 2, oldTask),
             ImmutableSet.of(0, 2),
-            ImmutableSet.of(1)),
+            ImmutableMap.of(1, newTask)),
         JobDiff.compute(store, JOB, JobDiff.asMap(newTask, 3), NO_SCOPE));
     assertEquals(
-        new JobDiff(ImmutableMap.of(0, oldTask), ImmutableSet.of(0), ImmutableSet.of(1, 2)),
+        new JobDiff(
+            ImmutableMap.of(0, oldTask),
+            ImmutableSet.of(0),
+            ImmutableMap.of(1, newTask, 2, oldTask)),
         JobDiff.compute(store, JOB, JobDiff.asMap(newTask, 3), CANARY_SCOPE));
     assertEquals(
-        new JobDiff(ImmutableMap.of(0, oldTask), ImmutableSet.of(0), ImmutableSet.of(1, 2)),
+        new JobDiff(
+            ImmutableMap.of(0, oldTask),
+            ImmutableSet.of(0),
+            ImmutableMap.of(1, newTask, 2, oldTask)),
         JobDiff.compute(store, JOB, JobDiff.asMap(newTask, 4), CANARY_SCOPE));
   }
 
@@ -177,19 +190,19 @@ public class JobDiffTest extends EasyMockTest {
     JobDiff a = new JobDiff(
         ImmutableMap.of(0, makeTask("job", "echo")),
         ImmutableSet.of(0),
-        ImmutableSet.of());
+        ImmutableMap.of());
     JobDiff b = new JobDiff(
         ImmutableMap.of(0, makeTask("job", "echo")),
         ImmutableSet.of(0),
-        ImmutableSet.of());
+        ImmutableMap.of());
     JobDiff c = new JobDiff(
         ImmutableMap.of(0, makeTask("job", "echo1")),
         ImmutableSet.of(0),
-        ImmutableSet.of());
+        ImmutableMap.of());
     JobDiff d = new JobDiff(
         ImmutableMap.of(0, makeTask("job", "echo")),
         ImmutableSet.of(1),
-        ImmutableSet.of());
+        ImmutableMap.of());
     assertEquals(a, b);
     assertEquals(ImmutableSet.of(a), ImmutableSet.of(a, b));
     assertNotEquals(a, c);