You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ch...@apache.org on 2018/03/29 05:07:38 UTC

[3/4] mesos git commit: Added tests for agent resource provider API idempotency.

Added tests for agent resource provider API idempotency.

This patche adds tests to verify that adding, updating and removing the
same resource provider config will return 200 OK without triggering any
resource provider launch/termination.

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


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

Branch: refs/heads/master
Commit: 1635171bc0e9c58c262592d89de715159c69919d
Parents: 667d043
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
Authored: Tue Mar 27 20:01:14 2018 -0700
Committer: Chun-Hung Hsiao <ch...@mesosphere.io>
Committed: Wed Mar 28 19:08:21 2018 -0700

----------------------------------------------------------------------
 ...agent_resource_provider_config_api_tests.cpp | 146 ++++++++++++++++++-
 1 file changed, 142 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1635171b/src/tests/agent_resource_provider_config_api_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/agent_resource_provider_config_api_tests.cpp b/src/tests/agent_resource_provider_config_api_tests.cpp
index 53b8cb8..6ced582 100644
--- a/src/tests/agent_resource_provider_config_api_tests.cpp
+++ b/src/tests/agent_resource_provider_config_api_tests.cpp
@@ -16,6 +16,7 @@
 
 #include <mesos/type_utils.hpp>
 
+#include <process/clock.hpp>
 #include <process/gtest.hpp>
 #include <process/gmock.hpp>
 
@@ -47,6 +48,7 @@ using mesos::internal::slave::Slave;
 
 using mesos::master::detector::MasterDetector;
 
+using process::Clock;
 using process::Future;
 using process::Owned;
 using process::PID;
@@ -365,6 +367,74 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Add)
 }
 
 
+// This test checks that adding a resource provider config that is identical to
+// an existing one is allowed due to idempotency.
+TEST_P(AgentResourceProviderConfigApiTest, ROOT_IdempotentAdd)
+{
+  const ContentType contentType = GetParam();
+
+  Clock::pause();
+
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+
+  slave::Flags slaveFlags = CreateSlaveFlags();
+  slaveFlags.isolation = "filesystem/linux";
+
+  // Disable HTTP authentication to simplify resource provider interactions.
+  slaveFlags.authenticate_http_readwrite = false;
+
+  slaveFlags.resource_provider_config_dir = resourceProviderConfigDir;
+
+  // Generate a pre-existing config.
+  const string configPath = path::join(resourceProviderConfigDir, "test.json");
+  ResourceProviderInfo info = createResourceProviderInfo("volume1:4GB");
+  ASSERT_SOME(os::write(configPath, stringify(JSON::protobuf(info))));
+
+  // Since the local resource provider daemon is started after the agent is
+  // registered, it is guaranteed that the slave will send two
+  // `UpdateSlaveMessage`s, where the latter one contains resources from the
+  // storage local resource provider.
+  // NOTE: The order of the two `FUTURE_PROTOBUF`s are reversed because GMock
+  // will search the expectations in reverse order.
+  Future<UpdateSlaveMessage> updateSlave2 =
+    FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
+  Future<UpdateSlaveMessage> updateSlave1 =
+    FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
+
+  Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), slaveFlags);
+  ASSERT_SOME(slave);
+
+  // Advance the clock to trigger agent registration and prevent retry.
+  Clock::advance(slaveFlags.registration_backoff_factor);
+
+  AWAIT_READY(updateSlave1);
+
+  // NOTE: We need to resume the clock so that the resource provider can
+  // periodically check if the CSI endpoint socket has been created by the
+  // plugin container, which runs in another Linux process.
+  Clock::resume();
+
+  AWAIT_READY(updateSlave2);
+  ASSERT_TRUE(updateSlave2->has_resource_providers());
+  ASSERT_EQ(1, updateSlave2->resource_providers().providers_size());
+
+  Clock::pause();
+
+  // No update message should be triggered by an idempotent add.
+  EXPECT_NO_FUTURE_PROTOBUFS(UpdateSlaveMessage(), _, _);
+
+  // Add a resource provider config that is identical to the existing one.
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(
+      http::OK().status,
+      addResourceProviderConfig(slave.get()->pid, contentType, info));
+
+  Clock::settle();
+}
+
+
 // This test checks that adding a resource provider config that already
 // exists is not allowed.
 TEST_P(AgentResourceProviderConfigApiTest, ROOT_AddConflict)
@@ -542,6 +612,74 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update)
 }
 
 
+// This test checks that updating an existing resource provider config with an
+// identical one will not relaunch the resource provider due to idempotency.
+TEST_P(AgentResourceProviderConfigApiTest, ROOT_IdempotentUpdate)
+{
+  const ContentType contentType = GetParam();
+
+  Clock::pause();
+
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+
+  slave::Flags slaveFlags = CreateSlaveFlags();
+  slaveFlags.isolation = "filesystem/linux";
+
+  // Disable HTTP authentication to simplify resource provider interactions.
+  slaveFlags.authenticate_http_readwrite = false;
+
+  slaveFlags.resource_provider_config_dir = resourceProviderConfigDir;
+
+  // Generate a pre-existing config.
+  const string configPath = path::join(resourceProviderConfigDir, "test.json");
+  ResourceProviderInfo info = createResourceProviderInfo("volume1:4GB");
+  ASSERT_SOME(os::write(configPath, stringify(JSON::protobuf(info))));
+
+  // Since the local resource provider daemon is started after the agent is
+  // registered, it is guaranteed that the slave will send two
+  // `UpdateSlaveMessage`s, where the latter one contains resources from the
+  // storage local resource provider.
+  // NOTE: The order of the two `FUTURE_PROTOBUF`s are reversed because GMock
+  // will search the expectations in reverse order.
+  Future<UpdateSlaveMessage> updateSlave2 =
+    FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
+  Future<UpdateSlaveMessage> updateSlave1 =
+    FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
+
+  Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), slaveFlags);
+  ASSERT_SOME(slave);
+
+  // Advance the clock to trigger agent registration and prevent retry.
+  Clock::advance(slaveFlags.registration_backoff_factor);
+
+  AWAIT_READY(updateSlave1);
+
+  // NOTE: We need to resume the clock so that the resource provider can
+  // periodically check if the CSI endpoint socket has been created by the
+  // plugin container, which runs in another Linux process.
+  Clock::resume();
+
+  AWAIT_READY(updateSlave2);
+  ASSERT_TRUE(updateSlave2->has_resource_providers());
+  ASSERT_EQ(1, updateSlave2->resource_providers().providers_size());
+
+  Clock::pause();
+
+  // No update message should be triggered by an idempotent update.
+  EXPECT_NO_FUTURE_PROTOBUFS(UpdateSlaveMessage(), _, _);
+
+  // Update the resource provider with an identical config.
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(
+      http::OK().status,
+      updateResourceProviderConfig(slave.get()->pid, contentType, info));
+
+  Clock::settle();
+}
+
+
 // This test checks that updating a nonexistent resource provider config
 // is not allowed.
 TEST_P(AgentResourceProviderConfigApiTest, UpdateNotFound)
@@ -666,9 +804,9 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Remove)
 }
 
 
-// This test checks that removing a nonexistent resource provider config
-// is not allowed.
-TEST_P(AgentResourceProviderConfigApiTest, RemoveNotFound)
+// This test checks that removing a nonexistent resource provider config is
+// allowed due to idempotency.
+TEST_P(AgentResourceProviderConfigApiTest, IdempotentRemove)
 {
   const ContentType contentType = GetParam();
 
@@ -695,7 +833,7 @@ TEST_P(AgentResourceProviderConfigApiTest, RemoveNotFound)
   ResourceProviderInfo info = createResourceProviderInfo("volume1:4GB");
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(
-      http::NotFound().status,
+      http::OK().status,
       removeResourceProviderConfig(
           slave.get()->pid, contentType, info.type(), info.name()));
 }