From fdf6a0f61c354928e4d9329841328ab52d37ab00 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Tue, 26 Jan 2016 23:08:50 -0800 Subject: [PATCH 1/2] skip update when deleting with grace-period=0 --- pkg/api/rest/resttest/resttest.go | 3 ++- pkg/registry/generic/etcd/etcd.go | 2 +- test/e2e/pods.go | 8 ++++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/api/rest/resttest/resttest.go b/pkg/api/rest/resttest/resttest.go index 17e0a584220..f170ec7711d 100644 --- a/pkg/api/rest/resttest/resttest.go +++ b/pkg/api/rest/resttest/resttest.go @@ -673,7 +673,8 @@ func (t *Tester) testDeleteGracefulImmediate(obj runtime.Object, setFn SetFunc, t.Errorf("unexpected error, object should be deleted immediately: %v", err) } objectMeta = t.getObjectMetaOrFail(out) - if objectMeta.DeletionTimestamp == nil || objectMeta.DeletionGracePeriodSeconds == nil || *objectMeta.DeletionGracePeriodSeconds != 0 { + // the second delete shouldn't update the object, so the objectMeta.DeletionGracePeriodSeconds should eqaul to the value set in the first delete. + if objectMeta.DeletionTimestamp == nil || objectMeta.DeletionGracePeriodSeconds == nil || *objectMeta.DeletionGracePeriodSeconds != expectedGrace { t.Errorf("unexpected deleted meta: %#v", objectMeta) } } diff --git a/pkg/registry/generic/etcd/etcd.go b/pkg/registry/generic/etcd/etcd.go index a6532d9d42f..d83180dba39 100644 --- a/pkg/registry/generic/etcd/etcd.go +++ b/pkg/registry/generic/etcd/etcd.go @@ -387,7 +387,7 @@ func (e *Etcd) Delete(ctx api.Context, name string, options *api.DeleteOptions) if pendingGraceful { return e.finalizeDelete(obj, false) } - if graceful { + if graceful && *options.GracePeriodSeconds > 0 { out := e.NewFunc() lastGraceful := int64(0) err := e.Storage.GuaranteedUpdate( diff --git a/test/e2e/pods.go b/test/e2e/pods.go index ed432d2a0a4..7f87aac8763 100644 --- a/test/e2e/pods.go +++ b/test/e2e/pods.go @@ -343,8 +343,12 @@ var _ = Describe("Pods", func() { Fail("Timeout while waiting for pod creation") } + // We need to wait for the pod to be scheduled, otherwise the deletion + // will be carried out immediately rather than gracefully. + expectNoError(framework.WaitForPodRunning(pod.Name)) + By("deleting the pod gracefully") - if err := podClient.Delete(pod.Name, nil); err != nil { + if err := podClient.Delete(pod.Name, api.NewDeleteOptions(30)); err != nil { Failf("Failed to delete pod: %v", err) } @@ -352,7 +356,7 @@ var _ = Describe("Pods", func() { deleted := false timeout := false var lastPod *api.Pod - timer := time.After(podStartTimeout) + timer := time.After(30 * time.Second) for !deleted && !timeout { select { case event, _ := <-w.ResultChan(): From a6d96a04d05f546b16b4199bbc8aeda6102aedad Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Sun, 31 Jan 2016 15:56:55 -0800 Subject: [PATCH 2/2] make kubelet.HandlePodsDeletion aware of api.Pod --- pkg/kubelet/container/runtime.go | 8 ++++++ pkg/kubelet/kubelet.go | 42 ++++++++++++++++++-------------- pkg/kubelet/kubelet_test.go | 2 +- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index 4f3dabadbbd..2ad1eba50cc 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -141,6 +141,14 @@ type Pod struct { Containers []*Container } +// PodPair contains both runtime#Pod and api#Pod +type PodPair struct { + // APIPod is the api.Pod + APIPod *api.Pod + // RunningPod is the pod defined defined in pkg/kubelet/container/runtime#Pod + RunningPod *Pod +} + // ContainerID is a type that identifies a container. type ContainerID struct { // The type of the container runtime. e.g. 'docker', 'rkt'. diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index f7ca16a3b7c..e8d4c1d6f39 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -450,7 +450,7 @@ func NewMainKubelet( klet.podWorkers = newPodWorkers(klet.syncPod, recorder, klet.workQueue, klet.resyncInterval, backOffPeriod, klet.podCache) klet.backOff = util.NewBackOff(backOffPeriod, MaxContainerBackOff) - klet.podKillingCh = make(chan *kubecontainer.Pod, podKillingChannelCapacity) + klet.podKillingCh = make(chan *kubecontainer.PodPair, podKillingChannelCapacity) klet.sourcesSeen = sets.NewString() return klet, nil } @@ -632,7 +632,7 @@ type Kubelet struct { backOff *util.Backoff // Channel for sending pods to kill. - podKillingCh chan *kubecontainer.Pod + podKillingCh chan *kubecontainer.PodPair // The configuration file used as the base to generate the container's // DNS resolver configuration file. This can be used in conjunction with @@ -1961,13 +1961,16 @@ func (kl *Kubelet) removeOrphanedPodStatuses(pods []*api.Pod, mirrorPods []*api. kl.statusManager.RemoveOrphanedStatuses(podUIDs) } -func (kl *Kubelet) deletePod(uid types.UID) error { +func (kl *Kubelet) deletePod(pod *api.Pod) error { + if pod == nil { + return fmt.Errorf("deletePod does not allow nil pod") + } if !kl.allSourcesReady() { // If the sources aren't ready, skip deletion, as we may accidentally delete pods // for sources that haven't reported yet. return fmt.Errorf("skipping delete because sources aren't ready yet") } - kl.podWorkers.ForgetWorker(uid) + kl.podWorkers.ForgetWorker(pod.UID) // Runtime cache may not have been updated to with the pod, but it's okay // because the periodic cleanup routine will attempt to delete again later. @@ -1975,12 +1978,13 @@ func (kl *Kubelet) deletePod(uid types.UID) error { if err != nil { return fmt.Errorf("error listing containers: %v", err) } - pod := kubecontainer.Pods(runningPods).FindPod("", uid) - if pod.IsEmpty() { + runningPod := kubecontainer.Pods(runningPods).FindPod("", pod.UID) + if runningPod.IsEmpty() { return fmt.Errorf("pod not found") } + podPair := kubecontainer.PodPair{pod, &runningPod} - kl.podKillingCh <- &pod + kl.podKillingCh <- &podPair // TODO: delete the mirror pod here? // We leave the volume/directory cleanup to the periodic cleanup routine. @@ -2023,7 +2027,7 @@ func (kl *Kubelet) HandlePodCleanups() error { } for _, pod := range runningPods { if _, found := desiredPods[pod.ID]; !found { - kl.podKillingCh <- pod + kl.podKillingCh <- &kubecontainer.PodPair{nil, pod} } } @@ -2082,25 +2086,27 @@ func (kl *Kubelet) podKiller() { defer close(resultCh) for { select { - case pod, ok := <-kl.podKillingCh: + case podPair, ok := <-kl.podKillingCh: + runningPod := podPair.RunningPod + apiPod := podPair.APIPod if !ok { return } - if killing.Has(string(pod.ID)) { + if killing.Has(string(runningPod.ID)) { // The pod is already being killed. break } - killing.Insert(string(pod.ID)) - go func(pod *kubecontainer.Pod, ch chan types.UID) { + killing.Insert(string(runningPod.ID)) + go func(apiPod *api.Pod, runningPod *kubecontainer.Pod, ch chan types.UID) { defer func() { - ch <- pod.ID + ch <- runningPod.ID }() - glog.V(2).Infof("Killing unwanted pod %q", pod.Name) - err := kl.killPod(nil, pod, nil) + glog.V(2).Infof("Killing unwanted pod %q", runningPod.Name) + err := kl.killPod(apiPod, runningPod, nil) if err != nil { - glog.Errorf("Failed killing the pod %q: %v", pod.Name, err) + glog.Errorf("Failed killing the pod %q: %v", runningPod.Name, err) } - }(pod, resultCh) + }(apiPod, runningPod, resultCh) case podID := <-resultCh: killing.Delete(string(podID)) @@ -2388,7 +2394,7 @@ func (kl *Kubelet) HandlePodDeletions(pods []*api.Pod) { } // Deletion is allowed to fail because the periodic cleanup routine // will trigger deletion again. - if err := kl.deletePod(pod.UID); err != nil { + if err := kl.deletePod(pod); err != nil { glog.V(2).Infof("Failed to delete pod %q, err: %v", format.Pod(pod), err) } kl.probeManager.RemovePod(pod) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 6a753d03482..7e64ba86f1b 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -176,7 +176,7 @@ func newTestKubelet(t *testing.T) *TestKubelet { fakeClock := &util.FakeClock{Time: time.Now()} kubelet.backOff = util.NewBackOff(time.Second, time.Minute) kubelet.backOff.Clock = fakeClock - kubelet.podKillingCh = make(chan *kubecontainer.Pod, 20) + kubelet.podKillingCh = make(chan *kubecontainer.PodPair, 20) kubelet.resyncInterval = 10 * time.Second kubelet.reservation = kubetypes.Reservation{ Kubernetes: api.ResourceList{