diff --git a/pkg/kubelet/dockershim/BUILD b/pkg/kubelet/dockershim/BUILD index ce520af36eb..9b7250a03d3 100644 --- a/pkg/kubelet/dockershim/BUILD +++ b/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", diff --git a/pkg/kubelet/dockershim/docker_container.go b/pkg/kubelet/dockershim/docker_container.go index 1f32f523897..147c7e59b13 100644 --- a/pkg/kubelet/dockershim/docker_container.go +++ b/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 diff --git a/pkg/kubelet/dockershim/docker_legacy.go b/pkg/kubelet/dockershim/docker_legacy.go deleted file mode 100644 index 24ebbe3d98b..00000000000 --- a/pkg/kubelet/dockershim/docker_legacy.go +++ /dev/null @@ -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) -} diff --git a/pkg/kubelet/dockershim/docker_legacy_test.go b/pkg/kubelet/dockershim/docker_legacy_test.go deleted file mode 100644 index cc56d47d88d..00000000000 --- a/pkg/kubelet/dockershim/docker_legacy_test.go +++ /dev/null @@ -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()) - } -} diff --git a/pkg/kubelet/dockershim/docker_sandbox.go b/pkg/kubelet/dockershim/docker_sandbox.go index fb321d59138..0d7f51b1065 100644 --- a/pkg/kubelet/dockershim/docker_sandbox.go +++ b/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 } diff --git a/pkg/kubelet/dockershim/docker_service.go b/pkg/kubelet/dockershim/docker_service.go index a42c7487a33..b99ee56cd65 100644 --- a/pkg/kubelet/dockershim/docker_service.go +++ b/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() } diff --git a/pkg/kubelet/dockershim/docker_service_test.go b/pkg/kubelet/dockershim/docker_service_test.go index 04bf5291bfb..aa5ac6fa236 100644 --- a/pkg/kubelet/dockershim/docker_service_test.go +++ b/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 diff --git a/pkg/kubelet/dockershim/libdocker/BUILD b/pkg/kubelet/dockershim/libdocker/BUILD index 3e7ecbd99a0..eccb6c52106 100644 --- a/pkg/kubelet/dockershim/libdocker/BUILD +++ b/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", ], ) diff --git a/pkg/kubelet/dockershim/libdocker/fake_client.go b/pkg/kubelet/dockershim/libdocker/fake_client.go index 0d42b3a32e4..dd0d9c86e2a 100644 --- a/pkg/kubelet/dockershim/libdocker/fake_client.go +++ b/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) - if err != nil { - return nil, fmt.Errorf("unexpected error: %v", err) - } - names = append(names, dockerName.ContainerName) + names = append(names, strings.TrimPrefix(f.ContainerMap[id].Name, dockerNamePrefix)) } 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 := "/" + c.Name + name := dockerNamePrefix + c.Name id := GetFakeContainerID(name) f.appendContainerTrace("Created", id) timestamp := f.Clock.Now() diff --git a/pkg/kubelet/dockershim/libdocker/legacy.go b/pkg/kubelet/dockershim/libdocker/legacy.go deleted file mode 100644 index 4212364ecbf..00000000000 --- a/pkg/kubelet/dockershim/libdocker/legacy.go +++ /dev/null @@ -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 -} diff --git a/pkg/kubelet/dockershim/libdocker/legacy_test.go b/pkg/kubelet/dockershim/libdocker/legacy_test.go deleted file mode 100644 index 0a9cc007932..00000000000 --- a/pkg/kubelet/dockershim/libdocker/legacy_test.go +++ /dev/null @@ -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) - } -}