feat: PostDelete hook support #16595
Conversation
alexmt
force-pushed
the
post-delete-hooks
branch
2 times, most recently
from
688ac4c
to
70c0a24
Compare
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #16595 +/- ##
========================================
Coverage 49.50% 49.51%
========================================
Files 270 271 +1
Lines 47488 47659 +171
========================================
+ Hits 23508 23597 +89
- Misses 21670 21731 +61
- Partials 2310 2331 +21 ☔ View full report in Codecov by Sentry. |
alexmt
force-pushed
the
post-delete-hooks
branch
from
70c0a24
to
0e74e77
Compare
alexmt
force-pushed
the
post-delete-hooks
branch
from
0e74e77
to
f3874a6
Compare
controller/appcontroller.go
Outdated
| } | ||
| logCtx.Infof("Successfully deleted %d resources", len(objs)) | ||
| logCtx.Infof("Resource entries removed from undefined cluster") |
There was a problem hiding this comment.
Is this log needed here? If it is needed not sure if it should be undefined cluster
There was a problem hiding this comment.
Ops, thank you for catching it. I just forgot to delete this line - fixed!
controller/hook.go
Outdated
| return false, err | ||
| } | ||
| for _, policy := range hook.DeletePolicies(obj) { | ||
| if policy == common.HookDeletePolicyHookFailed && hookHealth.Status == health.HealthStatusDegraded || policy == common.HookDeletePolicyHookSucceeded && hookHealth.Status == health.HealthStatusHealthy { |
There was a problem hiding this comment.
I think this behaviour is different from the delete policy of the other hook types, as far as I remember in gitops-engine the delete policy would be compared with the whole app status instead of just the hook status. We might want to keep the behaviour same or else it can be confusing. I understand at deletion we probably won't have the app status to compare maybe so might be better to have special deletion policy just for the delete hooks?
There was a problem hiding this comment.
Another question here, what do we do if the deletion policy condition isn't met. Does the app itself remains or is it just the hook that sticks around?
There was a problem hiding this comment.
You are right. App deletion does not have status, however, I think the use case is similar: the user just needs control to preserve some hook for debugging. To match with gitops engine behavior it makes sense to use aggregated hooks status. Implemented
controller/hook.go
Outdated
| return false, err | ||
| } | ||
| for _, policy := range hook.DeletePolicies(obj) { | ||
| if policy == common.HookDeletePolicyHookFailed && hookHealth.Status == health.HealthStatusDegraded || policy == common.HookDeletePolicyHookSucceeded && hookHealth.Status == health.HealthStatusHealthy { |
There was a problem hiding this comment.
Another question here, what do we do if the deletion policy condition isn't met. Does the app itself remains or is it just the hook that sticks around?
|
If the deletion policy does not match, then the hook stays. It matches Helm behavior . Updated documentation as well. Thank you for review @gdsoumya . PTAL |
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
crenshaw-dev
changed the title
* feat: PostDelete hooks support Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> --------- Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
* feat: PostDelete hooks support Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> --------- Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> Signed-off-by: penglongli <pelenli@tencent.com>
* feat: PostDelete hooks support Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> --------- Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> Signed-off-by: Kevin Lyda <kevin@lyda.ie>
* feat: PostDelete hooks support Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> --------- Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Closes #7575
PR introduces Post delete hooks support. Hooks are executed during application deletion after all resources as removed. Hooks respect deletion policies.
Screen.Recording.2023-12-15.at.3.04.37.PM.mov