Put TerminationGracePeriod, DeletionGracePeriod and PreStopHandler to docker labels

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"
+	kubernetesPodNameLabel                   = "io.kubernetes.pod.name"
-	kubernetesPodNamespaceLabel = "io.kubernetes.pod.namespace"
+	kubernetesPodNamespaceLabel              = "io.kubernetes.pod.namespace"
-	kubernetesPodUIDLabel       = "io.kubernetes.pod.uid"
+	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"
+	// TODO(random-liu): Keep this for old containers, remove this when we drop support for v1.1.
-	kubernetesTerminationGracePeriodLabel = "io.kubernetes.pod.terminationGracePeriod"
+	kubernetesPodLabel = "io.kubernetes.pod.data"
-	kubernetesContainerLabel              = "io.kubernetes.container.name"
 )
 
 // 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
+	PodName                   string
-	PodNamespace           string
+	PodNamespace              string
-	PodUID                 types.UID
+	PodUID                    types.UID
-	Name                   string
+	PodDeletionGracePeriod    *int64
-	Hash                   string
+	PodTerminationGracePeriod *int64
-	RestartCount           int
+	Name                      string
-	TerminationMessagePath 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)
+	if containerInfo.RestartCount, err = getIntValueFromLabel(labels, kubernetesContainerRestartCountLabel); err != nil {
-	return &containerInfo, err
+		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)
+}

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,
+		PodName:                   pod.Name,
-		PodNamespace:           pod.Namespace,
+		PodNamespace:              pod.Namespace,
-		PodUID:                 pod.UID,
+		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)
+	containerInfo := getContainerInfoFromLabel(labels)
-	if err != nil {
+	if !reflect.DeepEqual(containerInfo, expected) {
-		t.Errorf("Unexpected error when getContainerInfoFromLabel: %v", err)
+		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)
 	}

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 {
+	containerInfo = getContainerInfoFromLabel(iResult.Config.Labels)
-		glog.Errorf("Get labelled container info error for container %v: %v", id, err)
-	}
 
 	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
+	// while the Kubelet is down and there is no information available to recover the pod.
-	// termination information like the termination grace period and the pre stop hooks.
 	// TODO: keep these labels up to date if the pod changes
 	labels := newLabels(container, pod, restartCount)
 
-	if pod.Spec.TerminationGracePeriodSeconds != nil {
+	// TODO(random-liu): Remove this when we start to use new labels for KillContainerInPod
-		labels[kubernetesTerminationGracePeriodLabel] = strconv.FormatInt(*pod.Spec.TerminationGracePeriodSeconds, 10)
-	}
 	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

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),
+					kubernetesPodLabel:           string(podString),
-					kubernetesContainerLabel: "foo",
+					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 := getContainerInfoFromLabel(inspectResult.Config.Labels)
-	containerInfo, err = getContainerInfoFromLabel(inspectResult.Config.Labels)
-	if err != nil {
-		t.Fatalf("Unexpected error when getContainerInfoFromLabel: %v", err)
-	}
 	terminationMessagePath := containerInfo.TerminationMessagePath
 	if terminationMessagePath != containers[0].TerminationMessagePath {
 		t.Errorf("expected termination message path %s, got %s", containers[0].TerminationMessagePath, terminationMessagePath)