You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ti...@apache.org on 2019/01/30 15:04:15 UTC

[mesos] branch 1.7.x updated (e3623a4 -> b40cbe0)

This is an automated email from the ASF dual-hosted git repository.

tillt pushed a change to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from e3623a4  Added MESOS-9517 to the 1.7.2 CHANGELOG.
     new 4018683  Fixed scheduler library on multiple SUBSCRIBE requests per connection.
     new b40cbe0  Added MESOS-9210 to 1.7.2 CHANGELOG.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 CHANGELOG                     |  1 +
 src/scheduler/scheduler.cpp   | 40 +++++++++++++++++----------------
 src/tests/scheduler_tests.cpp | 52 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 19 deletions(-)


[mesos] 01/02: Fixed scheduler library on multiple SUBSCRIBE requests per connection.

Posted by ti...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tillt pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 4018683beeebadf6722a722ed5d9d8a7e396de8e
Author: Till Toenshoff <to...@me.com>
AuthorDate: Wed Jan 30 05:37:17 2019 +0100

    Fixed scheduler library on multiple SUBSCRIBE requests per connection.
    
    The HTTP scheduler API dictates that on a single connection, the
    scheduler may only send a single SUBSCRIBE request. Due to recent
    authentication related changes, this contract got broken. This patch
    restores the contract and adds a test validating that the library is
    enforcing it.
    
    Review: https://reviews.apache.org/r/69839/
    
    (cherry picked from commit a5b9fcafbdf8663707e19c818be8a2da1eff8622)
---
 src/scheduler/scheduler.cpp   | 40 +++++++++++++++++----------------
 src/tests/scheduler_tests.cpp | 52 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 19 deletions(-)

diff --git a/src/scheduler/scheduler.cpp b/src/scheduler/scheduler.cpp
index cb24ba9..674483a 100644
--- a/src/scheduler/scheduler.cpp
+++ b/src/scheduler/scheduler.cpp
@@ -231,22 +231,6 @@ public:
       return;
     }
 
-    if (call.type() == Call::SUBSCRIBE && state != CONNECTED) {
-      // It might be possible that the scheduler is retrying. We drop the
-      // request if we have an ongoing subscribe request in flight or if the
-      // scheduler is already subscribed.
-      drop(call, "Scheduler is in state " + stringify(state));
-      return;
-    }
-
-    if (call.type() != Call::SUBSCRIBE && state != SUBSCRIBED) {
-      // We drop all non-subscribe calls if we are not currently subscribed.
-      drop(call, "Scheduler is in state " + stringify(state));
-      return;
-    }
-
-    VLOG(1) << "Sending " << call.type() << " call to " << master.get();
-
     // TODO(vinod): Add support for sending MESSAGE calls directly
     // to the slave, instead of relaying it through the master, as
     // the scheduler driver does.
@@ -259,6 +243,9 @@ public:
     request.headers = {{"Accept", stringify(contentType)},
                        {"Content-Type", stringify(contentType)}};
 
+    VLOG(1) << "Adding authentication headers to " << call.type() << " call to "
+            << master.get();
+
     // TODO(tillt): Add support for multi-step authentication protocols.
     authenticatee->authenticate(request, credential)
       .onAny(defer(self(), &Self::_send, call, lambda::_1));
@@ -584,9 +571,22 @@ protected:
   void _send(const Call& call, const Future<process::http::Request>& future)
   {
     if (!future.isReady()) {
-      LOG(ERROR) << "HTTP authenticatee "
-                 << (future.isFailed() ? "failed: " + future.failure()
-                                       : "discarded");
+      LOG(ERROR) << "HTTP authenticatee failed while adding authentication"
+                 << " header to request: " << future;
+      return;
+    }
+
+    if (call.type() == Call::SUBSCRIBE && state != CONNECTED) {
+      // It might be possible that the scheduler is retrying. We drop the
+      // request if we have an ongoing subscribe request in flight or if the
+      // scheduler is already subscribed.
+      drop(call, "Scheduler is in state " + stringify(state));
+      return;
+    }
+
+    if (call.type() != Call::SUBSCRIBE && state != SUBSCRIBED) {
+      // We drop all non-subscribe calls if we are not currently subscribed.
+      drop(call, "Scheduler is in state " + stringify(state));
       return;
     }
 
@@ -597,6 +597,8 @@ protected:
       return;
     }
 
+    VLOG(1) << "Sending " << call.type() << " call to " << master.get();
+
     Future<process::http::Response> response;
     if (call.type() == Call::SUBSCRIBE) {
       state = SUBSCRIBING;
diff --git a/src/tests/scheduler_tests.cpp b/src/tests/scheduler_tests.cpp
index 0ee5b77..fb5f1ad 100644
--- a/src/tests/scheduler_tests.cpp
+++ b/src/tests/scheduler_tests.cpp
@@ -162,6 +162,58 @@ TEST_P(SchedulerTest, Subscribe)
 }
 
 
+// Test validates that the scheduler library will not allow multiple
+// SUBSCRIBE requests over the same connection.
+TEST_P(SchedulerTest, SubscribeDrop)
+{
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  auto scheduler = std::make_shared<v1::MockHTTPScheduler>();
+
+  Future<Nothing> connected;
+  EXPECT_CALL(*scheduler, connected(_))
+    .WillOnce(FutureSatisfy(&connected));
+
+  ContentType contentType = GetParam();
+
+  v1::scheduler::TestMesos mesos(
+      master.get()->pid,
+      contentType,
+      scheduler);
+
+  AWAIT_READY(connected);
+
+  Future<Nothing> subscribed;
+  EXPECT_CALL(*scheduler, subscribed(_, _))
+    .WillOnce(FutureSatisfy(&subscribed));
+
+  Future<Nothing> heartbeat;
+  EXPECT_CALL(*scheduler, heartbeat(_))
+    .WillOnce(FutureSatisfy(&heartbeat));
+
+  Clock::pause();
+
+  mesos.send(v1::createCallSubscribe(v1::DEFAULT_FRAMEWORK_INFO));
+
+  // Send another SUBSCRIBE request. This one should get dropped as we
+  // already have a SUBSCRIBE in flight on that same connection.
+
+  mesos.send(v1::createCallSubscribe(v1::DEFAULT_FRAMEWORK_INFO));
+
+  AWAIT_READY(subscribed);
+  AWAIT_READY(heartbeat);
+
+  Clock::resume();
+
+  {
+    JSON::Object metrics = Metrics();
+
+    EXPECT_EQ(1u, metrics.values["master/messages_register_framework"]);
+  }
+}
+
+
 // This test verifies that a scheduler can subscribe with the master after
 // failing over to another instance.
 TEST_P(SchedulerTest, SchedulerFailover)


[mesos] 02/02: Added MESOS-9210 to 1.7.2 CHANGELOG.

Posted by ti...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tillt pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit b40cbe090fa505ee0dbcb5893436630eab6b5ea5
Author: Till Toenshoff <to...@me.com>
AuthorDate: Wed Jan 30 06:25:49 2019 +0100

    Added MESOS-9210 to 1.7.2 CHANGELOG.
---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index 3e40356..e4e9f66 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,7 @@ Release Notes - Mesos - Version 1.7.2 (WIP)
 * This is a bug fix release.
 
 ** Bug
+  * [MESOS-9210] - Mesos v1 scheduler library does not properly handle SUBSCRIBE retries.
   * [MESOS-9517] - SLRP should treat gRPC timeouts as non-terminal errors, instead of reporting OPERATION_FAILED.
   * [MESOS-9531] - chown error handling is incorrect in createSandboxDirectory.
   * [MESOS-9532] - ResourceOffersTest.ResourceOfferWithMultipleSlaves is flaky.