Added new UpdateSlave registry operation.

This operation can be used to update the stored state
of an existing, admitted slave.

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


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

Branch: refs/heads/master
Commit: 054bd5abc6b9788553cd5beb285bd46bbe2dec0d
Parents: 114519a
Author: Benno Evers <bev...@mesosphere.com>
Authored: Tue Dec 5 13:55:30 2017 -0800
Committer: Vinod Kone <vinodk...@gmail.com>
Committed: Tue Dec 5 13:55:30 2017 -0800

----------------------------------------------------------------------
 src/master/registry_operations.cpp | 44 ++++++++++++++++++++++++++++
 src/master/registry_operations.hpp | 14 +++++++++
 src/tests/registrar_tests.cpp      | 51 +++++++++++++++++++++++++++++++++
 3 files changed, 109 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/054bd5ab/src/master/registry_operations.cpp
----------------------------------------------------------------------
diff --git a/src/master/registry_operations.cpp 
b/src/master/registry_operations.cpp
index f9c2162..149009b 100644
--- a/src/master/registry_operations.cpp
+++ b/src/master/registry_operations.cpp
@@ -51,6 +51,50 @@ Try<bool> AdmitSlave::perform(Registry* registry, 
hashset<SlaveID>* slaveIDs)
 }
 
 
+UpdateSlave::UpdateSlave(const SlaveInfo& _info) : info(_info)
+{
+  CHECK(info.has_id()) << "SlaveInfo is missing the 'id' field";
+}
+
+
+Try<bool> UpdateSlave::perform(Registry* registry, hashset<SlaveID>* slaveIDs)
+{
+  if (!slaveIDs->contains(info.id())) {
+    return Error("Agent not yet admitted.");
+  }
+
+  for (int i = 0; i < registry->slaves().slaves().size(); i++) {
+    Registry::Slave* slave = registry->mutable_slaves()->mutable_slaves(i);
+
+    if (slave->info().id() == info.id()) {
+      // The SlaveInfo in the registry is stored in the
+      // `PRE_RESERVATION_REFINEMENT` format, but the equality operator
+      // asserts that resources are in `POST_RESERVATION_REFINEMENT` format,
+      // so we have to upgrade before we can do the comparison.
+      SlaveInfo _previousInfo(slave->info());
+      convertResourceFormat(_previousInfo.mutable_resources(),
+          POST_RESERVATION_REFINEMENT);
+
+      if (info == _previousInfo) {
+        return false; // No mutation.
+      }
+
+      // Convert the resource format back to `PRE_RESERVATION_REFINEMENT` so
+      // the data stored in the registry can be read by older master versions.
+      SlaveInfo _info(info);
+      convertResourceFormat(_info.mutable_resources(),
+          PRE_RESERVATION_REFINEMENT);
+
+      slave->mutable_info()->CopyFrom(_info);
+      return true; // Mutation.
+    }
+  }
+
+  // Shouldn't happen
+  return Error("Failed to find agent " + stringify(info.id()));
+}
+
+
 // Move a slave from the list of admitted slaves to the list of
 // unreachable slaves.
 MarkSlaveUnreachable::MarkSlaveUnreachable(

http://git-wip-us.apache.org/repos/asf/mesos/blob/054bd5ab/src/master/registry_operations.hpp
----------------------------------------------------------------------
diff --git a/src/master/registry_operations.hpp 
b/src/master/registry_operations.hpp
index 5b78306..06f68a3 100644
--- a/src/master/registry_operations.hpp
+++ b/src/master/registry_operations.hpp
@@ -41,6 +41,20 @@ private:
 };
 
 
+// Update the SlaveInfo of an existing admitted slave.
+class UpdateSlave : public Operation
+{
+public:
+  explicit UpdateSlave(const SlaveInfo& _info);
+
+protected:
+  virtual Try<bool> perform(Registry* registry, hashset<SlaveID>* slaveIDs);
+
+private:
+  const SlaveInfo info;
+};
+
+
 // Move a slave from the list of admitted slaves to the list of
 // unreachable slaves.
 class MarkSlaveUnreachable : public Operation

http://git-wip-us.apache.org/repos/asf/mesos/blob/054bd5ab/src/tests/registrar_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/registrar_tests.cpp b/src/tests/registrar_tests.cpp
index 21d78e9..f93129f 100644
--- a/src/tests/registrar_tests.cpp
+++ b/src/tests/registrar_tests.cpp
@@ -265,6 +265,57 @@ TEST_F(RegistrarTest, Admit)
 }
 
 
+TEST_F(RegistrarTest, UpdateSlave)
+{
+  // Add a new slave to the registry.
+  {
+    Registrar registrar(flags, state);
+    AWAIT_READY(registrar.recover(master));
+
+    slave.set_hostname("original");
+    AWAIT_TRUE(
+        registrar.apply(Owned<Operation>(new AdmitSlave(slave))));
+  }
+
+
+  // Verify that the slave is present, and update its hostname.
+  {
+    Registrar registrar(flags, state);
+    Future<Registry> registry = registrar.recover(master);
+    AWAIT_READY(registry);
+
+    ASSERT_EQ(1, registry->slaves().slaves().size());
+    EXPECT_EQ("original", registry->slaves().slaves(0).info().hostname());
+
+    slave.set_hostname("changed");
+    AWAIT_TRUE(registrar.apply(Owned<Operation>(new UpdateSlave(slave))));
+  }
+
+  // Verify that the hostname indeed changed, and do one additional update
+  // to check that the operation is idempotent.
+  {
+    Registrar registrar(flags, state);
+    Future<Registry> registry = registrar.recover(master);
+    AWAIT_READY(registry);
+
+    ASSERT_EQ(1, registry->slaves().slaves().size());
+    EXPECT_EQ("changed", registry->slaves().slaves(0).info().hostname());
+
+    AWAIT_TRUE(registrar.apply(Owned<Operation>(new UpdateSlave(slave))));
+  }
+
+  // Verify that nothing changed from the second update.
+  {
+    Registrar registrar(flags, state);
+    Future<Registry> registry = registrar.recover(master);
+    AWAIT_READY(registry);
+
+    ASSERT_EQ(1, registry->slaves().slaves().size());
+    EXPECT_EQ("changed", registry->slaves().slaves(0).info().hostname());
+  }
+}
+
+
 TEST_F(RegistrarTest, MarkReachable)
 {
   Registrar registrar(flags, state);

Reply via email to