[Flaking test] unit test TestStoreListResourceVersion

[mimowo]

Which jobs are flaking?

pull-kubernetes-unit

Which tests are flaking?

TestStoreListResourceVersion in k8s.io/apiserver/pkg/registry/generic: registry

Since when has it been flaking?

seems like July 7th 2024:

Testgrid link

https://testgrid.k8s.io/presubmits-kubernetes-blocking#pull-kubernetes-unit

Reason for failure (if possible)

Don't know, here is the log:

{Failed;  === RUN   TestStoreListResourceVersion
W0610 08:12:32.485776   69887 logging.go:59] [core] [Channel #205 SubChannel #208] grpc: addrConn.createTransport failed to connect to {Addr: "localhost:45055", ServerName: "localhost:45055", }. Err: connection error: desc = "transport: Error while dialing: dial tcp 127.0.0.1:45055: connect: connection refused"
    store_test.go:287: storage is (re)initializing
    store_test.go:296: expected waiting, but get <nil>
W0610 08:12:32.576001   69887 logging.go:59] [core] [Channel #325 SubChannel #327] grpc: addrConn.createTransport failed to connect to {Addr: "localhost:34737", ServerName: "localhost:34737", }. Err: connection error: desc = "transport: Error while dialing: dial tcp 127.0.0.1:34737: connect: connection refused"
--- FAIL: TestStoreListResourceVersion (0.33s)
;}

Anything else we need to know?

No response

Relevant SIG(s)

/sig

[k8s-ci-robot]

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.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[neolit123]

kubernetes/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go

Line 255 in d145bf0

func TestStoreListResourceVersion(t *testing.T) {

/sig api-machinery

[BenTheElder]

See also https://testgrid.k8s.io/sig-release-master-blocking#ci-kubernetes-unit

https://go.k8s.io/triage?pr=1&test=k8s.io%2Fapiserver%2Fpkg%2Fregistry%2Fgeneric

[logicalhan]

/sig etcd

[fedebongio]

/assign @serathius
/cc @wenjiaswe

[serathius]

Bisected this to a8ef6e9
cc @wojtek-t

[mauri870]

I'm not sure if this is the proper way to fix it, but setting a limit in the ListOptions deflakes the test.

The flakiness comes from the fact that the cache is sometimes not initialized in time which causes it to return a "too many requests".

Perhaps in tests we could wait till the cache is ready in order to proceed?

[chaochn47]

I'm not sure if this is the proper way to fix it, but setting a limit in the ListOptions deflakes the test.

Right, setting a limit when a resource version is set would delegate the list to etcd when cache is not initialized based on the KEP description.

It is due to this type of request is not that costly to etcd.

Perhaps in tests we could wait till the cache is ready in order to proceed?

Looks like it is exactly how the wait cache ready is added in https://github.com/kubernetes/kubernetes/pull/124642/files#diff-d82df65cc0625a3f621333245e76fbc801b579eedd3d266fe36eeb99366b5680.

See below

kubernetes/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_test.go

Lines 472 to 479 in 9d8edca

	if utilfeature.DefaultFeatureGate.Enabled(features.ResilientWatchCacheInitialization) {
	// The tests assume that Get/GetList/Watch calls shouldn't fail.
	// However, 429 error can now be returned if watchcache is under initialization.
	// To avoid rewriting all tests, we wait for watcache to initialize.
	if err := cacher.ready.wait(ctx); err != nil {
	t.Fatal(err)
	}
	}

So I think we should implement this same thing in newTestGenericStoreRegistry when initializing the cacher.

[wojtek-t]

Agree - waiting for cacher to be initialized when creating it here is a better fix for it:
https://github.com/kubernetes/kubernetes/blob/22512e7a3c6db15ac7a9d46edfe4563fe561418f/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go#L2448C4-L2449C1

[mauri870]

I thought about this as well, but waiting on cacher.ready.wait() is not possible outside of the cacher module, since it is a private field.

I'm thinking to either add an option WaitUntilReady to cacherstorage.Config and make cacherstorage.NewCacherFromConfig wait or add a new public method Wait(context.Context) error to Cacher.

Do you guys have a better idea? Thanks.

[wojtek-t]

Exposing Wait(context) method sounds fine

[mauri870]

I have pushed the changes we discussed. Thank you all for your guidance.