> On Sept. 21, 2016, 6:26 p.m., Jie Yu wrote: > > src/tests/containerizer/provisioner_backend_tests.cpp, lines 116-131 > > <https://reviews.apache.org/r/51124/diff/4/?file=1505767#file1505767line116> > > > > I'd suggest we create a separate test. Also, i suggest we construct a > > lot of layers (using a for loop), and make sure the test fail without > > applying this patch and succeed after applying this patch. This better > > captures the regression. > > Zhitao Li wrote: > +1 for the for loop based test w/ many layers. > For the comment of "create a separate test", do you mean move these link > related comparisons to that test? > > Jie Yu wrote: > well, i don't think we need to test the links in the unit tests. It's an > impl. detail. We just need to test if we can handle many layers, if without > the link solution will definitely fail.
I get your idea now. I personally prefer testing certain impl. details as long as the test is still robust, unflaky and it does not slow test down, because it helps people to read and understand the code, and also makes sure some behaviors are not broken w/o noticed. In this case, there is nothing checking that symlinks are cleaned up to the best effort if we move these tests. Still, if you think strong on this, I'm happy to remove these assertions for links. The test for many layers part is added. - Zhitao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51124/#review149863 ----------------------------------------------------------- On Sept. 22, 2016, 8:52 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51124/ > ----------------------------------------------------------- > > (Updated Sept. 22, 2016, 8:52 p.m.) > > > Review request for mesos, Xiaojian Huang, Gilbert Song, and Jie Yu. > > > Bugs: MESOS-6000 > https://issues.apache.org/jira/browse/MESOS-6000 > > > Repository: mesos > > > Description > ------- > > Support more layers through symlink for overlay backend. > > > Diffs > ----- > > src/slave/containerizer/mesos/provisioner/backend.hpp > 93819c887f2f801f06aae7020084b56cc81ce818 > src/slave/containerizer/mesos/provisioner/backends/aufs.hpp > a5ac13bc17b8f291472b622ebfdd0cd1dd072273 > src/slave/containerizer/mesos/provisioner/backends/aufs.cpp > 4c5cdb660dea205aea29d217ba80d845d1108fdf > src/slave/containerizer/mesos/provisioner/backends/bind.hpp > 19d44c6a9a2cd5f8de79f33c76e9d79900ab003f > src/slave/containerizer/mesos/provisioner/backends/bind.cpp > 62ccaec1a5cfb466e929f4db7bf7e5376f2a0c2d > src/slave/containerizer/mesos/provisioner/backends/copy.hpp > 5bd6a5279d8dd6d088afcd58e4839bf6087ccd1e > src/slave/containerizer/mesos/provisioner/backends/copy.cpp > e57bb3d2fb4e67e9caae416824a6a748db1460a1 > src/slave/containerizer/mesos/provisioner/backends/overlay.hpp > 387f28a331813c75a509b4a31dbbdc764080b8c1 > src/slave/containerizer/mesos/provisioner/backends/overlay.cpp > e20cd48463a78ecd8a652a4d5ac923dc02cca4d9 > src/slave/containerizer/mesos/provisioner/provisioner.cpp > f37c45ccfa572876dfbba6a0797c223896db5a7f > src/tests/containerizer/provisioner_backend_tests.cpp > 7c9a7cd5c733f74e10316fc1234115e6820cc2cb > > Diff: https://reviews.apache.org/r/51124/diff/ > > > Testing > ------- > > 1. Make sure `OverlayBackendTest.*` passes; > 2. Use mesos-execute to provision a container using overlay backend. Observed > following log lines: > > ``` > I0816 01:04:27.823420 46584 overlay.cpp:167] Created tempLink at > '/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/links' > to '/tmp/NcmRZt' > I0816 01:04:27.824834 46584 overlay.cpp:191] Provisioning image rootfs with > overlayfs: > 'lowerdir=/tmp/NcmRZt/42:/tmp/NcmRZt/41:/tmp/NcmRZt/40:/tmp/NcmRZt/39:/tmp/NcmRZt/38:/tmp/NcmRZt/37:/tmp/NcmRZt/36:/tmp/NcmRZt/35:/tmp/NcmRZt/34:/tmp/NcmRZt/33:/tmp/NcmRZt/32:/tmp/NcmRZt/31:/tmp/NcmRZt/30:/tmp/NcmRZt/29:/tmp/NcmRZt/28:/tmp/NcmRZt/27:/tmp/NcmRZt/26:/tmp/NcmRZt/25:/tmp/NcmRZt/24:/tmp/NcmRZt/23:/tmp/NcmRZt/22:/tmp/NcmRZt/21:/tmp/NcmRZt/20:/tmp/NcmRZt/19:/tmp/NcmRZt/18:/tmp/NcmRZt/17:/tmp/NcmRZt/16:/tmp/NcmRZt/15:/tmp/NcmRZt/14:/tmp/NcmRZt/13:/tmp/NcmRZt/12:/tmp/NcmRZt/11:/tmp/NcmRZt/10:/tmp/NcmRZt/9:/tmp/NcmRZt/8:/tmp/NcmRZt/7:/tmp/NcmRZt/6:/tmp/NcmRZt/5:/tmp/NcmRZt/4:/tmp/NcmRZt/3:/tmp/NcmRZt/2:/tmp/NcmRZt/1:/tmp/NcmRZt/0,upperdir=/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/scratch/25d9b6e4-fd0e-401e-bb9e-754dbaec4f55/upperdir,workdir=/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/scratch/2 5d9b6e4-fd0e-401e-bb9e-754dbaec4f55/workdir' > ... > (after executor exited) > I0816 01:04:34.859851 46584 overlay.cpp:281] Removed temporary directory > '/tmp/NcmRZt' pointed by > '/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/links' > ``` > > > Thanks, > > Zhitao Li > >
