diff --git a/pkg/kubelet/dockertools/labels.go b/pkg/kubelet/dockertools/labels.go index c74e137754f..3925a8c7472 100644 --- a/pkg/kubelet/dockertools/labels.go +++ b/pkg/kubelet/dockertools/labels.go @@ -17,11 +17,14 @@ limitations under the License. package dockertools import ( + "encoding/json" "strconv" "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/latest" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" + "k8s.io/kubernetes/pkg/kubelet/util/format" "k8s.io/kubernetes/pkg/types" ) @@ -30,49 +33,69 @@ import ( // * label filters (maybe in the future) const ( - kubernetesPodNameLabel = "io.kubernetes.pod.name" - kubernetesPodNamespaceLabel = "io.kubernetes.pod.namespace" - kubernetesPodUIDLabel = "io.kubernetes.pod.uid" + kubernetesPodNameLabel = "io.kubernetes.pod.name" + kubernetesPodNamespaceLabel = "io.kubernetes.pod.namespace" + kubernetesPodUIDLabel = "io.kubernetes.pod.uid" + kubernetesPodDeletionGracePeriodLabel = "io.kubernetes.pod.deletionGracePeriod" + kubernetesPodTerminationGracePeriodLabel = "io.kubernetes.pod.terminationGracePeriod" kubernetesContainerNameLabel = "io.kubernetes.container.name" kubernetesContainerHashLabel = "io.kubernetes.container.hash" kubernetesContainerRestartCountLabel = "io.kubernetes.container.restartCount" kubernetesContainerTerminationMessagePathLabel = "io.kubernetes.container.terminationMessagePath" + kubernetesContainerPreStopHandlerLabel = "io.kubernetes.container.preStopHandler" - kubernetesPodLabel = "io.kubernetes.pod.data" - kubernetesTerminationGracePeriodLabel = "io.kubernetes.pod.terminationGracePeriod" - kubernetesContainerLabel = "io.kubernetes.container.name" + // TODO(random-liu): Keep this for old containers, remove this when we drop support for v1.1. + kubernetesPodLabel = "io.kubernetes.pod.data" ) // Container information which has been labelled on each docker container +// TODO(random-liu): The type of Hash should be compliance with kubelet container status. type labelledContainerInfo struct { - PodName string - PodNamespace string - PodUID types.UID - Name string - Hash string - RestartCount int - TerminationMessagePath string + PodName string + PodNamespace string + PodUID types.UID + PodDeletionGracePeriod *int64 + PodTerminationGracePeriod *int64 + Name string + Hash string + RestartCount int + TerminationMessagePath string + PreStopHandler *api.Handler } 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[kubernetesPodUIDLabel] = string(pod.UID) + if pod.DeletionGracePeriodSeconds != nil { + labels[kubernetesPodDeletionGracePeriodLabel] = strconv.FormatInt(*pod.DeletionGracePeriodSeconds, 10) + } + if pod.Spec.TerminationGracePeriodSeconds != nil { + labels[kubernetesPodTerminationGracePeriodLabel] = strconv.FormatInt(*pod.Spec.TerminationGracePeriodSeconds, 10) + } labels[kubernetesContainerNameLabel] = container.Name labels[kubernetesContainerHashLabel] = strconv.FormatUint(kubecontainer.HashContainer(container), 16) labels[kubernetesContainerRestartCountLabel] = strconv.Itoa(restartCount) labels[kubernetesContainerTerminationMessagePathLabel] = container.TerminationMessagePath + if container.Lifecycle != nil && container.Lifecycle.PreStop != nil { + // Using json enconding so that the PreStop handler object is readable after writing as a label + rawPreStop, err := json.Marshal(container.Lifecycle.PreStop) + if err != nil { + glog.Errorf("Unable to marshal lifecycle PreStop handler for container %q of pod %q: %v", container.Name, format.Pod(pod), err) + } else { + labels[kubernetesContainerPreStopHandlerLabel] = string(rawPreStop) + } + } return labels } -func getContainerInfoFromLabel(labels map[string]string) (*labelledContainerInfo, error) { +func getContainerInfoFromLabel(labels map[string]string) *labelledContainerInfo { var err error - containerInfo := labelledContainerInfo{ + containerInfo := &labelledContainerInfo{ PodName: getStringValueFromLabel(labels, kubernetesPodNameLabel), PodNamespace: getStringValueFromLabel(labels, kubernetesPodNamespaceLabel), PodUID: types.UID(getStringValueFromLabel(labels, kubernetesPodUIDLabel)), @@ -80,8 +103,23 @@ func getContainerInfoFromLabel(labels map[string]string) (*labelledContainerInfo Hash: getStringValueFromLabel(labels, kubernetesContainerHashLabel), TerminationMessagePath: getStringValueFromLabel(labels, kubernetesContainerTerminationMessagePathLabel), } - containerInfo.RestartCount, err = getIntValueFromLabel(labels, kubernetesContainerRestartCountLabel) - return &containerInfo, err + if containerInfo.RestartCount, err = getIntValueFromLabel(labels, kubernetesContainerRestartCountLabel); err != nil { + logError(containerInfo, kubernetesContainerRestartCountLabel, err) + } + if containerInfo.PodDeletionGracePeriod, err = getInt64PointerFromLabel(labels, kubernetesPodDeletionGracePeriodLabel); err != nil { + logError(containerInfo, kubernetesPodDeletionGracePeriodLabel, err) + } + if containerInfo.PodTerminationGracePeriod, err = getInt64PointerFromLabel(labels, kubernetesPodTerminationGracePeriodLabel); err != nil { + logError(containerInfo, kubernetesPodTerminationGracePeriodLabel, err) + } + preStopHandler := &api.Handler{} + if found, err := getJsonObjectFromLabel(labels, kubernetesContainerPreStopHandlerLabel, preStopHandler); err != nil { + logError(containerInfo, kubernetesContainerPreStopHandlerLabel, err) + } else if found { + containerInfo.PreStopHandler = preStopHandler + } + supplyContainerInfoWithOldLabel(labels, containerInfo) + return containerInfo } func getStringValueFromLabel(labels map[string]string, label string) string { @@ -108,3 +146,78 @@ func getIntValueFromLabel(labels map[string]string, label string) (int, error) { // Just set the value to 0 return 0, nil } + +func getInt64PointerFromLabel(labels map[string]string, label string) (*int64, error) { + if strValue, found := labels[label]; found { + int64Value, err := strconv.ParseInt(strValue, 10, 64) + if err != nil { + return nil, err + } + return &int64Value, nil + } + // Because it's normal that a container has no PodDeletionGracePeriod and PodTerminationGracePeriod label, + // don't report any error here. + return nil, nil +} + +// getJsonObjectFromLabel returns a bool value indicating whether an object is found +func getJsonObjectFromLabel(labels map[string]string, label string, value interface{}) (bool, error) { + if strValue, found := labels[label]; found { + err := json.Unmarshal([]byte(strValue), value) + return found, err + } + // Because it's normal that a container has no PreStopHandler label, don't report any error here. + return false, nil +} + +// The label kubernetesPodLabel is added a long time ago (#7421), it serialized the whole api.Pod to a docker label. +// We want to remove this label because it serialized too much useless information. However kubelet may still work +// with old containers which only have this label for a long time until we completely deprecate the old label. +// Before that to ensure correctness we have to supply information with the old labels when newly added labels +// are not available. +// TODO(random-liu): Remove this function when we can completely remove label kubernetesPodLabel, probably after +// dropping support for v1.1. +func supplyContainerInfoWithOldLabel(labels map[string]string, containerInfo *labelledContainerInfo) { + // Get api.Pod from old label + var pod *api.Pod + data, found := labels[kubernetesPodLabel] + if !found { + // Don't report any error here, because it's normal that a container has no pod label, especially + // when we gradually deprecate the old label + return + } + pod = &api.Pod{} + err := latest.GroupOrDie(api.GroupName).Codec.DecodeInto([]byte(data), pod) + if err != nil { + // If the pod label can't be parsed, we should report an error + logError(containerInfo, kubernetesPodLabel, err) + return + } + if containerInfo.PodDeletionGracePeriod == nil { + containerInfo.PodDeletionGracePeriod = pod.DeletionGracePeriodSeconds + } + if containerInfo.PodTerminationGracePeriod == nil { + containerInfo.PodTerminationGracePeriod = pod.Spec.TerminationGracePeriodSeconds + } + + // Get api.Container from api.Pod + var container *api.Container + for i := range pod.Spec.Containers { + if pod.Spec.Containers[i].Name == containerInfo.Name { + container = &pod.Spec.Containers[i] + break + } + } + if container == nil { + glog.Errorf("Unable to find container %q in pod %q", containerInfo.Name, format.Pod(pod)) + return + } + if containerInfo.PreStopHandler == nil && container.Lifecycle != nil { + containerInfo.PreStopHandler = container.Lifecycle.PreStop + } +} + +func logError(containerInfo *labelledContainerInfo, label string, err error) { + glog.Errorf("Unable to get %q for container %q of pod %q: %v", label, containerInfo.Name, + kubecontainer.BuildPodFullName(containerInfo.PodName, containerInfo.PodNamespace), err) +} diff --git a/pkg/kubelet/dockertools/labels_test.go b/pkg/kubelet/dockertools/labels_test.go index 27370f49af9..ea882887f7f 100644 --- a/pkg/kubelet/dockertools/labels_test.go +++ b/pkg/kubelet/dockertools/labels_test.go @@ -22,37 +22,102 @@ import ( "testing" "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/latest" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" + "k8s.io/kubernetes/pkg/kubelet/util/format" + "k8s.io/kubernetes/pkg/util/intstr" ) func TestLabels(t *testing.T) { restartCount := 5 + deletionGracePeriod := int64(10) + terminationGracePeriod := int64(10) + lifecycle := &api.Lifecycle{ + // Left PostStart as nil + PreStop: &api.Handler{ + Exec: &api.ExecAction{ + Command: []string{"action1", "action2"}, + }, + HTTPGet: &api.HTTPGetAction{ + Path: "path", + Host: "host", + Port: intstr.FromInt(8080), + Scheme: "scheme", + }, + TCPSocket: &api.TCPSocketAction{ + Port: intstr.FromString("80"), + }, + }, + } container := &api.Container{ Name: "test_container", TerminationMessagePath: "/tmp", + Lifecycle: lifecycle, } pod := &api.Pod{ ObjectMeta: api.ObjectMeta{ Name: "test_pod", Namespace: "test_pod_namespace", UID: "test_pod_uid", + DeletionGracePeriodSeconds: &deletionGracePeriod, + }, + Spec: api.PodSpec{ + Containers: []api.Container{*container}, + TerminationGracePeriodSeconds: &terminationGracePeriod, }, } expected := &labelledContainerInfo{ - PodName: pod.Name, - PodNamespace: pod.Namespace, - PodUID: pod.UID, + PodName: pod.Name, + PodNamespace: pod.Namespace, + PodUID: pod.UID, + PodDeletionGracePeriod: pod.DeletionGracePeriodSeconds, + PodTerminationGracePeriod: pod.Spec.TerminationGracePeriodSeconds, Name: container.Name, Hash: strconv.FormatUint(kubecontainer.HashContainer(container), 16), RestartCount: restartCount, TerminationMessagePath: container.TerminationMessagePath, + PreStopHandler: container.Lifecycle.PreStop, } + // Test whether we can get right information from label labels := newLabels(container, pod, restartCount) - containerInfo, err := getContainerInfoFromLabel(labels) - if err != nil { - t.Errorf("Unexpected error when getContainerInfoFromLabel: %v", err) + containerInfo := getContainerInfoFromLabel(labels) + if !reflect.DeepEqual(containerInfo, expected) { + t.Errorf("expected %v, got %v", expected, containerInfo) } + + // Test when DeletionGracePeriodSeconds, TerminationGracePeriodSeconds and Lifecycle are nil, + // the information got from label should also be nil + container.Lifecycle = nil + pod.DeletionGracePeriodSeconds = nil + pod.Spec.TerminationGracePeriodSeconds = nil + expected.PodDeletionGracePeriod = nil + expected.PodTerminationGracePeriod = nil + expected.PreStopHandler = nil + // Because container is changed, the Hash should be updated + expected.Hash = strconv.FormatUint(kubecontainer.HashContainer(container), 16) + labels = newLabels(container, pod, restartCount) + containerInfo = getContainerInfoFromLabel(labels) + if !reflect.DeepEqual(containerInfo, expected) { + t.Errorf("expected %v, got %v", expected, containerInfo) + } + + // Test when DeletionGracePeriodSeconds, TerminationGracePeriodSeconds and Lifecycle are nil, + // but the old label kubernetesPodLabel is set, the information got from label should also be set + pod.DeletionGracePeriodSeconds = &deletionGracePeriod + pod.Spec.TerminationGracePeriodSeconds = &terminationGracePeriod + container.Lifecycle = lifecycle + data, err := latest.GroupOrDie(api.GroupName).Codec.Encode(pod) + if err != nil { + t.Fatalf("Failed to encode pod %q into string: %v", format.Pod(pod), err) + } + labels[kubernetesPodLabel] = string(data) + expected.PodDeletionGracePeriod = pod.DeletionGracePeriodSeconds + expected.PodTerminationGracePeriod = pod.Spec.TerminationGracePeriodSeconds + expected.PreStopHandler = container.Lifecycle.PreStop + // Do not update expected.Hash here, because we directly use the labels in last test, so we never + // changed the kubernetesContainerHashLabel in this test, the expected.Hash shouldn't be changed. + containerInfo = getContainerInfoFromLabel(labels) if !reflect.DeepEqual(containerInfo, expected) { t.Errorf("expected %v, got %v", expected, containerInfo) } diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index dd9e38020ea..d751e34fd05 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -347,9 +347,7 @@ func (dm *DockerManager) inspectContainer(id string, podName, podNamespace strin containerName := dockerName.ContainerName var containerInfo *labelledContainerInfo - if containerInfo, err = getContainerInfoFromLabel(iResult.Config.Labels); err != nil { - glog.Errorf("Get labelled container info error for container %v: %v", id, err) - } + containerInfo = getContainerInfoFromLabel(iResult.Config.Labels) status := kubecontainer.ContainerStatus{ Name: containerName, @@ -649,14 +647,11 @@ func (dm *DockerManager) runContainer( } // Pod information is recorded on the container as labels to preserve it in the event the pod is deleted - // 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. + // while the Kubelet is down and there is no information available to recover the pod. // TODO: keep these labels up to date if the pod changes labels := newLabels(container, pod, restartCount) - if pod.Spec.TerminationGracePeriodSeconds != nil { - labels[kubernetesTerminationGracePeriodLabel] = strconv.FormatInt(*pod.Spec.TerminationGracePeriodSeconds, 10) - } + // TODO(random-liu): Remove this when we start to use new labels for KillContainerInPod if container.Lifecycle != nil && container.Lifecycle.PreStop != nil { // TODO: This is kind of hacky, we should really just encode the bits we need. data, err := latest.GroupOrDie(api.GroupName).Codec.Encode(pod) @@ -664,7 +659,6 @@ func (dm *DockerManager) runContainer( glog.Errorf("Failed to encode pod: %s for prestop hook", pod.Name) } else { labels[kubernetesPodLabel] = string(data) - labels[kubernetesContainerLabel] = container.Name } } memoryLimit := container.Resources.Limits.Memory().Value() @@ -1226,6 +1220,8 @@ func (dm *DockerManager) GetContainerIP(containerID, interfaceName string) (stri // TODO(random-liu): Change running pod to pod status in the future. We can't do it now, because kubelet also uses this function without pod status. // We can only deprecate this after refactoring kubelet. +// TODO(random-liu): After using pod status for KillPod(), we can also remove the kubernetesPodLabel, because all the needed information should have +// been extract from new labels and stored in pod status. func (dm *DockerManager) KillPod(pod *api.Pod, runningPod kubecontainer.Pod) error { // Send the kills in parallel since they may take a long time. Len + 1 since there // can be Len errors + the networkPlugin teardown error. @@ -1411,7 +1407,7 @@ func containerAndPodFromLabels(inspect *docker.Container) (pod *api.Pod, contain if body, found := labels[kubernetesPodLabel]; found { pod = &api.Pod{} if err = latest.GroupOrDie(api.GroupName).Codec.DecodeInto([]byte(body), pod); err == nil { - name := labels[kubernetesContainerLabel] + name := labels[kubernetesContainerNameLabel] for ix := range pod.Spec.Containers { if pod.Spec.Containers[ix].Name == name { container = &pod.Spec.Containers[ix] @@ -1429,7 +1425,7 @@ func containerAndPodFromLabels(inspect *docker.Container) (pod *api.Pod, contain // attempt to find the default grace period if we didn't commit a pod, but set the generic metadata // field (the one used by kill) if pod == nil { - if period, ok := labels[kubernetesTerminationGracePeriodLabel]; ok { + if period, ok := labels[kubernetesPodTerminationGracePeriodLabel]; ok { if seconds, err := strconv.ParseInt(period, 10, 64); err == nil { pod = &api.Pod{} pod.DeletionGracePeriodSeconds = &seconds diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index af3a4707656..612d6d04a12 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/pkg/kubelet/dockertools/manager_test.go @@ -458,8 +458,8 @@ func TestKillContainerInPodWithPreStop(t *testing.T) { Name: "/k8s_foo_qux_new_1234_42", Config: &docker.Config{ Labels: map[string]string{ - kubernetesPodLabel: string(podString), - kubernetesContainerLabel: "foo", + kubernetesPodLabel: string(podString), + kubernetesContainerNameLabel: "foo", }, }, }, @@ -1440,11 +1440,7 @@ func TestGetTerminationMessagePath(t *testing.T) { if err != nil { t.Fatalf("Unexpected inspect error: %v", err) } - var containerInfo *labelledContainerInfo - containerInfo, err = getContainerInfoFromLabel(inspectResult.Config.Labels) - if err != nil { - t.Fatalf("Unexpected error when getContainerInfoFromLabel: %v", err) - } + containerInfo := getContainerInfoFromLabel(inspectResult.Config.Labels) terminationMessagePath := containerInfo.TerminationMessagePath if terminationMessagePath != containers[0].TerminationMessagePath { t.Errorf("expected termination message path %s, got %s", containers[0].TerminationMessagePath, terminationMessagePath)