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

skip subresource in resource discovery #6635

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

27149chen
Copy link
Contributor

@27149chen 27149chen commented Aug 10, 2023

Please add a summary of your change

skip subresource in resource discovery

Does your change fix a particular issue?

Fixes #6636

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Merging #6635 (0f9e582) into main (a88cb46) will increase coverage by 0.00%.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##             main    #6635   +/-   ##
=======================================
  Coverage   60.18%   60.18%           
=======================================
  Files         242      242           
  Lines       25711    25722   +11     
=======================================
+ Hits        15474    15481    +7     
- Misses       9156     9160    +4     
  Partials     1081     1081           
Files Changed Coverage Δ
pkg/discovery/helper.go 69.00% <66.66%> (-0.37%) ⬇️

Signed-off-by: lou <[email protected]>
@reasonerjt
Copy link
Contributor

I think it would be pretty safe to skip /status subresources, but we should double check whether we wanna skip ALL subresources.

Labeling it as defer-candidate as it may be out of v1.12.0 release.

@27149chen
Copy link
Contributor Author

I think it would be pretty safe to skip /status subresources, but we should double check whether we wanna skip ALL subresources.

Labeling it as defer-candidate as it may be out of v1.12.0 release.

yes, we should skip ALL, because the RESTMapper we use skip ALL subresources, see https://github.com/kubernetes/client-go/blob/2c8128c8fba0146d0694cc8507e0527dde476101/restmapper/discovery.go#L103C4-L103C4
A backup will fail when calling .discoveryHelper.ResourceFor() for any subresources

@reasonerjt reasonerjt merged commit 3e61386 into vmware-tanzu:main Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backup failed because of a status subresource
3 participants