From 302fe7c0c8b615c5c26f14283244c4055505aae6 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Fri, 13 Oct 2017 11:48:17 -0700 Subject: [PATCH 1/2] sarapprover: ignore authz errors --- pkg/controller/certificates/BUILD | 1 + .../certificates/approver/sarapprove.go | 2 +- .../certificates/certificate_controller.go | 20 ++++++++++++++++++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pkg/controller/certificates/BUILD b/pkg/controller/certificates/BUILD index f99fbb06901..b4470499d06 100644 --- a/pkg/controller/certificates/BUILD +++ b/pkg/controller/certificates/BUILD @@ -15,6 +15,7 @@ go_library( deps = [ "//pkg/controller:go_default_library", "//vendor/github.com/golang/glog:go_default_library", + "//vendor/github.com/juju/ratelimit:go_default_library", "//vendor/k8s.io/api/certificates/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", diff --git a/pkg/controller/certificates/approver/sarapprove.go b/pkg/controller/certificates/approver/sarapprove.go index b8061fff176..3aa6be1ce08 100644 --- a/pkg/controller/certificates/approver/sarapprove.go +++ b/pkg/controller/certificates/approver/sarapprove.go @@ -115,7 +115,7 @@ func (a *sarApprover) handle(csr *capi.CertificateSigningRequest) error { } if len(tried) != 0 { - return fmt.Errorf("recognized csr %q as %v but subject access review was not approved", csr.Name, tried) + return certificates.IgnorableError("recognized csr %q as %v but subject access review was not approved", csr.Name, tried) } return nil diff --git a/pkg/controller/certificates/certificate_controller.go b/pkg/controller/certificates/certificate_controller.go index b07d2fb5a42..36d3a2ba0bf 100644 --- a/pkg/controller/certificates/certificate_controller.go +++ b/pkg/controller/certificates/certificate_controller.go @@ -135,7 +135,11 @@ func (cc *CertificateController) processNextWorkItem() bool { if err := cc.syncFunc(cKey.(string)); err != nil { cc.queue.AddRateLimited(cKey) - utilruntime.HandleError(fmt.Errorf("Sync %v failed with : %v", cKey, err)) + if _, ignorable := err.(ignorableError); !ignorable { + utilruntime.HandleError(fmt.Errorf("Sync %v failed with : %v", cKey, err)) + } else { + glog.V(4).Infof("Sync %v failed with : %v", cKey, err) + } return true } @@ -181,3 +185,17 @@ func (cc *CertificateController) syncFunc(key string) error { return cc.handler(csr) } + +// IgnorableError returns an error that we shouldn't handle (i.e. log) because +// it's spammy and usually user error. Instead we will log these errors at a +// higher log level. We still need to throw these errors to signal that the +// sync should be retried. +func IgnorableError(s string, args ...interface{}) ignorableError { + return ignorableError(fmt.Sprintf(s, args...)) +} + +type ignorableError string + +func (e ignorableError) Error() string { + return string(e) +} From 0e0f8346e7539562b46b881496fee749556191e2 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Fri, 13 Oct 2017 11:48:17 -0700 Subject: [PATCH 2/2] sarapprover: increase base delay of per item rate limit from 5 miliseconds to 1 second --- pkg/controller/certificates/certificate_controller.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/controller/certificates/certificate_controller.go b/pkg/controller/certificates/certificate_controller.go index 36d3a2ba0bf..e86810a4be1 100644 --- a/pkg/controller/certificates/certificate_controller.go +++ b/pkg/controller/certificates/certificate_controller.go @@ -36,6 +36,7 @@ import ( "k8s.io/kubernetes/pkg/controller" "github.com/golang/glog" + "github.com/juju/ratelimit" ) type CertificateController struct { @@ -61,8 +62,12 @@ func NewCertificateController( cc := &CertificateController{ kubeClient: kubeClient, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "certificate"), - handler: handler, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.NewMaxOfRateLimiter( + workqueue.NewItemExponentialFailureRateLimiter(200*time.Millisecond, 1000*time.Second), + // 10 qps, 100 bucket size. This is only for retry speed and its only the overall factor (not per item) + &workqueue.BucketRateLimiter{Bucket: ratelimit.NewBucketWithRate(float64(10), int64(100))}, + ), "certificate"), + handler: handler, } // Manage the addition/update of certificate requests