-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Parse resource from backup tarball directly rather than resolving it via discovery service to avoid #4009 #4398
Conversation
gvr = schema.ParseGroupResource(resource).WithVersion("") | ||
|
||
ctx.Log.Infof("got \"no matches\" error when trying to resolve the resource %s via discovery, use ParseGroupResource directly, gvr: %s", resource, gvr.String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, we can't use the gvr variable on line 81 directly, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean to replace gvr, _, err := ctx.DiscoveryHelper.ResourceFor(schema.ParseGroupResource(resource).WithVersion(""))
with gvr := schema.ParseGroupResource(resource).WithVersion("")
in line 81?
I'm not sure whether the input resource
is fully resolved or not. Handle this when getting errors may be more safe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the same question, @ywk253100 could you please check with Nolan.
Seems the resources in the backup tarball should be resolved already?
50cacbd
to
fe3c797
Compare
@reasonerjt @jenting Confirmed the fix with @nrb, please take a look again |
I think the code change makes sense to me, but the commit message no longer matches the code change? |
fe3c797
to
47bcfc6
Compare
…via discovery service to avoid vmware-tanzu#4009 If the resource is a CR and the CRD is removed, the resource cannot be fully resolved via discovery and the backup cannot be deleted anymore. This commit parses the resource from backup tarball directly to avoid this Fixes vmware-tanzu#4009 Signed-off-by: Wenkai Yin(尹文开) <[email protected]>
47bcfc6
to
52cb7a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…via discovery service to avoid vmware-tanzu#4009 (vmware-tanzu#4398)
…via discovery service to avoid vmware-tanzu#4009 (vmware-tanzu#4398)
If the resource is a CR and the CRD is removed, the resource cannot be fully resolved via discovery and the backup cannot be deleted anymore. This commit parses the resource from backup tarball directly to avoid this
Fixes #4009
Signed-off-by: Wenkai Yin(尹文开) [email protected]
Thank you for contributing to Velero!
Please add a summary of your change
Does your change fix a particular issue?
Fixes #(issue)
Please indicate you've done the following:
/kind changelog-not-required
.site/content/docs/main
.