Merge pull request #93646 from gobomb/fix-panic-defer

let panics propagate up when processLoop in informer controller panic in client-go

Kubernetes-commit: 5a7970a42bddb415a5ef3b21f181abba0a5c37de

Godeps/Godeps.json

@@ -440,7 +440,7 @@
 		},
 		{
 			"ImportPath": "k8s.io/api",
-			"Rev": "4022903d1fba"
+			"Rev": "9a1561067c54"
 		},
 		{
 			"ImportPath": "k8s.io/apimachinery",

go.mod

@@ -26,7 +26,7 @@ require (
 	golang.org/x/net v0.0.0-20200707034311-ab3426394381
 	golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6
 	golang.org/x/time v0.0.0-20191024005414-555d28b269f0
-	k8s.io/api v0.0.0-20200830011551-4022903d1fba
+	k8s.io/api v0.0.0-20200831211624-9a1561067c54
 	k8s.io/apimachinery v0.0.0-20200830011411-94222d04a590
 	k8s.io/klog/v2 v2.2.0
 	k8s.io/utils v0.0.0-20200729134348-d5654de09c73
@@ -34,6 +34,6 @@ require (
 )
 
 replace (
-	k8s.io/api => k8s.io/api v0.0.0-20200830011551-4022903d1fba
+	k8s.io/api => k8s.io/api v0.0.0-20200831211624-9a1561067c54
 	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20200830011411-94222d04a590
 )

go.sum

@@ -333,7 +333,7 @@ honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
-k8s.io/api v0.0.0-20200830011551-4022903d1fba/go.mod h1:5BitZN65u1la1OS/5z3nGIi+SrJ2LusBghXaztMVE0Q=
+k8s.io/api v0.0.0-20200831211624-9a1561067c54/go.mod h1:qMn0Ckwm7+s/YbS3iDBlHErlW/SeOXttpV83LV5iO5M=
 k8s.io/apimachinery v0.0.0-20200830011411-94222d04a590/go.mod h1:DnPGDnARWFvYa3pMHgSxtbZb7gpzzAZ1pTfaUNDVlmA=
 k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0=
 k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=

tools/cache/controller.go

@@ -144,11 +144,11 @@ func (c *controller) Run(stopCh <-chan struct{}) {
 	c.reflectorMutex.Unlock()
 
 	var wg wait.Group
-	defer wg.Wait()
 
 	wg.StartWithChannel(stopCh, r.Run)
 
 	wait.Until(c.processLoop, time.Second, stopCh)
+	wg.Wait()
 }
 
 // Returns true once this controller has completed an initial resource listing

tools/cache/controller_test.go

@@ -402,3 +402,49 @@ func TestUpdate(t *testing.T) {
 	testDoneWG.Wait()
 	close(stop)
 }
+
+func TestPanicPropagated(t *testing.T) {
+	// source simulates an apiserver object endpoint.
+	source := fcache.NewFakeControllerSource()
+
+	// Make a controller that just panic if the AddFunc is called.
+	_, controller := NewInformer(
+		source,
+		&v1.Pod{},
+		time.Millisecond*100,
+		ResourceEventHandlerFuncs{
+			AddFunc: func(obj interface{}) {
+				// Create a panic.
+				panic("Just panic.")
+			},
+		},
+	)
+
+	// Run the controller and run it until we close stop.
+	stop := make(chan struct{})
+	defer close(stop)
+
+	propagated := make(chan interface{})
+	go func() {
+		defer func() {
+			if r := recover(); r != nil {
+				propagated <- r
+			}
+		}()
+		controller.Run(stop)
+	}()
+	// Let's add a object to the source. It will trigger a panic.
+	source.Add(&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test"}})
+
+	// Check if the panic propagated up.
+	select {
+	case p := <-propagated:
+		if p == "Just panic." {
+			t.Logf("Test Passed")
+		} else {
+			t.Errorf("unrecognized panic in controller run: %v", p)
+		}
+	case <-time.After(wait.ForeverTestTimeout):
+		t.Errorf("timeout: the panic failed to propagate from the controller run method!")
+	}
+}