mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 38417: Unified the implementations of image provisioners.
Date Wed, 16 Sep 2015 18:56:06 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message