Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu 


> On Sept. 16, 2015, 9:39 p.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 290-295
> > 
> >
> > I was using this to test these paths::XYZ methods: Code being tested is 
> > using these methods and the test itself is using literals to reconsturct 
> > the path.

DIscussed offline. Path tests should be in a different test. This test is to 
test provisioning with APPC images.


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jiang Yan Xu 

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

Ship it!



src/slave/containerizer/provisioner/appc/store.cpp (line 49)


Yup this is a better approach!



src/slave/containerizer/provisioner/provisioner.cpp (line 324)


This is used as an alias? `const &` so it's more idiomatic?



src/slave/flags.cpp (lines 62 - 65)


This is the first flag IIRC that we require that capital letters be used. 
It's no big deal but my original code made it OK to use lower case "appc" is 
thoughtful and less error-prone for operators at little cost for code 
complexity.



src/slave/flags.cpp (line 67)


Assuming this is going to be changed to --image_provisioner_backend.



src/tests/containerizer/provisioner_appc_tests.cpp (lines 284 - 289)


I was using this to test these paths::XYZ methods: Code being tested is 
using these methods and the test itself is using literals to reconsturct the 
path.


- Jiang Yan Xu


On Sept. 15, 2015, 6:28 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 15, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Timothy Chen 


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.
> 
> Vinod Kone wrote:
> +1 for changing backend to provisioner and provisioner to provider. my 
> original inspiration for the suggestion was vagrant.
> 
> https://docs.vagrantup.com/v2/provisioning/index.html
> https://docs.vagrantup.com/v2/providers/index.html
> 
> Jie Yu wrote:
> I think `Store` could be renamed to `Provider` because that's the one 
> that 'provides' image layers.
> 
> I would still keeps the frontend being called `Provisioner` because it's 
> the one that take layers from the `Provider` and constructs (provisions) a 
> root filesystem.
> 
> Jie Yu wrote:
> OK, I don't think we should spend too much time on this naming. How about 
> just keep the class name unchanged and use `--image_provisioner_backend` 
> instead:
> 
> ```
> --image_providers=APPC,DOCKER
> --image_provisioner_backend=bind
> ```
> 
> Jie Yu wrote:
> We pretty much need to use 'auto' for --image_provisioner_backend in the 
> future. If only one layer is returned, user 'bind' backend. Otherwise, use 
> overlayfs. If overlayfs is not available, use copy.
> 
> Jiang Yan Xu wrote:
> +1 for the above flags for now. It's probably too large of an undertaking 
> to be justified.

I think we need to keep the flag even when auto is available, as it's obvious 
from previous conversations with users they need to be able to choose.
But yes we can always rename later.


- Timothy


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jiang Yan Xu 


> On Sept. 16, 2015, 10:54 a.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.
> 
> Vinod Kone wrote:
> +1 for changing backend to provisioner and provisioner to provider. my 
> original inspiration for the suggestion was vagrant.
> 
> https://docs.vagrantup.com/v2/provisioning/index.html
> https://docs.vagrantup.com/v2/providers/index.html
> 
> Jie Yu wrote:
> I think `Store` could be renamed to `Provider` because that's the one 
> that 'provides' image layers.
> 
> I would still keeps the frontend being called `Provisioner` because it's 
> the one that take layers from the `Provider` and constructs (provisions) a 
> root filesystem.
> 
> Jie Yu wrote:
> OK, I don't think we should spend too much time on this naming. How about 
> just keep the class name unchanged and use `--image_provisioner_backend` 
> instead:
> 
> ```
> --image_providers=APPC,DOCKER
> --image_provisioner_backend=bind
> ```
> 
> Jie Yu wrote:
> We pretty much need to use 'auto' for --image_provisioner_backend in the 
> future. If only one layer is returned, user 'bind' backend. Otherwise, use 
> overlayfs. If overlayfs is not available, use copy.

+1 for the above flags for now. It's probably too large of an undertaking to be 
justified.


- Jiang Yan


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


On Sept. 15, 2015, 6:28 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 15, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu 


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.
> 
> Vinod Kone wrote:
> +1 for changing backend to provisioner and provisioner to provider. my 
> original inspiration for the suggestion was vagrant.
> 
> https://docs.vagrantup.com/v2/provisioning/index.html
> https://docs.vagrantup.com/v2/providers/index.html
> 
> Jie Yu wrote:
> I think `Store` could be renamed to `Provider` because that's the one 
> that 'provides' image layers.
> 
> I would still keeps the frontend being called `Provisioner` because it's 
> the one that take layers from the `Provider` and constructs (provisions) a 
> root filesystem.
> 
> Jie Yu wrote:
> OK, I don't think we should spend too much time on this naming. How about 
> just keep the class name unchanged and use `--image_provisioner_backend` 
> instead:
> 
> ```
> --image_providers=APPC,DOCKER
> --image_provisioner_backend=bind
> ```

We pretty much need to use 'auto' for --image_provisioner_backend in the 
future. If only one layer is returned, user 'bind' backend. Otherwise, use 
overlayfs. If overlayfs is not available, use copy.


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/f

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu 


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.
> 
> Vinod Kone wrote:
> +1 for changing backend to provisioner and provisioner to provider. my 
> original inspiration for the suggestion was vagrant.
> 
> https://docs.vagrantup.com/v2/provisioning/index.html
> https://docs.vagrantup.com/v2/providers/index.html
> 
> Jie Yu wrote:
> I think `Store` could be renamed to `Provider` because that's the one 
> that 'provides' image layers.
> 
> I would still keeps the frontend being called `Provisioner` because it's 
> the one that take layers from the `Provider` and constructs (provisions) a 
> root filesystem.

OK, I don't think we should spend too much time on this naming. How about just 
keep the class name unchanged and use `--image_provisioner_backend` instead:

```
--image_providers=APPC,DOCKER
--image_provisioner_backend=bind
```


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu 


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.
> 
> Vinod Kone wrote:
> +1 for changing backend to provisioner and provisioner to provider. my 
> original inspiration for the suggestion was vagrant.
> 
> https://docs.vagrantup.com/v2/provisioning/index.html
> https://docs.vagrantup.com/v2/providers/index.html

I think `Store` could be renamed to `Provider` because that's the one that 
'provides' image layers.

I would still keeps the frontend being called `Provisioner` because it's the 
one that take layers from the `Provider` and constructs (provisions) a root 
filesystem.


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Vinod Kone 


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.
> 
> Timothy Chen wrote:
> this is probably the best patch to change all the names if we want to. 
> Backend in comparison with Provisioner, definitely I feel like Provisioner is 
> more natural.

+1 for changing backend to provisioner and provisioner to provider. my original 
inspiration for the suggestion was vagrant.

https://docs.vagrantup.com/v2/provisioning/index.html
https://docs.vagrantup.com/v2/providers/index.html


- Vinod


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Timothy Chen 


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.
> 
> Jiang Yan Xu wrote:
> About naming, I would prefer leaving the word "backend" in the second 
> flag: `--image_provisioner_backend`, just because the code is written this 
> way. The code has a fontend called `Provisioner` and the backend called 
> `Backend`. Having the flag seemingly refer to the previous when it's in fact 
> referring to the latter is very confusing.
> 
> I actually like the frontend being called "Provider" and the backend 
> called "Provisioner" better but to do the change more thoroughly we should be 
> renaming current `Provisioner` to `Provider` and the `Backend` to 
> `Provisioner`, which is a larger undertaking.

this is probably the best patch to change all the names if we want to. Backend 
in comparison with Provisioner, definitely I feel like Provisioner is more 
natural.


- Timothy


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jiang Yan Xu 


> On Sept. 16, 2015, 10:54 a.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`
> 
> Timothy Chen wrote:
> sounds good to me.

About naming, I would prefer leaving the word "backend" in the second flag: 
`--image_provisioner_backend`, just because the code is written this way. The 
code has a fontend called `Provisioner` and the backend called `Backend`. 
Having the flag seemingly refer to the previous when it's in fact referring to 
the latter is very confusing.

I actually like the frontend being called "Provider" and the backend called 
"Provisioner" better but to do the change more thoroughly we should be renaming 
current `Provisioner` to `Provider` and the `Backend` to `Provisioner`, which 
is a larger undertaking.


- Jiang Yan


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


On Sept. 15, 2015, 6:28 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 15, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Timothy Chen 


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners
> 
> Jie Yu wrote:
> OK, i got confused a little bit. WE only need to specify a single image 
> provisioner.
> 
> `--image_provisioner=bind`

sounds good to me.


- Timothy


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu 


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?
> 
> Jie Yu wrote:
> Sorry, that's typo :( yeah, i mean --image_provisioners

OK, i got confused a little bit. WE only need to specify a single image 
provisioner.

`--image_provisioner=bind`


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu 


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```
> 
> Vinod Kone wrote:
> one singular one plural? can we name the latter --image_provisioners?

Sorry, that's typo :( yeah, i mean --image_provisioners


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Vinod Kone 


> On Sept. 16, 2015, 5:54 p.m., Jie Yu wrote:
> > Chatted with Viond about the flag naming. We prefer the following:
> > 
> > ```
> > --image_providers=APPC,DOCKER
> > --image_provisioner=bind,copy,overlayfs
> > ```

one singular one plural? can we name the latter --image_provisioners?


- Vinod


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-16 Thread Jie Yu 

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


Chatted with Viond about the flag naming. We prefer the following:

```
--image_providers=APPC,DOCKER
--image_provisioner=bind,copy,overlayfs
```

- Jie Yu


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-15 Thread Timothy Chen 

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

Ship it!


Ship It!

- Timothy Chen


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-15 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [38407, 38408, 38417]

All tests passed.

- Mesos ReviewBot


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-15 Thread Jie Yu 

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

(Updated Sept. 16, 2015, 1:28 a.m.)


Review request for mesos, Timothy Chen and Jiang Yan Xu.


Changes
---

Changed image_types to container_image_providers


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


Repository: mesos


Description
---

Unified the implementations of image provisioners. See ticket for motivation.


Diffs (updated)
-

  src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
  src/slave/containerizer/isolators/filesystem/linux.hpp 
6cfe9fa2971d50f545587b57721f75a981f6d5ed 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
dbdbf8722d088ef671b705c0c42191d0f9e05be9 
  src/slave/containerizer/mesos/containerizer.cpp 
1b83a8725b35435531038e37188b4c97189cef03 
  src/slave/containerizer/provisioner/appc/provisioner.hpp 
764b119edf670a44cff4719a2301b1baac88c78a 
  src/slave/containerizer/provisioner/appc/provisioner.cpp 
77f9cbe778785bd93c30eba5dfd7a470d9258661 
  src/slave/containerizer/provisioner/appc/store.hpp 
c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
  src/slave/containerizer/provisioner/appc/store.cpp 
33f692c9b7780bdde96fddd8b07a2f4eb3452471 
  src/slave/containerizer/provisioner/paths.hpp 
5b82591fbe0d1ea48e4b09727424d0547f21adc2 
  src/slave/containerizer/provisioner/paths.cpp 
4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
  src/slave/containerizer/provisioner/provisioner.hpp 
9e0e0b8ef290a31b67bd2415253408e811e1c720 
  src/slave/containerizer/provisioner/provisioner.cpp 
2ac9008243b0dc2ba6051e75c508d183068cebcb 
  src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
  src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
  src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
  src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
  src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
  src/tests/containerizer/provisioner.hpp 
a26b8138d8cc3086058b15a797dd15354a84019f 
  src/tests/containerizer/provisioner_appc_tests.cpp 
8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
  src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-15 Thread Jie Yu 


> On Sept. 16, 2015, 1:14 a.m., Timothy Chen wrote:
> > src/slave/flags.cpp, line 62
> > 
> >
> > image types sounds very generic, but I guess could be ok.
> > How about image_providers or image_formats?
> > Not really necessary to change but just a mere suggestion.

Changed it to container_image_providers


- Jie


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


On Sept. 16, 2015, 1:28 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:28 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38417: Unified the implementations of image provisioners.

2015-09-15 Thread Timothy Chen 

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



src/slave/flags.cpp (line 62)


image types sounds very generic, but I guess could be ok.
How about image_providers or image_formats?
Not really necessary to change but just a mere suggestion.


- Timothy Chen


On Sept. 16, 2015, 1:05 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38417/
> ---
> 
> (Updated Sept. 16, 2015, 1:05 a.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3432
> https://issues.apache.org/jira/browse/MESOS-3432
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the implementations of image provisioners. See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 6cfe9fa2971d50f545587b57721f75a981f6d5ed 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> dbdbf8722d088ef671b705c0c42191d0f9e05be9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1b83a8725b35435531038e37188b4c97189cef03 
>   src/slave/containerizer/provisioner/appc/provisioner.hpp 
> 764b119edf670a44cff4719a2301b1baac88c78a 
>   src/slave/containerizer/provisioner/appc/provisioner.cpp 
> 77f9cbe778785bd93c30eba5dfd7a470d9258661 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> 33f692c9b7780bdde96fddd8b07a2f4eb3452471 
>   src/slave/containerizer/provisioner/paths.hpp 
> 5b82591fbe0d1ea48e4b09727424d0547f21adc2 
>   src/slave/containerizer/provisioner/paths.cpp 
> 4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 9e0e0b8ef290a31b67bd2415253408e811e1c720 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 2ac9008243b0dc2ba6051e75c508d183068cebcb 
>   src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
>   src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
>   src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
>   src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
>   src/tests/containerizer/provisioner.hpp 
> a26b8138d8cc3086058b15a797dd15354a84019f 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
>   src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 
> 
> Diff: https://reviews.apache.org/r/38417/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 38417: Unified the implementations of image provisioners.

2015-09-15 Thread Jie Yu 

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

Review request for mesos, Timothy Chen and Jiang Yan Xu.


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


Repository: mesos


Description
---

Unified the implementations of image provisioners. See ticket for motivation.


Diffs
-

  src/Makefile.am 509256f40ecc1ff5b3a5ce2b380facd153137a18 
  src/slave/containerizer/isolators/filesystem/linux.hpp 
6cfe9fa2971d50f545587b57721f75a981f6d5ed 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
dbdbf8722d088ef671b705c0c42191d0f9e05be9 
  src/slave/containerizer/mesos/containerizer.cpp 
1b83a8725b35435531038e37188b4c97189cef03 
  src/slave/containerizer/provisioner/appc/provisioner.hpp 
764b119edf670a44cff4719a2301b1baac88c78a 
  src/slave/containerizer/provisioner/appc/provisioner.cpp 
77f9cbe778785bd93c30eba5dfd7a470d9258661 
  src/slave/containerizer/provisioner/appc/store.hpp 
c4ce4b90d71791c7fd558221cb2526b1ff245d3b 
  src/slave/containerizer/provisioner/appc/store.cpp 
33f692c9b7780bdde96fddd8b07a2f4eb3452471 
  src/slave/containerizer/provisioner/paths.hpp 
5b82591fbe0d1ea48e4b09727424d0547f21adc2 
  src/slave/containerizer/provisioner/paths.cpp 
4293dd2fe62bd6aee9243717916c86ff9e39d9ce 
  src/slave/containerizer/provisioner/provisioner.hpp 
9e0e0b8ef290a31b67bd2415253408e811e1c720 
  src/slave/containerizer/provisioner/provisioner.cpp 
2ac9008243b0dc2ba6051e75c508d183068cebcb 
  src/slave/containerizer/provisioner/store.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/store.cpp PRE-CREATION 
  src/slave/flags.hpp 799c963806fadee814bac4b9bded679b0ebbbe9c 
  src/slave/flags.cpp ff167ecd6ef224b061b37bbd3dbb4573448f8de2 
  src/slave/paths.hpp 43c65af03a6af8d3d2d50f7c8366d7fbd26cb990 
  src/slave/paths.cpp f104ecdbbb78093ccb968c0a01ea0924bfa391c5 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
ffa371fab69ce5eebe3b02afc2a1724a0f52110f 
  src/tests/containerizer/provisioner.hpp 
a26b8138d8cc3086058b15a797dd15354a84019f 
  src/tests/containerizer/provisioner_appc_tests.cpp 
8fee7ace4d8207796a5d3fb6d52fc25d002b783d 
  src/tests/paths_tests.cpp 1ccc5c70b06f44f54d843d6ed92e9105eda40a7b 

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


Testing
---

sudo make check


Thanks,

Jie Yu