From a7588723f7e9b88c2e8b9d601a2391e0f6b19a17 Mon Sep 17 00:00:00 2001 From: Tomas Nozicka Date: Thu, 17 Jan 2019 18:32:36 +0100 Subject: [PATCH 1/2] Switch WaitForCertificate to informers to avoid broken watches --- cmd/kubelet/app/server_bootstrap_test.go | 10 ++++-- .../util/certificate/certificate_manager.go | 33 ++++--------------- .../certificate/certificate_manager_test.go | 13 ++------ .../client-go/util/certificate/csr/csr.go | 29 +++++++++------- 4 files changed, 34 insertions(+), 51 deletions(-) diff --git a/cmd/kubelet/app/server_bootstrap_test.go b/cmd/kubelet/app/server_bootstrap_test.go index c59029fae8..4217550125 100644 --- a/cmd/kubelet/app/server_bootstrap_test.go +++ b/cmd/kubelet/app/server_bootstrap_test.go @@ -254,7 +254,13 @@ func (s *csrSimulator) ServeHTTP(w http.ResponseWriter, req *http.Request) { defer s.lock.Unlock() t := s.t - t.Logf("Request %s %s %s", req.Method, req.URL, req.UserAgent()) + // filter out timeouts as csrSimulator don't support them + q := req.URL.Query() + q.Del("timeout") + q.Del("timeoutSeconds") + req.URL.RawQuery = q.Encode() + + t.Logf("Request %q %q %q", req.Method, req.URL, req.UserAgent()) if len(s.expectUserAgent) > 0 && req.UserAgent() != s.expectUserAgent { t.Errorf("Unexpected user agent: %s", req.UserAgent()) @@ -305,7 +311,7 @@ func (s *csrSimulator) ServeHTTP(w http.ResponseWriter, req *http.Request) { } s.csr = csr - case req.Method == "GET" && req.URL.Path == "/apis/certificates.k8s.io/v1beta1/certificatesigningrequests" && req.URL.RawQuery == "fieldSelector=metadata.name%3Dtest-csr&limit=500": + case req.Method == "GET" && req.URL.Path == "/apis/certificates.k8s.io/v1beta1/certificatesigningrequests" && req.URL.RawQuery == "fieldSelector=metadata.name%3Dtest-csr&limit=500&resourceVersion=0": if s.csr == nil { t.Fatalf("no csr") } diff --git a/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go b/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go index 4aa9f3ab4a..2610fac4a8 100644 --- a/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go +++ b/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go @@ -40,9 +40,9 @@ import ( "k8s.io/client-go/util/certificate/csr" ) -// certificateWaitBackoff controls the amount and timing of retries when the -// watch for certificate approval is interrupted. -var certificateWaitBackoff = wait.Backoff{Duration: 30 * time.Second, Steps: 4, Factor: 1.5, Jitter: 0.1} +// certificateWaitTimeout controls the amount of time we wait for certificate +// approval in one iteration. +var certificateWaitTimeout = 15 * time.Minute // Manager maintains and updates the certificates in use by this certificate // manager. In the background it communicates with the API server to get new @@ -388,29 +388,10 @@ func (m *manager) rotateCerts() (bool, error) { // Once we've successfully submitted a CSR for this template, record that we did so m.setLastRequest(template) - // Wait for the certificate to be signed. Instead of one long watch, we retry with slightly longer - // intervals each time in order to tolerate failures from the server AND to preserve the liveliness - // of the cert manager loop. This creates slightly more traffic against the API server in return - // for bounding the amount of time we wait when a certificate expires. - var crtPEM []byte - watchDuration := time.Minute - if err := wait.ExponentialBackoff(certificateWaitBackoff, func() (bool, error) { - data, err := csr.WaitForCertificate(client, req, watchDuration) - switch { - case err == nil: - crtPEM = data - return true, nil - case err == wait.ErrWaitTimeout: - watchDuration += time.Minute - if watchDuration > 5*time.Minute { - watchDuration = 5 * time.Minute - } - return false, nil - default: - utilruntime.HandleError(fmt.Errorf("Unable to check certificate signing status: %v", err)) - return false, m.updateServerError(err) - } - }); err != nil { + // Wait for the certificate to be signed. This interface and internal timout + // is a remainder after the old design using raw watch wrapped with backoff. + crtPEM, err := csr.WaitForCertificate(client, req, certificateWaitTimeout) + if err != nil { utilruntime.HandleError(fmt.Errorf("Certificate request was not signed: %v", err)) return false, nil } diff --git a/staging/src/k8s.io/client-go/util/certificate/certificate_manager_test.go b/staging/src/k8s.io/client-go/util/certificate/certificate_manager_test.go index 3ca7f767a0..81f59b33eb 100644 --- a/staging/src/k8s.io/client-go/util/certificate/certificate_manager_test.go +++ b/staging/src/k8s.io/client-go/util/certificate/certificate_manager_test.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/wait" watch "k8s.io/apimachinery/pkg/watch" certificatesclient "k8s.io/client-go/kubernetes/typed/certificates/v1beta1" ) @@ -433,7 +432,8 @@ func TestRotateCertWaitingForResultError(t *testing.T) { }, } - certificateWaitBackoff = wait.Backoff{Steps: 1} + defer func(t time.Duration) { certificateWaitTimeout = t }(certificateWaitTimeout) + certificateWaitTimeout = 1 * time.Millisecond if success, err := m.rotateCerts(); success { t.Errorf("Got success from 'rotateCerts', wanted failure.") } else if err != nil { @@ -880,15 +880,6 @@ func TestServerHealth(t *testing.T) { expectRotateFail: true, expectHealthy: true, }, - { - description: "Conflict error on watch", - certs: currentCerts, - - failureType: watchError, - clientErr: errors.NewGenericServerResponse(409, "POST", schema.GroupResource{}, "", "", 0, false), - expectRotateFail: true, - expectHealthy: false, - }, } for _, tc := range testCases { diff --git a/staging/src/k8s.io/client-go/util/certificate/csr/csr.go b/staging/src/k8s.io/client-go/util/certificate/csr/csr.go index 6338eef933..1c8d0eb89c 100644 --- a/staging/src/k8s.io/client-go/util/certificate/csr/csr.go +++ b/staging/src/k8s.io/client-go/util/certificate/csr/csr.go @@ -17,6 +17,7 @@ limitations under the License. package csr import ( + "context" "crypto" "crypto/x509" "encoding/pem" @@ -83,19 +84,23 @@ func RequestCertificate(client certificatesclient.CertificateSigningRequestInter // WaitForCertificate waits for a certificate to be issued until timeout, or returns an error. func WaitForCertificate(client certificatesclient.CertificateSigningRequestInterface, req *certificates.CertificateSigningRequest, timeout time.Duration) (certData []byte, err error) { fieldSelector := fields.OneTermEqualSelector("metadata.name", req.Name).String() - - event, err := watchtools.ListWatchUntil( - timeout, - &cache.ListWatch{ - ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - options.FieldSelector = fieldSelector - return client.List(options) - }, - WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - options.FieldSelector = fieldSelector - return client.Watch(options) - }, + lw := &cache.ListWatch{ + ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { + options.FieldSelector = fieldSelector + return client.List(options) }, + WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { + options.FieldSelector = fieldSelector + return client.Watch(options) + }, + } + ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), timeout) + defer cancel() + event, err := watchtools.UntilWithSync( + ctx, + lw, + &certificates.CertificateSigningRequest{}, + nil, func(event watch.Event) (bool, error) { switch event.Type { case watch.Modified, watch.Added: From 29ba8b261a326609029020da2b091e7ef6879e29 Mon Sep 17 00:00:00 2001 From: Tomas Nozicka Date: Thu, 17 Jan 2019 18:34:02 +0100 Subject: [PATCH 2/2] Update bazel --- staging/src/k8s.io/client-go/util/certificate/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/staging/src/k8s.io/client-go/util/certificate/BUILD b/staging/src/k8s.io/client-go/util/certificate/BUILD index b86acbd42c..3d16014c00 100644 --- a/staging/src/k8s.io/client-go/util/certificate/BUILD +++ b/staging/src/k8s.io/client-go/util/certificate/BUILD @@ -20,7 +20,6 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/typed/certificates/v1beta1:go_default_library", ],