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 <[email protected]> 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));
