From f3c793512b45ea3910d5e5a379292c13b62ab64b Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Wed, 28 Aug 2019 15:31:27 -0700 Subject: [PATCH] fix semantics of the rejected label in webhook metrics when error calling webhook is ignored, do not log the request as rejected --- .../admission/plugin/webhook/mutating/dispatcher.go | 12 ++++++++++-- .../plugin/webhook/validating/dispatcher.go | 12 ++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go index b69b5fa274a..6f882a900ef 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go @@ -133,7 +133,16 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attrib round = 1 } changed, err := a.callAttrMutatingHook(ctx, hook, invocation, versionedAttr, o, round, i) - admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, versionedAttr.Attributes, "admit", hook.Name) + ignoreClientCallFailures := hook.FailurePolicy != nil && *hook.FailurePolicy == v1beta1.Ignore + rejected := false + if err != nil { + // ErrCallingWebhook is ignored if the webhook is configured to failopen. + // Otherwise the request is rejected. + if _, ok := err.(*webhookutil.ErrCallingWebhook); !ok || !ignoreClientCallFailures { + rejected = true + } + } + admissionmetrics.Metrics.ObserveWebhook(time.Since(t), rejected, versionedAttr.Attributes, "admit", hook.Name) if changed { // Patch had changed the object. Prepare to reinvoke all previous webhooks that are eligible for re-invocation. webhookReinvokeCtx.RequireReinvokingPreviouslyInvokedPlugins() @@ -146,7 +155,6 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attrib continue } - ignoreClientCallFailures := hook.FailurePolicy != nil && *hook.FailurePolicy == v1beta1.Ignore if callErr, ok := err.(*webhookutil.ErrCallingWebhook); ok { if ignoreClientCallFailures { klog.Warningf("Failed calling webhook, failing open %v: %v", hook.Name, callErr) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go index 653cf5a9de9..019b6e86c55 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go @@ -101,12 +101,20 @@ func (d *validatingDispatcher) Dispatch(ctx context.Context, attr admission.Attr versionedAttr := versionedAttrs[invocation.Kind] t := time.Now() err := d.callHook(ctx, hook, invocation, versionedAttr) - admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, versionedAttr.Attributes, "validating", hook.Name) + ignoreClientCallFailures := hook.FailurePolicy != nil && *hook.FailurePolicy == v1beta1.Ignore + rejected := false + if err != nil { + // ErrCallingWebhook is ignored if the webhook is configured to failopen. + // Otherwise the request is rejected. + if _, ok := err.(*webhookutil.ErrCallingWebhook); !ok || !ignoreClientCallFailures { + rejected = true + } + } + admissionmetrics.Metrics.ObserveWebhook(time.Since(t), rejected, versionedAttr.Attributes, "validating", hook.Name) if err == nil { return } - ignoreClientCallFailures := hook.FailurePolicy != nil && *hook.FailurePolicy == v1beta1.Ignore if callErr, ok := err.(*webhookutil.ErrCallingWebhook); ok { if ignoreClientCallFailures { klog.Warningf("Failed calling webhook, failing open %v: %v", hook.Name, callErr)