Fix deadlock in ready test

[wojtek-t]

Fix #116225

NONE

/kind flake
/sig api-machinery
/priority important-soon

/assign @aojea

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wojtek-t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• staging/src/k8s.io/apiserver/pkg/storage/OWNERS [wojtek-t]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[aojea]

yeah, it can deadlock, great catch
/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: ca84027d46482b36b29888ae52b43a99c9224382

[aojea]

/hold

this still can race

[aojea]

if there is a deadlock and the context is cancaled it will return an error, the error will make the test fail and be flaky

We need to ensure that the ready is set to true, check this diff , I think that this will f

EDIT - the code was completely wrong

EDIT2 -- this works

diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/ready_test.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/ready_test.go
index e8720e2ad32..67dcb1a6a1c 100644
--- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/ready_test.go
+++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/ready_test.go
@@ -18,6 +18,7 @@ package cacher
 
 import (
        "context"
+       "sync"
        "testing"
        "time"
 )
@@ -82,21 +83,25 @@ func Test_newReadyRacy(t *testing.T) {
        ready := newReady()
        ready.set(false)
 
-       ctx, cancel := context.WithTimeout(context.Background(), 5 * time.Second)
-       defer cancel()
-
+       var wg sync.WaitGroup
        for i := 0; i < concurrency; i++ {
+               wg.Add(2)
                go func() {
-                       errCh <- ready.wait(ctx)
+                       errCh <- ready.wait(context.Background())
                }()
                go func() {
+                       defer wg.Done()
                        ready.set(false)
                }()
                go func() {
+                       defer wg.Done()
                        ready.set(true)
                }()
        }
+       // last one has to set ready to true
+       wg.Wait()
         ready.set(true)

tress ./cacher.test -test.run Test_newReadyRacy
5s: 1116 runs so far, 0 failures
10s: 2275 runs so far, 0 failures
15s: 3419 runs so far, 0 failures
20s: 4548 runs so far, 0 failures
25s: 5704 runs so far, 0 failures
30s: 6859 runs so far, 0 failures
35s: 8041 runs so far, 0 failures
40s: 9186 runs so far, 0 failures
45s: 10353 runs so far, 0 failures

[wojtek-t]

if there is a deadlock and the context is cancaled it will return an error,

ok - good catch, forgot about it.

Yeah - what you have makes sense - changing.

[aojea]

/lgtm
/hold cancel

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 585146b78161861f4ac3d832ef83fe96e9c4d754

[fedebongio]

/triage accepted