diff --git a/pkg/registry/pod/etcd/etcd_test.go b/pkg/registry/pod/etcd/etcd_test.go index fe977a48ed7..155e60d4346 100644 --- a/pkg/registry/pod/etcd/etcd_test.go +++ b/pkg/registry/pod/etcd/etcd_test.go @@ -142,8 +142,10 @@ func TestDelete(t *testing.T) { key = etcdtest.AddPrefix(key) test := resttest.New(t, storage, fakeClient.SetError) + expectedNode := "some-node" createFn := func() runtime.Object { pod := validChangedPod() + pod.Spec.NodeName = expectedNode fakeClient.Data[key] = tools.EtcdResponseWithError{ R: &etcd.Response{ Node: &etcd.Node{ @@ -158,8 +160,17 @@ func TestDelete(t *testing.T) { if fakeClient.Data[key].R.Node == nil { return false } - return fakeClient.Data[key].R.Node.TTL == 30 + obj, err := testapi.Codec().Decode([]byte(fakeClient.Data[key].R.Node.Value)) + if err != nil { + return false + } + pod := obj.(*api.Pod) + t.Logf("found object %#v", pod.ObjectMeta) + return pod.DeletionTimestamp != nil && pod.DeletionGracePeriodSeconds != nil && *pod.DeletionGracePeriodSeconds != 0 } + test.TestDeleteGraceful(createFn, 30, gracefulSetFn) + + expectedNode = "" test.TestDelete(createFn, gracefulSetFn) } diff --git a/pkg/registry/pod/strategy.go b/pkg/registry/pod/strategy.go index ef10127e658..622c689bdba 100644 --- a/pkg/registry/pod/strategy.go +++ b/pkg/registry/pod/strategy.go @@ -81,15 +81,49 @@ func (podStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) fiel return append(errorList, validation.ValidatePodUpdate(obj.(*api.Pod), old.(*api.Pod))...) } +// AllowUnconditionalUpdate allows pods to be overwritten func (podStrategy) AllowUnconditionalUpdate() bool { return true } -// CheckGracefulDelete allows a pod to be gracefully deleted. +// CheckGracefulDelete allows a pod to be gracefully deleted. It updates the DeleteOptions to +// reflect the desired grace value. func (podStrategy) CheckGracefulDelete(obj runtime.Object, options *api.DeleteOptions) bool { + if options == nil { + return false + } + pod := obj.(*api.Pod) + period := int64(0) + // user has specified a value + if options.GracePeriodSeconds != nil { + period = *options.GracePeriodSeconds + } else { + // use the default value if set, or deletes the pod immediately (0) + if pod.Spec.TerminationGracePeriodSeconds != nil { + period = *pod.Spec.TerminationGracePeriodSeconds + } + } + // if the pod is not scheduled, delete immediately + if len(pod.Spec.NodeName) == 0 { + period = 0 + } + // ensure the options and the pod are in sync + options.GracePeriodSeconds = &period + return true +} + +type podStrategyWithoutGraceful struct { + podStrategy +} + +// CheckGracefulDelete prohibits graceful deletion. +func (podStrategyWithoutGraceful) CheckGracefulDelete(obj runtime.Object, options *api.DeleteOptions) bool { return false } +// StrategyWithoutGraceful implements the legacy instant delele behavior. +var StrategyWithoutGraceful = podStrategyWithoutGraceful{Strategy} + type podStatusStrategy struct { podStrategy } diff --git a/test/e2e/pods.go b/test/e2e/pods.go index de881d1e567..4b9623d9f7b 100644 --- a/test/e2e/pods.go +++ b/test/e2e/pods.go @@ -255,7 +255,7 @@ var _ = Describe("Pods", func() { Fail("Timeout while waiting for pod creation") } - By("deleting the pod") + By("deleting the pod gracefully") if err := podClient.Delete(pod.Name, nil); err != nil { Failf("Failed to delete pod: %v", err) } @@ -263,11 +263,13 @@ var _ = Describe("Pods", func() { By("verifying pod deletion was observed") deleted := false timeout := false + var lastPod *api.Pod timer := time.After(podStartTimeout) for !deleted && !timeout { select { case event, _ := <-w.ResultChan(): if event.Type == watch.Deleted { + lastPod = event.Object.(*api.Pod) deleted = true } case <-timer: @@ -278,6 +280,9 @@ var _ = Describe("Pods", func() { Fail("Failed to observe pod deletion") } + Expect(lastPod.DeletionTimestamp).ToNot(BeNil()) + Expect(lastPod.Spec.TerminationGracePeriodSeconds).ToNot(BeZero()) + pods, err = podClient.List(labels.SelectorFromSet(labels.Set(map[string]string{"time": value})), fields.Everything()) if err != nil { Fail(fmt.Sprintf("Failed to list pods to verify deletion: %v", err)) diff --git a/test/e2e/util.go b/test/e2e/util.go index 1f6e96f0677..c514f4a3050 100644 --- a/test/e2e/util.go +++ b/test/e2e/util.go @@ -259,8 +259,9 @@ func podReady(pod *api.Pod) bool { // logPodStates logs basic info of provided pods for debugging. func logPodStates(pods []api.Pod) { // Find maximum widths for pod, node, and phase strings for column printing. - maxPodW, maxNodeW, maxPhaseW := len("POD"), len("NODE"), len("PHASE") - for _, pod := range pods { + maxPodW, maxNodeW, maxPhaseW, maxGraceW := len("POD"), len("NODE"), len("PHASE"), len("GRACE") + for i := range pods { + pod := &pods[i] if len(pod.ObjectMeta.Name) > maxPodW { maxPodW = len(pod.ObjectMeta.Name) } @@ -275,13 +276,18 @@ func logPodStates(pods []api.Pod) { maxPodW++ maxNodeW++ maxPhaseW++ + maxGraceW++ // Log pod info. * does space padding, - makes them left-aligned. - Logf("%-[1]*[2]s %-[3]*[4]s %-[5]*[6]s %[7]s", - maxPodW, "POD", maxNodeW, "NODE", maxPhaseW, "PHASE", "CONDITIONS") + Logf("%-[1]*[2]s %-[3]*[4]s %-[5]*[6]s %-[7]*[8]s %[9]s", + maxPodW, "POD", maxNodeW, "NODE", maxPhaseW, "PHASE", maxGraceW, "GRACE", "CONDITIONS") for _, pod := range pods { - Logf("%-[1]*[2]s %-[3]*[4]s %-[5]*[6]s %[7]s", - maxPodW, pod.ObjectMeta.Name, maxNodeW, pod.Spec.NodeName, maxPhaseW, pod.Status.Phase, pod.Status.Conditions) + grace := "" + if pod.DeletionGracePeriodSeconds != nil { + grace = fmt.Sprintf("%ds", *pod.DeletionGracePeriodSeconds) + } + Logf("%-[1]*[2]s %-[3]*[4]s %-[5]*[6]s %[7]s %[8]s", + maxPodW, pod.ObjectMeta.Name, maxNodeW, pod.Spec.NodeName, maxPhaseW, pod.Status.Phase, grace, pod.Status.Conditions) } Logf("") // Final empty line helps for readability. } @@ -1350,9 +1356,7 @@ func dumpAllPodInfo(c *client.Client) { if err != nil { Logf("unable to fetch pod debug info: %v", err) } - for _, pod := range pods.Items { - Logf("Pod %s %s node=%s, deletionTimestamp=%s", pod.Namespace, pod.Name, pod.Spec.NodeName, pod.DeletionTimestamp) - } + logPodStates(pods.Items) } func dumpNodeDebugInfo(c *client.Client, nodeNames []string) {