Re: Review Request 43336: Introduced Appc image fetcher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43336/ --- (Updated Feb. 16, 2016, 11:36 p.m.) Review request for mesos and Jie Yu. Changes --- Review addressed. Bugs: MESOS-3193 https://issues.apache.org/jira/browse/MESOS-3193 Repository: mesos Description --- Added implementation for simple image discovery for Appc images. TODO: Add tests. Diffs (updated) - src/CMakeLists.txt 9ab84c0898b3adce6063cc50b04ee74cf1471609 src/Makefile.am 5813ab2c33a7de6b612064e894e5f15b5a474e2b src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp PRE-CREATION src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp PRE-CREATION src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 Diff: https://reviews.apache.org/r/43336/diff/ Testing --- Tested with local http(s) server. Thanks, Jojy Varghese
Review Request 43638: Fixed minor style issues in command_utils.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43638/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Made return statements and lambdas consistent throughout the file. Diffs - src/common/command_utils.cpp 68b7facf62b867ac831ae616dad6623ac271110e Diff: https://reviews.apache.org/r/43638/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 43775: Fixed bug in appc cache's find logic.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43775/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Fixed bug in appc cache's find logic. Diffs - src/slave/containerizer/mesos/provisioner/appc/cache.cpp ff392efb2e0e50ac9ae5e5a8f19b57202439f74e Diff: https://reviews.apache.org/r/43775/diff/ Testing --- Simple integration testing with store. Thanks, Jojy Varghese
Review Request 43855: Added Appc fetcher support to store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43855/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- This change allows store to fetch an image using Appc image fetcher when an image is not found in the cache. It also recursively fetches the dependencies for the image. Diffs - src/slave/containerizer/mesos/provisioner/appc/store.cpp 4b3829175f57fb9aea2478040d96f2f127cbc551 Diff: https://reviews.apache.org/r/43855/diff/ Testing --- make check; local image server. Thanks, Jojy Varghese
Re: Review Request 43855: Added Appc fetcher support to store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43855/ --- (Updated Feb. 23, 2016, 3:30 p.m.) Review request for mesos and Jie Yu. Changes --- addressed review. Repository: mesos Description --- This change allows store to fetch an image using Appc image fetcher when an image is not found in the cache. It also recursively fetches the dependencies for the image. Diffs (updated) - src/slave/containerizer/mesos/provisioner/appc/store.cpp 4b3829175f57fb9aea2478040d96f2f127cbc551 src/tests/fetcher_cache_tests.cpp e10b3f7ebc21c8c1095564fc40f123087dcf320e Diff: https://reviews.apache.org/r/43855/diff/ Testing --- make check; local image server. Thanks, Jojy Varghese
Re: Review Request 43860: Used uri::Fetcher to pull docker images in docker registry puller.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43860/#review120367 --- src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp (line 214) <https://reviews.apache.org/r/43860/#comment181802> Not yours but do we need a CHECK here or a graceful failure. src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp (line 256) <https://reviews.apache.org/r/43860/#comment181803> Does not apply for this patch, but might have to change the port type to posix type `in_port_t ` - Jojy Varghese On Feb. 23, 2016, 12:54 a.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43860/ > --- > > (Updated Feb. 23, 2016, 12:54 a.m.) > > > Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen. > > > Bugs: MESOS-4261 and MESOS-4499 > https://issues.apache.org/jira/browse/MESOS-4261 > https://issues.apache.org/jira/browse/MESOS-4499 > > > Repository: mesos > > > Description > --- > > This is the second step of cleaning/simplying the docker registry puller code. > > The new code uses the uri::Fetcher to download docker manifest and blobs > (instead of writting our own http logic which requires enabling of SSL for > Mesos). > > > Diffs > - > > src/slave/containerizer/mesos/provisioner/docker/puller.hpp > 5b2d72c22fcbcc379b4901607cf3eb682de66206 > src/slave/containerizer/mesos/provisioner/docker/puller.cpp > a239b97557ad20353c67050dbc89ef16da898330 > src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp > bccbac3357cf942446604e6cf5d16c3d594b > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp > 3fcf1471a035e35a2cac22442655ad65a84a9793 > src/slave/containerizer/mesos/provisioner/docker/store.cpp > 2f1d3e002140f34c646aab445a419c9c3d712f99 > src/slave/flags.hpp 54c1a69d8777f417cdd8f73ce638447d9951ab61 > src/slave/flags.cpp 855812e9f7cb4b96d4297f4bd5ac5de7f1d3c39a > src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e > src/tests/containerizer/provisioner_docker_tests.cpp > 4db6793a21abbb7ea4de0d0fca0431237d38d013 > > Diff: https://reviews.apache.org/r/43860/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jie Yu > >
Re: Review Request 43855: Added Appc fetcher support to store.
> On Feb. 23, 2016, 5:21 a.m., Guangya Liu wrote: > > src/slave/containerizer/mesos/provisioner/appc/store.cpp, line 334 > > <https://reviews.apache.org/r/43855/diff/1/?file=1264857#file1264857line334> > > > > s/Fetches/Fetch Since i am describing what the function does, I believe the function "fetches" the image descibes what it does the best. Having said that, I am not a grammer expert and my arms could be twisted :) - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43855/#review120260 --- On Feb. 23, 2016, 3:30 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43855/ > --- > > (Updated Feb. 23, 2016, 3:30 p.m.) > > > Review request for mesos and Jie Yu. > > > Repository: mesos > > > Description > --- > > This change allows store to fetch an image using Appc image fetcher when an > image is not found in the cache. It also recursively fetches the dependencies > for the image. > > > Diffs > - > > src/slave/containerizer/mesos/provisioner/appc/store.cpp > 4b3829175f57fb9aea2478040d96f2f127cbc551 > src/tests/fetcher_cache_tests.cpp e10b3f7ebc21c8c1095564fc40f123087dcf320e > > Diff: https://reviews.apache.org/r/43855/diff/ > > > Testing > --- > > make check; local image server. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 43960: Added an end-to-end test for docker registry puller.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43960/#review120565 --- src/tests/containerizer/provisioner_docker_tests.cpp (line 360) <https://reviews.apache.org/r/43960/#comment182033> Would it add value if we assert that the store does not currently have the image? Also, have assertions around the image being placed in the right dircetory? src/tests/containerizer/provisioner_docker_tests.cpp (line 365) <https://reviews.apache.org/r/43960/#comment182032> Would it make sense to name these as per their semantics? `status1` does not say a lot about them. - Jojy Varghese On Feb. 24, 2016, 6:51 p.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43960/ > --- > > (Updated Feb. 24, 2016, 6:51 p.m.) > > > Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen. > > > Repository: mesos > > > Description > --- > > Added an end-to-end test for docker registry puller. > > > Diffs > - > > src/tests/containerizer/provisioner_docker_tests.cpp > b3c6f8848f2ceb2bd4dce35d6e7f813c2d4d2bd9 > > Diff: https://reviews.apache.org/r/43960/diff/ > > > Testing > --- > > sudo make check > > > Thanks, > > Jie Yu > >
Re: Review Request 43958: Added a verbose logging in docker registry puller.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43958/#review120566 --- Ship it! Ship It! - Jojy Varghese On Feb. 24, 2016, 6:50 p.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43958/ > --- > > (Updated Feb. 24, 2016, 6:50 p.m.) > > > Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen. > > > Repository: mesos > > > Description > --- > > Added a verbose logging in docker registry puller. > > > Diffs > - > > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp > e08a9597c34fc74b96e7ef35cc1a25429a7f99c7 > > Diff: https://reviews.apache.org/r/43958/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jie Yu > >
Review Request 43969: Added test for Appc image fetcher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43969/ --- Review request for mesos and Jie Yu. Bugs: MESOS-4748 https://issues.apache.org/jira/browse/MESOS-4748 Repository: mesos Description --- Added simple appc Fetcher test with mock HTTP image server. Diffs - src/tests/containerizer/provisioner_appc_tests.cpp 9d9779ac04e32348ce0e28da54fd7aa039a7fbcd Diff: https://reviews.apache.org/r/43969/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 43855: Added Appc fetcher support to store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43855/ --- (Updated Feb. 26, 2016, 4:07 a.m.) Review request for mesos and Jie Yu. Changes --- review addressed. Repository: mesos Description --- This change allows store to fetch an image using Appc image fetcher when an image is not found in the cache. It also recursively fetches the dependencies for the image. Diffs (updated) - src/slave/containerizer/mesos/provisioner/appc/store.cpp 4b3829175f57fb9aea2478040d96f2f127cbc551 src/tests/fetcher_cache_tests.cpp e10b3f7ebc21c8c1095564fc40f123087dcf320e Diff: https://reviews.apache.org/r/43855/diff/ Testing --- make check; local image server. Thanks, Jojy Varghese
Re: Review Request 43855: Added Appc fetcher support to store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43855/ --- (Updated Feb. 26, 2016, 8:36 a.m.) Review request for mesos and Jie Yu. Changes --- review addressed. Bugs: MESOS-3424 https://issues.apache.org/jira/browse/MESOS-3424 Repository: mesos Description --- This change allows store to fetch an image using Appc image fetcher when an image is not found in the cache. It also recursively fetches the dependencies for the image. Diffs (updated) - src/slave/containerizer/mesos/provisioner/appc/store.cpp 4b3829175f57fb9aea2478040d96f2f127cbc551 src/tests/fetcher_cache_tests.cpp e10b3f7ebc21c8c1095564fc40f123087dcf320e Diff: https://reviews.apache.org/r/43855/diff/ Testing --- make check; local image server. Thanks, Jojy Varghese
Re: Review Request 43855: Added Appc fetcher support to store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43855/ --- (Updated Feb. 26, 2016, 10:42 p.m.) Review request for mesos and Jie Yu. Changes --- review addressed. Bugs: MESOS-3424 https://issues.apache.org/jira/browse/MESOS-3424 Repository: mesos Description --- This change allows store to fetch an image using Appc image fetcher when an image is not found in the cache. It also recursively fetches the dependencies for the image. Diffs (updated) - src/slave/containerizer/mesos/provisioner/appc/store.cpp 4b3829175f57fb9aea2478040d96f2f127cbc551 src/tests/fetcher_cache_tests.cpp e10b3f7ebc21c8c1095564fc40f123087dcf320e Diff: https://reviews.apache.org/r/43855/diff/ Testing --- make check; local image server. Thanks, Jojy Varghese
Re: Review Request 43969: Added test for Appc image fetcher.
> On Feb. 27, 2016, 12:21 a.m., Jie Yu wrote: > > src/tests/containerizer/provisioner_appc_tests.cpp, line 539 > > <https://reviews.apache.org/r/43969/diff/1/?file=1268497#file1268497line539> > > > > We might want to serve multiple images. Can you make it more general so > > that we can serve any files under 'server' directory? I thought this is a good start and we can expand as we go. We could add a map of `name->path` in the test fixture. I can add a TODO. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43969/#review121010 --- On Feb. 24, 2016, 11:19 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43969/ > --- > > (Updated Feb. 24, 2016, 11:19 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-4748 > https://issues.apache.org/jira/browse/MESOS-4748 > > > Repository: mesos > > > Description > --- > > Added simple appc Fetcher test with mock HTTP image server. > > > Diffs > - > > src/tests/containerizer/provisioner_appc_tests.cpp > 9d9779ac04e32348ce0e28da54fd7aa039a7fbcd > > Diff: https://reviews.apache.org/r/43969/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 43969: Added test for Appc image fetcher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43969/ --- (Updated Feb. 27, 2016, 4:05 a.m.) Review request for mesos and Jie Yu. Changes --- review addressed. Bugs: MESOS-4748 https://issues.apache.org/jira/browse/MESOS-4748 Repository: mesos Description --- Added simple appc Fetcher test with mock HTTP image server. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 9d9779ac04e32348ce0e28da54fd7aa039a7fbcd Diff: https://reviews.apache.org/r/43969/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44230: Added FS_DEFAULT case in rmdir.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44230/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- We currently dont handle special files like device files in rmdir. This change adds FS_DEFAULT as one of the cases where we try to unlink a file. Reference: http://man7.org/linux/man-pages/man3/fts.3.html Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp bc420c9c10d93ddd619a9eb2c5f4db67f31d722f Diff: https://reviews.apache.org/r/44230/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44231: Added rmdir error string to Appc store fetch.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44231/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Added rmdir error string to Appc store fetch. Diffs - src/slave/containerizer/mesos/provisioner/appc/store.cpp 955647d2e4a26783898042ab80e641db7e4d4980 Diff: https://reviews.apache.org/r/44231/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44239: Introduced copy fetcher plugin for file URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44239/ --- Review request for mesos and Jie Yu. Bugs: MESOS-4834 https://issues.apache.org/jira/browse/MESOS-4834 Repository: mesos Description --- This change adds support for fetching `file` URIs using the fetcher plugin framework. Diffs - src/CMakeLists.txt 0832f1e9a850cc0d2c1112a446f0daf0190f8d7f src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 src/tests/uri_fetcher_tests.cpp dcfba784d6ae49b8181b5a69dc00e82854ac4ff1 src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1 src/uri/fetcher.cpp 2d8d5b18b7a9b55be52288f17e85d6c406cfef88 src/uri/fetchers/copy.hpp PRE-CREATION src/uri/fetchers/copy.cpp PRE-CREATION Diff: https://reviews.apache.org/r/44239/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44239: Introduced copy fetcher plugin for file URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44239/ --- (Updated March 2, 2016, 9:44 p.m.) Review request for mesos and Jie Yu. Changes --- review addressed. Bugs: MESOS-4834 https://issues.apache.org/jira/browse/MESOS-4834 Repository: mesos Description --- This change adds support for fetching `file` URIs using the fetcher plugin framework. Diffs (updated) - src/CMakeLists.txt 0832f1e9a850cc0d2c1112a446f0daf0190f8d7f src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 src/tests/uri_fetcher_tests.cpp dcfba784d6ae49b8181b5a69dc00e82854ac4ff1 src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1 src/uri/fetcher.cpp 2d8d5b18b7a9b55be52288f17e85d6c406cfef88 src/uri/fetchers/copy.hpp PRE-CREATION src/uri/fetchers/copy.cpp PRE-CREATION Diff: https://reviews.apache.org/r/44239/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44239: Introduced copy fetcher plugin for file URIs.
> On March 2, 2016, 3:26 p.m., Guangya Liu wrote: > > src/uri/fetchers/copy.cpp, line 72 > > <https://reviews.apache.org/r/44239/diff/1/?file=1275837#file1275837line72> > > > > Printing the `uri` in the failure may help improve the debug ability? The pattern is same as all other fetchers. Will drop this issue. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44239/#review121655 --- On March 2, 2016, 12:13 a.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44239/ > --- > > (Updated March 2, 2016, 12:13 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-4834 > https://issues.apache.org/jira/browse/MESOS-4834 > > > Repository: mesos > > > Description > --- > > This change adds support for fetching `file` URIs using the fetcher plugin > framework. > > > Diffs > - > > src/CMakeLists.txt 0832f1e9a850cc0d2c1112a446f0daf0190f8d7f > src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 > src/tests/uri_fetcher_tests.cpp dcfba784d6ae49b8181b5a69dc00e82854ac4ff1 > src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1 > src/uri/fetcher.cpp 2d8d5b18b7a9b55be52288f17e85d6c406cfef88 > src/uri/fetchers/copy.hpp PRE-CREATION > src/uri/fetchers/copy.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/44239/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Review Request 44298: Added support for file URI in Appc fetcher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44298/ --- Review request for mesos and Jie Yu. Bugs: MESOS-4822 https://issues.apache.org/jira/browse/MESOS-4822 Repository: mesos Description --- Added support for file URI in Appc fetcher. Diffs - src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp e12a6f27866b6362191ea4dafe8bf818b33cd9e3 Diff: https://reviews.apache.org/r/44298/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 43969: Added test for Appc image fetcher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43969/ --- (Updated March 2, 2016, 9:47 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-4748 https://issues.apache.org/jira/browse/MESOS-4748 Repository: mesos Description --- Added simple appc Fetcher test with mock HTTP image server. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 9d9779ac04e32348ce0e28da54fd7aa039a7fbcd Diff: https://reviews.apache.org/r/43969/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44299: Added unit test for file URI fetcher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44299/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Added unit test for file URI fetcher. Diffs - src/tests/containerizer/provisioner_appc_tests.cpp 9d9779ac04e32348ce0e28da54fd7aa039a7fbcd Diff: https://reviews.apache.org/r/44299/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44239: Introduced copy fetcher plugin for file URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44239/ --- (Updated March 3, 2016, 1:47 a.m.) Review request for mesos and Jie Yu. Changes --- review addressed. Bugs: MESOS-4834 https://issues.apache.org/jira/browse/MESOS-4834 Repository: mesos Description --- This change adds support for fetching `file` URIs using the fetcher plugin framework. Diffs (updated) - src/CMakeLists.txt 0832f1e9a850cc0d2c1112a446f0daf0190f8d7f src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 src/tests/uri_fetcher_tests.cpp dcfba784d6ae49b8181b5a69dc00e82854ac4ff1 src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1 src/uri/fetcher.cpp 2d8d5b18b7a9b55be52288f17e85d6c406cfef88 src/uri/fetchers/copy.hpp PRE-CREATION src/uri/fetchers/copy.cpp PRE-CREATION Diff: https://reviews.apache.org/r/44239/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44239: Introduced copy fetcher plugin for file URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44239/ --- (Updated March 3, 2016, 2:02 a.m.) Review request for mesos and Jie Yu. Changes --- fixed patch. Bugs: MESOS-4834 https://issues.apache.org/jira/browse/MESOS-4834 Repository: mesos Description --- This change adds support for fetching `file` URIs using the fetcher plugin framework. Diffs (updated) - src/CMakeLists.txt 0832f1e9a850cc0d2c1112a446f0daf0190f8d7f src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 src/tests/uri_fetcher_tests.cpp dcfba784d6ae49b8181b5a69dc00e82854ac4ff1 src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1 src/uri/fetcher.cpp 2d8d5b18b7a9b55be52288f17e85d6c406cfef88 src/uri/fetchers/copy.hpp PRE-CREATION src/uri/fetchers/copy.cpp PRE-CREATION Diff: https://reviews.apache.org/r/44239/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44339: Disabled Appc simple fetch test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44339/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Due to dependency on 'curl'. Diffs - src/tests/containerizer/provisioner_appc_tests.cpp 032c555e571bd6196b495ba0d6e8b56336ea4a10 Diff: https://reviews.apache.org/r/44339/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44357: Added check for Appc image's dependency id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44357/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Since dependency's image id is an optional field, we need to check for its existence before using it for fetching an image. Diffs - src/slave/containerizer/mesos/provisioner/appc/store.cpp 1e893daa63d48d4290bf4c4e8655ca3f39133c3e Diff: https://reviews.apache.org/r/44357/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44299: Added unit test for file URI fetcher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44299/ --- (Updated March 3, 2016, 11:37 p.m.) Review request for mesos and Jie Yu. Changes --- rebased. Repository: mesos Description --- Added unit test for file URI fetcher. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 6c8087e17aa8b7139ba12337d5be472b7099e77f Diff: https://reviews.apache.org/r/44299/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44455: Fixed AppcImageFetcherTest for manifest formatting.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44455/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Fixed AppcImageFetcherTest for manifest formatting. Diffs - src/tests/containerizer/provisioner_appc_tests.cpp 6c8087e17aa8b7139ba12337d5be472b7099e77f Diff: https://reviews.apache.org/r/44455/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44456: Added Appc provisioner integration test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44456/ --- Review request for mesos and Jie Yu. Bugs: MESOS-4818 https://issues.apache.org/jira/browse/MESOS-4818 Repository: mesos Description --- The test provides simple integartion test for a single layered linux image. Diffs - src/tests/containerizer/provisioner_appc_tests.cpp 6c8087e17aa8b7139ba12337d5be472b7099e77f Diff: https://reviews.apache.org/r/44456/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44455: Fixed AppcImageFetcherTest for manifest formatting.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44455/ --- (Updated March 8, 2016, 10:45 p.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Fixed AppcImageFetcherTest for manifest formatting. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 6c8087e17aa8b7139ba12337d5be472b7099e77f Diff: https://reviews.apache.org/r/44455/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44533: Fixed AppcStoreTest fixture to remove imageId from test image.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44533/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Fixed AppcStoreTest fixture to remove imageId from test image. Diffs - src/tests/containerizer/provisioner_appc_tests.cpp 6c8087e17aa8b7139ba12337d5be472b7099e77f Diff: https://reviews.apache.org/r/44533/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44534: Refactored AppcImageFetcherTest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44534/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Changes: - Changed TestAppcImageServer mock methods to real implementation that serves http requests. This will allow the TestAppcImageServer to serve multiple images in a test. - Made TestAppcImageServer loadable so that it can serve images from its 'images' directory (currently defaulted to 'server'). - Renamed prepareServerImage to prepareImage to reflect the semantics. Diffs - src/tests/containerizer/provisioner_appc_tests.cpp 6c8087e17aa8b7139ba12337d5be472b7099e77f Diff: https://reviews.apache.org/r/44534/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44299: Added unit test for file URI fetcher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44299/ --- (Updated March 8, 2016, 10:49 p.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Added unit test for file URI fetcher. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 6c8087e17aa8b7139ba12337d5be472b7099e77f Diff: https://reviews.apache.org/r/44299/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44456: Added Appc provisioner integration test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44456/ --- (Updated March 8, 2016, 10:50 p.m.) Review request for mesos and Jie Yu. Changes --- rebased. Bugs: MESOS-4818 https://issues.apache.org/jira/browse/MESOS-4818 Repository: mesos Description (updated) --- Added Appc provisioner integration test. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 6c8087e17aa8b7139ba12337d5be472b7099e77f Diff: https://reviews.apache.org/r/44456/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44299: Added unit test for file URI fetcher.
> On March 8, 2016, 1:57 a.m., Jie Yu wrote: > > src/tests/containerizer/provisioner_appc_tests.cpp, lines 590-591 > > <https://reviews.apache.org/r/44299/diff/2/?file=1280158#file1280158line590> > > > > Looking at the original code, why do we need this id? Should that > > always be the same? Why do we need to pass it in? moved the image id up to `prepareImage` method. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44299/#review122461 --- On March 8, 2016, 10:49 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44299/ > --- > > (Updated March 8, 2016, 10:49 p.m.) > > > Review request for mesos and Jie Yu. > > > Repository: mesos > > > Description > --- > > Added unit test for file URI fetcher. > > > Diffs > - > > src/tests/containerizer/provisioner_appc_tests.cpp > 6c8087e17aa8b7139ba12337d5be472b7099e77f > > Diff: https://reviews.apache.org/r/44299/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 44456: Added Appc provisioner integration test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44456/ --- (Updated March 11, 2016, 5:06 p.m.) Review request for mesos and Jie Yu. Changes --- review addressed. Bugs: MESOS-4818 https://issues.apache.org/jira/browse/MESOS-4818 Repository: mesos Description --- Added Appc provisioner integration test. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 6c8087e17aa8b7139ba12337d5be472b7099e77f Diff: https://reviews.apache.org/r/44456/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44755: Added getAppcImage for Appc provisioning tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44755/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Added helper function to get an Appc image protobuf object with given parameters. Diffs - src/tests/containerizer/provisioner_appc_tests.cpp 069af8a0b5e031d9032fd13154cee250f279b404 Diff: https://reviews.apache.org/r/44755/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44456: Added Appc provisioner integration test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44456/ --- (Updated March 12, 2016, 5:14 p.m.) Review request for mesos and Jie Yu. Changes --- review addressed. Bugs: MESOS-4818 https://issues.apache.org/jira/browse/MESOS-4818 Repository: mesos Description --- Added Appc provisioner integration test. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 069af8a0b5e031d9032fd13154cee250f279b404 Diff: https://reviews.apache.org/r/44456/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.
> On March 15, 2016, 4:56 p.m., Jie Yu wrote: > > src/tests/containerizer/provisioner_appc_tests.cpp, lines 587-610 > > <https://reviews.apache.org/r/44755/diff/1/?file=1296713#file1296713line587> > > > > Can you move this function to the top of this file and make it 'static'? > > Anand Mazumdar wrote: > Wondering why does this function needs to be a global and can't be part > of the test fixture i.e. `AppcImageFetcherTest`? The intend is to use it in all fixtures. Eg, for integegration tests, we have a separate fixture. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44755/#review123688 --- On March 12, 2016, 5:11 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44755/ > --- > > (Updated March 12, 2016, 5:11 p.m.) > > > Review request for mesos and Jie Yu. > > > Repository: mesos > > > Description > --- > > Added helper function to get an Appc image protobuf object with given > parameters. > > > Diffs > - > > src/tests/containerizer/provisioner_appc_tests.cpp > 069af8a0b5e031d9032fd13154cee250f279b404 > > Diff: https://reviews.apache.org/r/44755/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44755/ --- (Updated March 15, 2016, 6:26 p.m.) Review request for mesos and Jie Yu. Changes --- review addressed. Repository: mesos Description --- Added helper function to get an Appc image protobuf object with given parameters. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 069af8a0b5e031d9032fd13154cee250f279b404 Diff: https://reviews.apache.org/r/44755/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.
> On March 15, 2016, 4:56 p.m., Jie Yu wrote: > > src/tests/containerizer/provisioner_appc_tests.cpp, lines 587-610 > > <https://reviews.apache.org/r/44755/diff/1/?file=1296713#file1296713line587> > > > > Can you move this function to the top of this file and make it 'static'? > > Anand Mazumdar wrote: > Wondering why does this function needs to be a global and can't be part > of the test fixture i.e. `AppcImageFetcherTest`? > > Jojy Varghese wrote: > The intend is to use it in all fixtures. Eg, for integegration tests, we > have a separate fixture. > > Anand Mazumdar wrote: > I see. If the function has static scope, how would we use it across > separate fixtures i.e. in some other test file? > > This is assuming that the integration tests would be part of a separate > test file since we typically avoid having multiple test fixtures in a single > file. Right now the integration tests are in the same file (in another patch). Not sure if having multiple fixtures in a test file is a no-no (eg. provisioner_docker_tests). - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44755/#review123688 --- On March 15, 2016, 6:26 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44755/ > --- > > (Updated March 15, 2016, 6:26 p.m.) > > > Review request for mesos and Jie Yu. > > > Repository: mesos > > > Description > --- > > Added helper function to get an Appc image protobuf object with given > parameters. > > > Diffs > - > > src/tests/containerizer/provisioner_appc_tests.cpp > 069af8a0b5e031d9032fd13154cee250f279b404 > > Diff: https://reviews.apache.org/r/44755/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 44230: Added FS_DEFAULT case in rmdir.
Would be happy to add test. Patch forthcoming. Will also address FTS_SLNONE. -Jojy > On Mar 15, 2016, at 2:26 PM, David Robinson > wrote: > > > >> On March 15, 2016, 8:02 p.m., Neil Conway wrote: >>> Is it feasible/portable to have a test case for this change? >> >> Cong Wang wrote: >>Yes, like in our case, you can create some socket or device file and try >> to remove the directory contains it, it would fail without this patch. > > AFAICT tests have already been added: > > https://github.com/apache/mesos/commit/c55ca2941b0119c13764bccdebcea46119e69e4e > > > - David > > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44230/#review123733 > --- > > > On March 1, 2016, 10:01 p.m., Jojy Varghese wrote: >> >> --- >> This is an automatically generated e-mail. To reply, visit: >> https://reviews.apache.org/r/44230/ >> --- >> >> (Updated March 1, 2016, 10:01 p.m.) >> >> >> Review request for mesos and Jie Yu. >> >> >> Repository: mesos >> >> >> Description >> --- >> >> We currently dont handle special files like device files in rmdir. This >> change >> adds FS_DEFAULT as one of the cases where we try to unlink a file. >> >> Reference: http://man7.org/linux/man-pages/man3/fts.3.html >> >> >> Diffs >> - >> >> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp >> bc420c9c10d93ddd619a9eb2c5f4db67f31d722f >> >> Diff: https://reviews.apache.org/r/44230/diff/ >> >> >> Testing >> --- >> >> make check. >> >> >> Thanks, >> >> Jojy Varghese >> >> >
Re: Review Request 44230: Added FS_DEFAULT case in rmdir.
Yes thats correct. I am planning to submit a patch that will add the test. Will also address the symlink case. -jojy > On Mar 15, 2016, at 3:33 PM, Cong Wang wrote: > > This is an automatically generated e-mail. To reply, > visit:https://reviews.apache.org/r/44230/ > <https://reviews.apache.org/r/44230/> > On March 15th, 2016, 8:02 p.m. UTC, Neil Conway wrote: > > Is it feasible/portable to have a test case for this change? > On March 15th, 2016, 8:58 p.m. UTC, Cong Wang wrote: > > Yes, like in our case, you can create some socket or device file and try to > remove the directory contains it, it would fail without this patch. > On March 15th, 2016, 9:26 p.m. UTC, David Robinson wrote: > > AFAICT tests have already been added: > https://github.com/apache/mesos/commit/c55ca2941b0119c13764bccdebcea46119e69e4e > None of these tests covers this bug. > > - Cong > > > On March 1st, 2016, 10:01 p.m. UTC, Jojy Varghese wrote: > > Review request for mesos and Jie Yu. > By Jojy Varghese. > Updated March 1, 2016, 10:01 p.m. > > Repository: mesos > Description > > We currently dont handle special files like device files in rmdir. This change > adds FS_DEFAULT as one of the cases where we try to unlink a file. > > Reference: http://man7.org/linux/man-pages/man3/fts.3.html > Testing > > make check. > Diffs > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp > (bc420c9c10d93ddd619a9eb2c5f4db67f31d722f) > View Diff <https://reviews.apache.org/r/44230/diff/>
Review Request 44873: Added test for rmdir with device file.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44873/ --- Review request for mesos, Jie Yu and Neil Conway. Repository: mesos Description --- Existing tests did not cover the case of removing directories with special files like device files. Diffs - 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 291a22b5aae53b0bc32ae18b9343ceb5a638b37b Diff: https://reviews.apache.org/r/44873/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44874: Added support for FTS_SLNONE in rmdir.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44874/ --- Review request for mesos, Jie Yu and Neil Conway. Repository: mesos Description --- Added support for FTS_SLNONE in rmdir. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp cf21e7fe458626c7533e596997cab3afdabb55f4 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 291a22b5aae53b0bc32ae18b9343ceb5a638b37b Diff: https://reviews.apache.org/r/44874/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44874: Added support for FTS_SLNONE in rmdir.
> On March 16, 2016, 12:25 a.m., Neil Conway wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71 > > <https://reviews.apache.org/r/44874/diff/1/?file=1300428#file1300428line71> > > > > "don't". Also, we should use backticks for `FTS_COMFOLLOW` for > > consistency. > > > > Is that comment correct? ISTM that the relevant option that controls > > whether symbolic links are resolved is `FTS_LOGICAL` or `FTS_PHYSICAL`; > > `FTS_COMFOLLOW` is a special-case for the root path that is passed to > > `fts_open`. > > > > While we're at it, the OSX and Linux versions of the manpage for > > `fts_open` both claim that "either FTS_LOGICAL or FTS_PHYSICAL" *must* be > > specified. Seems like we're specifying neither, both here and in the other > > call-site to `fts_open` in `src/linux/cgroups.cpp`? > > Jie Yu wrote: > Looks like we should use `FTS_PHYSICAL` in rmdir? I made the assumption looking at some implementation source code at http://osxr.org:8080/glibc/source/io/fts.c?v=glibc-2.17. L147 is where `fts_open` calls `fts_stat` with a check for `FTS_COMFOLLOW`. In `fts_stat`, L879, the check is made for `logical` or `follow`. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44874/#review123802 --- On March 15, 2016, 11:47 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44874/ > --- > > (Updated March 15, 2016, 11:47 p.m.) > > > Review request for mesos, Jie Yu and Neil Conway. > > > Repository: mesos > > > Description > --- > > Added support for FTS_SLNONE in rmdir. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp > cf21e7fe458626c7533e596997cab3afdabb55f4 > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp > 291a22b5aae53b0bc32ae18b9343ceb5a638b37b > > Diff: https://reviews.apache.org/r/44874/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 44874: Added support for FTS_SLNONE in rmdir.
> On March 16, 2016, 12:29 a.m., Neil Conway wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp, line 222 > > <https://reviews.apache.org/r/44874/diff/1/?file=1300429#file1300429line222> > > > > Why is this a `string&` and `newDirectory` is a `string`? I'd think > > both should probably be `string`. > > Jie Yu wrote: > Good catch, we should use 'string'. We avoid using const ref to capture > temp variable. A pattern I copied from `os_tests.cpp`. Copy paste horror. I think we need to fix it there as well as we fix it here. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44874/#review123803 ------- On March 15, 2016, 11:47 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44874/ > --- > > (Updated March 15, 2016, 11:47 p.m.) > > > Review request for mesos, Jie Yu and Neil Conway. > > > Repository: mesos > > > Description > --- > > Added support for FTS_SLNONE in rmdir. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp > cf21e7fe458626c7533e596997cab3afdabb55f4 > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp > 291a22b5aae53b0bc32ae18b9343ceb5a638b37b > > Diff: https://reviews.apache.org/r/44874/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 44934: Updated mesos-execute to add support for Appc.
> On March 17, 2016, 2:16 a.m., Guangya Liu wrote: > > src/cli/execute.cpp, lines 251-281 > > <https://reviews.apache.org/r/44934/diff/1/?file=1301837#file1301837line251> > > > > What is the behavior of this if with mesos containerizer but without a > > docker/appc image? > > > > The current logic is a bit different from previous logic, the mesos > > execute will not construct the containerInfo if no docker image. Thanks for the review ! Updated the patch. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44934/#review123969 ------- On March 16, 2016, 9:21 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44934/ > --- > > (Updated March 16, 2016, 9:21 p.m.) > > > Review request for mesos and Jie Yu. > > > Repository: mesos > > > Description > --- > > Updated mesos-execute to add support for Appc. > > > Diffs > - > > src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e > > Diff: https://reviews.apache.org/r/44934/diff/ > > > Testing > --- > > Tested with various Appc images. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 44934: Updated mesos-execute to add support for Appc.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44934/ --- (Updated March 17, 2016, 6:54 a.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Updated mesos-execute to add support for Appc. Diffs (updated) - src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e Diff: https://reviews.apache.org/r/44934/diff/ Testing --- Tested with various Appc images. Thanks, Jojy Varghese
Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44755/ --- (Updated March 18, 2016, 12:48 a.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Added helper function to get an Appc image protobuf object with given parameters. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 069af8a0b5e031d9032fd13154cee250f279b404 Diff: https://reviews.apache.org/r/44755/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44934: Updated mesos-execute to add support for Appc.
> On March 17, 2016, 7:48 a.m., Guangya Liu wrote:
> > src/cli/execute.cpp, line 241
> > <https://reviews.apache.org/r/44934/diff/2/?file=1302214#file1302214line241>
> >
> > The default value of `containerizer` is `mesos`, do we need to check
> > this?
> >
> > What about make the logic as this? This will involve less code change
> > and the logic may be more clear?
> >
> > // Do not touch old dockerImage logic.
> > if (dockerImage.isSome()) {
> > old logic here;
> > }
> >
> > if (appcImage.isSome() && containerizer == "mesos") {
> > ContainerInfo containerInfo;
> > containerInfo.set_type(ContainerInfo::MESOS);
> > ContainerInfo::MesosInfo mesosInfo;
> >
> > Image::Appc appc;
> >
> > appc.set_name(appcImage.get());
> >
> > // TODO(jojy): Labels are hard coded right now.
> > // Consider adding label flags for customization.
> > Label arch;
> > arch.set_key("arch");
> > arch.set_value("amd64");
> >
> > Label os;
> > os.set_key("os");
> > os.set_value("linux");
> >
> > Labels labels;
> > labels.add_labels()->CopyFrom(os);
> > labels.add_labels()->CopyFrom(arch);
> >
> > appc.mutable_labels()->CopyFrom(labels);
> >
> > Image mesosImage;
> >
> > mesosImage.set_type(Image::APPC);
> > mesosImage.mutable_appc()->CopyFrom(appc);
> >
> > mesosInfo.mutable_image()->CopyFrom(mesosImage);
> >
> > containerInfo.mutable_mesos()->CopyFrom(mesosInfo);
> >
> > task.mutable_container()->CopyFrom(containerInfo);
> > }
>
> Jojy Varghese wrote:
> Then we will have to repeat the `if (containerizer == "mesos")` for both
> dockerImage case and appcImage case. By seperating the logic by cotainerizer,
> we now can add more image types into the "mesos" containerizer block (say OCI
> image).
>
> Guangya Liu wrote:
> Yes, that's also my concern when I open this issue, I think it is ok to
> keep your current logic.
>
> The logic of checking `containerizer.empty()` should be removed?
>
> Jojy Varghese wrote:
> We could remove that but the idea is that the block can be cleanly
> separated out now to a method (say `handleContainerizer`) that handles the
> containerizers.
cleaned up/refactored the flow to make it easy to follow.
- Jojy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44934/#review123991
---
On March 18, 2016, 5:26 p.m., Jojy Varghese wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44934/
> -------
>
> (Updated March 18, 2016, 5:26 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Updated mesos-execute to add support for Appc.
>
>
> Diffs
> -
>
> src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e
>
> Diff: https://reviews.apache.org/r/44934/diff/
>
>
> Testing
> ---
>
> Tested with various Appc images.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 44934: Updated mesos-execute to add support for Appc.
> On March 17, 2016, 7:48 a.m., Guangya Liu wrote:
> > src/cli/execute.cpp, line 241
> > <https://reviews.apache.org/r/44934/diff/2/?file=1302214#file1302214line241>
> >
> > The default value of `containerizer` is `mesos`, do we need to check
> > this?
> >
> > What about make the logic as this? This will involve less code change
> > and the logic may be more clear?
> >
> > // Do not touch old dockerImage logic.
> > if (dockerImage.isSome()) {
> > old logic here;
> > }
> >
> > if (appcImage.isSome() && containerizer == "mesos") {
> > ContainerInfo containerInfo;
> > containerInfo.set_type(ContainerInfo::MESOS);
> > ContainerInfo::MesosInfo mesosInfo;
> >
> > Image::Appc appc;
> >
> > appc.set_name(appcImage.get());
> >
> > // TODO(jojy): Labels are hard coded right now.
> > // Consider adding label flags for customization.
> > Label arch;
> > arch.set_key("arch");
> > arch.set_value("amd64");
> >
> > Label os;
> > os.set_key("os");
> > os.set_value("linux");
> >
> > Labels labels;
> > labels.add_labels()->CopyFrom(os);
> > labels.add_labels()->CopyFrom(arch);
> >
> > appc.mutable_labels()->CopyFrom(labels);
> >
> > Image mesosImage;
> >
> > mesosImage.set_type(Image::APPC);
> > mesosImage.mutable_appc()->CopyFrom(appc);
> >
> > mesosInfo.mutable_image()->CopyFrom(mesosImage);
> >
> > containerInfo.mutable_mesos()->CopyFrom(mesosInfo);
> >
> > task.mutable_container()->CopyFrom(containerInfo);
> > }
Then we will have to repeat the `if (containerizer == "mesos")` for both
dockerImage case and appcImage case. By seperating the logic by cotainerizer,
we now can add more image types into the "mesos" containerizer block (say OCI
image).
- Jojy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44934/#review123991
---
On March 17, 2016, 6:54 a.m., Jojy Varghese wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44934/
> ---
>
> (Updated March 17, 2016, 6:54 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Updated mesos-execute to add support for Appc.
>
>
> Diffs
> -
>
> src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e
>
> Diff: https://reviews.apache.org/r/44934/diff/
>
>
> Testing
> ---
>
> Tested with various Appc images.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Review Request 45001: Replaced const ref to temporary in tests for consistency.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45001/ --- Review request for mesos, Jie Yu and Neil Conway. Repository: mesos Description --- Replaced const ref to temporary in tests for consistency. Diffs - 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp daa46e05d113fd62ea9dad042ec41aaab28ad003 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp a69fd2d08838e9f0b5738531b29dbca996b70dcb Diff: https://reviews.apache.org/r/45001/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44934: Updated mesos-execute to add support for Appc.
> On March 17, 2016, 7:48 a.m., Guangya Liu wrote:
> > src/cli/execute.cpp, line 241
> > <https://reviews.apache.org/r/44934/diff/2/?file=1302214#file1302214line241>
> >
> > The default value of `containerizer` is `mesos`, do we need to check
> > this?
> >
> > What about make the logic as this? This will involve less code change
> > and the logic may be more clear?
> >
> > // Do not touch old dockerImage logic.
> > if (dockerImage.isSome()) {
> > old logic here;
> > }
> >
> > if (appcImage.isSome() && containerizer == "mesos") {
> > ContainerInfo containerInfo;
> > containerInfo.set_type(ContainerInfo::MESOS);
> > ContainerInfo::MesosInfo mesosInfo;
> >
> > Image::Appc appc;
> >
> > appc.set_name(appcImage.get());
> >
> > // TODO(jojy): Labels are hard coded right now.
> > // Consider adding label flags for customization.
> > Label arch;
> > arch.set_key("arch");
> > arch.set_value("amd64");
> >
> > Label os;
> > os.set_key("os");
> > os.set_value("linux");
> >
> > Labels labels;
> > labels.add_labels()->CopyFrom(os);
> > labels.add_labels()->CopyFrom(arch);
> >
> > appc.mutable_labels()->CopyFrom(labels);
> >
> > Image mesosImage;
> >
> > mesosImage.set_type(Image::APPC);
> > mesosImage.mutable_appc()->CopyFrom(appc);
> >
> > mesosInfo.mutable_image()->CopyFrom(mesosImage);
> >
> > containerInfo.mutable_mesos()->CopyFrom(mesosInfo);
> >
> > task.mutable_container()->CopyFrom(containerInfo);
> > }
>
> Jojy Varghese wrote:
> Then we will have to repeat the `if (containerizer == "mesos")` for both
> dockerImage case and appcImage case. By seperating the logic by cotainerizer,
> we now can add more image types into the "mesos" containerizer block (say OCI
> image).
>
> Guangya Liu wrote:
> Yes, that's also my concern when I open this issue, I think it is ok to
> keep your current logic.
>
> The logic of checking `containerizer.empty()` should be removed?
We could remove that but the idea is that the block can be cleanly separated
out now to a method (say `handleContainerizer`) that handles the containerizers.
- Jojy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44934/#review123991
---
On March 17, 2016, 6:54 a.m., Jojy Varghese wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44934/
> ---
>
> (Updated March 17, 2016, 6:54 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Updated mesos-execute to add support for Appc.
>
>
> Diffs
> -
>
> src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e
>
> Diff: https://reviews.apache.org/r/44934/diff/
>
>
> Testing
> ---
>
> Tested with various Appc images.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44755/ --- (Updated March 17, 2016, 4:44 p.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Added helper function to get an Appc image protobuf object with given parameters. Diffs (updated) - src/tests/containerizer/provisioner_appc_tests.cpp 069af8a0b5e031d9032fd13154cee250f279b404 Diff: https://reviews.apache.org/r/44755/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44934: Updated mesos-execute to add support for Appc.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44934/ --- (Updated March 18, 2016, 5:39 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-4978 https://issues.apache.org/jira/browse/MESOS-4978 Repository: mesos Description --- Updated mesos-execute to add support for Appc. Diffs - src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e Diff: https://reviews.apache.org/r/44934/diff/ Testing --- Tested with various Appc images. Thanks, Jojy Varghese
Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45003/ --- Review request for mesos, Jie Yu and Neil Conway. Repository: mesos Description --- Fixed rmdir comment for FTS_SLNONE as per coding guidelines. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp cbc97596cd8ed1e6d4261568fd0086c86e063232 Diff: https://reviews.apache.org/r/45003/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 45010: Fixed newlines for include directives.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45010/ --- Review request for mesos, Jie Yu and Neil Conway. Repository: mesos Description --- Fixed newlines for include directives. Diffs - 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp daa46e05d113fd62ea9dad042ec41aaab28ad003 Diff: https://reviews.apache.org/r/45010/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> On March 20, 2016, 2:33 a.m., Neil Conway wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71 > > <https://reviews.apache.org/r/45003/diff/1/?file=1304569#file1304569line71> > > > > This comment seems incorrect: > > > > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as > > well as `FTS_COMFOLLOW`. > > > > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for > > `FTS_SLNONE` to be returned? I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for FTS_SLNONE to be returned. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45003/#review124433 ------- On March 18, 2016, 12:17 a.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45003/ > --- > > (Updated March 18, 2016, 12:17 a.m.) > > > Review request for mesos, Jie Yu and Neil Conway. > > > Repository: mesos > > > Description > --- > > Fixed rmdir comment for FTS_SLNONE as per coding guidelines. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp > cbc97596cd8ed1e6d4261568fd0086c86e063232 > > Diff: https://reviews.apache.org/r/45003/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.
> On March 20, 2016, 2:36 a.m., Neil Conway wrote: > > Per discussion in https://reviews.apache.org/r/45003/, if we can actually > > run into `FTS_SLNONE`, we should have a test-case for it. I dont think with FTS_PHYSICAL (and without FTW_COMFOLLOW) we will get FTS_SLNONE. I could be wrong but having a peek inside the glibc code for fts_open, I believe thats the case. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45002/#review124434 --- On March 18, 2016, 12:16 a.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45002/ > --- > > (Updated March 18, 2016, 12:16 a.m.) > > > Review request for mesos, Jie Yu and Neil Conway. > > > Repository: mesos > > > Description > --- > > According to the documentation for fts_open, either FTS_PHYSICAL or > FTS_LOGICAL > SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want > to resolve the symlink targets while deleting them. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp > cbc97596cd8ed1e6d4261568fd0086c86e063232 > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp > daa46e05d113fd62ea9dad042ec41aaab28ad003 > > Diff: https://reviews.apache.org/r/45002/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 44934: Updated mesos-execute to add support for Appc.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44934/ --- (Updated March 20, 2016, 4:19 a.m.) Review request for mesos and Jie Yu. Bugs: MESOS-4978 https://issues.apache.org/jira/browse/MESOS-4978 Repository: mesos Description --- Updated mesos-execute to add support for Appc. Diffs (updated) - src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e Diff: https://reviews.apache.org/r/44934/diff/ Testing --- Tested with various Appc images. Thanks, Jojy Varghese
Re: Review Request 44934: Updated mesos-execute to add support for Appc.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44934/ --- (Updated March 18, 2016, 5:26 p.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Updated mesos-execute to add support for Appc. Diffs (updated) - src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e Diff: https://reviews.apache.org/r/44934/diff/ Testing --- Tested with various Appc images. Thanks, Jojy Varghese
Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45002/ --- Review request for mesos, Jie Yu and Neil Conway. Repository: mesos Description --- According to the documentation for fts_open, either FTS_PHYSICAL or FTS_LOGICAL SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want to resolve the symlink targets while deleting them. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp cbc97596cd8ed1e6d4261568fd0086c86e063232 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp daa46e05d113fd62ea9dad042ec41aaab28ad003 Diff: https://reviews.apache.org/r/45002/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 44934: Updated mesos-execute to add support for Appc.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44934/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Updated mesos-execute to add support for Appc. Diffs - src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e Diff: https://reviews.apache.org/r/44934/diff/ Testing --- Tested with various Appc images. Thanks, Jojy Varghese
Re: Review Request 44934: Updated mesos-execute to add support for Appc.
> On March 22, 2016, 5:10 p.m., Tom Runyon wrote: > > It would be helpful to update docs/container-image.md to include example > > appc deployments using mesos-execute. Created MESOS-5006. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44934/#review124834 --- On March 20, 2016, 4:19 a.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44934/ > --- > > (Updated March 20, 2016, 4:19 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-4978 > https://issues.apache.org/jira/browse/MESOS-4978 > > > Repository: mesos > > > Description > --- > > Updated mesos-execute to add support for Appc. > > > Diffs > - > > src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e > > Diff: https://reviews.apache.org/r/44934/diff/ > > > Testing > --- > > Tested with various Appc images. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> On March 20, 2016, 2:33 a.m., Neil Conway wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71 > > <https://reviews.apache.org/r/45003/diff/1/?file=1304569#file1304569line71> > > > > This comment seems incorrect: > > > > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as > > well as `FTS_COMFOLLOW`. > > > > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for > > `FTS_SLNONE` to be returned? > > Jojy Varghese wrote: > I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for > FTS_SLNONE to be returned. > > Jie Yu wrote: > Can you do a simple test to validate that (see if FTS_SLNONE is hit or > not)? My understanding is that FTS_SLNONE will still be hit if we're using > FTS_PHYSICAL. > > Cong Wang wrote: > Looking at > https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fts.c;h=0c5abfcdd660871d876751fba351ab25b921e88a;hb=HEAD#l901 > I think Jojy is correct here, I don't see FTS_SLNONE could be set without > FTS_LOGICAL or FTS_COMFOLLOW. @jie. I tested it already and verified that FTS_SLNONE is not hit (using logs). - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45003/#review124433 --- On March 18, 2016, 12:17 a.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45003/ > --- > > (Updated March 18, 2016, 12:17 a.m.) > > > Review request for mesos, Jie Yu and Neil Conway. > > > Repository: mesos > > > Description > --- > > Fixed rmdir comment for FTS_SLNONE as per coding guidelines. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp > cbc97596cd8ed1e6d4261568fd0086c86e063232 > > Diff: https://reviews.apache.org/r/45003/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Review Request 45259: Added Appc provisioner integration test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45259/ --- Review request for mesos and Jie Yu. Bugs: MESOS-4818 https://issues.apache.org/jira/browse/MESOS-4818 Repository: mesos Description --- Added Appc provisioner integration test. Diffs - src/tests/containerizer/provisioner_appc_tests.cpp 23a5b1059b4d9fde1e4a1aab5cd4fa6d05862332 Diff: https://reviews.apache.org/r/45259/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45002/ --- (Updated March 24, 2016, 1:07 a.m.) Review request for mesos, Jie Yu and Neil Conway. Changes --- review addressed. Repository: mesos Description --- According to the documentation for fts_open, either FTS_PHYSICAL or FTS_LOGICAL SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want to resolve the symlink targets while deleting them. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp cbc97596cd8ed1e6d4261568fd0086c86e063232 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 5bd154e40c6b45e74967b620141cc373bdee src/tests/containerizer/provisioner_appc_tests.cpp 23a5b1059b4d9fde1e4a1aab5cd4fa6d05862332 Diff: https://reviews.apache.org/r/45002/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45003/ --- (Updated March 24, 2016, 1:10 a.m.) Review request for mesos, Jie Yu and Neil Conway. Changes --- review addressed. Repository: mesos Description --- Fixed rmdir comment for FTS_SLNONE as per coding guidelines. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp cbc97596cd8ed1e6d4261568fd0086c86e063232 Diff: https://reviews.apache.org/r/45003/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> On March 20, 2016, 2:33 a.m., Neil Conway wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71 > > <https://reviews.apache.org/r/45003/diff/1/?file=1304569#file1304569line71> > > > > This comment seems incorrect: > > > > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as > > well as `FTS_COMFOLLOW`. > > > > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for > > `FTS_SLNONE` to be returned? > > Jojy Varghese wrote: > I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for > FTS_SLNONE to be returned. > > Jie Yu wrote: > Can you do a simple test to validate that (see if FTS_SLNONE is hit or > not)? My understanding is that FTS_SLNONE will still be hit if we're using > FTS_PHYSICAL. > > Cong Wang wrote: > Looking at > https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fts.c;h=0c5abfcdd660871d876751fba351ab25b921e88a;hb=HEAD#l901 > I think Jojy is correct here, I don't see FTS_SLNONE could be set without > FTS_LOGICAL or FTS_COMFOLLOW. > > Jojy Varghese wrote: > @jie. I tested it already and verified that FTS_SLNONE is not hit (using > logs). > > Neil Conway wrote: > Can we add a unit test anyway, please? This is corner-case behavior that > we should be validating we handle correctly, regardless of whether > `FTS_SLNONE` is actually set. I have added unit tests in patch https://reviews.apache.org/r/45002/. Would love to hear more test ideas. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45003/#review124433 --- On March 24, 2016, 1:10 a.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45003/ > --- > > (Updated March 24, 2016, 1:10 a.m.) > > > Review request for mesos, Jie Yu and Neil Conway. > > > Repository: mesos > > > Description > --- > > Fixed rmdir comment for FTS_SLNONE as per coding guidelines. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp > cbc97596cd8ed1e6d4261568fd0086c86e063232 > > Diff: https://reviews.apache.org/r/45003/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> On March 20, 2016, 2:33 a.m., Neil Conway wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71 > > <https://reviews.apache.org/r/45003/diff/1/?file=1304569#file1304569line71> > > > > This comment seems incorrect: > > > > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as > > well as `FTS_COMFOLLOW`. > > > > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for > > `FTS_SLNONE` to be returned? > > Jojy Varghese wrote: > I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for > FTS_SLNONE to be returned. > > Jie Yu wrote: > Can you do a simple test to validate that (see if FTS_SLNONE is hit or > not)? My understanding is that FTS_SLNONE will still be hit if we're using > FTS_PHYSICAL. > > Cong Wang wrote: > Looking at > https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fts.c;h=0c5abfcdd660871d876751fba351ab25b921e88a;hb=HEAD#l901 > I think Jojy is correct here, I don't see FTS_SLNONE could be set without > FTS_LOGICAL or FTS_COMFOLLOW. > > Jojy Varghese wrote: > @jie. I tested it already and verified that FTS_SLNONE is not hit (using > logs). > > Neil Conway wrote: > Can we add a unit test anyway, please? This is corner-case behavior that > we should be validating we handle correctly, regardless of whether > `FTS_SLNONE` is actually set. > > Jojy Varghese wrote: > I have added unit tests in patch https://reviews.apache.org/r/45002/. > Would love to hear more test ideas. > > Neil Conway wrote: > I'd like a test case that covers `rmdir` of a symbolic link that points > to a non-existent file/directory; as far as I can see, the existing tests do > not cover this case. What do you think about the test case `RemoveDirectoryWithNoTargetSymbolicLink` in the test file? - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45003/#review124433 --- On March 24, 2016, 1:10 a.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45003/ > --- > > (Updated March 24, 2016, 1:10 a.m.) > > > Review request for mesos, Jie Yu and Neil Conway. > > > Repository: mesos > > > Description > --- > > Fixed rmdir comment for FTS_SLNONE as per coding guidelines. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp > cbc97596cd8ed1e6d4261568fd0086c86e063232 > > Diff: https://reviews.apache.org/r/45003/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45002/ --- (Updated March 24, 2016, 1:34 a.m.) Review request for mesos, Jie Yu and Neil Conway. Changes --- review addressed. Repository: mesos Description --- According to the documentation for fts_open, either FTS_PHYSICAL or FTS_LOGICAL SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want to resolve the symlink targets while deleting them. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp cbc97596cd8ed1e6d4261568fd0086c86e063232 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 5bd154e40c6b45e74967b620141cc373bdee Diff: https://reviews.apache.org/r/45002/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 44934: Updated mesos-execute to add support for Appc.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44934/ --- (Updated April 1, 2016, 8:05 p.m.) Review request for mesos and Jie Yu. Changes --- rebased. Bugs: MESOS-4978 https://issues.apache.org/jira/browse/MESOS-4978 Repository: mesos Description --- Updated mesos-execute to add support for Appc. Diffs (updated) - src/cli/execute.cpp af62f41622e1c43acd8f257c54f8754162c433b8 Diff: https://reviews.apache.org/r/44934/diff/ Testing --- Tested with various Appc images. Thanks, Jojy Varghese
Re: Review Request 46044: Cleaned up c-tor signature in mesos-execute.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46044/#review128265
---
src/cli/execute.cpp
<https://reviews.apache.org/r/46044/#comment191684>
I like the idea of simplifying the ctor. I am not too excited about the
idea of moving everything to 'flag'. A `CommandScheduler` object should have
some properties like `command`, `master`, `name`. Others like 'image'
information should be moved to its own class/struct (say `ContainerInfo`). Just
my 2 cents.
- Jojy Varghese
On April 11, 2016, 7:01 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46044/
> ---
>
> (Updated April 11, 2016, 7:01 p.m.)
>
>
> Review request for mesos, Anand Mazumdar and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Pass complete `flags` instance rather than each flag value separately
> to `CommandScheduler` in mesos-execute for brevity.
>
>
> Diffs
> -
>
> src/cli/execute.cpp 763dd26c359d1dd92c6e0365e4808b673efb1f40
>
> Diff: https://reviews.apache.org/r/46044/diff/
>
>
> Testing
> ---
>
> On Mac OS 10.10.4:
> make check
>
> Additionally manually tested mesos-execute with both responsive and
> unresponsive (https://github.com/rukletsov/unresponsive-process) tasks:
> ./src/mesos-execute --master=127.0.0.1:5050 --name=test --command="sleep 10"
> --env='{"GLOG_v": "2"}' --kill_after=2secs
> ./src/mesos-execute --master=127.0.0.1:5050 --name=test
> --command="/Users/alex/bin/unresponsive_process" --env='{"GLOG_v": "2"}'
> --kill_after=2secs
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 46044: Cleaned up c-tor signature in mesos-execute.
> On April 11, 2016, 9:53 p.m., Jojy Varghese wrote:
> > src/cli/execute.cpp, line 196
> > <https://reviews.apache.org/r/46044/diff/1/?file=1339684#file1339684line196>
> >
> > I like the idea of simplifying the ctor. I am not too excited about the
> > idea of moving everything to 'flag'. A `CommandScheduler` object should
> > have some properties like `command`, `master`, `name`. Others like 'image'
> > information should be moved to its own class/struct (say `ContainerInfo`).
> > Just my 2 cents.
>
> Joseph Wu wrote:
> I partially agree, but in this case, the `Flags` class is effectively the
> class/struct you're asking for. It just happens to have everything packed
> along with it :)
hmm i could extend that argument to get away with any type system or class
design and replace all types with a `flag` type. Good design would allow
translation between a user input (flag) to a CommandScheduler object. Passing a
user input to CommandScheduler does not seem right. I think this was the reason
the original author wanted input validation to happen before the object was
created. CommandScheduler should only accept a strongly typed input in its
ctor. I exceeded my 2 cents :|
- Jojy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46044/#review128265
---
On April 11, 2016, 7:01 p.m., Alexander Rukletsov wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46044/
> ---
>
> (Updated April 11, 2016, 7:01 p.m.)
>
>
> Review request for mesos, Anand Mazumdar and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Pass complete `flags` instance rather than each flag value separately
> to `CommandScheduler` in mesos-execute for brevity.
>
>
> Diffs
> -
>
> src/cli/execute.cpp 763dd26c359d1dd92c6e0365e4808b673efb1f40
>
> Diff: https://reviews.apache.org/r/46044/diff/
>
>
> Testing
> ---
>
> On Mac OS 10.10.4:
> make check
>
> Additionally manually tested mesos-execute with both responsive and
> unresponsive (https://github.com/rukletsov/unresponsive-process) tasks:
> ./src/mesos-execute --master=127.0.0.1:5050 --name=test --command="sleep 10"
> --env='{"GLOG_v": "2"}' --kill_after=2secs
> ./src/mesos-execute --master=127.0.0.1:5050 --name=test
> --command="/Users/alex/bin/unresponsive_process" --env='{"GLOG_v": "2"}'
> --kill_after=2secs
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Review Request 46369: Added capabilities support in ContanerInfo protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46369/ --- Review request for mesos and Jie Yu. Bugs: MESOS-5232 https://issues.apache.org/jira/browse/MESOS-5232 Repository: mesos Description --- Added capabilities support in ContanerInfo protobuf. Diffs - include/mesos/mesos.proto 63c181ae0a1e350fc27e36b1698e02db100b8861 Diff: https://reviews.apache.org/r/46369/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 46370: Introduced linux capabilities API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46370/ --- Review request for mesos, Jie Yu and Kevin Klues. Bugs: MESOS-5051 https://issues.apache.org/jira/browse/MESOS-5051 Repository: mesos Description --- This change introduces basic API for linux capabilities. This is not a comprehensive API but is strictly limited to the need for securing Mesos containers using linux capabilities. Diffs - src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 src/linux/capabilities.hpp PRE-CREATION src/linux/capabilities.cpp PRE-CREATION Diff: https://reviews.apache.org/r/46370/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 46371: Added basic tests for capabilities API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46371/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- Added basic tests for capabilities API. Diffs - src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 src/tests/capabilities_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/46371/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 46370: Introduced linux capabilities API.
> On April 20, 2016, 3:15 a.m., Kevin Klues wrote: > > src/linux/capabilities.hpp, lines 32-33 > > <https://reviews.apache.org/r/46370/diff/1/?file=1350684#file1350684line32> > > > > This should all probably live in the mesos::internal::capabilities > > namespace. The reasons why I chose to have it in mesos::internal is that we have Capabilities class (explained in the class's documentation why we need a class). > On April 20, 2016, 3:15 a.m., Kevin Klues wrote: > > src/linux/capabilities.hpp, lines 94-99 > > <https://reviews.apache.org/r/46370/diff/1/?file=1350684#file1350684line94> > > > > From my reading of: > > http://man7.org/linux/man-pages/man7/capabilities.7.html > > > > this enum should probably be called `Set`. > > > > Note, the name `Capability` at the front is unnecessary if we embed > > this in the `capabilities` namespace. > > > > Also, it's pretty standard practice in C++ to define an `enum` as a > > `enum class` for better type checking. As such, you can define the final > > element with a common name of `COUNT` to get at the size of the enum. > > > > For example, you can get at the size of the enum as: > > `capabilities::Set::COUNT` instead of relying on the `const` for > > `NUMBER_OF_CAP_SETS` defined above. Although i agree that C++11 supports enum classes, couple of reasons for using plain enum here: 1. Other places in the code mostly use plain old enums. 2. Its hard(not impossible) to get the value of the enum class's element (say for printing). > On April 20, 2016, 3:15 a.m., Kevin Klues wrote: > > src/linux/capabilities.hpp, line 178 > > <https://reviews.apache.org/r/46370/diff/1/?file=1350684#file1350684line178> > > > > Didn't we discuss not making this a class, and only having get()/set() > > calls as part of the namespace? Explained in the class's documentation. > On April 20, 2016, 3:15 a.m., Kevin Klues wrote: > > src/linux/capabilities.hpp, line 209 > > <https://reviews.apache.org/r/46370/diff/1/?file=1350684#file1350684line209> > > > > What did we decide about the `add()` pairing to this `drop()` call? As mentioned in the documentation of `drop`, the `drop` API is for dropping `bounding` capabilities. > On April 20, 2016, 3:15 a.m., Kevin Klues wrote: > > src/linux/capabilities.cpp, lines 36-38 > > <https://reviews.apache.org/r/46370/diff/1/?file=1350685#file1350685line36> > > > > Is there not a header file you can just include here? No the standard header files dont provide the syscall declaration. > On April 20, 2016, 3:15 a.m., Kevin Klues wrote: > > src/linux/capabilities.hpp, lines 50-90 > > <https://reviews.apache.org/r/46370/diff/1/?file=1350684#file1350684line50> > > > > Since we should probably be embedding this in a `capabilities` > > namespace, it is redundant to call this enum `Capability`. I'd sugggest > > `Privilege`. That way one of these can be accessed as e.g. > > `capabiliites::Privilege::SETGID`. > > > > Also, as mentioned in a comment below, this should probably be declared > > as an `enum class` for better type checking. > > > > The `COUNT` trick mentioned below should probably be applied here as > > well. I like Capability because that is what its referenced as in every documentation and literature. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46370/#review129674 --- On April 19, 2016, 5:02 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46370/ > --- > > (Updated April 19, 2016, 5:02 p.m.) > > > Review request for mesos, Jie Yu and Kevin Klues. > > > Bugs: MESOS-5051 > https://issues.apache.org/jira/browse/MESOS-5051 > > > Repository: mesos > > > Description > --- > > This change introduces basic API for linux capabilities. This is not a > comprehensive API but is strictly limited to the need for securing Mesos > containers using linux capabilities. > > > Diffs > - > > src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 > src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 > src/linux/capabilities.hpp PRE-CREATION > src/linux/capabilities.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46370/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 46370: Introduced linux capabilities API.
> On April 20, 2016, 3:15 a.m., Kevin Klues wrote: > > src/linux/capabilities.hpp, lines 94-99 > > <https://reviews.apache.org/r/46370/diff/1/?file=1350684#file1350684line94> > > > > From my reading of: > > http://man7.org/linux/man-pages/man7/capabilities.7.html > > > > this enum should probably be called `Set`. > > > > Note, the name `Capability` at the front is unnecessary if we embed > > this in the `capabilities` namespace. > > > > Also, it's pretty standard practice in C++ to define an `enum` as a > > `enum class` for better type checking. As such, you can define the final > > element with a common name of `COUNT` to get at the size of the enum. > > > > For example, you can get at the size of the enum as: > > `capabilities::Set::COUNT` instead of relying on the `const` for > > `NUMBER_OF_CAP_SETS` defined above. > > Jojy Varghese wrote: > Although i agree that C++11 supports enum classes, couple of reasons for > using plain enum here: > 1. Other places in the code mostly use plain old enums. > 2. Its hard(not impossible) to get the value of the enum class's element > (say for printing). regarding naming, If you look at the code of libcap (a standard portable capability interface) (https://github.com/abstrakraft/lxc-android-libcap/blob/master/libcap/libcap.h), the name `set` is referenced for the value and not the type. In fact the `SystemCallPayload` structure in my code is same from libcap. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46370/#review129674 --- On April 19, 2016, 5:02 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46370/ > --- > > (Updated April 19, 2016, 5:02 p.m.) > > > Review request for mesos, Jie Yu and Kevin Klues. > > > Bugs: MESOS-5051 > https://issues.apache.org/jira/browse/MESOS-5051 > > > Repository: mesos > > > Description > --- > > This change introduces basic API for linux capabilities. This is not a > comprehensive API but is strictly limited to the need for securing Mesos > containers using linux capabilities. > > > Diffs > - > > src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 > src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 > src/linux/capabilities.hpp PRE-CREATION > src/linux/capabilities.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46370/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 46370: Introduced linux capabilities API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46370/ --- (Updated April 20, 2016, 7:05 p.m.) Review request for mesos, Jie Yu and Kevin Klues. Bugs: MESOS-5051 https://issues.apache.org/jira/browse/MESOS-5051 Repository: mesos Description --- This change introduces basic API for linux capabilities. This is not a comprehensive API but is strictly limited to the need for securing Mesos containers using linux capabilities. Diffs (updated) - src/CMakeLists.txt 8e3eecb172e6c6b5256313f0937dc1156e91 src/Makefile.am 08d5279bbc0c763255fda3c3360d8f24f41bccda src/linux/capabilities.hpp PRE-CREATION src/linux/capabilities.cpp PRE-CREATION Diff: https://reviews.apache.org/r/46370/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 46371: Added basic tests for capabilities API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46371/ --- (Updated April 20, 2016, 7:12 p.m.) Review request for mesos and Jie Yu. Repository: mesos Description --- Added basic tests for capabilities API. Diffs (updated) - src/Makefile.am 08d5279bbc0c763255fda3c3360d8f24f41bccda src/tests/capabilities_test_helper.hpp PRE-CREATION src/tests/capabilities_test_helper.cpp PRE-CREATION src/tests/capabilities_test_helper_main.cpp PRE-CREATION src/tests/capabilities_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/46371/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 46370: Introduced linux capabilities API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46370/ --- (Updated April 20, 2016, 8:19 p.m.) Review request for mesos, Jie Yu and Kevin Klues. Bugs: MESOS-5051 https://issues.apache.org/jira/browse/MESOS-5051 Repository: mesos Description --- This change introduces basic API for linux capabilities. This is not a comprehensive API but is strictly limited to the need for securing Mesos containers using linux capabilities. Diffs (updated) - src/CMakeLists.txt 8e3eecb172e6c6b5256313f0937dc1156e91 src/Makefile.am 08d5279bbc0c763255fda3c3360d8f24f41bccda src/linux/capabilities.hpp PRE-CREATION src/linux/capabilities.cpp PRE-CREATION Diff: https://reviews.apache.org/r/46370/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 46370: Introduced linux capabilities API.
> On April 20, 2016, 3:15 a.m., Kevin Klues wrote: > > src/linux/capabilities.cpp, lines 124-125 > > <https://reviews.apache.org/r/46370/diff/1/?file=1350685#file1350685line124> > > > > This should be unnecessary. See: > > https://github.com/klueska-mesosphere/mesos/blob/master/src/linux/cgroups.cpp#L2435 Not sure about this as I like to lean towards good programming practice than depending on compiler flags. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46370/#review129674 --- On April 20, 2016, 8:19 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46370/ > --- > > (Updated April 20, 2016, 8:19 p.m.) > > > Review request for mesos, Jie Yu and Kevin Klues. > > > Bugs: MESOS-5051 > https://issues.apache.org/jira/browse/MESOS-5051 > > > Repository: mesos > > > Description > --- > > This change introduces basic API for linux capabilities. This is not a > comprehensive API but is strictly limited to the need for securing Mesos > containers using linux capabilities. > > > Diffs > - > > src/CMakeLists.txt 8e3eecb172e6c6b5256313f0937dc1156e91 > src/Makefile.am 08d5279bbc0c763255fda3c3360d8f24f41bccda > src/linux/capabilities.hpp PRE-CREATION > src/linux/capabilities.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46370/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 46370: Introduced linux capabilities API.
> On April 20, 2016, 3:15 a.m., Kevin Klues wrote: > > I added a few comments below, but in general, I feel like there are places > > this code could be greatly simplified. Specifically, it's not obvious to > > me why we need all of the different classes you define (or maybe more about > > why they need to be part of the public interface). Could you clarify the > > rational here a bit? Classes like `CapabilityFlags` allows good encapsulation of the bitmap and makes the API more fiendly to deal with. Leaving them as bitmap would leave the complexity to the caller. Same idea with other classes. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46370/#review129674 --- On April 20, 2016, 8:19 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46370/ > --- > > (Updated April 20, 2016, 8:19 p.m.) > > > Review request for mesos, Jie Yu and Kevin Klues. > > > Bugs: MESOS-5051 > https://issues.apache.org/jira/browse/MESOS-5051 > > > Repository: mesos > > > Description > --- > > This change introduces basic API for linux capabilities. This is not a > comprehensive API but is strictly limited to the need for securing Mesos > containers using linux capabilities. > > > Diffs > - > > src/CMakeLists.txt 8e3eecb172e6c6b5256313f0937dc1156e91 > src/Makefile.am 08d5279bbc0c763255fda3c3360d8f24f41bccda > src/linux/capabilities.hpp PRE-CREATION > src/linux/capabilities.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46370/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jojy Varghese > >
Re: Review Request 46369: Added capabilities support in ContanerInfo protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46369/ --- (Updated April 21, 2016, 4:58 p.m.) Review request for mesos and Jie Yu. Bugs: MESOS-5232 https://issues.apache.org/jira/browse/MESOS-5232 Repository: mesos Description --- Added capabilities support in ContanerInfo protobuf. Diffs (updated) - include/mesos/mesos.proto 87af4a06db8cc3889fe4d3b314206103f5ce5f2d include/mesos/v1/mesos.proto 34da0a1484dc2f71262d8b97484b8edaae35bb6c Diff: https://reviews.apache.org/r/46369/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 46182: Adding app runtime processing to appc store and provisioner.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46182/#review129922
---
include/mesos/appc/spec.proto (line 62)
<https://reviews.apache.org/r/46182/#comment193475>
Todo comment style is `TODO(name): `. Also we end comments with
period.
src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 199)
<https://reviews.apache.org/r/46182/#comment193481>
Why this variable?
src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 202)
<https://reviews.apache.org/r/46182/#comment193478>
indentation.
src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 203)
<https://reviews.apache.org/r/46182/#comment193479>
recommend better name for the variable `manifest1`
src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 213)
<https://reviews.apache.org/r/46182/#comment193480>
This could return the default contructed ImageManifest object if none of
the images's manifest could be found.
src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 307)
<https://reviews.apache.org/r/46182/#comment193476>
Fix indentation
src/slave/containerizer/mesos/provisioner/store.hpp (line 26)
<https://reviews.apache.org/r/46182/#comment193477>
headers should be alphabetically sorted.
- Jojy Varghese
On April 15, 2016, 6:23 p.m., Srinivas Brahmaroutu wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46182/
> ---
>
> (Updated April 15, 2016, 6:23 p.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Adding app runtime processing to appc store and provisioner.
>
>
> Diffs
> -
>
> include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4
> include/mesos/slave/isolator.proto 60a9bb637e12593a97ed1a7c510ebccd4e5a9615
> src/slave/containerizer/mesos/provisioner/appc/store.cpp
> 33e97fc645a9b7fbc9ae47f67c1b5dacf999fce0
> src/slave/containerizer/mesos/provisioner/provisioner.hpp
> 5c96e9f6603d39889e6bc807874d35d0cb3556be
> src/slave/containerizer/mesos/provisioner/provisioner.cpp
> dcbbbaf797d4467bfd0bb1ee91ee9ce843e7d546
> src/slave/containerizer/mesos/provisioner/store.hpp
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746
>
> Diff: https://reviews.apache.org/r/46182/diff/
>
>
> Testing
> ---
>
> make check on linux passes
> AppcProvisionerIntegrationTest.ROOT_SimpleLinuxImageTest
>
>
> Thanks,
>
> Srinivas Brahmaroutu
>
>
Re: Review Request 34943: Added validation to flags.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34943/#review86981
---
3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
<https://reviews.apache.org/r/34943/#comment139192>
style question: Why not use static array declaration :
const char *args[] = {
"/path/to/program",
"blah"
};
and also make the function argument const char**?
- Jojy Varghese
On June 5, 2015, 2:48 p.m., Benjamin Hindman wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34943/
> ---
>
> (Updated June 5, 2015, 2:48 p.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Also refactored existing 'lambda::bind' arguments to use C++11 lambdas,
> enabling us to get rid of our "loader" and "stringifier" functors.
>
>
> Diffs
> -
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am
> 6ac2f04a6a1d8db1725972a3b4b46a0dd734566f
> 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp
> 87606d860dea3235ec70d127d3379065d42dc90b
> 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
> ee855da6128c2d671eb2fc7eaa2c0aab2341aebb
> 3rdparty/libprocess/3rdparty/stout/include/stout/flags/loader.hpp
> 51d3ab020bd2bae1f12b66cac0da5383c813d5d7
> 3rdparty/libprocess/3rdparty/stout/include/stout/flags/stringifier.hpp
> fda5ae1328a23a4371475652f921341dce7448d5
> 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
> 80450185f60c5b273face490e0bb9e695b0cb984
>
> Diff: https://reviews.apache.org/r/34943/diff/
>
>
> Testing
> ---
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 34943: Added validation to flags.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34943/#review87573 --- 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp <https://reviews.apache.org/r/34943/#comment139961> Style comment: Default captures are considered stylistically bad. Captures should be explicit(Meyers, Effective Modern C++, Item 31). - Jojy Varghese On June 11, 2015, 1:52 p.m., Benjamin Hindman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34943/ > --- > > (Updated June 11, 2015, 1:52 p.m.) > > > Review request for mesos. > > > Repository: mesos > > > Description > --- > > Also refactored existing 'lambda::bind' arguments to use C++11 lambdas, > enabling us to get rid of our "loader" and "stringifier" functors. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/stout/include/Makefile.am > 6ac2f04a6a1d8db1725972a3b4b46a0dd734566f > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp > 87606d860dea3235ec70d127d3379065d42dc90b > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp > ee855da6128c2d671eb2fc7eaa2c0aab2341aebb > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/loader.hpp > 51d3ab020bd2bae1f12b66cac0da5383c813d5d7 > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/stringifier.hpp > fda5ae1328a23a4371475652f921341dce7448d5 > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp > 95b4b33b70c37640d97dff5d5724550d42048b76 > 3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp > 134453e91d39fd086baa9396ab42a002fed8b73e > 3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp > 59cde89b5e035807a510b331b85a8cd48da36ae3 > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp > 4485e41feb8320d2ec7251b8b894ce437f61addd > 3rdparty/libprocess/3rdparty/stout/include/stout/os/sunos.hpp > cfe6d742560f50079fb1ed7346526d432615613c > 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp > 80450185f60c5b273face490e0bb9e695b0cb984 > 3rdparty/libprocess/include/Makefile.am > d01880c66ca544d62d3ada1ea79c8045884858da > 3rdparty/libprocess/include/process/firewall.hpp > 16ed852d07344e53c6f9d30d4cd7d99e07c6606d > 3rdparty/libprocess/include/process/process.hpp > e70dd388601bd26fcc82f66e0cb95924da8a8c2f > 3rdparty/libprocess/src/process.cpp > aadd7bb0ae12b93336900c76d8d5aaa4421ea198 > 3rdparty/libprocess/src/tests/process_tests.cpp > 660af45e7fd45bdf5d43ad9aa54477fd94f87058 > Doxyfile 1b0a2beda1d9223eafd9d14d335c651eb4961a15 > docs/configuration.md aaf65bfa2848318c8d07c772ba2c521aa72c2677 > docs/engineering-principles-and-practices.md > 6babb929ead758ee5187d5fc5760d084876c3298 > docs/home.md ee3d3e9a417d75af776dad3ea9627d3c6d780dca > docs/mesos-c++-style-guide.md 94107ed21c6f09349ce691f9f4d36b43bbbe809e > docs/powered-by-mesos.md 0ee763ffe32e6667e4ba708dd275d9f1b5116815 > include/mesos/containerizer/containerizer.proto > f16ccc89f83da28c413ccfa0687a06b7515a605c > include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 > include/mesos/mesos.proto 7457ff11f6a55099ccb95beb2f0ccb9a2f7ccd87 > src/Makefile.am 10b190288425d735b116ed2dd3f040e871c5cb18 > src/cli/mesos-ps ae48c8cde31e90024381ed3e6e22ec3bc96560b6 > src/examples/no_executor_framework.cpp > 6c5c7d3121e3a2ea78f66dffdeaecb72cca6293c > src/files/files.cpp 3cdd38a3c7122bd5e13c8928279d85ab1373a63e > src/linux/routing/handle.hpp 052c7cc1a967797d245a275d08cc774f627398a5 > src/linux/routing/queueing/fq_codel.hpp > 18c53b416718c27045986d939bb85f9fec731ca5 > src/linux/routing/queueing/fq_codel.cpp > 446863391d1c6dd2f91f890596927a20ee59a35f > src/main.dox d5b29cf96b18ba84ac39f1747c4a174a5bfbc2e1 > src/master/allocator/mesos/allocator.hpp > 6cfa04650d91a80211cfbc0809236f9438926c78 > src/master/allocator/mesos/hierarchical.hpp > c220ea673320d4d06403afa76f0eb9f3b7a4e5eb > src/master/flags.hpp 55ed3a9ef84a39841305cca2ba6c5df45c1990e1 > src/master/flags.cpp 4377715029878cfee36f067e9c53c42b522b79d8 > src/master/main.cpp 1c33e3bf7baae45b1671f9566b4993f39d9f1294 > src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 > src/master/master.cpp e91e8815471b49d3c649470197d6da3f06de62fb > src/master/metrics.hpp 3d389e68f22e3d1e00bde0db4e25f897c79a2316 > src/master/metrics.cpp d2489c8
Re: Review Request 29406: Introduce libevent ssl socket.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29406/#review87885 --- 3rdparty/libprocess/include/process/address.hpp <https://reviews.apache.org/r/29406/#comment140298> General question about the net namespace in the code: why is the network structure not templatized on the Layer 3(IP) family(inet/inet6 etc)? This would make it easy to specialize on these families and also make Ipv6 easy. - Jojy Varghese On June 13, 2015, 9:23 a.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29406/ > --- > > (Updated June 13, 2015, 9:23 a.m.) > > > Review request for mesos, Benjamin Hindman and Michael Park. > > > Bugs: MESOS-1913 > https://issues.apache.org/jira/browse/MESOS-1913 > > > Repository: mesos > > > Description > --- > > Requires: > configure --enable-libevent --enable-libevent-socket --enable-ssl > New environment variables: > ``` > SSL_ENABLED=(false|0,true|1) > SSL_CERT_FILE=(path to certificate) > SSL_KEY_FILE=(path to key) > SSL_VERIFY_CERT=(false|0,true|1) > SSL_REQUIRE_CERT=(false|0,true|1) > SSL_VERIFY_DEPTH=(4) > SSL_CA_DIR=(path to CA directory) > SSL_CA_FILE=(path to CA file) > SSL_CIPHERS=(accepted ciphers separated by ':') > SSL_ENABLE_SSL_V2=(false|0,true|1) > SSL_ENABLE_SSL_V3=(false|0,true|1) > SSL_ENABLE_TLS_V1_0=(false|0,true|1) > SSL_ENABLE_TLS_V1_1=(false|0,true|1) > SSL_ENABLE_TLS_V1_2=(false|0,true|1) > ``` > > Only TLSV1.2 is enabled by default. > Use the `ENABLE_SSL_V*` and `ENABLE_TLS_V*` environment variables to open up > more protocols. > Use the `SSL_CIPHERS` environment variable to restrict or open up the > supported ciphers. > > > Diffs > - > > 3rdparty/libprocess/Makefile.am 489ce359f383d819299335cbaa8c95724b0c6ac2 > 3rdparty/libprocess/include/process/address.hpp > 729f5cd7ea981e43a33c1fe9d99d58b906a31158 > 3rdparty/libprocess/include/process/socket.hpp > b8c2274de535ac473e49a09165b601c96d3ebe8b > 3rdparty/libprocess/src/libevent.hpp > f6cc72178613a30446629532a773afccfd404212 > 3rdparty/libprocess/src/libevent.cpp > fb038597358135a06c1927d079cb7cb09fea7452 > 3rdparty/libprocess/src/libevent_ssl_socket.hpp PRE-CREATION > 3rdparty/libprocess/src/libevent_ssl_socket.cpp PRE-CREATION > 3rdparty/libprocess/src/openssl.hpp PRE-CREATION > 3rdparty/libprocess/src/openssl.cpp PRE-CREATION > 3rdparty/libprocess/src/process.cpp > aadd7bb0ae12b93336900c76d8d5aaa4421ea198 > 3rdparty/libprocess/src/socket.cpp 0e1cebb19e21c706b152d35a0b8722924c971a35 > > Diff: https://reviews.apache.org/r/29406/diff/ > > > Testing > --- > > make check (uses non-ssl socket) > benchmarks using ssl sockets > master, slave, framework, webui launch with ssl sockets > > > Thanks, > > Joris Van Remoortere > >
Re: Review Request 29406: Introduce libevent ssl socket.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29406/#review87936 --- 3rdparty/libprocess/include/process/address.hpp <https://reviews.apache.org/r/29406/#comment140363> There are a few "if family == INET" (or similar) in the code. By specializing the net address structures on FAMILY, we will get rid of them. As most of the network structures (socket for example) are classified based on family, this should naturally fit into the overall scheme. - Jojy Varghese On June 13, 2015, 9:23 a.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29406/ > --- > > (Updated June 13, 2015, 9:23 a.m.) > > > Review request for mesos, Benjamin Hindman and Michael Park. > > > Bugs: MESOS-1913 > https://issues.apache.org/jira/browse/MESOS-1913 > > > Repository: mesos > > > Description > --- > > Requires: > configure --enable-libevent --enable-libevent-socket --enable-ssl > New environment variables: > ``` > SSL_ENABLED=(false|0,true|1) > SSL_CERT_FILE=(path to certificate) > SSL_KEY_FILE=(path to key) > SSL_VERIFY_CERT=(false|0,true|1) > SSL_REQUIRE_CERT=(false|0,true|1) > SSL_VERIFY_DEPTH=(4) > SSL_CA_DIR=(path to CA directory) > SSL_CA_FILE=(path to CA file) > SSL_CIPHERS=(accepted ciphers separated by ':') > SSL_ENABLE_SSL_V2=(false|0,true|1) > SSL_ENABLE_SSL_V3=(false|0,true|1) > SSL_ENABLE_TLS_V1_0=(false|0,true|1) > SSL_ENABLE_TLS_V1_1=(false|0,true|1) > SSL_ENABLE_TLS_V1_2=(false|0,true|1) > ``` > > Only TLSV1.2 is enabled by default. > Use the `ENABLE_SSL_V*` and `ENABLE_TLS_V*` environment variables to open up > more protocols. > Use the `SSL_CIPHERS` environment variable to restrict or open up the > supported ciphers. > > > Diffs > - > > 3rdparty/libprocess/Makefile.am 489ce359f383d819299335cbaa8c95724b0c6ac2 > 3rdparty/libprocess/include/process/address.hpp > 729f5cd7ea981e43a33c1fe9d99d58b906a31158 > 3rdparty/libprocess/include/process/socket.hpp > b8c2274de535ac473e49a09165b601c96d3ebe8b > 3rdparty/libprocess/src/libevent.hpp > f6cc72178613a30446629532a773afccfd404212 > 3rdparty/libprocess/src/libevent.cpp > fb038597358135a06c1927d079cb7cb09fea7452 > 3rdparty/libprocess/src/libevent_ssl_socket.hpp PRE-CREATION > 3rdparty/libprocess/src/libevent_ssl_socket.cpp PRE-CREATION > 3rdparty/libprocess/src/openssl.hpp PRE-CREATION > 3rdparty/libprocess/src/openssl.cpp PRE-CREATION > 3rdparty/libprocess/src/process.cpp > aadd7bb0ae12b93336900c76d8d5aaa4421ea198 > 3rdparty/libprocess/src/socket.cpp 0e1cebb19e21c706b152d35a0b8722924c971a35 > > Diff: https://reviews.apache.org/r/29406/diff/ > > > Testing > --- > > make check (uses non-ssl socket) > benchmarks using ssl sockets > master, slave, framework, webui launch with ssl sockets > > > Thanks, > > Joris Van Remoortere > >
Review Request 35544: containerizer: statically initialize isolator factories
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35544/ --- Review request for mesos and Timothy Chen. Repository: mesos Description --- Replaced dynamic hashmap creation with c++11's static initialization. Diffs - src/slave/containerizer/mesos/containerizer.cpp 8c102fb7d1f79ee768cb06de3a976ea12f958712 Diff: https://reviews.apache.org/r/35544/diff/ Testing --- make check Thanks, Jojy Varghese
Re: Review Request 35544: containerizer: statically initialize isolator factories
> On June 17, 2015, 9:38 a.m., Till Toenshoff wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 135 > > <https://reviews.apache.org/r/35544/diff/1/?file=986152#file986152line135> > > > > Why is this change needed? the operator[] is not constant. So we cant use it for constant objects. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35544/#review88203 --- On June 17, 2015, 12:13 a.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35544/ > --- > > (Updated June 17, 2015, 12:13 a.m.) > > > Review request for mesos and Timothy Chen. > > > Repository: mesos > > > Description > --- > > Replaced dynamic hashmap creation with c++11's static initialization. > > > Diffs > - > > src/slave/containerizer/mesos/containerizer.cpp > 8c102fb7d1f79ee768cb06de3a976ea12f958712 > > Diff: https://reviews.apache.org/r/35544/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jojy Varghese > >
Review Request 35743: flags: fixed const'ness of load
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35743/ --- Review request for mesos, Benjamin Hindman and Till Toenshoff. Repository: mesos Description --- Changes: - replacing argument arrays with static initializers in tests. - making the argument 'argv' const in load method to reflect the semantics. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 7584cb871d02ad01021f0c3439ea205736d4f6b4 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp c2c6a6ac97044f2317418295f48d75e94de4112b Diff: https://reviews.apache.org/r/35743/diff/ Testing --- make check Thanks, Jojy Varghese
Re: Review Request 35743: flags: fixed const'ness of load
> On June 22, 2015, 10:02 p.m., Anand Mazumdar wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp, line 418 > > <https://reviews.apache.org/r/35743/diff/1/?file=989917#file989917line418> > > > > boost::size(...) would suffice here too albeit minus the small trivial > > run-time cost. Not sure if adding a boost dependency just for size operator would get us anything here. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35743/#review88852 --- On June 22, 2015, 9:27 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35743/ > --- > > (Updated June 22, 2015, 9:27 p.m.) > > > Review request for mesos, Benjamin Hindman and Till Toenshoff. > > > Repository: mesos > > > Description > --- > > Changes: > - replacing argument arrays with static initializers in tests. > - making the argument 'argv' const in load method to reflect the semantics. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp > 7584cb871d02ad01021f0c3439ea205736d4f6b4 > 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp > c2c6a6ac97044f2317418295f48d75e94de4112b > > Diff: https://reviews.apache.org/r/35743/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jojy Varghese > >
Re: Review Request 35743: flags: fixed const'ness of load
> On June 22, 2015, 10:02 p.m., Anand Mazumdar wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp, line 418 > > <https://reviews.apache.org/r/35743/diff/1/?file=989917#file989917line418> > > > > boost::size(...) would suffice here too albeit minus the small trivial > > run-time cost. > > Jojy Varghese wrote: > Not sure if adding a boost dependency just for size operator would get us > anything here. > > Anand Mazumdar wrote: > Then can we change these to std::array and just invoke the size() > function ? > > (Posting it again here , my bad , instead of adding a new comment) thats a possibility but at teh cost/inconvenience of having to change the std::array size argument every time we add/remove an argument in the test. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35743/#review88852 --- On June 23, 2015, 9:51 p.m., Jojy Varghese wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35743/ > --- > > (Updated June 23, 2015, 9:51 p.m.) > > > Review request for mesos, Benjamin Hindman, Joris Van Remoortere, and Till > Toenshoff. > > > Repository: mesos > > > Description > --- > > Changes: > - replacing argument arrays with static initializers in tests. > - making the argument 'argv' const in load method to reflect the semantics. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp > 7584cb871d02ad01021f0c3439ea205736d4f6b4 > 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp > c2c6a6ac97044f2317418295f48d75e94de4112b > > Diff: https://reviews.apache.org/r/35743/diff/ > > > Testing > --- > > make check > > > Thanks, > > Jojy Varghese > >
Re: Review Request 35743: flags: fixed const'ness of load
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35743/ --- (Updated June 25, 2015, 12:29 a.m.) Review request for mesos, Benjamin Hindman, Joris Van Remoortere, and Till Toenshoff. Changes --- addressed review comments. Note: Chose to move the util function to util file as opposed to calling std::extent as std::extent looked like a heavy hammer to solve something simple. Repository: mesos Description --- Changes: - replacing argument arrays with static initializers in tests. - making the argument 'argv' const in load method to reflect the semantics. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 7584cb871d02ad01021f0c3439ea205736d4f6b4 3rdparty/libprocess/3rdparty/stout/include/stout/utils.hpp 09a1dcd3b3a082544d221fbfeab9a3d3d9f85e2f 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp c2c6a6ac97044f2317418295f48d75e94de4112b Diff: https://reviews.apache.org/r/35743/diff/ Testing --- make check Thanks, Jojy Varghese
Review Request 36106: cgroups: added cpuacct subsystem
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- Review request for mesos, Ian Downes, Joris Van Remoortere, and Timothy Chen. Repository: mesos Description --- cgroups implementation does not have a cpuacct subsystem implementation as of today. Adding the implementation for stat function. Changes: - added Stats class to encapsulate cpuacct.stat data - added implementation for cpuacct::stats - added unit tests Jira: MESOS-2961 Diffs - src/linux/cgroups.hpp 73b98317880eea3d6a2ba37ac56d1f7e3600ba94 src/linux/cgroups.cpp 4c006d0c7382b940a83359d636c0d48952cdbb00 src/tests/cgroups_tests.cpp 475f48a474eea708f98d8c0300862351a2d4379a Diff: https://reviews.apache.org/r/36106/diff/ Testing --- make check Thanks, Jojy Varghese
Re: Review Request 36106: cgroups: added cpuacct subsystem
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36106/ --- (Updated July 1, 2015, 9:38 p.m.) Review request for mesos, Ian Downes, Joris Van Remoortere, and Timothy Chen. Repository: mesos Description (updated) --- cgroups implementation does not have a cpuacct subsystem implementation as of today. Adding the implementation for stat function. Changes: - added Stats class to encapsulate cpuacct.stat data - added implementation for cpuacct::stats - added unit tests Jira: MESOS-2961 Diffs - src/linux/cgroups.hpp 73b98317880eea3d6a2ba37ac56d1f7e3600ba94 src/linux/cgroups.cpp 4c006d0c7382b940a83359d636c0d48952cdbb00 src/tests/cgroups_tests.cpp 475f48a474eea708f98d8c0300862351a2d4379a Diff: https://reviews.apache.org/r/36106/diff/ Testing --- make check Thanks, Jojy Varghese
