----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14292/#review26502 -----------------------------------------------------------
src/master/master.hpp <https://reviews.apache.org/r/14292/#comment51677> const &? src/master/master.cpp <https://reviews.apache.org/r/14292/#comment51679> The dispatch is authenticate would need to pass the pid here, since 'from' could have changed! src/master/master.cpp <https://reviews.apache.org/r/14292/#comment51678> s/Cancel it/Try to cancel it/ Can you add a comment about what happens when the authenticating future is ready? (E.g. If the Future was Ready at this point, the pending _authenticate could result in a successful authentication for a previous Authenticatee and a new Authenticatee will be created on the client side to retry?). src/master/master.cpp <https://reviews.apache.org/r/14292/#comment51680> Should we be retrying in the Master? Seems like the retry logic should rest solely in the client (if the client is down, it looks like this will retry forever). Be sure to update the "retry" comments if you think we should remove this bit. src/sched/sched.cpp <https://reviews.apache.org/r/14292/#comment51683> Can you add a comment describing what we discussed for when the future was already ready at this point (say, if two newMasterDetected calls occurred in quick succession)? // Authentication is in progress, try to cancel it. authenticating.get().discard(); // If authenticating was already ready, this means there is a pending dispatch to _authenticate. This call will consider authentication successful even though we may not be authenticated (say, if two masters were elected in quick succession). In this case, the driver will proceed with registration but will receive a framework error and will exit as a result. This is a sufficiently rare race condition so it's not worth complicating the code here to handle it. return; src/sched/sched.cpp <https://reviews.apache.org/r/14292/#comment51684> Do we want to add a shared initialize(...) function? It looks like the only difference between the two constructors is the credential option passed to the SchedulerProcess? - Ben Mahler On Sept. 30, 2013, 2:46 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14292/ > ----------------------------------------------------------- > > (Updated Sept. 30, 2013, 2:46 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-704 > https://issues.apache.org/jira/browse/MESOS-704 > > > Repository: mesos-git > > > Description > ------- > > Added authentication support for scheduler driver and master. > > > Diffs > ----- > > include/mesos/mesos.proto 957576bbc1c73513a9591194d017f76fe562a616 > include/mesos/scheduler.hpp cf3ecdaaf40fd878a80fe0b6f7e61a0997329cbd > src/Makefile.am ee336130ad93d8b524c841f75be36f00d4a2b147 > src/common/type_utils.hpp 674a8820c339c6446dfa7d444457477ab4512e79 > src/java/jni/construct.cpp b01bd7ae2eda2dc5e0dcd68848c65bd9f9ea81f0 > src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp > 6d2a03b6a88e71ac4e2e2d1ee8e15925e393ef3d > src/java/src/org/apache/mesos/MesosSchedulerDriver.java > 7ef1fe7755286bf92b94d7ece4f72d54e5b57a84 > src/master/flags.hpp d59e67d5b2799d6d7a37e9cfe7246ae7372091ac > src/master/master.hpp bd5cb1ff05967518d7dc7f3a9dc0d32c22eb9643 > src/master/master.cpp a49b17ef43fca5b385a89731ca8776a26b61399a > src/python/native/mesos_scheduler_driver_impl.cpp > f25d41d38caf2701813dbec0d342a3b327e9dedf > src/sasl/authenticator.hpp 2f78cf0fdd97f0ddc3a6ebd162e6559497d708e4 > src/sched/sched.cpp c399f2481259683a8e178abb3478307042292f23 > src/tests/allocator_tests.cpp c57da6eb3c431b47468b6a6941c3de06af9209e5 > src/tests/allocator_zookeeper_tests.cpp > 6e3214c15c14dc8ba82082738c172c8833cd0887 > src/tests/authentication_tests.cpp PRE-CREATION > src/tests/exception_tests.cpp 3fc1ac32d553644080a88f04f22077691ae1820b > src/tests/fault_tolerance_tests.cpp > 10e52c401476eb8416361de49b8e4061bb7ac4f3 > src/tests/gc_tests.cpp e404de3bfacfcac5515995f1b45c3d39181e138f > src/tests/isolator_tests.cpp cd3b3000060b379ef10e38a2a98a2eebe69d90fc > src/tests/master_detector_tests.cpp > 2d140ba1a364a7af4d643951d6016ac17dd10526 > src/tests/master_tests.cpp 52f09d4f1ddeabcc1a797a13fae9641b72425dd5 > src/tests/mesos.hpp 8fbd56c8dd438a08673b54630bfe25d60ad5ee0e > src/tests/mesos.cpp 776cb0f13d10b4ae437fe9a3c97dc8b3481290af > src/tests/resource_offers_tests.cpp > 3888e461de5c8fa807cff2fd2bd7ca12c704823a > src/tests/slave_recovery_tests.cpp 48b2e6380a9ae688291992f3bf25c3cc473bc808 > src/tests/status_update_manager_tests.cpp > cf420e4764356402f05b27c3b8e8802c21a58f8e > > Diff: https://reviews.apache.org/r/14292/diff/ > > > Testing > ------- > > OSX: > make check > ./bin/mesos-tests.sh --gtest_filter="*FaultTolerance*" --gtest_repeat=1000 > --gtest_break_on_failure --gtest_shuffle > > Linux: > make check GTEST_FILTER='' > > Couldn't run tests on linux because the boxes I've access to do not support > SASL :/ It all compiles fwiw. > > All our tests do auth now. yay! > > > Thanks, > > Vinod Kone > >
