Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes a bug with resource.error watch events #72

Merged
merged 1 commit into from
Jan 12, 2023

Conversation

MbolotSuse
Copy link
Contributor

Watch events need to have a type defined by
k8s.io/apimachinery/pkg/watch to be properly interpreted. Fixes a bug where this was not the case

Related to rancher/rancher#40131

Watch events need to have a type defined by
k8s.io/apimachinery/pkg/watch to be properly interpreted. Fixes
a bug where this was not the case
@MbolotSuse
Copy link
Contributor Author

Background

When a k8s watch returns an error, we create a watch.Event that has an error type. This creates an api object that has a name of resource.error, which signals to users that the watch function has failed to process an item from the watch stream.

When converting these watch events to the apiObject defined by rancher's apiserver package, there's a switch which determines the name of the event - this is done in the partition store.

Bug Details and Fix

  • The watch.Event needs to have the type field set to watch.Error in cases of errors, so that the partition store can convert the name to "resource.error". In the current format, since the type was a manual string not defined in the watch package, it used the default "resource.change" event, which resulted in the issue reported by the UI

Testing

  • Manual test run to validate that this fixes the issue. No Unit test yet. Issue is reproducible on v2 cluster create/delete which appears to create a couple of these watch errors

@MbolotSuse MbolotSuse merged commit 1d517bb into rancher:master Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants