diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index 491feea71ca..6469410c632 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go @@ -104,7 +104,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 } } @@ -429,24 +429,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 @@ -485,19 +544,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 541e10b763b..af622818f93 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 diff --git a/pkg/kubelet/lifecycle/handlers.go b/pkg/kubelet/lifecycle/handlers.go index 4dbaedfa53c..c4d0d7b7a97 100644 --- a/pkg/kubelet/lifecycle/handlers.go +++ b/pkg/kubelet/lifecycle/handlers.go @@ -63,14 +63,14 @@ func (hr *HandlerRunner) Run(containerID kubecontainer.ContainerID, pod *api.Pod output := ioutils.WriteCloserWrapper(&buffer) err := hr.commandRunner.ExecInContainer(containerID, handler.Exec.Command, nil, output, output, false, nil) if err != nil { - msg := fmt.Sprintf("Exec lifecycle hook (%v) for Container %q in Pod %q failed - %q", handler.Exec.Command, container.Name, format.Pod(pod), buffer.String()) + msg := fmt.Sprintf("Exec lifecycle hook (%v) for Container %q in Pod %q failed - error: %v, message: %q", handler.Exec.Command, container.Name, format.Pod(pod), err, buffer.String()) glog.V(1).Infof(msg) } return msg, err case handler.HTTPGet != nil: msg, err := hr.runHTTPHandler(pod, container, handler) if err != nil { - msg := fmt.Sprintf("Http lifecycle hook (%s) for Container %q in Pod %q failed - %q", handler.HTTPGet.Path, container.Name, format.Pod(pod), msg) + msg := fmt.Sprintf("Http lifecycle hook (%s) for Container %q in Pod %q failed - error: %v, message: %q", handler.HTTPGet.Path, container.Name, format.Pod(pod), err, msg) glog.V(1).Infof(msg) } return msg, err diff --git a/test/e2e/framework/pods.go b/test/e2e/framework/pods.go index 970a7ec12a6..f7c7c618585 100644 --- a/test/e2e/framework/pods.go +++ b/test/e2e/framework/pods.go @@ -149,3 +149,19 @@ func (c *PodClient) mungeSpec(pod *api.Pod) { } // TODO(random-liu): Move pod wait function into this file +// WaitForSuccess waits for pod to success. +func (c *PodClient) WaitForSuccess(name string, timeout time.Duration) { + f := c.f + Expect(waitForPodCondition(f.Client, f.Namespace.Name, name, "success or failure", timeout, + func(pod *api.Pod) (bool, error) { + switch pod.Status.Phase { + case api.PodFailed: + return true, fmt.Errorf("pod %q failed with reason: %q, message: %q", name, pod.Status.Reason, pod.Status.Message) + case api.PodSucceeded: + return true, nil + default: + return false, nil + } + }, + )).To(Succeed(), "wait for pod %q to success", name) +} diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 6c0f9891ba1..069153bb3eb 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1455,6 +1455,7 @@ func waitForPodTerminatedInNamespace(c *client.Client, podName, reason, namespac func waitForPodSuccessInNamespaceTimeout(c *client.Client, podName string, contName string, namespace string, timeout time.Duration) error { return waitForPodCondition(c, namespace, podName, "success or failure", timeout, func(pod *api.Pod) (bool, error) { // Cannot use pod.Status.Phase == api.PodSucceeded/api.PodFailed due to #2632 + // TODO: This was not true from long time ago. We can use api.PodSucceeded now. ci, ok := api.GetContainerStatus(pod.Status.ContainerStatuses, contName) if !ok { Logf("No Status.Info for container '%s' in pod '%s' yet", contName, podName) diff --git a/test/e2e_node/lifecycle_hook_test.go b/test/e2e_node/lifecycle_hook_test.go new file mode 100644 index 00000000000..b09ba102f8c --- /dev/null +++ b/test/e2e_node/lifecycle_hook_test.go @@ -0,0 +1,182 @@ +/* +Copyright 2016 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 e2e_node + +import ( + "fmt" + "time" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/util/intstr" + "k8s.io/kubernetes/pkg/util/uuid" + "k8s.io/kubernetes/test/e2e/framework" + + . "github.com/onsi/ginkgo" +) + +var _ = framework.KubeDescribe("Container Lifecycle Hook", func() { + f := framework.NewDefaultFramework("container-lifecycle-hook") + var podClient *framework.PodClient + var file string + const podWaitTimeout = 2 * time.Minute + + testPodWithHook := func(podWithHook *api.Pod) { + podCheckHook := getLifecycleHookTestPod("pod-check-hook", + // Wait until the file is created. + []string{"sh", "-c", fmt.Sprintf("while [ ! -e %s ]; do sleep 1; done", file)}, + ) + By("create the pod with lifecycle hook") + podClient.CreateSync(podWithHook) + if podWithHook.Spec.Containers[0].Lifecycle.PostStart != nil { + By("create the hook check pod") + podClient.Create(podCheckHook) + By("wait for the hook check pod to success") + podClient.WaitForSuccess(podCheckHook.Name, podWaitTimeout) + } + By("delete the pod with lifecycle hook") + podClient.DeleteSync(podWithHook.Name, api.NewDeleteOptions(15), podWaitTimeout) + if podWithHook.Spec.Containers[0].Lifecycle.PreStop != nil { + By("create the hook check pod") + podClient.Create(podCheckHook) + By("wait for the prestop check pod to success") + podClient.WaitForSuccess(podCheckHook.Name, podWaitTimeout) + } + } + + Context("when create a pod with lifecycle hook", func() { + BeforeEach(func() { + podClient = f.PodClient() + file = "/tmp/test-" + string(uuid.NewUUID()) + }) + + AfterEach(func() { + By("cleanup the temporary file created in the test.") + cleanupPod := getLifecycleHookTestPod("pod-clean-up", []string{"rm", file}) + podClient.Create(cleanupPod) + podClient.WaitForSuccess(cleanupPod.Name, podWaitTimeout) + }) + + Context("when it is exec hook", func() { + It("should execute poststart exec hook properly", func() { + podWithHook := getLifecycleHookTestPod("pod-with-poststart-exec-hook", + // Block forever + []string{"tail", "-f", "/dev/null"}, + ) + podWithHook.Spec.Containers[0].Lifecycle = &api.Lifecycle{ + PostStart: &api.Handler{ + Exec: &api.ExecAction{Command: []string{"touch", file}}, + }, + } + testPodWithHook(podWithHook) + }) + + It("should execute prestop exec hook properly", func() { + podWithHook := getLifecycleHookTestPod("pod-with-prestop-exec-hook", + // Block forever + []string{"tail", "-f", "/dev/null"}, + ) + podWithHook.Spec.Containers[0].Lifecycle = &api.Lifecycle{ + PreStop: &api.Handler{ + Exec: &api.ExecAction{Command: []string{"touch", file}}, + }, + } + testPodWithHook(podWithHook) + }) + }) + + Context("when it is http hook", func() { + var targetIP string + BeforeEach(func() { + By("cleanup the container to handle the HTTPGet hook request.") + podHandleHookRequest := getLifecycleHookTestPod("pod-handle-http-request", + []string{"sh", "-c", + // Create test file when receive request on 1234. + fmt.Sprintf("echo -e \"HTTP/1.1 200 OK\n\" | nc -l -p 1234; touch %s", file), + }, + ) + podHandleHookRequest.Spec.Containers[0].Ports = []api.ContainerPort{ + { + ContainerPort: 1234, + Protocol: api.ProtocolTCP, + }, + } + podHandleHookRequest = podClient.CreateSync(podHandleHookRequest) + targetIP = podHandleHookRequest.Status.PodIP + }) + It("should execute poststart http hook properly", func() { + podWithHook := getLifecycleHookTestPod("pod-with-poststart-http-hook", + // Block forever + []string{"tail", "-f", "/dev/null"}, + ) + podWithHook.Spec.Containers[0].Lifecycle = &api.Lifecycle{ + PostStart: &api.Handler{ + HTTPGet: &api.HTTPGetAction{ + Host: targetIP, + Port: intstr.FromInt(1234), + }, + }, + } + testPodWithHook(podWithHook) + }) + It("should execute prestop http hook properly", func() { + podWithHook := getLifecycleHookTestPod("pod-with-prestop-http-hook", + // Block forever + []string{"tail", "-f", "/dev/null"}, + ) + podWithHook.Spec.Containers[0].Lifecycle = &api.Lifecycle{ + PreStop: &api.Handler{ + HTTPGet: &api.HTTPGetAction{ + Host: targetIP, + Port: intstr.FromInt(1234), + }, + }, + } + testPodWithHook(podWithHook) + }) + }) + }) +}) + +func getLifecycleHookTestPod(name string, cmd []string) *api.Pod { + return &api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: name, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: name, + Image: "gcr.io/google_containers/busybox:1.24", + VolumeMounts: []api.VolumeMount{ + { + Name: "tmpfs", + MountPath: "/tmp", + }, + }, + Command: cmd, + }, + }, + RestartPolicy: api.RestartPolicyNever, + Volumes: []api.Volume{ + { + Name: "tmpfs", + VolumeSource: api.VolumeSource{HostPath: &api.HostPathVolumeSource{Path: "/tmp"}}, + }, + }, + }, + } +}