add finalizer to ensure integration children are cleaned up #480
Conversation
pkg/cmd/run.go
Outdated
| @@ -84,6 +86,7 @@ func newCmdRun(rootCmdOptions *RootCmdOptions) *cobra.Command { | |||
| cmd.Flags().BoolVar(&options.Compression, "compression", false, "Enable store source as a compressed binary blob") | |||
| cmd.Flags().StringSliceVar(&options.Resources, "resource", nil, "Add a resource") | |||
| cmd.Flags().StringSliceVar(&options.OpenAPIs, "open-api", nil, "Add an OpenAPI v2 spec") | |||
| cmd.Flags().BoolVar(&options.Owner, "owner", true, "Use resource ownership to cleanup child resources, if set to false finalizers are used") | |||
There was a problem hiding this comment.
We may want to name the option to something more explicit like deletion-policy with possible values owner / ownerReference or label / labelSelector to fit into k8s garbage collection scheme.
pkg/controller/integration/delete.go
Outdated
| // | ||
| // Remove the finalizer to unlock resource | ||
| // | ||
| _, err = finalizer.Remove(target, finalizer.CamelIntegrationFinalizer) |
There was a problem hiding this comment.
I wonder whether we should check the foregroundDeletion finalizer and emulate a background cascading deletion when no present by removing our finaliser first and then delete the child resources.
|
will do |
lburgazzoli
force-pushed
the
github-477-finalizers
branch
from
224c213
to
984485e
Compare
|
@astefanutti done |
lburgazzoli
force-pushed
the
github-477-finalizers
branch
from
984485e
to
6202575
Compare
pkg/controller/integration/delete.go
Outdated
| return err | ||
| } | ||
|
|
||
| if ok { |
There was a problem hiding this comment.
I think the logic should be reverse. In background cascading deletion, Kubernetes deletes the owner object immediately and the garbage collector then deletes the dependents in the background.
pkg/controller/integration/delete.go
Outdated
| resource := resource | ||
|
|
||
| // Pods are automatically deleted by the removal of Deployment | ||
| if resource.GetKind() == "Pod" { |
There was a problem hiding this comment.
I wonder whether we should deactivate cascading in the delete operation so that we fully rely on the label deletion policy and avoid mixing label and owner strategies and get rid of these special cases.
pkg/controller/integration/delete.go
Outdated
|
|
||
| l.Infof("Deleting child resource: %s/%s", resource.GetKind(), resource.GetName()) | ||
|
|
||
| err := action.client.Delete(ctx, &resource) |
There was a problem hiding this comment.
We could set the PropagationPolicy to orphan here.
|
@astefanutti should be better now |
|
👍 |
Fixes #477