diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index aa3b702f349..ac98be6e2bb 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -547,26 +547,17 @@ func HashContainer(container *api.Container) uint64 { // Creates a name which can be reversed to identify both full pod name and container name. func BuildDockerName(podUID, podFullName string, container *api.Container) string { containerName := container.Name + "." + strconv.FormatUint(HashContainer(container), 16) - // Note, manifest.ID could be blank. - if len(podUID) == 0 { - return fmt.Sprintf("%s_%s_%s_%08x", - containerNamePrefix, - containerName, - podFullName, - rand.Uint32()) - } else { - return fmt.Sprintf("%s_%s_%s_%s_%08x", - containerNamePrefix, - containerName, - podFullName, - podUID, - rand.Uint32()) - } + return fmt.Sprintf("%s_%s_%s_%s_%08x", + containerNamePrefix, + containerName, + podFullName, + podUID, + rand.Uint32()) } // Unpacks 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 the one we created, we may return empty strings. -func ParseDockerName(name string) (podFullName, uuid, containerName string, hash uint64) { +func ParseDockerName(name string) (podFullName, podUID, containerName string, hash uint64) { // For some reason docker appears to be appending '/' to names. // If it's there, strip it. if name[0] == '/' { @@ -576,29 +567,31 @@ func ParseDockerName(name string) (podFullName, uuid, containerName string, hash if len(parts) == 0 || parts[0] != containerNamePrefix { return } - if len(parts) > 1 { - pieces := strings.Split(parts[1], ".") - containerName = pieces[0] - if len(pieces) > 1 { - var err error - hash, err = strconv.ParseUint(pieces[1], 16, 32) - if err != nil { - glog.Warningf("invalid container hash: %s", pieces[1]) - } + if len(parts) < 5 { + // We have at least 5 fields. We may have more in the future. + // Anything with less fields than this is not something we can + // manage. + glog.Warningf("found a container with the %q prefix, but too few fields (%d): ", containerNamePrefix, len(parts), name) + return + } + + // Container name. + nameParts := strings.Split(parts[1], ".") + containerName = nameParts[0] + if len(nameParts) > 1 { + var err error + hash, err = strconv.ParseUint(nameParts[1], 16, 32) + if err != nil { + glog.Warningf("invalid container hash: %s", nameParts[1]) } } - if len(parts) > 2 { - podFullName = parts[2] - } - // This is not an off-by-one. We check for > 4 here because (sadly) the - // format generated by BuildDockerName() has an optional field in the - // middle. If len(parts) > 3, parts[3] might be the optional field or - // the (poorly documented) random suffix. If len(parts) > 4, then we - // know [3] is the UUID and [4] is the suffix. Sort of pukey, should - // be fixed by making UID non-optional. - if len(parts) > 4 { - uuid = parts[3] - } + + // Pod fullname. + podFullName = parts[2] + + // Pod UID. + podUID = parts[3] + return } diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 29347ace9ba..5a47ec3083c 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -53,11 +53,11 @@ func TestGetContainerID(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { ID: "foobar", - Names: []string{"/k8s_foo_qux_1234"}, + Names: []string{"/k8s_foo_qux_1234_42"}, }, { ID: "barbar", - Names: []string{"/k8s_bar_qux_2565"}, + Names: []string{"/k8s_bar_qux_2565_42"}, }, } fakeDocker.Container = &docker.Container{ @@ -99,27 +99,23 @@ func verifyPackUnpack(t *testing.T, podNamespace, podUID, podName, containerName } func TestContainerManifestNaming(t *testing.T) { - podUID := "d1b925c9-444a-11e4-a576-42010af0a203" - verifyPackUnpack(t, "file", podUID, "manifest1234", "container5678") - verifyPackUnpack(t, "file", podUID, "mani-fest-1234", "container5678") + podUID := "12345678" + verifyPackUnpack(t, "file", podUID, "name", "container") + verifyPackUnpack(t, "file", podUID, "name-with-dashes", "container") // UID is same as pod name - verifyPackUnpack(t, "file", podUID, podUID, "container123") - // empty namespace - verifyPackUnpack(t, "", podUID, podUID, "container123") - // No UID - verifyPackUnpack(t, "other", "", podUID, "container456") + verifyPackUnpack(t, "file", podUID, podUID, "container") // No Container name - verifyPackUnpack(t, "other", "", podUID, "") + verifyPackUnpack(t, "other", podUID, "name", "") 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_%s_42", container.Name, podName, podNamespace, podUID) 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) + + returnedPodFullName, returnedPodUID, returnedContainerName, hash := ParseDockerName(name) + if returnedPodFullName != podFullName || returnedPodUID != podUID || returnedContainerName != container.Name || hash != 0 { + t.Errorf("unexpected parse: %s %s %s %d", returnedPodFullName, returnedPodUID, returnedContainerName, hash) } } diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 16c00f8c30f..5e28360af61 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -220,11 +220,11 @@ func TestKillContainerWithError(t *testing.T) { ContainerList: []docker.APIContainers{ { ID: "1234", - Names: []string{"/k8s_foo_qux_1234"}, + Names: []string{"/k8s_foo_qux_1234_42"}, }, { ID: "5678", - Names: []string{"/k8s_bar_qux_5678"}, + Names: []string{"/k8s_bar_qux_5678_42"}, }, }, } @@ -242,11 +242,11 @@ func TestKillContainer(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { ID: "1234", - Names: []string{"/k8s_foo_qux_1234"}, + Names: []string{"/k8s_foo_qux_1234_42"}, }, { ID: "5678", - Names: []string{"/k8s_bar_qux_5678"}, + Names: []string{"/k8s_bar_qux_5678_42"}, }, } fakeDocker.Container = &docker.Container{ @@ -333,7 +333,7 @@ func TestSyncPodsWithTerminationLog(t *testing.T) { err := kubelet.SyncPods([]api.BoundPod{ { ObjectMeta: api.ObjectMeta{ - UID: "0123-45-67-89ab-cdef", + UID: "12345678", Name: "foo", Namespace: "new", Annotations: map[string]string{ConfigSourceAnnotationKey: "test"}, @@ -354,7 +354,7 @@ func TestSyncPodsWithTerminationLog(t *testing.T) { fakeDocker.Lock() parts := strings.Split(fakeDocker.Container.HostConfig.Binds[0], ":") - if !matchString(t, kubelet.GetPodContainerDir("0123-45-67-89ab-cdef", "bar")+"/k8s_bar\\.[a-f0-9]", parts[0]) { + if !matchString(t, kubelet.GetPodContainerDir("12345678", "bar")+"/k8s_bar\\.[a-f0-9]", parts[0]) { t.Errorf("Unexpected host path: %s", parts[0]) } if parts[1] != "/dev/somepath" { @@ -391,6 +391,7 @@ func TestSyncPodsCreatesNetAndContainer(t *testing.T) { err := kubelet.SyncPods([]api.BoundPod{ { ObjectMeta: api.ObjectMeta{ + UID: "12345678", Name: "foo", Namespace: "new", Annotations: map[string]string{ConfigSourceAnnotationKey: "test"}, @@ -439,6 +440,7 @@ func TestSyncPodsCreatesNetAndContainerPullsImage(t *testing.T) { err := kubelet.SyncPods([]api.BoundPod{ { ObjectMeta: api.ObjectMeta{ + UID: "12345678", Name: "foo", Namespace: "new", Annotations: map[string]string{ConfigSourceAnnotationKey: "test"}, @@ -620,12 +622,12 @@ func TestSyncPodsDeletesWhenSourcesAreReady(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { // the k8s prefix is required for the kubelet to manage the container - Names: []string{"/k8s_foo_bar.new.test"}, + Names: []string{"/k8s_foo_bar.new.test_12345678_42"}, ID: "1234", }, { // network container - Names: []string{"/k8s_net_foo.new.test_"}, + Names: []string{"/k8s_net_foo.new.test_12345678_42"}, ID: "9876", }, } @@ -660,12 +662,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.new.test"}, + Names: []string{"/k8s_foo_bar.new.test_12345678_42"}, ID: "1234", }, { // network container - Names: []string{"/k8s_net_foo.new.test_"}, + Names: []string{"/k8s_net_foo.new.test_12345678_42"}, ID: "9876", }, { @@ -713,7 +715,7 @@ func TestSyncPodDeletesDuplicate(t *testing.T) { }, "2304": &docker.APIContainers{ // Container for another pod, untouched. - Names: []string{"/k8s_baz_fiz.new.test_6"}, + Names: []string{"/k8s_baz_fiz.new.test_6_42"}, ID: "2304", }, } @@ -758,17 +760,18 @@ 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.new.test"}, + Names: []string{"/k8s_bar.1234_foo.new.test_12345678_42"}, ID: "1234", }, "9876": &docker.APIContainers{ // network container - Names: []string{"/k8s_net_foo.new.test_"}, + Names: []string{"/k8s_net_foo.new.test_12345678_42"}, ID: "9876", }, } err := kubelet.syncPod(&api.BoundPod{ ObjectMeta: api.ObjectMeta{ + UID: "12345678", Name: "foo", Namespace: "new", Annotations: map[string]string{ConfigSourceAnnotationKey: "test"}, @@ -804,17 +807,18 @@ 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.new.test"}, + Names: []string{"/k8s_bar_foo.new.test_12345678_42"}, ID: "1234", }, "9876": &docker.APIContainers{ // network container - Names: []string{"/k8s_net_foo.new.test_"}, + Names: []string{"/k8s_net_foo.new.test_12345678_42"}, ID: "9876", }, } err := kubelet.syncPod(&api.BoundPod{ ObjectMeta: api.ObjectMeta{ + UID: "12345678", Name: "foo", Namespace: "new", Annotations: map[string]string{ConfigSourceAnnotationKey: "test"}, @@ -877,6 +881,7 @@ func TestMountExternalVolumes(t *testing.T) { kubelet, _, _ := newTestKubelet(t) pod := api.BoundPod{ ObjectMeta: api.ObjectMeta{ + UID: "12345678", Name: "foo", Namespace: "test", }, @@ -932,6 +937,7 @@ func TestMakeVolumesAndBinds(t *testing.T) { pod := api.BoundPod{ ObjectMeta: api.ObjectMeta{ + UID: "12345678", Name: "pod", Namespace: "test", }, @@ -1129,7 +1135,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_42"}, }, } @@ -1284,7 +1290,7 @@ func TestRunInContainer(t *testing.T) { fakeDocker.ContainerList = []docker.APIContainers{ { ID: containerID, - Names: []string{"/k8s_" + containerName + "_" + podName + "." + podNamespace + ".test_1234"}, + Names: []string{"/k8s_" + containerName + "_" + podName + "." + podNamespace + ".test_12345678_42"}, }, } @@ -1292,6 +1298,7 @@ func TestRunInContainer(t *testing.T) { _, err := kubelet.RunInContainer( GetPodFullName(&api.BoundPod{ ObjectMeta: api.ObjectMeta{ + UID: "12345678", Name: podName, Namespace: podNamespace, Annotations: map[string]string{ConfigSourceAnnotationKey: "test"}, @@ -1324,7 +1331,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 + "_12345678_42"}, }, } @@ -1428,12 +1435,13 @@ func TestSyncPodEventHandlerFails(t *testing.T) { dockerContainers := dockertools.DockerContainers{ "9876": &docker.APIContainers{ // network container - Names: []string{"/k8s_net_foo.new.test_"}, + Names: []string{"/k8s_net_foo.new.test_12345678_42"}, ID: "9876", }, } err := kubelet.syncPod(&api.BoundPod{ ObjectMeta: api.ObjectMeta{ + UID: "12345678", Name: "foo", Namespace: "new", Annotations: map[string]string{ConfigSourceAnnotationKey: "test"}, @@ -1475,32 +1483,32 @@ func TestKubeletGarbageCollection(t *testing.T) { containers: []docker.APIContainers{ { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "1876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "2876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "3876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "4876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "5876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "6876", }, }, @@ -1519,37 +1527,37 @@ func TestKubeletGarbageCollection(t *testing.T) { containers: []docker.APIContainers{ { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "1876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "2876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "3876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "4876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "5876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "6876", }, { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "7876", }, }, @@ -1575,7 +1583,7 @@ func TestKubeletGarbageCollection(t *testing.T) { containers: []docker.APIContainers{ { // network container - Names: []string{"/k8s_net_foo.new.test_.deadbeef"}, + Names: []string{"/k8s_net_foo.new.test_.deadbeef_42"}, ID: "1876", }, }, @@ -1768,6 +1776,7 @@ func TestSyncPodsWithPullPolicy(t *testing.T) { err := kubelet.SyncPods([]api.BoundPod{ { ObjectMeta: api.ObjectMeta{ + UID: "12345678", Name: "foo", Namespace: "new", Annotations: map[string]string{ConfigSourceAnnotationKey: "test"}, diff --git a/pkg/kubelet/runonce_test.go b/pkg/kubelet/runonce_test.go index 37d58265cac..e747b214778 100644 --- a/pkg/kubelet/runonce_test.go +++ b/pkg/kubelet/runonce_test.go @@ -69,12 +69,12 @@ func TestRunOnce(t *testing.T) { kb := &Kubelet{} podContainers := []docker.APIContainers{ { - Names: []string{"/k8s_bar." + strconv.FormatUint(dockertools.HashContainer(&api.Container{Name: "bar"}), 16) + "_foo.new.test"}, + Names: []string{"/k8s_bar." + strconv.FormatUint(dockertools.HashContainer(&api.Container{Name: "bar"}), 16) + "_foo.new.test_12345678_42"}, ID: "1234", Status: "running", }, { - Names: []string{"/k8s_net_foo.new.test_"}, + Names: []string{"/k8s_net_foo.new.test_abcdefgh_42"}, ID: "9876", Status: "running", }, @@ -109,6 +109,7 @@ func TestRunOnce(t *testing.T) { results, err := kb.runOnce([]api.BoundPod{ { ObjectMeta: api.ObjectMeta{ + UID: "12345678", Name: "foo", Namespace: "new", Annotations: map[string]string{ConfigSourceAnnotationKey: "test"},