Merge pull request #351 from discordianfish/use-api-for-pull

Use api for pulling images instead of shelling out

CONTRIB.md

@@ -22,3 +22,7 @@ Follow either of the two links above to access the appropriate CLA and instructi
 ## Protocols for Collaborative Development
 
 Please read [this doc](https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/collab.md) for information on how we're runnig development for the project.
+
+## Adding dependencies
+
+If your patch depends on new packages, add them to `third_party/deps.sh` and run `third_party/update.sh [package]` to fetch and commit the dependency.

cmd/kubelet/kubelet.go

@@ -47,8 +47,6 @@ var (
 	dockerEndpoint     = flag.String("docker_endpoint", "", "If non-empty, use this for the docker endpoint to communicate with")
 )
 
-const dockerBinary = "/usr/bin/docker"
-
 func main() {
 	flag.Parse()
 	util.InitLogs()

pkg/kubelet/fake_docker_client.go

@@ -17,6 +17,8 @@ limitations under the License.
 package kubelet
 
 import (
+	"fmt"
+
 	"github.com/fsouza/go-dockerclient"
 )
 
@@ -27,6 +29,7 @@ type FakeDockerClient struct {
 	err           error
 	called        []string
 	stopped       []string
+	pulled        []string
 	Created       []string
 }
 
@@ -68,6 +71,12 @@ func (f *FakeDockerClient) StopContainer(id string, timeout uint) error {
 	return f.err
 }
 
+func (f *FakeDockerClient) PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error {
+	f.appendCall("pull")
+	f.pulled = append(f.pulled, fmt.Sprintf("%s/%s:%s", opts.Repository, opts.Registry, opts.Tag))
+	return f.err
+}
+
 type FakeDockerPuller struct {
 	ImagesPulled []string
 

pkg/kubelet/kubelet.go

@@ -25,7 +25,6 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"os/exec"
 	"path"
 	"path/filepath"
 	"sort"
@@ -61,6 +60,7 @@ type DockerInterface interface {
 	CreateContainer(docker.CreateContainerOptions) (*docker.Container, error)
 	StartContainer(id string, hostConfig *docker.HostConfig) error
 	StopContainer(id string, timeout uint) error
+	PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error
 }
 
 // Type to make it clear when we're working with docker container Ids
@@ -109,7 +109,7 @@ const (
 // they are not watched. Never returns.
 func (kl *Kubelet) RunKubelet(dockerEndpoint, config_path, manifest_url, etcd_servers, address string, port uint) {
 	if kl.DockerPuller == nil {
-		kl.DockerPuller = MakeDockerPuller(dockerEndpoint)
+		kl.DockerPuller = kl.MakeDockerPuller()
 	}
 	updateChannel := make(chan manifestUpdate)
 	if config_path != "" {
@@ -211,28 +211,23 @@ func (kl *Kubelet) getContainerID(manifest *api.ContainerManifest, container *ap
 	return "", nil
 }
 
-type dockerPuller struct {
+func (kl *Kubelet) MakeDockerPuller() DockerPuller {
-	endpoint string
+	return dockerPuller{
+		client: kl.DockerClient,
+	}
 }
 
-func MakeDockerPuller(endpoint string) DockerPuller {
+type dockerPuller struct {
-	return dockerPuller{
+	client DockerInterface
-		endpoint: endpoint,
-	}
 }
 
 func (p dockerPuller) Pull(image string) error {
-	var cmd *exec.Cmd
+	image, tag := parseImageName(image)
-	if len(p.endpoint) == 0 {
+	opts := docker.PullImageOptions{
-		cmd = exec.Command("docker", "pull", image)
+		Repository: image,
-	} else {
+		Tag:        tag,
-		cmd = exec.Command("docker", "-H", p.endpoint, "pull", image)
 	}
-	err := cmd.Start()
+	return p.client.PullImage(opts, docker.AuthConfiguration{})
-	if err != nil {
-		return err
-	}
-	return cmd.Wait()
 }
 
 // Converts "-" to "_-_" and "_" to "___" so that we can use "--" to meaningfully separate parts of a docker name.
@@ -337,6 +332,29 @@ 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
+// TODO: Future Docker versions can parse the tag on daemon side, see:
+// https://github.com/dotcloud/docker/issues/6876
+// So this can be deprecated at some point.
+func parseImageName(image string) (string, string) {
+	tag := ""
+	parts := strings.SplitN(image, "/", 2)
+	repo := ""
+	if len(parts) == 2 {
+		repo = parts[0]
+		image = parts[1]
+	}
+	parts = strings.SplitN(image, ":", 2)
+	if len(parts) == 2 {
+		image = parts[0]
+		tag = parts[1]
+	}
+	if repo != "" {
+		image = fmt.Sprintf("%s/%s", repo, image)
+	}
+	return image, tag
+}
+
 // Run a single container from a manifest. Returns the docker container ID
 func (kl *Kubelet) runContainer(manifest *api.ContainerManifest, container *api.Container, netMode string) (id DockerID, err error) {
 	envVariables := makeEnvironmentVariables(container)

pkg/kubelet/kubelet_test.go

@@ -1027,3 +1027,30 @@ func TestGetContainerStatsOnNonExistContainer(t *testing.T) {
 	}
 	mockCadvisor.AssertExpectations(t)
 }
+
+func TestParseImageName(t *testing.T) {
+	name, tag := parseImageName("ubuntu")
+	if name != "ubuntu" || tag != "" {
+		t.Fatal("Unexpected name/tag: %s/%s", name, tag)
+	}
+
+	name, tag = parseImageName("ubuntu:2342")
+	if name != "ubuntu" || tag != "2342" {
+		t.Fatal("Unexpected name/tag: %s/%s", name, tag)
+	}
+
+	name, tag = parseImageName("foo/bar:445566")
+	if name != "foo/bar" || tag != "445566" {
+		t.Fatal("Unexpected name/tag: %s/%s", name, tag)
+	}
+
+	name, tag = parseImageName("registry.example.com:5000/foobar")
+	if name != "registry.example.com:5000/foobar" || tag != "" {
+		t.Fatal("Unexpected name/tag: %s/%s", name, tag)
+	}
+
+	name, tag = parseImageName("registry.example.com:5000/foobar:5342")
+	if name != "registry.example.com:5000/foobar" || tag != "5342" {
+		t.Fatal("Unexpected name/tag: %s/%s", name, tag)
+	}
+}