Merge pull request #1218 from bcwaldon/docker-auth

Authenticated docker pulls, pt. I

cmd/integration/integration.go

@@ -34,6 +34,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/controller"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/config"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/master"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
@@ -46,7 +47,7 @@ import (
 )
 
 var (
-	fakeDocker1, fakeDocker2 kubelet.FakeDockerClient
+	fakeDocker1, fakeDocker2 dockertools.FakeDockerClient
 )
 
 type fakePodInfoGetter struct{}

pkg/kubelet/dockertools/config.go

@@ -0,0 +1,116 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package dockertools
+
+import (
+	"encoding/base64"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/url"
+	"strings"
+
+	"github.com/fsouza/go-dockerclient"
+	"github.com/golang/glog"
+)
+
+const (
+	dockerConfigFileLocation = ".dockercfg"
+)
+
+func readDockerConfigFile() (cfg dockerConfig, err error) {
+	contents, err := ioutil.ReadFile(dockerConfigFileLocation)
+	err = json.Unmarshal(contents, &cfg)
+	return
+}
+
+// dockerConfig represents the config file used by the docker CLI.
+// This config that represents the credentials that should be used
+// when pulling images from specific image repositories.
+type dockerConfig map[string]dockerConfigEntry
+
+func (dc dockerConfig) addToKeyring(dk *dockerKeyring) {
+	for loc, ident := range dc {
+		creds := docker.AuthConfiguration{
+			Username: ident.Username,
+			Password: ident.Password,
+			Email:    ident.Email,
+		}
+
+		parsed, err := url.Parse(loc)
+		if err != nil {
+			glog.Errorf("Entry %q in dockercfg invalid (%v), ignoring", loc, err)
+			continue
+		}
+
+		dk.add(parsed.Host+parsed.Path, creds)
+	}
+}
+
+type dockerConfigEntry struct {
+	Username string
+	Password string
+	Email    string
+}
+
+// dockerConfigEntryWithAuth is used solely for deserializing the Auth field
+// into a dockerConfigEntry during JSON deserialization.
+type dockerConfigEntryWithAuth struct {
+	Username string
+	Password string
+	Email    string
+	Auth     string
+}
+
+func (ident *dockerConfigEntry) UnmarshalJSON(data []byte) error {
+	var tmp dockerConfigEntryWithAuth
+	err := json.Unmarshal(data, &tmp)
+	if err != nil {
+		return err
+	}
+
+	ident.Username = tmp.Username
+	ident.Password = tmp.Password
+	ident.Email = tmp.Email
+
+	if len(tmp.Auth) == 0 {
+		return nil
+	}
+
+	ident.Username, ident.Password, err = decodeDockerConfigFieldAuth(tmp.Auth)
+	return err
+}
+
+// decodeDockerConfigFieldAuth deserializes the "auth" field from dockercfg into a
+// username and a password. The format of the auth field is base64(<username>:<password>).
+func decodeDockerConfigFieldAuth(field string) (username, password string, err error) {
+	decoded, err := base64.StdEncoding.DecodeString(field)
+	if err != nil {
+		return
+	}
+
+	parts := strings.SplitN(string(decoded), ":", 2)
+	if len(parts) != 2 {
+		err = fmt.Errorf("unable to parse auth field")
+		return
+	}
+
+	username = parts[0]
+	password = parts[1]
+
+	return
+}

pkg/kubelet/dockertools/config_test.go

@@ -0,0 +1,208 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package dockertools
+
+import (
+	"encoding/json"
+	"reflect"
+	"testing"
+
+	"github.com/fsouza/go-dockerclient"
+)
+
+func TestDockerConfigJSONDecode(t *testing.T) {
+	input := []byte(`{"http://foo.example.com":{"username": "foo", "password": "bar", "email": "foo@example.com"}, "http://bar.example.com":{"username": "bar", "password": "baz", "email": "bar@example.com"}}`)
+
+	expect := dockerConfig(map[string]dockerConfigEntry{
+		"http://foo.example.com": dockerConfigEntry{
+			Username: "foo",
+			Password: "bar",
+			Email:    "foo@example.com",
+		},
+		"http://bar.example.com": dockerConfigEntry{
+			Username: "bar",
+			Password: "baz",
+			Email:    "bar@example.com",
+		},
+	})
+
+	var output dockerConfig
+	err := json.Unmarshal(input, &output)
+	if err != nil {
+		t.Errorf("Received unexpected error: %v", err)
+	}
+
+	if !reflect.DeepEqual(expect, output) {
+		t.Errorf("Received unexpected output. Expected %#v, got %#v", expect, output)
+	}
+}
+
+func TestDockerConfigEntryJSONDecode(t *testing.T) {
+	tests := []struct {
+		input  []byte
+		expect dockerConfigEntry
+		fail   bool
+	}{
+		// simple case, just decode the fields
+		{
+			input: []byte(`{"username": "foo", "password": "bar", "email": "foo@example.com"}`),
+			expect: dockerConfigEntry{
+				Username: "foo",
+				Password: "bar",
+				Email:    "foo@example.com",
+			},
+			fail: false,
+		},
+
+		// auth field decodes to username & password
+		{
+			input: []byte(`{"auth": "Zm9vOmJhcg==", "email": "foo@example.com"}`),
+			expect: dockerConfigEntry{
+				Username: "foo",
+				Password: "bar",
+				Email:    "foo@example.com",
+			},
+			fail: false,
+		},
+
+		// auth field overrides username & password
+		{
+			input: []byte(`{"username": "foo", "password": "bar", "auth": "cGluZzpwb25n", "email": "foo@example.com"}`),
+			expect: dockerConfigEntry{
+				Username: "ping",
+				Password: "pong",
+				Email:    "foo@example.com",
+			},
+			fail: false,
+		},
+
+		// poorly-formatted auth causes failure
+		{
+			input: []byte(`{"auth": "pants", "email": "foo@example.com"}`),
+			expect: dockerConfigEntry{
+				Username: "",
+				Password: "",
+				Email:    "foo@example.com",
+			},
+			fail: true,
+		},
+
+		// invalid JSON causes failure
+		{
+			input: []byte(`{"email": false}`),
+			expect: dockerConfigEntry{
+				Username: "",
+				Password: "",
+				Email:    "",
+			},
+			fail: true,
+		},
+	}
+
+	for i, tt := range tests {
+		var output dockerConfigEntry
+		err := json.Unmarshal(tt.input, &output)
+		if (err != nil) != tt.fail {
+			t.Errorf("case %d: expected fail=%t, got err=%v", i, tt.fail, err)
+		}
+
+		if !reflect.DeepEqual(tt.expect, output) {
+			t.Errorf("case %d: expected output %#v, got %#v", i, tt.expect, output)
+		}
+	}
+}
+
+func TestDecodeDockerConfigFieldAuth(t *testing.T) {
+	tests := []struct {
+		input    string
+		username string
+		password string
+		fail     bool
+	}{
+		// auth field decodes to username & password
+		{
+			input:    "Zm9vOmJhcg==",
+			username: "foo",
+			password: "bar",
+		},
+
+		// good base64 data, but no colon separating username & password
+		{
+			input: "cGFudHM=",
+			fail:  true,
+		},
+
+		// bad base64 data
+		{
+			input: "pants",
+			fail:  true,
+		},
+	}
+
+	for i, tt := range tests {
+		username, password, err := decodeDockerConfigFieldAuth(tt.input)
+		if (err != nil) != tt.fail {
+			t.Errorf("case %d: expected fail=%t, got err=%v", i, tt.fail, err)
+		}
+
+		if tt.username != username {
+			t.Errorf("case %d: expected username %q, got %q", i, tt.username, username)
+		}
+
+		if tt.password != password {
+			t.Errorf("case %d: expected password %q, got %q", i, tt.password, password)
+		}
+	}
+}
+
+func TestDockerKeyringFromConfig(t *testing.T) {
+	cfg := dockerConfig(map[string]dockerConfigEntry{
+		"http://foo.example.com": dockerConfigEntry{
+			Username: "foo",
+			Password: "bar",
+			Email:    "foo@example.com",
+		},
+		"https://bar.example.com": dockerConfigEntry{
+			Username: "bar",
+			Password: "baz",
+			Email:    "bar@example.com",
+		},
+	})
+
+	dk := newDockerKeyring()
+	cfg.addToKeyring(dk)
+
+	expect := newDockerKeyring()
+	expect.add("foo.example.com",
+		docker.AuthConfiguration{
+			Username: "foo",
+			Password: "bar",
+			Email:    "foo@example.com",
+		},
+	)
+	expect.add("bar.example.com",
+		docker.AuthConfiguration{
+			Username: "bar",
+			Password: "baz",
+			Email:    "bar@example.com",
+		},
+	)
+
+	if !reflect.DeepEqual(expect, dk) {
+		t.Errorf("Received unexpected output. Expected %#v, got %#v", expect, dk)
+	}
+}

pkg/kubelet/dockertools/docker.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package kubelet
+package dockertools
 
 import (
 	"errors"
@@ -22,6 +22,7 @@ import (
 	"hash/adler32"
 	"math/rand"
 	"os/exec"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -58,13 +59,28 @@ type DockerPuller interface {
 // dockerPuller is the default implementation of DockerPuller.
 type dockerPuller struct {
 	client  DockerInterface
+	keyring *dockerKeyring
 }
 
 // NewDockerPuller creates a new instance of the default implementation of DockerPuller.
 func NewDockerPuller(client DockerInterface) DockerPuller {
-	return dockerPuller{
+	dp := dockerPuller{
 		client:  client,
+		keyring: newDockerKeyring(),
 	}
+
+	cfg, err := readDockerConfigFile()
+	if err == nil {
+		cfg.addToKeyring(dp.keyring)
+	} else {
+		glog.Errorf("Unable to parse docker config file: %v", err)
+	}
+
+	if dp.keyring.count() == 0 {
+		glog.Infof("Continuing with empty docker keyring")
+	}
+
+	return dp
 }
 
 type dockerContainerCommandRunner struct{}
@@ -103,7 +119,13 @@ func (p dockerPuller) Pull(image string) error {
 		Repository: image,
 		Tag:        tag,
 	}
-	return p.client.PullImage(opts, docker.AuthConfiguration{})
+
+	creds, ok := p.keyring.lookup(image)
+	if !ok {
+		glog.V(1).Infof("Pulling image %s without credentials", image)
+	}
+
+	return p.client.PullImage(opts, creds)
 }
 
 // DockerContainers is a map of containers
@@ -111,7 +133,7 @@ type DockerContainers map[DockerID]*docker.APIContainers
 
 func (c DockerContainers) FindPodContainer(podFullName, uuid, containerName string) (*docker.APIContainers, bool, uint64) {
 	for _, dockerContainer := range c {
-		dockerManifestID, dockerUUID, dockerContainerName, hash := parseDockerName(dockerContainer.Names[0])
+		dockerManifestID, dockerUUID, dockerContainerName, hash := ParseDockerName(dockerContainer.Names[0])
 		if dockerManifestID == podFullName &&
 			(uuid == "" || dockerUUID == uuid) &&
 			dockerContainerName == containerName {
@@ -126,7 +148,7 @@ func (c DockerContainers) FindContainersByPodFullName(podFullName string) map[st
 	containers := make(map[string]*docker.APIContainers)
 
 	for _, dockerContainer := range c {
-		dockerManifestID, _, dockerContainerName, _ := parseDockerName(dockerContainer.Names[0])
+		dockerManifestID, _, dockerContainerName, _ := ParseDockerName(dockerContainer.Names[0])
 		if dockerManifestID == podFullName {
 			containers[dockerContainerName] = dockerContainer
 		}
@@ -135,7 +157,7 @@ func (c DockerContainers) FindContainersByPodFullName(podFullName string) map[st
 }
 
 // GetKubeletDockerContainers returns a map of docker containers that we manage. The map key is the docker container ID
-func getKubeletDockerContainers(client DockerInterface) (DockerContainers, error) {
+func GetKubeletDockerContainers(client DockerInterface) (DockerContainers, error) {
 	result := make(DockerContainers)
 	containers, err := client.ListContainers(docker.ListContainersOptions{})
 	if err != nil {
@@ -153,16 +175,16 @@ func getKubeletDockerContainers(client DockerInterface) (DockerContainers, error
 	return result, nil
 }
 
-// getRecentDockerContainersWithName returns a list of dead docker containers which matches the name
+// GetRecentDockerContainersWithNameAndUUID returns a list of dead docker containers which matches the name
 // and uuid given.
-func getRecentDockerContainersWithNameAndUUID(client DockerInterface, podFullName, uuid, containerName string) ([]*docker.Container, error) {
+func GetRecentDockerContainersWithNameAndUUID(client DockerInterface, podFullName, uuid, containerName string) ([]*docker.Container, error) {
 	var result []*docker.Container
 	containers, err := client.ListContainers(docker.ListContainersOptions{All: true})
 	if err != nil {
 		return nil, err
 	}
 	for _, dockerContainer := range containers {
-		dockerPodName, dockerUUID, dockerContainerName, _ := parseDockerName(dockerContainer.Names[0])
+		dockerPodName, dockerUUID, dockerContainerName, _ := ParseDockerName(dockerContainer.Names[0])
 		if dockerPodName != podFullName {
 			continue
 		}
@@ -184,7 +206,7 @@ func getRecentDockerContainersWithNameAndUUID(client DockerInterface, podFullNam
 var ErrNoContainersInPod = errors.New("no containers exist for this pod")
 
 // GetDockerPodInfo returns docker info for all containers in the pod/manifest.
-func getDockerPodInfo(client DockerInterface, podFullName, uuid string) (api.PodInfo, error) {
+func GetDockerPodInfo(client DockerInterface, podFullName, uuid string) (api.PodInfo, error) {
 	info := api.PodInfo{}
 
 	containers, err := client.ListContainers(docker.ListContainersOptions{All: true})
@@ -193,7 +215,7 @@ func getDockerPodInfo(client DockerInterface, podFullName, uuid string) (api.Pod
 	}
 
 	for _, value := range containers {
-		dockerManifestID, dockerUUID, dockerContainerName, _ := parseDockerName(value.Names[0])
+		dockerManifestID, dockerUUID, dockerContainerName, _ := ParseDockerName(value.Names[0])
 		if dockerManifestID != podFullName {
 			continue
 		}
@@ -239,35 +261,35 @@ func unescapeDash(in string) (out string) {
 
 const containerNamePrefix = "k8s"
 
-func hashContainer(container *api.Container) uint64 {
+func HashContainer(container *api.Container) uint64 {
 	hash := adler32.New()
 	fmt.Fprintf(hash, "%#v", *container)
 	return uint64(hash.Sum32())
 }
 
 // Creates a name which can be reversed to identify both full pod name and container name.
-func buildDockerName(pod *Pod, container *api.Container) string {
+func BuildDockerName(manifestUUID, podFullName string, container *api.Container) string {
-	containerName := escapeDash(container.Name) + "." + strconv.FormatUint(hashContainer(container), 16)
+	containerName := escapeDash(container.Name) + "." + strconv.FormatUint(HashContainer(container), 16)
 	// Note, manifest.ID could be blank.
-	if len(pod.Manifest.UUID) == 0 {
+	if len(manifestUUID) == 0 {
 		return fmt.Sprintf("%s--%s--%s--%08x",
 			containerNamePrefix,
 			containerName,
-			escapeDash(GetPodFullName(pod)),
+			escapeDash(podFullName),
 			rand.Uint32())
 	} else {
 		return fmt.Sprintf("%s--%s--%s--%s--%08x",
 			containerNamePrefix,
 			containerName,
-			escapeDash(GetPodFullName(pod)),
+			escapeDash(podFullName),
-			escapeDash(pod.Manifest.UUID),
+			escapeDash(manifestUUID),
 			rand.Uint32())
 	}
 }
 
 // Upacks a container name, returning the pod full name and container name we would have used to
 // construct the docker name. If the docker name isn't one we created, we may return empty strings.
-func parseDockerName(name string) (podFullName, uuid, containerName string, hash uint64) {
+func ParseDockerName(name string) (podFullName, uuid, containerName string, hash uint64) {
 	// For some reason docker appears to be appending '/' to names.
 	// If it's there, strip it.
 	if name[0] == '/' {
@@ -319,3 +341,59 @@ func parseImageName(image string) (string, string) {
 	}
 	return image, tag
 }
+
+type ContainerCommandRunner interface {
+	RunInContainer(containerID string, cmd []string) ([]byte, error)
+}
+
+// dockerKeyring tracks a set of docker registry credentials, maintaining a
+// reverse index across the registry endpoints. A registry endpoint is made
+// up of a host (e.g. registry.example.com), but it may also contain a path
+// (e.g. registry.example.com/foo) This index is important for two reasons:
+// - registry endpoints may overlap, and when this happens we must find the
+//   most specific match for a given image
+// - iterating a map does not yield predictable results
+type dockerKeyring struct {
+	index []string
+	creds map[string]docker.AuthConfiguration
+}
+
+func newDockerKeyring() *dockerKeyring {
+	return &dockerKeyring{
+		index: make([]string, 0),
+		creds: make(map[string]docker.AuthConfiguration),
+	}
+}
+
+func (dk *dockerKeyring) add(registry string, creds docker.AuthConfiguration) {
+	dk.creds[registry] = creds
+
+	dk.index = append(dk.index, registry)
+	dk.reindex()
+}
+
+// reindex updates the index used to identify which credentials to use for
+// a given image. The index is reverse-sorted so more specific paths are
+// matched first. For example, if for the given image "quay.io/coreos/etcd",
+// credentials for "quay.io/coreos" should match before "quay.io".
+func (dk *dockerKeyring) reindex() {
+	sort.Sort(sort.Reverse(sort.StringSlice(dk.index)))
+}
+
+func (dk *dockerKeyring) lookup(image string) (docker.AuthConfiguration, bool) {
+	// range over the index as iterating over a map does not provide
+	// a predictable ordering
+	for _, k := range dk.index {
+		if !strings.HasPrefix(image, k) {
+			continue
+		}
+
+		return dk.creds[k], true
+	}
+
+	return docker.AuthConfiguration{}, false
+}
+
+func (dk dockerKeyring) count() int {
+	return len(dk.creds)
+}

pkg/kubelet/dockertools/docker_test.go

@@ -0,0 +1,209 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package dockertools
+
+import (
+	"fmt"
+	"hash/adler32"
+	"reflect"
+	"testing"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/fsouza/go-dockerclient"
+)
+
+func verifyCalls(t *testing.T, fakeDocker *FakeDockerClient, calls []string) {
+	fakeDocker.Lock()
+	defer fakeDocker.Unlock()
+	verifyStringArrayEquals(t, fakeDocker.called, calls)
+}
+
+func verifyStringArrayEquals(t *testing.T, actual, expected []string) {
+	invalid := len(actual) != len(expected)
+	if !invalid {
+		for ix, value := range actual {
+			if expected[ix] != value {
+				invalid = true
+			}
+		}
+	}
+	if invalid {
+		t.Errorf("Expected: %#v, Actual: %#v", expected, actual)
+	}
+}
+
+func TestGetContainerID(t *testing.T) {
+	fakeDocker := &FakeDockerClient{}
+	fakeDocker.ContainerList = []docker.APIContainers{
+		{
+			ID:    "foobar",
+			Names: []string{"/k8s--foo--qux--1234"},
+		},
+		{
+			ID:    "barbar",
+			Names: []string{"/k8s--bar--qux--2565"},
+		},
+	}
+	fakeDocker.Container = &docker.Container{
+		ID: "foobar",
+	}
+
+	dockerContainers, err := GetKubeletDockerContainers(fakeDocker)
+	if err != nil {
+		t.Errorf("Expected no error, Got %#v", err)
+	}
+	if len(dockerContainers) != 2 {
+		t.Errorf("Expected %#v, Got %#v", fakeDocker.ContainerList, dockerContainers)
+	}
+	verifyCalls(t, fakeDocker, []string{"list"})
+	dockerContainer, found, _ := dockerContainers.FindPodContainer("qux", "", "foo")
+	if dockerContainer == nil || !found {
+		t.Errorf("Failed to find container %#v", dockerContainer)
+	}
+
+	fakeDocker.clearCalls()
+	dockerContainer, found, _ = dockerContainers.FindPodContainer("foobar", "", "foo")
+	verifyCalls(t, fakeDocker, []string{})
+	if dockerContainer != nil || found {
+		t.Errorf("Should not have found container %#v", dockerContainer)
+	}
+}
+
+func verifyPackUnpack(t *testing.T, podNamespace, podName, containerName string) {
+	container := &api.Container{Name: containerName}
+	hasher := adler32.New()
+	data := fmt.Sprintf("%#v", *container)
+	hasher.Write([]byte(data))
+	computedHash := uint64(hasher.Sum32())
+	podFullName := fmt.Sprintf("%s.%s", podName, podNamespace)
+	name := BuildDockerName("", podFullName, container)
+	returnedPodFullName, _, returnedContainerName, hash := ParseDockerName(name)
+	if podFullName != returnedPodFullName || containerName != returnedContainerName || computedHash != hash {
+		t.Errorf("For (%s, %s, %d), unpacked (%s, %s, %d)", podFullName, containerName, computedHash, returnedPodFullName, returnedContainerName, hash)
+	}
+}
+
+func TestContainerManifestNaming(t *testing.T) {
+	verifyPackUnpack(t, "file", "manifest1234", "container5678")
+	verifyPackUnpack(t, "file", "manifest--", "container__")
+	verifyPackUnpack(t, "file", "--manifest", "__container")
+	verifyPackUnpack(t, "", "m___anifest_", "container-_-")
+	verifyPackUnpack(t, "other", "_m___anifest", "-_-container")
+
+	container := &api.Container{Name: "container"}
+	podName := "foo"
+	podNamespace := "test"
+	name := fmt.Sprintf("k8s--%s--%s.%s--12345", container.Name, podName, podNamespace)
+
+	podFullName := fmt.Sprintf("%s.%s", podName, podNamespace)
+	returnedPodFullName, _, returnedContainerName, hash := ParseDockerName(name)
+	if returnedPodFullName != podFullName || returnedContainerName != container.Name || hash != 0 {
+		t.Errorf("unexpected parse: %s %s %d", returnedPodFullName, returnedContainerName, hash)
+	}
+}
+
+func TestDockerContainerCommand(t *testing.T) {
+	runner := dockerContainerCommandRunner{}
+	containerID := "1234"
+	command := []string{"ls"}
+	cmd, _ := runner.getRunInContainerCommand(containerID, command)
+	if cmd.Dir != "/var/lib/docker/execdriver/native/"+containerID {
+		t.Errorf("unexpected command CWD: %s", cmd.Dir)
+	}
+	if !reflect.DeepEqual(cmd.Args, []string{"/usr/sbin/nsinit", "exec", "ls"}) {
+		t.Errorf("unexpectd command args: %s", cmd.Args)
+	}
+}
+
+var parseImageNameTests = []struct {
+	imageName string
+	name      string
+	tag       string
+}{
+	{"ubuntu", "ubuntu", ""},
+	{"ubuntu:2342", "ubuntu", "2342"},
+	{"ubuntu:latest", "ubuntu", "latest"},
+	{"foo/bar:445566", "foo/bar", "445566"},
+	{"registry.example.com:5000/foobar", "registry.example.com:5000/foobar", ""},
+	{"registry.example.com:5000/foobar:5342", "registry.example.com:5000/foobar", "5342"},
+	{"registry.example.com:5000/foobar:latest", "registry.example.com:5000/foobar", "latest"},
+}
+
+func TestParseImageName(t *testing.T) {
+	for _, tt := range parseImageNameTests {
+		name, tag := parseImageName(tt.imageName)
+		if name != tt.name || tag != tt.tag {
+			t.Errorf("Expected name/tag: %s/%s, got %s/%s", tt.name, tt.tag, name, tag)
+		}
+	}
+}
+
+func TestDockerKeyringLookup(t *testing.T) {
+	empty := docker.AuthConfiguration{}
+
+	ada := docker.AuthConfiguration{
+		Username: "ada",
+		Password: "smash",
+		Email:    "ada@example.com",
+	}
+
+	grace := docker.AuthConfiguration{
+		Username: "grace",
+		Password: "squash",
+		Email:    "grace@example.com",
+	}
+
+	dk := newDockerKeyring()
+	dk.add("bar.example.com/pong", grace)
+	dk.add("bar.example.com", ada)
+
+	tests := []struct {
+		image string
+		match docker.AuthConfiguration
+		ok    bool
+	}{
+		// direct match
+		{"bar.example.com", ada, true},
+
+		// direct match deeper than other possible matches
+		{"bar.example.com/pong", grace, true},
+
+		// no direct match, deeper path ignored
+		{"bar.example.com/ping", ada, true},
+
+		// match first part of path token
+		{"bar.example.com/pongz", grace, true},
+
+		// match regardless of sub-path
+		{"bar.example.com/pong/pang", grace, true},
+
+		// no host match
+		{"example.com", empty, false},
+		{"foo.example.com", empty, false},
+	}
+
+	for i, tt := range tests {
+		match, ok := dk.lookup(tt.image)
+		if tt.ok != ok {
+			t.Errorf("case %d: expected ok=%t, got %t", i, tt.ok, ok)
+		}
+
+		if !reflect.DeepEqual(tt.match, match) {
+			t.Errorf("case %d: expected match=%#v, got %#v", i, tt.match, match)
+		}
+	}
+}

pkg/kubelet/dockertools/fake_docker_client.go

@@ -14,10 +14,11 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package kubelet
+package dockertools
 
 import (
 	"fmt"
+	"reflect"
 	"sync"
 
 	"github.com/fsouza/go-dockerclient"
@@ -25,93 +26,105 @@ import (
 
 // FakeDockerClient is a simple fake docker client, so that kubelet can be run for testing without requiring a real docker setup.
 type FakeDockerClient struct {
-	lock          sync.Mutex
+	sync.Mutex
-	containerList []docker.APIContainers
+	ContainerList []docker.APIContainers
-	container     *docker.Container
+	Container     *docker.Container
-	err           error
+	Err           error
 	called        []string
-	stopped       []string
+	Stopped       []string
 	pulled        []string
 	Created       []string
 }
 
 func (f *FakeDockerClient) clearCalls() {
-	f.lock.Lock()
+	f.Lock()
-	defer f.lock.Unlock()
+	defer f.Unlock()
 	f.called = []string{}
 }
 
+func (f *FakeDockerClient) AssertCalls(calls []string) (err error) {
+	f.Lock()
+	defer f.Unlock()
+
+	if !reflect.DeepEqual(calls, f.called) {
+		err = fmt.Errorf("expected %#v, got %#v", calls, f.called)
+	}
+
+	return
+}
+
 // ListContainers is a test-spy implementation of DockerInterface.ListContainers.
 // It adds an entry "list" to the internal method call record.
 func (f *FakeDockerClient) ListContainers(options docker.ListContainersOptions) ([]docker.APIContainers, error) {
-	f.lock.Lock()
+	f.Lock()
-	defer f.lock.Unlock()
+	defer f.Unlock()
 	f.called = append(f.called, "list")
-	return f.containerList, f.err
+	return f.ContainerList, f.Err
 }
 
 // InspectContainer is a test-spy implementation of DockerInterface.InspectContainer.
 // It adds an entry "inspect" to the internal method call record.
 func (f *FakeDockerClient) InspectContainer(id string) (*docker.Container, error) {
-	f.lock.Lock()
+	f.Lock()
-	defer f.lock.Unlock()
+	defer f.Unlock()
 	f.called = append(f.called, "inspect")
-	return f.container, f.err
+	return f.Container, f.Err
 }
 
 // CreateContainer is a test-spy implementation of DockerInterface.CreateContainer.
 // It adds an entry "create" to the internal method call record.
 func (f *FakeDockerClient) CreateContainer(c docker.CreateContainerOptions) (*docker.Container, error) {
-	f.lock.Lock()
+	f.Lock()
-	defer f.lock.Unlock()
+	defer f.Unlock()
 	f.called = append(f.called, "create")
 	f.Created = append(f.Created, c.Name)
 	// This is not a very good fake. We'll just add this container's name to the list.
 	// Docker likes to add a '/', so copy that behavior.
 	name := "/" + c.Name
-	f.containerList = append(f.containerList, docker.APIContainers{ID: name, Names: []string{name}})
+	f.ContainerList = append(f.ContainerList, docker.APIContainers{ID: name, Names: []string{name}})
 	return &docker.Container{ID: name}, nil
 }
 
 // StartContainer is a test-spy implementation of DockerInterface.StartContainer.
 // It adds an entry "start" to the internal method call record.
 func (f *FakeDockerClient) StartContainer(id string, hostConfig *docker.HostConfig) error {
-	f.lock.Lock()
+	f.Lock()
-	defer f.lock.Unlock()
+	defer f.Unlock()
 	f.called = append(f.called, "start")
-	return f.err
+	return f.Err
 }
 
 // StopContainer is a test-spy implementation of DockerInterface.StopContainer.
 // It adds an entry "stop" to the internal method call record.
 func (f *FakeDockerClient) StopContainer(id string, timeout uint) error {
-	f.lock.Lock()
+	f.Lock()
-	defer f.lock.Unlock()
+	defer f.Unlock()
 	f.called = append(f.called, "stop")
-	f.stopped = append(f.stopped, id)
+	f.Stopped = append(f.Stopped, id)
 	var newList []docker.APIContainers
-	for _, container := range f.containerList {
+	for _, container := range f.ContainerList {
 		if container.ID != id {
 			newList = append(newList, container)
 		}
 	}
-	f.containerList = newList
+	f.ContainerList = newList
-	return f.err
+	return f.Err
 }
 
 // PullImage is a test-spy implementation of DockerInterface.StopContainer.
 // It adds an entry "pull" to the internal method call record.
 func (f *FakeDockerClient) PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error {
-	f.lock.Lock()
+	f.Lock()
-	defer f.lock.Unlock()
+	defer f.Unlock()
 	f.called = append(f.called, "pull")
 	f.pulled = append(f.pulled, fmt.Sprintf("%s/%s:%s", opts.Repository, opts.Registry, opts.Tag))
-	return f.err
+	return f.Err
 }
 
 // FakeDockerPuller is a stub implementation of DockerPuller.
 type FakeDockerPuller struct {
-	lock         sync.Mutex
+	sync.Mutex
+
 	ImagesPulled []string
 
 	// Every pull will return the first error here, and then reslice
@@ -121,8 +134,8 @@ type FakeDockerPuller struct {
 
 // Pull records the image pull attempt, and optionally injects an error.
 func (f *FakeDockerPuller) Pull(image string) (err error) {
-	f.lock.Lock()
+	f.Lock()
-	defer f.lock.Unlock()
+	defer f.Unlock()
 	f.ImagesPulled = append(f.ImagesPulled, image)
 
 	if len(f.ErrorsToInject) > 0 {

pkg/kubelet/kubelet.go

@@ -30,6 +30,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/health"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/volume"
@@ -62,7 +63,7 @@ type volumeMap map[string]volume.Interface
 // New creates a new Kubelet for use in main
 func NewMainKubelet(
 	hn string,
-	dc DockerInterface,
+	dc dockertools.DockerInterface,
 	cc CadvisorInterface,
 	ec tools.EtcdClient,
 	rd string,
@@ -75,27 +76,23 @@ func NewMainKubelet(
 		rootDirectory:  rd,
 		resyncInterval: ri,
 		podWorkers:     newPodWorkers(),
-		runner:         NewDockerContainerCommandRunner(),
+		runner:         dockertools.NewDockerContainerCommandRunner(),
 		httpClient:     &http.Client{},
 	}
 }
 
 // NewIntegrationTestKubelet creates a new Kubelet for use in integration tests.
 // TODO: add more integration tests, and expand parameter list as needed.
-func NewIntegrationTestKubelet(hn string, dc DockerInterface) *Kubelet {
+func NewIntegrationTestKubelet(hn string, dc dockertools.DockerInterface) *Kubelet {
 	return &Kubelet{
 		hostname:       hn,
 		dockerClient:   dc,
-		dockerPuller:   &FakeDockerPuller{},
+		dockerPuller:   &dockertools.FakeDockerPuller{},
 		resyncInterval: 3 * time.Second,
 		podWorkers:     newPodWorkers(),
 	}
 }
 
-type ContainerCommandRunner interface {
-	RunInContainer(containerID string, cmd []string) ([]byte, error)
-}
-
 type httpGetInterface interface {
 	Get(url string) (*http.Response, error)
 }
@@ -103,7 +100,7 @@ type httpGetInterface interface {
 // Kubelet is the main kubelet implementation.
 type Kubelet struct {
 	hostname       string
-	dockerClient   DockerInterface
+	dockerClient   dockertools.DockerInterface
 	rootDirectory  string
 	podWorkers     podWorkers
 	resyncInterval time.Duration
@@ -115,11 +112,11 @@ type Kubelet struct {
 	// Optional, defaults to simple implementaiton
 	healthChecker health.HealthChecker
 	// Optional, defaults to simple Docker implementation
-	dockerPuller DockerPuller
+	dockerPuller dockertools.DockerPuller
 	// Optional, defaults to /logs/ from /var/log
 	logServer http.Handler
 	// Optional, defaults to simple Docker implementation
-	runner ContainerCommandRunner
+	runner dockertools.ContainerCommandRunner
 	// Optional, client for http requests, defaults to empty client
 	httpClient httpGetInterface
 }
@@ -130,7 +127,7 @@ func (kl *Kubelet) Run(updates <-chan PodUpdate) {
 		kl.logServer = http.StripPrefix("/logs/", http.FileServer(http.Dir("/var/log/")))
 	}
 	if kl.dockerPuller == nil {
-		kl.dockerPuller = NewDockerPuller(kl.dockerClient)
+		kl.dockerPuller = dockertools.NewDockerPuller(kl.dockerClient)
 	}
 	if kl.healthChecker == nil {
 		kl.healthChecker = health.NewHealthChecker()
@@ -316,13 +313,13 @@ func (kl *Kubelet) runHandler(podFullName, uuid string, container *api.Container
 }
 
 // Run a single container from a pod. Returns the docker container ID
-func (kl *Kubelet) runContainer(pod *Pod, container *api.Container, podVolumes volumeMap, netMode string) (id DockerID, err error) {
+func (kl *Kubelet) runContainer(pod *Pod, container *api.Container, podVolumes volumeMap, netMode string) (id dockertools.DockerID, err error) {
 	envVariables := makeEnvironmentVariables(container)
 	binds := makeBinds(pod, container, podVolumes)
 	exposedPorts, portBindings := makePortsAndBindings(container)
 
 	opts := docker.CreateContainerOptions{
-		Name: buildDockerName(pod, container),
+		Name: dockertools.BuildDockerName(pod.Manifest.UUID, GetPodFullName(pod), container),
 		Config: &docker.Config{
 			Cmd:          container.Command,
 			Env:          envVariables,
@@ -347,10 +344,10 @@ func (kl *Kubelet) runContainer(pod *Pod, container *api.Container, podVolumes v
 		handlerErr := kl.runHandler(GetPodFullName(pod), pod.Manifest.UUID, container, container.Lifecycle.PostStart)
 		if handlerErr != nil {
 			kl.killContainerByID(dockerContainer.ID, "")
-			return DockerID(""), fmt.Errorf("failed to call event handler: %v", handlerErr)
+			return dockertools.DockerID(""), fmt.Errorf("failed to call event handler: %v", handlerErr)
 		}
 	}
-	return DockerID(dockerContainer.ID), err
+	return dockertools.DockerID(dockerContainer.ID), err
 }
 
 // Kill a docker container
@@ -364,7 +361,7 @@ func (kl *Kubelet) killContainerByID(ID, name string) error {
 	if len(name) == 0 {
 		return err
 	}
-	podFullName, uuid, containerName, _ := parseDockerName(name)
+	podFullName, uuid, containerName, _ := dockertools.ParseDockerName(name)
 	kl.LogEvent(&api.Event{
 		Event: "STOP",
 		Manifest: &api.ContainerManifest{
@@ -386,7 +383,7 @@ const (
 )
 
 // createNetworkContainer starts the network container for a pod. Returns the docker container ID of the newly created container.
-func (kl *Kubelet) createNetworkContainer(pod *Pod) (DockerID, error) {
+func (kl *Kubelet) createNetworkContainer(pod *Pod) (dockertools.DockerID, error) {
 	var ports []api.Port
 	// Docker only exports ports from the network container.  Let's
 	// collect all of the relevant ports and export them.
@@ -404,7 +401,7 @@ func (kl *Kubelet) createNetworkContainer(pod *Pod) (DockerID, error) {
 
 // Delete all containers in a pod (except the network container) returns the number of containers deleted
 // and an error if one occurs.
-func (kl *Kubelet) deleteAllContainers(pod *Pod, podFullName string, dockerContainers DockerContainers) (int, error) {
+func (kl *Kubelet) deleteAllContainers(pod *Pod, podFullName string, dockerContainers dockertools.DockerContainers) (int, error) {
 	count := 0
 	errs := make(chan error, len(pod.Manifest.Containers))
 	wg := sync.WaitGroup{}
@@ -436,16 +433,16 @@ func (kl *Kubelet) deleteAllContainers(pod *Pod, podFullName string, dockerConta
 
 type empty struct{}
 
-func (kl *Kubelet) syncPod(pod *Pod, dockerContainers DockerContainers) error {
+func (kl *Kubelet) syncPod(pod *Pod, dockerContainers dockertools.DockerContainers) error {
 	podFullName := GetPodFullName(pod)
 	uuid := pod.Manifest.UUID
-	containersToKeep := make(map[DockerID]empty)
+	containersToKeep := make(map[dockertools.DockerID]empty)
-	killedContainers := make(map[DockerID]empty)
+	killedContainers := make(map[dockertools.DockerID]empty)
 
 	// Make sure we have a network container
-	var netID DockerID
+	var netID dockertools.DockerID
 	if networkDockerContainer, found, _ := dockerContainers.FindPodContainer(podFullName, uuid, networkContainerName); found {
-		netID = DockerID(networkDockerContainer.ID)
+		netID = dockertools.DockerID(networkDockerContainer.ID)
 	} else {
 		glog.Infof("Network container doesn't exist, creating")
 		count, err := kl.deleteAllContainers(pod, podFullName, dockerContainers)
@@ -460,7 +457,7 @@ func (kl *Kubelet) syncPod(pod *Pod, dockerContainers DockerContainers) error {
 		netID = dockerNetworkID
 		if count > 0 {
 			// relist everything, otherwise we'll think we're ok
-			dockerContainers, err = getKubeletDockerContainers(kl.dockerClient)
+			dockerContainers, err = dockertools.GetKubeletDockerContainers(kl.dockerClient)
 			if err != nil {
 				glog.Errorf("Error listing containers %#v", dockerContainers)
 				return err
@@ -487,9 +484,9 @@ func (kl *Kubelet) syncPod(pod *Pod, dockerContainers DockerContainers) error {
 	}
 
 	for _, container := range pod.Manifest.Containers {
-		expectedHash := hashContainer(&container)
+		expectedHash := dockertools.HashContainer(&container)
 		if dockerContainer, found, hash := dockerContainers.FindPodContainer(podFullName, uuid, container.Name); found {
-			containerID := DockerID(dockerContainer.ID)
+			containerID := dockertools.DockerID(dockerContainer.ID)
 			glog.V(1).Infof("pod %s container %s exists as %v", podFullName, container.Name, containerID)
 
 			// look for changes in the container.
@@ -517,7 +514,7 @@ func (kl *Kubelet) syncPod(pod *Pod, dockerContainers DockerContainers) error {
 		}
 
 		// Check RestartPolicy for container
-		recentContainers, err := getRecentDockerContainersWithNameAndUUID(kl.dockerClient, podFullName, uuid, container.Name)
+		recentContainers, err := dockertools.GetRecentDockerContainersWithNameAndUUID(kl.dockerClient, podFullName, uuid, container.Name)
 		if err != nil {
 			glog.Errorf("Error listing recent containers with name and uuid:%s--%s--%s", podFullName, uuid, container.Name)
 			// TODO(dawnchen): error handling here?
@@ -556,7 +553,7 @@ func (kl *Kubelet) syncPod(pod *Pod, dockerContainers DockerContainers) error {
 
 	// Kill any containers in this pod which were not identified above (guards against duplicates).
 	for id, container := range dockerContainers {
-		curPodFullName, curUUID, _, _ := parseDockerName(container.Names[0])
+		curPodFullName, curUUID, _, _ := dockertools.ParseDockerName(container.Names[0])
 		if curPodFullName == podFullName && curUUID == uuid {
 			// Don't kill containers we want to keep or those we already killed.
 			_, keep := containersToKeep[id]
@@ -618,7 +615,7 @@ func (kl *Kubelet) SyncPods(pods []Pod) error {
 	var err error
 	desiredContainers := make(map[podContainer]empty)
 
-	dockerContainers, err := getKubeletDockerContainers(kl.dockerClient)
+	dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient)
 	if err != nil {
 		glog.Errorf("Error listing containers %#v", dockerContainers)
 		return err
@@ -646,14 +643,14 @@ func (kl *Kubelet) SyncPods(pods []Pod) error {
 	}
 
 	// Kill any containers we don't need
-	existingContainers, err := getKubeletDockerContainers(kl.dockerClient)
+	existingContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient)
 	if err != nil {
 		glog.Errorf("Error listing containers: %v", err)
 		return err
 	}
 	for _, container := range existingContainers {
 		// Don't kill containers that are in the desired pods.
-		podFullName, uuid, containerName, _ := parseDockerName(container.Names[0])
+		podFullName, uuid, containerName, _ := dockertools.ParseDockerName(container.Names[0])
 		if _, ok := desiredContainers[podContainer{podFullName, uuid, containerName}]; !ok {
 			err = kl.killContainer(container)
 			if err != nil {
@@ -745,7 +742,7 @@ func (kl *Kubelet) statsFromContainerPath(containerPath string, req *info.Contai
 
 // GetPodInfo returns information from Docker about the containers in a pod
 func (kl *Kubelet) GetPodInfo(podFullName, uuid string) (api.PodInfo, error) {
-	return getDockerPodInfo(kl.dockerClient, podFullName, uuid)
+	return dockertools.GetDockerPodInfo(kl.dockerClient, podFullName, uuid)
 }
 
 // GetContainerInfo returns stats (from Cadvisor) for a container.
@@ -753,7 +750,7 @@ func (kl *Kubelet) GetContainerInfo(podFullName, uuid, containerName string, req
 	if kl.cadvisorClient == nil {
 		return nil, nil
 	}
-	dockerContainers, err := getKubeletDockerContainers(kl.dockerClient)
+	dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient)
 	if err != nil {
 		return nil, err
 	}
@@ -798,7 +795,7 @@ func (kl *Kubelet) RunInContainer(podFullName, uuid, container string, cmd []str
 	if kl.runner == nil {
 		return nil, fmt.Errorf("no runner specified.")
 	}
-	dockerContainers, err := getKubeletDockerContainers(kl.dockerClient)
+	dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient)
 	if err != nil {
 		return nil, err
 	}

pkg/kubelet/kubelet_test.go

@@ -19,7 +19,6 @@ package kubelet
 import (
 	"encoding/json"
 	"fmt"
-	"hash/adler32"
 	"net/http"
 	"reflect"
 	"regexp"
@@ -30,6 +29,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/health"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/volume"
@@ -38,25 +38,24 @@ import (
 	"github.com/stretchr/testify/mock"
 )
 
-func newTestKubelet(t *testing.T) (*Kubelet, *tools.FakeEtcdClient, *FakeDockerClient) {
+func newTestKubelet(t *testing.T) (*Kubelet, *tools.FakeEtcdClient, *dockertools.FakeDockerClient) {
 	fakeEtcdClient := tools.NewFakeEtcdClient(t)
-	fakeDocker := &FakeDockerClient{
+	fakeDocker := &dockertools.FakeDockerClient{}
-		err: nil,
-	}
 
 	kubelet := &Kubelet{}
 	kubelet.dockerClient = fakeDocker
-	kubelet.dockerPuller = &FakeDockerPuller{}
+	kubelet.dockerPuller = &dockertools.FakeDockerPuller{}
 	kubelet.etcdClient = fakeEtcdClient
 	kubelet.rootDirectory = "/tmp/kubelet"
 	kubelet.podWorkers = newPodWorkers()
 	return kubelet, fakeEtcdClient, fakeDocker
 }
 
-func verifyCalls(t *testing.T, fakeDocker *FakeDockerClient, calls []string) {
+func verifyCalls(t *testing.T, fakeDocker *dockertools.FakeDockerClient, calls []string) {
-	fakeDocker.lock.Lock()
+	err := fakeDocker.AssertCalls(calls)
-	defer fakeDocker.lock.Unlock()
+	if err != nil {
-	verifyStringArrayEquals(t, fakeDocker.called, calls)
+		t.Error(err)
+	}
 }
 
 func verifyStringArrayEquals(t *testing.T, actual, expected []string) {
@@ -73,89 +72,16 @@ func verifyStringArrayEquals(t *testing.T, actual, expected []string) {
 	}
 }
 
-func verifyPackUnpack(t *testing.T, podNamespace, podName, containerName string) {
-	container := &api.Container{Name: containerName}
-	hasher := adler32.New()
-	data := fmt.Sprintf("%#v", *container)
-	hasher.Write([]byte(data))
-	computedHash := uint64(hasher.Sum32())
-	name := buildDockerName(
-		&Pod{Name: podName, Namespace: podNamespace},
-		container,
-	)
-	podFullName := fmt.Sprintf("%s.%s", podName, podNamespace)
-	returnedPodFullName, _, returnedContainerName, hash := parseDockerName(name)
-	if podFullName != returnedPodFullName || containerName != returnedContainerName || computedHash != hash {
-		t.Errorf("For (%s, %s, %d), unpacked (%s, %s, %d)", podFullName, containerName, computedHash, returnedPodFullName, returnedContainerName, hash)
-	}
-}
-
 func verifyBoolean(t *testing.T, expected, value bool) {
 	if expected != value {
 		t.Errorf("Unexpected boolean.  Expected %t.  Found %t", expected, value)
 	}
 }
 
-func TestContainerManifestNaming(t *testing.T) {
-	verifyPackUnpack(t, "file", "manifest1234", "container5678")
-	verifyPackUnpack(t, "file", "manifest--", "container__")
-	verifyPackUnpack(t, "file", "--manifest", "__container")
-	verifyPackUnpack(t, "", "m___anifest_", "container-_-")
-	verifyPackUnpack(t, "other", "_m___anifest", "-_-container")
-
-	container := &api.Container{Name: "container"}
-	pod := &Pod{Name: "foo", Namespace: "test"}
-	name := fmt.Sprintf("k8s--%s--%s.%s--12345", container.Name, pod.Name, pod.Namespace)
-
-	podFullName := fmt.Sprintf("%s.%s", pod.Name, pod.Namespace)
-	returnedPodFullName, _, returnedContainerName, hash := parseDockerName(name)
-	if returnedPodFullName != podFullName || returnedContainerName != container.Name || hash != 0 {
-		t.Errorf("unexpected parse: %s %s %d", returnedPodFullName, returnedContainerName, hash)
-	}
-
-}
-
-func TestGetContainerID(t *testing.T) {
-	_, _, fakeDocker := newTestKubelet(t)
-	fakeDocker.containerList = []docker.APIContainers{
-		{
-			ID:    "foobar",
-			Names: []string{"/k8s--foo--qux--1234"},
-		},
-		{
-			ID:    "barbar",
-			Names: []string{"/k8s--bar--qux--2565"},
-		},
-	}
-	fakeDocker.container = &docker.Container{
-		ID: "foobar",
-	}
-
-	dockerContainers, err := getKubeletDockerContainers(fakeDocker)
-	if err != nil {
-		t.Errorf("Expected no error, Got %#v", err)
-	}
-	if len(dockerContainers) != 2 {
-		t.Errorf("Expected %#v, Got %#v", fakeDocker.containerList, dockerContainers)
-	}
-	verifyCalls(t, fakeDocker, []string{"list"})
-	dockerContainer, found, _ := dockerContainers.FindPodContainer("qux", "", "foo")
-	if dockerContainer == nil || !found {
-		t.Errorf("Failed to find container %#v", dockerContainer)
-	}
-
-	fakeDocker.clearCalls()
-	dockerContainer, found, _ = dockerContainers.FindPodContainer("foobar", "", "foo")
-	verifyCalls(t, fakeDocker, []string{})
-	if dockerContainer != nil || found {
-		t.Errorf("Should not have found container %#v", dockerContainer)
-	}
-}
-
 func TestKillContainerWithError(t *testing.T) {
-	fakeDocker := &FakeDockerClient{
+	fakeDocker := &dockertools.FakeDockerClient{
-		err: fmt.Errorf("sample error"),
+		Err: fmt.Errorf("sample error"),
-		containerList: []docker.APIContainers{
+		ContainerList: []docker.APIContainers{
 			{
 				ID:    "1234",
 				Names: []string{"/k8s--foo--qux--1234"},
@@ -168,7 +94,7 @@ func TestKillContainerWithError(t *testing.T) {
 	}
 	kubelet, _, _ := newTestKubelet(t)
 	kubelet.dockerClient = fakeDocker
-	err := kubelet.killContainer(&fakeDocker.containerList[0])
+	err := kubelet.killContainer(&fakeDocker.ContainerList[0])
 	if err == nil {
 		t.Errorf("expected error, found nil")
 	}
@@ -177,7 +103,7 @@ func TestKillContainerWithError(t *testing.T) {
 
 func TestKillContainer(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			ID:    "1234",
 			Names: []string{"/k8s--foo--qux--1234"},
@@ -187,11 +113,11 @@ func TestKillContainer(t *testing.T) {
 			Names: []string{"/k8s--bar--qux--5678"},
 		},
 	}
-	fakeDocker.container = &docker.Container{
+	fakeDocker.Container = &docker.Container{
 		ID: "foobar",
 	}
 
-	err := kubelet.killContainer(&fakeDocker.containerList[0])
+	err := kubelet.killContainer(&fakeDocker.ContainerList[0])
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -227,10 +153,10 @@ func (cr *channelReader) GetList() [][]Pod {
 func TestSyncPodsDoesNothing(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
 	container := api.Container{Name: "bar"}
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			// format is k8s--<container-id>--<pod-fullname>
-			Names: []string{"/k8s--bar." + strconv.FormatUint(hashContainer(&container), 16) + "--foo.test"},
+			Names: []string{"/k8s--bar." + strconv.FormatUint(dockertools.HashContainer(&container), 16) + "--foo.test"},
 			ID:    "1234",
 		},
 		{
@@ -281,7 +207,7 @@ func matchString(t *testing.T, pattern, str string) bool {
 
 func TestSyncPodsCreatesNetAndContainer(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
-	fakeDocker.containerList = []docker.APIContainers{}
+	fakeDocker.ContainerList = []docker.APIContainers{}
 	err := kubelet.SyncPods([]Pod{
 		{
 			Name:      "foo",
@@ -302,18 +228,18 @@ func TestSyncPodsCreatesNetAndContainer(t *testing.T) {
 	verifyCalls(t, fakeDocker, []string{
 		"list", "list", "create", "start", "list", "inspect", "list", "create", "start"})
 
-	fakeDocker.lock.Lock()
+	fakeDocker.Lock()
 	if len(fakeDocker.Created) != 2 ||
 		!matchString(t, "k8s--net\\.[a-f0-9]+--foo.test--", fakeDocker.Created[0]) ||
 		!matchString(t, "k8s--bar\\.[a-f0-9]+--foo.test--", fakeDocker.Created[1]) {
 		t.Errorf("Unexpected containers created %v", fakeDocker.Created)
 	}
-	fakeDocker.lock.Unlock()
+	fakeDocker.Unlock()
 }
 
 func TestSyncPodsWithNetCreatesContainer(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			// network container
 			Names: []string{"/k8s--net--foo.test--"},
@@ -340,19 +266,19 @@ func TestSyncPodsWithNetCreatesContainer(t *testing.T) {
 	verifyCalls(t, fakeDocker, []string{
 		"list", "list", "list", "inspect", "list", "create", "start"})
 
-	fakeDocker.lock.Lock()
+	fakeDocker.Lock()
 	if len(fakeDocker.Created) != 1 ||
 		!matchString(t, "k8s--bar\\.[a-f0-9]+--foo.test--", fakeDocker.Created[0]) {
 		t.Errorf("Unexpected containers created %v", fakeDocker.Created)
 	}
-	fakeDocker.lock.Unlock()
+	fakeDocker.Unlock()
 }
 
 func TestSyncPodsWithNetCreatesContainerCallsHandler(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
 	fakeHttp := fakeHTTP{}
 	kubelet.httpClient = &fakeHttp
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			// network container
 			Names: []string{"/k8s--net--foo.test--"},
@@ -390,12 +316,12 @@ func TestSyncPodsWithNetCreatesContainerCallsHandler(t *testing.T) {
 	verifyCalls(t, fakeDocker, []string{
 		"list", "list", "list", "inspect", "list", "create", "start"})
 
-	fakeDocker.lock.Lock()
+	fakeDocker.Lock()
 	if len(fakeDocker.Created) != 1 ||
 		!matchString(t, "k8s--bar\\.[a-f0-9]+--foo.test--", fakeDocker.Created[0]) {
 		t.Errorf("Unexpected containers created %v", fakeDocker.Created)
 	}
-	fakeDocker.lock.Unlock()
+	fakeDocker.Unlock()
 	if fakeHttp.url != "http://foo:8080/bar" {
 		t.Errorf("Unexpected handler: %s", fakeHttp.url)
 	}
@@ -403,7 +329,7 @@ func TestSyncPodsWithNetCreatesContainerCallsHandler(t *testing.T) {
 
 func TestSyncPodsDeletesWithNoNetContainer(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			// format is k8s--<container-id>--<pod-fullname>
 			Names: []string{"/k8s--bar--foo.test"},
@@ -435,16 +361,16 @@ func TestSyncPodsDeletesWithNoNetContainer(t *testing.T) {
 	expectedToStop := map[string]bool{
 		"1234": true,
 	}
-	fakeDocker.lock.Lock()
+	fakeDocker.Lock()
-	if len(fakeDocker.stopped) != 1 || !expectedToStop[fakeDocker.stopped[0]] {
+	if len(fakeDocker.Stopped) != 1 || !expectedToStop[fakeDocker.Stopped[0]] {
-		t.Errorf("Wrong containers were stopped: %v", fakeDocker.stopped)
+		t.Errorf("Wrong containers were stopped: %v", fakeDocker.Stopped)
 	}
-	fakeDocker.lock.Unlock()
+	fakeDocker.Unlock()
 }
 
 func TestSyncPodsDeletes(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			// the k8s prefix is required for the kubelet to manage the container
 			Names: []string{"/k8s--foo--bar.test"},
@@ -473,16 +399,16 @@ func TestSyncPodsDeletes(t *testing.T) {
 		"1234": true,
 		"9876": true,
 	}
-	if len(fakeDocker.stopped) != 2 ||
+	if len(fakeDocker.Stopped) != 2 ||
-		!expectedToStop[fakeDocker.stopped[0]] ||
+		!expectedToStop[fakeDocker.Stopped[0]] ||
-		!expectedToStop[fakeDocker.stopped[1]] {
+		!expectedToStop[fakeDocker.Stopped[1]] {
-		t.Errorf("Wrong containers were stopped: %v", fakeDocker.stopped)
+		t.Errorf("Wrong containers were stopped: %v", fakeDocker.Stopped)
 	}
 }
 
 func TestSyncPodDeletesDuplicate(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
-	dockerContainers := DockerContainers{
+	dockerContainers := dockertools.DockerContainers{
 		"1234": &docker.APIContainers{
 			// the k8s prefix is required for the kubelet to manage the container
 			Names: []string{"/k8s--foo--bar.test--1"},
@@ -521,8 +447,8 @@ func TestSyncPodDeletesDuplicate(t *testing.T) {
 	verifyCalls(t, fakeDocker, []string{"list", "stop"})
 
 	// Expect one of the duplicates to be killed.
-	if len(fakeDocker.stopped) != 1 || (len(fakeDocker.stopped) != 0 && fakeDocker.stopped[0] != "1234" && fakeDocker.stopped[0] != "4567") {
+	if len(fakeDocker.Stopped) != 1 || (len(fakeDocker.Stopped) != 0 && fakeDocker.Stopped[0] != "1234" && fakeDocker.Stopped[0] != "4567") {
-		t.Errorf("Wrong containers were stopped: %v", fakeDocker.stopped)
+		t.Errorf("Wrong containers were stopped: %v", fakeDocker.Stopped)
 	}
 }
 
@@ -535,7 +461,7 @@ func (f *FalseHealthChecker) HealthCheck(podFullName string, state api.PodState,
 func TestSyncPodBadHash(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
 	kubelet.healthChecker = &FalseHealthChecker{}
-	dockerContainers := DockerContainers{
+	dockerContainers := dockertools.DockerContainers{
 		"1234": &docker.APIContainers{
 			// the k8s prefix is required for the kubelet to manage the container
 			Names: []string{"/k8s--bar.1234--foo.test"},
@@ -568,16 +494,16 @@ func TestSyncPodBadHash(t *testing.T) {
 	expectedToStop := map[string]bool{
 		"1234": true,
 	}
-	if len(fakeDocker.stopped) != 1 ||
+	if len(fakeDocker.Stopped) != 1 ||
-		!expectedToStop[fakeDocker.stopped[0]] {
+		!expectedToStop[fakeDocker.Stopped[0]] {
-		t.Errorf("Wrong containers were stopped: %v", fakeDocker.stopped)
+		t.Errorf("Wrong containers were stopped: %v", fakeDocker.Stopped)
 	}
 }
 
 func TestSyncPodUnhealthy(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
 	kubelet.healthChecker = &FalseHealthChecker{}
-	dockerContainers := DockerContainers{
+	dockerContainers := dockertools.DockerContainers{
 		"1234": &docker.APIContainers{
 			// the k8s prefix is required for the kubelet to manage the container
 			Names: []string{"/k8s--bar--foo.test"},
@@ -615,9 +541,9 @@ func TestSyncPodUnhealthy(t *testing.T) {
 	expectedToStop := map[string]bool{
 		"1234": true,
 	}
-	if len(fakeDocker.stopped) != 1 ||
+	if len(fakeDocker.Stopped) != 1 ||
-		!expectedToStop[fakeDocker.stopped[0]] {
+		!expectedToStop[fakeDocker.Stopped[0]] {
-		t.Errorf("Wrong containers were stopped: %v", fakeDocker.stopped)
+		t.Errorf("Wrong containers were stopped: %v", fakeDocker.Stopped)
 	}
 }
 
@@ -934,7 +860,7 @@ func TestGetContainerInfo(t *testing.T) {
 
 	kubelet, _, fakeDocker := newTestKubelet(t)
 	kubelet.cadvisorClient = mockCadvisor
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			ID: containerID,
 			// pod id: qux
@@ -958,7 +884,7 @@ func TestGetContainerInfo(t *testing.T) {
 	mockCadvisor.AssertExpectations(t)
 }
 
-func TestGetRooInfo(t *testing.T) {
+func TestGetRootInfo(t *testing.T) {
 	containerPath := "/"
 	containerInfo := &info.ContainerInfo{
 		ContainerReference: info.ContainerReference{
@@ -973,9 +899,7 @@ func TestGetRooInfo(t *testing.T) {
 			},
 		},
 	}
-	fakeDocker := FakeDockerClient{
+	fakeDocker := dockertools.FakeDockerClient{}
-		err: nil,
-	}
 
 	mockCadvisor := &mockCadvisorClient{}
 	req := &info.ContainerInfoRequest{}
@@ -984,7 +908,7 @@ func TestGetRooInfo(t *testing.T) {
 
 	kubelet := Kubelet{
 		dockerClient:   &fakeDocker,
-		dockerPuller:   &FakeDockerPuller{},
+		dockerPuller:   &dockertools.FakeDockerPuller{},
 		cadvisorClient: mockCadvisor,
 		podWorkers:     newPodWorkers(),
 	}
@@ -1004,7 +928,7 @@ func TestGetRooInfo(t *testing.T) {
 
 func TestGetContainerInfoWithoutCadvisor(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			ID: "foobar",
 			// pod id: qux
@@ -1042,7 +966,7 @@ func TestGetContainerInfoWhenCadvisorFailed(t *testing.T) {
 
 	kubelet, _, fakeDocker := newTestKubelet(t)
 	kubelet.cadvisorClient = mockCadvisor
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			ID: containerID,
 			// pod id: qux
@@ -1070,7 +994,7 @@ func TestGetContainerInfoOnNonExistContainer(t *testing.T) {
 
 	kubelet, _, fakeDocker := newTestKubelet(t)
 	kubelet.cadvisorClient = mockCadvisor
-	fakeDocker.containerList = []docker.APIContainers{}
+	fakeDocker.ContainerList = []docker.APIContainers{}
 
 	stats, _ := kubelet.GetContainerInfo("qux", "", "foo", nil)
 	if stats != nil {
@@ -1094,7 +1018,7 @@ func (f *fakeContainerCommandRunner) RunInContainer(id string, cmd []string) ([]
 func TestRunInContainerNoSuchPod(t *testing.T) {
 	fakeCommandRunner := fakeContainerCommandRunner{}
 	kubelet, _, fakeDocker := newTestKubelet(t)
-	fakeDocker.containerList = []docker.APIContainers{}
+	fakeDocker.ContainerList = []docker.APIContainers{}
 	kubelet.runner = &fakeCommandRunner
 
 	podName := "podFoo"
@@ -1123,7 +1047,7 @@ func TestRunInContainer(t *testing.T) {
 	podNamespace := "etcd"
 	containerName := "containerFoo"
 
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			ID:    containerID,
 			Names: []string{"/k8s--" + containerName + "--" + podName + "." + podNamespace + "--1234"},
@@ -1147,43 +1071,6 @@ func TestRunInContainer(t *testing.T) {
 	}
 }
 
-func TestDockerContainerCommand(t *testing.T) {
-	runner := dockerContainerCommandRunner{}
-	containerID := "1234"
-	command := []string{"ls"}
-	cmd, _ := runner.getRunInContainerCommand(containerID, command)
-	if cmd.Dir != "/var/lib/docker/execdriver/native/"+containerID {
-		t.Errorf("unexpected command CWD: %s", cmd.Dir)
-	}
-	if !reflect.DeepEqual(cmd.Args, []string{"/usr/sbin/nsinit", "exec", "ls"}) {
-		t.Errorf("unexpectd command args: %s", cmd.Args)
-	}
-
-}
-
-var parseImageNameTests = []struct {
-	imageName string
-	name      string
-	tag       string
-}{
-	{"ubuntu", "ubuntu", ""},
-	{"ubuntu:2342", "ubuntu", "2342"},
-	{"ubuntu:latest", "ubuntu", "latest"},
-	{"foo/bar:445566", "foo/bar", "445566"},
-	{"registry.example.com:5000/foobar", "registry.example.com:5000/foobar", ""},
-	{"registry.example.com:5000/foobar:5342", "registry.example.com:5000/foobar", "5342"},
-	{"registry.example.com:5000/foobar:latest", "registry.example.com:5000/foobar", "latest"},
-}
-
-func TestParseImageName(t *testing.T) {
-	for _, tt := range parseImageNameTests {
-		name, tag := parseImageName(tt.imageName)
-		if name != tt.name || tag != tt.tag {
-			t.Errorf("Expected name/tag: %s/%s, got %s/%s", tt.name, tt.tag, name, tag)
-		}
-	}
-}
-
 func TestRunHandlerExec(t *testing.T) {
 	fakeCommandRunner := fakeContainerCommandRunner{}
 	kubelet, _, fakeDocker := newTestKubelet(t)
@@ -1194,7 +1081,7 @@ func TestRunHandlerExec(t *testing.T) {
 	podNamespace := "etcd"
 	containerName := "containerFoo"
 
-	fakeDocker.containerList = []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			ID:    containerID,
 			Names: []string{"/k8s--" + containerName + "--" + podName + "." + podNamespace + "--1234"},
@@ -1298,7 +1185,7 @@ func TestSyncPodEventHandlerFails(t *testing.T) {
 	kubelet.httpClient = &fakeHTTP{
 		err: fmt.Errorf("test error"),
 	}
-	dockerContainers := DockerContainers{
+	dockerContainers := dockertools.DockerContainers{
 		"9876": &docker.APIContainers{
 			// network container
 			Names: []string{"/k8s--net--foo.test--"},
@@ -1331,7 +1218,7 @@ func TestSyncPodEventHandlerFails(t *testing.T) {
 
 	verifyCalls(t, fakeDocker, []string{"list", "list", "create", "start", "stop"})
 
-	if len(fakeDocker.stopped) != 1 {
+	if len(fakeDocker.Stopped) != 1 {
-		t.Errorf("Wrong containers were stopped: %v", fakeDocker.stopped)
+		t.Errorf("Wrong containers were stopped: %v", fakeDocker.Stopped)
 	}
 }

pkg/kubelet/server.go

@@ -33,6 +33,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/healthz"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/httplog"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"github.com/golang/glog"
 	"github.com/google/cadvisor/info"
 	"gopkg.in/v1/yaml"
@@ -159,7 +160,7 @@ func (s *Server) handlePodInfo(w http.ResponseWriter, req *http.Request) {
 	// TODO: backwards compatibility with existing API, needs API change
 	podFullName := GetPodFullName(&Pod{Name: podID, Namespace: "etcd"})
 	info, err := s.host.GetPodInfo(podFullName, podUUID)
-	if err == ErrNoContainersInPod {
+	if err == dockertools.ErrNoContainersInPod {
 		http.Error(w, "Pod does not exist", http.StatusNotFound)
 		return
 	}