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 2018/05/25 12:03:25 UTC
[2/2] mesos git commit: Ensured `SlaveRegisteredMessage`s trigger
appropriate expectations.
Ensured `SlaveRegisteredMessage`s trigger appropriate expectations.
An agent may retry `SlaveRegisteredMessage` if it does not receive
the registration confirmation on time. In this case the confirmation
may be sent twice. In tests with multiple registering agents, this
may result that an expectation set for one agent is satisfied by
a retried confirmation for another agent.
This patch unifies the way how this case is handled. An expectation
is augmented with a matcher for with the agent pid for which the
expectation is set.
Review: https://reviews.apache.org/r/67217
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/415451ee
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/415451ee
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/415451ee
Branch: refs/heads/master
Commit: 415451ee819d384af25f23885a26d15e83b1b785
Parents: 5984e68
Author: Alexander Rukletsov <al...@apache.org>
Authored: Fri May 18 15:25:22 2018 +0200
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Fri May 25 14:02:47 2018 +0200
----------------------------------------------------------------------
src/tests/master_maintenance_tests.cpp | 3 +--
src/tests/master_tests.cpp | 20 ++++++-------------
src/tests/partition_tests.cpp | 21 ++++++++++++--------
src/tests/persistent_volume_endpoints_tests.cpp | 3 +--
.../storage_local_resource_provider_tests.cpp | 4 +++-
5 files changed, 24 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/415451ee/src/tests/master_maintenance_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_maintenance_tests.cpp b/src/tests/master_maintenance_tests.cpp
index f3fb224..2d48d6a 100644
--- a/src/tests/master_maintenance_tests.cpp
+++ b/src/tests/master_maintenance_tests.cpp
@@ -1470,8 +1470,7 @@ TEST_F(MasterMaintenanceTest, InverseOffersFilters)
// Capture the registration message for the second slave.
Future<SlaveRegisteredMessage> slave2RegisteredMessage =
- FUTURE_PROTOBUF(
- SlaveRegisteredMessage(), master.get()->pid, Not(slave1.get()->pid));
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, Not(slave1.get()->pid));
slave::Flags slaveFlags2 = MesosTest::CreateSlaveFlags();
slaveFlags2.hostname = maintenanceHostname + "-2";
http://git-wip-us.apache.org/repos/asf/mesos/blob/415451ee/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 6a0fa98..6efcbd1 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -2507,8 +2507,7 @@ TEST_F(MasterTest, SlavesEndpointTwoSlaves)
AWAIT_READY(slave1RegisteredMessage);
Future<SlaveRegisteredMessage> slave2RegisteredMessage =
- FUTURE_PROTOBUF(
- SlaveRegisteredMessage(), master.get()->pid, Not(slave1.get()->pid));
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, Not(slave1.get()->pid));
Try<Owned<cluster::Slave>> slave2 = StartSlave(detector.get());
ASSERT_SOME(slave2);
@@ -2562,10 +2561,7 @@ TEST_F(MasterTest, SlavesEndpointQuerySlave)
AWAIT_READY(slave1RegisteredMessage);
Future<SlaveRegisteredMessage> slave2RegisteredMessage =
- FUTURE_PROTOBUF(
- SlaveRegisteredMessage(),
- master.get()->pid,
- Not(slave1.get()->pid));
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, Not(slave1.get()->pid));
Try<Owned<cluster::Slave>> slave2 = StartSlave(detector.get());
ASSERT_SOME(slave2);
@@ -6128,7 +6124,7 @@ TEST_F(MasterTest, DuplicatedSlaveIdWhenSlaveReregister)
ASSERT_SOME(master);
Future<SlaveRegisteredMessage> slaveRegisteredMessage2 =
- FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, Not(slave1.get()->pid));
// Start a new slave and make sure it registers before the old slave.
slave::Flags slaveFlags2 = CreateSlaveFlags();
@@ -8539,14 +8535,10 @@ TEST_F(MasterTest, RegistryGcByCount)
AWAIT_EXPECT_RESPONSE_STATUS_EQ(process::http::OK().status, response);
}
- // Wait for outstanding messages to be dropped, for example if
- // the `SlaveRegisteredMessage` was re-sent after a timeout.
- Clock::pause();
- Clock::settle();
- Clock::resume();
-
+ // Ensure we catch a brand new `SlaveRegisteredMessage` message
+ // and not the one eventually resent for the first agent.
Future<SlaveRegisteredMessage> slaveRegisteredMessage2 =
- FUTURE_PROTOBUF(SlaveRegisteredMessage(), master.get()->pid, _);
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, Not(slave.get()->pid));
slave::Flags slaveFlags2 = CreateSlaveFlags();
http://git-wip-us.apache.org/repos/asf/mesos/blob/415451ee/src/tests/partition_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/partition_tests.cpp b/src/tests/partition_tests.cpp
index 390b7c8..788d3b9 100644
--- a/src/tests/partition_tests.cpp
+++ b/src/tests/partition_tests.cpp
@@ -73,6 +73,7 @@ using testing::AllOf;
using testing::AtMost;
using testing::DoAll;
using testing::Eq;
+using testing::Not;
using testing::Return;
using testing::SaveArg;
@@ -2971,6 +2972,10 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcByCount)
AWAIT_READY(slaveLost1);
+ // Set the expectation before resetting `slave1`.
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage2 =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, Not(slave1.get()->pid));
+
// Shutdown the first slave. This is necessary because we only drop
// PONG messages; after advancing the clock below, the slave would
// try to reregister and would succeed. Hence, stop the slave first.
@@ -2978,9 +2983,6 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcByCount)
slave1->reset();
// Start another slave.
- Future<SlaveRegisteredMessage> slaveRegisteredMessage2 =
- FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
-
slave::Flags agentFlags2 = CreateSlaveFlags();
Owned<MasterDetector> slaveDetector2 = master.get()->createDetector();
Try<Owned<cluster::Slave>> slave2 =
@@ -3323,6 +3325,10 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcByAge)
AWAIT_READY(slaveLost1);
+ // Set the expectation before resetting `slave1`.
+ Future<SlaveRegisteredMessage> slaveRegisteredMessage2 =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, Not(slave1.get()->pid));
+
// Shutdown the first slave. This is necessary because we only drop
// PONG messages; after advancing the clock below, the slave would
// try to reregister and would succeed. Hence, stop the slave first.
@@ -3341,9 +3347,6 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcByAge)
agentFlags2.registration_backoff_factor);
// Start another slave.
- Future<SlaveRegisteredMessage> slaveRegisteredMessage2 =
- FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
-
Owned<MasterDetector> slaveDetector2 = master.get()->createDetector();
Try<Owned<cluster::Slave>> slave2 = StartSlave(slaveDetector2.get(),
agentFlags2);
@@ -3582,7 +3585,7 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcRace)
DROP_MESSAGES(_, slave1.get()->pid, _);
Future<SlaveRegisteredMessage> slaveRegisteredMessage2 =
- FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, Not(slave1.get()->pid));
StandaloneMasterDetector detector2(master.get()->pid);
slave::Flags agentFlags2 = CreateSlaveFlags();
@@ -3622,7 +3625,9 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, RegistryGcRace)
master.get()->pid);
Future<SlaveRegisteredMessage> slaveRegisteredMessage3 =
- FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, AllOf(
+ Not(slave1.get()->pid),
+ Not(slave2.get()->pid)));
Owned<MasterDetector> detector3 = master.get()->createDetector();
http://git-wip-us.apache.org/repos/asf/mesos/blob/415451ee/src/tests/persistent_volume_endpoints_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/persistent_volume_endpoints_tests.cpp b/src/tests/persistent_volume_endpoints_tests.cpp
index 770c482..ab2680f 100644
--- a/src/tests/persistent_volume_endpoints_tests.cpp
+++ b/src/tests/persistent_volume_endpoints_tests.cpp
@@ -1807,8 +1807,7 @@ TEST_F(PersistentVolumeEndpointsTest, ReserveAndSlaveRemoval)
// Capture the registration message for the second agent.
Future<SlaveRegisteredMessage> slave2RegisteredMessage =
- FUTURE_PROTOBUF(
- SlaveRegisteredMessage(), master.get()->pid, Not(slave1.get()->pid));
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, Not(slave1.get()->pid));
// Each slave needs its own flags to ensure work_dirs are unique.
slave::Flags slave2Flags = CreateSlaveFlags();
http://git-wip-us.apache.org/repos/asf/mesos/blob/415451ee/src/tests/storage_local_resource_provider_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/storage_local_resource_provider_tests.cpp b/src/tests/storage_local_resource_provider_tests.cpp
index 45cb389..04a75fc 100644
--- a/src/tests/storage_local_resource_provider_tests.cpp
+++ b/src/tests/storage_local_resource_provider_tests.cpp
@@ -59,6 +59,7 @@ using process::Owned;
using testing::AtMost;
using testing::DoAll;
+using testing::Not;
using testing::Sequence;
namespace mesos {
@@ -1145,7 +1146,8 @@ TEST_F(StorageLocalResourceProviderTest, ROOT_AgentRegisteredWithNewId)
ASSERT_SOME(os::rm(slave::paths::getLatestSlavePath(metaDir)));
// A new registration would trigger another `SlaveRegisteredMessage`.
- slaveRegisteredMessage = FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+ slaveRegisteredMessage =
+ FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, Not(slave.get()->pid));
// After the agent fails over, any volume created before becomes a
// pre-existing volume, which has an ID but no profile.