From 7249c9bd8af96f8e74f2c0fa7c6a43932a488685 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Wed, 21 Sep 2016 17:29:46 -0700 Subject: [PATCH] fix TestCreateWithNonExistentOwner remove the use of gc.QueuesDrained --- .../garbagecollector/garbagecollector.go | 6 -- .../garbage_collector_test.go | 74 ++++++------------- test/integration/utils.go | 20 +++++ 3 files changed, 42 insertions(+), 58 deletions(-) diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go index a4018a7842e..e3beadff917 100644 --- a/pkg/controller/garbagecollector/garbagecollector.go +++ b/pkg/controller/garbagecollector/garbagecollector.go @@ -786,12 +786,6 @@ func (gc *GarbageCollector) Run(workers int, stopCh <-chan struct{}) { gc.propagator.eventQueue.ShutDown() } -// QueueDrained returns if the dirtyQueue and eventQueue are drained. It's -// useful for debugging. Note that it doesn't guarantee the workers are idle. -func (gc *GarbageCollector) QueuesDrained() bool { - return gc.dirtyQueue.Len() == 0 && gc.propagator.eventQueue.Len() == 0 && gc.orphanQueue.Len() == 0 -} - // *FOR TEST USE ONLY* It's not safe to call this function when the GC is still // busy. // GraphHasUID returns if the Propagator has a particular UID store in its diff --git a/test/integration/garbagecollector/garbage_collector_test.go b/test/integration/garbagecollector/garbage_collector_test.go index 3f1e54d3a7b..7830d17cafe 100644 --- a/test/integration/garbagecollector/garbage_collector_test.go +++ b/test/integration/garbagecollector/garbage_collector_test.go @@ -30,11 +30,10 @@ import ( "github.com/golang/glog" dto "github.com/prometheus/client_model/go" "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/api/errors" "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/apimachinery/registered" - clientset "k8s.io/kubernetes/pkg/client/clientset_generated/release_1_3" + clientset "k8s.io/kubernetes/pkg/client/clientset_generated/release_1_5" "k8s.io/kubernetes/pkg/client/restclient" "k8s.io/kubernetes/pkg/client/typed/dynamic" "k8s.io/kubernetes/pkg/controller/garbagecollector" @@ -42,6 +41,7 @@ import ( "k8s.io/kubernetes/pkg/runtime/serializer" "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util/wait" + "k8s.io/kubernetes/test/integration" "k8s.io/kubernetes/test/integration/framework" ) @@ -213,13 +213,6 @@ func TestCascadingDeletion(t *testing.T) { if err := rcClient.Delete(toBeDeletedRCName, getNonOrphanOptions()); err != nil { t.Fatalf("failed to delete replication controller: %v", err) } - - // wait for the garbage collector to drain its queue - if err := wait.Poll(10*time.Second, 120*time.Second, func() (bool, error) { - return gc.QueuesDrained(), nil - }); err != nil { - t.Fatal(err) - } // sometimes the deletion of the RC takes long time to be observed by // the gc, so wait for the garbage collector to observe the deletion of // the toBeDeletedRC @@ -228,32 +221,21 @@ func TestCascadingDeletion(t *testing.T) { }); err != nil { t.Fatal(err) } - // wait for the garbage collector to drain its queue again because it's - // possible it just processed the delete of the toBeDeletedRC. - if err := wait.Poll(10*time.Second, 120*time.Second, func() (bool, error) { - return gc.QueuesDrained(), nil - }); err != nil { - t.Fatal(err) + if err := integration.WaitForPodToDisappear(podClient, garbageCollectedPodName, 5*time.Second, 30*time.Second); err != nil { + t.Fatalf("expect pod %s to be garbage collected, got err= %v", garbageCollectedPodName, err) } - - t.Logf("garbage collector queues drained") - // checks the garbage collect doesn't delete pods it shouldn't do. + // checks the garbage collect doesn't delete pods it shouldn't delete. if _, err := podClient.Get(independentPodName); err != nil { t.Fatal(err) } if _, err := podClient.Get(oneValidOwnerPodName); err != nil { t.Fatal(err) } - if _, err := podClient.Get(garbageCollectedPodName); err == nil || !errors.IsNotFound(err) { - t.Fatalf("expect pod %s to be garbage collected, got err= %v", garbageCollectedPodName, err) - } } // This test simulates the case where an object is created with an owner that // doesn't exist. It verifies the GC will delete such an object. func TestCreateWithNonExistentOwner(t *testing.T) { - glog.V(6).Infof("TestCreateWithNonExistentOwner starts") - defer glog.V(6).Infof("TestCreateWithNonExistentOwner ends") s, gc, clientSet := setup(t) defer s.Close() @@ -279,15 +261,9 @@ func TestCreateWithNonExistentOwner(t *testing.T) { stopCh := make(chan struct{}) go gc.Run(5, stopCh) defer close(stopCh) - // wait for the garbage collector to drain its queue - if err := wait.Poll(10*time.Second, 120*time.Second, func() (bool, error) { - return gc.QueuesDrained(), nil - }); err != nil { - t.Fatal(err) - } - t.Logf("garbage collector queues drained") - if _, err := podClient.Get(garbageCollectedPodName); err == nil || !errors.IsNotFound(err) { - t.Fatalf("expect pod %s to be garbage collected", garbageCollectedPodName) + // wait for the garbage collector to delete the pod + if err := integration.WaitForPodToDisappear(podClient, garbageCollectedPodName, 5*time.Second, 30*time.Second); err != nil { + t.Fatalf("expect pod %s to be garbage collected, got err= %v", garbageCollectedPodName, err) } } @@ -382,16 +358,8 @@ func TestStressingCascadingDeletion(t *testing.T) { } wg.Wait() t.Logf("all pods are created, all replications controllers are created then deleted") - // wait for the garbage collector to drain its queue - if err := wait.Poll(10*time.Second, 300*time.Second, func() (bool, error) { - return gc.QueuesDrained(), nil - }); err != nil { - t.Fatal(err) - } - t.Logf("garbage collector queues drained") - // wait for the RCs and Pods to reach the expected numbers. This shouldn't - // take long, because the queues are already drained. - if err := wait.Poll(5*time.Second, 30*time.Second, func() (bool, error) { + // wait for the RCs and Pods to reach the expected numbers. + if err := wait.Poll(5*time.Second, 300*time.Second, func() (bool, error) { podsInEachCollection := 3 // see the comments on the calls to setupRCsPods for details remainingGroups := 3 @@ -478,12 +446,19 @@ func TestOrphaning(t *testing.T) { if err != nil { t.Fatalf("Failed to gracefully delete the rc: %v", err) } - - // wait for the garbage collector to drain its queue - if err := wait.Poll(10*time.Second, 300*time.Second, func() (bool, error) { - return gc.QueuesDrained(), nil + // verify the toBeDeleteRC is deleted + if err := wait.PollImmediate(5*time.Second, 30*time.Second, func() (bool, error) { + rcs, err := rcClient.List(api.ListOptions{}) + if err != nil { + return false, err + } + if len(rcs.Items) == 0 { + t.Logf("Still has %d RCs", len(rcs.Items)) + return true, nil + } + return false, nil }); err != nil { - t.Fatal(err) + t.Errorf("unexpected error: %v", err) } // verify pods don't have the ownerPod as an owner anymore @@ -499,9 +474,4 @@ func TestOrphaning(t *testing.T) { t.Errorf("pod %s still has non-empty OwnerRefereces: %v", pod.ObjectMeta.Name, pod.ObjectMeta.OwnerReferences) } } - // verify the toBeDeleteRC is deleted - rcs, err := rcClient.List(api.ListOptions{}) - if len(rcs.Items) != 0 { - t.Errorf("Expect RCs to be deleted, but got %#v", rcs.Items) - } } diff --git a/test/integration/utils.go b/test/integration/utils.go index d34cc64b97b..ec39fdcd43e 100644 --- a/test/integration/utils.go +++ b/test/integration/utils.go @@ -20,11 +20,15 @@ import ( "fmt" "math/rand" "testing" + "time" etcd "github.com/coreos/etcd/client" "github.com/golang/glog" "golang.org/x/net/context" + "k8s.io/kubernetes/pkg/api/errors" + coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/release_1_5/typed/core/v1" client "k8s.io/kubernetes/pkg/client/unversioned" + "k8s.io/kubernetes/pkg/util/wait" "k8s.io/kubernetes/test/integration/framework" ) @@ -69,3 +73,19 @@ var Code409 = map[int]bool{409: true} var Code422 = map[int]bool{422: true} var Code500 = map[int]bool{500: true} var Code503 = map[int]bool{503: true} + +// WaitForPodToDisappear polls the API server if the pod has been deleted. +func WaitForPodToDisappear(podClient coreclient.PodInterface, podName string, interval, timeout time.Duration) error { + return wait.PollImmediate(interval, timeout, func() (bool, error) { + _, err := podClient.Get(podName) + if err == nil { + return false, nil + } else { + if errors.IsNotFound(err) { + return true, nil + } else { + return false, err + } + } + }) +}