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
Filter unknown custom resources in Delete- and ToggleTask #1552
Conversation
Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
…unknown object types Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
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.
A few nits about code duplication which can be fixed with a nested method. But otherwise LGTM 🚢
@@ -67,6 +72,30 @@ func isNamespaced(gvk schema.GroupVersionKind, di discovery.CachedDiscoveryInter | |||
return false, fmt.Errorf("a resource with GVK %v seems to be missing in API resource list", gvk) | |||
} | |||
|
|||
func isKnownType(gvk schema.GroupVersionKind, di discovery.CachedDiscoveryInterface) (bool, error) { | |||
// First try, this may return nil because of the cache | |||
apiResource, err := getAPIResource(gvk, di) |
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 guess, it's a good thing that the discovery client is cached 😉
// Second try, now with invalidated cache. If we still get nil, we know it's not there. | ||
log.Printf("Failed to get APIResource for %v, retry with invalidated cache.", gvk) | ||
di.Invalidate() | ||
apiResource, err = getAPIResource(gvk, di) |
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.
Wdyt about making these next few lines a nested known := func(..)
method? The same goes for the above isNamespaced/namespaced
method.
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.
Not sure about the inline func, I tried it and it didn't really help much. But you're right that there's a bit of duplication, so I extracted a getUncachedAPIResource
Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
What this PR does / why we need it:
For the
DeleteTask
andToggleTask
we use the enhance process - but this fails when the resource to delete is a custom resource for a CRD which is not installed in the cluster. This is especially problematic for theToggleTask
which is often used to guard deployment of features.We need to filter the rendered templates - If one of them is of an unknown type, we shouldn't try to delete it as it can't be there anyway.
This PR splits the convert functionality from the enhancer and skips enhancing for the DeleteTask, then filters the resources for unknown types
Fixes #1547
Signed-off-by: Andreas Neumann aneumann@mesosphere.com