Re: Review Request 44163: Used temporary directory for fixture creating output files.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44163/#review121224
---
src/tests/state_tests.cpp (line 404)
<https://reviews.apache.org/r/44163/#comment182912>
IMHO this comment is unnecessary.
src/tests/state_tests.cpp (line 409)
<https://reviews.apache.org/r/44163/#comment182908>
This is no longer needed, because `TemporaryDirectoryTest::TearDown` will
remove the sandbox directory.
src/tests/state_tests.cpp (line 416)
<https://reviews.apache.org/r/44163/#comment182910>
Make this the last statement of the function, because `storage` and `state`
rely on directories that will be deleted by this function.
src/tests/state_tests.cpp (line 420)
<https://reviews.apache.org/r/44163/#comment182909>
Same as above.
- Jan Schlicht
On Feb. 29, 2016, 11:53 a.m., Benjamin Bannier wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44163/
> ---
>
> (Updated Feb. 29, 2016, 11:53 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Jan Schlicht, and Till Toenshoff.
>
>
> Bugs: MESOS-4806
> https://issues.apache.org/jira/browse/MESOS-4806
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Used temporary directory for fixture creating output files.
>
>
> Diffs
> -
>
> src/tests/state_tests.cpp 0b0bc9cb1c6e4906aa65c113207e5a2d8ebe61d9
>
> Diff: https://reviews.apache.org/r/44163/diff/
>
>
> Testing
> ---
>
> make check (OS X, not optimized)
>
> * before this patch running the test from a non-writable directory fails;
> with this patch the test passes,
> * before this patch running gtest-parallel with filter `LevelDBState*` fails;
> with the patch the test passes.
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 44163: Used temporary directory for fixture creating output files.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44163/#review121231 --- Ship it! Ship It! - Jan Schlicht On Feb. 29, 2016, 12:37 p.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44163/ > --- > > (Updated Feb. 29, 2016, 12:37 p.m.) > > > Review request for mesos, Benjamin Hindman, Jan Schlicht, and Till Toenshoff. > > > Bugs: MESOS-4806 > https://issues.apache.org/jira/browse/MESOS-4806 > > > Repository: mesos > > > Description > --- > > Used temporary directory for fixture creating output files. > > > Diffs > - > > src/tests/state_tests.cpp 0b0bc9cb1c6e4906aa65c113207e5a2d8ebe61d9 > > Diff: https://reviews.apache.org/r/44163/diff/ > > > Testing > --- > > make check (OS X, not optimized) > > * before this patch running the test from a non-writable directory fails; > with this patch the test passes, > * before this patch running gtest-parallel with filter `LevelDBState*` fails; > with the patch the test passes. > > > Thanks, > > Benjamin Bannier > >
Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/ --- Review request for mesos, Adam B and Joerg Schad. Bugs: MESOS-4772 https://issues.apache.org/jira/browse/MESOS-4772 Repository: mesos Description --- To be able to authenticate HTTP requests for tasks, the authorizer has to determine who owns the tasks. This is archived by adding an owner field to TaskInfo and ExecutorInfo. Diffs - include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca src/tests/master_validation_tests.cpp c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 Diff: https://reviews.apache.org/r/44570/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/ --- (Updated March 9, 2016, 3:51 p.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Typo in description. Bugs: MESOS-4772 https://issues.apache.org/jira/browse/MESOS-4772 Repository: mesos Description (updated) --- To be able to authenticate HTTP requests for tasks, the authorizer has to determine who owns the tasks. This is achieved by adding an owner field to TaskInfo and ExecutorInfo. Diffs - include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca src/tests/master_validation_tests.cpp c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 Diff: https://reviews.apache.org/r/44570/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/ --- (Updated March 9, 2016, 4 p.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Addressed issues. Bugs: MESOS-4772 https://issues.apache.org/jira/browse/MESOS-4772 Repository: mesos Description --- To be able to authenticate HTTP requests for tasks, the authorizer has to determine who owns the tasks. This is achieved by adding an owner field to TaskInfo and ExecutorInfo. Diffs (updated) - include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca src/tests/master_validation_tests.cpp c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 Diff: https://reviews.apache.org/r/44570/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/ --- (Updated March 9, 2016, 4:48 p.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Document behavior if both, `TaskInfo` and `ExecutorInfo` have a set owner. Bugs: MESOS-4772 https://issues.apache.org/jira/browse/MESOS-4772 Repository: mesos Description --- To be able to authenticate HTTP requests for tasks, the authorizer has to determine who owns the tasks. This is achieved by adding an owner field to TaskInfo and ExecutorInfo. Diffs (updated) - include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca src/tests/master_validation_tests.cpp c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 Diff: https://reviews.apache.org/r/44570/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
> On March 9, 2016, 4:33 p.m., James DeFelice wrote: > > include/mesos/mesos.proto, line 442 > > <https://reviews.apache.org/r/44570/diff/2/?file=1293285#file1293285line442> > > > > what if it's specified in both places, two different ways? I think > > there's room for a little more description in these comments. Yes, it makes sense to document the resulting error here. Though I'd argue that saying "Setting both will result in an error." includes the case that owner is specified in both places, two different ways. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/#review122725 ------- On March 9, 2016, 4 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44570/ > --- > > (Updated March 9, 2016, 4 p.m.) > > > Review request for mesos, Adam B and Joerg Schad. > > > Bugs: MESOS-4772 > https://issues.apache.org/jira/browse/MESOS-4772 > > > Repository: mesos > > > Description > --- > > To be able to authenticate HTTP requests for tasks, the authorizer has to > determine who owns the tasks. This is achieved by adding an owner field to > TaskInfo and ExecutorInfo. > > > Diffs > - > > include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 > src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b > src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca > src/tests/master_validation_tests.cpp > c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 > > Diff: https://reviews.apache.org/r/44570/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
> On March 10, 2016, 9:22 a.m., Klaus Ma wrote: > > include/mesos/mesos.proto, line 443 > > <https://reviews.apache.org/r/44570/diff/3/?file=1293310#file1293310line443> > > > > Also added in v1 APIs? Oh sure, good catch! - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/#review122897 --- On March 9, 2016, 4:48 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44570/ > --- > > (Updated March 9, 2016, 4:48 p.m.) > > > Review request for mesos, Adam B and Joerg Schad. > > > Bugs: MESOS-4772 > https://issues.apache.org/jira/browse/MESOS-4772 > > > Repository: mesos > > > Description > --- > > To be able to authenticate HTTP requests for tasks, the authorizer has to > determine who owns the tasks. This is achieved by adding an owner field to > TaskInfo and ExecutorInfo. > > > Diffs > - > > include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 > src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b > src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca > src/tests/master_validation_tests.cpp > c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 > > Diff: https://reviews.apache.org/r/44570/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
> On March 10, 2016, 7:31 a.m., Adam B wrote: > > src/master/validation.cpp, line 381 > > <https://reviews.apache.org/r/44570/diff/3/?file=1293312#file1293312line381> > > > > It may be sufficient to only check `if (task.has_owner() && > > task.has_executor())` since a custom executor should set the owner on the > > ExecutorInfo not the TaskInfo. That is if the `ExecutorInfo` is reused in a `TaskInfo` it's sufficient to check for that? That makes sense because the `ExecutorInfo` validation compares the `ExecutorId` against the ones of existing `ExecutorInfo`s. But how'd we make sure that the owner is set in `ExecutorInfo` if that executor isn't existing yet? - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/#review122887 ------- On March 9, 2016, 4:48 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44570/ > --- > > (Updated March 9, 2016, 4:48 p.m.) > > > Review request for mesos, Adam B and Joerg Schad. > > > Bugs: MESOS-4772 > https://issues.apache.org/jira/browse/MESOS-4772 > > > Repository: mesos > > > Description > --- > > To be able to authenticate HTTP requests for tasks, the authorizer has to > determine who owns the tasks. This is achieved by adding an owner field to > TaskInfo and ExecutorInfo. > > > Diffs > - > > include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 > src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b > src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca > src/tests/master_validation_tests.cpp > c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 > > Diff: https://reviews.apache.org/r/44570/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/ --- (Updated March 10, 2016, 1:49 p.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Addressed some issues. Bugs: MESOS-4772 https://issues.apache.org/jira/browse/MESOS-4772 Repository: mesos Description --- To be able to authenticate HTTP requests for tasks, the authorizer has to determine who owns the tasks. This is achieved by adding an owner field to TaskInfo and ExecutorInfo. Diffs (updated) - include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 include/mesos/v1/mesos.proto 31960a52061f70d80528fb8326522ae1d6f75b2c src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca src/tests/master_validation_tests.cpp c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 Diff: https://reviews.apache.org/r/44570/diff/ Testing --- make check Thanks, Jan Schlicht
Review Request 44620: Documented how to make executors work with SSL.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44620/ --- Review request for mesos, Adam B and Alexander Rojas. Bugs: MESOS-4750 https://issues.apache.org/jira/browse/MESOS-4750 Repository: mesos Description --- see summary Diffs - docs/ssl.md 3de2a3e931091e002dc4b259c70eadd89a52b059 Diff: https://reviews.apache.org/r/44620/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44620: Documented how to make executors work with SSL.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44620/ --- (Updated March 11, 2016, 12:26 p.m.) Review request for mesos, Adam B and Alexander Rojas. Changes --- Mention volumes to mount SSL files in dockerized environments. Bugs: MESOS-4750 https://issues.apache.org/jira/browse/MESOS-4750 Repository: mesos Description (updated) --- Documented how to make executors work with SSL. Diffs (updated) - docs/ssl.md 3de2a3e931091e002dc4b259c70eadd89a52b059 Diff: https://reviews.apache.org/r/44620/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44620: Documented how to make executors work with SSL.
> On March 10, 2016, 10:32 p.m., Joseph Wu wrote: > > docs/ssl.md, line 102 > > <https://reviews.apache.org/r/44620/diff/1/?file=1294364#file1294364line102> > > > > Not sure exactly how much detail we want to add here about specific > > configurations (especially considering the upcoming change to the > > containerizer's environment variables). But here's some more specifics: > > > > * When using the DockerContainerizer + normal Mesos agent, the docker > > executor shares the same root as the agent. So as long as the environment > > variables are passed on (via the methods you listed), this will work. > > * When using the DockerContainerizer + dockerized Mesos agent, you will > > need to add an additional `ContainerInfo.volumes` which mounts from the > > host (*not the agent*) to the executor. In this case, the task will have > > access to the keys/certs :( > > * The UnifiedContainerizer + normal Mesos agent case will probably be > > similar to the DockerContainerizer + normal Mesos agent case (I haven't > > tried this). > > * UnifiedContainerizer + Mesos launched via UnifiedContainerizer ??? Thanks for mentioning the need for volumes for dockerized executors! I've updated the documentation without going too much into detail and mentioning a more general "containerized" environment. I hope that this is enough information, not too less. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44620/#review122997 --- On March 11, 2016, 12:26 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44620/ > --- > > (Updated March 11, 2016, 12:26 p.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-4750 > https://issues.apache.org/jira/browse/MESOS-4750 > > > Repository: mesos > > > Description > --- > > Documented how to make executors work with SSL. > > > Diffs > - > > docs/ssl.md 3de2a3e931091e002dc4b259c70eadd89a52b059 > > Diff: https://reviews.apache.org/r/44620/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 44620: Documented how to make executors work with SSL.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44620/ --- (Updated March 11, 2016, 4:21 p.m.) Review request for mesos, Adam B and Alexander Rojas. Changes --- Addressed Alexander's comments. Bugs: MESOS-4750 https://issues.apache.org/jira/browse/MESOS-4750 Repository: mesos Description --- Documented how to make executors work with SSL. Diffs (updated) - docs/ssl.md 3de2a3e931091e002dc4b259c70eadd89a52b059 Diff: https://reviews.apache.org/r/44620/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44620: Documented how to make executors work with SSL.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44620/ --- (Updated March 11, 2016, 8:11 p.m.) Review request for mesos, Adam B and Alexander Rojas. Changes --- Addressed issues. Bugs: MESOS-4750 https://issues.apache.org/jira/browse/MESOS-4750 Repository: mesos Description --- Documented how to make executors work with SSL. Diffs (updated) - docs/ssl.md 3de2a3e931091e002dc4b259c70eadd89a52b059 Diff: https://reviews.apache.org/r/44620/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/ --- (Updated March 14, 2016, 10:37 a.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Addressed issues. Bugs: MESOS-4772 https://issues.apache.org/jira/browse/MESOS-4772 Repository: mesos Description --- To be able to authenticate HTTP requests for tasks, the authorizer has to determine who owns the tasks. This is achieved by adding an owner field to TaskInfo and ExecutorInfo. Diffs (updated) - include/mesos/mesos.proto 56d456acfd35fa59f394b27d62f62772eec03f6a include/mesos/v1/mesos.proto 4fba77464bb052d27c424f7721397142850b1144 src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca src/tests/master_validation_tests.cpp c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 Diff: https://reviews.apache.org/r/44570/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/ --- (Updated March 14, 2016, 11:46 a.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Updated comments. Bugs: MESOS-4772 https://issues.apache.org/jira/browse/MESOS-4772 Repository: mesos Description --- To be able to authenticate HTTP requests for tasks, the authorizer has to determine who owns the tasks. This is achieved by adding an owner field to TaskInfo and ExecutorInfo. Diffs (updated) - include/mesos/mesos.proto 56d456acfd35fa59f394b27d62f62772eec03f6a include/mesos/v1/mesos.proto 4fba77464bb052d27c424f7721397142850b1144 src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca src/tests/master_validation_tests.cpp c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 Diff: https://reviews.apache.org/r/44570/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/ --- (Updated March 14, 2016, 12:06 p.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Updated V1 comments. Bugs: MESOS-4772 https://issues.apache.org/jira/browse/MESOS-4772 Repository: mesos Description --- To be able to authenticate HTTP requests for tasks, the authorizer has to determine who owns the tasks. This is achieved by adding an owner field to TaskInfo and ExecutorInfo. Diffs (updated) - include/mesos/mesos.proto 56d456acfd35fa59f394b27d62f62772eec03f6a include/mesos/v1/mesos.proto 4fba77464bb052d27c424f7721397142850b1144 src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca src/tests/master_validation_tests.cpp c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 Diff: https://reviews.apache.org/r/44570/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44570/ --- (Updated March 15, 2016, 10:59 a.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Addressed Jörg's issues. Bugs: MESOS-4772 https://issues.apache.org/jira/browse/MESOS-4772 Repository: mesos Description --- To be able to authenticate HTTP requests for tasks, the authorizer has to determine who owns the tasks. This is achieved by adding an owner field to TaskInfo and ExecutorInfo. Diffs (updated) - include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca src/tests/master_validation_tests.cpp c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 Diff: https://reviews.apache.org/r/44570/diff/ Testing --- make check Thanks, Jan Schlicht
Review Request 44846: Deprecated the plain text credential format.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44846/ --- Review request for mesos, Adam B and Joerg Schad. Bugs: MESOS-2281 https://issues.apache.org/jira/browse/MESOS-2281 Repository: mesos Description --- See summary Diffs - CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 src/credentials/credentials.hpp aad17c21bb0ce98907bbfa22b890b66130e081e4 src/master/flags.cpp e6fea6421ea1a16b9cd78b0e42b830829b95ad61 src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 Diff: https://reviews.apache.org/r/44846/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44846: Deprecated the plain text credential format.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44846/ --- (Updated March 15, 2016, 3:01 p.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Addressed issues. Bugs: MESOS-2281 https://issues.apache.org/jira/browse/MESOS-2281 Repository: mesos Description (updated) --- Deprecated the plain text credential format. Diffs (updated) - CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 src/credentials/credentials.hpp aad17c21bb0ce98907bbfa22b890b66130e081e4 src/master/flags.cpp e6fea6421ea1a16b9cd78b0e42b830829b95ad61 src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 Diff: https://reviews.apache.org/r/44846/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44846: Deprecated the plain text credential format.
> On March 17, 2016, 6:21 p.m., Joerg Schad wrote: > > src/master/flags.cpp, line 227 > > <https://reviews.apache.org/r/44846/diff/2/?file=1299753#file1299753line227> > > > > We now mention deprecated twice here. > > The 'Note that' also seems to be out of sync with the configuration.md. The whole sentence will be removed, as per Adam's comments below the deprecated format won't be mentioned at all. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44846/#review124051 ------- On March 15, 2016, 3:01 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44846/ > --- > > (Updated March 15, 2016, 3:01 p.m.) > > > Review request for mesos, Adam B and Joerg Schad. > > > Bugs: MESOS-2281 > https://issues.apache.org/jira/browse/MESOS-2281 > > > Repository: mesos > > > Description > --- > > Deprecated the plain text credential format. > > > Diffs > - > > CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 > docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 > docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b > docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 > src/credentials/credentials.hpp aad17c21bb0ce98907bbfa22b890b66130e081e4 > src/master/flags.cpp e6fea6421ea1a16b9cd78b0e42b830829b95ad61 > src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 > > Diff: https://reviews.apache.org/r/44846/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 44846: Deprecated the plain text credential format.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44846/ --- (Updated March 24, 2016, 1:45 p.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Addressed issues and rebased. Bugs: MESOS-2281 https://issues.apache.org/jira/browse/MESOS-2281 Repository: mesos Description --- Deprecated the plain text credential format. Diffs (updated) - CHANGELOG 1fadaa1819c9b1a801752b1c0ddd572b82e6c170 docs/authentication.md 9f64f657d1a01f85f0132cb7264e2b85d0d3dc59 docs/configuration.md 70686adeca239b5962652a17661c8b0d44ca6708 docs/upgrades.md 85d02929b58b465f22d0e9991f6cf66f52b6d9a6 src/credentials/credentials.hpp aad17c21bb0ce98907bbfa22b890b66130e081e4 src/master/flags.cpp 9b65dee02e3505f22aaa967899903f80fe0e7c57 src/slave/flags.cpp 71685cee58322e608139f7c344c2e954f7e14322 Diff: https://reviews.apache.org/r/44846/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44846: Deprecated the plain text credential format.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44846/ --- (Updated March 24, 2016, 2:22 p.m.) Review request for mesos, Adam B and Joerg Schad. Changes --- Removed another mention of the deprecated format. Bugs: MESOS-2281 https://issues.apache.org/jira/browse/MESOS-2281 Repository: mesos Description --- Deprecated the plain text credential format. Diffs (updated) - CHANGELOG 1fadaa1819c9b1a801752b1c0ddd572b82e6c170 docs/authentication.md 9f64f657d1a01f85f0132cb7264e2b85d0d3dc59 docs/configuration.md 70686adeca239b5962652a17661c8b0d44ca6708 docs/upgrades.md 85d02929b58b465f22d0e9991f6cf66f52b6d9a6 src/credentials/credentials.hpp aad17c21bb0ce98907bbfa22b890b66130e081e4 src/master/flags.cpp 9b65dee02e3505f22aaa967899903f80fe0e7c57 src/slave/flags.cpp 71685cee58322e608139f7c344c2e954f7e14322 Diff: https://reviews.apache.org/r/44846/diff/ Testing (updated) --- make check Created the documentation using the Dockerfile in `support/site-docker` to make sure that the markdown files renders correctly. Thanks, Jan Schlicht
Re: Review Request 45429: Added authentication to the '/registry' endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45429/ --- (Updated March 29, 2016, 5:05 p.m.) Review request for mesos, Adam B and Till Toenshoff. Bugs: MESOS-4933 https://issues.apache.org/jira/browse/MESOS-4933 Repository: mesos Description (updated) --- See summary. Diffs - src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 src/master/registrar.hpp f087569bd6774ac16093f9204e6870e4d4404420 src/master/registrar.cpp def40b94689c363617a7cfbcce82ea8d357cb345 src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc Diff: https://reviews.apache.org/r/45429/diff/ Testing --- make check Thanks, Jan Schlicht
Review Request 45429: Added authentication to the '/registry' endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45429/ --- Review request for mesos, Adam B and Till Toenshoff. Bugs: MESOS-4933 https://issues.apache.org/jira/browse/MESOS-4933 Repository: mesos Description --- Added authentication to the '/registry' endpoint. Diffs - src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 src/master/registrar.hpp f087569bd6774ac16093f9204e6870e4d4404420 src/master/registrar.cpp def40b94689c363617a7cfbcce82ea8d357cb345 src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc Diff: https://reviews.apache.org/r/45429/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 45429: Added authentication to the '/registry' endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45429/ --- (Updated March 29, 2016, 5:46 p.m.) Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff. Changes --- Added tests case of missing credentials. Bugs: MESOS-4933 https://issues.apache.org/jira/browse/MESOS-4933 Repository: mesos Description (updated) --- Added authentication to the '/registry' endpoint. Diffs (updated) - src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 src/master/registrar.hpp f087569bd6774ac16093f9204e6870e4d4404420 src/master/registrar.cpp def40b94689c363617a7cfbcce82ea8d357cb345 src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc Diff: https://reviews.apache.org/r/45429/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 45429: Added authentication to the '/registry' endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45429/ --- (Updated March 29, 2016, 5:46 p.m.) Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff. Bugs: MESOS-4933 https://issues.apache.org/jira/browse/MESOS-4933 Repository: mesos Description (updated) --- See summary. Diffs - src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa src/master/main.cpp 58561cffa440aaf1293e9ffe19b5685e6d2f1952 src/master/registrar.hpp f087569bd6774ac16093f9204e6870e4d4404420 src/master/registrar.cpp def40b94689c363617a7cfbcce82ea8d357cb345 src/tests/cluster.cpp 2da0bd7612d571277e76d0a95ad8e776434af323 src/tests/registrar_tests.cpp 39caf9bb950c0b229a66becb039c7a830a18f6bc Diff: https://reviews.apache.org/r/45429/diff/ Testing --- make check Thanks, Jan Schlicht
Review Request 45434: Added test cases for '/files' endpoint authentication.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45434/ --- Review request for mesos, Adam B and Greg Mann. Repository: mesos Description --- The current authentication tests of the `/files/` endpoint only test the case of missing credentials. This commit adds the additional case of wrong credentials. Diffs - src/tests/files_tests.cpp 2f0f84cd78e7f382b70b97c8711f71cea64ce0a5 Diff: https://reviews.apache.org/r/45434/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 45434: Added test cases for '/files' endpoint authentication.
> On March 30, 2016, 2:38 a.m., Greg Mann wrote: > > BenM and I discussed this while I was working on the previous patches: > > since the code paths for the authenticated and unauthenticated cases are > > the same once the handler is reached, it shouldn't be necessary to > > explicitly test the full functioning of the endpoints in both cases. We > > test the full functionality of the endpoints with authentication disabled, > > then we test that requests that don't authenticate properly are refused > > when authentication is on. This should test all of the code paths through > > the handlers, since we don't actually do anything with the principal > > currently. What do you think? Yes, I agree. The authenticated cases are already covered implicitly and the authenticator differs between "bad credentials" vs "missing credentials" -- which is already covered elsewhere. I'd then probably only remove the `badCredentials` as they're unused. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45434/#review125996 --- On March 29, 2016, 6:36 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45434/ > --- > > (Updated March 29, 2016, 6:36 p.m.) > > > Review request for mesos, Adam B and Greg Mann. > > > Repository: mesos > > > Description > --- > > The current authentication tests of the `/files/` endpoint only test the case > of missing credentials. This commit adds the additional case of wrong > credentials. > > > Diffs > - > > src/tests/files_tests.cpp 2f0f84cd78e7f382b70b97c8711f71cea64ce0a5 > > Diff: https://reviews.apache.org/r/45434/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 45434: Removed unused credentials.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45434/ --- (Updated March 30, 2016, 10:20 a.m.) Review request for mesos, Adam B and Greg Mann. Summary (updated) - Removed unused credentials. Repository: mesos Description (updated) --- See summary. Diffs (updated) - src/tests/files_tests.cpp 2f0f84cd78e7f382b70b97c8711f71cea64ce0a5 Diff: https://reviews.apache.org/r/45434/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44571: Added timeout for destroying Docker containers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44571/ --- (Updated March 31, 2016, 3:26 p.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-4673 https://issues.apache.org/jira/browse/MESOS-4673 Repository: mesos Description --- Commands issued to the Docker daemon can hang, causing problems within Mesos. For example a hanging 'docker stop' can result in an unresponsive executor, causing the Mesos agent to issue an to run a 'docker stop' itself which might result in an unresponsive agent (see MESOS-4673). Adding a timeout can be used as a workaround. Diffs - src/slave/containerizer/docker.hpp 89d450e10a84f24ddd46d517e2b4b46ab02c4fda src/slave/containerizer/docker.cpp c5007a311ae9c1766dd4522ccbddbdb506d4ae4e Diff: https://reviews.apache.org/r/44571/diff/ Testing --- sudo ./bin/mesos-tests.sh (to test if existing tests break due to the changed behavior) Because docker must hang for both the Mesos agent as well as the `mesos-docker-executor`, it can't currently be tested as part of the Mesos integration tests. Here's how to test that the timeout works: Run with Fedora 23 (Kernel 4.2.3, Docker 1.9.1) # Start a master ./bin/mesos-master.sh --work_dir=/tmp/mesos & # Start an agent sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --containerizers=docker & # Run a task using the docker containerizer ./src/mesos-execute --containerizer=docker --docker_image=alpine --master=127.0.0.1:5050 --name="sleep" --command="sleep 1000" & # Note the pid of `mesos-execute` as well as the pid of the sleep task run by docker (eg 3323 and 3474) # Have mesos run `docker inspect` to gather the pid of the docker task curl -X GET localhost:5051/monitor/statistics # Now overload docker by trying to run a lot of tasks in parallel for i in `seq 1 100`; do sudo docker run --rm alpine sleep 60 & done # Wait until the first of these docker tasks finish, `sudo docker ps` should be unresponsible now # Kill the `mesos-execute` task (eg 3323) kill 3323 # Watch the logs of the Mesos agent. At some point it will send a SIGKILL to the docker task (eg 3474) # Make sure that the docker task is indeed termintad (using `ps fax` or the like) Thanks, Jan Schlicht
Re: Review Request 44571: Added timeout for destroying Docker containers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44571/ --- (Updated April 4, 2016, 4:05 p.m.) Review request for mesos, Jie Yu and Joris Van Remoortere. Changes --- Changed order of continuations. Bugs: MESOS-4673 https://issues.apache.org/jira/browse/MESOS-4673 Repository: mesos Description --- Commands issued to the Docker daemon can hang, causing problems within Mesos. For example a hanging 'docker stop' can result in an unresponsive executor, causing the Mesos agent to issue an to run a 'docker stop' itself which might result in an unresponsive agent (see MESOS-4673). Adding a timeout can be used as a workaround. Diffs (updated) - src/slave/containerizer/docker.hpp 89d450e10a84f24ddd46d517e2b4b46ab02c4fda src/slave/containerizer/docker.cpp 9314d1f9e0b6077fe7c48b860783ab21acc48be6 Diff: https://reviews.apache.org/r/44571/diff/ Testing --- sudo ./bin/mesos-tests.sh (to test if existing tests break due to the changed behavior) Because docker must hang for both the Mesos agent as well as the `mesos-docker-executor`, it can't currently be tested as part of the Mesos integration tests. Here's how to test that the timeout works: Run with Fedora 23 (Kernel 4.2.3, Docker 1.9.1) # Start a master ./bin/mesos-master.sh --work_dir=/tmp/mesos & # Start an agent sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --containerizers=docker & # Run a task using the docker containerizer ./src/mesos-execute --containerizer=docker --docker_image=alpine --master=127.0.0.1:5050 --name="sleep" --command="sleep 1000" & # Note the pid of `mesos-execute` as well as the pid of the sleep task run by docker (eg 3323 and 3474) # Have mesos run `docker inspect` to gather the pid of the docker task curl -X GET localhost:5051/monitor/statistics # Now overload docker by trying to run a lot of tasks in parallel for i in `seq 1 100`; do sudo docker run --rm alpine sleep 60 & done # Wait until the first of these docker tasks finish, `sudo docker ps` should be unresponsible now # Kill the `mesos-execute` task (eg 3323) kill 3323 # Watch the logs of the Mesos agent. At some point it will send a SIGKILL to the docker task (eg 3474) # Make sure that the docker task is indeed termintad (using `ps fax` or the like) Thanks, Jan Schlicht
Review Request 45922: Added agent authorization flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- Review request for mesos, Adam B and Alexander Rojas. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description --- See summary. Diffs - src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960 src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6 src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599 src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3 src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3 src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6 src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 Diff: https://reviews.apache.org/r/45922/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review127751 --- src/slave/flags.cpp (lines 459 - 491) <https://reviews.apache.org/r/45922/#comment191138> The agent ACLs will probably change, but because they aren't defined/used yet, I've kept the example the same as in `master/flags.cpp`, but could also remove it completely. - Jan Schlicht On April 8, 2016, 10:52 a.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45922/ > --- > > (Updated April 8, 2016, 10:52 a.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce > src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c > src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960 > src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6 > src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599 > src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 > src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e > src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3 > src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3 > src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6 > src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 > > Diff: https://reviews.apache.org/r/45922/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 45922: Added agent authorization flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 8, 2016, 11:26 a.m.) Review request for mesos, Adam B and Alexander Rojas. Changes --- Added flags documentation in `docs/configuration.md`. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description --- See summary. Diffs (updated) - docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960 src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6 src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599 src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3 src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3 src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6 src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 Diff: https://reviews.apache.org/r/45922/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 44571: Added timeout for destroying Docker containers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44571/ --- (Updated April 8, 2016, 1:20 p.m.) Review request for mesos, Jie Yu and Joris Van Remoortere. Changes --- Addressed issues. Bugs: MESOS-4673 https://issues.apache.org/jira/browse/MESOS-4673 Repository: mesos Description (updated) --- Commands issued to the Docker daemon can hang, causing problems within Mesos. For example a hanging 'docker stop' can result in an unresponsive executor, causing the Mesos agent to issue an to run a 'docker stop' itself which might result in an unresponsive agent (see MESOS-4673). Adding a timeout can be used as a workaround. Diffs (updated) - src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960 src/slave/containerizer/docker.hpp 35673214ab4bf50151f15e3fad10ff374cda3bbc src/slave/containerizer/docker.cpp 5755effec065650aac4473e4b622f4342ad020a3 Diff: https://reviews.apache.org/r/44571/diff/ Testing --- sudo ./bin/mesos-tests.sh (to test if existing tests break due to the changed behavior) Because docker must hang for both the Mesos agent as well as the `mesos-docker-executor`, it can't currently be tested as part of the Mesos integration tests. Here's how to test that the timeout works: Run with Fedora 23 (Kernel 4.2.3, Docker 1.9.1) # Start a master ./bin/mesos-master.sh --work_dir=/tmp/mesos & # Start an agent sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --containerizers=docker & # Run a task using the docker containerizer ./src/mesos-execute --containerizer=docker --docker_image=alpine --master=127.0.0.1:5050 --name="sleep" --command="sleep 1000" & # Note the pid of `mesos-execute` as well as the pid of the sleep task run by docker (eg 3323 and 3474) # Have mesos run `docker inspect` to gather the pid of the docker task curl -X GET localhost:5051/monitor/statistics # Now overload docker by trying to run a lot of tasks in parallel for i in `seq 1 100`; do sudo docker run --rm alpine sleep 60 & done # Wait until the first of these docker tasks finish, `sudo docker ps` should be unresponsible now # Kill the `mesos-execute` task (eg 3323) kill 3323 # Watch the logs of the Mesos agent. At some point it will send a SIGKILL to the docker task (eg 3474) # Make sure that the docker task is indeed termintad (using `ps fax` or the like) Thanks, Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 8, 2016, 4:25 p.m.) Review request for mesos, Adam B and Alexander Rojas. Changes --- Add a comment explaining the current ACLs example. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description (updated) --- Added agent authorization flags. Diffs (updated) - docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960 src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6 src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599 src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3 src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3 src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6 src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 Diff: https://reviews.apache.org/r/45922/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
> On April 12, 2016, 11:31 a.m., Adam B wrote: > > docs/configuration.md, line 94 > > <https://reviews.apache.org/r/45922/diff/3/?file=1337109#file1337109line94> > > > > Why would we need to support multiple authorizers? Would a particular > > request check two authorizers then and/or the results? > > Or are we somehow using one authorizer for some things and another for > > others? This could be explainable for the master's framework authz vs. http > > authz, but makes no sense for the agent. > > Alexander Rojas wrote: > That was an original requirement on the master, since the original > crammd5 authorizer was supposed to come also in the multiple version. I think > we can, not only remove it here, but also in the master. I don't think we > will move onto the multiple authorizers anytime soon. > > Adam B wrote: > +1, although I dread deprecating the plural flag in master. I could live > with it being singular on agent and plural on master, at least for this > release; but I guess we should start deprecations ASAP if we have to wait 6mo > before we can remove them. Got it! I'll remove support for multiple authorizers on the agent. > On April 12, 2016, 11:31 a.m., Adam B wrote: > > src/slave/flags.cpp, lines 446-449 > > <https://reviews.apache.org/r/45922/diff/3/?file=1337114#file1337114line446> > > > > I'd rather have no example than a confusing/invalid example. We can add > > an example after we have at least one ACL > > Joerg Schad wrote: > +1 Will remove the example. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review128350 --- On April 8, 2016, 4:25 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45922/ > --- > > (Updated April 8, 2016, 4:25 p.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > Added agent authorization flags. > > > Diffs > - > > docs/configuration.md 309a5a05eab386c8943ba6bdee8d5efeb448aa0c > src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce > src/master/flags.cpp e522499586b731d522180f171731a9dd38b8344c > src/slave/constants.hpp 449c8cd9f43f71b4612023eb463969e9db0bc960 > src/slave/flags.hpp 4236b7fe40b9aa26d57b651be06a0e36037f65c6 > src/slave/flags.cpp 7164afef9c82a9bba0f137d7387c3569912b6599 > src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 > src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e > src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3 > src/tests/cluster.hpp 39ca15e3cc2f4f2ac9b8a73482c2bd945b1824e3 > src/tests/cluster.cpp 7e488d28b7a0d3bef9cd76bf7df2de4822256ef6 > src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 > > Diff: https://reviews.apache.org/r/45922/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 45922: Added agent authorization flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 14, 2016, 1 p.m.) Review request for mesos, Adam B and Alexander Rojas. Changes --- Interim patch. It's fixing the open issues but doesn't add a test case. This will be done in a later rev. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description (updated) --- See summary. Diffs (updated) - docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e src/slave/slave.cpp 49fa4a06e26d0d4475ed50db254a320a7030f896 src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 Diff: https://reviews.apache.org/r/45922/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
> On April 13, 2016, 3:58 p.m., Benjamin Bannier wrote: > > src/tests/cluster.cpp, lines 398-410 > > <https://reviews.apache.org/r/45922/diff/3/?file=1337119#file1337119line398> > > > > You are shadowing the outer `Option` here with a > > `Result`. All of the following assignments are to that and not > > the outer one. I believe this leads us to assigning not what was intended > > in l.462 below. Having at least a different name would help readability > > some here. > > > > As an aside, for my taste the control flow in this block has too much > > nesting making this less transparent than it should be. The shadowing has been resolved. If no authorizer and no ACLs were provided, a segfault could occur. This has been fixed as well by making it an error. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review128666 ------- On April 14, 2016, 1 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45922/ > --- > > (Updated April 14, 2016, 1 p.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c > src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce > src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea > src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a > src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 > src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 > src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e > src/slave/slave.cpp 49fa4a06e26d0d4475ed50db254a320a7030f896 > src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 > src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c > src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 > > Diff: https://reviews.apache.org/r/45922/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 45922: Added agent authorization flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 14, 2016, 6:16 p.m.) Review request for mesos, Adam B and Alexander Rojas. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description (updated) --- Added agent authorization flags. Diffs (updated) - docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e src/slave/slave.cpp 49fa4a06e26d0d4475ed50db254a320a7030f896 src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 Diff: https://reviews.apache.org/r/45922/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
> On April 12, 2016, 11:31 a.m., Adam B wrote: > > docs/configuration.md, line 886 > > <https://reviews.apache.org/r/45922/diff/3/?file=1337109#file1337109line886> > > > > None of these examples apply to an agent. Maybe we need to implement an > > ACL (e.g. GET_ENDPOINT_WITH_PATH "/flags") before we can provide any > > example ACLs for the agent. Will be implemented in https://reviews.apache.org/r/46203 - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review128350 ------- On April 14, 2016, 6:16 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45922/ > --- > > (Updated April 14, 2016, 6:16 p.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > Added agent authorization flags. > > > Diffs > - > > docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c > src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce > src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea > src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a > src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 > src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 > src/slave/slave.hpp 76f3aff394e5cecc54dcb3065cb872e238bc228e > src/slave/slave.cpp 49fa4a06e26d0d4475ed50db254a320a7030f896 > src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 > src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c > src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 > > Diff: https://reviews.apache.org/r/45922/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 45922: Added agent authorization flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 14, 2016, 6:51 p.m.) Review request for mesos, Adam B and Alexander Rojas. Changes --- Rebased. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description (updated) --- See summary. Diffs (updated) - docs/configuration.md ce51f26f0584eeecb5c96e39a5bcc21f5906a29c src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 src/slave/slave.cpp de99e9eb5cc812b2e07deb749b98b4f4db363728 src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 src/tests/cluster.cpp b4d69106388892b88f7de20b248cac8b950b861c src/tests/mesos.cpp 1b7a8fd703b90d77ffa3df079bdc2744752caac6 Diff: https://reviews.apache.org/r/45922/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 18, 2016, 11:49 a.m.) Review request for mesos, Adam B and Alexander Rojas. Changes --- Addressed issues and rebased. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description --- See summary. Diffs (updated) - docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31 src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb Diff: https://reviews.apache.org/r/45922/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 46203: Added authorization of the '/flags' endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46203/ --- (Updated April 18, 2016, 11:52 a.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier. Changes --- Addressed issues and rebased. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description --- See summary. Diffs (updated) - docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 include/mesos/authorizer/acls.proto c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 include/mesos/authorizer/authorizer.proto 40d93ea257d1df8d22eee8a21667db90d579a8fe src/Makefile.am ec855263d620e4723c8ba9cd056c40a3a2e9ca99 src/authorizer/local/authorizer.cpp c744a16041c2466d3839a37fbee2bf86887bf4e1 src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb src/tests/slave_authorization_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/46203/diff/ Testing --- make check (SlaveAuthorizationTest.AuthorizeFlagsEndpointWithoutPrincipal fails though it shouldn't, tests sometimes segfault) Thanks, Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
> On April 15, 2016, 12:16 p.m., Alexander Rojas wrote: > > src/slave/slave.hpp, line 108 > > <https://reviews.apache.org/r/45922/diff/6/?file=1345034#file1345034line108> > > > > I think is a better practice to move optional parameters at the end and > > default initialize them to `None()`. > > > > In fact if you check `master.hpp` it is how it is done there. It's moved to the end now. Nevertheless, they default initialization to `None()` is done in `cluster.hpp`, because how it's done for all of the other parameters. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review129089 ------- On April 18, 2016, 11:49 a.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45922/ > --- > > (Updated April 18, 2016, 11:49 a.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 > src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce > src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea > src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a > src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 > src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 > src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 > src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31 > src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b > src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb > > Diff: https://reviews.apache.org/r/45922/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 45922: Added agent authorization flags.
> On April 15, 2016, 1:33 p.m., Benjamin Bannier wrote: > > src/tests/mesos.cpp, lines 373-377 > > <https://reviews.apache.org/r/45922/diff/6/?file=1345038#file1345038line373> > > > > Could you add a similar helper to inject e.g., a mock authorizer? It looks like these helper functions are created on demand, so I'd rather not add one here now. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review129097 ------- On April 18, 2016, 11:49 a.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45922/ > --- > > (Updated April 18, 2016, 11:49 a.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 > src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce > src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea > src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a > src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 > src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 > src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 > src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31 > src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b > src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb > > Diff: https://reviews.apache.org/r/45922/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 46203: Added authorization of the '/flags' endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46203/ --- (Updated April 18, 2016, 12:07 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier. Changes --- s/AccessEndpoints/AccessEndpoint Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description --- See summary. Diffs (updated) - docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 include/mesos/authorizer/acls.proto c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 include/mesos/authorizer/authorizer.proto 40d93ea257d1df8d22eee8a21667db90d579a8fe src/Makefile.am ec855263d620e4723c8ba9cd056c40a3a2e9ca99 src/authorizer/local/authorizer.cpp c744a16041c2466d3839a37fbee2bf86887bf4e1 src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb src/tests/slave_authorization_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/46203/diff/ Testing --- make check (SlaveAuthorizationTest.AuthorizeFlagsEndpointWithoutPrincipal fails though it shouldn't, tests sometimes segfault) Thanks, Jan Schlicht
Re: Review Request 46203: Added authorization of the '/flags' endpoint.
> On April 15, 2016, 11:34 a.m., Alexander Rojas wrote: > > docs/configuration.md, line 871 > > <https://reviews.apache.org/r/46203/diff/5/?file=1345039#file1345039line871> > > > > s/get_endpoints/access_endpoint/ I'd keep it plural here. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46203/#review129083 ------- On April 18, 2016, 12:07 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46203/ > --- > > (Updated April 18, 2016, 12:07 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 > include/mesos/authorizer/acls.proto > c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 > include/mesos/authorizer/authorizer.proto > 40d93ea257d1df8d22eee8a21667db90d579a8fe > src/Makefile.am ec855263d620e4723c8ba9cd056c40a3a2e9ca99 > src/authorizer/local/authorizer.cpp > c744a16041c2466d3839a37fbee2bf86887bf4e1 > src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 > src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb > src/tests/slave_authorization_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46203/diff/ > > > Testing > --- > > make check (SlaveAuthorizationTest.AuthorizeFlagsEndpointWithoutPrincipal > fails though it shouldn't, tests sometimes segfault) > > > Thanks, > > Jan Schlicht > >
Re: Review Request 46203: Added authorization of the '/flags' endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46203/ --- (Updated April 18, 2016, 2:53 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier. Changes --- Fixed segfaults and `SlaveAuthorizationTest.AuthorizeFlagsEndpointWithoutPrincipal`. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description --- See summary. Diffs (updated) - docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 include/mesos/authorizer/acls.proto c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 include/mesos/authorizer/authorizer.proto 40d93ea257d1df8d22eee8a21667db90d579a8fe src/Makefile.am ec855263d620e4723c8ba9cd056c40a3a2e9ca99 src/authorizer/local/authorizer.cpp c744a16041c2466d3839a37fbee2bf86887bf4e1 src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb src/tests/slave_authorization_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/46203/diff/ Testing (updated) --- make check Thanks, Jan Schlicht
Re: Review Request 46319: Added authorization to agents' `/statistics` endpoints.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46319/#review129327 --- src/slave/http.cpp (lines 613 - 616) <https://reviews.apache.org/r/46319/#comment192779> You don't need to do that. If authorization is disabled, `authorizedEndpoint` will always return `true`. src/slave/http.cpp (line 624) <https://reviews.apache.org/r/46319/#comment192781> Use the resulting `bool` directly, instead of the future. `then` only gets called when the future is ready, no need to test if the future failed. src/slave/http.cpp (lines 625 - 626) <https://reviews.apache.org/r/46319/#comment192780> Break before the '?', like ``` return !authorized.get() ? Forbidden() : _statistics(context, *limiter, request); - Jan Schlicht On April 18, 2016, 1:15 p.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46319/ > --- > > (Updated April 18, 2016, 1:15 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht. > > > Bugs: MESOS-5164 > https://issues.apache.org/jira/browse/MESOS-5164 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/tests/slave_tests.cpp ee58488b0b927c7c5833add4718941539663e6d2 > > Diff: https://reviews.apache.org/r/46319/diff/ > > > Testing > --- > > make check (OS X, clang w/o optimization) > > > Thanks, > > Benjamin Bannier > >
Re: Review Request 46318: Added helper to create test slave with injected `Authorizer`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46318/#review129330 --- Ship it! Ship It! - Jan Schlicht On April 18, 2016, 1:15 p.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46318/ > --- > > (Updated April 18, 2016, 1:15 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht. > > > Bugs: MESOS-5164 > https://issues.apache.org/jira/browse/MESOS-5164 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b > src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb > > Diff: https://reviews.apache.org/r/46318/diff/ > > > Testing > --- > > make check (OS X, clang w/o optimization) > > > Thanks, > > Benjamin Bannier > >
Re: Review Request 46319: Added authorization to agents' `/statistics` endpoints.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46319/#review129506 --- Ship it! The test case may need to be moved into `slave_authorization_tests.cpp` that was added in https://reviews.apache.org/r/46318/, but that really depends on whether that change there gets accepted or not. - Jan Schlicht On April 18, 2016, 3:42 p.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46319/ > --- > > (Updated April 18, 2016, 3:42 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht. > > > Bugs: MESOS-5164 > https://issues.apache.org/jira/browse/MESOS-5164 > > > Repository: mesos > > > Description > --- > > Added authorization to agents' `/statistics` endpoints. > > > Diffs > - > > src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/tests/slave_tests.cpp ee58488b0b927c7c5833add4718941539663e6d2 > > Diff: https://reviews.apache.org/r/46319/diff/ > > > Testing > --- > > make check (OS X, clang w/o optimization) > > > Thanks, > > Benjamin Bannier > >
Re: Review Request 45922: Added agent authorization flags.
> On April 12, 2016, 12:30 p.m., Joerg Schad wrote: > > src/slave/flags.hpp, line 102 > > <https://reviews.apache.org/r/45922/diff/3/?file=1337113#file1337113line102> > > > > Do we have a particular order here? I don't know exactly. I kept it consistent to the one in `master/flags.hpp`. In that context it does make sense to place `acls` next to `credentials`. Of course other orders could be possible as well. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review128356 ------- On April 18, 2016, 11:49 a.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45922/ > --- > > (Updated April 18, 2016, 11:49 a.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 > src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce > src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea > src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a > src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 > src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 > src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 > src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31 > src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b > src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb > > Diff: https://reviews.apache.org/r/45922/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 46203: Added authorization of the '/flags' endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46203/ --- (Updated April 19, 2016, 2:05 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier. Changes --- Addressed Alexanders' issues. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description (updated) --- Added authorization of the '/flags' endpoint. Diffs (updated) - docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 include/mesos/authorizer/acls.proto c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 include/mesos/authorizer/authorizer.proto 40d93ea257d1df8d22eee8a21667db90d579a8fe src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac src/authorizer/local/authorizer.cpp c744a16041c2466d3839a37fbee2bf86887bf4e1 src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb src/tests/slave_authorization_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/46203/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 46203: Added authorization of the '/flags' endpoint.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46203/ --- (Updated April 19, 2016, 2:08 p.m.) Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description (updated) --- See summary. Diffs - docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 include/mesos/authorizer/acls.proto c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 include/mesos/authorizer/authorizer.proto 40d93ea257d1df8d22eee8a21667db90d579a8fe src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac src/authorizer/local/authorizer.cpp c744a16041c2466d3839a37fbee2bf86887bf4e1 src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb src/tests/slave_authorization_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/46203/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
> On April 20, 2016, 9:26 a.m., Adam B wrote: > > src/tests/cluster.hpp, line 151 > > <https://reviews.apache.org/r/45922/diff/7/?file=1348010#file1348010line151> > > > > Why do you even need the overload for the authorizer here? Seems like > > most tests will either provide --acls and use the default, or set the > > modules/authorizer flags. We can leave this part out until a test needs it. We will need the overload to be able to use mocked authorizers. This will be important for later authz patches. Of course it is not needed right now, and I could remove it until a test needs it, on the other hand I think it should be part of this patch to provide a "ready-to-use" implementation of an agent authorizer. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review129694 ------- On April 18, 2016, 11:49 a.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45922/ > --- > > (Updated April 18, 2016, 11:49 a.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 > src/local/local.cpp df72ac52110b75d63df1076496b48e63d06d42ce > src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea > src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a > src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 > src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 > src/tests/cluster.hpp 887342acc72b33b4c904d610da47394f9a7d7188 > src/tests/cluster.cpp 31d2556a078429dd45315aa74cd21ad436372d31 > src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b > src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb > > Diff: https://reviews.apache.org/r/45922/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 45922: Added agent authorization flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 21, 2016, 3:45 p.m.) Review request for mesos, Adam B and Alexander Rojas. Changes --- Addressed Adam's issues. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description (updated) --- Added agent authorization flags. Diffs (updated) - docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956 src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4 src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb Diff: https://reviews.apache.org/r/45922/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 45922: Added agent authorization flags.
> On April 20, 2016, 9:26 a.m., Adam B wrote: > > src/tests/cluster.cpp, lines 406-412 > > <https://reviews.apache.org/r/45922/diff/7/?file=1348011#file1348011line406> > > > > Why is it an error to start an agent with no authorizer and no ACLs? > > What if I don't want to do any authorization? This was due to a bug that will set `slave::Slave::authorizer` to `nullptr` instead of `None()` if no ACLs were provided -- which is how to disable authorization. While the above code fixed that, it also removed the possibility to disable agent authorization in tests. I've changed the approach, to support tests with disabled agent authorization. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review129694 ------- On April 21, 2016, 3:45 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45922/ > --- > > (Updated April 21, 2016, 3:45 p.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > Added agent authorization flags. > > > Diffs > - > > docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 > src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956 > src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea > src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a > src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 > src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 > src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4 > src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f > src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b > src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb > > Diff: https://reviews.apache.org/r/45922/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 46203: Added authorization of the '/flags' endpoint.
> On April 20, 2016, 10:35 a.m., Adam B wrote: > > src/tests/slave_authorization_tests.cpp, line 49 > > <https://reviews.apache.org/r/46203/diff/9/?file=1350395#file1350395line49> > > > > Any reason why these shouldn't just go in authorization_tests.cpp? I'm following the pattern established with `authorization_tests.cpp` vs `master_authorization_tests.cpp`: I'd consider the tests in `authorization_tests.cpp` the unit tests of the various actions and `master_authorization_tests.cpp` the integration tests, putting those actions into use. Because there wasn't an equivalent for agents yet, I created it. > On April 20, 2016, 10:35 a.m., Adam B wrote: > > src/tests/slave_authorization_tests.cpp, lines 110-114 > > <https://reviews.apache.org/r/46203/diff/9/?file=1350395#file1350395line110> > > > > This seems wrong. You don't even bother to reset the authenticator > > after you're done? > > Why do you even need to unset the authenticator when you already set > > `slaveFlags.authenticate_http = false`? Unfortunately the libprocess environment is shared between tests. An agent sets the libprocess-wide authenticator when initialized but doesn't unset it -- which is good, because otherwise tests instanciating multiple agents may not work correctly. As a consequence, the authenticator may still be set when this test case is started, forcing us to have to explicitly unset it here. > On April 20, 2016, 10:35 a.m., Adam B wrote: > > src/slave/http.cpp, line 354 > > <https://reviews.apache.org/r/46203/diff/9/?file=1350392#file1350392line354> > > > > Forgive my lambda-ignorance here, but are you creating this locally > > scoped pointer just so that you can expose it to the lambda? Can you not > > reference `slave` directly? I can't use `slave` directly because it's a member of `this`. We can't capture `this` in the lambda because it may be no longer known when the lambda is called, resulting in a segfault. > On April 20, 2016, 10:35 a.m., Adam B wrote: > > src/tests/slave_authorization_tests.cpp, line 119 > > <https://reviews.apache.org/r/46203/diff/9/?file=1350395#file1350395line119> > > > > You shouldn't need to change these I need to, because the agent would fail to start otherwise, see https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L439 - Jan ------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46203/#review129696 --- On April 19, 2016, 2:08 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46203/ > --- > > (Updated April 19, 2016, 2:08 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > --- > > See summary. > > > Diffs > - > > docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 > include/mesos/authorizer/acls.proto > c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 > include/mesos/authorizer/authorizer.proto > 40d93ea257d1df8d22eee8a21667db90d579a8fe > src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac > src/authorizer/local/authorizer.cpp > c744a16041c2466d3839a37fbee2bf86887bf4e1 > src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 > src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb > src/tests/slave_authorization_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46203/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 45922: Added agent authorization flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 21, 2016, 4:48 p.m.) Review request for mesos, Adam B and Alexander Rojas. Bugs: MESOS-5142 https://issues.apache.org/jira/browse/MESOS-5142 Repository: mesos Description (updated) --- See summary. Diffs - docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956 src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4 src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb Diff: https://reviews.apache.org/r/45922/diff/ Testing --- make check Thanks, Jan Schlicht
Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check & external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 2:48 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check & external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 6:55 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check & external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
> On July 17, 2015, 4:59 a.m., Adam B wrote: > > src/launcher/fetcher.cpp, lines 127-128 > > <https://reviews.apache.org/r/36547/diff/3/?file=1013405#file1013405line127> > > > > Why not just check for any 2xx code then? Aren't they all successful in > > one way or another? HTTP can return 206, which means that only a part of the content has been (successfully) transferred. Therefore we should really check for the designated return codes that indicate a successful transfer of a whole resource. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92015 ------- On July 16, 2015, 6:55 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36547/ > --- > > (Updated July 16, 2015, 6:55 p.m.) > > > Review request for mesos, Bernd Mathiske and Joerg Schad. > > > Bugs: MESOS-3060 > https://issues.apache.org/jira/browse/MESOS-3060 > > > Repository: mesos > > > Description > --- > > The response code for successful FTP file transfers is 226, while it is 200 > for HTTP. The fetcher has been changed to check for a response code of 226 > for FTP URIs. > > > Diffs > - > > src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd > > Diff: https://reviews.apache.org/r/36547/diff/ > > > Testing > --- > > make check & external FTP server test. > > > Thanks, > > Jan Schlicht > >
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
> On July 16, 2015, 7:41 p.m., Joerg Schad wrote: > > src/launcher/fetcher.cpp, line 134 > > <https://reviews.apache.org/r/36547/diff/3/?file=1013405#file1013405line134> > > > > I assume those are spaces? Could you please doublecheck? These are spaces. It's probably like this to indicate that the spacing changed. > On July 16, 2015, 7:41 p.m., Joerg Schad wrote: > > src/launcher/fetcher.cpp, line 136 > > <https://reviews.apache.org/r/36547/diff/3/?file=1013405#file1013405line136> > > > > can you add a short comment why we assume this here? Added a check to narrow the supported URI protocols. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review91913 ------- On July 16, 2015, 6:55 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36547/ > --- > > (Updated July 16, 2015, 6:55 p.m.) > > > Review request for mesos, Bernd Mathiske and Joerg Schad. > > > Bugs: MESOS-3060 > https://issues.apache.org/jira/browse/MESOS-3060 > > > Repository: mesos > > > Description > --- > > The response code for successful FTP file transfers is 226, while it is 200 > for HTTP. The fetcher has been changed to check for a response code of 226 > for FTP URIs. > > > Diffs > - > > src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd > > Diff: https://reviews.apache.org/r/36547/diff/ > > > Testing > --- > > make check & external FTP server test. > > > Thanks, > > Jan Schlicht > >
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 10:32 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check & external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 10:40 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check & external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 10:56 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Changes --- Remove redundant CHECK. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check & external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 11:05 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check & external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 20, 2015, 4:21 p.m.) Review request for mesos and Joerg Schad. Repository: mesos Description --- Improved task reconciliation documentation. Diffs (updated) - docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 21, 2015, 11:29 a.m.) Review request for mesos and Joerg Schad. Repository: mesos Description (updated) --- Improved task reconciliation documentation. Diffs - docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 22, 2015, 11:14 a.m.) Review request for mesos and Joerg Schad. Repository: mesos Description --- Improved task reconciliation documentation. Diffs (updated) - 3rdparty/libprocess/3rdparty/CMakeLists.txt fc6125edb66142d26758ebf3feeb6eb68f65620f 3rdparty/libprocess/CMakeLists.txt 6bc5a687fbf78327a16d34ee0bddac40d0020f70 3rdparty/libprocess/cmake/ProcessConfigure.cmake cb5fd1d2cdbffaa65a7951fe6b74c035dc30ca71 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake d349d2e1432cd4317bed17eb9580b416b7e1c766 3rdparty/libprocess/cmake/macros/External.cmake e3901b67048f1c028216ae8323ee1c318a46f3cc 3rdparty/libprocess/cmake/macros/PatchCommand.cmake 12ee3f177a490c9b2319ed865aa3bb73019dad0d 3rdparty/libprocess/include/process/io.hpp 975923f40f82357f31b89428f24d01df6a8ac9fc 3rdparty/libprocess/src/CMakeLists.txt 9d1b1f54d63b3c4740550c2b7934200c54b48021 3rdparty/libprocess/src/tests/CMakeLists.txt 56b1861d6623b720ed603c6de64562f9a787d5d8 CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 cmake/MesosConfigure.cmake 16b72f1aa135666174cf10d73d52443d51529c6c docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 include/mesos/hook.hpp bb5a635dcf189e1023f1eec66fc06955f816fc0b include/mesos/mesos.proto bcb38d934c7f223b9a23746b273e581e0e8da886 include/mesos/slave/isolator.hpp 85e38f5e4aa66527f1756fa259b93389f45028b3 src/Makefile.am 489ddb424b342635c3dbc4d14ff5d69ce76a237b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/common/protobuf_utils.hpp 2e827a0923de83d5cf853a12435b451cc7c55891 src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/examples/test_hook_module.cpp c664b565bcf18dd2153205990119cc679e4ad6cf src/hook/manager.hpp 8153ce4826f94d5771c93d37c59fdc4991352e66 src/hook/manager.cpp 11e6b0a2c0df1d0d7039aaad94e1c6f0e5cc6bc2 src/linux/cgroups.hpp a651f3434b908b54d217117933740d52dbe50adf src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 src/slave/containerizer/isolators/filesystem/posix.hpp 16ba26f4f5b515acbeb3c4d514d4eecf2f277df8 src/slave/containerizer/isolators/filesystem/posix.cpp 1904279c92ef00ef931c909b4bb15bef89a4fc59 src/slave/containerizer/mesos/containerizer.hpp f6c580d1b629ee799977cc8824f337764d893c5f src/slave/containerizer/mesos/containerizer.cpp 609620c4322e41562597ee682b311cd320bca6d2 src/slave/containerizer/provisioner.hpp f7fb068ca5b0a8da1fb756411d59536ed7a1aec8 src/slave/containerizer/provisioner.cpp df52e36b23ad3cd28f50e96865d0b163cc245cb2 src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e src/slave/paths.hpp c7f85f188d9bd4c8d3dc194adff0cf9065fc400a src/slave/paths.cpp 404c2143e70771747d356b15eac9137495fd9a75 src/slave/slave.cpp dc12c45516ab39d74a5c29b657f22f74d0acf24e src/slave/state.hpp 4e00468a777145e3c61b8dee7dfe496f8d65b0e4 src/slave/state.cpp b9f2d8a0d6395b92bd50f1e0927f3ae9fd04b81c src/tests/cgroups_tests.cpp b63d956b9dafb2c485080ff5e016e2a05f03db15 src/tests/containerizer_tests.cpp 29114e7322b9239fb3e5f4921f542bd991fd426e src/tests/docker_containerizer_tests.cpp 5086af376e3f22726328fdb9618307fa9e84d6f8 src/tests/hook_tests.cpp 86e53d8d4609c483b676cef471512dc53f595dd0 src/tests/master_tests.cpp 8b8d3865ee1baf03d013a1357bfdd3088828e799 src/tests/mesos.hpp 69134e1c2664ca24a1ecd80a662c841311104a6a src/tests/mesos.cpp f09ef0f99573716de8905f49dcc0c9df20e31ea9 src/tests/slave_tests.cpp e1390ad84b0003052681600deb9ca518defc0970 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
Re: Review Request 36617: Improved task reconciliation documentation.
> On July 21, 2015, 7:28 p.m., Vinod Kone wrote: > > docs/reconciliation.md, line 96 > > <https://reviews.apache.org/r/36617/diff/2/?file=1016806#file1016806line96> > > > > what about other terminal states? Of course! I had only the TASK_LOST send during explicit reconciliation in mind. Other terminal states are now mentioned as well. > On July 21, 2015, 7:28 p.m., Vinod Kone wrote: > > docs/reconciliation.md, line 98 > > <https://reviews.apache.org/r/36617/diff/2/?file=1016806#file1016806line98> > > > > what does this mean? > > > > s/Let/Ask/ ? It means to kill the task using `SchedulerDriver::killTask`. Another option might be to try to add this task to the scheduler's internal state. But this will probably only work for trivial schedulers as more informations than the task state reported from the master are neccessary to represent a task in the scheduler. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/#review92436 ------- On July 22, 2015, 11:14 a.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36617/ > --- > > (Updated July 22, 2015, 11:14 a.m.) > > > Review request for mesos and Joerg Schad. > > > Repository: mesos > > > Description > --- > > Improved task reconciliation documentation. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/CMakeLists.txt > fc6125edb66142d26758ebf3feeb6eb68f65620f > 3rdparty/libprocess/CMakeLists.txt 6bc5a687fbf78327a16d34ee0bddac40d0020f70 > 3rdparty/libprocess/cmake/ProcessConfigure.cmake > cb5fd1d2cdbffaa65a7951fe6b74c035dc30ca71 > 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake > d349d2e1432cd4317bed17eb9580b416b7e1c766 > 3rdparty/libprocess/cmake/macros/External.cmake > e3901b67048f1c028216ae8323ee1c318a46f3cc > 3rdparty/libprocess/cmake/macros/PatchCommand.cmake > 12ee3f177a490c9b2319ed865aa3bb73019dad0d > 3rdparty/libprocess/include/process/io.hpp > 975923f40f82357f31b89428f24d01df6a8ac9fc > 3rdparty/libprocess/src/CMakeLists.txt > 9d1b1f54d63b3c4740550c2b7934200c54b48021 > 3rdparty/libprocess/src/tests/CMakeLists.txt > 56b1861d6623b720ed603c6de64562f9a787d5d8 > CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 > cmake/MesosConfigure.cmake 16b72f1aa135666174cf10d73d52443d51529c6c > docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 > include/mesos/hook.hpp bb5a635dcf189e1023f1eec66fc06955f816fc0b > include/mesos/mesos.proto bcb38d934c7f223b9a23746b273e581e0e8da886 > include/mesos/slave/isolator.hpp 85e38f5e4aa66527f1756fa259b93389f45028b3 > src/Makefile.am 489ddb424b342635c3dbc4d14ff5d69ce76a237b > src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 > src/common/protobuf_utils.hpp 2e827a0923de83d5cf853a12435b451cc7c55891 > src/common/protobuf_utils.cpp e0f82b53f5e106bbf4e21d6ac946df0fae821882 > src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e > src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 > src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 > src/examples/test_hook_module.cpp c664b565bcf18dd2153205990119cc679e4ad6cf > src/hook/manager.hpp 8153ce4826f94d5771c93d37c59fdc4991352e66 > src/hook/manager.cpp 11e6b0a2c0df1d0d7039aaad94e1c6f0e5cc6bc2 > src/linux/cgroups.hpp a651f3434b908b54d217117933740d52dbe50adf > src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 > src/slave/containerizer/isolators/filesystem/posix.hpp > 16ba26f4f5b515acbeb3c4d514d4eecf2f277df8 > src/slave/containerizer/isolators/filesystem/posix.cpp > 1904279c92ef00ef931c909b4bb15bef89a4fc59 > src/slave/containerizer/mesos/containerizer.hpp > f6c580d1b629ee799977cc8824f337764d893c5f > src/slave/containerizer/mesos/containerizer.cpp > 609620c4322e41562597ee682b311cd320bca6d2 > src/slave/containerizer/provisioner.hpp > f7fb068ca5b0a8da1fb756411d59536ed7a1aec8 > src/slave/containerizer/provisioner.cpp > df52e36b23ad3cd28f50e96865d0b163cc245cb2 > src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 > src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e > src/slave/paths.hpp c7f85f188d9bd4c8d3dc194adff0cf9065fc400a > src/slave/paths.cpp 404c2143e70771747d356b15eac9137495fd9a75 > src/slave/slave.cpp dc12c45516ab39d74a5c29b657f22f74d0acf24e > src/slave/st
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 22, 2015, 11:21 a.m.) Review request for mesos and Joerg Schad. Repository: mesos Description --- Improved task reconciliation documentation. Diffs (updated) - docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 22, 2015, 11:41 a.m.) Review request for mesos and Joerg Schad. Bugs: MESOS-3127 https://issues.apache.org/jira/browse/MESOS-3127 Repository: mesos Description --- Improved task reconciliation documentation. Diffs - docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 23, 2015, 1:48 p.m.) Review request for mesos and Joerg Schad. Bugs: MESOS-3127 https://issues.apache.org/jira/browse/MESOS-3127 Repository: mesos Description --- Improved task reconciliation documentation. Diffs (updated) - docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
Re: Review Request 36617: Improved task reconciliation documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36617/ --- (Updated July 23, 2015, 2:01 p.m.) Review request for mesos and Joerg Schad. Bugs: MESOS-3127 https://issues.apache.org/jira/browse/MESOS-3127 Repository: mesos Description --- Improved task reconciliation documentation. Diffs (updated) - docs/reconciliation.md 17537ba8420c95d833e64ccf82ff9bb4530497f0 Diff: https://reviews.apache.org/r/36617/diff/ Testing --- https://gist.github.com/nfnt/73532d62fe39d27ff33d Thanks, Jan Schlicht
Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- Review request for mesos. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs - src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 27, 2015, 2:52 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Changes --- Fix test failure with Linux. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
> On July 24, 2015, 2:14 p.m., Joerg Schad wrote: > > src/slave/containerizer/fetcher.cpp, line 997 > > <https://reviews.apache.org/r/36773/diff/1/?file=1020832#file1020832line997> > > > > This new behavior should be reflected in the documentation. AFAIK there > > are two markdown files which would need updating.. Cache eviction is mentioned in fetcher.md and fetcher-cache-internals.md. As fetcher.md does not describe how cache eviction is done, only that it is done, it is not needed to mention LRU here. On the other hand it is now mentioned in fetcher-cache-internals.md, because this file describes what is done during cache eviction. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review92895 ------- On July 27, 2015, 5:38 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36773/ > --- > > (Updated July 27, 2015, 5:38 p.m.) > > > Review request for mesos, Bernd Mathiske and Joerg Schad. > > > Bugs: MESOS-3112 > https://issues.apache.org/jira/browse/MESOS-3112 > > > Repository: mesos > > > Description > --- > > A linked list is used to keep cache entries in LRU-to-MRU order. Each time an > existing cache entry is requested, it is moved to the back of the list. > During cache eviction entries are removed from the front of the list until > enough cache space can be freed. > > > Diffs > - > > docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b > src/slave/containerizer/fetcher.hpp > 17225072ba5c1c9a7209f2923bcf562fcb76201f > src/slave/containerizer/fetcher.cpp > e030deabd5e749100cbccabb256dbd4af8b2fe58 > src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 > > Diff: https://reviews.apache.org/r/36773/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 27, 2015, 5:38 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Changes --- Document LRU cache eviction in fetcher-cache-internals.md Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 28, 2015, 2:16 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
> On July 28, 2015, 1:42 p.m., Joerg Schad wrote: > > src/slave/containerizer/fetcher.cpp, line 987 > > <https://reviews.apache.org/r/36773/diff/3/?file=1022383#file1022383line987> > > > > Should we put a comment above the function saying that it is LRU based? Yes, we should! > On July 28, 2015, 1:42 p.m., Joerg Schad wrote: > > src/tests/fetcher_cache_tests.cpp, line 1463 > > <https://reviews.apache.org/r/36773/diff/3/?file=1022384#file1022384line1463> > > > > Just as a comment: I find cacheSize() very unintuive name for this > > function, would have expected something along cacheFiles().size() or > > numberEntries() +1 - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93272 ------- On July 28, 2015, 2:16 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36773/ > --- > > (Updated July 28, 2015, 2:16 p.m.) > > > Review request for mesos, Bernd Mathiske and Joerg Schad. > > > Bugs: MESOS-3112 > https://issues.apache.org/jira/browse/MESOS-3112 > > > Repository: mesos > > > Description > --- > > A linked list is used to keep cache entries in LRU-to-MRU order. Each time an > existing cache entry is requested, it is moved to the back of the list. > During cache eviction entries are removed from the front of the list until > enough cache space can be freed. > > > Diffs > - > > docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b > src/slave/containerizer/fetcher.hpp > 17225072ba5c1c9a7209f2923bcf562fcb76201f > src/slave/containerizer/fetcher.cpp > e030deabd5e749100cbccabb256dbd4af8b2fe58 > src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 > > Diff: https://reviews.apache.org/r/36773/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 36889: FetcherTests: Use ASSERT instead of EXPECT if the subsequent logic relies on the outcome.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36889/#review93288 --- src/tests/fetcher_cache_tests.cpp (line 368) <https://reviews.apache.org/r/36889/#comment147669> s/EXPECT_NE/ASSERT_NE src/tests/fetcher_cache_tests.cpp (line 568) <https://reviews.apache.org/r/36889/#comment147661> s/EXPECT_SOME/ASSERT_SOME, as Try::isSome() is expected true in the next line. There are probably more cases like this. src/tests/fetcher_cache_tests.cpp (line 606) <https://reviews.apache.org/r/36889/#comment147662> s/EXPECT_SOME/ASSERT_SOME src/tests/fetcher_cache_tests.cpp (line 656) <https://reviews.apache.org/r/36889/#comment147664> s/EXPECT_SOME/ASSERT_SOME src/tests/fetcher_cache_tests.cpp (line 693) <https://reviews.apache.org/r/36889/#comment147665> s/EXPECT_SOME/ASSERT_SOME src/tests/fetcher_cache_tests.cpp (line 729) <https://reviews.apache.org/r/36889/#comment147666> s/EXPECT_SOME/ASSERT_SOME src/tests/fetcher_cache_tests.cpp (line 867) <https://reviews.apache.org/r/36889/#comment147667> s/EXPECT_SOME/ASSERT_SOME src/tests/fetcher_cache_tests.cpp (line 930) <https://reviews.apache.org/r/36889/#comment147668> s/EXPECT_SOME/ASSERT_SOME - Jan Schlicht On July 28, 2015, 2:52 p.m., Joerg Schad wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36889/ > --- > > (Updated July 28, 2015, 2:52 p.m.) > > > Review request for mesos, Bernd Mathiske and Jan Schlicht. > > > Bugs: MESOS-3112 > https://issues.apache.org/jira/browse/MESOS-3112 > > > Repository: mesos > > > Description > --- > > FetcherTests: Use ASSERT instead of EXPECT if the subsequent logic relies on > the outcome. > > > Diffs > - > > src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 > > Diff: https://reviews.apache.org/r/36889/diff/ > > > Testing > --- > > make check > > > Thanks, > > Joerg Schad > >
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 28, 2015, 4:04 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
> On July 28, 2015, 1:42 p.m., Joerg Schad wrote: > > src/tests/fetcher_cache_tests.cpp, line 1450 > > <https://reviews.apache.org/r/36773/diff/3/?file=1022384#file1022384line1450> > > > > Is this an expectation or assertion? IMO the test shouldn't continue if > > this fails. (I see that it is the same pattern in other tests, still > > doens't mean it is correct ;-) ) Line 1469 is the same case and has been changed as well. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93272 ------- On July 28, 2015, 4:04 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36773/ > --- > > (Updated July 28, 2015, 4:04 p.m.) > > > Review request for mesos, Bernd Mathiske and Joerg Schad. > > > Bugs: MESOS-3112 > https://issues.apache.org/jira/browse/MESOS-3112 > > > Repository: mesos > > > Description > --- > > A linked list is used to keep cache entries in LRU-to-MRU order. Each time an > existing cache entry is requested, it is moved to the back of the list. > During cache eviction entries are removed from the front of the list until > enough cache space can be freed. > > > Diffs > - > > docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b > src/slave/containerizer/fetcher.hpp > 17225072ba5c1c9a7209f2923bcf562fcb76201f > src/slave/containerizer/fetcher.cpp > e030deabd5e749100cbccabb256dbd4af8b2fe58 > src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 > > Diff: https://reviews.apache.org/r/36773/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 36889: FetcherTests: Use ASSERT instead of EXPECT if the subsequent logic relies on the outcome.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36889/#review93297 --- Ship it! Ship It! - Jan Schlicht On July 28, 2015, 3:48 p.m., Joerg Schad wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36889/ > --- > > (Updated July 28, 2015, 3:48 p.m.) > > > Review request for mesos, Bernd Mathiske and Jan Schlicht. > > > Bugs: MESOS-3112 > https://issues.apache.org/jira/browse/MESOS-3112 > > > Repository: mesos > > > Description > --- > > FetcherTests: Use ASSERT instead of EXPECT if the subsequent logic relies on > the outcome. > > > Diffs > - > > src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 > > Diff: https://reviews.apache.org/r/36889/diff/ > > > Testing > --- > > make check > > > Thanks, > > Joerg Schad > >
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 1:52 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 2:59 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93447 --- src/tests/fetcher_cache_tests.cpp (line 1474) <https://reviews.apache.org/r/36773/#comment147817> Path::basename() is not marked const (which it probably should), hence we have to iterate over Path by value. - Jan Schlicht On July 29, 2015, 3:59 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36773/ > --- > > (Updated July 29, 2015, 3:59 p.m.) > > > Review request for mesos, Bernd Mathiske and Joerg Schad. > > > Bugs: MESOS-3112 > https://issues.apache.org/jira/browse/MESOS-3112 > > > Repository: mesos > > > Description > --- > > A linked list is used to keep cache entries in LRU-to-MRU order. Each time an > existing cache entry is requested, it is moved to the back of the list. > During cache eviction entries are removed from the front of the list until > enough cache space can be freed. > > > Diffs > - > > docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b > src/slave/containerizer/fetcher.hpp > 17225072ba5c1c9a7209f2923bcf562fcb76201f > src/slave/containerizer/fetcher.cpp > e030deabd5e749100cbccabb256dbd4af8b2fe58 > src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 > > Diff: https://reviews.apache.org/r/36773/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 3:59 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
> On July 29, 2015, 3:34 p.m., Bernd Mathiske wrote: > > src/tests/fetcher_cache_tests.cpp, line 1474 > > <https://reviews.apache.org/r/36773/diff/7/?file=1024458#file1024458line1474> > > > > const Path& Path::basename() is not marked const (which it probably should), hence we have to iterate over Path by value. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93443 ------- On July 29, 2015, 3:59 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36773/ > --- > > (Updated July 29, 2015, 3:59 p.m.) > > > Review request for mesos, Bernd Mathiske and Joerg Schad. > > > Bugs: MESOS-3112 > https://issues.apache.org/jira/browse/MESOS-3112 > > > Repository: mesos > > > Description > --- > > A linked list is used to keep cache entries in LRU-to-MRU order. Each time an > existing cache entry is requested, it is moved to the back of the list. > During cache eviction entries are removed from the front of the list until > enough cache space can be freed. > > > Diffs > - > > docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b > src/slave/containerizer/fetcher.hpp > 17225072ba5c1c9a7209f2923bcf562fcb76201f > src/slave/containerizer/fetcher.cpp > e030deabd5e749100cbccabb256dbd4af8b2fe58 > src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 > > Diff: https://reviews.apache.org/r/36773/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
> On July 29, 2015, 3:34 p.m., Bernd Mathiske wrote: > > src/tests/fetcher_cache_tests.cpp, line 1474 > > <https://reviews.apache.org/r/36773/diff/7/?file=1024458#file1024458line1474> > > > > const Path& > > Jan Schlicht wrote: > Path::basename() is not marked const (which it probably should), hence we > have to iterate over Path by value. Created a ticket for that: https://issues.apache.org/jira/browse/MESOS-3173 - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93443 ------- On July 29, 2015, 3:59 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36773/ > --- > > (Updated July 29, 2015, 3:59 p.m.) > > > Review request for mesos, Bernd Mathiske and Joerg Schad. > > > Bugs: MESOS-3112 > https://issues.apache.org/jira/browse/MESOS-3112 > > > Repository: mesos > > > Description > --- > > A linked list is used to keep cache entries in LRU-to-MRU order. Each time an > existing cache entry is requested, it is moved to the back of the list. > During cache eviction entries are removed from the front of the list until > enough cache space can be freed. > > > Diffs > - > > docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b > src/slave/containerizer/fetcher.hpp > 17225072ba5c1c9a7209f2923bcf562fcb76201f > src/slave/containerizer/fetcher.cpp > e030deabd5e749100cbccabb256dbd4af8b2fe58 > src/tests/fetcher_cache_tests.cpp bd9c406a532a85fa95a5e9cfa6003f4893191c57 > > Diff: https://reviews.apache.org/r/36773/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Review Request 36914: Marked Path::basename, Path::dirname const.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36914/ --- Review request for mesos and Till Toenshoff. Bugs: MESOS-3173 https://issues.apache.org/jira/browse/MESOS-3173 Repository: mesos Description --- Marked Path::basename, Path::dirname const. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp d58f8a4ab8f6f761adb6a9db2dac438ffe0e211b Diff: https://reviews.apache.org/r/36914/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated July 29, 2015, 6:34 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Changes --- Use `const Path&`. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp 4e1d348c412c0cec6a73d6e43f2c7124db1f3bfe Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
> On July 29, 2015, 3:34 p.m., Bernd Mathiske wrote: > > src/tests/fetcher_cache_tests.cpp, line 1474 > > <https://reviews.apache.org/r/36773/diff/7/?file=1024458#file1024458line1474> > > > > const Path& > > Jan Schlicht wrote: > Path::basename() is not marked const (which it probably should), hence we > have to iterate over Path by value. > > Jan Schlicht wrote: > Created a ticket for that: > https://issues.apache.org/jira/browse/MESOS-3173 Path::basename() is const since 3eb17f6. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/#review93443 ------- On July 29, 2015, 6:34 p.m., Jan Schlicht wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36773/ > --- > > (Updated July 29, 2015, 6:34 p.m.) > > > Review request for mesos, Bernd Mathiske and Joerg Schad. > > > Bugs: MESOS-3112 > https://issues.apache.org/jira/browse/MESOS-3112 > > > Repository: mesos > > > Description > --- > > A linked list is used to keep cache entries in LRU-to-MRU order. Each time an > existing cache entry is requested, it is moved to the back of the list. > During cache eviction entries are removed from the front of the list until > enough cache space can be freed. > > > Diffs > - > > docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b > src/slave/containerizer/fetcher.hpp > 17225072ba5c1c9a7209f2923bcf562fcb76201f > src/slave/containerizer/fetcher.cpp > e030deabd5e749100cbccabb256dbd4af8b2fe58 > src/tests/fetcher_cache_tests.cpp 4e1d348c412c0cec6a73d6e43f2c7124db1f3bfe > > Diff: https://reviews.apache.org/r/36773/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jan Schlicht > >
Re: Review Request 36773: Implemented a LRU entry selection criteria for cache eviction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36773/ --- (Updated Aug. 3, 2015, 10:43 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3112 https://issues.apache.org/jira/browse/MESOS-3112 Repository: mesos Description --- A linked list is used to keep cache entries in LRU-to-MRU order. Each time an existing cache entry is requested, it is moved to the back of the list. During cache eviction entries are removed from the front of the list until enough cache space can be freed. Diffs (updated) - docs/fetcher-cache-internals.md 327cbc3074bcc110e1250e2151dd5401ccaadb4b src/slave/containerizer/fetcher.hpp 17225072ba5c1c9a7209f2923bcf562fcb76201f src/slave/containerizer/fetcher.cpp e030deabd5e749100cbccabb256dbd4af8b2fe58 src/tests/fetcher_cache_tests.cpp 4e1d348c412c0cec6a73d6e43f2c7124db1f3bfe Diff: https://reviews.apache.org/r/36773/diff/ Testing --- make check Thanks, Jan Schlicht
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/#review93912 --- src/authorizer/authorizer.cpp (line 209) <https://reviews.apache.org/r/36048/#comment148352> I stumbled over this syntax. While it's valid C++11, I'd suggest using `Authorizer* local = new LocalAuthorizer` as that's easier to grok. - Jan Schlicht On Aug. 3, 2015, 11:47 a.m., Alexander Rojas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36048/ > --- > > (Updated Aug. 3, 2015, 11:47 a.m.) > > > Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil > Arya, Jan Schlicht, and Till Toenshoff. > > > Bugs: MESOS-2946 > https://issues.apache.org/jira/browse/MESOS-2946 > > > Repository: mesos > > > Description > --- > > Splits and updates the original declaration of the `Authorizer` into its > interface and a default implementation, the `LocalAuthorizer`. > > Following the pattern of the modularized `Authenticator`, it generates a > default constructor which is required when writing a `TYPED_TEST` in > a follow up patch. Additionally, an initialize method has been added, needed > for passing in the current ACL definitions as provided via > flags. > > Other changes are just updates to allow for compilation. > > > Diffs > - > > include/mesos/authorizer/authorizer.hpp PRE-CREATION > include/mesos/authorizer/authorizer.proto PRE-CREATION > include/mesos/mesos.proto cb24125a3f05e0d38fb22e481a15ceb21f882d27 > include/mesos/type_utils.hpp 86b37ca1f63e4687af4e86731dceeb1c9c219c5e > src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784 > src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f > src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc > src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 > src/examples/persistent_volume_framework.cpp > c6d6ed337bfca91dc146cb31298cabebdbb13509 > src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff > src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f > src/master/http.cpp 6f5ca02c52462495b84e77525a6c88299746ece2 > src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 > src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec > src/master/master.cpp 2f00f240ed2cd59ec0c2eae7fd2567f0edb8d9e0 > src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 > src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b > src/tests/mesos.hpp 23d9841c7cfb16abef89a53d29a256a2c5e94b52 > src/tests/mesos.cpp f09ef0f99573716de8905f49dcc0c9df20e31ea9 > > Diff: https://reviews.apache.org/r/36048/diff/ > > > Testing > --- > > make check > > > Thanks, > > Alexander Rojas > >
