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 38137: Added Docker provisioner and local store
Date Sat, 12 Sep 2015 01:41:19 GMT


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

I see. But this kind of sucks...

I am all for stick


> On Sept. 10, 2015, 3:04 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/provisioners/docker/metadata_manager.hpp, line 55
> > <https://reviews.apache.org/r/38137/diff/5/?file=1066160#file1066160line55>
> >
> >     We can iterate on this but I feel strongly that the metadata here is an inherent
part of the store (it ties to the structure of the local cache) and we don't need another
"actor".
> >     
> >     I understand that this is partially for sharing code between local and remote
stores and depending on the source of the images (local vs. remote store in the curent terminology),
the source and intermediate artifacts are different and it requires different actions to process
them but ultimately they both convert and store things into the same local cache and register
them with the same metadata file. It's then the save logic to provide an locally cached image
to the provisioner. These all suggest that we should have "one store".
> >     
> >     What we need are not two discovery mechanisms but rather two types of "pullers"
(the term used by [docker code|https://github.com/docker/docker/blob/634a848b8e3bdd8aed834559f3b2e0dfc7f5ae3a/graph/pull.go#L28]
as well) that handle two types of sources. ("Local store" can entirely be on HDFS with no
added complexity, i.e. same mesos fetcher)
> 
> Timothy Chen wrote:
>     I see, I think conceptually it probably fits nicer to have two pullers as you mentioned.
>     How about I can post another patch after this to restructure this as I like to get
everything reviewed and merged so it's unblocking some other folks?

Sure. Thanks.


> On Sept. 10, 2015, 3:04 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/provisioners/docker.cpp, lines 104-107
> > <https://reviews.apache.org/r/38137/diff/5/?file=1066157#file1066157line104>
> >
> >     I guess you are modeling this after the `docker pull` syntax here: `[REGISTRY_HOST[:REGISTRY_PORT]/]NAME[:TAG]`
> >     
> >     I found that some docker help pages have conflicting use of repository vs. name
and we should probably refer to the spec.
> >     
> >     I think the following questions need to be answered:
> >     
> >     1) If we use image name as the indentifier, it defines "what the image is".
The registry specifies "where the image comes from". If an identical image is hosted on two
registries, should they have the same name?
> >     
> >     2) ID. `docker pull` supports @sha256 syntax and `repositories` file (which
our docker metadata file is modeled after) maps repo:tag to an image ID. Shouldn't the ID
be part of the name? If ubuntu:latest changes when use releases come out, I don't think repo:tag
uniquely identifies the image.
> 
> Timothy Chen wrote:
>     I think I'm trying to conform to the docker client experience, which is what users
perceive is the standard.
>     Although docker supports sha256 syntax, users still all the time refers to images
as [registry_host:port]/repository:tag. When images are conflicting between two registries,
the user is usually impliciting defining which one to pick based on the registry source. If
user swithces the default registry and we are now not in sync, this is at least identical
to what docker client behaves today and users should expect the same. We will provide comments
in the user guide.
>     
>     The current docker client simpliy matches the full name like we do in the code here,
and caches that tag until a docker pull happens that attempts to get the latest tag.
>     
>     And yes Docker doesn't consistently name things correctly themselves :( I'm going
with the Docker registry API naming for now, which is repository instead of name.

I am all for not diverging from the "standard" docker user experience here but I think 
1) We have to support specifying image by ID.

Docker's tags are imprecise and mutable, as a large organization running production services
we have to have the option to be very precise about what to deploy. If users don't want to
be precise, they don't have to use it.

2) From what I can tell, registry is not persisted in the "repositories" file, so how does
Docker engine know the locally cached image with the same repo:tag matches a request for "myregistry/repo:tag"
or not?

I am actually digging through docker code so feel free to share the link.


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


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