Revert "apiserver: improve logging for apf tests in server/filters pa…

…ckage" This reverts commit 8fa3e61.
LionelJouin · Sep 3, 2024 · 9fe3b84 · 9fe3b84
1 parent 7de63a8
commit 9fe3b84
Showing 1 changed file with 32 additions and 62 deletions.
diff --git a/staging/src/k8s.io/apiserver/pkg/server/filters/priority-and-fairness_test.go b/staging/src/k8s.io/apiserver/pkg/server/filters/priority-and-fairness_test.go
@@ -18,7 +18,6 @@ package filters
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -179,19 +178,11 @@ func newApfHandlerWithFilter(t *testing.T, flowControlFilter utilflowcontrol.Int
 		r = r.WithContext(apirequest.WithUser(r.Context(), &user.DefaultInfo{
 			Groups: []string{user.AllUnauthenticated},
 		}))
-		func() {
-			// the defer ensures that the following assertion is
-			// executed, even if the APF handler panics
-			// TODO: all test(s) using this filter must run serially to each other
-			defer func() {
-				t.Logf("the APF handler has finished, checking atomicReadOnlyExecuting")
-				if want, got := int32(0), atomic.LoadInt32(&atomicReadOnlyExecuting); want != got {
-					t.Errorf("Wanted %d requests executing, got %d", want, got)
-				}
-			}()
-			apfHandler.ServeHTTP(w, r)
-			postExecute()
-		}()
+		apfHandler.ServeHTTP(w, r)
+		postExecute()
+		if want, got := int32(0), atomic.LoadInt32(&atomicReadOnlyExecuting); want != got {
+			t.Errorf("Wanted %d requests executing, got %d", want, got)
+		}
 	}), requestInfoFactory)
 
 	return handler
@@ -355,21 +346,19 @@ func TestApfCancelWaitRequest(t *testing.T) {
 }
 
 type fakeWatchApfFilter struct {
-	t        *testing.T
 	lock     sync.Mutex
 	inflight int
 	capacity int
 
-	postExecutePanic error
-	preExecutePanic  error
+	postExecutePanic bool
+	preExecutePanic  bool
 
 	utilflowcontrol.WatchTracker
 	utilflowcontrol.MaxSeatsTracker
 }
 
-func newFakeWatchApfFilter(t *testing.T, capacity int) *fakeWatchApfFilter {
+func newFakeWatchApfFilter(capacity int) *fakeWatchApfFilter {
 	return &fakeWatchApfFilter{
-		t:               t,
 		capacity:        capacity,
 		WatchTracker:    utilflowcontrol.NewWatchTracker(),
 		MaxSeatsTracker: utilflowcontrol.NewMaxSeatsTracker(),
@@ -397,23 +386,17 @@ func (f *fakeWatchApfFilter) Handle(ctx context.Context,
 		return
 	}
 
-	func() {
-		defer func() {
-			f.lock.Lock()
-			defer f.lock.Unlock()
-			f.inflight--
-		}()
+	if f.preExecutePanic {
+		panic("pre-exec-panic")
+	}
+	execFn()
+	if f.postExecutePanic {
+		panic("post-exec-panic")
+	}
 
-		if f.preExecutePanic != nil {
-			f.t.Logf("going to panic (pre-exec) as expected with error: %v, fakeWatchApfFilter: %#v", f.preExecutePanic, f)
-			panic(f.preExecutePanic)
-		}
-		execFn()
-		if f.postExecutePanic != nil {
-			f.t.Logf("going to panic (post-exec) as expected with error: %v, fakeWatchApfFilter: %#v", f.postExecutePanic, f)
-			panic(f.postExecutePanic)
-		}
-	}()
+	f.lock.Lock()
+	defer f.lock.Unlock()
+	f.inflight--
 }
 
 func (f *fakeWatchApfFilter) Run(stopCh <-chan struct{}) error {
@@ -465,7 +448,7 @@ func TestApfExecuteWatchRequestsWithInitializationSignal(t *testing.T) {
 	allRunning := sync.WaitGroup{}
 	allRunning.Add(2 * concurrentRequests)
 
-	fakeFilter := newFakeWatchApfFilter(t, concurrentRequests)
+	fakeFilter := newFakeWatchApfFilter(concurrentRequests)
 
 	onExecuteFunc := func() {
 		firstRunning.Done()
@@ -511,7 +494,7 @@ func TestApfExecuteWatchRequestsWithInitializationSignal(t *testing.T) {
 }
 
 func TestApfRejectWatchRequestsWithInitializationSignal(t *testing.T) {
-	fakeFilter := newFakeWatchApfFilter(t, 0)
+	fakeFilter := newFakeWatchApfFilter(0)
 
 	onExecuteFunc := func() {
 		t.Errorf("Request unexepectedly executing")
@@ -530,7 +513,7 @@ func TestApfWatchPanic(t *testing.T) {
 	epmetrics.Register()
 	fcmetrics.Register()
 
-	fakeFilter := newFakeWatchApfFilter(t, 1)
+	fakeFilter := newFakeWatchApfFilter(1)
 
 	onExecuteFunc := func() {
 		panic("test panic")
@@ -557,11 +540,11 @@ func TestApfWatchPanic(t *testing.T) {
 func TestApfWatchHandlePanic(t *testing.T) {
 	epmetrics.Register()
 	fcmetrics.Register()
-	preExecutePanicingFilter := newFakeWatchApfFilter(t, 1)
-	preExecutePanicingFilter.preExecutePanic = http.ErrAbortHandler
+	preExecutePanicingFilter := newFakeWatchApfFilter(1)
+	preExecutePanicingFilter.preExecutePanic = true
 
-	postExecutePanicingFilter := newFakeWatchApfFilter(t, 1)
-	postExecutePanicingFilter.postExecutePanic = http.ErrAbortHandler
+	postExecutePanicingFilter := newFakeWatchApfFilter(1)
+	postExecutePanicingFilter.postExecutePanic = true
 
 	testCases := []struct {
 		name   string
@@ -577,31 +560,18 @@ func TestApfWatchHandlePanic(t *testing.T) {
 		},
 	}
 
+	onExecuteFunc := func() {
+		time.Sleep(5 * time.Second)
+	}
+	postExecuteFunc := func() {}
+
 	for _, test := range testCases {
 		t.Run(test.name, func(t *testing.T) {
-			onExecuteFunc := func() {
-				time.Sleep(5 * time.Second)
-
-				// this function should not be executed if
-				// pre-execute panic is set
-				if test.filter.preExecutePanic != nil {
-					t.Errorf("did not expect the execute function to be executed")
-				}
-				t.Logf("on-execute function invoked")
-			}
-
-			// we either panic before the execute function, or after,
-			// so the following function should never be executed.
-			postExecuteFunc := func() {
-				t.Errorf("did not expect the post-execute function to be invoked")
-			}
-
 			apfHandler := newApfHandlerWithFilter(t, test.filter, time.Minute/4, onExecuteFunc, postExecuteFunc)
 			handler := func(w http.ResponseWriter, r *http.Request) {
 				defer func() {
-					recovered := recover()
-					if err, ok := recovered.(error); !ok || !errors.Is(err, http.ErrAbortHandler) {
-						t.Errorf("expected panic with error: %v, but got: %v", http.ErrAbortHandler, err)
+					if err := recover(); err == nil {
+						t.Errorf("expected panic, got %v", err)
 					}
 				}()
 				apfHandler.ServeHTTP(w, r)