Merge pull request #52220 from yujuhong/rm-legacy-code

Automatic merge from submit-queue (batch tested with PRs 52240, 48145, 52220, 51698, 51777). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>..

dockershim: remove support for legacy containers

The code was first introduced in 1.6 to help pre-CRI-kubelet upgrade to
using the CRI implementation. They can safely be removed now.

pkg/kubelet/dockershim/BUILD

@@ -15,7 +15,6 @@ go_library(
         "docker_checkpoint.go",
         "docker_container.go",
         "docker_image.go",
-        "docker_legacy.go",
         "docker_sandbox.go",
         "docker_service.go",
         "docker_stats.go",
@@ -70,8 +69,6 @@ go_library(
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//vendor/k8s.io/client-go/tools/remotecommand:go_default_library",
         "//vendor/k8s.io/utils/exec:go_default_library",
     ],
@@ -85,7 +82,6 @@ go_test(
         "docker_checkpoint_test.go",
         "docker_container_test.go",
         "docker_image_test.go",
-        "docker_legacy_test.go",
         "docker_sandbox_test.go",
         "docker_service_test.go",
         "helpers_test.go",

pkg/kubelet/dockershim/docker_container.go

@@ -75,15 +75,6 @@ func (ds *dockerService) ListContainers(filter *runtimeapi.ContainerFilter) ([]*
 
 		result = append(result, converted)
 	}
-	// Include legacy containers if there are still legacy containers not cleaned up yet.
-	if !ds.legacyCleanup.Done() {
-		legacyContainers, err := ds.ListLegacyContainers(filter)
-		if err != nil {
-			return nil, err
-		}
-		// Legacy containers are always older, so we can safely append them to the end.
-		result = append(result, legacyContainers...)
-	}
 	return result, nil
 }
 
@@ -370,15 +361,6 @@ func (ds *dockerService) ContainerStatus(containerID string) (*runtimeapi.Contai
 	ct, st, ft := createdAt.UnixNano(), startedAt.UnixNano(), finishedAt.UnixNano()
 	exitCode := int32(r.State.ExitCode)
 
-	// If the container has no containerTypeLabelKey label, treat it as a legacy container.
-	if _, ok := r.Config.Labels[containerTypeLabelKey]; !ok {
-		names, labels, err := convertLegacyNameAndLabels([]string{r.Name}, r.Config.Labels)
-		if err != nil {
-			return nil, err
-		}
-		r.Name, r.Config.Labels = names[0], labels
-	}
-
 	metadata, err := parseContainerName(r.Name)
 	if err != nil {
 		return nil, err

pkg/kubelet/dockershim/docker_legacy.go

@@ -1,284 +0,0 @@
-/*
-Copyright 2017 The Kubernetes Authors.
-
-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 dockershim
-
-import (
-	"fmt"
-	"strings"
-	"sync/atomic"
-	"time"
-
-	dockertypes "github.com/docker/docker/api/types"
-	dockerfilters "github.com/docker/docker/api/types/filters"
-	"github.com/golang/glog"
-
-	"k8s.io/apimachinery/pkg/util/sets"
-	"k8s.io/apimachinery/pkg/util/wait"
-	runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"
-	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
-	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
-	"k8s.io/kubernetes/pkg/kubelet/leaky"
-)
-
-// These are labels used by kuberuntime. Ideally, we should not rely on kuberuntime implementation
-// detail in dockershim. However, we need these labels for legacy container (containers created by
-// kubernetes 1.4 and 1.5) support.
-// TODO(random-liu): Remove this file and related code in kubernetes 1.8.
-const (
-	podDeletionGracePeriodLabel    = "io.kubernetes.pod.deletionGracePeriod"
-	podTerminationGracePeriodLabel = "io.kubernetes.pod.terminationGracePeriod"
-
-	containerHashLabel                     = "io.kubernetes.container.hash"
-	containerRestartCountLabel             = "io.kubernetes.container.restartCount"
-	containerTerminationMessagePathLabel   = "io.kubernetes.container.terminationMessagePath"
-	containerTerminationMessagePolicyLabel = "io.kubernetes.container.terminationMessagePolicy"
-	containerPreStopHandlerLabel           = "io.kubernetes.container.preStopHandler"
-	containerPortsLabel                    = "io.kubernetes.container.ports"
-)
-
-// NOTE that we can't handle the following dockershim internal labels, so they will be empty:
-// * containerLogPathLabelKey ("io.kubernetes.container.logpath"): RemoveContainer will ignore
-// the label if it's empty.
-// * sandboxIDLabelKey ("io.kubernetes.sandbox.id"): This is used in 2 places:
-//   * filter.PodSandboxId: The filter is not used in kuberuntime now.
-//   * runtimeapi.Container.PodSandboxId: toRuntimeAPIContainer retrieves PodSandboxId from the
-//   label. The field is used in kuberuntime sandbox garbage collection. Missing this may cause
-//   pod sandbox to be removed before its containers are removed.
-
-// convertLegacyNameAndLabels converts legacy name and labels into dockershim name and labels.
-// The function can be used to either legacy infra container or regular container.
-// NOTE that legacy infra container doesn't have restart count label, so the returned attempt for
-// sandbox will always be 0. The sandbox attempt is only used to generate new sandbox name, and
-// there is no naming conflict between legacy and new containers/sandboxes, so it should be fine.
-func convertLegacyNameAndLabels(names []string, labels map[string]string) ([]string, map[string]string, error) {
-	if len(names) == 0 {
-		return nil, nil, fmt.Errorf("unexpected empty name")
-	}
-
-	// Generate new dockershim name.
-	m, _, err := libdocker.ParseDockerName(names[0])
-	if err != nil {
-		return nil, nil, err
-	}
-	sandboxName, sandboxNamespace, err := kubecontainer.ParsePodFullName(m.PodFullName)
-	if err != nil {
-		return nil, nil, err
-	}
-	newNames := []string{strings.Join([]string{
-		kubePrefix,                         // 0
-		m.ContainerName,                    // 1: container name
-		sandboxName,                        // 2: sandbox name
-		sandboxNamespace,                   // 3: sandbox namesapce
-		string(m.PodUID),                   // 4: sandbox uid
-		labels[containerRestartCountLabel], // 5
-	}, nameDelimiter)}
-
-	// Generate new labels.
-	legacyAnnotations := sets.NewString(
-		containerHashLabel,
-		containerRestartCountLabel,
-		containerTerminationMessagePathLabel,
-		containerTerminationMessagePolicyLabel,
-		containerPreStopHandlerLabel,
-		containerPortsLabel,
-	)
-	newLabels := map[string]string{}
-	for k, v := range labels {
-		if legacyAnnotations.Has(k) {
-			// Add annotation prefix for all legacy labels which should be annotations in dockershim.
-			newLabels[fmt.Sprintf("%s%s", annotationPrefix, k)] = v
-		} else {
-			newLabels[k] = v
-		}
-	}
-	// Add containerTypeLabelKey indicating the container is sandbox or application container.
-	if m.ContainerName == leaky.PodInfraContainerName {
-		newLabels[containerTypeLabelKey] = containerTypeLabelSandbox
-	} else {
-		newLabels[containerTypeLabelKey] = containerTypeLabelContainer
-	}
-	return newNames, newLabels, nil
-}
-
-// legacyCleanupCheckInterval is the interval legacyCleanupCheck is performed.
-const legacyCleanupCheckInterval = 10 * time.Second
-
-// LegacyCleanupInit initializes the legacy cleanup flag. If necessary, it will starts a goroutine
-// which periodically checks legacy cleanup until it finishes.
-func (ds *dockerService) LegacyCleanupInit() {
-	// If there is no legacy container/sandbox, just return.
-	if clean, _ := ds.checkLegacyCleanup(); clean {
-		return
-	}
-	// Or else start the cleanup routine.
-	go wait.PollInfinite(legacyCleanupCheckInterval, ds.checkLegacyCleanup)
-}
-
-// checkLegacyCleanup lists legacy containers/sandboxes, if no legacy containers/sandboxes are found,
-// mark the legacy cleanup flag as done.
-func (ds *dockerService) checkLegacyCleanup() (bool, error) {
-	// Always do legacy cleanup when list fails.
-	sandboxes, err := ds.ListLegacyPodSandbox(nil)
-	if err != nil {
-		glog.Errorf("Failed to list legacy pod sandboxes: %v", err)
-		return false, nil
-	}
-	containers, err := ds.ListLegacyContainers(nil)
-	if err != nil {
-		glog.Errorf("Failed to list legacy containers: %v", err)
-		return false, nil
-	}
-	if len(sandboxes) != 0 || len(containers) != 0 {
-		glog.V(4).Infof("Found legacy sandboxes %+v, legacy containers %+v, continue legacy cleanup",
-			sandboxes, containers)
-		return false, nil
-	}
-	ds.legacyCleanup.MarkDone()
-	glog.V(2).Infof("No legacy containers found, stop performing legacy cleanup.")
-	return true, nil
-}
-
-// ListLegacyPodSandbox only lists all legacy pod sandboxes.
-func (ds *dockerService) ListLegacyPodSandbox(filter *runtimeapi.PodSandboxFilter) ([]*runtimeapi.PodSandbox, error) {
-	// By default, list all containers whether they are running or not.
-	opts := dockertypes.ContainerListOptions{All: true, Filters: dockerfilters.NewArgs()}
-	filterOutReadySandboxes := false
-	f := newDockerFilter(&opts.Filters)
-	if filter != nil {
-		if filter.Id != "" {
-			f.Add("id", filter.Id)
-		}
-		if filter.State != nil {
-			if filter.GetState().State == runtimeapi.PodSandboxState_SANDBOX_READY {
-				// Only list running containers.
-				opts.All = false
-			} else {
-				// runtimeapi.PodSandboxState_SANDBOX_NOTREADY can mean the
-				// container is in any of the non-running state (e.g., created,
-				// exited). We can't tell docker to filter out running
-				// containers directly, so we'll need to filter them out
-				// ourselves after getting the results.
-				filterOutReadySandboxes = true
-			}
-		}
-		if filter.LabelSelector != nil {
-			for k, v := range filter.LabelSelector {
-				f.AddLabel(k, v)
-			}
-		}
-	}
-	containers, err := ds.client.ListContainers(opts)
-	if err != nil {
-		return nil, err
-	}
-
-	// Convert docker containers to runtime api sandboxes.
-	result := make([]*runtimeapi.PodSandbox, 0, len(containers))
-	for i := range containers {
-		c := containers[i]
-		// Skip new containers with containerTypeLabelKey label.
-		if _, ok := c.Labels[containerTypeLabelKey]; ok {
-			continue
-		}
-		// If the container has no containerTypeLabelKey label, treat it as a legacy container.
-		c.Names, c.Labels, err = convertLegacyNameAndLabels(c.Names, c.Labels)
-		if err != nil {
-			glog.V(4).Infof("Unable to convert legacy container %+v: %v", c, err)
-			continue
-		}
-		if c.Labels[containerTypeLabelKey] != containerTypeLabelSandbox {
-			continue
-		}
-		converted, err := containerToRuntimeAPISandbox(&c)
-		if err != nil {
-			glog.V(4).Infof("Unable to convert docker to runtime API sandbox %+v: %v", c, err)
-			continue
-		}
-		if filterOutReadySandboxes && converted.State == runtimeapi.PodSandboxState_SANDBOX_READY {
-			continue
-		}
-		result = append(result, converted)
-	}
-	return result, nil
-}
-
-// ListLegacyPodSandbox only lists all legacy containers.
-func (ds *dockerService) ListLegacyContainers(filter *runtimeapi.ContainerFilter) ([]*runtimeapi.Container, error) {
-	opts := dockertypes.ContainerListOptions{All: true, Filters: dockerfilters.NewArgs()}
-	f := newDockerFilter(&opts.Filters)
-
-	if filter != nil {
-		if filter.Id != "" {
-			f.Add("id", filter.Id)
-		}
-		if filter.State != nil {
-			f.Add("status", toDockerContainerStatus(filter.GetState().State))
-		}
-		// NOTE: Legacy container doesn't have sandboxIDLabelKey label, so we can't filter with
-		// it. Fortunately, PodSandboxId is not used in kuberuntime now.
-		if filter.LabelSelector != nil {
-			for k, v := range filter.LabelSelector {
-				f.AddLabel(k, v)
-			}
-		}
-	}
-	containers, err := ds.client.ListContainers(opts)
-	if err != nil {
-		return nil, err
-	}
-
-	// Convert docker to runtime api containers.
-	result := make([]*runtimeapi.Container, 0, len(containers))
-	for i := range containers {
-		c := containers[i]
-		// Skip new containers with containerTypeLabelKey label.
-		if _, ok := c.Labels[containerTypeLabelKey]; ok {
-			continue
-		}
-		// If the container has no containerTypeLabelKey label, treat it as a legacy container.
-		c.Names, c.Labels, err = convertLegacyNameAndLabels(c.Names, c.Labels)
-		if err != nil {
-			glog.V(4).Infof("Unable to convert legacy container %+v: %v", c, err)
-			continue
-		}
-		if c.Labels[containerTypeLabelKey] != containerTypeLabelContainer {
-			continue
-		}
-		converted, err := toRuntimeAPIContainer(&c)
-		if err != nil {
-			glog.V(4).Infof("Unable to convert docker container to runtime API container %+v: %v", c, err)
-			continue
-		}
-		result = append(result, converted)
-	}
-	return result, nil
-}
-
-// legacyCleanupFlag is the flag indicating whether legacy cleanup is done.
-type legacyCleanupFlag struct {
-	done int32
-}
-
-// Done checks whether legacy cleanup is done.
-func (f *legacyCleanupFlag) Done() bool {
-	return atomic.LoadInt32(&f.done) == 1
-}
-
-// MarkDone sets legacy cleanup as done.
-func (f *legacyCleanupFlag) MarkDone() {
-	atomic.StoreInt32(&f.done, 1)
-}

pkg/kubelet/dockershim/docker_legacy_test.go

@@ -1,266 +0,0 @@
-/*
-Copyright 2017 The Kubernetes Authors.
-
-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 dockershim
-
-import (
-	"testing"
-
-	dockercontainer "github.com/docker/docker/api/types/container"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
-	"k8s.io/kubernetes/pkg/kubelet/types"
-)
-
-func TestConvertLegacyNameAndLabels(t *testing.T) {
-	for desc, test := range map[string]struct {
-		names        []string
-		labels       map[string]string
-		expectNames  []string
-		expectLabels map[string]string
-		expectError  bool
-	}{
-
-		"legacy infra container": {
-			names: []string{"k8s_POD.hash1_podname_podnamespace_poduid_randomid"},
-			labels: map[string]string{
-				types.KubernetesPodNameLabel:       "podname",
-				types.KubernetesPodNamespaceLabel:  "podnamespace",
-				types.KubernetesPodUIDLabel:        "poduid",
-				types.KubernetesContainerNameLabel: "POD",
-				containerHashLabel:                 "hash",
-				containerRestartCountLabel:         "0",
-			},
-			expectNames: []string{"k8s_POD_podname_podnamespace_poduid_0"},
-			expectLabels: map[string]string{
-				types.KubernetesPodNameLabel:                  "podname",
-				types.KubernetesPodNamespaceLabel:             "podnamespace",
-				types.KubernetesPodUIDLabel:                   "poduid",
-				types.KubernetesContainerNameLabel:            "POD",
-				annotationPrefix + containerHashLabel:         "hash",
-				annotationPrefix + containerRestartCountLabel: "0",
-				containerTypeLabelKey:                         containerTypeLabelSandbox,
-			},
-		},
-		"legacy application container": {
-			names: []string{"k8s_containername.hash_podname_podnamespace_poduid_randomid"},
-			labels: map[string]string{
-				types.KubernetesPodNameLabel:           "podname",
-				types.KubernetesPodNamespaceLabel:      "podnamespace",
-				types.KubernetesPodUIDLabel:            "poduid",
-				types.KubernetesContainerNameLabel:     "containername",
-				containerHashLabel:                     "hash",
-				containerRestartCountLabel:             "5",
-				containerTerminationMessagePathLabel:   "terminationmessagepath",
-				containerTerminationMessagePolicyLabel: "terminationmessagepolicy",
-				containerPreStopHandlerLabel:           "prestophandler",
-				containerPortsLabel:                    "ports",
-			},
-			expectNames: []string{"k8s_containername_podname_podnamespace_poduid_5"},
-			expectLabels: map[string]string{
-				types.KubernetesPodNameLabel:                              "podname",
-				types.KubernetesPodNamespaceLabel:                         "podnamespace",
-				types.KubernetesPodUIDLabel:                               "poduid",
-				types.KubernetesContainerNameLabel:                        "containername",
-				annotationPrefix + containerHashLabel:                     "hash",
-				annotationPrefix + containerRestartCountLabel:             "5",
-				annotationPrefix + containerTerminationMessagePathLabel:   "terminationmessagepath",
-				annotationPrefix + containerTerminationMessagePolicyLabel: "terminationmessagepolicy",
-				annotationPrefix + containerPreStopHandlerLabel:           "prestophandler",
-				annotationPrefix + containerPortsLabel:                    "ports",
-				containerTypeLabelKey:                                     containerTypeLabelContainer,
-			},
-			expectError: false,
-		},
-		"invalid sandbox name": {
-			names:       []string{"POD_podname_podnamespace_poduid_0"},
-			expectError: true,
-		},
-		"invalid dockershim container": {
-			names:       []string{"containername_podname_podnamespace_poduid_5"},
-			expectError: true,
-		},
-	} {
-		t.Logf("TestCase %q", desc)
-		names, labels, err := convertLegacyNameAndLabels(test.names, test.labels)
-		require.Equal(t, test.expectError, err != nil)
-		assert.Equal(t, test.expectNames, names)
-		assert.Equal(t, test.expectLabels, labels)
-	}
-}
-
-// getFakeLegacyContainers returns a list of fake legacy containers.
-func getFakeLegacyContainers() []*libdocker.FakeContainer {
-	return []*libdocker.FakeContainer{
-		{
-			ID:   "12",
-			Name: "k8s_POD.hash1_podname_podnamespace_poduid_randomid",
-			Config: &dockercontainer.Config{
-				Labels: map[string]string{
-					types.KubernetesPodNameLabel:       "podname",
-					types.KubernetesPodNamespaceLabel:  "podnamespace",
-					types.KubernetesPodUIDLabel:        "poduid",
-					types.KubernetesContainerNameLabel: "POD",
-					containerHashLabel:                 "hash1",
-					containerRestartCountLabel:         "0",
-				},
-			},
-		},
-		{
-			ID:   "34",
-			Name: "k8s_legacycontainer.hash2_podname_podnamespace_poduid_randomid",
-			Config: &dockercontainer.Config{
-				Labels: map[string]string{
-					types.KubernetesPodNameLabel:       "podname",
-					types.KubernetesPodNamespaceLabel:  "podnamespace",
-					types.KubernetesPodUIDLabel:        "poduid",
-					types.KubernetesContainerNameLabel: "legacyContainer",
-					containerHashLabel:                 "hash2",
-					containerRestartCountLabel:         "5",
-				},
-			},
-		},
-	}
-}
-
-// getFakeNewContainers returns a list of fake new containers.
-func getFakeNewContainers() []*libdocker.FakeContainer {
-	return []*libdocker.FakeContainer{
-		{
-			ID:   "56",
-			Name: "k8s_POD_podname_podnamespace_poduid_0",
-			Config: &dockercontainer.Config{
-				Labels: map[string]string{
-					types.KubernetesPodNameLabel:       "podname",
-					types.KubernetesPodNamespaceLabel:  "podnamespace",
-					types.KubernetesPodUIDLabel:        "poduid",
-					types.KubernetesContainerNameLabel: "POD",
-					containerTypeLabelKey:              containerTypeLabelSandbox,
-				},
-			},
-		},
-		{
-			ID:   "78",
-			Name: "k8s_newcontainer_podname_podnamespace_poduid_3",
-			Config: &dockercontainer.Config{
-				Labels: map[string]string{
-					types.KubernetesPodNameLabel:                  "podname",
-					types.KubernetesPodNamespaceLabel:             "podnamespace",
-					types.KubernetesPodUIDLabel:                   "poduid",
-					types.KubernetesContainerNameLabel:            "newcontainer",
-					annotationPrefix + containerHashLabel:         "hash4",
-					annotationPrefix + containerRestartCountLabel: "3",
-					containerTypeLabelKey:                         containerTypeLabelContainer,
-				},
-			},
-		},
-	}
-
-}
-
-func TestListLegacyContainers(t *testing.T) {
-	ds, fDocker, _ := newTestDockerService()
-	newContainers := getFakeLegacyContainers()
-	legacyContainers := getFakeNewContainers()
-	fDocker.SetFakeContainers(append(newContainers, legacyContainers...))
-
-	// ListContainers should list only new containers when legacyCleanup is done.
-	containers, err := ds.ListContainers(nil)
-	assert.NoError(t, err)
-	require.Len(t, containers, 1)
-	assert.Equal(t, "78", containers[0].Id)
-
-	// ListLegacyContainers should list only legacy containers.
-	containers, err = ds.ListLegacyContainers(nil)
-	assert.NoError(t, err)
-	require.Len(t, containers, 1)
-	assert.Equal(t, "34", containers[0].Id)
-
-	// Mark legacyCleanup as not done.
-	ds.legacyCleanup.done = 0
-
-	// ListContainers should list all containers when legacyCleanup is not done.
-	containers, err = ds.ListContainers(nil)
-	assert.NoError(t, err)
-	require.Len(t, containers, 2)
-	assert.Contains(t, []string{containers[0].Id, containers[1].Id}, "34")
-	assert.Contains(t, []string{containers[0].Id, containers[1].Id}, "78")
-}
-
-func TestListLegacyPodSandbox(t *testing.T) {
-	ds, fDocker, _ := newTestDockerService()
-	newContainers := getFakeLegacyContainers()
-	legacyContainers := getFakeNewContainers()
-	fDocker.SetFakeContainers(append(newContainers, legacyContainers...))
-
-	// ListPodSandbox should list only new sandboxes when legacyCleanup is done.
-	sandboxes, err := ds.ListPodSandbox(nil)
-	assert.NoError(t, err)
-	require.Len(t, sandboxes, 1)
-	assert.Equal(t, "56", sandboxes[0].Id)
-
-	// ListLegacyPodSandbox should list only legacy sandboxes.
-	sandboxes, err = ds.ListLegacyPodSandbox(nil)
-	assert.NoError(t, err)
-	require.Len(t, sandboxes, 1)
-	assert.Equal(t, "12", sandboxes[0].Id)
-
-	// Mark legacyCleanup as not done.
-	ds.legacyCleanup.done = 0
-
-	// ListPodSandbox should list all sandboxes when legacyCleanup is not done.
-	sandboxes, err = ds.ListPodSandbox(nil)
-	assert.NoError(t, err)
-	require.Len(t, sandboxes, 2)
-	assert.Contains(t, []string{sandboxes[0].Id, sandboxes[1].Id}, "12")
-	assert.Contains(t, []string{sandboxes[0].Id, sandboxes[1].Id}, "56")
-}
-
-func TestCheckLegacyCleanup(t *testing.T) {
-	for desc, test := range map[string]struct {
-		containers []*libdocker.FakeContainer
-		done       bool
-	}{
-		"no containers": {
-			containers: []*libdocker.FakeContainer{},
-			done:       true,
-		},
-		"only new containers": {
-			containers: getFakeNewContainers(),
-			done:       true,
-		},
-		"only legacy containers": {
-			containers: getFakeLegacyContainers(),
-			done:       false,
-		},
-		"both legacy and new containers": {
-			containers: append(getFakeNewContainers(), getFakeLegacyContainers()...),
-			done:       false,
-		},
-	} {
-		t.Logf("TestCase %q", desc)
-		ds, fDocker, _ := newTestDockerService()
-		fDocker.SetFakeContainers(test.containers)
-		ds.legacyCleanup.done = 0
-
-		clean, err := ds.checkLegacyCleanup()
-		assert.NoError(t, err)
-		assert.Equal(t, test.done, clean)
-		assert.Equal(t, test.done, ds.legacyCleanup.Done())
-	}
-}

pkg/kubelet/dockershim/docker_sandbox.go

@@ -373,17 +373,6 @@ func (ds *dockerService) PodSandboxStatus(podSandboxID string) (*runtimeapi.PodS
 	}
 	hostNetwork := sharesHostNetwork(r)
 
-	// If the sandbox has no containerTypeLabelKey label, treat it as a legacy sandbox.
-	if _, ok := r.Config.Labels[containerTypeLabelKey]; !ok {
-		names, labels, err := convertLegacyNameAndLabels([]string{r.Name}, r.Config.Labels)
-		if err != nil {
-			return nil, err
-		}
-		r.Name, r.Config.Labels = names[0], labels
-		// Forcibly trigger infra container restart.
-		hostNetwork = !hostNetwork
-	}
-
 	metadata, err := parseSandboxName(r.Name)
 	if err != nil {
 		return nil, err
@@ -503,15 +492,6 @@ func (ds *dockerService) ListPodSandbox(filter *runtimeapi.PodSandboxFilter) ([]
 		result = append(result, checkpointToRuntimeAPISandbox(id, checkpoint))
 	}
 
-	// Include legacy sandboxes if there are still legacy sandboxes not cleaned up yet.
-	if !ds.legacyCleanup.Done() {
-		legacySandboxes, err := ds.ListLegacyPodSandbox(filter)
-		if err != nil {
-			return nil, err
-		}
-		// Legacy sandboxes are always older, so we can safely append them to the end.
-		result = append(result, legacySandboxes...)
-	}
 	return result, nil
 }
 

pkg/kubelet/dockershim/docker_service.go

@@ -258,8 +258,6 @@ type dockerService struct {
 	// cgroup driver used by Docker runtime.
 	cgroupDriver      string
 	checkpointHandler CheckpointHandler
-	// legacyCleanup indicates whether legacy cleanup has finished or not.
-	legacyCleanup legacyCleanupFlag
 	// caches the version of the runtime.
 	// To be compatible with multiple docker versions, we need to perform
 	// version checking for some operations. Use this cache to avoid querying
@@ -352,7 +350,6 @@ func (ds *dockerService) GetPodPortMappings(podSandboxID string) ([]*hostport.Po
 // Start initializes and starts components in dockerService.
 func (ds *dockerService) Start() error {
 	// Initialize the legacy cleanup flag.
-	ds.LegacyCleanupInit()
 	return ds.containerManager.Start()
 }
 

pkg/kubelet/dockershim/docker_service_test.go

@@ -50,7 +50,6 @@ func newTestDockerService() (*dockerService, *libdocker.FakeDockerClient, *clock
 		client:            c,
 		os:                &containertest.FakeOS{},
 		network:           pm,
-		legacyCleanup:     legacyCleanupFlag{done: 1},
 		checkpointHandler: NewTestPersistentCheckpointHandler(),
 		networkReady:      make(map[string]bool),
 	}, c, fakeClock

pkg/kubelet/dockershim/libdocker/BUILD

@@ -11,15 +11,11 @@ go_test(
     srcs = [
         "helpers_test.go",
         "kube_docker_client_test.go",
-        "legacy_test.go",
     ],
     library = ":go_default_library",
     deps = [
-        "//pkg/util/hash:go_default_library",
         "//vendor/github.com/docker/docker/api/types:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
-        "//vendor/k8s.io/api/core/v1:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
     ],
 )
 
@@ -31,10 +27,8 @@ go_library(
         "helpers.go",
         "instrumented_client.go",
         "kube_docker_client.go",
-        "legacy.go",
     ],
     deps = [
-        "//pkg/kubelet/container:go_default_library",
         "//pkg/kubelet/metrics:go_default_library",
         "//vendor/github.com/docker/distribution/reference:go_default_library",
         "//vendor/github.com/docker/docker/api/types:go_default_library",
@@ -47,7 +41,6 @@ go_library(
         "//vendor/github.com/opencontainers/go-digest:go_default_library",
         "//vendor/golang.org/x/net/context:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/clock:go_default_library",
     ],
 )

pkg/kubelet/dockershim/libdocker/fake_client.go

@@ -83,6 +83,9 @@ const (
 	fakeDockerVersion = "1.11.2"
 
 	fakeImageSize = 1024
+
+	// Docker prepends '/' to the container name.
+	dockerNamePrefix = "/"
 )
 
 func NewFakeDockerClient() *FakeDockerClient {
@@ -289,11 +292,7 @@ func (f *FakeDockerClient) AssertCallDetails(calls ...calledDetail) (err error)
 func (f *FakeDockerClient) idsToNames(ids []string) ([]string, error) {
 	names := []string{}
 	for _, id := range ids {
-		dockerName, _, err := ParseDockerName(f.ContainerMap[id].Name)
+		names = append(names, strings.TrimPrefix(f.ContainerMap[id].Name, dockerNamePrefix))
-		if err != nil {
-			return nil, fmt.Errorf("unexpected error: %v", err)
-		}
-		names = append(names, dockerName.ContainerName)
 	}
 	return names, nil
 }
@@ -523,8 +522,7 @@ func (f *FakeDockerClient) CreateContainer(c dockertypes.ContainerCreateConfig)
 		return nil, err
 	}
 	// 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 := dockerNamePrefix + c.Name
-	name := "/" + c.Name
 	id := GetFakeContainerID(name)
 	f.appendContainerTrace("Created", id)
 	timestamp := f.Clock.Now()

pkg/kubelet/dockershim/libdocker/legacy.go

@@ -1,92 +0,0 @@
-/*
-Copyright 2014 The Kubernetes Authors.
-
-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 libdocker
-
-import (
-	"fmt"
-	"math/rand"
-	"strconv"
-	"strings"
-
-	"github.com/golang/glog"
-
-	"k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
-	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
-)
-
-// This file contains functions used in the non-CRI integration (< 1.6). They
-// are currently used for recoginzing containers created by pre-1.6 kubelets.
-// TODO: Remove this file for kubernetes 1.8+.
-
-// Creates a name which can be reversed to identify both full pod name and container name.
-// This function returns stable name, unique name and a unique id.
-// Although rand.Uint32() is not really unique, but it's enough for us because error will
-// only occur when instances of the same container in the same pod have the same UID. The
-// chance is really slim.
-func BuildDockerName(dockerName KubeletContainerName, container *v1.Container) (string, string, string) {
-	containerName := dockerName.ContainerName + "." + strconv.FormatUint(kubecontainer.HashContainerLegacy(container), 16)
-	stableName := fmt.Sprintf("%s_%s_%s_%s",
-		containerNamePrefix,
-		containerName,
-		dockerName.PodFullName,
-		dockerName.PodUID)
-	UID := fmt.Sprintf("%08x", rand.Uint32())
-	return stableName, fmt.Sprintf("%s_%s", stableName, UID), UID
-}
-
-// Unpacks a container name, returning the pod full name and container name we would have used to
-// construct the docker name. If we are unable to parse the name, an error is returned.
-func ParseDockerName(name string) (dockerName *KubeletContainerName, hash uint64, err error) {
-	// For some reason docker appears to be appending '/' to names.
-	// If it's there, strip it.
-	name = strings.TrimPrefix(name, "/")
-	parts := strings.Split(name, "_")
-	if len(parts) == 0 || parts[0] != containerNamePrefix {
-		err = fmt.Errorf("failed to parse Docker container name %q into parts", name)
-		return nil, 0, err
-	}
-	if len(parts) < 6 {
-		// 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): %q", containerNamePrefix, len(parts), name)
-		err = fmt.Errorf("Docker container name %q has less parts than expected %v", name, parts)
-		return nil, 0, err
-	}
-
-	nameParts := strings.Split(parts[1], ".")
-	containerName := nameParts[0]
-	if len(nameParts) > 1 {
-		hash, err = strconv.ParseUint(nameParts[1], 16, 32)
-		if err != nil {
-			glog.Warningf("invalid container hash %q in container %q", nameParts[1], name)
-		}
-	}
-
-	podFullName := parts[2] + "_" + parts[3]
-	podUID := types.UID(parts[4])
-
-	return &KubeletContainerName{podFullName, podUID, containerName}, hash, nil
-}
-
-// KubeletContainerName encapsulates a pod name and a Kubernetes container name.
-type KubeletContainerName struct {
-	PodFullName   string
-	PodUID        types.UID
-	ContainerName string
-}

pkg/kubelet/dockershim/libdocker/legacy_test.go

@@ -1,140 +0,0 @@
-/*
-Copyright 2014 The Kubernetes Authors.
-
-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 libdocker
-
-import (
-	"fmt"
-	"hash/adler32"
-	"testing"
-
-	dockertypes "github.com/docker/docker/api/types"
-	"github.com/stretchr/testify/assert"
-	"k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
-	hashutil "k8s.io/kubernetes/pkg/util/hash"
-)
-
-func verifyCalls(t *testing.T, fakeDocker *FakeDockerClient, calls []string) {
-	assert.New(t).NoError(fakeDocker.AssertCalls(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 findPodContainer(dockerContainers []*dockertypes.Container, podFullName string, uid types.UID, containerName string) (*dockertypes.Container, bool, uint64) {
-	for _, dockerContainer := range dockerContainers {
-		if len(dockerContainer.Names) == 0 {
-			continue
-		}
-		dockerName, hash, err := ParseDockerName(dockerContainer.Names[0])
-		if err != nil {
-			continue
-		}
-		if dockerName.PodFullName == podFullName &&
-			(uid == "" || dockerName.PodUID == uid) &&
-			dockerName.ContainerName == containerName {
-			return dockerContainer, true, hash
-		}
-	}
-	return nil, false, 0
-}
-
-func TestGetContainerID(t *testing.T) {
-	fakeDocker := NewFakeDockerClient()
-	fakeDocker.SetFakeRunningContainers([]*FakeContainer{
-		{
-			ID:   "foobar",
-			Name: "/k8s_foo_qux_ns_1234_42",
-		},
-		{
-			ID:   "barbar",
-			Name: "/k8s_bar_qux_ns_2565_42",
-		},
-	})
-
-	dockerContainers, err := GetKubeletDockerContainers(fakeDocker, false)
-	if err != nil {
-		t.Errorf("Expected no error, Got %#v", err)
-	}
-	if len(dockerContainers) != 2 {
-		t.Errorf("Expected %#v, Got %#v", fakeDocker.RunningContainerList, dockerContainers)
-	}
-	verifyCalls(t, fakeDocker, []string{"list"})
-
-	dockerContainer, found, _ := findPodContainer(dockerContainers, "qux_ns", "", "foo")
-	if dockerContainer == nil || !found {
-		t.Errorf("Failed to find container %#v", dockerContainer)
-	}
-
-	fakeDocker.ClearCalls()
-	dockerContainer, found, _ = findPodContainer(dockerContainers, "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, podUID, podName, containerName string) {
-	container := &v1.Container{Name: containerName}
-	hasher := adler32.New()
-	hashutil.DeepHashObject(hasher, *container)
-	computedHash := uint64(hasher.Sum32())
-	podFullName := fmt.Sprintf("%s_%s", podName, podNamespace)
-	_, name, _ := BuildDockerName(KubeletContainerName{podFullName, types.UID(podUID), container.Name}, container)
-	returned, hash, err := ParseDockerName(name)
-	if err != nil {
-		t.Errorf("Failed to parse Docker container name %q: %v", name, err)
-	}
-	if podFullName != returned.PodFullName || podUID != string(returned.PodUID) || containerName != returned.ContainerName || computedHash != hash {
-		t.Errorf("For (%s, %s, %s, %d), unpacked (%s, %s, %s, %d)", podFullName, podUID, containerName, computedHash, returned.PodFullName, returned.PodUID, returned.ContainerName, hash)
-	}
-}
-
-func TestContainerNaming(t *testing.T) {
-	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, "container")
-	// No Container name
-	verifyPackUnpack(t, "other", podUID, "name", "")
-
-	container := &v1.Container{Name: "container"}
-	podName := "foo"
-	podNamespace := "test"
-	name := fmt.Sprintf("k8s_%s_%s_%s_%s_42", container.Name, podName, podNamespace, podUID)
-	podFullName := fmt.Sprintf("%s_%s", podName, podNamespace)
-
-	returned, hash, err := ParseDockerName(name)
-	if err != nil {
-		t.Errorf("Failed to parse Docker container name %q: %v", name, err)
-	}
-	if returned.PodFullName != podFullName || string(returned.PodUID) != podUID || returned.ContainerName != container.Name || hash != 0 {
-		t.Errorf("unexpected parse: %s %s %s %d", returned.PodFullName, returned.PodUID, returned.ContainerName, hash)
-	}
-}