New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use api for pulling images instead of shelling out #351
Conversation
I have two questions
|
Personally I like using the api, since it provides a structured response, It also provides better future proofing. APIs have deprecation policies, I haven't looked at the PR (on my phone) but can we make certain that the Thanks
|
Actually, with Docker the CLI deprecation policy has been much stronger than the API deprecation policy. With 1.0 in place the API now has a much stronger change policy, but I would say they're equally stable now. |
Ok, I discussed that parsing in #docker-dev and it seems it's not necessary to do that because the 'fromImage' parameter (which is called, a bit misfortune, 'Repository' in go-dockerclient) gets parsed server side for repo and so on. So I've removed the parsing. The docker API now is stable, nobody should run docker < 1.0 already because the security fixes in 1.0. |
So the parallel pulls should behave the same: It blocks until the pull finished, then returns: But the tags need to provided explicitly. So if Pull(image string) should support tags (user/image:tag), we need to either parse the tag or extend kubernetes container structs to support a tags attribute. Parsing the tag is rather ugly because we basically need to parse everything which, as you saw earlier, is pretty complex. So to me the right way to solve this is either:
What do you think? |
Where do you see tag in the docker API? Maybe I'm looking in the wrong I'm in favor of (1) - this is not something the client should be expected On Fri, Jul 4, 2014 at 5:55 AM, discordianfish notifications@github.com
|
@thockin See "Create an image" here: http://docs.docker.com/reference/api/docker_remote_api_v1.12/#22-images And to me it also looks like the server should parse that, given it parses all other parameters from the name as well. But maybe there are reasons for not doing it. I'll discuss that, but if this gets changed kubernetes would require at least some future version of Docker. Would that be okay? Or should this be rather backward compatible? In that case we still need to parse the tag to support older docker daemons. |
@thockin I've opened moby/moby#6876 to track adding parsing of the tag to the daemon. But since it probably make sense to support already existing Docker versions, I've added the parsing to kubernetes for :). See updated code. |
I find no "Creating" anywhere on the page at ? On Mon, Jul 7, 2014 at 3:26 AM, discordianfish notifications@github.com
|
@thockin Not "Creating", but "Create" :) But yes it's rather hard to navigate the docs.. |
@@ -335,6 +330,26 @@ func makePortsAndBindings(container *api.Container) (map[docker.Port]struct{}, m | |||
return exposedPorts, portBindings | |||
} | |||
|
|||
// Parses image name including an tag and returns image name and tag . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a "TODO" comment explaining that we don'tr really want to be doing this and pointing at the docker issues?
Use api for pulling images instead of shelling out
Add a validation handler to check cAdvisor's underlying dependencies.
Change apiserver setup for baremetal cluster.
Change apiserver setup for baremetal cluster.
Change apiserver setup for baremetal cluster.
Change apiserver setup for baremetal cluster.
Change apiserver setup for baremetal cluster.
Add testing script to invoke example testing
Bug 1873043: Commit openapi definitions
*: add license header test + travis badge
I've just copied the functions to parse and validate the repo name from Docker because otherwise this would require including the complete Docker sources.
/cc @brendandburns