You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by al...@apache.org on 2017/10/20 05:57:19 UTC

[1/2] mesos git commit: Added and used more v1 scheduler API test helpers.

Repository: mesos
Updated Branches:
  refs/heads/master daa751b8b -> 5cc37c2cd


Added and used more v1 scheduler API test helpers.

Review: https://reviews.apache.org/r/63172/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/377b684d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/377b684d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/377b684d

Branch: refs/heads/master
Commit: 377b684dceb94bce86f91947a1318ea5cd4d47d3
Parents: daa751b
Author: Gaston Kleiman <ga...@mesosphere.io>
Authored: Thu Oct 19 22:55:01 2017 -0700
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Thu Oct 19 22:55:01 2017 -0700

----------------------------------------------------------------------
 src/tests/default_executor_tests.cpp | 61 ++++++-------------------------
 src/tests/mesos.hpp                  | 22 +++++++++++
 2 files changed, 33 insertions(+), 50 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/377b684d/src/tests/default_executor_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/default_executor_tests.cpp b/src/tests/default_executor_tests.cpp
index b3c1d82..a248aa1 100644
--- a/src/tests/default_executor_tests.cpp
+++ b/src/tests/default_executor_tests.cpp
@@ -398,16 +398,7 @@ TEST_P(DefaultExecutorTest, KillTask)
     .WillOnce(FutureArg<1>(&executorFailure));
 
   // Now kill a task in the first task group.
-  {
-    Call call;
-    call.mutable_framework_id()->CopyFrom(frameworkId);
-    call.set_type(Call::KILL);
-
-    Call::Kill* kill = call.mutable_kill();
-    kill->mutable_task_id()->CopyFrom(taskInfo1.task_id());
-
-    mesos.send(call);
-  }
+  mesos.send(v1::createCallKill(frameworkId, taskInfo1.task_id()));
 
   // All the tasks in the first task group should be killed.
 
@@ -434,16 +425,7 @@ TEST_P(DefaultExecutorTest, KillTask)
     .WillOnce(FutureArg<1>(&killedUpdate3));
 
   // Now kill the only task present in the second task group.
-  {
-    Call call;
-    call.mutable_framework_id()->CopyFrom(frameworkId);
-    call.set_type(Call::KILL);
-
-    Call::Kill* kill = call.mutable_kill();
-    kill->mutable_task_id()->CopyFrom(taskInfo3.task_id());
-
-    mesos.send(call);
-  }
+  mesos.send(v1::createCallKill(frameworkId, taskInfo3.task_id()));
 
   AWAIT_READY(killedUpdate3);
   ASSERT_EQ(TASK_KILLED, killedUpdate3->status().state());
@@ -514,22 +496,6 @@ TEST_P(DefaultExecutorTest, KillTaskGroupOnTaskFailure)
   const v1::Offer& offer = offers->offers(0);
   const v1::AgentID& agentId = offer.agent_id();
 
-  auto acknowledge = [&](const Future<v1::scheduler::Event::Update>& update) {
-    Call call;
-    call.mutable_framework_id()->CopyFrom(frameworkId);
-    call.set_type(Call::ACKNOWLEDGE);
-
-    Call::Acknowledge* acknowledge = call.mutable_acknowledge();
-
-    acknowledge->mutable_task_id()->CopyFrom(
-        update->status().task_id());
-
-    acknowledge->mutable_agent_id()->CopyFrom(agentId);
-    acknowledge->set_uuid(update->status().uuid());
-
-    mesos.send(call);
-  };
-
   // The first task exits with a non-zero status code.
   v1::TaskInfo taskInfo1 = v1::createTask(agentId, resources, "exit 1");
 
@@ -561,8 +527,10 @@ TEST_P(DefaultExecutorTest, KillTaskGroupOnTaskFailure)
   AWAIT_READY(startingUpdate2);
   ASSERT_EQ(TASK_STARTING, startingUpdate2->status().state());
 
-  acknowledge(startingUpdate1);
-  acknowledge(startingUpdate2);
+  mesos.send(
+      v1::createCallAcknowledge(frameworkId, agentId, startingUpdate1.get()));
+  mesos.send(
+      v1::createCallAcknowledge(frameworkId, agentId, startingUpdate2.get()));
 
   AWAIT_READY(runningUpdate1);
   ASSERT_EQ(TASK_RUNNING, runningUpdate1->status().state());
@@ -585,8 +553,10 @@ TEST_P(DefaultExecutorTest, KillTaskGroupOnTaskFailure)
     .WillOnce(FutureArg<1>(&update2));
 
   // Acknowledge the TASK_RUNNING updates to receive the next updates.
-  acknowledge(runningUpdate1);
-  acknowledge(runningUpdate2);
+  mesos.send(
+      v1::createCallAcknowledge(frameworkId, agentId, runningUpdate1.get()));
+  mesos.send(
+      v1::createCallAcknowledge(frameworkId, agentId, runningUpdate2.get()));
 
   // Updates for the tasks in a task group can be received in any order.
   set<pair<v1::TaskID, v1::TaskState>> taskStates;
@@ -1050,16 +1020,7 @@ TEST_P(DefaultExecutorTest, CommitSuicideOnKillTask)
     .WillOnce(FutureArg<1>(&killedUpdate));
 
   // Now kill the second task in the task group.
-  {
-    Call call;
-    call.mutable_framework_id()->CopyFrom(frameworkId);
-    call.set_type(Call::KILL);
-
-    Call::Kill* kill = call.mutable_kill();
-    kill->mutable_task_id()->CopyFrom(taskInfo2.task_id());
-
-    mesos.send(call);
-  }
+  mesos.send(v1::createCallKill(frameworkId, taskInfo2.task_id()));
 
   AWAIT_READY(killedUpdate);
   ASSERT_EQ(TASK_KILLED, killedUpdate->status().state());

http://git-wip-us.apache.org/repos/asf/mesos/blob/377b684d/src/tests/mesos.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index 3cc70b6..4b61f2d 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -1816,6 +1816,28 @@ inline mesos::v1::scheduler::Call createCallAccept(
 }
 
 
+inline mesos::v1::scheduler::Call createCallAcknowledge(
+    const mesos::v1::FrameworkID& frameworkId,
+    const mesos::v1::AgentID& agentId,
+    const mesos::v1::scheduler::Event::Update& update)
+{
+  mesos::v1::scheduler::Call call;
+  call.set_type(mesos::v1::scheduler::Call::ACKNOWLEDGE);
+  call.mutable_framework_id()->CopyFrom(frameworkId);
+
+  mesos::v1::scheduler::Call::Acknowledge* acknowledge =
+    call.mutable_acknowledge();
+
+  acknowledge->mutable_task_id()->CopyFrom(
+      update.status().task_id());
+
+  acknowledge->mutable_agent_id()->CopyFrom(agentId);
+  acknowledge->set_uuid(update.status().uuid());
+
+  return call;
+}
+
+
 inline mesos::v1::scheduler::Call createCallKill(
     const mesos::v1::FrameworkID& frameworkId,
     const mesos::v1::TaskID& taskId,


[2/2] mesos git commit: Refactored and fixed `DefaultExecutorTest.CommitSuicideOnKillTask`.

Posted by al...@apache.org.
Refactored and fixed `DefaultExecutorTest.CommitSuicideOnKillTask`.

Make this flaky test more readable, and cleaner by not trying to guess
the order in which task status updates will arrive.

Review: https://reviews.apache.org/r/63173/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5cc37c2c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5cc37c2c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5cc37c2c

Branch: refs/heads/master
Commit: 5cc37c2cdf1a4e1ca1c23cbad2764679f24174e1
Parents: 377b684
Author: Gaston Kleiman <ga...@mesosphere.io>
Authored: Thu Oct 19 22:55:08 2017 -0700
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Thu Oct 19 22:56:59 2017 -0700

----------------------------------------------------------------------
 src/tests/default_executor_tests.cpp | 125 ++++++++++++++++++------------
 1 file changed, 74 insertions(+), 51 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/5cc37c2c/src/tests/default_executor_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/default_executor_tests.cpp b/src/tests/default_executor_tests.cpp
index a248aa1..5078bd4 100644
--- a/src/tests/default_executor_tests.cpp
+++ b/src/tests/default_executor_tests.cpp
@@ -944,34 +944,29 @@ TEST_P(DefaultExecutorTest, CommitSuicideOnKillTask)
   // The first task finishes successfully while the second
   // task is explicitly killed later.
 
-  v1::TaskInfo taskInfo1 = v1::createTask(agentId, resources, "exit 0");
+  v1::TaskInfo task1 = v1::createTask(agentId, resources, "exit 0");
 
-  v1::TaskInfo taskInfo2 =
-    v1::createTask(agentId, resources, SLEEP_COMMAND(1000));
+  v1::TaskInfo task2 = v1::createTask(agentId, resources, SLEEP_COMMAND(1000));
 
-  const hashset<v1::TaskID> tasks{taskInfo1.task_id(), taskInfo2.task_id()};
+  const hashset<v1::TaskID> taskIds{task1.task_id(), task2.task_id()};
 
-  Future<v1::scheduler::Event::Update> startingUpdate1;
-  Future<v1::scheduler::Event::Update> startingUpdate2;
-  Future<v1::scheduler::Event::Update> runningUpdate1;
-  Future<v1::scheduler::Event::Update> runningUpdate2;
-  EXPECT_CALL(*scheduler, update(_, _))
-    .WillOnce(
-        DoAll(
-            FutureArg<1>(&startingUpdate1),
-            v1::scheduler::SendAcknowledge(frameworkId, agentId)))
-    .WillOnce(
-        DoAll(
-            FutureArg<1>(&startingUpdate2),
-            v1::scheduler::SendAcknowledge(frameworkId, agentId)))
-    .WillOnce(
-        DoAll(
-            FutureArg<1>(&runningUpdate1),
-            v1::scheduler::SendAcknowledge(frameworkId, agentId)))
-    .WillOnce(
-        DoAll(
-            FutureArg<1>(&runningUpdate2),
-            v1::scheduler::SendAcknowledge(frameworkId, agentId)));
+  // We expect two TASK_STARTING, two TASK_RUNNING, and one TASK_FINISHED
+  // updates.
+  vector<Future<v1::scheduler::Event::Update>> updates(5);
+
+  {
+    // This variable doesn't have to be used explicitly. We need it so that the
+    // futures are satisfied in the order in which the updates are received.
+    testing::InSequence inSequence;
+
+    foreach (Future<v1::scheduler::Event::Update>& update, updates) {
+      EXPECT_CALL(*scheduler, update(_, _))
+        .WillOnce(
+            DoAll(
+                FutureArg<1>(&update),
+                v1::scheduler::SendAcknowledge(frameworkId, agentId)));
+    }
+  }
 
   Future<v1::scheduler::Event::Failure> executorFailure;
   EXPECT_CALL(*scheduler, failure(_, _))
@@ -982,49 +977,77 @@ TEST_P(DefaultExecutorTest, CommitSuicideOnKillTask)
           frameworkId,
           offer,
           {v1::LAUNCH_GROUP(
-              executorInfo, v1::createTaskGroupInfo({taskInfo1, taskInfo2}))}));
+              executorInfo, v1::createTaskGroupInfo({task1, task2}))}));
 
-  AWAIT_READY(startingUpdate1);
-  ASSERT_EQ(TASK_STARTING, startingUpdate1->status().state());
+  enum class Stage
+  {
+    INITIAL,
+    STARTING,
+    RUNNING,
+    FINISHED
+  };
 
-  // We only check the first and last update, because the other two might
-  // arrive in a different order.
+  hashmap<v1::TaskID, Stage> taskStages;
+  taskStages[task1.task_id()] = Stage::INITIAL;
+  taskStages[task2.task_id()] = Stage::INITIAL;
 
-  AWAIT_READY(runningUpdate2);
-  ASSERT_EQ(TASK_RUNNING, runningUpdate2->status().state());
+  foreach (Future<v1::scheduler::Event::Update>& update, updates) {
+    AWAIT_READY(update);
 
-  // When running a task, TASK_RUNNING updates for the tasks in a
-  // task group can be received in any order.
-  const hashset<v1::TaskID> tasksRunning{
-    startingUpdate1->status().task_id(),
-    startingUpdate2->status().task_id(),
-    runningUpdate1->status().task_id(),
-    runningUpdate2->status().task_id()};
+    const v1::TaskStatus& taskStatus = update->status();
 
-  ASSERT_EQ(tasks, tasksRunning);
+    Option<Stage> taskStage = taskStages.get(taskStatus.task_id());
+    ASSERT_SOME(taskStage);
 
-  Future<v1::scheduler::Event::Update> finishedUpdate;
-  EXPECT_CALL(*scheduler, update(_, _))
-    .WillOnce(FutureArg<1>(&finishedUpdate));
+    switch (taskStage.get()) {
+      case Stage::INITIAL: {
+        ASSERT_EQ(TASK_STARTING, taskStatus.state());
 
-  AWAIT_READY(finishedUpdate);
-  ASSERT_EQ(TASK_FINISHED, finishedUpdate->status().state());
-  ASSERT_EQ(taskInfo1.task_id(), finishedUpdate->status().task_id());
+        taskStages[taskStatus.task_id()] = Stage::STARTING;
+
+        break;
+      }
+      case Stage::STARTING: {
+        ASSERT_EQ(TASK_RUNNING, taskStatus.state());
+
+        taskStages[taskStatus.task_id()] = Stage::RUNNING;
+
+        break;
+      }
+      case Stage::RUNNING: {
+        ASSERT_EQ(TASK_FINISHED, taskStatus.state());
+
+        taskStages[taskStatus.task_id()] = Stage::FINISHED;
+
+        break;
+      }
+      case Stage::FINISHED: {
+        FAIL() << "Unexpected task update: " << update->DebugString();
+        break;
+      }
+    }
+  }
+
+  // `task1` should have finished, `task2` should still be running.
+  ASSERT_EQ(Stage::FINISHED, taskStages[task1.task_id()]);
+  ASSERT_EQ(Stage::RUNNING, taskStages[task2.task_id()]);
 
-  // The executor should still be alive after the task
-  // has finished successfully.
+  // The executor should still be alive after task1 has finished successfully.
   ASSERT_TRUE(executorFailure.isPending());
 
   Future<v1::scheduler::Event::Update> killedUpdate;
   EXPECT_CALL(*scheduler, update(_, _))
-    .WillOnce(FutureArg<1>(&killedUpdate));
+    .WillOnce(
+        DoAll(
+            FutureArg<1>(&killedUpdate),
+            v1::scheduler::SendAcknowledge(frameworkId, agentId)));
 
   // Now kill the second task in the task group.
-  mesos.send(v1::createCallKill(frameworkId, taskInfo2.task_id()));
+  mesos.send(v1::createCallKill(frameworkId, task2.task_id()));
 
   AWAIT_READY(killedUpdate);
   ASSERT_EQ(TASK_KILLED, killedUpdate->status().state());
-  ASSERT_EQ(taskInfo2.task_id(), killedUpdate->status().task_id());
+  ASSERT_EQ(task2.task_id(), killedUpdate->status().task_id());
 
   // The executor should commit suicide after the task is killed.
   AWAIT_READY(executorFailure);