diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index d0b92497e1..4f3dc26ea7 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -169,7 +169,10 @@ func GetKubeletDockerContainers(client DockerInterface) (DockerContainers, error container := &containers[i] // Skip containers that we didn't create to allow users to manually // spin up their own containers if they want. - if !strings.HasPrefix(container.Names[0], "/"+containerNamePrefix+"--") { + // TODO(dchen1107): Remove the old separator "--" by end of Oct + if !strings.HasPrefix(container.Names[0], "/"+containerNamePrefix+"_") && + !strings.HasPrefix(container.Names[0], "/"+containerNamePrefix+"--") { + glog.Infof("Docker Container:%s is not managed by kubelet.", container.Names[0]) continue } result[DockerID(container.ID)] = container @@ -287,20 +290,6 @@ func GetDockerPodInfo(client DockerInterface, podFullName, uuid string) (api.Pod return info, nil } -// Converts "-" to "_-_" and "_" to "___" so that we can use "--" to meaningfully separate parts of a docker name. -func escapeDash(in string) (out string) { - out = strings.Replace(in, "_", "___", -1) - out = strings.Replace(out, "-", "_-_", -1) - return -} - -// Reverses the transformation of escapeDash. -func unescapeDash(in string) (out string) { - out = strings.Replace(in, "_-_", "-", -1) - out = strings.Replace(out, "___", "_", -1) - return -} - const containerNamePrefix = "k8s" func HashContainer(container *api.Container) uint64 { @@ -311,20 +300,20 @@ func HashContainer(container *api.Container) uint64 { // Creates a name which can be reversed to identify both full pod name and container name. func BuildDockerName(manifestUUID, podFullName string, container *api.Container) string { - containerName := escapeDash(container.Name) + "." + strconv.FormatUint(HashContainer(container), 16) + containerName := container.Name + "." + strconv.FormatUint(HashContainer(container), 16) // Note, manifest.ID could be blank. if len(manifestUUID) == 0 { - return fmt.Sprintf("%s--%s--%s--%08x", + return fmt.Sprintf("%s_%s_%s_%08x", containerNamePrefix, containerName, - escapeDash(podFullName), + podFullName, rand.Uint32()) } else { - return fmt.Sprintf("%s--%s--%s--%s--%08x", + return fmt.Sprintf("%s_%s_%s_%s_%08x", containerNamePrefix, containerName, - escapeDash(podFullName), - escapeDash(manifestUUID), + podFullName, + manifestUUID, rand.Uint32()) } } @@ -337,13 +326,13 @@ func ParseDockerName(name string) (podFullName, uuid, containerName string, hash if name[0] == '/' { name = name[1:] } - parts := strings.Split(name, "--") + parts := strings.Split(name, "_") if len(parts) == 0 || parts[0] != containerNamePrefix { return } if len(parts) > 1 { pieces := strings.Split(parts[1], ".") - containerName = unescapeDash(pieces[0]) + containerName = pieces[0] if len(pieces) > 1 { var err error hash, err = strconv.ParseUint(pieces[1], 16, 32) @@ -353,10 +342,10 @@ func ParseDockerName(name string) (podFullName, uuid, containerName string, hash } } if len(parts) > 2 { - podFullName = unescapeDash(parts[2]) + podFullName = parts[2] } if len(parts) > 4 { - uuid = unescapeDash(parts[3]) + uuid = parts[3] } return } diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 6630549275..717c41f023 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -51,11 +51,11 @@ func TestGetContainerID(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { ID: "foobar", - Names: []string{"/k8s--foo--qux--1234"}, + Names: []string{"/k8s_foo_qux_1234"}, }, { ID: "barbar", - Names: []string{"/k8s--bar--qux--2565"}, + Names: []string{"/k8s_bar_qux_2565"}, }, } fakeDocker.Container = &docker.Container{ @@ -83,31 +83,37 @@ func TestGetContainerID(t *testing.T) { } } -func verifyPackUnpack(t *testing.T, podNamespace, podName, containerName string) { +func verifyPackUnpack(t *testing.T, podNamespace, manifestUUID, 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) + name := BuildDockerName(manifestUUID, podFullName, container) + returnedPodFullName, returnedUUID, returnedContainerName, hash := ParseDockerName(name) + if podFullName != returnedPodFullName || manifestUUID != returnedUUID || containerName != returnedContainerName || computedHash != hash { + t.Errorf("For (%s, %s, %s, %d), unpacked (%s, %s, %s, %d)", podFullName, manifestUUID, containerName, computedHash, returnedPodFullName, returnedUUID, 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") + manifestUUID := "d1b925c9-444a-11e4-a576-42010af0a203" + verifyPackUnpack(t, "file", manifestUUID, "manifest1234", "container5678") + verifyPackUnpack(t, "file", manifestUUID, "mani-fest-1234", "container5678") + // UUID is same as pod name + verifyPackUnpack(t, "file", manifestUUID, manifestUUID, "container123") + // empty namespace + verifyPackUnpack(t, "", manifestUUID, manifestUUID, "container123") + // No UUID + verifyPackUnpack(t, "other", "", manifestUUID, "container456") + // No Container name + verifyPackUnpack(t, "other", "", manifestUUID, "") container := &api.Container{Name: "container"} podName := "foo" podNamespace := "test" - name := fmt.Sprintf("k8s--%s--%s.%s--12345", container.Name, podName, podNamespace) + name := fmt.Sprintf("k8s_%s_%s.%s_12345", container.Name, podName, podNamespace) podFullName := fmt.Sprintf("%s.%s", podName, podNamespace) returnedPodFullName, _, returnedContainerName, hash := ParseDockerName(name) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 74c0b10cc1..5509bf7b62 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -83,11 +83,11 @@ func TestKillContainerWithError(t *testing.T) { ContainerList: []docker.APIContainers{ { ID: "1234", - Names: []string{"/k8s--foo--qux--1234"}, + Names: []string{"/k8s_foo_qux_1234"}, }, { ID: "5678", - Names: []string{"/k8s--bar--qux--5678"}, + Names: []string{"/k8s_bar_qux_5678"}, }, }, } @@ -105,11 +105,11 @@ func TestKillContainer(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { ID: "1234", - Names: []string{"/k8s--foo--qux--1234"}, + Names: []string{"/k8s_foo_qux_1234"}, }, { ID: "5678", - Names: []string{"/k8s--bar--qux--5678"}, + Names: []string{"/k8s_bar_qux_5678"}, }, } fakeDocker.Container = &docker.Container{ @@ -154,13 +154,13 @@ func TestSyncPodsDoesNothing(t *testing.T) { container := api.Container{Name: "bar"} fakeDocker.ContainerList = []docker.APIContainers{ { - // format is k8s---- - Names: []string{"/k8s--bar." + strconv.FormatUint(dockertools.HashContainer(&container), 16) + "--foo.test"}, + // format is k8s__ + Names: []string{"/k8s_bar." + strconv.FormatUint(dockertools.HashContainer(&container), 16) + "_foo.test"}, ID: "1234", }, { // network container - Names: []string{"/k8s--net--foo.test--"}, + Names: []string{"/k8s_net_foo.test_"}, ID: "9876", }, } @@ -229,8 +229,8 @@ func TestSyncPodsCreatesNetAndContainer(t *testing.T) { 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]) { + !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.Unlock() @@ -241,7 +241,7 @@ func TestSyncPodsWithNetCreatesContainer(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { // network container - Names: []string{"/k8s--net--foo.test--"}, + Names: []string{"/k8s_net_foo.test_"}, ID: "9876", }, } @@ -267,7 +267,7 @@ func TestSyncPodsWithNetCreatesContainer(t *testing.T) { fakeDocker.Lock() if len(fakeDocker.Created) != 1 || - !matchString(t, "k8s--bar\\.[a-f0-9]+--foo.test--", fakeDocker.Created[0]) { + !matchString(t, "k8s_bar\\.[a-f0-9]+_foo.test_", fakeDocker.Created[0]) { t.Errorf("Unexpected containers created %v", fakeDocker.Created) } fakeDocker.Unlock() @@ -280,7 +280,7 @@ func TestSyncPodsWithNetCreatesContainerCallsHandler(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { // network container - Names: []string{"/k8s--net--foo.test--"}, + Names: []string{"/k8s_net_foo.test_"}, ID: "9876", }, } @@ -317,7 +317,7 @@ func TestSyncPodsWithNetCreatesContainerCallsHandler(t *testing.T) { fakeDocker.Lock() if len(fakeDocker.Created) != 1 || - !matchString(t, "k8s--bar\\.[a-f0-9]+--foo.test--", fakeDocker.Created[0]) { + !matchString(t, "k8s_bar\\.[a-f0-9]+_foo.test_", fakeDocker.Created[0]) { t.Errorf("Unexpected containers created %v", fakeDocker.Created) } fakeDocker.Unlock() @@ -330,8 +330,8 @@ func TestSyncPodsDeletesWithNoNetContainer(t *testing.T) { kubelet, _, fakeDocker := newTestKubelet(t) fakeDocker.ContainerList = []docker.APIContainers{ { - // format is k8s---- - Names: []string{"/k8s--bar--foo.test"}, + // format is k8s__ + Names: []string{"/k8s_bar_foo.test"}, ID: "1234", }, } @@ -372,12 +372,12 @@ func TestSyncPodsDeletes(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { // the k8s prefix is required for the kubelet to manage the container - Names: []string{"/k8s--foo--bar.test"}, + Names: []string{"/k8s_foo_bar.test"}, ID: "1234", }, { // network container - Names: []string{"/k8s--net--foo.test--"}, + Names: []string{"/k8s_net_foo.test_"}, ID: "9876", }, { @@ -410,22 +410,22 @@ func TestSyncPodDeletesDuplicate(t *testing.T) { 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"}, + Names: []string{"/k8s_foo_bar.test_1"}, ID: "1234", }, "9876": &docker.APIContainers{ // network container - Names: []string{"/k8s--net--bar.test--"}, + Names: []string{"/k8s_net_bar.test_"}, ID: "9876", }, "4567": &docker.APIContainers{ // Duplicate for the same container. - Names: []string{"/k8s--foo--bar.test--2"}, + Names: []string{"/k8s_foo_bar.test_2"}, ID: "4567", }, "2304": &docker.APIContainers{ // Container for another pod, untouched. - Names: []string{"/k8s--baz--fiz.test--6"}, + Names: []string{"/k8s_baz_fiz.test_6"}, ID: "2304", }, } @@ -463,12 +463,12 @@ func TestSyncPodBadHash(t *testing.T) { 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"}, + Names: []string{"/k8s_bar.1234_foo.test"}, ID: "1234", }, "9876": &docker.APIContainers{ // network container - Names: []string{"/k8s--net--foo.test--"}, + Names: []string{"/k8s_net_foo.test_"}, ID: "9876", }, } @@ -505,12 +505,12 @@ func TestSyncPodUnhealthy(t *testing.T) { dockerContainers := dockertools.DockerContainers{ "1234": &docker.APIContainers{ // the k8s prefix is required for the kubelet to manage the container - Names: []string{"/k8s--bar--foo.test"}, + Names: []string{"/k8s_bar_foo.test"}, ID: "1234", }, "9876": &docker.APIContainers{ // network container - Names: []string{"/k8s--net--foo.test--"}, + Names: []string{"/k8s_net_foo.test_"}, ID: "9876", }, } @@ -776,7 +776,7 @@ func TestGetContainerInfo(t *testing.T) { ID: containerID, // pod id: qux // container id: foo - Names: []string{"/k8s--foo--qux--1234"}, + Names: []string{"/k8s_foo_qux_1234"}, }, } @@ -826,7 +826,7 @@ func TestGetContainerInfoWithoutCadvisor(t *testing.T) { ID: "foobar", // pod id: qux // container id: foo - Names: []string{"/k8s--foo--qux--uuid--1234"}, + Names: []string{"/k8s_foo_qux_uuid_1234"}, }, } @@ -855,7 +855,7 @@ func TestGetContainerInfoWhenCadvisorFailed(t *testing.T) { ID: containerID, // pod id: qux // container id: foo - Names: []string{"/k8s--foo--qux--uuid--1234"}, + Names: []string{"/k8s_foo_qux_uuid_1234"}, }, } @@ -934,7 +934,7 @@ func TestRunInContainer(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { ID: containerID, - Names: []string{"/k8s--" + containerName + "--" + podName + "." + podNamespace + "--1234"}, + Names: []string{"/k8s_" + containerName + "_" + podName + "." + podNamespace + "_1234"}, }, } @@ -968,7 +968,7 @@ func TestRunHandlerExec(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { ID: containerID, - Names: []string{"/k8s--" + containerName + "--" + podName + "." + podNamespace + "--1234"}, + Names: []string{"/k8s_" + containerName + "_" + podName + "." + podNamespace + "_1234"}, }, } @@ -1072,7 +1072,7 @@ func TestSyncPodEventHandlerFails(t *testing.T) { dockerContainers := dockertools.DockerContainers{ "9876": &docker.APIContainers{ // network container - Names: []string{"/k8s--net--foo.test--"}, + Names: []string{"/k8s_net_foo.test_"}, ID: "9876", }, } diff --git a/pkg/kubelet/validation_test.go b/pkg/kubelet/validation_test.go index dfb02fa2f2..499199750b 100644 --- a/pkg/kubelet/validation_test.go +++ b/pkg/kubelet/validation_test.go @@ -33,6 +33,7 @@ func TestValidatePodNoName(t *testing.T) { "zero-length name": {Name: "", Manifest: api.ContainerManifest{Version: "v1beta1"}}, "name > 255 characters": {Name: strings.Repeat("a", 256), Manifest: api.ContainerManifest{Version: "v1beta1"}}, "name not a DNS subdomain": {Name: "a.b.c.", Manifest: api.ContainerManifest{Version: "v1beta1"}}, + "name with underscore": {Name: "a_b_c", Manifest: api.ContainerManifest{Version: "v1beta1"}}, } for k, v := range errorCases { if errs := ValidatePod(&v); len(errs) == 0 {