> On Jan. 25, 2017, 7:45 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/provisioner/docker/store.cpp, lines 276-279 > > <https://reviews.apache.org/r/54215/diff/6/?file=1613732#file1613732line276> > > > > Can you also validate the ordering with docker code? For instance, say > > the layer paths are: > > > > [a, b, c, d, a, e, f] > > > > will the end result be: > > > > [b, c, d, a, e, f] > > > > or > > > > [a, b, c, d, e, f] > > > > Please note that the ordering we have here in our code might not be the > > same as that in docker (i.e., whether base layer is to the left most or to > > the right most?) > > Gilbert Song wrote: > Did some research on docker. Basically, as I mentioned above, the layer > ids are read from the disk, so they are `unique` and should be ordered > `alphabetically`. Then, docker loads each layer depending on the layer > `child-parent` relationship. It is similar to what we currently do in local > puller and it would not have the case of duplicate layer paths since it is > like a linked list from bottom to the top. The order should be: > > [Grandparent, parent, child] > > In our registry puller, the vector of layer ids are collected differently > (not relying on finding next parent from the base layer). We just queue all > the layer ids using the returned manifest, and fortunately the > `v1Compatibility::id` from the manifest are sorted before hand depending on > the `parent-child` relationship. > > Only some self-built images may contain duplicate layers (e.g., [a, a, b, > b, b, c] from the manifest). We should still fix it for them though. It seems > to me that our current fix in docker store is just a workaround. I would > propose to fix it in registry puller `pull()`: > > 1. Depending on the leaf layer id, collect all layer ids using > `parent-child` relationship. > 2. Filter the vector of layer ids to make sure ids are unique in registry > puller. > > I prefer (2). Thoughts?
Tested the same image `mesosphere/inky` using both pullers on master branch. As expected, `local puller` passed and `registry puller` failed. - Gilbert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54215/#review162963 ----------------------------------------------------------- On Jan. 24, 2017, 12:35 p.m., Gilbert Song wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54215/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2017, 12:35 p.m.) > > > Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, Qian > Zhang, and Zhitao Li. > > > Bugs: MESOS-6654 > https://issues.apache.org/jira/browse/MESOS-6654 > > > Repository: mesos > > > Description > ------- > > This issue is exposed by pulling the 'mesosphere/inky' docker > image using registry puller. Due to the duplicate layer id > from the manifest, there are duplicate layer pathes passed > to the backend. The aufs backend cannot handle this case and > returns 'invalid arguments' error. Ideally, we should make > sure that layer paths that are passed to the backend are > unique. > > > Diffs > ----- > > src/slave/containerizer/mesos/provisioner/docker/store.cpp > 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 > > Diff: https://reviews.apache.org/r/54215/diff/ > > > Testing > ------- > > make check > > Tested by the unit test > `ROOT_CURL_INTERNET_DockerDefaultEntryptRegistryPuller`. > > Manually tested using the `mesosphere/inky` image, which contains duplicate > layer ids. > > > Thanks, > > Gilbert Song > >
