You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2016/01/11 23:40:13 UTC

Review Request 42165: Removed `hasPrincipal` parameter from unreserve validation.

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

Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.


Bugs: MESOS-3940
    https://issues.apache.org/jira/browse/MESOS-3940


Repository: mesos


Description
-------

Removed `hasPrincipal` parameter from unreserve validation.


Diffs
-----

  src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
  src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
  src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
  src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
  src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f 

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


Testing
-------

Several instances of the relevant validation function were altered in the master validation tests.

`make check` was used to test on both OSX and Ubuntu 14.04


Thanks,

Greg Mann


Re: Review Request 42165: Removed `hasPrincipal` parameter from unreserve validation.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42165/#review114569
-----------------------------------------------------------


So after some discussions it sounds like we're going to wait on implementing this patch until we have migrated the principal field in ReservationInfo to optional. So for the time being, HTTP authentication will be required in order to use the /reserve and /unreserve endpoints. There are several options, none of which are ideal, and this seems to be the cleanest solution overall.

- Greg Mann


On Jan. 11, 2016, 10:40 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42165/
> -----------------------------------------------------------
> 
> (Updated Jan. 11, 2016, 10:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
>     https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removed `hasPrincipal` parameter from unreserve validation.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f 
> 
> Diff: https://reviews.apache.org/r/42165/diff/
> 
> 
> Testing
> -------
> 
> Several instances of the relevant validation function were altered in the master validation tests.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 42165: Removed `hasPrincipal` parameter from unreserve validation.

Posted by Guangya Liu <gy...@gmail.com>.

> On 一月 12, 2016, 2:33 a.m., Guangya Liu wrote:
> > Greg, I think that the document of reservation.md should also be updated. 
> > 
> > BTW: Did you test your patch? I did some test as this and mesos-master core dump.
> > 
> > curl -i -d slaveId="4c737d19-e3b2-41c0-b4d0-ffc60190b8eb-S0" -d resources='[ { "name": "cpus",  "type": "SCALAR", "scalar": { "value": 3 }, "role": "ads", "reservation": { "principal": "p1" } }, {  "name": "mem", "type": "SCALAR",  "scalar": { "value": 3000 }, "role": "ads", "reservation": { "principal": "p1" } }  ]'  -X POST http://9.110.49.27:5050/master/reserve
> > curl: (52) Empty reply from server
> > 
> > --
> > I0112 10:26:35.901185 333451264 http.cpp:315] HTTP POST for /master/reserve from 9.110.49.27:50492 with User-Agent='curl/7.43.0'
> > Assertion failed: (isSome()), function get, file ../../3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp, line 108.
> > *** Aborted at 1452565595 (unix time) try "date -d @1452565595" if you are using GNU date ***
> > PC: @     0x7fff8ca15286 __pthread_kill
> > *** SIGABRT (@0x7fff8ca15286) received by PID 43187 (TID 0x113e01000) stack trace: ***
> >     @     0x7fff89a0cf1a _sigtramp
> >     @     0x7f917bd08fd0 (unknown)
> >     @     0x7fff867139ab abort
> >     @     0x7fff866dba91 __assert_rtn
> >     @        0x10c517947 Option<>::get()
> >     @        0x10cf6c0f9 mesos::internal::master::validation::operation::validate()
> >     @        0x10ca5c0c2 mesos::internal::master::Master::Http::reserve()
> >     @        0x10cba6634 mesos::internal::master::Master::initialize()::$_9::operator()()
> >     @        0x10cba65d4 _ZNSt3__128__invoke_void_return_wrapperIN7process6FutureINS1_4http8ResponseEEEE6__callIJRZN5mesos8internal6master6Master10initializeEvE3$_9RKNS3_7RequestERK6OptionINS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEEEEES5_DpOT_
> >     @        0x10cba64a3 std::__1::__function::__func<>::operator()()
> >     @        0x10eacfaa2 std::__1::function<>::operator()()
> >     @        0x10ea0b399 process::ProcessBase::visit()::$_3::operator()()
> >     @        0x10ea0e9f1 _ZZZNK7process9_DeferredIZNS_11ProcessBase5visitERKNS_9HttpEventEE3$_3EcvNSt3__18functionIFvT_EEEIRKNS_6FutureI6OptionINS_4http14authentication20AuthenticationResultEEEEEEvENKUlSL_E_clESL_ENKUlvE_clEv
> >     @        0x10ea0e9bd _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZZNK7process9_DeferredIZNS3_11ProcessBase5visitERKNS3_9HttpEventEE3$_3EcvNS_8functionIFvT_EEEIRKNS3_6FutureI6OptionINS3_4http14authentication20AuthenticationResultEEEEEEvENKUlSO_E_clESO_EUlvE_EEEvDpOT_
> >     @        0x10ea0e6fc _ZNSt3__110__function6__funcIZZNK7process9_DeferredIZNS2_11ProcessBase5visitERKNS2_9HttpEventEE3$_3EcvNS_8functionIFvT_EEEIRKNS2_6FutureI6OptionINS2_4http14authentication20AuthenticationResultEEEEEEvENKUlSN_E_clESN_EUlvE_NS_9allocatorISP_EEFvvEEclEv
> >     @        0x10c510371 std::__1::function<>::operator()()
> >     @        0x10c56f999 _ZZN7process8dispatchERKNS_4UPIDERKNSt3__18functionIFvvEEEENKUlPNS_11ProcessBaseEE_clESA_
> >     @        0x10c56f970 _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZN7process8dispatchERKNS3_4UPIDERKNS_8functionIFvvEEEEUlPNS3_11ProcessBaseEE_SD_EEEvDpOT_
> >     @        0x10c56f70c _ZNSt3__110__function6__funcIZN7process8dispatchERKNS2_4UPIDERKNS_8functionIFvvEEEEUlPNS2_11ProcessBaseEE_NS_9allocatorISD_EEFvSC_EEclEOSC_
> >     @        0x10ea2529f std::__1::function<>::operator()()
> >     @        0x10e9ff2ff process::ProcessBase::visit()
> >     @        0x10ea531be process::DispatchEvent::visit()
> >     @        0x10c4efd71 process::ProcessBase::serve()
> >     @        0x10e9fc091 process::ProcessManager::resume()
> >     @        0x10ea0784f process::ProcessManager::init_threads()::$_1::operator()()
> >     @        0x10ea074d2 _ZNSt3__114__thread_proxyINS_5tupleIJNS_6__bindIZN7process14ProcessManager12init_threadsEvE3$_1JNS_17reference_wrapperIKNS_6atomicIbEEEEEEEEEEEEPvSD_
> >     @     0x7fff8c6e105a _pthread_body
> >     @     0x7fff8c6e0fd7 _pthread_start
> >     @     0x7fff8c6de3ed thread_start
> 
> Greg Mann wrote:
>     Yep, I tested as noted in the "Testing Done" section, but I didn't manually apply an operation as you did - thanks a bunch for doing this Guangya, as you found a mistake that I made! In the parent patch of this one (https://reviews.apache.org/r/42164/) you'll find the fix, as well as a couple new test cases that apply reserve/unreserve operations without a principal. The tests produced a segfault before the fix, and succeeded after the fix.
>     
>     Thanks again!!!
>     
>     I noticed in your `curl` command that you included a principal "p1" in the resources you were attempting to reserve even though no authentication headers were included in the request. Note that this will produce an error given the new changes I introduced, since if no principal is provided with the operation, the `ReservationInfo` should contain an empty principal as well.

Thanks Greg, the new patch works!


- Guangya


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


On 一月 11, 2016, 10:40 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42165/
> -----------------------------------------------------------
> 
> (Updated 一月 11, 2016, 10:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
>     https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removed `hasPrincipal` parameter from unreserve validation.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f 
> 
> Diff: https://reviews.apache.org/r/42165/diff/
> 
> 
> Testing
> -------
> 
> Several instances of the relevant validation function were altered in the master validation tests.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 42165: Removed `hasPrincipal` parameter from unreserve validation.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Jan. 12, 2016, 2:33 a.m., Guangya Liu wrote:
> > Greg, I think that the document of reservation.md should also be updated. 
> > 
> > BTW: Did you test your patch? I did some test as this and mesos-master core dump.
> > 
> > curl -i -d slaveId="4c737d19-e3b2-41c0-b4d0-ffc60190b8eb-S0" -d resources='[ { "name": "cpus",  "type": "SCALAR", "scalar": { "value": 3 }, "role": "ads", "reservation": { "principal": "p1" } }, {  "name": "mem", "type": "SCALAR",  "scalar": { "value": 3000 }, "role": "ads", "reservation": { "principal": "p1" } }  ]'  -X POST http://9.110.49.27:5050/master/reserve
> > curl: (52) Empty reply from server
> > 
> > --
> > I0112 10:26:35.901185 333451264 http.cpp:315] HTTP POST for /master/reserve from 9.110.49.27:50492 with User-Agent='curl/7.43.0'
> > Assertion failed: (isSome()), function get, file ../../3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp, line 108.
> > *** Aborted at 1452565595 (unix time) try "date -d @1452565595" if you are using GNU date ***
> > PC: @     0x7fff8ca15286 __pthread_kill
> > *** SIGABRT (@0x7fff8ca15286) received by PID 43187 (TID 0x113e01000) stack trace: ***
> >     @     0x7fff89a0cf1a _sigtramp
> >     @     0x7f917bd08fd0 (unknown)
> >     @     0x7fff867139ab abort
> >     @     0x7fff866dba91 __assert_rtn
> >     @        0x10c517947 Option<>::get()
> >     @        0x10cf6c0f9 mesos::internal::master::validation::operation::validate()
> >     @        0x10ca5c0c2 mesos::internal::master::Master::Http::reserve()
> >     @        0x10cba6634 mesos::internal::master::Master::initialize()::$_9::operator()()
> >     @        0x10cba65d4 _ZNSt3__128__invoke_void_return_wrapperIN7process6FutureINS1_4http8ResponseEEEE6__callIJRZN5mesos8internal6master6Master10initializeEvE3$_9RKNS3_7RequestERK6OptionINS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEEEEES5_DpOT_
> >     @        0x10cba64a3 std::__1::__function::__func<>::operator()()
> >     @        0x10eacfaa2 std::__1::function<>::operator()()
> >     @        0x10ea0b399 process::ProcessBase::visit()::$_3::operator()()
> >     @        0x10ea0e9f1 _ZZZNK7process9_DeferredIZNS_11ProcessBase5visitERKNS_9HttpEventEE3$_3EcvNSt3__18functionIFvT_EEEIRKNS_6FutureI6OptionINS_4http14authentication20AuthenticationResultEEEEEEvENKUlSL_E_clESL_ENKUlvE_clEv
> >     @        0x10ea0e9bd _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZZNK7process9_DeferredIZNS3_11ProcessBase5visitERKNS3_9HttpEventEE3$_3EcvNS_8functionIFvT_EEEIRKNS3_6FutureI6OptionINS3_4http14authentication20AuthenticationResultEEEEEEvENKUlSO_E_clESO_EUlvE_EEEvDpOT_
> >     @        0x10ea0e6fc _ZNSt3__110__function6__funcIZZNK7process9_DeferredIZNS2_11ProcessBase5visitERKNS2_9HttpEventEE3$_3EcvNS_8functionIFvT_EEEIRKNS2_6FutureI6OptionINS2_4http14authentication20AuthenticationResultEEEEEEvENKUlSN_E_clESN_EUlvE_NS_9allocatorISP_EEFvvEEclEv
> >     @        0x10c510371 std::__1::function<>::operator()()
> >     @        0x10c56f999 _ZZN7process8dispatchERKNS_4UPIDERKNSt3__18functionIFvvEEEENKUlPNS_11ProcessBaseEE_clESA_
> >     @        0x10c56f970 _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZN7process8dispatchERKNS3_4UPIDERKNS_8functionIFvvEEEEUlPNS3_11ProcessBaseEE_SD_EEEvDpOT_
> >     @        0x10c56f70c _ZNSt3__110__function6__funcIZN7process8dispatchERKNS2_4UPIDERKNS_8functionIFvvEEEEUlPNS2_11ProcessBaseEE_NS_9allocatorISD_EEFvSC_EEclEOSC_
> >     @        0x10ea2529f std::__1::function<>::operator()()
> >     @        0x10e9ff2ff process::ProcessBase::visit()
> >     @        0x10ea531be process::DispatchEvent::visit()
> >     @        0x10c4efd71 process::ProcessBase::serve()
> >     @        0x10e9fc091 process::ProcessManager::resume()
> >     @        0x10ea0784f process::ProcessManager::init_threads()::$_1::operator()()
> >     @        0x10ea074d2 _ZNSt3__114__thread_proxyINS_5tupleIJNS_6__bindIZN7process14ProcessManager12init_threadsEvE3$_1JNS_17reference_wrapperIKNS_6atomicIbEEEEEEEEEEEEPvSD_
> >     @     0x7fff8c6e105a _pthread_body
> >     @     0x7fff8c6e0fd7 _pthread_start
> >     @     0x7fff8c6de3ed thread_start

Yep, I tested as noted in the "Testing Done" section, but I didn't manually apply an operation as you did - thanks a bunch for doing this Guangya, as you found a mistake that I made! In the parent patch of this one (https://reviews.apache.org/r/42164/) you'll find the fix, as well as a couple new test cases that apply reserve/unreserve operations without a principal. The tests produced a segfault before the fix, and succeeded after the fix.

Thanks again!!!

I noticed in your `curl` command that you included a principal "p1" in the resources you were attempting to reserve even though no authentication headers were included in the request. Note that this will produce an error given the new changes I introduced, since if no principal is provided with the operation, the `ReservationInfo` should contain an empty principal as well.


- Greg


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


On Jan. 11, 2016, 10:40 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42165/
> -----------------------------------------------------------
> 
> (Updated Jan. 11, 2016, 10:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
>     https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removed `hasPrincipal` parameter from unreserve validation.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f 
> 
> Diff: https://reviews.apache.org/r/42165/diff/
> 
> 
> Testing
> -------
> 
> Several instances of the relevant validation function were altered in the master validation tests.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 42165: Removed `hasPrincipal` parameter from unreserve validation.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42165/#review113913
-----------------------------------------------------------


Greg, I think that the document of reservation.md should also be updated. 

BTW: Did you test your patch? I did some test as this and mesos-master core dump.

curl -i -d slaveId="4c737d19-e3b2-41c0-b4d0-ffc60190b8eb-S0" -d resources='[ { "name": "cpus",  "type": "SCALAR", "scalar": { "value": 3 }, "role": "ads", "reservation": { "principal": "p1" } }, {  "name": "mem", "type": "SCALAR",  "scalar": { "value": 3000 }, "role": "ads", "reservation": { "principal": "p1" } }  ]'  -X POST http://9.110.49.27:5050/master/reserve
curl: (52) Empty reply from server

--
I0112 10:26:35.901185 333451264 http.cpp:315] HTTP POST for /master/reserve from 9.110.49.27:50492 with User-Agent='curl/7.43.0'
Assertion failed: (isSome()), function get, file ../../3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp, line 108.
*** Aborted at 1452565595 (unix time) try "date -d @1452565595" if you are using GNU date ***
PC: @     0x7fff8ca15286 __pthread_kill
*** SIGABRT (@0x7fff8ca15286) received by PID 43187 (TID 0x113e01000) stack trace: ***
    @     0x7fff89a0cf1a _sigtramp
    @     0x7f917bd08fd0 (unknown)
    @     0x7fff867139ab abort
    @     0x7fff866dba91 __assert_rtn
    @        0x10c517947 Option<>::get()
    @        0x10cf6c0f9 mesos::internal::master::validation::operation::validate()
    @        0x10ca5c0c2 mesos::internal::master::Master::Http::reserve()
    @        0x10cba6634 mesos::internal::master::Master::initialize()::$_9::operator()()
    @        0x10cba65d4 _ZNSt3__128__invoke_void_return_wrapperIN7process6FutureINS1_4http8ResponseEEEE6__callIJRZN5mesos8internal6master6Master10initializeEvE3$_9RKNS3_7RequestERK6OptionINS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEEEEES5_DpOT_
    @        0x10cba64a3 std::__1::__function::__func<>::operator()()
    @        0x10eacfaa2 std::__1::function<>::operator()()
    @        0x10ea0b399 process::ProcessBase::visit()::$_3::operator()()
    @        0x10ea0e9f1 _ZZZNK7process9_DeferredIZNS_11ProcessBase5visitERKNS_9HttpEventEE3$_3EcvNSt3__18functionIFvT_EEEIRKNS_6FutureI6OptionINS_4http14authentication20AuthenticationResultEEEEEEvENKUlSL_E_clESL_ENKUlvE_clEv
    @        0x10ea0e9bd _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZZNK7process9_DeferredIZNS3_11ProcessBase5visitERKNS3_9HttpEventEE3$_3EcvNS_8functionIFvT_EEEIRKNS3_6FutureI6OptionINS3_4http14authentication20AuthenticationResultEEEEEEvENKUlSO_E_clESO_EUlvE_EEEvDpOT_
    @        0x10ea0e6fc _ZNSt3__110__function6__funcIZZNK7process9_DeferredIZNS2_11ProcessBase5visitERKNS2_9HttpEventEE3$_3EcvNS_8functionIFvT_EEEIRKNS2_6FutureI6OptionINS2_4http14authentication20AuthenticationResultEEEEEEvENKUlSN_E_clESN_EUlvE_NS_9allocatorISP_EEFvvEEclEv
    @        0x10c510371 std::__1::function<>::operator()()
    @        0x10c56f999 _ZZN7process8dispatchERKNS_4UPIDERKNSt3__18functionIFvvEEEENKUlPNS_11ProcessBaseEE_clESA_
    @        0x10c56f970 _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZN7process8dispatchERKNS3_4UPIDERKNS_8functionIFvvEEEEUlPNS3_11ProcessBaseEE_SD_EEEvDpOT_
    @        0x10c56f70c _ZNSt3__110__function6__funcIZN7process8dispatchERKNS2_4UPIDERKNS_8functionIFvvEEEEUlPNS2_11ProcessBaseEE_NS_9allocatorISD_EEFvSC_EEclEOSC_
    @        0x10ea2529f std::__1::function<>::operator()()
    @        0x10e9ff2ff process::ProcessBase::visit()
    @        0x10ea531be process::DispatchEvent::visit()
    @        0x10c4efd71 process::ProcessBase::serve()
    @        0x10e9fc091 process::ProcessManager::resume()
    @        0x10ea0784f process::ProcessManager::init_threads()::$_1::operator()()
    @        0x10ea074d2 _ZNSt3__114__thread_proxyINS_5tupleIJNS_6__bindIZN7process14ProcessManager12init_threadsEvE3$_1JNS_17reference_wrapperIKNS_6atomicIbEEEEEEEEEEEEPvSD_
    @     0x7fff8c6e105a _pthread_body
    @     0x7fff8c6e0fd7 _pthread_start
    @     0x7fff8c6de3ed thread_start

- Guangya Liu


On 一月 11, 2016, 10:40 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42165/
> -----------------------------------------------------------
> 
> (Updated 一月 11, 2016, 10:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
>     https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removed `hasPrincipal` parameter from unreserve validation.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f 
> 
> Diff: https://reviews.apache.org/r/42165/diff/
> 
> 
> Testing
> -------
> 
> Several instances of the relevant validation function were altered in the master validation tests.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 42165: Removed `hasPrincipal` parameter from unreserve validation.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42165/#review114167
-----------------------------------------------------------


Patch looks great!

Reviews applied: [42164, 42165]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 11, 2016, 10:40 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42165/
> -----------------------------------------------------------
> 
> (Updated Jan. 11, 2016, 10:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
>     https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removed `hasPrincipal` parameter from unreserve validation.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f 
> 
> Diff: https://reviews.apache.org/r/42165/diff/
> 
> 
> Testing
> -------
> 
> Several instances of the relevant validation function were altered in the master validation tests.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 42165: Removed `hasPrincipal` parameter from unreserve validation.

Posted by Greg Mann <gr...@mesosphere.io>.

> On Jan. 13, 2016, 2:13 a.m., Guangya Liu wrote:
> > I think that you may also want a coming patch to address the document issue: reservation.md. Update the document to clarify how to use (un)reserve without principal.

Yep, thanks Guangya! I'll follow up soon with a patch for the documentation.


- Greg


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


On Jan. 11, 2016, 10:40 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42165/
> -----------------------------------------------------------
> 
> (Updated Jan. 11, 2016, 10:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
>     https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removed `hasPrincipal` parameter from unreserve validation.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f 
> 
> Diff: https://reviews.apache.org/r/42165/diff/
> 
> 
> Testing
> -------
> 
> Several instances of the relevant validation function were altered in the master validation tests.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Re: Review Request 42165: Removed `hasPrincipal` parameter from unreserve validation.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42165/#review114135
-----------------------------------------------------------


I think that you may also want a coming patch to address the document issue: reservation.md. Update the document to clarify how to use (un)reserve without principal.

- Guangya Liu


On 一月 11, 2016, 10:40 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42165/
> -----------------------------------------------------------
> 
> (Updated 一月 11, 2016, 10:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
>     https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removed `hasPrincipal` parameter from unreserve validation.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f 
> 
> Diff: https://reviews.apache.org/r/42165/diff/
> 
> 
> Testing
> -------
> 
> Several instances of the relevant validation function were altered in the master validation tests.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>