resource: Work around a rare race in initial sync

[joamaki]

TestResource_WithFakeClient is sometimes failing due to seeing two Upserts instead of Upsert+Sync. This is due to the fact that we add the initial set of keys from the store to the queue when subscribing in Events() and the fact that adding to the store is happening before our event handler runs. The sequence of events looks like this:

[informer 1/2] add object A to store
[resource] subscribe and add object with key A to queue
[resource] dequeue A and emit Event{Upsert, A}
[informer 2/2] call AddFunc handler for A and queue A
[resource] dequeue A and emit Event{Upsert, A}

Work around the issue by allowing the extra Upsert event in the test case.

Proper fix for this would be to either detect a duplicate using ResourceVersion, or implementing a custom (cache.Config) Process function that synchronizes the updating of the store with subscribing (e.g. pkg/k8s/informer/informer.go).

Fixes: #23079
Fixes: 4101e2c ("k8s: Add resource package")

[joestringer]

I marked this for backport to 1.13 since it looks like it will mitigate a unit test failure. If this relies on changes only in the master branch, feel free to remove the label.

[joamaki]

/test

Job 'Cilium-PR-K8s-1.25-kernel-4.19' failed:

Click to show.

Test Name

K8sDatapathConfig Encapsulation Check connectivity with transparent encryption, VXLAN, and endpoint routes

Failure Output

FAIL: Kubernetes DNS did not become ready in time

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.25-kernel-4.19 so I can create one.

Job 'Cilium-PR-K8s-1.16-kernel-4.9' failed:

Click to show.

Test Name

K8sAgentHubbleTest Hubble Observe Test L3/L4 Flow

Failure Output

FAIL: hubble-relay was not able to get into ready state

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.16-kernel-4.9 so I can create one.

[dylandreimerink]

LGTM

[pchaigno]

@joamaki Why were the end-to-end tests executed here? AFAICS, it's only affecting the non-privileged unit tests which run in Travis CI. Is that was a mistake, I can probably merge.

[joamaki]

@joamaki Why were the end-to-end tests executed here? AFAICS, it's only affecting the non-privileged unit tests which run in Travis CI. Is that was a mistake, I can probably merge.

Yep only Travis CI needed to check that the test passes. Go ahead and merge.

[julianwiedmann]

@joamaki is the backport for 1.13 still on your radar, or is it fine to drop that?

[julianwiedmann]

Taking that as a no :).

[joamaki]

@julianwiedmann oops, sorry about the delay (had half-answered this and then it got buried). Yes no need to backport to 1.13.