You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by John Sirois <jo...@gmail.com> on 2012/07/10 01:52:31 UTC

Review Request: Handle secured parent-dirs in Group

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5869/
-----------------------------------------------------------

Review request for mesos and Benjamin Hindman.


Description
-------

commit 9c8316e84ec048df8d866fb50ebdf78d0e337d66
Author: jsirois <js...@twitter.com>
Date:   Mon Jul 9 17:45:30 2012 -0600

    Handle secured parent-dirs in Group
    
    Modify Group to work with znodes that have read-only parent nodes
    and only require the Group znode leaf itself is writable.

 src/tests/zookeeper_tests.cpp | 40 ++++++++++++++++++++++++++++++++++++++++
 src/zookeeper/group.cpp       | 24 +++++++++++++++++++++++-
 2 files changed, 63 insertions(+), 1 deletion(-)


This addresses bug MESOS-229.
    https://issues.apache.org/jira/browse/MESOS-229


Diffs
-----

  src/tests/zookeeper_tests.cpp d318ce7 
  src/zookeeper/group.cpp a4a8b5a 

Diff: https://reviews.apache.org/r/5869/diff/


Testing
-------

Before fix with new test case:

$ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
Source directory: /Users/jsirois/development/third_party/mesos
Build directory: /Users/jsirois/development/third_party/mesos/build
Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ZooKeeperTest
[ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
/Users/jsirois/development/third_party/mesos/build/../src/tests/zookeeper_tests.cpp:405: Failure
Value of: membership.isFailed()
  Actual: true
Expected: false
Failed to create '/read-only/writable' in ZooKeeper: not authenticated
[  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms (6346 ms)
[----------] 1 test from ZooKeeperTest (6346 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (6516 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms

 1 FAILED TEST
  YOU HAVE 6 DISABLED TESTS

After fix:
$ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
Source directory: /Users/jsirois/development/third_party/mesos
Build directory: /Users/jsirois/development/third_party/mesos/build
Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ZooKeeperTest
[ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
[       OK ] ZooKeeperTest.GroupPathWithRestrictivePerms (6295 ms)
[----------] 1 test from ZooKeeperTest (6295 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (6469 ms total)
[  PASSED  ] 1 test.

  YOU HAVE 6 DISABLED TESTS


Thanks,

John Sirois


Re: Review Request: Handle secured parent-dirs in Group

Posted by John Sirois <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5869/#review9004
-----------------------------------------------------------


Sorry - I think I git diff head^^ head >> path instead of > and this messed things up with double interdiffs.  The final diff 3 is correct :/

- John Sirois


On July 10, 2012, 1:15 a.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5869/
> -----------------------------------------------------------
> 
> (Updated July 10, 2012, 1:15 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> commit 9c8316e84ec048df8d866fb50ebdf78d0e337d66
> Author: jsirois <js...@twitter.com>
> Date:   Mon Jul 9 17:45:30 2012 -0600
> 
>     Handle secured parent-dirs in Group
>     
>     Modify Group to work with znodes that have read-only parent nodes
>     and only require the Group znode leaf itself is writable.
> 
>  src/tests/zookeeper_tests.cpp | 40 ++++++++++++++++++++++++++++++++++++++++
>  src/zookeeper/group.cpp       | 24 +++++++++++++++++++++++-
>  2 files changed, 63 insertions(+), 1 deletion(-)
> 
> 
> This addresses bug MESOS-229.
>     https://issues.apache.org/jira/browse/MESOS-229
> 
> 
> Diffs
> -----
> 
>   src/tests/zookeeper_tests.cpp d318ce7 
>   src/zookeeper/group.cpp a4a8b5a 
> 
> Diff: https://reviews.apache.org/r/5869/diff/
> 
> 
> Testing
> -------
> 
> Before fix with new test case:
> 
> $ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
> Source directory: /Users/jsirois/development/third_party/mesos
> Build directory: /Users/jsirois/development/third_party/mesos/build
> Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ZooKeeperTest
> [ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
> /Users/jsirois/development/third_party/mesos/build/../src/tests/zookeeper_tests.cpp:405: Failure
> Value of: membership.isFailed()
>   Actual: true
> Expected: false
> Failed to create '/read-only/writable' in ZooKeeper: not authenticated
> [  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms (6346 ms)
> [----------] 1 test from ZooKeeperTest (6346 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6516 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms
> 
>  1 FAILED TEST
>   YOU HAVE 6 DISABLED TESTS
> 
> After fix:
> $ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
> Source directory: /Users/jsirois/development/third_party/mesos
> Build directory: /Users/jsirois/development/third_party/mesos/build
> Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ZooKeeperTest
> [ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
> [       OK ] ZooKeeperTest.GroupPathWithRestrictivePerms (6295 ms)
> [----------] 1 test from ZooKeeperTest (6295 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6469 ms total)
> [  PASSED  ] 1 test.
> 
>   YOU HAVE 6 DISABLED TESTS
> 
> 
> Thanks,
> 
> John Sirois
> 
>


Re: Review Request: Handle secured parent-dirs in Group

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5869/#review9039
-----------------------------------------------------------

Ship it!


Thank you much sir! The comments are fantastic.

- Benjamin Hindman


On July 10, 2012, 4:56 a.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5869/
> -----------------------------------------------------------
> 
> (Updated July 10, 2012, 4:56 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> commit 9c8316e84ec048df8d866fb50ebdf78d0e337d66
> Author: jsirois <js...@twitter.com>
> Date:   Mon Jul 9 17:45:30 2012 -0600
> 
>     Handle secured parent-dirs in Group
>     
>     Modify Group to work with znodes that have read-only parent nodes
>     and only require the Group znode leaf itself is writable.
> 
>  src/tests/zookeeper_tests.cpp | 40 ++++++++++++++++++++++++++++++++++++++++
>  src/zookeeper/group.cpp       | 24 +++++++++++++++++++++++-
>  2 files changed, 63 insertions(+), 1 deletion(-)
> 
> 
> This addresses bug MESOS-229.
>     https://issues.apache.org/jira/browse/MESOS-229
> 
> 
> Diffs
> -----
> 
>   src/tests/zookeeper_tests.cpp d318ce7 
>   src/zookeeper/group.cpp a4a8b5a 
> 
> Diff: https://reviews.apache.org/r/5869/diff/
> 
> 
> Testing
> -------
> 
> Before fix with new test case:
> 
> $ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
> Source directory: /Users/jsirois/development/third_party/mesos
> Build directory: /Users/jsirois/development/third_party/mesos/build
> Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ZooKeeperTest
> [ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
> /Users/jsirois/development/third_party/mesos/build/../src/tests/zookeeper_tests.cpp:405: Failure
> Value of: membership.isFailed()
>   Actual: true
> Expected: false
> Failed to create '/read-only/writable' in ZooKeeper: not authenticated
> [  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms (6346 ms)
> [----------] 1 test from ZooKeeperTest (6346 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6516 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms
> 
>  1 FAILED TEST
>   YOU HAVE 6 DISABLED TESTS
> 
> After fix:
> $ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
> Source directory: /Users/jsirois/development/third_party/mesos
> Build directory: /Users/jsirois/development/third_party/mesos/build
> Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ZooKeeperTest
> [ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
> [       OK ] ZooKeeperTest.GroupPathWithRestrictivePerms (6295 ms)
> [----------] 1 test from ZooKeeperTest (6295 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6469 ms total)
> [  PASSED  ] 1 test.
> 
>   YOU HAVE 6 DISABLED TESTS
> 
> 
> Thanks,
> 
> John Sirois
> 
>


Re: Review Request: Handle secured parent-dirs in Group

Posted by John Sirois <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5869/
-----------------------------------------------------------

(Updated July 10, 2012, 4:56 a.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

commit d4121befc2bd99db6516481579f40e1b92a3b118
Author: jsirois <js...@twitter.com>
Date:   Mon Jul 9 22:53:12 2012 -0600

    Add missing include for zookeeper.h - we get ZOO_OPEN_ACL_UNSAFE
    from here in ZooKeeperTest.GroupPathWithRestrictivePerms.
    
    Fixup docs.

 src/tests/zookeeper_tests.cpp | 2 ++
 src/zookeeper/group.cpp       | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)


Description
-------

commit 9c8316e84ec048df8d866fb50ebdf78d0e337d66
Author: jsirois <js...@twitter.com>
Date:   Mon Jul 9 17:45:30 2012 -0600

    Handle secured parent-dirs in Group
    
    Modify Group to work with znodes that have read-only parent nodes
    and only require the Group znode leaf itself is writable.

 src/tests/zookeeper_tests.cpp | 40 ++++++++++++++++++++++++++++++++++++++++
 src/zookeeper/group.cpp       | 24 +++++++++++++++++++++++-
 2 files changed, 63 insertions(+), 1 deletion(-)


This addresses bug MESOS-229.
    https://issues.apache.org/jira/browse/MESOS-229


Diffs (updated)
-----

  src/tests/zookeeper_tests.cpp d318ce7 
  src/zookeeper/group.cpp a4a8b5a 

Diff: https://reviews.apache.org/r/5869/diff/


Testing
-------

Before fix with new test case:

$ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
Source directory: /Users/jsirois/development/third_party/mesos
Build directory: /Users/jsirois/development/third_party/mesos/build
Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ZooKeeperTest
[ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
/Users/jsirois/development/third_party/mesos/build/../src/tests/zookeeper_tests.cpp:405: Failure
Value of: membership.isFailed()
  Actual: true
Expected: false
Failed to create '/read-only/writable' in ZooKeeper: not authenticated
[  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms (6346 ms)
[----------] 1 test from ZooKeeperTest (6346 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (6516 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms

 1 FAILED TEST
  YOU HAVE 6 DISABLED TESTS

After fix:
$ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
Source directory: /Users/jsirois/development/third_party/mesos
Build directory: /Users/jsirois/development/third_party/mesos/build
Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ZooKeeperTest
[ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
[       OK ] ZooKeeperTest.GroupPathWithRestrictivePerms (6295 ms)
[----------] 1 test from ZooKeeperTest (6295 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (6469 ms total)
[  PASSED  ] 1 test.

  YOU HAVE 6 DISABLED TESTS


Thanks,

John Sirois


Re: Review Request: Handle secured parent-dirs in Group

Posted by John Sirois <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5869/
-----------------------------------------------------------

(Updated July 10, 2012, 1:15 a.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

commit 3fc4a6d432fcdc842db69dec2a1047cb215abd23
Author: jsirois <js...@twitter.com>
Date:   Mon Jul 9 19:10:42 2012 -0600

    Add acomment explaining skipping NOAUTH errors when creating
    a Group's znode path.
    
    Fixup column overflow.

 src/tests/zookeeper_tests.cpp |  1 +
 src/zookeeper/group.cpp       | 13 ++++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)


Description
-------

commit 9c8316e84ec048df8d866fb50ebdf78d0e337d66
Author: jsirois <js...@twitter.com>
Date:   Mon Jul 9 17:45:30 2012 -0600

    Handle secured parent-dirs in Group
    
    Modify Group to work with znodes that have read-only parent nodes
    and only require the Group znode leaf itself is writable.

 src/tests/zookeeper_tests.cpp | 40 ++++++++++++++++++++++++++++++++++++++++
 src/zookeeper/group.cpp       | 24 +++++++++++++++++++++++-
 2 files changed, 63 insertions(+), 1 deletion(-)


This addresses bug MESOS-229.
    https://issues.apache.org/jira/browse/MESOS-229


Diffs (updated)
-----

  src/tests/zookeeper_tests.cpp d318ce7 
  src/zookeeper/group.cpp a4a8b5a 

Diff: https://reviews.apache.org/r/5869/diff/


Testing
-------

Before fix with new test case:

$ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
Source directory: /Users/jsirois/development/third_party/mesos
Build directory: /Users/jsirois/development/third_party/mesos/build
Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ZooKeeperTest
[ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
/Users/jsirois/development/third_party/mesos/build/../src/tests/zookeeper_tests.cpp:405: Failure
Value of: membership.isFailed()
  Actual: true
Expected: false
Failed to create '/read-only/writable' in ZooKeeper: not authenticated
[  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms (6346 ms)
[----------] 1 test from ZooKeeperTest (6346 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (6516 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms

 1 FAILED TEST
  YOU HAVE 6 DISABLED TESTS

After fix:
$ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
Source directory: /Users/jsirois/development/third_party/mesos
Build directory: /Users/jsirois/development/third_party/mesos/build
Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ZooKeeperTest
[ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
[       OK ] ZooKeeperTest.GroupPathWithRestrictivePerms (6295 ms)
[----------] 1 test from ZooKeeperTest (6295 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (6469 ms total)
[  PASSED  ] 1 test.

  YOU HAVE 6 DISABLED TESTS


Thanks,

John Sirois


Re: Review Request: Handle secured parent-dirs in Group

Posted by John Sirois <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5869/
-----------------------------------------------------------

(Updated July 10, 2012, 12:25 a.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

commit 6e7afc2d65f9aea090265fbc08bcec93b7030f36
Author: jsirois <js...@twitter.com>
Date:   Mon Jul 9 18:23:07 2012 -0600

    Add a test for a Group path creation that fails due to no-auth.

 src/tests/zookeeper_tests.cpp | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)


Description
-------

commit 9c8316e84ec048df8d866fb50ebdf78d0e337d66
Author: jsirois <js...@twitter.com>
Date:   Mon Jul 9 17:45:30 2012 -0600

    Handle secured parent-dirs in Group
    
    Modify Group to work with znodes that have read-only parent nodes
    and only require the Group znode leaf itself is writable.

 src/tests/zookeeper_tests.cpp | 40 ++++++++++++++++++++++++++++++++++++++++
 src/zookeeper/group.cpp       | 24 +++++++++++++++++++++++-
 2 files changed, 63 insertions(+), 1 deletion(-)


This addresses bug MESOS-229.
    https://issues.apache.org/jira/browse/MESOS-229


Diffs (updated)
-----

  src/tests/zookeeper_tests.cpp d318ce7 
  src/tests/zookeeper_tests.cpp d318ce7 
  src/zookeeper/group.cpp a4a8b5a 
  src/zookeeper/group.cpp a4a8b5a 

Diff: https://reviews.apache.org/r/5869/diff/


Testing
-------

Before fix with new test case:

$ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
Source directory: /Users/jsirois/development/third_party/mesos
Build directory: /Users/jsirois/development/third_party/mesos/build
Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ZooKeeperTest
[ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
/Users/jsirois/development/third_party/mesos/build/../src/tests/zookeeper_tests.cpp:405: Failure
Value of: membership.isFailed()
  Actual: true
Expected: false
Failed to create '/read-only/writable' in ZooKeeper: not authenticated
[  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms (6346 ms)
[----------] 1 test from ZooKeeperTest (6346 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (6516 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms

 1 FAILED TEST
  YOU HAVE 6 DISABLED TESTS

After fix:
$ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
Source directory: /Users/jsirois/development/third_party/mesos
Build directory: /Users/jsirois/development/third_party/mesos/build
Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ZooKeeperTest
[ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
[       OK ] ZooKeeperTest.GroupPathWithRestrictivePerms (6295 ms)
[----------] 1 test from ZooKeeperTest (6295 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (6469 ms total)
[  PASSED  ] 1 test.

  YOU HAVE 6 DISABLED TESTS


Thanks,

John Sirois


Re: Review Request: Handle secured parent-dirs in Group

Posted by John Sirois <jo...@gmail.com>.

> On July 10, 2012, 12:19 a.m., Benjamin Hindman wrote:
> > src/tests/zookeeper_tests.cpp, line 369
> > <https://reviews.apache.org/r/5869/diff/1/?file=121061#file121061line369>
> >
> >     Newline.

fixed


> On July 10, 2012, 12:19 a.m., Benjamin Hindman wrote:
> > src/zookeeper/group.cpp, line 414
> > <https://reviews.apache.org/r/5869/diff/1/?file=121062#file121062line414>
> >
> >     Why ignore the ZNOAUTH? What's the case in which we get ZNOAUTH as a result but then can actually create the ZNODE below? Maybe this just needs some more comments?

Added comments.  Could separate another elseif and do a get to verify exists but we can't write, but the idea was its simplest to just continue on and find out in the new final write check.


> On July 10, 2012, 12:19 a.m., Benjamin Hindman wrote:
> > src/zookeeper/group.cpp, line 426
> > <https://reviews.apache.org/r/5869/diff/1/?file=121062#file121062line426>
> >
> >     Please wrap for 80 characters. ;)

oops - fixed


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5869/#review8996
-----------------------------------------------------------


On July 10, 2012, 1:15 a.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5869/
> -----------------------------------------------------------
> 
> (Updated July 10, 2012, 1:15 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> commit 9c8316e84ec048df8d866fb50ebdf78d0e337d66
> Author: jsirois <js...@twitter.com>
> Date:   Mon Jul 9 17:45:30 2012 -0600
> 
>     Handle secured parent-dirs in Group
>     
>     Modify Group to work with znodes that have read-only parent nodes
>     and only require the Group znode leaf itself is writable.
> 
>  src/tests/zookeeper_tests.cpp | 40 ++++++++++++++++++++++++++++++++++++++++
>  src/zookeeper/group.cpp       | 24 +++++++++++++++++++++++-
>  2 files changed, 63 insertions(+), 1 deletion(-)
> 
> 
> This addresses bug MESOS-229.
>     https://issues.apache.org/jira/browse/MESOS-229
> 
> 
> Diffs
> -----
> 
>   src/tests/zookeeper_tests.cpp d318ce7 
>   src/zookeeper/group.cpp a4a8b5a 
> 
> Diff: https://reviews.apache.org/r/5869/diff/
> 
> 
> Testing
> -------
> 
> Before fix with new test case:
> 
> $ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
> Source directory: /Users/jsirois/development/third_party/mesos
> Build directory: /Users/jsirois/development/third_party/mesos/build
> Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ZooKeeperTest
> [ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
> /Users/jsirois/development/third_party/mesos/build/../src/tests/zookeeper_tests.cpp:405: Failure
> Value of: membership.isFailed()
>   Actual: true
> Expected: false
> Failed to create '/read-only/writable' in ZooKeeper: not authenticated
> [  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms (6346 ms)
> [----------] 1 test from ZooKeeperTest (6346 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6516 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms
> 
>  1 FAILED TEST
>   YOU HAVE 6 DISABLED TESTS
> 
> After fix:
> $ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
> Source directory: /Users/jsirois/development/third_party/mesos
> Build directory: /Users/jsirois/development/third_party/mesos/build
> Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ZooKeeperTest
> [ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
> [       OK ] ZooKeeperTest.GroupPathWithRestrictivePerms (6295 ms)
> [----------] 1 test from ZooKeeperTest (6295 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6469 ms total)
> [  PASSED  ] 1 test.
> 
>   YOU HAVE 6 DISABLED TESTS
> 
> 
> Thanks,
> 
> John Sirois
> 
>


Re: Review Request: Handle secured parent-dirs in Group

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5869/#review8996
-----------------------------------------------------------



src/tests/zookeeper_tests.cpp
<https://reviews.apache.org/r/5869/#comment19128>

    Newline.



src/zookeeper/group.cpp
<https://reviews.apache.org/r/5869/#comment19129>

    Why ignore the ZNOAUTH? What's the case in which we get ZNOAUTH as a result but then can actually create the ZNODE below? Maybe this just needs some more comments?



src/zookeeper/group.cpp
<https://reviews.apache.org/r/5869/#comment19130>

    Please wrap for 80 characters. ;)


- Benjamin Hindman


On July 9, 2012, 11:52 p.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5869/
> -----------------------------------------------------------
> 
> (Updated July 9, 2012, 11:52 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> commit 9c8316e84ec048df8d866fb50ebdf78d0e337d66
> Author: jsirois <js...@twitter.com>
> Date:   Mon Jul 9 17:45:30 2012 -0600
> 
>     Handle secured parent-dirs in Group
>     
>     Modify Group to work with znodes that have read-only parent nodes
>     and only require the Group znode leaf itself is writable.
> 
>  src/tests/zookeeper_tests.cpp | 40 ++++++++++++++++++++++++++++++++++++++++
>  src/zookeeper/group.cpp       | 24 +++++++++++++++++++++++-
>  2 files changed, 63 insertions(+), 1 deletion(-)
> 
> 
> This addresses bug MESOS-229.
>     https://issues.apache.org/jira/browse/MESOS-229
> 
> 
> Diffs
> -----
> 
>   src/tests/zookeeper_tests.cpp d318ce7 
>   src/zookeeper/group.cpp a4a8b5a 
> 
> Diff: https://reviews.apache.org/r/5869/diff/
> 
> 
> Testing
> -------
> 
> Before fix with new test case:
> 
> $ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
> Source directory: /Users/jsirois/development/third_party/mesos
> Build directory: /Users/jsirois/development/third_party/mesos/build
> Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ZooKeeperTest
> [ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
> /Users/jsirois/development/third_party/mesos/build/../src/tests/zookeeper_tests.cpp:405: Failure
> Value of: membership.isFailed()
>   Actual: true
> Expected: false
> Failed to create '/read-only/writable' in ZooKeeper: not authenticated
> [  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms (6346 ms)
> [----------] 1 test from ZooKeeperTest (6346 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6516 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] ZooKeeperTest.GroupPathWithRestrictivePerms
> 
>  1 FAILED TEST
>   YOU HAVE 6 DISABLED TESTS
> 
> After fix:
> $ ./bin/mesos-tests.sh --gtest_filter=ZooKeeperTest.GroupPathWithRestrictivePerms
> Source directory: /Users/jsirois/development/third_party/mesos
> Build directory: /Users/jsirois/development/third_party/mesos/build
> Note: Google Test filter = ZooKeeperTest.GroupPathWithRestrictivePerms-
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ZooKeeperTest
> [ RUN      ] ZooKeeperTest.GroupPathWithRestrictivePerms
> [       OK ] ZooKeeperTest.GroupPathWithRestrictivePerms (6295 ms)
> [----------] 1 test from ZooKeeperTest (6295 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6469 ms total)
> [  PASSED  ] 1 test.
> 
>   YOU HAVE 6 DISABLED TESTS
> 
> 
> Thanks,
> 
> John Sirois
> 
>