aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject incubator-aurora git commit: Add more test coverage to SchedulerThriftInterface.
Date Wed, 19 Nov 2014 23:22:32 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master 2aa0026fd -> 4e52d00b8


Add more test coverage to SchedulerThriftInterface.

Bugs closed: AURORA-937

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


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

Branch: refs/heads/master
Commit: 4e52d00b881a346b50dae54b2b3f71f9d3f08d8b
Parents: 2aa0026
Author: Bill Farner <wfarner@apache.org>
Authored: Wed Nov 19 15:13:28 2014 -0800
Committer: Bill Farner <wfarner@apache.org>
Committed: Wed Nov 19 15:13:28 2014 -0800

----------------------------------------------------------------------
 .../apache/aurora/auth/SessionValidator.java    |   4 -
 .../scheduler/storage/backup/Recovery.java      |   6 +-
 .../thrift/SchedulerThriftInterface.java        |  77 ++----
 .../apache/aurora/scheduler/thrift/Util.java    |  11 +
 .../aurora/scheduler/thrift/aop/AopModule.java  |   7 +-
 .../thrift/SchedulerThriftInterfaceTest.java    | 277 +++++++++++++++++--
 6 files changed, 300 insertions(+), 82 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4e52d00b/src/main/java/org/apache/aurora/auth/SessionValidator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/auth/SessionValidator.java b/src/main/java/org/apache/aurora/auth/SessionValidator.java
index eeebb78..b688a0f 100644
--- a/src/main/java/org/apache/aurora/auth/SessionValidator.java
+++ b/src/main/java/org/apache/aurora/auth/SessionValidator.java
@@ -64,9 +64,5 @@ public interface SessionValidator {
     public AuthFailedException(String msg) {
       super(msg);
     }
-
-    public AuthFailedException(String msg, Throwable cause) {
-      super(msg, cause);
-    }
   }
 }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4e52d00b/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java b/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
index 4744dc9..38764e5 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java
@@ -93,12 +93,12 @@ public interface Recovery {
    * Thrown when a recovery operation could not be completed due to internal errors or improper
    * invocation order.
    */
-  class RecoveryException extends Exception {
-    RecoveryException(String message) {
+  class RecoveryException extends RuntimeException {
+    public RecoveryException(String message) {
       super(message);
     }
 
-    RecoveryException(String message, Throwable cause) {
+    public RecoveryException(String message, Throwable cause) {
       super(message, cause);
     }
   }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4e52d00b/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 4b65a2c..b66b916 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
@@ -20,7 +20,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import javax.annotation.Nullable;
@@ -35,7 +34,6 @@ import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.base.Strings;
-import com.google.common.base.Throwables;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableMultimap;
@@ -142,7 +140,6 @@ import org.apache.aurora.scheduler.storage.Storage.NonVolatileStorage;
 import org.apache.aurora.scheduler.storage.Storage.StoreProvider;
 import org.apache.aurora.scheduler.storage.Storage.Work;
 import org.apache.aurora.scheduler.storage.backup.Recovery;
-import org.apache.aurora.scheduler.storage.backup.Recovery.RecoveryException;
 import org.apache.aurora.scheduler.storage.backup.StorageBackup;
 import org.apache.aurora.scheduler.storage.entities.IAssignedTask;
 import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
@@ -176,7 +173,6 @@ import static com.twitter.common.base.MorePreconditions.checkNotBlank;
 
 import static org.apache.aurora.auth.SessionValidator.SessionContext;
 import static org.apache.aurora.gen.ResponseCode.AUTH_FAILED;
-import static org.apache.aurora.gen.ResponseCode.ERROR;
 import static org.apache.aurora.gen.ResponseCode.INVALID_REQUEST;
 import static org.apache.aurora.gen.ResponseCode.LOCK_ERROR;
 import static org.apache.aurora.gen.ResponseCode.OK;
@@ -987,45 +983,26 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
 
   @Override
   public Response stageRecovery(String backupId, SessionKey session) {
-    try {
-      recovery.stage(backupId);
-      return okEmptyResponse();
-    } catch (RecoveryException e) {
-      LOG.log(Level.WARNING, "Failed to stage recovery: " + e, e);
-      return errorResponse(ERROR, e);
-    }
+    recovery.stage(backupId);
+    return okEmptyResponse();
   }
 
   @Override
   public Response queryRecovery(TaskQuery query, SessionKey session) {
-    try {
-      return okResponse(Result.queryRecoveryResult(new QueryRecoveryResult()
-              .setTasks(IScheduledTask.toBuildersSet(recovery.query(Query.arbitrary(query))))));
-    } catch (RecoveryException e) {
-      LOG.log(Level.WARNING, "Failed to query recovery: " + e, e);
-      return errorResponse(ERROR, e);
-    }
+    return okResponse(Result.queryRecoveryResult(new QueryRecoveryResult()
+            .setTasks(IScheduledTask.toBuildersSet(recovery.query(Query.arbitrary(query))))));
   }
 
   @Override
   public Response deleteRecoveryTasks(TaskQuery query, SessionKey session) {
-    try {
-      recovery.deleteTasks(Query.arbitrary(query));
-      return okEmptyResponse();
-    } catch (RecoveryException e) {
-      LOG.log(Level.WARNING, "Failed to delete recovery tasks: " + e, e);
-      return errorResponse(ERROR, e);
-    }
+    recovery.deleteTasks(Query.arbitrary(query));
+    return okEmptyResponse();
   }
 
   @Override
   public Response commitRecovery(SessionKey session) {
-    try {
-      recovery.commit();
-      return okEmptyResponse();
-    } catch (RecoveryException e) {
-      return errorResponse(ERROR, e);
-    }
+    recovery.commit();
+    return okEmptyResponse();
   }
 
   @Override
@@ -1036,34 +1013,25 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
 
   @Override
   public Response snapshot(SessionKey session) {
-    try {
-      storage.snapshot();
-      return okEmptyResponse();
-    } catch (Storage.StorageException e) {
-      LOG.log(Level.WARNING, "Requested snapshot failed.", e);
-      return errorResponse(ERROR, e);
-    }
+    storage.snapshot();
+    return okEmptyResponse();
   }
 
   private static Multimap<String, IJobConfiguration> jobsByKey(JobStore jobStore, IJobKey
jobKey) {
     ImmutableMultimap.Builder<String, IJobConfiguration> matches = ImmutableMultimap.builder();
     for (String managerId : jobStore.fetchManagerIds()) {
-      for (IJobConfiguration job : jobStore.fetchJobs(managerId)) {
-        if (job.getKey().equals(jobKey)) {
-          matches.put(managerId, job);
-        }
+      Optional<IJobConfiguration> job = jobStore.fetchJob(managerId, jobKey);
+      if (job.isPresent()) {
+        matches.put(managerId, job.get());
       }
     }
     return matches.build();
   }
 
   @Override
-  public Response rewriteConfigs(
-      final RewriteConfigsRequest request,
-      SessionKey session) {
-
+  public Response rewriteConfigs(final RewriteConfigsRequest request, SessionKey session)
{
     if (request.getRewriteCommandsSize() == 0) {
-      return addMessage(Util.emptyResponse(), ERROR, "No rewrite commands provided.");
+      return addMessage(Util.emptyResponse(), INVALID_REQUEST, "No rewrite commands provided.");
     }
 
     return storage.write(new MutateWork.Quiet<Response>() {
@@ -1101,7 +1069,7 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
     } catch (TaskDescriptionException e) {
       // We could add an error here, but this is probably a hint of something wrong in
       // the client that's causing a bad configuration to be applied.
-      throw Throwables.propagate(e);
+      throw new RuntimeException(e);
     }
 
     if (existingJob.getKey().equals(rewrittenJob.getKey())) {
@@ -1454,10 +1422,6 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
             JobDiff.asMap(request.getTaskConfig(), request.getInstanceCount()),
             settings.getUpdateOnlyTheseInstances());
 
-        if (diff.isNoop()) {
-          return addMessage(emptyResponse(), OK, NOOP_JOB_UPDATE_MESSAGE);
-        }
-
         Set<Integer> invalidScope = diff.getOutOfScopeInstances(
             Numbers.rangesToInstanceIds(settings.getUpdateOnlyTheseInstances()));
         if (!invalidScope.isEmpty()) {
@@ -1466,6 +1430,10 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
                   + invalidScope);
         }
 
+        if (diff.isNoop()) {
+          return addMessage(emptyResponse(), OK, NOOP_JOB_UPDATE_MESSAGE);
+        }
+
         JobUpdateInstructions instructions = new JobUpdateInstructions()
             .setSettings(settings.newBuilder())
             .setInitialState(buildInitialState(diff.getReplacedInstances()));
@@ -1557,9 +1525,12 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface {
     });
   }
 
+  @VisibleForTesting
+  static final String NOT_IMPLEMENTED_MESSAGE = "Not implemented";
+
   @Override
   public Response pulseJobUpdate(String updateId, SessionKey session) {
-    throw new UnsupportedOperationException("Not implemented");
+    throw new UnsupportedOperationException(NOT_IMPLEMENTED_MESSAGE);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4e52d00b/src/main/java/org/apache/aurora/scheduler/thrift/Util.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/Util.java b/src/main/java/org/apache/aurora/scheduler/thrift/Util.java
index 18e2bdf..d879db4 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/Util.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/Util.java
@@ -21,6 +21,8 @@ import org.apache.aurora.gen.Response;
 import org.apache.aurora.gen.ResponseCode;
 import org.apache.aurora.gen.ResponseDetail;
 
+import static org.apache.aurora.gen.ResponseCode.OK;
+
 /**
  * Utility class for constructing responses to API calls.
  */
@@ -85,4 +87,13 @@ public final class Util {
     response.addToDetails(new ResponseDetail(message));
     return response;
   }
+
+  /**
+   * Creates an OK response that has no result entity.
+   *
+   * @return Ok response with an empty result.
+   */
+  public static Response okEmptyResponse()  {
+    return emptyResponse().setResponseCode(OK);
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4e52d00b/src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java b/src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java
index dca855c..83b8f39 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java
@@ -86,9 +86,13 @@ public class AopModule extends AbstractModule {
     requireBinding(CapabilityValidator.class);
 
     // Layer ordering:
-    // Log -> CapabilityValidator -> FeatureToggle -> StatsExporter -> APIVersion
->
+    // APIVersion -> Log -> CapabilityValidator -> FeatureToggle -> StatsExporter
->
     // SchedulerThriftInterface
 
+    // It's important for this interceptor to be registered first to ensure it's at the 'top'
of
+    // the stack and the standard message is always applied.
+    bindThriftDecorator(new ServerInfoInterceptor());
+
     // TODO(Sathya): Consider using provider pattern for constructing interceptors to facilitate
     // unit testing without the creation of Guice injectors.
     bindThriftDecorator(new LoggingInterceptor());
@@ -126,7 +130,6 @@ public class AopModule extends AbstractModule {
     });
     bindThriftDecorator(new FeatureToggleInterceptor());
     bindThriftDecorator(new ThriftStatsExporterInterceptor());
-    bindThriftDecorator(new ServerInfoInterceptor());
   }
 
   private void bindThriftDecorator(MethodInterceptor interceptor) {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4e52d00b/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 6089032..168290a 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
@@ -16,6 +16,7 @@ package org.apache.aurora.scheduler.thrift;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
+import java.util.Arrays;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
@@ -25,9 +26,11 @@ import java.util.UUID;
 import javax.annotation.Nullable;
 
 import com.google.common.base.Function;
+import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
+import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -74,9 +77,11 @@ import org.apache.aurora.gen.JobUpdateRequest;
 import org.apache.aurora.gen.JobUpdateSettings;
 import org.apache.aurora.gen.JobUpdateSummary;
 import org.apache.aurora.gen.LimitConstraint;
+import org.apache.aurora.gen.ListBackupsResult;
 import org.apache.aurora.gen.Lock;
 import org.apache.aurora.gen.LockKey;
 import org.apache.aurora.gen.PendingReason;
+import org.apache.aurora.gen.QueryRecoveryResult;
 import org.apache.aurora.gen.Range;
 import org.apache.aurora.gen.ResourceAggregate;
 import org.apache.aurora.gen.Response;
@@ -147,6 +152,7 @@ import static org.apache.aurora.auth.CapabilityValidator.Capability.PROVISIONER;
 import static org.apache.aurora.auth.CapabilityValidator.Capability.ROOT;
 import static org.apache.aurora.auth.SessionValidator.SessionContext;
 import static org.apache.aurora.gen.LockValidation.CHECKED;
+import static org.apache.aurora.gen.LockValidation.UNCHECKED;
 import static org.apache.aurora.gen.MaintenanceMode.DRAINING;
 import static org.apache.aurora.gen.MaintenanceMode.NONE;
 import static org.apache.aurora.gen.MaintenanceMode.SCHEDULED;
@@ -161,6 +167,7 @@ import static org.apache.aurora.gen.apiConstants.THRIFT_API_VERSION;
 import static org.apache.aurora.scheduler.configuration.ConfigurationManager.DEDICATED_ATTRIBUTE;
 import static org.apache.aurora.scheduler.quota.QuotaCheckResult.Result.INSUFFICIENT_QUOTA;
 import static org.apache.aurora.scheduler.quota.QuotaCheckResult.Result.SUFFICIENT_QUOTA;
+import static org.apache.aurora.scheduler.storage.backup.Recovery.RecoveryException;
 import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.MAX_TASKS_PER_JOB;
 import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.MAX_TASK_ID_LENGTH;
 import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.NOOP_JOB_UPDATE_MESSAGE;
@@ -575,9 +582,8 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   }
 
   @Test
-  public void testCreateHomogeneousJobNoShards() throws Exception {
+  public void testCreateHomogeneousJobNoInstances() throws Exception {
     JobConfiguration job = makeJob();
-    job.setInstanceCount(0);
     job.unsetInstanceCount();
     expectAuth(ROLE, true);
 
@@ -587,6 +593,17 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   }
 
   @Test
+  public void testCreateJobNegativeInstanceCount() throws Exception {
+    JobConfiguration job = makeJob();
+    job.setInstanceCount(0 - 1);
+    expectAuth(ROLE, true);
+
+    control.replay();
+
+    assertResponse(INVALID_REQUEST, thrift.createJob(job, DEFAULT_LOCK, SESSION));
+  }
+
+  @Test
   public void testCreateJobNoResources() throws Exception {
     expectAuth(ROLE, true);
 
@@ -772,6 +789,19 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   }
 
   @Test
+  public void testKillByJobName() throws Exception {
+    TaskQuery query = new TaskQuery().setJobName("job");
+    expectAuth(ROOT, true);
+    storageUtil.expectTaskFetch(Query.arbitrary(query).active(), buildScheduledTask());
+    lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
+    expectTransitionsToKilling();
+
+    control.replay();
+
+    assertEquals(okEmptyResponse(), thrift.killTasks(query, DEFAULT_LOCK, SESSION));
+  }
+
+  @Test
   public void testKillQueryActive() throws Exception {
     Query.Builder query = Query.unscoped().byJob(JOB_KEY);
     expectAuth(ROOT, true);
@@ -1002,9 +1032,83 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     // Note: This will change after AOP-style session validation passes in a SessionContext.
     expectAuth(ROOT, true).times(2);
 
+    // Auth for the second call.  AOP auth fails, but second auth attempt required to get
+    // SessionContext fails.  This is pretty contrived, but not technically impossible.
+    expectAuth(ROOT, true);
+    expectAuth(ROOT, false);
+
     control.replay();
 
     assertOkResponse(thrift.forceTaskState(TASK_ID, status, SESSION));
+    assertEquals(
+        response(AUTH_FAILED, Optional.<Result>absent(), AUTH_DENIED_MESSAGE),
+        thrift.forceTaskState(TASK_ID, status, SESSION));
+  }
+
+  @Test
+  public void testBackupControls() throws Exception {
+    expectAuth(ROOT, true);
+    backup.backupNow();
+
+    expectAuth(ROOT, true);
+    Set<String> backups = ImmutableSet.of("a", "b");
+    expect(recovery.listBackups()).andReturn(backups);
+
+    expectAuth(ROOT, true);
+    String backupId = "backup";
+    recovery.stage(backupId);
+
+    expectAuth(ROOT, true);
+    Query.Builder query = Query.taskScoped("taskId");
+    Set<IScheduledTask> queryResult = ImmutableSet.of(
+        IScheduledTask.build(new ScheduledTask().setStatus(ScheduleStatus.RUNNING)));
+    expect(recovery.query(query)).andReturn(queryResult);
+
+    expectAuth(ROOT, true);
+    recovery.deleteTasks(query);
+
+    expectAuth(ROOT, true);
+    recovery.commit();
+
+    expectAuth(ROOT, true);
+    recovery.unload();
+
+    control.replay();
+
+    assertEquals(okEmptyResponse(), thrift.performBackup(SESSION));
+
+    assertEquals(
+        okResponse(Result.listBackupsResult(new ListBackupsResult().setBackups(backups))),
+        thrift.listBackups(SESSION));
+
+    assertEquals(okEmptyResponse(), thrift.stageRecovery(backupId, SESSION));
+
+    assertEquals(
+        okResponse(Result.queryRecoveryResult(
+            new QueryRecoveryResult().setTasks(IScheduledTask.toBuildersSet(queryResult)))),
+        thrift.queryRecovery(query.get(), SESSION));
+
+    assertEquals(okEmptyResponse(), thrift.deleteRecoveryTasks(query.get(), SESSION));
+
+    assertEquals(okEmptyResponse(), thrift.commitRecovery(SESSION));
+
+    assertEquals(okEmptyResponse(), thrift.unloadRecovery(SESSION));
+  }
+
+  @Test
+  public void testRecoveryException() throws Exception {
+    Throwable recoveryException = new RecoveryException("Injected");
+
+    expectAuth(ROOT, true);
+    String backupId = "backup";
+    recovery.stage(backupId);
+    expectLastCall().andThrow(recoveryException);
+
+    control.replay();
+
+    assertEquals(
+        errorResponse(recoveryException.getMessage()),
+        thrift.stageRecovery(backupId, SESSION));
   }
 
   @Test
@@ -1287,6 +1391,49 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   }
 
   @Test
+  public void testRewriteNoCommands() throws Exception {
+    expectAuth(ROOT, true);
+
+    control.replay();
+
+    RewriteConfigsRequest request = new RewriteConfigsRequest(ImmutableList.<ConfigRewrite>of());
+    assertResponse(INVALID_REQUEST, thrift.rewriteConfigs(request, SESSION));
+  }
+
+  @Test
+  public void testRewriteInvalidJob() throws Exception {
+    expectAuth(ROOT, true);
+
+    control.replay();
+
+    IJobConfiguration job = IJobConfiguration.build(makeJob());
+    RewriteConfigsRequest request = new RewriteConfigsRequest(
+        ImmutableList.of(ConfigRewrite.jobRewrite(
+            new JobConfigRewrite(job.newBuilder(), job.newBuilder().setTaskConfig(null)))));
+    assertResponse(ERROR, thrift.rewriteConfigs(request, SESSION));
+  }
+
+  @Test
+  public void testRewriteChangeJobKey() throws Exception {
+    expectAuth(ROOT, true);
+
+    control.replay();
+
+    IJobConfiguration job = IJobConfiguration.build(makeJob());
+    JobKey rewrittenJobKey = JobKeys.from("a", "b", "c").newBuilder();
+    Identity rewrittenIdentity = new Identity(rewrittenJobKey.getRole(), "steve");
+    RewriteConfigsRequest request = new RewriteConfigsRequest(
+        ImmutableList.of(ConfigRewrite.jobRewrite(new JobConfigRewrite(
+            job.newBuilder(),
+            job.newBuilder()
+                .setTaskConfig(job.getTaskConfig().newBuilder().setJob(rewrittenJobKey)
+                    .setOwner(rewrittenIdentity))
+                .setOwner(rewrittenIdentity)
+                .setKey(rewrittenJobKey)))));
+    assertResponse(WARNING, thrift.rewriteConfigs(request, SESSION));
+  }
+
+  @Test
   public void testRewriteShardCasMismatch() throws Exception {
     TaskConfig storedConfig = productionTask();
     TaskConfig modifiedConfig =
@@ -1313,7 +1460,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   }
 
   @Test
-  public void testRewriteShard() throws Exception {
+  public void testRewriteInstance() throws Exception {
     TaskConfig storedConfig = productionTask();
     ITaskConfig modifiedConfig = ITaskConfig.build(
         storedConfig.deepCopy().setExecutorConfig(new ExecutorConfig("aurora", "rewritten")));
@@ -1346,6 +1493,37 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   }
 
   @Test
+  public void testRewriteInstanceUnchanged() throws Exception {
+    TaskConfig config = productionTask();
+    String taskId = "task_id";
+    IScheduledTask task = IScheduledTask.build(new ScheduledTask().setAssignedTask(
+        new AssignedTask()
+            .setTaskId(taskId)
+            .setTask(config)));
+    InstanceKey instanceKey = new InstanceKey(
+        JobKeys.from(
+            config.getOwner().getRole(),
+            config.getEnvironment(),
+            config.getJobName()).newBuilder(),
+        0);
+
+    expectAuth(ROOT, true);
+    storageUtil.expectTaskFetch(
+        Query.instanceScoped(IInstanceKey.build(instanceKey)).active(), task);
+    expect(storageUtil.taskStore.unsafeModifyInPlace(
+        taskId,
+        ITaskConfig.build(ConfigurationManager.applyDefaultsIfUnset(config.deepCopy()))))
+        .andReturn(false);
+
+    control.replay();
+
+    RewriteConfigsRequest request = new RewriteConfigsRequest(
+        ImmutableList.of(ConfigRewrite.instanceRewrite(
+            new InstanceConfigRewrite(instanceKey, config, config))));
+    assertResponse(WARNING, thrift.rewriteConfigs(request, SESSION));
+  }
+
+  @Test
   public void testRewriteJobCasMismatch() throws Exception {
     JobConfiguration oldJob = makeJob(productionTask());
     JobConfiguration newJob = oldJob.deepCopy();
@@ -1353,8 +1531,8 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     String manager = "manager_key";
     expectAuth(ROOT, true);
     expect(storageUtil.jobStore.fetchManagerIds()).andReturn(ImmutableSet.of(manager));
-    expect(storageUtil.jobStore.fetchJobs(manager))
-        .andReturn(ImmutableList.of(IJobConfiguration.build(oldJob)));
+    expect(storageUtil.jobStore.fetchJob(manager, IJobKey.build(oldJob.getKey())))
+        .andReturn(Optional.of(IJobConfiguration.build(oldJob)));
 
     control.replay();
 
@@ -1372,8 +1550,8 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     String manager = "manager_key";
     expectAuth(ROOT, true);
     expect(storageUtil.jobStore.fetchManagerIds()).andReturn(ImmutableSet.of(manager));
-    expect(storageUtil.jobStore.fetchJobs(manager))
-        .andReturn(ImmutableList.<IJobConfiguration>of());
+    expect(storageUtil.jobStore.fetchJob(manager, IJobKey.build(oldJob.getKey())))
+        .andReturn(Optional.<IJobConfiguration>absent());
 
     control.replay();
 
@@ -1388,11 +1566,19 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     JobConfiguration oldJob = makeJob(productionTask());
     JobConfiguration newJob = oldJob.deepCopy();
     newJob.getTaskConfig().setExecutorConfig(new ExecutorConfig("aurora", "rewritten"));
-    String manager = "manager_key";
+    String manager1 = "manager1";
+    String manager2 = "manager2";
+    String manager3 = "manager3";
     expectAuth(ROOT, true);
-    expect(storageUtil.jobStore.fetchManagerIds()).andReturn(ImmutableSet.of(manager));
-    expect(storageUtil.jobStore.fetchJobs(manager))
-        .andReturn(IJobConfiguration.listFromBuilders(ImmutableList.of(oldJob, makeJob())));
+    expect(storageUtil.jobStore.fetchManagerIds())
+        .andReturn(ImmutableSet.of(manager1, manager2, manager3));
+    expect(storageUtil.jobStore.fetchJob(manager1, IJobKey.build(oldJob.getKey())))
+        .andReturn(Optional.of(IJobConfiguration.build(oldJob)));
+    expect(storageUtil.jobStore.fetchJob(manager2, IJobKey.build(oldJob.getKey())))
+        .andReturn(Optional.of(IJobConfiguration.build(makeJob())));
+    expect(storageUtil.jobStore.fetchJob(manager3, IJobKey.build(oldJob.getKey())))
+        .andReturn(Optional.of(IJobConfiguration.build(
+            makeJob().setKey(JobKeys.from("1", "2", "3").newBuilder()))));
 
     control.replay();
 
@@ -1410,8 +1596,8 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     String manager = "manager_key";
     expectAuth(ROOT, true);
     expect(storageUtil.jobStore.fetchManagerIds()).andReturn(ImmutableSet.of(manager));
-    expect(storageUtil.jobStore.fetchJobs(manager))
-        .andReturn(ImmutableList.of(IJobConfiguration.build(oldJob)));
+    expect(storageUtil.jobStore.fetchJob(manager, IJobKey.build(oldJob.getKey())))
+        .andReturn(Optional.of(IJobConfiguration.build(oldJob)));
     storageUtil.jobStore.saveAcceptedJob(
         manager,
         ConfigurationManager.validateAndPopulate(IJobConfiguration.build(newJob)));
@@ -1579,12 +1765,42 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     return okResponse(Result.jobSummaryResult(new JobSummaryResult().setSummaries(jobSummaries)));
   }
 
-  private Response okResponse(Result result) {
-    return Util.emptyResponse()
-        .setResponseCode(OK)
+  private static final Function<String, ResponseDetail> MESSAGE_TO_DETAIL =
+      new Function<String, ResponseDetail>() {
+        @Override
+        public ResponseDetail apply(String message) {
+          return new ResponseDetail().setMessage(message);
+        }
+      };
+
+  private Response response(ResponseCode code, Optional<Result> result, String... messages)
{
+    Response response = Util.emptyResponse()
+        .setResponseCode(code)
         .setDEPRECATEDversion(API_VERSION)
         .setServerInfo(SERVER_INFO)
-        .setResult(result);
+        .setResult(result.orNull());
+    if (messages.length > 0) {
+      response.setMessageDEPRECATED(Joiner.on(", ").join(messages));
+      response
+          .setDetails(FluentIterable.from(Arrays.asList(messages)).transform(MESSAGE_TO_DETAIL)
+              .toList());
+    }
+
+    return response;
+  }
+
+  private Response okResponse(Result result) {
+    return response(OK, Optional.of(result));
+  }
+
+  private Response okEmptyResponse() {
+    return response(OK, Optional.<Result>absent());
+  }
+
+  private Response errorResponse(String message) {
+    return response(ERROR, Optional.<Result>absent())
+        .setMessageDEPRECATED(message)
+        .setDetails(ImmutableList.of(new ResponseDetail().setMessage(message)));
   }
 
   private Response invalidResponse(String message) {
@@ -2213,6 +2429,16 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   }
 
   @Test
+  public void testReleaseLockUnchecked() throws Exception {
+    expectAuth(ROLE, true);
+    lockManager.releaseLock(LOCK);
+
+    control.replay();
+
+    assertEquals(okEmptyResponse(), thrift.releaseLock(LOCK.newBuilder(), UNCHECKED, SESSION));
+  }
+
+  @Test
   public void testGetLocks() throws Exception {
     expect(lockManager.getLocks()).andReturn(ImmutableSet.of(LOCK));
 
@@ -2530,7 +2756,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
 
     control.replay();
     JobUpdateRequest request = buildJobUpdateRequest(IJobUpdate.build(builder));
-    assertResponse(OK, thrift.startJobUpdate(request, SESSION));
+    assertResponse(INVALID_REQUEST, thrift.startJobUpdate(request, SESSION));
   }
 
   @Test
@@ -2736,6 +2962,15 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     assertResponse(INVALID_REQUEST, thrift.abortJobUpdate(JOB_KEY.newBuilder(), SESSION));
   }
 
+  @Test
+  public void testPulseJobUpdate() throws Exception {
+    control.replay();
+
+    assertEquals(
+        errorResponse(SchedulerThriftInterface.NOT_IMPLEMENTED_MESSAGE),
+        thrift.pulseJobUpdate("update", SESSION));
+  }
+
   private static JobConfiguration makeProdJob() {
     return makeJob(productionTask(), 1);
   }
@@ -2770,12 +3005,14 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
         .setKey(JOB_KEY.newBuilder());
   }
 
+  private static final String AUTH_DENIED_MESSAGE = "Denied!";
+
   private IExpectationSetters<?> expectAuth(Set<String> roles, boolean allowed)
       throws AuthFailedException {
 
     if (!allowed) {
       return expect(userValidator.checkAuthenticated(SESSION, roles))
-          .andThrow(new AuthFailedException("Denied!"));
+          .andThrow(new AuthFailedException(AUTH_DENIED_MESSAGE));
     } else {
       return expect(userValidator.checkAuthenticated(SESSION, roles))
           .andReturn(context);
@@ -2795,7 +3032,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
       return expect(userValidator.checkAuthorized(
           eq(SESSION),
           eq(capability),
-          anyObject(AuditCheck.class))).andThrow(new AuthFailedException("Denied!"));
+          anyObject(AuditCheck.class))).andThrow(new AuthFailedException(AUTH_DENIED_MESSAGE));
     } else {
       return expect(userValidator.checkAuthorized(
           eq(SESSION),


Mime
View raw message