You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2015/09/16 03:06:02 UTC
Review Request 38417: Unified the implementations of image
provisioners.
-----------------------------------------------------------
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
Re: Review Request 38417: Unified the implementations of image
provisioners.
Posted by Jie Yu <yu...@gmail.com>.
> On Sept. 16, 2015, 1:14 a.m., Timothy Chen wrote:
> > src/slave/flags.cpp, line 62
> > <https://reviews.apache.org/r/38417/diff/1/?file=1074744#file1074744line62>
> >
> > 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.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38417/#review99142
-----------------------------------------------------------
src/slave/flags.cpp (line 62)
<https://reviews.apache.org/r/38417/#comment156027>
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
>
>
Re: Review Request 38417: Unified the implementations of image
provisioners.
Posted by Timothy Chen <tn...@apache.org>.
> 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.
Posted by Timothy Chen <tn...@apache.org>.
> 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/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.
Posted by Jie Yu <yu...@gmail.com>.
> 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/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.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> 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.
Posted by Vinod Kone <vi...@gmail.com>.
> 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.
Posted by Timothy Chen <tn...@apache.org>.
> 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.
Posted by Jie Yu <yu...@gmail.com>.
> 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.
Posted by Vinod Kone <vi...@gmail.com>.
> 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.
Posted by Jie Yu <yu...@gmail.com>.
> 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.
Posted by Jie Yu <yu...@gmail.com>.
> 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.
Posted by Jie Yu <yu...@gmail.com>.
> 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
> 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.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> 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 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.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
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.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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.
Posted by Jie Yu <yu...@gmail.com>.
> On Sept. 16, 2015, 9:39 p.m., Jiang Yan Xu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 290-295
> > <https://reviews.apache.org/r/38417/diff/2/?file=1074771#file1074771line290>
> >
> > 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.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/38417/#comment156191>
Yup this is a better approach!
src/slave/containerizer/provisioner/provisioner.cpp (line 324)
<https://reviews.apache.org/r/38417/#comment156185>
This is used as an alias? `const &` so it's more idiomatic?
src/slave/flags.cpp (lines 62 - 65)
<https://reviews.apache.org/r/38417/#comment156214>
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)
<https://reviews.apache.org/r/38417/#comment156216>
Assuming this is going to be changed to --image_provisioner_backend.
src/tests/containerizer/provisioner_appc_tests.cpp (lines 284 - 289)
<https://reviews.apache.org/r/38417/#comment156193>
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.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
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