Merge pull request #43110 from caesarxuchao/fix-42952
Automatic merge from submit-queue (batch tested with PRs 43106, 43110) Wait for garbagecollector to be synced in test Fix #42952 Without the `cache.WaitForCacheSync` in the test, it's possible for the GC to get a merged event of RC's creation and its update (update to deletionTimestamp != 0), before GC gets the creation event of the pod, so it's possible the GC will handle the foreground deletion of the RC before it adds the Pod to the dependency graph, thus the race. With the `cache.WaitForCacheSync` in the test, because GC runs a single thread to process graph changes, it's guaranteed the Pod will be added to the dependency graph before GC handles the foreground deletion of the RC. Note that this pull fixes the race in the test. The race described in the first point of #26120 still exists.
This commit is contained in:
@@ -125,6 +125,10 @@ func (gc *GarbageCollector) Run(workers int, stopCh <-chan struct{}) {
|
|||||||
glog.Infof("Garbage Collector: Shutting down")
|
glog.Infof("Garbage Collector: Shutting down")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (gc *GarbageCollector) HasSynced() bool {
|
||||||
|
return gc.dependencyGraphBuilder.HasSynced()
|
||||||
|
}
|
||||||
|
|
||||||
func (gc *GarbageCollector) runAttemptToDeleteWorker() {
|
func (gc *GarbageCollector) runAttemptToDeleteWorker() {
|
||||||
for gc.attemptToDeleteWorker() {
|
for gc.attemptToDeleteWorker() {
|
||||||
}
|
}
|
||||||
|
@@ -37,6 +37,7 @@ import (
|
|||||||
"k8s.io/client-go/discovery"
|
"k8s.io/client-go/discovery"
|
||||||
"k8s.io/client-go/dynamic"
|
"k8s.io/client-go/dynamic"
|
||||||
restclient "k8s.io/client-go/rest"
|
restclient "k8s.io/client-go/rest"
|
||||||
|
"k8s.io/client-go/tools/cache"
|
||||||
"k8s.io/kubernetes/pkg/api"
|
"k8s.io/kubernetes/pkg/api"
|
||||||
"k8s.io/kubernetes/pkg/api/v1"
|
"k8s.io/kubernetes/pkg/api/v1"
|
||||||
"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
|
"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
|
||||||
@@ -607,7 +608,7 @@ func TestBlockingOwnerRefDoesBlock(t *testing.T) {
|
|||||||
s, gc, clientSet := setup(t)
|
s, gc, clientSet := setup(t)
|
||||||
defer s.Close()
|
defer s.Close()
|
||||||
|
|
||||||
ns := framework.CreateTestingNamespace("gc-foreground2", s, t)
|
ns := framework.CreateTestingNamespace("gc-foreground3", s, t)
|
||||||
defer framework.DeleteTestingNamespace(ns, s, t)
|
defer framework.DeleteTestingNamespace(ns, s, t)
|
||||||
|
|
||||||
podClient := clientSet.Core().Pods(ns.Name)
|
podClient := clientSet.Core().Pods(ns.Name)
|
||||||
@@ -632,6 +633,19 @@ func TestBlockingOwnerRefDoesBlock(t *testing.T) {
|
|||||||
go gc.Run(5, stopCh)
|
go gc.Run(5, stopCh)
|
||||||
defer close(stopCh)
|
defer close(stopCh)
|
||||||
|
|
||||||
|
// this makes sure the garbage collector will have added the pod to its
|
||||||
|
// dependency graph before handling the foreground deletion of the rc.
|
||||||
|
timeout := make(chan struct{})
|
||||||
|
go func() {
|
||||||
|
select {
|
||||||
|
case <-time.After(5 * time.Second):
|
||||||
|
close(timeout)
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
if !cache.WaitForCacheSync(timeout, gc.HasSynced) {
|
||||||
|
t.Fatalf("failed to wait for garbage collector to be synced")
|
||||||
|
}
|
||||||
|
|
||||||
err = rcClient.Delete(toBeDeletedRCName, getForegroundOptions())
|
err = rcClient.Delete(toBeDeletedRCName, getForegroundOptions())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("Failed to delete the rc: %v", err)
|
t.Fatalf("Failed to delete the rc: %v", err)
|
||||||
|
Reference in New Issue
Block a user