mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jiang Yan Xu" <...@jxu.me>
Subject Re: Review Request 34140: AppC image store
Date Fri, 17 Jul 2015 18:56:15 GMT

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


Not necessary to be done in this review but we should allow `putting` images that are already
in the staging folder, this will support mechanisms that download stuff out-of-band.
Also we should rely on a `paths.hpp` when doing all the path manipulation.


src/slave/containerizer/provisioners/appc/store.hpp (line 50)
<https://reviews.apache.org/r/34140/#comment145928>

    It's worth noting that the id is computed from the sha512 hash here as this is actually
the only place where the mechanism by which that this id is derived matters. Everywhere else
it should be understood as an opaque string.



src/slave/containerizer/provisioners/appc/store.hpp (lines 80 - 83)
<https://reviews.apache.org/r/34140/#comment145766>

    Who uses this?



src/slave/containerizer/provisioners/appc/store.cpp (line 148)
<https://reviews.apache.org/r/34140/#comment146052>

    Here if the `stage` directory is moved into the store then `rmdir()` returns Error. Of
course we don't check the result but maybe checking its existence before rmdir and log other
rmdir Errors is better?



src/slave/containerizer/provisioners/appc/store.cpp (line 397)
<https://reviews.apache.org/r/34140/#comment145936>

    So at this point we have already downloaded the image, we might as well just go ahead
and overwrite the existing folder. If not, we probably don't want to spend all the resources
to download it in the first place (i.e. directly return in `put()`).
    
    But I think it would make sense to overwrite if we are going to open up some HTTP API
to allow operators to resue some faulty images.



src/slave/containerizer/provisioners/appc/store.cpp (lines 406 - 415)
<https://reviews.apache.org/r/34140/#comment145933>

    Manifest validation is only one type of validation.
    We should also do other validation checks. e.g. whether rootfs exists; whether there are
additional files outside of rootfs, etc.


- Jiang Yan Xu


On July 7, 2015, 12:43 p.m., Ian Downes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34140/
> -----------------------------------------------------------
> 
> (Updated July 7, 2015, 12:43 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Images are fetched into the store (after discovery). Stored images are
> currently kept indefinitely.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/provisioners/appc/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/appc/store.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
> 
> Diff: https://reviews.apache.org/r/34140/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ian Downes
> 
>


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