From abc39053b2bef7b7a1297e0e4b93eac06255992b Mon Sep 17 00:00:00 2001 From: Inteon <42113979+inteon@users.noreply.github.com> Date: Sat, 7 Aug 2021 10:19:07 +0200 Subject: [PATCH] resolve .Stop() failures Signed-off-by: Inteon <42113979+inteon@users.noreply.github.com> --- pkg/webhook/authority/BUILD.bazel | 1 + pkg/webhook/authority/authority.go | 21 ++++++++++++++----- pkg/webhook/server/tls/dynamic_source.go | 11 ++++++++++ .../webhook/dynamic_authority_test.go | 17 +++++++++++++-- .../webhook/dynamic_source_test.go | 17 +++++++++++++-- 5 files changed, 58 insertions(+), 9 deletions(-) diff --git a/pkg/webhook/authority/BUILD.bazel b/pkg/webhook/authority/BUILD.bazel index fbb660b5a..7b7aee7e0 100644 --- a/pkg/webhook/authority/BUILD.bazel +++ b/pkg/webhook/authority/BUILD.bazel @@ -6,6 +6,7 @@ go_library( importpath = "github.com/jetstack/cert-manager/pkg/webhook/authority", visibility = ["//visibility:public"], deps = [ + "//cmd/util:go_default_library", "//pkg/apis/certmanager/v1:go_default_library", "//pkg/internal/apis/meta:go_default_library", "//pkg/logs:go_default_library", diff --git a/pkg/webhook/authority/authority.go b/pkg/webhook/authority/authority.go index edd954468..b1930c5f1 100644 --- a/pkg/webhook/authority/authority.go +++ b/pkg/webhook/authority/authority.go @@ -44,6 +44,7 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" + "github.com/jetstack/cert-manager/cmd/util" cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/jetstack/cert-manager/pkg/internal/apis/meta" "github.com/jetstack/cert-manager/pkg/util/pki" @@ -93,6 +94,8 @@ type SignFunc func(template *x509.Certificate) (*x509.Certificate, error) var _ SignFunc = (&DynamicAuthority{}).Sign func (d *DynamicAuthority) Run(stopCh <-chan struct{}) error { + ctx := util.ContextWithStopCh(context.Background(), stopCh) + if d.SecretNamespace == "" { return fmt.Errorf("SecretNamespace must be set") } @@ -129,8 +132,8 @@ func (d *DynamicAuthority) Run(stopCh <-chan struct{}) error { d.client = cl.CoreV1().Secrets(d.SecretNamespace) // start the informers and wait for the cache to sync - factory.Start(stopCh) - if !cache.WaitForCacheSync(stopCh, informer.HasSynced) { + factory.Start(ctx.Done()) + if !cache.WaitForCacheSync(ctx.Done(), informer.HasSynced) { return fmt.Errorf("failed waiting for informer caches to sync") } @@ -138,15 +141,23 @@ func (d *DynamicAuthority) Run(stopCh <-chan struct{}) error { // been missed that could cause us to get into an idle state where the // Secret resource does not exist and so the informers handler functions // are not triggered. - return wait.PollImmediateUntil(time.Second*10, func() (done bool, err error) { - ctx := context.Background() + if err = wait.PollImmediateUntil(time.Second*10, func() (done bool, err error) { if err := d.ensureCA(ctx); err != nil { d.Log.Error(err, "error ensuring CA") } // never return 'done'. // this poll only ends when stopCh is closed. return false, nil - }, stopCh) + }, ctx.Done()); err != nil { + // If error cause was context, return that error instead + if ctx.Err() != nil { + return ctx.Err() + } + + return err + } + + return nil } // Sign will sign the given certificate template using the current version of diff --git a/pkg/webhook/server/tls/dynamic_source.go b/pkg/webhook/server/tls/dynamic_source.go index 6642bacc5..1d4be54c1 100644 --- a/pkg/webhook/server/tls/dynamic_source.go +++ b/pkg/webhook/server/tls/dynamic_source.go @@ -21,6 +21,7 @@ import ( "crypto" "crypto/tls" "crypto/x509" + "errors" "fmt" "sync" "time" @@ -95,6 +96,11 @@ func (f *DynamicSource) Run(stopCh <-chan struct{}) error { // In case of an error, the stopCh is closed; wait for authorityErrChan to be closed too <-authorityErrChan + // If there was an ErrWaitTimeout error, this must be caused by closing stopCh + if errors.Is(err, wait.ErrWaitTimeout) { + return context.Canceled + } + return err } @@ -183,6 +189,11 @@ func (f *DynamicSource) Run(stopCh <-chan struct{}) error { <-rotationChan <-renewalChan + // If there was an ErrWaitTimeout error, this must be caused by closing stopCh + if errors.Is(err, wait.ErrWaitTimeout) { + return context.Canceled + } + return err } diff --git a/test/integration/webhook/dynamic_authority_test.go b/test/integration/webhook/dynamic_authority_test.go index 45449c491..374e7d799 100644 --- a/test/integration/webhook/dynamic_authority_test.go +++ b/test/integration/webhook/dynamic_authority_test.go @@ -21,6 +21,7 @@ import ( "context" "crypto/tls" "crypto/x509" + "errors" "fmt" "testing" "time" @@ -63,9 +64,15 @@ func TestDynamicAuthority_Bootstrap(t *testing.T) { Log: logtesting.TestLogger{T: t}, } stopCh := make(chan struct{}) + doneCh := make(chan struct{}) + defer func() { + close(stopCh) + <-doneCh + }() // run the dynamic authority controller in the background go func() { - if err := auth.Run(stopCh); err != nil { + defer close(doneCh) + if err := auth.Run(stopCh); err != nil && !errors.Is(err, context.Canceled) { t.Fatalf("Unexpected error running authority: %v", err) } }() @@ -102,9 +109,15 @@ func TestDynamicAuthority_Recreates(t *testing.T) { Log: logtesting.TestLogger{T: t}, } stopCh := make(chan struct{}) + doneCh := make(chan struct{}) + defer func() { + close(stopCh) + <-doneCh + }() // run the dynamic authority controller in the background go func() { - if err := auth.Run(stopCh); err != nil { + defer close(doneCh) + if err := auth.Run(stopCh); err != nil && !errors.Is(err, context.Canceled) { t.Fatalf("Unexpected error running authority: %v", err) } }() diff --git a/test/integration/webhook/dynamic_source_test.go b/test/integration/webhook/dynamic_source_test.go index fd2b58c35..f7e08940b 100644 --- a/test/integration/webhook/dynamic_source_test.go +++ b/test/integration/webhook/dynamic_source_test.go @@ -19,6 +19,7 @@ package webhook import ( "context" "crypto/x509" + "errors" "math/big" "testing" "time" @@ -63,9 +64,15 @@ func TestDynamicSource_Bootstrap(t *testing.T) { Log: log, } stopCh := make(chan struct{}) + doneCh := make(chan struct{}) + defer func() { + close(stopCh) + <-doneCh + }() // run the dynamic authority controller in the background go func() { - if err := source.Run(stopCh); err != nil { + defer close(doneCh) + if err := source.Run(stopCh); err != nil && !errors.Is(err, context.Canceled) { t.Fatalf("Unexpected error running source: %v", err) } }() @@ -120,9 +127,15 @@ func TestDynamicSource_CARotation(t *testing.T) { Log: log, } stopCh := make(chan struct{}) + doneCh := make(chan struct{}) + defer func() { + close(stopCh) + <-doneCh + }() // run the dynamic authority controller in the background go func() { - if err := source.Run(stopCh); err != nil { + defer close(doneCh) + if err := source.Run(stopCh); err != nil && !errors.Is(err, context.Canceled) { t.Fatalf("Unexpected error running source: %v", err) } }()