You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2016/07/06 18:26:30 UTC

[1/4] mesos git commit: Removed CHECK from orphan_task filtering.

Repository: mesos
Updated Branches:
  refs/heads/master 485dcaf8c -> d6a209a79


Removed CHECK from orphan_task filtering.

This CHECK can be problematic when upgrading from old agents to
a new master.

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


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

Branch: refs/heads/master
Commit: 9113bad786d1adfe7c618b07264868a74535b1d9
Parents: 485dcaf
Author: Joerg Schad <jo...@mesosphere.io>
Authored: Wed Jul 6 13:25:56 2016 -0500
Committer: Vinod Kone <vi...@gmail.com>
Committed: Wed Jul 6 13:25:56 2016 -0500

----------------------------------------------------------------------
 src/master/http.cpp | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/9113bad7/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 3640486..c1f5690 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -2606,15 +2606,24 @@ Future<Response> Master::Http::state(
             foreachvalue (const TaskMap& tasks, slave->tasks) {
               foreachvalue (const Task* task, tasks) {
                 CHECK_NOTNULL(task);
-                if (!master->frameworks.registered.contains(
-                    task->framework_id())) {
-                  CHECK(master->frameworks.recovered.contains(
-                      task->framework_id()));
-
-                  if (!approveViewTask(
-                      tasksApprover,
-                      *task,
-                      master->frameworks.recovered[task->framework_id()])) {
+                const FrameworkID& frameworkId = task->framework_id();
+                if (!master->frameworks.registered.contains(frameworkId)) {
+                  // TODO(joerg84): This logic should be simplified after
+                  // a deprecation cycle starting with 1.0 as after that
+                  // we can rely on 'master->frameworks.recovered' containing
+                  // all FrameworkInfos.
+                  // Until then there are 3 cases:
+                  // - No authorization enabled: show all orphaned tasks.
+                  // - Authorization enabled, but no FrameworkInfo present:
+                  //   do not show orphaned tasks.
+                  // - Authorization enabled, FrameworkInfo present: filter
+                  //   based on 'approveViewTask'.
+                  if (master->authorizer.isSome() &&
+                     (!master->frameworks.recovered.contains(frameworkId) ||
+                      !approveViewTask(
+                          tasksApprover,
+                          *task,
+                          master->frameworks.recovered[frameworkId]))) {
                     continue;
                   }
 


[3/4] mesos git commit: Changed name of authorization test.

Posted by vi...@apache.org.
Changed name of authorization test.

The name should reflect the name of the
submessage in ReRegisterSlave message.

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


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

Branch: refs/heads/master
Commit: a626dfeed26026c7645df38f60db4ccd6119c00e
Parents: 1f2555a
Author: Joerg Schad <jo...@mesosphere.io>
Authored: Wed Jul 6 13:26:08 2016 -0500
Committer: Vinod Kone <vi...@gmail.com>
Committed: Wed Jul 6 13:26:08 2016 -0500

----------------------------------------------------------------------
 src/tests/master_slave_reconciliation_tests.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/a626dfee/src/tests/master_slave_reconciliation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_slave_reconciliation_tests.cpp b/src/tests/master_slave_reconciliation_tests.cpp
index 87cff8c..69ec707 100644
--- a/src/tests/master_slave_reconciliation_tests.cpp
+++ b/src/tests/master_slave_reconciliation_tests.cpp
@@ -548,7 +548,7 @@ TEST_F(MasterSlaveReconciliationTest, SlaveReregisterTerminalTask)
 
 // This test verifies that when the slave re-registers, we correctly
 // send the information about actively running frameworks.
-TEST_F(MasterSlaveReconciliationTest, SlaveReregisterActiveFrameworks)
+TEST_F(MasterSlaveReconciliationTest, SlaveReregisterFrameworks)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);


[2/4] mesos git commit: Renamed incorrect variable name in authorization tests.

Posted by vi...@apache.org.
Renamed incorrect variable name in authorization tests.

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


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

Branch: refs/heads/master
Commit: 1f2555abc4596fcb93fe60da3d676f73722940a2
Parents: 9113bad
Author: Joerg Schad <jo...@mesosphere.io>
Authored: Wed Jul 6 13:26:02 2016 -0500
Committer: Vinod Kone <vi...@gmail.com>
Committed: Wed Jul 6 13:26:02 2016 -0500

----------------------------------------------------------------------
 src/tests/master_authorization_tests.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1f2555ab/src/tests/master_authorization_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_authorization_tests.cpp b/src/tests/master_authorization_tests.cpp
index 21a65c4..3632042 100644
--- a/src/tests/master_authorization_tests.cpp
+++ b/src/tests/master_authorization_tests.cpp
@@ -1521,7 +1521,7 @@ TYPED_TEST(MasterAuthorizerTest, FilterFrameworksEndpoint)
 // one is allowed to view the tasks.
 TYPED_TEST(MasterAuthorizerTest, FilterTasksEndpoint)
 {
-  const string stateEndpoint = "tasks";
+  const string tasksEndpoint = "tasks";
   const string user = "bar";
 
   ACLs acls;
@@ -1649,7 +1649,7 @@ TYPED_TEST(MasterAuthorizerTest, FilterTasksEndpoint)
   {
     Future<Response> response = http::get(
         master.get()->pid,
-        stateEndpoint,
+        tasksEndpoint,
         None(),
         createBasicAuthHeaders(DEFAULT_CREDENTIAL));
 
@@ -1669,7 +1669,7 @@ TYPED_TEST(MasterAuthorizerTest, FilterTasksEndpoint)
   {
     Future<Response> response = http::get(
         master.get()->pid,
-        stateEndpoint,
+        tasksEndpoint,
         None(),
         createBasicAuthHeaders(DEFAULT_CREDENTIAL_2));
 


[4/4] mesos git commit: Added test to check orphaned tasks are filtered.

Posted by vi...@apache.org.
Added test to check orphaned tasks are filtered.

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


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

Branch: refs/heads/master
Commit: d6a209a79cbc4ccf14d2397d079b21230ecd6494
Parents: a626dfe
Author: Joerg Schad <jo...@mesosphere.io>
Authored: Wed Jul 6 13:26:16 2016 -0500
Committer: Vinod Kone <vi...@gmail.com>
Committed: Wed Jul 6 13:26:16 2016 -0500

----------------------------------------------------------------------
 src/tests/master_authorization_tests.cpp | 165 ++++++++++++++++++++++++++
 1 file changed, 165 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d6a209a7/src/tests/master_authorization_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_authorization_tests.cpp b/src/tests/master_authorization_tests.cpp
index 3632042..e43b264 100644
--- a/src/tests/master_authorization_tests.cpp
+++ b/src/tests/master_authorization_tests.cpp
@@ -1854,6 +1854,171 @@ TYPED_TEST(MasterAuthorizerTest, FilterRolesEndpoint)
   }
 }
 
+
+// This test verifies that authorization based endpoint filtering
+// works correctly on the /state endpoint with orphaned tasks.
+// Both default users are allowed to to view high level frameworks, but only
+// one is allowed to view the tasks.
+TYPED_TEST(MasterAuthorizerTest, FilterOrphanedTasks)
+{
+  ACLs acls;
+
+  {
+    // Default principal can see all frameworks.
+    mesos::ACL::ViewFramework* acl = acls.add_view_frameworks();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+    acl->mutable_users()->set_type(ACL::Entity::ANY);
+  }
+
+  {
+    // Second default principal can see all frameworks.
+    mesos::ACL::ViewFramework* acl = acls.add_view_frameworks();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL_2.principal());
+    acl->mutable_users()->set_type(ACL::Entity::ANY);
+  }
+
+  {
+    // No other principal can see frameworks running under any user.
+    ACL::ViewFramework* acl = acls.add_view_frameworks();
+    acl->mutable_principals()->set_type(ACL::Entity::ANY);
+    acl->mutable_users()->set_type(ACL::Entity::NONE);
+  }
+
+  {
+    // Default principal can see all executors.
+    mesos::ACL::ViewExecutor* acl = acls.add_view_executors();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+    acl->mutable_users()->set_type(ACL::Entity::ANY);
+  }
+
+  {
+    // No other principal can see executors running under any user.
+    ACL::ViewExecutor* acl = acls.add_view_executors();
+    acl->mutable_principals()->set_type(ACL::Entity::ANY);
+    acl->mutable_users()->set_type(ACL::Entity::NONE);
+  }
+
+  {
+    // Default principal can see all tasks.
+    mesos::ACL::ViewTask* acl = acls.add_view_tasks();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+    acl->mutable_users()->set_type(ACL::Entity::ANY);
+  }
+
+  {
+    // No other principal can see tasks running under any user.
+    ACL::ViewTask* acl = acls.add_view_tasks();
+    acl->mutable_principals()->set_type(ACL::Entity::ANY);
+    acl->mutable_users()->set_type(ACL::Entity::NONE);
+  }
+
+  // Create an `Authorizer` with the ACLs.
+  Try<Authorizer*> create = TypeParam::create(parameterize(acls));
+  ASSERT_SOME(create);
+  Owned<Authorizer> authorizer(create.get());
+
+  Try<Owned<cluster::Master>> master = this->StartMaster(authorizer.get());
+  ASSERT_SOME(master);
+
+  MockExecutor exec(DEFAULT_EXECUTOR_ID);
+  TestContainerizer containerizer(&exec);
+
+  StandaloneMasterDetector detector(master.get()->pid);
+  Try<Owned<cluster::Slave>> slave = this->StartSlave(
+      &detector, &containerizer);
+
+  ASSERT_SOME(slave);
+
+  MockScheduler sched;
+  MesosSchedulerDriver driver(
+      &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
+
+  EXPECT_CALL(sched, registered(&driver, _, _));
+
+  EXPECT_CALL(sched, resourceOffers(&driver, _))
+    .WillOnce(LaunchTasks(DEFAULT_EXECUTOR_INFO, 1, 1, 16, "*"))
+    .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+  driver.start();
+
+  Future<Nothing> statusUpdate;
+  EXPECT_CALL(sched, statusUpdate(&driver, _))
+    .WillOnce(FutureSatisfy(&statusUpdate));    // TASK_RUNNING.
+
+  EXPECT_CALL(exec, registered(_, _, _, _));
+
+  // Send an update right away.
+  EXPECT_CALL(exec, launchTask(_, _))
+    .WillOnce(SendStatusUpdateFromTask(TASK_RUNNING));
+
+    // Wait until TASK_RUNNING of the task is received.
+  AWAIT_READY(statusUpdate);
+
+  Future<SlaveReregisteredMessage> slaveReregisteredMessage =
+    FUTURE_PROTOBUF(SlaveReregisteredMessage(), _, _);
+
+  // The master failover.
+  master->reset();
+  master = this->StartMaster(authorizer.get());
+  ASSERT_SOME(master);
+
+  // Simulate a new master detected event to the slave.
+  detector.appoint(master.get()->pid);
+
+  // The framework will not re-register with the new master as the
+  // scheduler is bound to the old master pid.
+
+  AWAIT_READY(slaveReregisteredMessage);
+
+  const string stateEndpoint = "state";
+
+  // Retrieve endpoint with the user allowed to view the framework and
+  // tasks.
+  {
+    Future<Response> response = http::get(
+        master.get()->pid,
+        stateEndpoint,
+        None(),
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+      << response.get().body;
+
+    Try<JSON::Object> parse = JSON::parse<JSON::Object>(response.get().body);
+    ASSERT_SOME(parse);
+
+    JSON::Object tasks = parse.get();
+    ASSERT_TRUE(tasks.values["orphan_tasks"].is<JSON::Array>());
+    EXPECT_EQ(1u, tasks.values["orphan_tasks"].as<JSON::Array>().values.size());
+  }
+
+  // Retrieve endpoint with the user allowed to view the framework,
+  // but not the tasks.
+  {
+    Future<Response> response = http::get(
+        master.get()->pid,
+        stateEndpoint,
+        None(),
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL_2));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+      << response.get().body;
+
+    Try<JSON::Object> parse = JSON::parse<JSON::Object>(response.get().body);
+    ASSERT_SOME(parse);
+
+    JSON::Object tasks = parse.get();
+    ASSERT_TRUE(tasks.values["orphan_tasks"].is<JSON::Array>());
+    EXPECT_TRUE(tasks.values["orphan_tasks"].as<JSON::Array>().values.empty());
+  }
+
+  EXPECT_CALL(exec, shutdown(_))
+    .Times(AtMost(1));
+
+  driver.stop();
+  driver.join();
+}
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {