Repository: kubernetes/kubernetes (language: go)

Issue:
Failure cluster [ae126d38...] `TestApfWatchPanic` flakes a lot

<img width="264" alt="image" src="https://github.com/user-attachments/assets/06293afe-01ec-4778-9182-82967bddacf3">

from https://storage.googleapis.com/k8s-triage/index.html?pr=1&text=TestApfWatchPanic

### Failure cluster [ae126d38f8ff6bab6735](https://go.k8s.io/triage#ae126d38f8ff6bab6735)

##### Error text:
```
Failed;
=== RUN   TestApfWatchPanic
    priority-and-fairness_test.go:187: the APF handler has finished, checking atomicReadOnlyExecuting
    priority-and-fairness_test.go:189: Wanted 0 requests executing, got 1
--- FAIL: TestApfWatchPanic (0.00s)
;
```
#### Recent failures:
[8/31/2024, 6:53:39 PM pr:pull-kubernetes-unit](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/127033/pull-kubernetes-unit/1830015921691824128)
[8/31/2024, 6:37:12 PM ci-kubernetes-unit](https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/ci-kubernetes-unit/1830011786368126976)
[8/31/2024, 5:37:12 PM ci-kubernetes-unit](https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/ci-kubernetes-unit/1829996686651428864)
[8/31/2024, 4:43:26 PM pr:pull-kubernetes-unit](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/127033/pull-kubernetes-unit/1829983138311311360)
[8/31/2024, 4:38:54 PM pr:pull-kubernetes-unit](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/127046/pull-kubernetes-unit/1829982003957600256)


/kind failing-test
<!-- If this is a flake, please add: /kind flake -->

<!-- Please assign a SIG using: /sig SIG-NAME -->


Discussion / hints:
This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the `triage/accepted` label and provide further guidance.

The `triage/accepted` label can be added by org members by writing `/triage accepted` in a comment.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
xref: https://github.com/kubernetes/kubernetes/issues/110139
xref: https://github.com/kubernetes/kubernetes/pull/126574#issuecomment-2322595802
possible fix will be in https://github.com/kubernetes/kubernetes/pull/127029
/sig api-machinery

/assign
I ran `stress`  with https://github.com/kubernetes/kubernetes/pull/127029, did not see the flake. 
If it takes time to merge the patch PR, can't we just revert the original PR for now?
I saw a lot of unit test failures; it roughly seems like around 30% (or maybe more) unit test runs are failed because of this test.
https://prow.k8s.io/job-history/gs/kubernetes-jenkins/pr-logs/directory/pull-kubernetes-unit
> If it takes time to merge the patch PR, can't we just revert the original PR for now? I saw a lot of unit test failures; it roughly seems like around 30% (or maybe more) unit test runs are failed because of this test. https://prow.k8s.io/job-history/gs/kubernetes-jenkins/pr-logs/directory/pull-kubernetes-unit

@sanposhiho please send a revert and tag me, it seems this situations comes from a fix for a data race that creates a new data race.

It is better to fix everything in one shot that keep fixing forward
Let's do that. Will soon send the revert.

Fix patch:
diff --git a/staging/src/k8s.io/apiserver/pkg/server/filters/priority-and-fairness.go b/staging/src/k8s.io/apiserver/pkg/server/filters/priority-and-fairness.go
index 1e91b9a31154b..decd9d6ca8277 100644
--- a/staging/src/k8s.io/apiserver/pkg/server/filters/priority-and-fairness.go
+++ b/staging/src/k8s.io/apiserver/pkg/server/filters/priority-and-fairness.go
@@ -266,17 +266,23 @@ func (h *priorityAndFairnessHandler) Handle(w http.ResponseWriter, r *http.Reque
 
 		select {
 		case <-shouldStartWatchCh:
-			watchCtx := utilflowcontrol.WithInitializationSignal(ctx, watchInitializationSignal)
-			watchReq = r.WithContext(watchCtx)
-			h.handler.ServeHTTP(w, watchReq)
-			// Protect from the situation when request will not reach storage layer
-			// and the initialization signal will not be send.
-			// It has to happen before waiting on the resultCh below.
-			watchInitializationSignal.Signal()
-			// TODO: Consider finishing the request as soon as Handle call panics.
-			if err := <-resultCh; err != nil {
-				panic(err)
-			}
+			func() {
+				// TODO: if both goroutines panic, propagate the stack traces from both
+				// goroutines so they are logged properly:
+				defer func() {
+					// Protect from the situation when request will not reach storage layer
+					// and the initialization signal will not be send.
+					// It has to happen before waiting on the resultCh below.
+					watchInitializationSignal.Signal()
+					// TODO: Consider finishing the request as soon as Handle call panics.
+					if err := <-resultCh; err != nil {
+						panic(err)
+					}
+				}()
+				watchCtx := utilflowcontrol.WithInitializationSignal(ctx, watchInitializationSignal)
+				watchReq = r.WithContext(watchCtx)
+				h.handler.ServeHTTP(w, watchReq)
+			}()
 		case err := <-resultCh:
 			if err != nil {
 				panic(err)


Test patch:
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
index ad3ac780b3059..55ef87fdf3658 100644
--- 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
@@ -177,11 +177,21 @@ func newApfHandlerWithFilter(t *testing.T, flowControlFilter utilflowcontrol.Int
 		r = r.WithContext(apirequest.WithUser(r.Context(), &user.DefaultInfo{
 			Groups: []string{user.AllUnauthenticated},
 		}))
-		apfHandler.ServeHTTP(w, r)
-		postExecute()
-		if atomicReadOnlyExecuting != 0 {
-			t.Errorf("Wanted %d requests executing, got %d", 0, atomicReadOnlyExecuting)
-		}
+		func() {
+			// the APF handler completes its run, either normally or
+			// with a panic, in either case, all APF book keeping must
+			// be completed by now. Also, whether the request is
+			// executed or rejected, we expect the counter to be zero.
+			// TODO: all test(s) using this filter must run
+			// serially to each other
+			defer func() {
+				if atomicReadOnlyExecuting != 0 {
+					t.Errorf("Wanted %d requests executing, got %d", 0, atomicReadOnlyExecuting)
+				}
+			}()
+			apfHandler.ServeHTTP(w, r)
+			postExecute()
+		}()
 	}), requestInfoFactory)
 
 	return handler