You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2017/01/03 14:19:26 UTC

Re: Review Request 54639: Implemented the 'create()' method of OCI store.

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

(Updated Jan. 3, 2017, 10:19 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

Implemented the 'create()' method of OCI store.


Diffs (updated)
-----

  src/CMakeLists.txt c8d4260c03d8cdee1951a50d293e9fdabcd2cf84 
  src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 
  src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 

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


Testing
-------


Thanks,

Qian Zhang


Re: Review Request 54639: Implemented the 'create()' method of OCI store.

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



Patch looks great!

Reviews applied: [52349, 55139, 55140, 52379, 54638, 52382, 54639]

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

- Mesos ReviewBot


On Jan. 3, 2017, 2:19 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54639/
> -----------------------------------------------------------
> 
> (Updated Jan. 3, 2017, 2:19 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
>     https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the 'create()' method of OCI store.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt c8d4260c03d8cdee1951a50d293e9fdabcd2cf84 
>   src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 
>   src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54639/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 54639: Implemented the 'create()' method of OCI store.

Posted by Jie Yu <yu...@gmail.com>.

> On Feb. 8, 2017, 11:36 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/oci/paths.hpp, lines 36-39
> > <https://reviews.apache.org/r/54639/diff/3/?file=1621023#file1621023line36>
> >
> >     Can you elaborate a bit more the layout here?
> >     
> >     so `layer_id` is the digest? What is `config.json`? Does it store decompressed content, or the original content? what about image configs?
> >     
> >     Given this https://github.com/opencontainers/image-spec/blob/master/image-layout.md, my hunch is that we should probably cache the original blob, as well as decompressed content, potentially in separate subdirectories.
> >     
> >     ```
> >     <oci_store_dir>
> >        |--- blobs/  # this is origional CAS blobs.
> >                |--- sha256/
> >                        |--- <digest>
> >                        |--- <digest>
> >                        |--- ...
> >        |--- layers/ # this is decompressed filesystem change sets
> >                |--- <id? digest?>
> >                |--- ...
> >     ```
> >     
> >     Looks like we probably do not need `configs` or `manifests` directory for now because the only media type is json, and can be directly read from `blobs/`.
> >     
> >     Another thing to consider is the conversion to overlayfs opaque file from whiteout files as you did before. So for `layers` directory, we probably need to have a `backend` in the path (e.g., `layers/overlayfs/...`).

Also, worth taking a look at how Docker (or other OCI system) organize its cache.


- Jie


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


On Feb. 1, 2017, 1:17 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54639/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
>     https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the 'create()' method of OCI store.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 09ef1aee680c6b91476092dbf61d4ca3a8d256bb 
>   src/Makefile.am 6c9be54fe0ce3faaa26e2f090773d1b77bb6e430 
>   src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54639/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 54639: Implemented the 'create()' method of OCI store.

Posted by Qian Zhang <zh...@gmail.com>.

> On Feb. 9, 2017, 7:36 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/oci/paths.hpp, lines 36-39
> > <https://reviews.apache.org/r/54639/diff/3/?file=1621023#file1621023line36>
> >
> >     Can you elaborate a bit more the layout here?
> >     
> >     so `layer_id` is the digest? What is `config.json`? Does it store decompressed content, or the original content? what about image configs?
> >     
> >     Given this https://github.com/opencontainers/image-spec/blob/master/image-layout.md, my hunch is that we should probably cache the original blob, as well as decompressed content, potentially in separate subdirectories.
> >     
> >     ```
> >     <oci_store_dir>
> >        |--- blobs/  # this is origional CAS blobs.
> >                |--- sha256/
> >                        |--- <digest>
> >                        |--- <digest>
> >                        |--- ...
> >        |--- layers/ # this is decompressed filesystem change sets
> >                |--- <id? digest?>
> >                |--- ...
> >     ```
> >     
> >     Looks like we probably do not need `configs` or `manifests` directory for now because the only media type is json, and can be directly read from `blobs/`.
> >     
> >     Another thing to consider is the conversion to overlayfs opaque file from whiteout files as you did before. So for `layers` directory, we probably need to have a `backend` in the path (e.g., `layers/overlayfs/...`).
> 
> Jie Yu wrote:
>     Also, worth taking a look at how Docker (or other OCI system) organize its cache.

Please take a look at https://reviews.apache.org/r/56147/diff/1#4, I have changed it to:
```
     |--layers
        |--<layer_id>
                |-- rootfs
        |--<layer_id>
                |-- rootfs
        |--<config.json>
```
Here both `<layer_id>` and `<config.json>` are digest. `<layer_id>/rootfs/` stores decompressed content(filesystem) extracted by `command::untar()` from the layer tar ball. `<config.json>` stores the image configuration.

Can you please let me know why we need to cache the original blobs? In my current implementation, the orignal blobs will only be fetched by fetcher into a temp directory under `staging` directory, `layers` directory will only store the decompressed content, and that temp directory under `staging` directory will be removed after fetching image is done. I do not think we need the original blobs once the fetching image is done and we have cached the image in the store.

And actually under `layers/<layer_id>`, there can be two sub-directories: `rootfs` (for aufs/bind/copy) and `rootfs.overlay` (for overlay), this is same with what I did for for Docker store. Do you think we need to change it to `layers/non-overlay/<layer_id>` and `layers/overlay/<layer_id>` for OCI store?


- Qian


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


On Feb. 1, 2017, 9:17 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54639/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2017, 9:17 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
>     https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the 'create()' method of OCI store.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 09ef1aee680c6b91476092dbf61d4ca3a8d256bb 
>   src/Makefile.am 6c9be54fe0ce3faaa26e2f090773d1b77bb6e430 
>   src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54639/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 54639: Implemented the 'create()' method of OCI store.

Posted by Qian Zhang <zh...@gmail.com>.

> On Feb. 9, 2017, 7:36 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/oci/paths.hpp, lines 36-39
> > <https://reviews.apache.org/r/54639/diff/3/?file=1621023#file1621023line36>
> >
> >     Can you elaborate a bit more the layout here?
> >     
> >     so `layer_id` is the digest? What is `config.json`? Does it store decompressed content, or the original content? what about image configs?
> >     
> >     Given this https://github.com/opencontainers/image-spec/blob/master/image-layout.md, my hunch is that we should probably cache the original blob, as well as decompressed content, potentially in separate subdirectories.
> >     
> >     ```
> >     <oci_store_dir>
> >        |--- blobs/  # this is origional CAS blobs.
> >                |--- sha256/
> >                        |--- <digest>
> >                        |--- <digest>
> >                        |--- ...
> >        |--- layers/ # this is decompressed filesystem change sets
> >                |--- <id? digest?>
> >                |--- ...
> >     ```
> >     
> >     Looks like we probably do not need `configs` or `manifests` directory for now because the only media type is json, and can be directly read from `blobs/`.
> >     
> >     Another thing to consider is the conversion to overlayfs opaque file from whiteout files as you did before. So for `layers` directory, we probably need to have a `backend` in the path (e.g., `layers/overlayfs/...`).
> 
> Jie Yu wrote:
>     Also, worth taking a look at how Docker (or other OCI system) organize its cache.
> 
> Qian Zhang wrote:
>     Please take a look at https://reviews.apache.org/r/56147/diff/1#4, I have changed it to:
>     ```
>          |--layers
>             |--<layer_id>
>                     |-- rootfs
>             |--<layer_id>
>                     |-- rootfs
>             |--<config.json>
>     ```
>     Here both `<layer_id>` and `<config.json>` are digest. `<layer_id>/rootfs/` stores decompressed content(filesystem) extracted by `command::untar()` from the layer tar ball. `<config.json>` stores the image configuration.
>     
>     Can you please let me know why we need to cache the original blobs? In my current implementation, the orignal blobs will only be fetched by fetcher into a temp directory under `staging` directory, `layers` directory will only store the decompressed content, and that temp directory under `staging` directory will be removed after fetching image is done. I do not think we need the original blobs once the fetching image is done and we have cached the image in the store.
>     
>     And actually under `layers/<layer_id>`, there can be two sub-directories: `rootfs` (for aufs/bind/copy) and `rootfs.overlay` (for overlay), this is same with what I did for for Docker store. Do you think we need to change it to `layers/non-overlay/<layer_id>` and `layers/overlay/<layer_id>` for OCI store?

And I checked Docker today, it does not cache blobs as well. When pulling an image, Docker will download the blobs in a temp directory (`/var/lib/docker/tmp/`), and once the pulling is done, the blobs in that temp directory will be removed. And Docker will store the decompressed content of each image layer in `/var/lib/docker/aufs/diff/<layerID>`.


- Qian


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


On Feb. 1, 2017, 9:17 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54639/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2017, 9:17 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
>     https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the 'create()' method of OCI store.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 09ef1aee680c6b91476092dbf61d4ca3a8d256bb 
>   src/Makefile.am 6c9be54fe0ce3faaa26e2f090773d1b77bb6e430 
>   src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54639/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 54639: Implemented the 'create()' method of OCI store.

Posted by Jie Yu <yu...@gmail.com>.

> On Feb. 8, 2017, 11:36 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/oci/paths.hpp, lines 36-39
> > <https://reviews.apache.org/r/54639/diff/3/?file=1621023#file1621023line36>
> >
> >     Can you elaborate a bit more the layout here?
> >     
> >     so `layer_id` is the digest? What is `config.json`? Does it store decompressed content, or the original content? what about image configs?
> >     
> >     Given this https://github.com/opencontainers/image-spec/blob/master/image-layout.md, my hunch is that we should probably cache the original blob, as well as decompressed content, potentially in separate subdirectories.
> >     
> >     ```
> >     <oci_store_dir>
> >        |--- blobs/  # this is origional CAS blobs.
> >                |--- sha256/
> >                        |--- <digest>
> >                        |--- <digest>
> >                        |--- ...
> >        |--- layers/ # this is decompressed filesystem change sets
> >                |--- <id? digest?>
> >                |--- ...
> >     ```
> >     
> >     Looks like we probably do not need `configs` or `manifests` directory for now because the only media type is json, and can be directly read from `blobs/`.
> >     
> >     Another thing to consider is the conversion to overlayfs opaque file from whiteout files as you did before. So for `layers` directory, we probably need to have a `backend` in the path (e.g., `layers/overlayfs/...`).
> 
> Jie Yu wrote:
>     Also, worth taking a look at how Docker (or other OCI system) organize its cache.
> 
> Qian Zhang wrote:
>     Please take a look at https://reviews.apache.org/r/56147/diff/1#4, I have changed it to:
>     ```
>          |--layers
>             |--<layer_id>
>                     |-- rootfs
>             |--<layer_id>
>                     |-- rootfs
>             |--<config.json>
>     ```
>     Here both `<layer_id>` and `<config.json>` are digest. `<layer_id>/rootfs/` stores decompressed content(filesystem) extracted by `command::untar()` from the layer tar ball. `<config.json>` stores the image configuration.
>     
>     Can you please let me know why we need to cache the original blobs? In my current implementation, the orignal blobs will only be fetched by fetcher into a temp directory under `staging` directory, `layers` directory will only store the decompressed content, and that temp directory under `staging` directory will be removed after fetching image is done. I do not think we need the original blobs once the fetching image is done and we have cached the image in the store.
>     
>     And actually under `layers/<layer_id>`, there can be two sub-directories: `rootfs` (for aufs/bind/copy) and `rootfs.overlay` (for overlay), this is same with what I did for for Docker store. Do you think we need to change it to `layers/non-overlay/<layer_id>` and `layers/overlay/<layer_id>` for OCI store?
> 
> Qian Zhang wrote:
>     And I checked Docker today, it does not cache blobs as well. When pulling an image, Docker will download the blobs in a temp directory (`/var/lib/docker/tmp/`), and once the pulling is done, the blobs in that temp directory will be removed. And Docker will store the decompressed content of each image layer in `/var/lib/docker/aufs/diff/<layerID>`.

OK, it's a little wierd that the parent directory name is `layers`, but it contains image config (I presume `config.json` is image config?)

The reason I suggested caching original blobs is for content addressibility. Now, I think you're right. Caching the compressed layer probably does not make sense.

Can we separate layers from other media type like image config or image manifest? For instance:
```
|--- layers
|--- manifests
|--- configs
```

For layers, the directory image will be the digest of its compressed artifact? That means it's possible that we have the same layer, but multiple directory under `layers` (depending on the compression type). But I guess that's fine. Not sure how docker handles this.


- Jie


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


On Feb. 1, 2017, 1:17 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54639/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
>     https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the 'create()' method of OCI store.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 09ef1aee680c6b91476092dbf61d4ca3a8d256bb 
>   src/Makefile.am 6c9be54fe0ce3faaa26e2f090773d1b77bb6e430 
>   src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54639/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 54639: Implemented the 'create()' method of OCI store.

Posted by Jie Yu <yu...@gmail.com>.

> On Feb. 8, 2017, 11:36 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/oci/paths.hpp, lines 36-39
> > <https://reviews.apache.org/r/54639/diff/3/?file=1621023#file1621023line36>
> >
> >     Can you elaborate a bit more the layout here?
> >     
> >     so `layer_id` is the digest? What is `config.json`? Does it store decompressed content, or the original content? what about image configs?
> >     
> >     Given this https://github.com/opencontainers/image-spec/blob/master/image-layout.md, my hunch is that we should probably cache the original blob, as well as decompressed content, potentially in separate subdirectories.
> >     
> >     ```
> >     <oci_store_dir>
> >        |--- blobs/  # this is origional CAS blobs.
> >                |--- sha256/
> >                        |--- <digest>
> >                        |--- <digest>
> >                        |--- ...
> >        |--- layers/ # this is decompressed filesystem change sets
> >                |--- <id? digest?>
> >                |--- ...
> >     ```
> >     
> >     Looks like we probably do not need `configs` or `manifests` directory for now because the only media type is json, and can be directly read from `blobs/`.
> >     
> >     Another thing to consider is the conversion to overlayfs opaque file from whiteout files as you did before. So for `layers` directory, we probably need to have a `backend` in the path (e.g., `layers/overlayfs/...`).
> 
> Jie Yu wrote:
>     Also, worth taking a look at how Docker (or other OCI system) organize its cache.
> 
> Qian Zhang wrote:
>     Please take a look at https://reviews.apache.org/r/56147/diff/1#4, I have changed it to:
>     ```
>          |--layers
>             |--<layer_id>
>                     |-- rootfs
>             |--<layer_id>
>                     |-- rootfs
>             |--<config.json>
>     ```
>     Here both `<layer_id>` and `<config.json>` are digest. `<layer_id>/rootfs/` stores decompressed content(filesystem) extracted by `command::untar()` from the layer tar ball. `<config.json>` stores the image configuration.
>     
>     Can you please let me know why we need to cache the original blobs? In my current implementation, the orignal blobs will only be fetched by fetcher into a temp directory under `staging` directory, `layers` directory will only store the decompressed content, and that temp directory under `staging` directory will be removed after fetching image is done. I do not think we need the original blobs once the fetching image is done and we have cached the image in the store.
>     
>     And actually under `layers/<layer_id>`, there can be two sub-directories: `rootfs` (for aufs/bind/copy) and `rootfs.overlay` (for overlay), this is same with what I did for for Docker store. Do you think we need to change it to `layers/non-overlay/<layer_id>` and `layers/overlay/<layer_id>` for OCI store?
> 
> Qian Zhang wrote:
>     And I checked Docker today, it does not cache blobs as well. When pulling an image, Docker will download the blobs in a temp directory (`/var/lib/docker/tmp/`), and once the pulling is done, the blobs in that temp directory will be removed. And Docker will store the decompressed content of each image layer in `/var/lib/docker/aufs/diff/<layerID>`.
> 
> Jie Yu wrote:
>     OK, it's a little wierd that the parent directory name is `layers`, but it contains image config (I presume `config.json` is image config?)
>     
>     The reason I suggested caching original blobs is for content addressibility. Now, I think you're right. Caching the compressed layer probably does not make sense.
>     
>     Can we separate layers from other media type like image config or image manifest? For instance:
>     ```
>     |--- layers
>     |--- manifests
>     |--- configs
>     ```
>     
>     For layers, the directory image will be the digest of its compressed artifact? That means it's possible that we have the same layer, but multiple directory under `layers` (depending on the compression type). But I guess that's fine. Not sure how docker handles this.

Also, worth taking a look at the layer layout of some snapshot based backend (e.g. brtfs). It might be complete different than the overlay(aufs) based backends. That's the reason I suggested using `layers/overlay` and `layers/default`


- Jie


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


On Feb. 1, 2017, 1:17 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54639/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
>     https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the 'create()' method of OCI store.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 09ef1aee680c6b91476092dbf61d4ca3a8d256bb 
>   src/Makefile.am 6c9be54fe0ce3faaa26e2f090773d1b77bb6e430 
>   src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54639/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 54639: Implemented the 'create()' method of OCI store.

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




src/Makefile.am (line 951)
<https://reviews.apache.org/r/54639/#comment236571>

    Let's align the tailing backslash.



src/Makefile.am (line 1092)
<https://reviews.apache.org/r/54639/#comment236572>

    Ditto.



src/slave/containerizer/mesos/provisioner/oci/paths.hpp (lines 36 - 39)
<https://reviews.apache.org/r/54639/#comment236601>

    Can you elaborate a bit more the layout here?
    
    so `layer_id` is the digest? What is `config.json`? Does it store decompressed content, or the original content? what about image configs?
    
    Given this https://github.com/opencontainers/image-spec/blob/master/image-layout.md, my hunch is that we should probably cache the original blob, as well as decompressed content, potentially in separate subdirectories.
    
    ```
    <oci_store_dir>
       |--- blobs/  # this is origional CAS blobs.
               |--- sha256/
                       |--- <digest>
                       |--- <digest>
                       |--- ...
       |--- layers/ # this is decompressed filesystem change sets
               |--- <id? digest?>
               |--- ...
    ```
    
    Looks like we probably do not need `configs` or `manifests` directory for now because the only media type is json, and can be directly read from `blobs/`.
    
    Another thing to consider is the conversion to overlayfs opaque file from whiteout files as you did before. So for `layers` directory, we probably need to have a `backend` in the path (e.g., `layers/overlayfs/...`).



src/slave/containerizer/mesos/provisioner/oci/store.cpp (lines 62 - 68)
<https://reviews.apache.org/r/54639/#comment236597>

    Joseph has a URI parser chain that parse any URI. We should try to support any uri that fetcher supports.
    https://reviews.apache.org/r/46580/
    https://reviews.apache.org/r/46588/
    
    We should try to land that and use the URI parser here to parse the uri into `URI` struct, and call `fetcher->fetch(URI uri)`


- Jie Yu


On Feb. 1, 2017, 1:17 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54639/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
>     https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the 'create()' method of OCI store.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 09ef1aee680c6b91476092dbf61d4ca3a8d256bb 
>   src/Makefile.am 6c9be54fe0ce3faaa26e2f090773d1b77bb6e430 
>   src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54639/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 54639: Implemented the 'create()' method of OCI store.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54639/
-----------------------------------------------------------

(Updated May 23, 2017, 9:34 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
-------

Created `default` and `overlay` layers directories explicitly.


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


Repository: mesos


Description
-------

Implemented the 'create()' method of OCI store.


Diffs (updated)
-----

  src/CMakeLists.txt ca7d538e2f7191c157e3e2237d56e3d75367f418 
  src/Makefile.am 7e4ce8532ec2c1b4216c2aa8d4262b2091e21c4a 
  src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/54639/diff/5/

Changes: https://reviews.apache.org/r/54639/diff/4-5/


Testing
-------


Thanks,

Qian Zhang


Re: Review Request 54639: Implemented the 'create()' method of OCI store.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54639/
-----------------------------------------------------------

(Updated March 30, 2017, 9:56 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
-------

Changed the filesystem layout of OCI store.


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


Repository: mesos


Description
-------

Implemented the 'create()' method of OCI store.


Diffs (updated)
-----

  src/CMakeLists.txt b67b512cafd90558abf712a872310ab913fa38ae 
  src/Makefile.am 071656ad7354a802e8292140a7181cb70b68fe9e 
  src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/54639/diff/4/

Changes: https://reviews.apache.org/r/54639/diff/3-4/


Testing
-------


Thanks,

Qian Zhang


Re: Review Request 54639: Implemented the 'create()' method of OCI store.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54639/
-----------------------------------------------------------

(Updated Feb. 1, 2017, 9:17 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

Implemented the 'create()' method of OCI store.


Diffs (updated)
-----

  src/CMakeLists.txt 09ef1aee680c6b91476092dbf61d4ca3a8d256bb 
  src/Makefile.am 6c9be54fe0ce3faaa26e2f090773d1b77bb6e430 
  src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 

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


Testing
-------


Thanks,

Qian Zhang