> On Sept. 10, 2015, 3:04 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/provisioners/docker/local_store.cpp, lines 158-160 > > <https://reviews.apache.org/r/38137/diff/5/?file=1066159#file1066159line158> > > > > Can we actually use fetcher here? > > Timothy Chen wrote: > Not sure what you're referring to? We use the fetcher to fetch local > images.
No. The fetcher pointer passed into the store is simply ignored in the code. > On Sept. 10, 2015, 3:04 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/provisioners/docker/metadata_manager.cpp, lines > > 78-81 > > <https://reviews.apache.org/r/38137/diff/5/?file=1066161#file1066161line78> > > > > How is the "latest" tag handled in this case? If I have downloaded an > > ubuntu:latest and 1 month later I want another ubuntu:latest, the > > store/slave is always using to use the old ubuntu:latest? > > Timothy Chen wrote: > Yep, that's how docker client works for now. We will provide the option > to pull again, or always pull latest which will attempt to update the > metadata. > > Jiang Yan Xu wrote: > I see. But this kind of sucks... > > I am all for stick Sorry for the partial comment, I didn't finish it. I meant to say that it's unfortunate because as a user I don't know when to do a force pull unless I always force a pull. It's probably less of an issue if I am a casual user doing this on a single host. (I can inspect the local cache with a sequence of docker commands.) If I have O(10000) hosts and serve O(100) users it's going to be messy. I wouldn't know if `ubuntu-patched-for-my-company:vivid` cached on any machine is really want I think it is. So to reiterate my point, I think an ID is necessary (and Docker engine supports it to enable preciseness). If an ID is used, here we need to rethink about the bookkeeping data structure and get() semantics. FWICT Docker does two lookups: first look it up by using repo:tag against the tag store and if not found, go look for it in the graph data structure which maintains an index of IDs. > On Sept. 10, 2015, 3:04 a.m., Jiang Yan Xu wrote: > > src/slave/containerizer/provisioners/docker.hpp, line 66 > > <https://reviews.apache.org/r/38137/diff/5/?file=1066156#file1066156line66> > > > > Should registry be part of the name? > > > > I know it has significance in discovery but it doesn't appear to be > > part of the name. > > > > I would suggest we model our data structure close to the spec so when > > there's confusion it's easy to refer to something that's definitive. > > > > I have another comment below about the "registry" in "name". > > Timothy Chen wrote: > I think registry should be part of the name since it's what uniquely > identifies a "tag" of a image. > A name of a docker image, is merely a tag instead of a immutable id, and > that's very different than how Appc works. > Only layers have uniquely identifable ids. Fine about registry being part of the repo name then. Is the ID of the image the ID of its leaf layer then? Image ID is definitely used throughout Docker's documentation, code, the "repositories" files and is what uniquely identify an image. Docker code (here and in various other places)[https://github.com/docker/docker/blob/9d0954a83d6c7b52287a62f8ab7ad42d544322a0/pkg/parsers/parsers.go#L104] kind of treats `sha256:<sha>` as a special case of tag, which I don't think is a good practice. It also treats registry as **part of** the repo name. So if we are separating the registry out as a field, we should separate out the ID as a separate field. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38137/#review98216 ----------------------------------------------------------- On Sept. 11, 2015, 12:42 a.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38137/ > ----------------------------------------------------------- > > (Updated Sept. 11, 2015, 12:42 a.m.) > > > Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang > Yan Xu. > > > Repository: mesos > > > Description > ------- > > Joining all the commits around provisioner and local store into one review so > it's easier to review, as patches > are changing code on top of each other. > > All the commits are going to committed together. > > > Diffs > ----- > > src/Makefile.am 8963cea > src/slave/containerizer/provisioner.hpp 9e0e0b8 > src/slave/containerizer/provisioner.cpp 2ac9008 > src/slave/containerizer/provisioners/docker/local_store.hpp PRE-CREATION > src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION > src/slave/containerizer/provisioners/docker/message.hpp PRE-CREATION > src/slave/containerizer/provisioners/docker/message.proto PRE-CREATION > src/slave/containerizer/provisioners/docker/metadata_manager.hpp > PRE-CREATION > src/slave/containerizer/provisioners/docker/metadata_manager.cpp > PRE-CREATION > src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION > src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION > src/slave/containerizer/provisioners/docker/provisioner.hpp PRE-CREATION > src/slave/containerizer/provisioners/docker/provisioner.cpp PRE-CREATION > src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION > src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION > src/slave/flags.hpp 799c963 > src/slave/flags.cpp b676bac > src/tests/containerizer/docker_provisioner_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/38137/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Timothy Chen > >
