feat(webhook): add garcedelete webhook for PodOpsLifecycle #134
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
|
|
||
| // GetID indicates ID of one PodOpsLifecycle | ||
| func (a *GraceDeleteOpsLifecycleAdapter) GetID() string { | ||
| return "grace-delete" |
There was a problem hiding this comment.
Can we reuse the existing delete adapter?
Because the grace-delete here can share the same lifecycle progress with delete.
There was a problem hiding this comment.
The existing delete adapter is resued instead of GraceDeleteOpsLifecycleAdapter.
| logger := h.Logger.WithValues( | ||
| "op", req.Operation, | ||
| "pod", commonutils.AdmissionRequestObjectKeyString(req), | ||
| ) | ||
|
|
||
| if req.Operation == admissionv1.Delete { | ||
| if err := gracedelete.New().Validating(ctx, h.Client, req); err != nil { |
There was a problem hiding this comment.
I think you can move gracedelete to webhooks list and run together later in line 77.
Of course, the for loop statement need to be changed to remove the short-circuit logic and aggregate all the returned errors, in order to execute every validating hook.
|
@cyh-ant I added some comments. PTAL. Thanks for your contribution. |
|
|
||
| // if Pod is allow to operate, delete it | ||
| if _, allowed := podopslifecycle.AllowOps(OpsLifecycleAdapter, 0, pod); !allowed { | ||
| return fmt.Errorf("podOpsLifecycle denied, waiting for pod resource processing") |
There was a problem hiding this comment.
Do we need to show this message more detailed and friendly to increase user experience?
There was a problem hiding this comment.
New error msg is show below podOpsLifecycle denied delete request, since related resources and finalizers have not been processed. Waiting for removing finalizers: %v.
…roved garcedeletewebhook return message
| return err | ||
| } | ||
|
|
||
| klog.Info("pod is deleted") |
There was a problem hiding this comment.
klog.V(2).Info is used in the new commit.
| @@ -30,7 +30,7 @@ import ( | |||
| ) | |||
|
|
|||
| func (lc *OpsLifecycle) Validating(ctx context.Context, c client.Client, oldPod, newPod *corev1.Pod, operation admissionv1.Operation) error { | |||
| if !utils.ControlledByKusionStack(newPod) { | |||
| if operation == admissionv1.Delete || !utils.ControlledByKusionStack(newPod) { | |||
There was a problem hiding this comment.
Add a comment for later reading?
There was a problem hiding this comment.
The following annotations were added.
// GraceDeleteWebhook FeatureGate defaults to false
// Add '--feature-gates=GraceDeleteWebhook=true' to container args, to enable gracedelete webhook
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #134 +/- ##
==========================================
+ Coverage 59.58% 60.69% +1.11%
==========================================
Files 46 47 +1
Lines 3489 3608 +119
==========================================
+ Hits 2079 2190 +111
- Misses 1192 1202 +10
+ Partials 218 216 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
fix Enhancement: PodOpsLifecycle supports
kubectl delete pod#592. What is the scope of this PR (e.g. component or file name):
pkg/controllers/utils/podopslifecycle/adapter.go
pkg/webhook/server/generic/pod/gracedelete/lifecycle_adapter.go
pkg/webhook/server/generic/pod/gracedelete/validating.go
pkg/webhook/server/generic/pod/gracedelete/webhook.go
pkg/webhook/server/generic/pod/gracedelete/webhook_test.go
pkg/webhook/server/generic/pod/pod_validating_handler.go
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
Add garcedelete validatingwebhookconfigurations of Pod. When , garceDeleteWebhook will trigger PodOpsLifecycle and reject the delete request. The delete request will be allowed until pod is labelled "PodOperateLabel".
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
pkg/webhook/server/generic/pod/gracedelete/webhook_test.go
6. Release note