From b3585a5209d4fb1124364523124680357ef71cd8 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Thu, 29 Oct 2015 22:42:25 -0700 Subject: [PATCH] Move docker label related functions into labels.go and add pod name, pod namespace and pod uid into docker label --- pkg/kubelet/dockertools/docker.go | 1 + pkg/kubelet/dockertools/fake_docker_client.go | 5 +- pkg/kubelet/dockertools/labels.go | 67 +++++++++++++++++++ pkg/kubelet/dockertools/manager.go | 49 +++++--------- pkg/kubelet/dockertools/manager_test.go | 23 ++++--- 5 files changed, 98 insertions(+), 47 deletions(-) create mode 100644 pkg/kubelet/dockertools/labels.go diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index fd881d12ef..7e194274e2 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -211,6 +211,7 @@ func (p throttledDockerPuller) IsImagePresent(name string) (bool, error) { return p.puller.IsImagePresent(name) } +// TODO (random-liu) Almost never used, should we remove this? // DockerContainers is a map of containers type DockerContainers map[kubetypes.DockerID]*docker.APIContainers diff --git a/pkg/kubelet/dockertools/fake_docker_client.go b/pkg/kubelet/dockertools/fake_docker_client.go index ee6c53fe53..ad588828ce 100644 --- a/pkg/kubelet/dockertools/fake_docker_client.go +++ b/pkg/kubelet/dockertools/fake_docker_client.go @@ -146,13 +146,14 @@ func (f *FakeDockerClient) ListContainers(options docker.ListContainersOptions) defer f.Unlock() f.called = append(f.called, "list") err := f.popError("list") + containerList := append([]docker.APIContainers{}, f.ContainerList...) if options.All { // Althought the container is not sorted, but the container with the same name should be in order, // that is enough for us now. // TODO (random-liu) Is a fully sorted array needed? - return append(f.ContainerList, f.ExitedContainerList...), err + containerList = append(containerList, f.ExitedContainerList...) } - return append([]docker.APIContainers{}, f.ContainerList...), err + return containerList, err } // InspectContainer is a test-spy implementation of DockerInterface.InspectContainer. diff --git a/pkg/kubelet/dockertools/labels.go b/pkg/kubelet/dockertools/labels.go new file mode 100644 index 0000000000..95b6dca77b --- /dev/null +++ b/pkg/kubelet/dockertools/labels.go @@ -0,0 +1,67 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package dockertools + +import ( + "strconv" + + "github.com/golang/glog" + "k8s.io/kubernetes/pkg/api" +) + +// This file contains all docker label related constants and functions, including: +// * label setters and getters +// * label filters (maybe in the future) + +const ( + kubernetesPodNameLabel = "io.kubernetes.pod.name" + kubernetesPodNamespaceLabel = "io.kubernetes.pod.namespace" + kubernetesPodUID = "io.kubernetes.pod.uid" + + kubernetesPodLabel = "io.kubernetes.pod.data" + kubernetesTerminationGracePeriodLabel = "io.kubernetes.pod.terminationGracePeriod" + kubernetesContainerLabel = "io.kubernetes.container.name" + kubernetesContainerRestartCountLabel = "io.kubernetes.container.restartCount" +) + +func newLabels(container *api.Container, pod *api.Pod, restartCount int) map[string]string { + // TODO (random-liu) Move more label initialization here + labels := map[string]string{} + labels[kubernetesPodNameLabel] = pod.Name + labels[kubernetesPodNamespaceLabel] = pod.Namespace + labels[kubernetesPodUID] = string(pod.UID) + + labels[kubernetesContainerRestartCountLabel] = strconv.Itoa(restartCount) + + return labels +} + +func getRestartCountFromLabel(labels map[string]string) (restartCount int, err error) { + if restartCountString, found := labels[kubernetesContainerRestartCountLabel]; found { + restartCount, err = strconv.Atoi(restartCountString) + if err != nil { + // This really should not happen. Just set restartCount to 0 to handle this abnormal case + restartCount = 0 + } + } else { + // Get restartCount from docker label. If there is no restart count label in a container, + // it should be an old container or an invalid container, we just set restart count to 0. + // Do not report error, because there should be many old containers without this label now + glog.V(3).Infof("Container doesn't have label %s, it may be an old or invalid container", kubernetesContainerRestartCountLabel) + } + return restartCount, err +} diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 4f70c9ed5c..74c4bc2fea 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -69,12 +69,6 @@ const ( // SIGTERM for certain process types, which may justify setting this to 0. minimumGracePeriodInSeconds = 2 - kubernetesNameLabel = "io.kubernetes.pod.name" - kubernetesPodLabel = "io.kubernetes.pod.data" - kubernetesTerminationGracePeriodLabel = "io.kubernetes.pod.terminationGracePeriod" - kubernetesContainerLabel = "io.kubernetes.container.name" - kubernetesContainerRestartCountLabel = "io.kubernetes.container.restartCount" - DockerNetnsFmt = "/proc/%v/ns/net" ) @@ -359,18 +353,9 @@ func (dm *DockerManager) inspectContainer(dockerID, containerName, tPath string, glog.V(4).Infof("Container inspect result: %+v", *inspectResult) - // Get restartCount from docker label, and add into the result. - // If there is no restart count label in an container: - // 1. It is an infraContainer, it will never use restart count. - // 2. It is an old container or an invalid container, we just set restart count to 0 now. var restartCount int - if restartCountString, found := inspectResult.Config.Labels[kubernetesContainerRestartCountLabel]; found { - restartCount, err = strconv.Atoi(restartCountString) - if err != nil { - glog.Errorf("Error parsing restart count string %s for container %s: %v,", restartCountString, dockerID, err) - // Just set restartCount to 0 to handle this abnormal case - restartCount = 0 - } + if restartCount, err = getRestartCountFromLabel(inspectResult.Config.Labels); err != nil { + glog.Errorf("Get restart count error for container %v: %v", dockerID, err) } result.status = api.ContainerStatus{ @@ -461,6 +446,9 @@ func (dm *DockerManager) GetPodStatus(pod *api.Pod) (*api.PodStatus, error) { } expectedContainers[PodInfraContainerName] = api.Container{} + // We have added labels like pod name and pod namespace, it seems that we can do filtered list here. + // However, there may be some old containers without these labels, so at least now we can't do that. + // TODO (random-liu) Add filter when we are sure that all the containers have the labels containers, err := dm.client.ListContainers(docker.ListContainersOptions{All: true}) if err != nil { return nil, err @@ -471,7 +459,6 @@ func (dm *DockerManager) GetPodStatus(pod *api.Pod) (*api.PodStatus, error) { // the statuses. We assume docker returns a list of containers sorted in // reverse by time. for _, value := range containers { - // TODO (random-liu) Filter by docker label later if len(value.Names) == 0 { continue } @@ -672,7 +659,7 @@ func (dm *DockerManager) runContainer( ipcMode string, utsMode string, pidMode string, - labels map[string]string) (kubecontainer.ContainerID, error) { + restartCount int) (kubecontainer.ContainerID, error) { dockerName := KubeletContainerName{ PodFullName: kubecontainer.GetPodFullName(pod), @@ -693,12 +680,8 @@ func (dm *DockerManager) runContainer( // while the Kubelet is down and there is no information available to recover the pod. This includes // termination information like the termination grace period and the pre stop hooks. // TODO: keep these labels up to date if the pod changes - namespacedName := types.NamespacedName{Namespace: pod.Namespace, Name: pod.Name} - // Just in case. If there is no label, just pass nil. An empty map will be created here. - if labels == nil { - labels = map[string]string{} - } - labels[kubernetesNameLabel] = namespacedName.String() + labels := newLabels(container, pod, restartCount) + if pod.Spec.TerminationGracePeriodSeconds != nil { labels[kubernetesTerminationGracePeriodLabel] = strconv.FormatInt(*pod.Spec.TerminationGracePeriodSeconds, 10) } @@ -1485,8 +1468,7 @@ func containerAndPodFromLabels(inspect *docker.Container) (pod *api.Pod, contain // Run a single container from a pod. Returns the docker container ID // If do not need to pass labels, just pass nil. -// TODO (random-liu) Just add labels directly now, maybe should make some abstraction. -func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Container, netMode, ipcMode, pidMode string, labels map[string]string) (kubecontainer.ContainerID, error) { +func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Container, netMode, ipcMode, pidMode string, restartCount int) (kubecontainer.ContainerID, error) { start := time.Now() defer func() { metrics.ContainerManagerLatency.WithLabelValues("runContainerInPod").Observe(metrics.SinceInMicroseconds(start)) @@ -1506,7 +1488,7 @@ func (dm *DockerManager) runContainerInPod(pod *api.Pod, container *api.Containe if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.HostNetwork { utsMode = "host" } - id, err := dm.runContainer(pod, container, opts, ref, netMode, ipcMode, utsMode, pidMode, labels) + id, err := dm.runContainer(pod, container, opts, ref, netMode, ipcMode, utsMode, pidMode, restartCount) if err != nil { return kubecontainer.ContainerID{}, err } @@ -1643,8 +1625,8 @@ func (dm *DockerManager) createPodInfraContainer(pod *api.Pod) (kubetypes.Docker return "", err } - // There is no meaningful labels for infraContainer now, so just pass nil. - id, err := dm.runContainerInPod(pod, container, netNamespace, getIPCMode(pod), getPidMode(pod), nil) + // Currently we don't care about restart count of infra container, just set it to 0. + id, err := dm.runContainerInPod(pod, container, netNamespace, getIPCMode(pod), getPidMode(pod), 0) if err != nil { return "", err } @@ -1905,17 +1887,16 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, pod } } - labels := map[string]string{} containerStatuses := podStatus.ContainerStatuses // podStatus is generated by GetPodStatus(). In GetPodStatus(), we make sure that ContainerStatuses // contains statuses of all containers in pod.Spec.Containers. // ContainerToStart is a subset of pod.Spec.Containers, we should always find a result here. // For a new container, the RestartCount should be 0 - labels[kubernetesContainerRestartCountLabel] = "0" + restartCount := 0 for _, containerStatus := range containerStatuses { // If the container's terminate state is not empty, it exited before. Increment the restart count. if containerStatus.Name == container.Name && (containerStatus.State.Terminated != nil || containerStatus.LastTerminationState.Terminated != nil) { - labels[kubernetesContainerRestartCountLabel] = strconv.Itoa(containerStatus.RestartCount + 1) + restartCount = containerStatus.RestartCount + 1 break } } @@ -1926,7 +1907,7 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, pod // and IPC namespace. PID mode cannot point to another container right now. // See createPodInfraContainer for infra container setup. namespaceMode := fmt.Sprintf("container:%v", podInfraContainerID) - _, err = dm.runContainerInPod(pod, container, namespaceMode, namespaceMode, getPidMode(pod), labels) + _, err = dm.runContainerInPod(pod, container, namespaceMode, namespaceMode, getPidMode(pod), restartCount) dm.updateReasonCache(pod, container, kubecontainer.ErrRunContainer.Error(), err) if err != nil { // TODO(bburns) : Perhaps blacklist a container after N failures? diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index b274360a3c..7f28b97bdb 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/pkg/kubelet/dockertools/manager_test.go @@ -1169,6 +1169,17 @@ func TestGetPodStatusWithLastTermination(t *testing.T) { {Name: "failed"}, } + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{ + UID: "12345678", + Name: "foo", + Namespace: "new", + }, + Spec: api.PodSpec{ + Containers: containers, + }, + } + exitedAPIContainers := []docker.APIContainers{ { // format is // k8s___ @@ -1248,17 +1259,7 @@ func TestGetPodStatusWithLastTermination(t *testing.T) { fakeDocker.ExitedContainerList = exitedAPIContainers fakeDocker.ContainerMap = containerMap fakeDocker.ClearCalls() - pod := &api.Pod{ - ObjectMeta: api.ObjectMeta{ - UID: "12345678", - Name: "foo", - Namespace: "new", - }, - Spec: api.PodSpec{ - Containers: containers, - RestartPolicy: tt.policy, - }, - } + pod.Spec.RestartPolicy = tt.policy fakeDocker.ContainerList = []docker.APIContainers{ { // pod infra container