Bookmark events in terminating VW#3900
Conversation
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
kcp-ci-bot
added
release-note-none
|
/lgtm |
|
LGTM label has been added. DetailsGit tree hash: de230aa6c7aab43d94dc7fd8a00f228dacdeb518 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mjudeikis The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kcp-ci-bot
added
the
approved
github-project-automation
Bot
moved this from In-Review
to Done
in Platform Mesh & Kube Projects - Backlog
SimonTheLeg
left a comment
SimonTheLeg
left a comment
There was a problem hiding this comment.
okay since it merged 2 minutes before I could send it, then let's just leave it as a question here @ntnn
| @@ -689,6 +686,91 @@ func TestTerminatingWorkspacesVirtualWorkspaceWatch(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestTerminatingWorkspacesVirtualWorkspaceWatchBookmark(t *testing.T) { | |||
There was a problem hiding this comment.
Instead of writing another test, couldn't we just add AllowWatchBookmarks: true, to the existing test and then check for bookmark events as well? My current understanding is that these events get auto-created for clients which have specified the bookmark capability - or putting it another way, from client side there is no way to forcefully trigger these events, right?
There was a problem hiding this comment.
Correct; I was thinking about that last night as well but I think it makes sense to have a dedicated test for it - on one hand because it's quite important and on the other if we make another VW and look at other tests we have a focus on also specifically checking bookmark events
There was a problem hiding this comment.
I'm starting to lean towards smaller tests. CI is already running a lot, and having smaller tests per feature makes it easier to identify flakes.
5 participants
Summary
ref: platform-mesh/security-operator#402
When watches are set up on the terminating VW with bookmarks the client logs an error with
awaiting required bookmark event for initial events stream, no events received for 10.00949897s.This was because the filter in the storage wrapper only allowed items with a deletion timestamp, which bookmark events don't have.
What Type of PR Is This?
/kind bug
Related Issue(s)
Fixes #
Release Notes