svn commit: r1359937 - in /incubator/mesos/trunk/src: tests/zookeeper_tests.cpp zookeeper/group.cpp

[be...@apache.org]

Author: benh
Date: Tue Jul 10 22:29:25 2012
New Revision: 1359937

URL: http://svn.apache.org/viewvc?rev=1359937&view=rev
Log:
Add functionality to create a ZooKeeper "group" even when some parent
znodes in the specified ZooKeeper path are not readable/writable using
the authentication provided but the final parent znode allows it
(contributed by John Sirois, https://reviews.apache.org/r/5869).

Modified:
    incubator/mesos/trunk/src/tests/zookeeper_tests.cpp
    incubator/mesos/trunk/src/zookeeper/group.cpp

Modified: incubator/mesos/trunk/src/tests/zookeeper_tests.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/tests/zookeeper_tests.cpp?rev=1359937&r1=1359936&r2=1359937&view=diff
==============================================================================
--- incubator/mesos/trunk/src/tests/zookeeper_tests.cpp (original)
+++ incubator/mesos/trunk/src/tests/zookeeper_tests.cpp Tue Jul 10 22:29:25 2012
@@ -16,6 +16,8 @@
  * limitations under the License.
  */
 
+#include <zookeeper.h>
+
 #include <string>
 
 #include <gtest/gtest.h>
@@ -366,3 +368,51 @@ TEST_F(ZooKeeperTest, MultipleGroups)
   ASSERT_TRUE(cancelled.isReady());
   ASSERT_FALSE(cancelled.get());
 }
+
+
+TEST_F(ZooKeeperTest, GroupPathWithRestrictivePerms)
+{
+  mesos::internal::test::BaseZooKeeperTest::TestWatcher watcher;
+
+  ZooKeeper authenticatedZk(zks->connectString(), NO_TIMEOUT, &watcher);
+  watcher.awaitSessionEvent(ZOO_CONNECTED_STATE);
+  authenticatedZk.authenticate("digest", "creator:creator");
+  authenticatedZk.create("/read-only",
+                         "42",
+                         zookeeper::EVERYONE_READ_CREATOR_ALL,
+                         0,
+                         NULL);
+  assertGet(&authenticatedZk, "/read-only", "42");
+  authenticatedZk.create("/read-only/writable",
+                         "37",
+                         ZOO_OPEN_ACL_UNSAFE,
+                         0,
+                         NULL);
+  assertGet(&authenticatedZk, "/read-only/writable", "37");
+
+  zookeeper::Authentication auth("digest", "non-creator:non-creator");
+
+  zookeeper::Group failedGroup1(zks->connectString(), NO_TIMEOUT,
+                                "/read-only/", auth);
+  process::Future<zookeeper::Group::Membership> failedMembership1 =
+    failedGroup1.join("fail");
+  failedMembership1.await();
+  ASSERT_TRUE(failedMembership1.isFailed());
+
+  zookeeper::Group failedGroup2(zks->connectString(), NO_TIMEOUT,
+                                "/read-only/new", auth);
+  process::Future<zookeeper::Group::Membership> failedMembership2 =
+    failedGroup2.join("fail");
+  failedMembership2.await();
+  ASSERT_TRUE(failedMembership2.isFailed());
+
+  zookeeper::Group successGroup(zks->connectString(), NO_TIMEOUT,
+                                "/read-only/writable/", auth);
+  process::Future<zookeeper::Group::Membership> successMembership =
+    successGroup.join("succeed");
+  successMembership.await();
+
+  ASSERT_FALSE(successMembership.isFailed()) << successMembership.failure();
+  ASSERT_FALSE(successMembership.isDiscarded());
+  ASSERT_TRUE(successMembership.isReady());
+}

Modified: incubator/mesos/trunk/src/zookeeper/group.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/zookeeper/group.cpp?rev=1359937&r1=1359936&r2=1359937&view=diff
==============================================================================
--- incubator/mesos/trunk/src/zookeeper/group.cpp (original)
+++ incubator/mesos/trunk/src/zookeeper/group.cpp Tue Jul 10 22:29:25 2012
@@ -411,7 +411,17 @@ void GroupProcess::connected(bool reconn
       if (code == ZINVALIDSTATE || (code != ZOK && zk->retryable(code))) {
         CHECK(zk->getState() != ZOO_AUTH_FAILED_STATE);
         return; // Try again later.
-      } else if (code != ZOK && code != ZNODEEXISTS) {
+      } else if (code != ZOK && code != ZNODEEXISTS && code != ZNOAUTH) {
+        // We fail all non-OK return codes except for ZNODEEXISTS and ZNOAUTH:
+        // ZNODEEXISTS says the node in the znode path we are trying to create
+        //   already exists - this is what we wanted, so we continue.
+        // ZNOAUTH says we can't write the node, but it doesn't tell us
+        //   whether the node already exists.  We take the optimistic approach
+        //   and assume the node's parent doesn't allow us to write an already
+        //   existing node.  As long as the last node in the znode path exists
+        //   (or we can create it) and we can write children to that last node,
+        //   we're good.  This condition is tested below when we're done trying
+        //   to create the znode path.
         Try<string> message = strings::format(
             "Failed to create '%s' in ZooKeeper: %s",
             prefix, zk->message(code));
@@ -422,6 +432,29 @@ void GroupProcess::connected(bool reconn
         return;
       }
     }
+
+    // Now check we have perms to write to the final znode - this is required
+    // to run the Group.
+    string result;
+    int code = zk->create(znode + "/__write_test_", "", acl,
+                          ZOO_SEQUENCE | ZOO_EPHEMERAL, &result);
+    if (code == ZINVALIDSTATE || (code != ZOK && zk->retryable(code))) {
+      CHECK(zk->getState() != ZOO_AUTH_FAILED_STATE);
+      return; // Try again later.
+    } else if (code != ZOK) {
+      Try<string> message = strings::format(
+          "Unable to write to configured group path '%s' in ZooKeeper: %s",
+          znode.c_str(), zk->message(code));
+      error = message.isSome()
+        ? message.get()
+        : "Failed to create node in ZooKeeper";
+      abort(); // Cancels everything pending.
+      return;
+    }
+
+    // Make a best-effort only attempt to clean up our write test node -
+    // it will die with our session if the attempts fails.
+    zk->remove(result, -1);
   }
 
   state = CONNECTED;