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