From 72524e45b5c09a27e1e71eca0e78102b81c8481b Mon Sep 17 00:00:00 2001 From: Random-Liu Date: Thu, 22 Sep 2016 17:50:36 -0700 Subject: [PATCH] Handle pod spec missing case in kuberuntime. --- .../kuberuntime/kuberuntime_container.go | 90 ++++++++++++++----- .../kuberuntime/kuberuntime_manager.go | 2 +- 2 files changed, 70 insertions(+), 22 deletions(-) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index ff9ca90ad1b..66219115355 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go @@ -103,7 +103,7 @@ func (m *kubeGenericRuntimeManager) startContainer(podSandboxID string, podSandb if handlerErr != nil { err := fmt.Errorf("PostStart handler: %v", handlerErr) m.generateContainerEvent(kubeContainerID, api.EventTypeWarning, events.FailedPostStartHook, msg) - m.killContainer(pod, kubeContainerID, container, "FailedPostStartHook", nil) + m.killContainer(pod, kubeContainerID, container.Name, "FailedPostStartHook", nil) return "PostStart Hook Failed", err } } @@ -423,24 +423,83 @@ func (m *kubeGenericRuntimeManager) executePreStopHook(pod *api.Pod, containerID return int64(unversioned.Now().Sub(start.Time).Seconds()) } +// restoreSpecsFromContainerLabels restores all information needed for killing a container. In some +// case we may not have pod and container spec when killing a container, e.g. pod is deleted during +// kubelet restart. +// To solve this problem, we've already written necessary information into container labels. Here we +// just need to retrieve them from container labels and restore the specs. +// TODO(random-liu): Add a node e2e test to test this behaviour. +// TODO(random-liu): Change the lifecycle handler to just accept information needed, so that we can +// just pass the needed function not create the fake object. +func (m *kubeGenericRuntimeManager) restoreSpecsFromContainerLabels(containerID kubecontainer.ContainerID) (*api.Pod, *api.Container, error) { + var pod *api.Pod + var container *api.Container + s, err := m.runtimeService.ContainerStatus(containerID.ID) + if err != nil { + return nil, nil, err + } + + l := getContainerInfoFromLabels(s.Labels) + a := getContainerInfoFromAnnotations(s.Annotations) + // Notice that the followings are not full spec. The container killing code should not use + // un-restored fields. + pod = &api.Pod{ + ObjectMeta: api.ObjectMeta{ + UID: l.PodUID, + Name: l.PodName, + Namespace: l.PodNamespace, + DeletionGracePeriodSeconds: a.PodDeletionGracePeriod, + }, + Spec: api.PodSpec{ + TerminationGracePeriodSeconds: a.PodTerminationGracePeriod, + }, + } + container = &api.Container{ + Name: l.ContainerName, + Ports: a.ContainerPorts, + TerminationMessagePath: a.TerminationMessagePath, + } + if a.PreStopHandler != nil { + container.Lifecycle = &api.Lifecycle{ + PreStop: a.PreStopHandler, + } + } + return pod, container, nil +} + // killContainer kills a container through the following steps: // * Run the pre-stop lifecycle hooks (if applicable). // * Stop the container. -func (m *kubeGenericRuntimeManager) killContainer(pod *api.Pod, containerID kubecontainer.ContainerID, containerSpec *api.Container, reason string, gracePeriodOverride *int64) error { - gracePeriod := int64(minimumGracePeriodInSeconds) +func (m *kubeGenericRuntimeManager) killContainer(pod *api.Pod, containerID kubecontainer.ContainerID, containerName string, reason string, gracePeriodOverride *int64) error { + var containerSpec *api.Container if pod != nil { - switch { - case pod.DeletionGracePeriodSeconds != nil: - gracePeriod = *pod.DeletionGracePeriodSeconds - case pod.Spec.TerminationGracePeriodSeconds != nil: - gracePeriod = *pod.Spec.TerminationGracePeriodSeconds + for i, c := range pod.Spec.Containers { + if containerName == c.Name { + containerSpec = &pod.Spec.Containers[i] + break + } } + } else { + // Restore necessary information if one of the specs is nil. + restoredPod, restoredContainer, err := m.restoreSpecsFromContainerLabels(containerID) + if err != nil { + return err + } + pod, containerSpec = restoredPod, restoredContainer + } + // From this point , pod and container must be non-nil. + gracePeriod := int64(minimumGracePeriodInSeconds) + switch { + case pod.DeletionGracePeriodSeconds != nil: + gracePeriod = *pod.DeletionGracePeriodSeconds + case pod.Spec.TerminationGracePeriodSeconds != nil: + gracePeriod = *pod.Spec.TerminationGracePeriodSeconds } glog.V(2).Infof("Killing container %q with %d second grace period", containerID.String(), gracePeriod) // Run the pre-stop lifecycle hooks if applicable. - if pod != nil && containerSpec != nil && containerSpec.Lifecycle != nil && containerSpec.Lifecycle.PreStop != nil { + if containerSpec.Lifecycle != nil && containerSpec.Lifecycle.PreStop != nil { gracePeriod = gracePeriod - m.executePreStopHook(pod, containerID, containerSpec, gracePeriod) } // always give containers a minimal shutdown window to avoid unnecessary SIGKILLs @@ -479,19 +538,8 @@ func (m *kubeGenericRuntimeManager) killContainersWithSyncResult(pod *api.Pod, r go func(container *kubecontainer.Container) { defer utilruntime.HandleCrash() defer wg.Done() - - var containerSpec *api.Container - if pod != nil { - for i, c := range pod.Spec.Containers { - if container.Name == c.Name { - containerSpec = &pod.Spec.Containers[i] - break - } - } - } - killContainerResult := kubecontainer.NewSyncResult(kubecontainer.KillContainer, container.Name) - if err := m.killContainer(pod, container.ID, containerSpec, "Need to kill Pod", gracePeriodOverride); err != nil { + if err := m.killContainer(pod, container.ID, container.Name, "Need to kill Pod", gracePeriodOverride); err != nil { killContainerResult.Fail(kubecontainer.ErrKillContainer, err.Error()) } containerResults <- killContainerResult diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index 855e6b440af..6473725373a 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -499,7 +499,7 @@ func (m *kubeGenericRuntimeManager) SyncPod(pod *api.Pod, _ api.PodStatus, podSt glog.V(3).Infof("Killing unwanted container %q(id=%q) for pod %q", containerInfo.name, containerID, format.Pod(pod)) killContainerResult := kubecontainer.NewSyncResult(kubecontainer.KillContainer, containerInfo.name) result.AddSyncResult(killContainerResult) - if err := m.killContainer(pod, containerID, containerInfo.container, containerInfo.message, nil); err != nil { + if err := m.killContainer(pod, containerID, containerInfo.name, containerInfo.message, nil); err != nil { killContainerResult.Fail(kubecontainer.ErrKillContainer, err.Error()) glog.Errorf("killContainer %q(id=%q) for pod %q failed: %v", containerInfo.name, containerID, format.Pod(pod), err) return