You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gilbert Song <so...@gmail.com> on 2017/01/03 22:47:33 UTC

Re: Review Request 50871: Supported auto backend in Unified Containerizer.

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

(Updated Jan. 3, 2017, 2:47 p.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

Select a backend smartly. Currently, the logic is really
simple:
1. Use overlayfs backend if it exists.
2. Use aufs backend if the overlayfs does not exists.
3. Use copy backend of both overlayfs and aufs do not exist.
Please note that the bind backend needs to be specified explicitly
through the agent flag '--image_provisioner_backend' since it
requires the sandbox already existed.


Diffs (updated)
-----

  docs/configuration.md e4beb2d5a72f1c5f59b2e40f4984cc60b8437d9d 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp b06ddff68a8d2df13abb838b03a8e73d4e273c31 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp db2d267996959453f9896287320dca37440b499b 
  src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
  src/slave/flags.cpp 1eccea920338032173be4df6c374ec50dbd2eaf3 
  src/tests/containerizer/provisioner_appc_tests.cpp 8b4b832a8fc022435b4d1ada8b8fd9c392ce685e 

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


Testing
-------

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/#review160442
-----------------------------------------------------------




src/slave/containerizer/mesos/provisioner/docker/paths.cpp (line 65)
<https://reviews.apache.org/r/50871/#comment231587>

    Backend no longer needed to be recorded when it's auto selected?



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 294)
<https://reviews.apache.org/r/50871/#comment231586>

    Remove this


- Timothy Chen


On Jan. 3, 2017, 10:47 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 3, 2017, 10:47 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md e4beb2d5a72f1c5f59b2e40f4984cc60b8437d9d 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp b06ddff68a8d2df13abb838b03a8e73d4e273c31 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp db2d267996959453f9896287320dca37440b499b 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp 1eccea920338032173be4df6c374ec50dbd2eaf3 
>   src/tests/containerizer/provisioner_appc_tests.cpp 8b4b832a8fc022435b4d1ada8b8fd9c392ce685e 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.

> On Jan. 19, 2017, 3:50 p.m., James Peach wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp, line 71
> > <https://reviews.apache.org/r/50871/diff/6/?file=1609612#file1609612line71>
> >
> >     For future maintenance, I think it would be very helpful to give some extended rationale for this support matrix.

Thanks. Craeted a followup document JIRA:
https://issues.apache.org/jira/browse/MESOS-6960


> On Jan. 19, 2017, 3:50 p.m., James Peach wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp, line 85
> > <https://reviews.apache.org/r/50871/diff/6/?file=1609612#file1609612line85>
> >
> >     This might be a bit picky, but I'd be inclined to avoid the hashset allocation in this case.
> >     
> >     Maybe:
> >     
> >     ```
> >     const std::array<fstype_t, 5> o {
> >     FS_TYPE_AUFS, FS_TYPE_BTRFS, FS_TYPE_ECRYPTFS,
> >     ...
> >     };
> >     
> >     std::find(o.begin(), o.end(), needle);
> >     
> >     ```
> >     
> >     It's unfortunately verbose though :(

Agreed. Using std::find is faster.


- Gilbert


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


On Jan. 19, 2017, 10:45 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2017, 10:45 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp ac9dae8ecbb897b8ff942d11ac70281a63e06831 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/containerizer/mesos/provisioner/store.cpp 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/#review162363
-----------------------------------------------------------




src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 69)
<https://reviews.apache.org/r/50871/#comment233725>

    For future maintenance, I think it would be very helpful to give some extended rationale for this support matrix.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 80)
<https://reviews.apache.org/r/50871/#comment233726>

    You really only need the strings to generate a sane error message. For actual validation, you can use the `f_type` constants directly.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 83)
<https://reviews.apache.org/r/50871/#comment233727>

    This might be a bit picky, but I'd be inclined to avoid the hashset allocation in this case.
    
    Maybe:
    
    ```
    const std::array<fstype_t, 5> o {
    FS_TYPE_AUFS, FS_TYPE_BTRFS, FS_TYPE_ECRYPTFS,
    ...
    };
    
    std::find(o.begin(), o.end(), needle);
    
    ```
    
    It's unfortunately verbose though :(


- James Peach


On Jan. 19, 2017, 6:45 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2017, 6:45 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp ac9dae8ecbb897b8ff942d11ac70281a63e06831 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/containerizer/mesos/provisioner/store.cpp 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

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



Patch looks great!

Reviews applied: [54216, 54212, 54213, 54214, 54816, 54215, 55713, 55714, 50871]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Jan. 19, 2017, 6:45 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2017, 6:45 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp ac9dae8ecbb897b8ff942d11ac70281a63e06831 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/containerizer/mesos/provisioner/store.cpp 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

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



Patch looks great!

Reviews applied: [54216, 54212, 54213, 54214, 54816, 54215, 55713, 55714, 50871]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Jan. 24, 2017, 11:22 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
>   src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

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



Patch looks great!

Reviews applied: [54216, 54212, 54213, 54214, 54816, 54215, 55713, 55714, 50871]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Jan. 26, 2017, 1:37 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2017, 1:37 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
>   src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.

> On March 16, 2017, 7:42 p.m., kaiyu shi wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp
> > Lines 171 (patched)
> > <https://reviews.apache.org/r/50871/diff/10/?file=1616083#file1616083line173>
> >
> >     Perhaps a typo
> >     `OVERLAY_BACKEND` should be `AUFS_BACKEND`

Thanks for reviewing on submitted patches!

It is already fixed in the followup patches.

please see:
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/provisioner/provisioner.cpp#L212~#L245


- Gilbert


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


On Jan. 25, 2017, 5:37 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2017, 5:37 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
>   src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 
> 
> 
> Diff: https://reviews.apache.org/r/50871/diff/10/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by kaiyu shi <sk...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/#review169251
-----------------------------------------------------------




src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 171 (patched)
<https://reviews.apache.org/r/50871/#comment241596>

    Perhaps a typo
    `OVERLAY_BACKEND` should be `AUFS_BACKEND`


- kaiyu shi


On Jan. 26, 2017, 1:37 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2017, 1:37 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
>   src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 
> 
> 
> Diff: https://reviews.apache.org/r/50871/diff/10/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/#review163438
-----------------------------------------------------------


Fix it, then Ship it!




I'll fix the issues below for you.


src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (lines 50 - 51)
<https://reviews.apache.org/r/50871/#comment234902>

    No needed anymore?



src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (lines 231 - 257)
<https://reviews.apache.org/r/50871/#comment234905>

    Looks like this missing layer check is best effort now. I'd suggest we simply remove it. The 'storedImage' will simply be a layer id cache.



src/slave/containerizer/mesos/provisioner/docker/paths.hpp (line 22)
<https://reviews.apache.org/r/50871/#comment234904>

    Why this?



src/slave/containerizer/mesos/provisioner/provisioner.hpp (line 117)
<https://reviews.apache.org/r/50871/#comment234892>

    Do you still need this for ProvisionerProcess?



src/slave/containerizer/mesos/provisioner/provisioner.cpp (lines 68 - 69)
<https://reviews.apache.org/r/50871/#comment234901>

    It's not host filesystem, but the underlying filesystem for a specific directory.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 78)
<https://reviews.apache.org/r/50871/#comment234900>

    I'd rename this to 'validateBackend'



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 121)
<https://reviews.apache.org/r/50871/#comment234899>

    I would kill this helper as it's only used once. I'll just inline the implementation in Provisioner::create.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 122)
<https://reviews.apache.org/r/50871/#comment234894>

    I would use `const Option<string>& backend` here.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 127)
<https://reviews.apache.org/r/50871/#comment234896>

    We should not only check 'provisionerDir' but also all the store dir as well. I'll leave a TODO here at the moment. Please create a ticket to track.
    
    I think what we might need to track a per store default backend because each store might have a different store dir.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 176)
<https://reviews.apache.org/r/50871/#comment234898>

    What if copy backend is not supported? I know it is not possible at the moment, but would prefer writing the code a bit more defensively in case there're future changes.


- Jie Yu


On Jan. 26, 2017, 1:37 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2017, 1:37 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
>   src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/
-----------------------------------------------------------

(Updated Jan. 25, 2017, 5:37 p.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Changes
-------

Cached the `defaultBackend` in `ProvisionerProcess`.


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


Repository: mesos


Description
-------

Select a backend smartly. Currently, the logic is really
simple:
1. Use overlayfs backend if it exists.
2. Use aufs backend if the overlayfs does not exists.
3. Use copy backend of both overlayfs and aufs do not exist.
Please note that the bind backend needs to be specified explicitly
through the agent flag '--image_provisioner_backend' since it
requires the sandbox already existed.


Diffs (updated)
-----

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
  src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
  src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
  src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
  src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
  src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
  src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 

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


Testing
-------

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/
-----------------------------------------------------------

(Updated Jan. 24, 2017, 3:22 p.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
-------

Select a backend smartly. Currently, the logic is really
simple:
1. Use overlayfs backend if it exists.
2. Use aufs backend if the overlayfs does not exists.
3. Use copy backend of both overlayfs and aufs do not exist.
Please note that the bind backend needs to be specified explicitly
through the agent flag '--image_provisioner_backend' since it
requires the sandbox already existed.


Diffs (updated)
-----

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
  src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
  src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
  src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
  src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
  src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
  src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 

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


Testing
-------

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/
-----------------------------------------------------------

(Updated Jan. 24, 2017, 12:37 p.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
-------

Select a backend smartly. Currently, the logic is really
simple:
1. Use overlayfs backend if it exists.
2. Use aufs backend if the overlayfs does not exists.
3. Use copy backend of both overlayfs and aufs do not exist.
Please note that the bind backend needs to be specified explicitly
through the agent flag '--image_provisioner_backend' since it
requires the sandbox already existed.


Diffs (updated)
-----

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
  src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
  src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
  src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
  src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
  src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
  src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 

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


Testing
-------

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

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



Patch looks great!

Reviews applied: [54216, 54212, 54213, 54214, 54816, 54215, 55713, 55714, 50871]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Jan. 20, 2017, 10:11 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2017, 10:11 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp ac9dae8ecbb897b8ff942d11ac70281a63e06831 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/containerizer/mesos/provisioner/store.cpp 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.

> On Jan. 22, 2017, 7:12 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.hpp, lines 149-154
> > <https://reviews.apache.org/r/50871/diff/7/?file=1610398#file1610398line149>
> >
> >     I'd suggest we call this `defaultBackend`. I would try to avoid passing `backend` to `Store::create` etc. Let's passing `backend` down in `Provisioner::provision` and `Store::get`. This way, we might be able to support per rootfs backend in the future. Thoughts?
> 
> Gilbert Song wrote:
>     The tradeoff is that we need to repeatly run this logic every time we call `provisioner::provision() and store::get()`. I would suggest we figure out the `defaultBackend` in `provisioner::create` and pass it all the way to `store::create()`, `puller::create()` and `metadata_manager::create()` etc. Then, to support rootfs backend per container in the future, we pass an `Option<string> backend` in `provisioner::provisioner` and `store::get()`.
>     
>     For now, I update the patch depending on your comment and leave a `TODO` for only figuring out `defaultBackend` once at the very beginning `provisioner::create()`.
> 
> Gilbert Song wrote:
>     Basically, I still keep the previous implementation as another local branch. Please let me know if we should change it back.

Please dismiss my comments above.


- Gilbert


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


On Jan. 24, 2017, 3:22 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2017, 3:22 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
>   src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.

> On Jan. 22, 2017, 7:12 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.hpp, lines 149-154
> > <https://reviews.apache.org/r/50871/diff/7/?file=1610398#file1610398line149>
> >
> >     I'd suggest we call this `defaultBackend`. I would try to avoid passing `backend` to `Store::create` etc. Let's passing `backend` down in `Provisioner::provision` and `Store::get`. This way, we might be able to support per rootfs backend in the future. Thoughts?
> 
> Gilbert Song wrote:
>     The tradeoff is that we need to repeatly run this logic every time we call `provisioner::provision() and store::get()`. I would suggest we figure out the `defaultBackend` in `provisioner::create` and pass it all the way to `store::create()`, `puller::create()` and `metadata_manager::create()` etc. Then, to support rootfs backend per container in the future, we pass an `Option<string> backend` in `provisioner::provisioner` and `store::get()`.
>     
>     For now, I update the patch depending on your comment and leave a `TODO` for only figuring out `defaultBackend` once at the very beginning `provisioner::create()`.

Basically, I still keep the previous implementation as another local branch. Please let me know if we should change it back.


- Gilbert


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


On Jan. 24, 2017, 12:37 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2017, 12:37 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
>   src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.

> On Jan. 22, 2017, 7:12 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.hpp, lines 149-154
> > <https://reviews.apache.org/r/50871/diff/7/?file=1610398#file1610398line149>
> >
> >     I'd suggest we call this `defaultBackend`. I would try to avoid passing `backend` to `Store::create` etc. Let's passing `backend` down in `Provisioner::provision` and `Store::get`. This way, we might be able to support per rootfs backend in the future. Thoughts?

The tradeoff is that we need to repeatly run this logic every time we call `provisioner::provision() and store::get()`. I would suggest we figure out the `defaultBackend` in `provisioner::create` and pass it all the way to `store::create()`, `puller::create()` and `metadata_manager::create()` etc. Then, to support rootfs backend per container in the future, we pass an `Option<string> backend` in `provisioner::provisioner` and `store::get()`.

For now, I update the patch depending on your comment and leave a `TODO` for only figuring out `defaultBackend` once at the very beginning `provisioner::create()`.


- Gilbert


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


On Jan. 24, 2017, 12:37 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2017, 12:37 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
>   src/tests/containerizer/store.hpp 5abe3f6d211f1d8f5b2a3da977f5a46f4d13b207 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/#review162586
-----------------------------------------------------------




src/slave/containerizer/mesos/provisioner/provisioner.hpp (lines 149 - 154)
<https://reviews.apache.org/r/50871/#comment233934>

    I'd suggest we call this `defaultBackend`. I would try to avoid passing `backend` to `Store::create` etc. Let's passing `backend` down in `Provisioner::provision` and `Store::get`. This way, we might be able to support per rootfs backend in the future. Thoughts?


- Jie Yu


On Jan. 20, 2017, 10:11 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2017, 10:11 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5931
>     https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp ac9dae8ecbb897b8ff942d11ac70281a63e06831 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/containerizer/mesos/provisioner/store.cpp 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/
-----------------------------------------------------------

(Updated Jan. 20, 2017, 2:11 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
-------

Select a backend smartly. Currently, the logic is really
simple:
1. Use overlayfs backend if it exists.
2. Use aufs backend if the overlayfs does not exists.
3. Use copy backend of both overlayfs and aufs do not exist.
Please note that the bind backend needs to be specified explicitly
through the agent flag '--image_provisioner_backend' since it
requires the sandbox already existed.


Diffs (updated)
-----

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp ac9dae8ecbb897b8ff942d11ac70281a63e06831 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
  src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
  src/slave/containerizer/mesos/provisioner/store.cpp 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
  src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
  src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
  src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
  src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 

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


Testing
-------

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/
-----------------------------------------------------------

(Updated Jan. 19, 2017, 10:45 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Changes
-------

Fixed `#ifdef __linux__`


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


Repository: mesos


Description
-------

Select a backend smartly. Currently, the logic is really
simple:
1. Use overlayfs backend if it exists.
2. Use aufs backend if the overlayfs does not exists.
3. Use copy backend of both overlayfs and aufs do not exist.
Please note that the bind backend needs to be specified explicitly
through the agent flag '--image_provisioner_backend' since it
requires the sandbox already existed.


Diffs (updated)
-----

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp ac9dae8ecbb897b8ff942d11ac70281a63e06831 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
  src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
  src/slave/containerizer/mesos/provisioner/store.cpp 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
  src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
  src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
  src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
  src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 

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


Testing
-------

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song


Re: Review Request 50871: Supported auto backend in Unified Containerizer.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50871/
-----------------------------------------------------------

(Updated Jan. 19, 2017, 4:08 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Changes
-------

Fixed the backing filesystem issue.


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


Repository: mesos


Description
-------

Select a backend smartly. Currently, the logic is really
simple:
1. Use overlayfs backend if it exists.
2. Use aufs backend if the overlayfs does not exists.
3. Use copy backend of both overlayfs and aufs do not exist.
Please note that the bind backend needs to be specified explicitly
through the agent flag '--image_provisioner_backend' since it
requires the sandbox already existed.


Diffs (updated)
-----

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp e63ae419e24212b887edddeb5cae114cd39b39c8 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp abb8e7e48422896f207a475661ced0530fc75e68 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp ac9dae8ecbb897b8ff942d11ac70281a63e06831 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp bbd6005317bed3fff3d86e2527ca3ead839d49e3 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp cecf34a23329a64fdbce7de4b83827a30975e9a4 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 1c2b149823e83dc5a3feb0af599d651d1dc05682 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 0a48617d6f9ade928993e1d5205de6486ef657c7 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
  src/slave/containerizer/mesos/provisioner/store.hpp a312ad953b406aa75506051593dcc1c27cdc93af 
  src/slave/containerizer/mesos/provisioner/store.cpp 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
  src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
  src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
  src/tests/containerizer/provisioner_appc_tests.cpp 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
  src/tests/containerizer/provisioner_docker_tests.cpp d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 

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


Testing
-------

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song