feat: handles cleanup by using owner ref and cleanup hook #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It is a cluster-scoped resource for tracking objects created by Ossm plugin. Primarily used as owner reference for resources created across namespaces so that they can be garbage collected by Kubernetes when they are not needed anymore.
- adds cleanup hook for Delete phase of KfDef operator - moves k8s_utils code to ossm package to keep it in one place for future extraction of the logic
bartoszmajsak
commented
Jul 14, 2023
bartoszmajsak
commented
Jul 14, 2023
bartoszmajsak
commented
Jul 14, 2023
Comment on lines
+31
to
+38
| objectMeta.SetOwnerReferences([]metav1.OwnerReference{ | ||
| { | ||
| APIVersion: o.tracker.APIVersion, | ||
| Kind: o.tracker.Kind, | ||
| Name: o.tracker.Name, | ||
| UID: o.tracker.UID, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Note to self: we have it in multiple places - worth extracting
bartoszmajsak
commented
Jul 14, 2023
pkg/kfapp/ossm/k8s_resources.go
Outdated
There was a problem hiding this comment.
Duplicated by purpose - we want to have our code self-contained and thus we should minimize coupling with other parts of the current operator as much as possible. Another benefit is that now we can refactor it to our liking, as I think we don't need all of the logic from the original k8s_utils.go
cam-garrison
approved these changes
Jul 14, 2023
| } | ||
|
|
||
| gvr := schema.GroupVersionResource{ | ||
| Group: "ossm.plugins.kubeflow.org", |
There was a problem hiding this comment.
noting this Group as something to potentially change in future based on our call
bartoszmajsak
added a commit
that referenced
this pull request
Sep 6, 2023
* feat: introduces OssmResourceTracker resource It is a cluster-scoped resource for tracking objects created by Ossm plugin. Primarily used as owner reference for resources created across namespaces so that they can be garbage collected by Kubernetes when they are not needed anymore. * feat: uses OssmResourceTracker to cleanup owned resources - adds cleanup hook for Delete phase of KfDef operator - moves k8s_utils code to ossm package to keep it in one place for future extraction of the logic * chore: renames plugin to OssmInstaller * chore: moves cleanup logic to single file * feat: handles token volume removal in SMCP * chore: adds plugin name as extra key-value to logger * feat: removes extAuthzProvider on deletion * chore: minor naming fixes
bartoszmajsak
added a commit
that referenced
this pull request
Sep 6, 2023
* feat: introduces OssmResourceTracker resource It is a cluster-scoped resource for tracking objects created by Ossm plugin. Primarily used as owner reference for resources created across namespaces so that they can be garbage collected by Kubernetes when they are not needed anymore. * feat: uses OssmResourceTracker to cleanup owned resources - adds cleanup hook for Delete phase of KfDef operator - moves k8s_utils code to ossm package to keep it in one place for future extraction of the logic * chore: renames plugin to OssmInstaller * chore: moves cleanup logic to single file * feat: handles token volume removal in SMCP * chore: adds plugin name as extra key-value to logger * feat: removes extAuthzProvider on deletion * chore: minor naming fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
OssmResourceTrackerA cluster-scoped resource for tracking objects created by OSSM installer. Primarily used as owner reference for resources created across namespaces so that they can be garbage collected by Kubernetes when they are not needed anymore.
Cleanup funcs
OSSM Installer keeps a chain of functions that will be invoked during the cleanup phase. Right now it includes:
OssmResourceTrackerso the owned resources will be garbage collected onKfDefdeletionConfigMaps,EnvoyFilters and so on - essentially every ns-scoped resource created by the installerOwnerReferencespointing to the tracker resource. OAuth client is also a cluster-scoped resource so it won't be garbage collected.With this in place, subsequent deployments of
KfDefwouldn't result inOAuth flow failederror which is caused by misaligned secrets between filter and client.Those cleanup functions are invoked by a hook in Delete phase of
KfDefoperator.Open points
OssmTrackerResource(we might want to change its name btw ;)) to keep old values in a.specand use it to restore on deletion.