[mesos] 02/02: Added a test to verify agent authentication retry backoff logic.

[ga...@apache.org]

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

gaston pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 961d12219893aae2978d194140d3a71b0379d4ac
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Thu Sep 20 14:55:03 2018 -0700

    Added a test to verify agent authentication retry backoff logic.
    
    This test verifies that the agent backs-off properly when
    retrying authentication according to the configured parameters.
    
    Also mocked `Slave::authenticate()` for this test.
    
    Review: https://reviews.apache.org/r/68354/
---
 src/slave/slave.hpp                |   3 +-
 src/tests/authentication_tests.cpp | 109 +++++++++++++++++++++++++++++++++++++
 src/tests/mock_slave.cpp           |  10 ++++
 src/tests/mock_slave.hpp           |   8 +++
 4 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index 6757eae..0bd3401 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -491,7 +491,8 @@ public:
   // not receive a ping.
   void pingTimeout(process::Future<Option<MasterInfo>> future);
 
-  void authenticate(Duration minTimeout, Duration maxTimeout);
+  // Made virtual for testing purpose.
+  virtual void authenticate(Duration minTimeout, Duration maxTimeout);
 
   // Helper routines to lookup a framework/executor.
   Framework* getFramework(const FrameworkID& frameworkId) const;
diff --git a/src/tests/authentication_tests.cpp b/src/tests/authentication_tests.cpp
index bbb7e10..1973810 100644
--- a/src/tests/authentication_tests.cpp
+++ b/src/tests/authentication_tests.cpp
@@ -44,6 +44,7 @@ using mesos::master::detector::StandaloneMasterDetector;
 using testing::_;
 using testing::Eq;
 using testing::Return;
+using testing::SaveArg;
 
 namespace mesos {
 namespace internal {
@@ -426,6 +427,114 @@ TEST_F(AuthenticationTest, RetrySlaveAuthentication)
   ASSERT_NE("", slaveRegisteredMessage->slave_id().value());
 }
 
+// Verify that the agent backs off properly when retrying authentication
+// according to the configured parameters.
+TEST_F(AuthenticationTest, SlaveAuthenticationRetryBackoff)
+{
+  Clock::pause();
+
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+
+  slave::Flags slaveFlags = CreateSlaveFlags();
+  slaveFlags.authentication_timeout_min = Seconds(5);
+  slaveFlags.authentication_timeout_max = Minutes(1);
+  slaveFlags.authentication_backoff_factor = Seconds(1);
+
+  // Expected retry timeout range:
+  //
+  //   [min, min + factor * 2^1]
+  //   [min, min + factor * 2^2]
+  //   ...
+  //   [min, min + factor * 2^N]
+  //   ...
+  //   [min, max] // Stop at max.
+  Duration expected[7][2] = {
+    {Seconds(5), Seconds(7)},
+    {Seconds(5), Seconds(9)},
+    {Seconds(5), Seconds(13)},
+    {Seconds(5), Seconds(21)},
+    {Seconds(5), Seconds(37)},
+    {Seconds(5), Minutes(1)},
+    {Seconds(5), Minutes(1)}
+  };
+
+  Try<Owned<cluster::Slave>> slave =
+    StartSlave(detector.get(), slaveFlags, true);
+  ASSERT_SOME(slave);
+
+  // Drop the first authentication attempt.
+  Future<Nothing> authenticate;
+  Duration minTimeout, maxTimeout;
+  EXPECT_CALL(*slave.get()->mock(), authenticate(_, _))
+    .WillOnce(DoAll(
+        SaveArg<0>(&minTimeout),
+        SaveArg<1>(&maxTimeout),
+        FutureSatisfy(&authenticate)))
+    .RetiresOnSaturation();
+
+  slave.get()->start();
+
+  // Trigger the first authentication request.
+  Clock::advance(slaveFlags.authentication_backoff_factor);
+  AWAIT_READY(authenticate);
+
+  for (int i = 0; i < 7; i++) {
+    EXPECT_EQ(minTimeout, expected[i][0]);
+    EXPECT_EQ(maxTimeout, expected[i][1]);
+
+    // Drop the authenticate message from the slave to incur retry.
+    Future<AuthenticateMessage> authenticateMessage =
+      DROP_PROTOBUF(AuthenticateMessage(), _, _);
+
+    // Drop the retry call and manually issue the retry call (instead
+    // of invoking the unmocked call directly in the expectation. We do this
+    // so that, even though clock is advanced for `authentication_timeout_max`
+    // in each iteration, we can still guarantee:
+    //
+    // (1) Retry only happens once in each iteration;
+    //
+    // (2) At the end of each iteration, the authentication timeout timer is
+    // not started. The timer will start only when we manually issue the
+    // retry call in the next iteration.
+    EXPECT_CALL(*slave.get()->mock(), authenticate(_, _))
+      .WillOnce(DoAll(
+          SaveArg<0>(&minTimeout),
+          SaveArg<1>(&maxTimeout),
+          FutureSatisfy(&authenticate)))
+      .RetiresOnSaturation();
+
+    slave.get()->mock()->unmocked_authenticate(minTimeout, maxTimeout);
+
+    // Slave should not retry until `slaveFlags.authentication_timeout_min`.
+    Clock::advance(slaveFlags.authentication_timeout_min - Milliseconds(1));
+    Clock::settle();
+    EXPECT_TRUE(authenticate.isPending());
+
+    // Slave will retry at least once in
+    // `slaveFlags.authentication_timeout_max`.
+    Clock::advance(
+        slaveFlags.authentication_timeout_max -
+        slaveFlags.authentication_timeout_min + Milliseconds(1));
+    Clock::settle();
+
+    AWAIT_READY(authenticateMessage);
+    AWAIT_READY(authenticate);
+  }
+
+  Future<AuthenticationCompletedMessage> authenticationCompletedMessage =
+    FUTURE_PROTOBUF(AuthenticationCompletedMessage(), _, _);
+
+  slave.get()->mock()->unmocked_authenticate(minTimeout, maxTimeout);
+
+  Clock::advance(slaveFlags.authentication_timeout_max);
+
+  // Slave should be able to get authenticated.
+  AWAIT_READY(authenticationCompletedMessage);
+}
+
 
 // This test ensures that when the master sees a new authentication
 // request for a particular agent or scheduler (we just test the
diff --git a/src/tests/mock_slave.cpp b/src/tests/mock_slave.cpp
index 94a5b0d..a78ca9c 100644
--- a/src/tests/mock_slave.cpp
+++ b/src/tests/mock_slave.cpp
@@ -134,6 +134,8 @@ MockSlave::MockSlave(
     .WillRepeatedly(Invoke(this, &MockSlave::unmocked_runTaskGroup));
   EXPECT_CALL(*this, killTask(_, _))
     .WillRepeatedly(Invoke(this, &MockSlave::unmocked_killTask));
+  EXPECT_CALL(*this, authenticate(_, _))
+    .WillRepeatedly(Invoke(this, &MockSlave::unmocked_authenticate));
   EXPECT_CALL(*this, removeFramework(_))
     .WillRepeatedly(Invoke(this, &MockSlave::unmocked_removeFramework));
   EXPECT_CALL(*this, __recover(_))
@@ -251,6 +253,14 @@ void MockSlave::unmocked_killTask(
 }
 
 
+void MockSlave::unmocked_authenticate(
+    Duration minTimeout,
+    Duration maxTimeout)
+{
+  slave::Slave::authenticate(minTimeout, maxTimeout);
+}
+
+
 void MockSlave::unmocked_removeFramework(slave::Framework* framework)
 {
   slave::Slave::removeFramework(framework);
diff --git a/src/tests/mock_slave.hpp b/src/tests/mock_slave.hpp
index 9a74bf3..3c0d602 100644
--- a/src/tests/mock_slave.hpp
+++ b/src/tests/mock_slave.hpp
@@ -191,6 +191,14 @@ public:
       const process::UPID& from,
       const KillTaskMessage& killTaskMessage);
 
+  MOCK_METHOD2(authenticate, void(
+      Duration minTimeout,
+      Duration maxTimeout));
+
+  void unmocked_authenticate(
+      Duration minTimeout,
+      Duration maxTimeout);
+
   MOCK_METHOD1(removeFramework, void(
       slave::Framework* framework));