Skip to content

Commit 0e644ff

Browse files
jetstack-botjetstack-bot
authored
Merge pull request #18 from cert-manager/add_timing_test
Add timing test for Check and Sign
2 parents e4e7106 + 300427a commit 0e644ff

File tree

4 files changed

+339
-0
lines changed

4 files changed

+339
-0
lines changed

‎controllers/combined_controller.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"k8s.io/client-go/tools/record"
2626
"k8s.io/utils/clock"
2727
ctrl "sigs.k8s.io/controller-runtime"
28+
"sigs.k8s.io/controller-runtime/pkg/builder"
2829
"sigs.k8s.io/controller-runtime/pkg/controller"
2930

3031
v1alpha1 "github.com/cert-manager/issuer-lib/api/v1alpha1"
@@ -81,6 +82,14 @@ type CombinedController struct {
8182
// controller.
8283
DisableKubernetesCSRController bool
8384

85+
// PreSetupWithManager is an optional function that can be used to perform
86+
// additional setup before the controller is built and registered with the
87+
// manager.
88+
PreSetupWithManager func(context.Context, schema.GroupVersionKind, ctrl.Manager, *builder.Builder) error
89+
90+
// PostSetupWithManager is an optional function that can be used to perform
91+
// additional setup after the controller is built and registered with the
92+
// manager.
8493
PostSetupWithManager func(context.Context, schema.GroupVersionKind, ctrl.Manager, controller.Controller) error
8594
}
8695

@@ -106,6 +115,7 @@ func (r *CombinedController) SetupWithManager(ctx context.Context, mgr ctrl.Mana
106115
EventRecorder: r.EventRecorder,
107116
Clock: r.Clock,
108117

118+
PreSetupWithManager: r.PreSetupWithManager,
109119
PostSetupWithManager: r.PostSetupWithManager,
110120
}).SetupWithManager(ctx, mgr); err != nil {
111121
return fmt.Errorf("%T: %w", issuerType, err)
@@ -132,6 +142,7 @@ func (r *CombinedController) SetupWithManager(ctx context.Context, mgr ctrl.Mana
132142
EventRecorder: r.EventRecorder,
133143
Clock: r.Clock,
134144

145+
PreSetupWithManager: r.PreSetupWithManager,
135146
PostSetupWithManager: r.PostSetupWithManager,
136147
},
137148

@@ -157,6 +168,7 @@ func (r *CombinedController) SetupWithManager(ctx context.Context, mgr ctrl.Mana
157168
EventRecorder: r.EventRecorder,
158169
Clock: r.Clock,
159170

171+
PreSetupWithManager: r.PreSetupWithManager,
160172
PostSetupWithManager: r.PostSetupWithManager,
161173
},
162174
}).SetupWithManager(ctx, mgr); err != nil {

‎controllers/combined_controller_integration_test.go‎

Lines changed: 295 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,27 @@ package controllers
1919
import (
2020
"context"
2121
"fmt"
22+
"sync"
2223
"testing"
2324
"time"
2425

2526
cmutil "github.com/cert-manager/cert-manager/pkg/api/util"
2627
cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
2728
cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
2829
cmgen "github.com/cert-manager/cert-manager/test/unit/gen"
30+
"github.com/stretchr/testify/assert"
2931
"github.com/stretchr/testify/require"
3032
"k8s.io/apimachinery/pkg/runtime"
33+
"k8s.io/apimachinery/pkg/runtime/schema"
3134
"k8s.io/apimachinery/pkg/watch"
3235
"k8s.io/client-go/tools/record"
36+
"k8s.io/client-go/util/retry"
37+
"k8s.io/client-go/util/workqueue"
3338
"k8s.io/utils/clock"
3439
ctrl "sigs.k8s.io/controller-runtime"
40+
"sigs.k8s.io/controller-runtime/pkg/builder"
41+
"sigs.k8s.io/controller-runtime/pkg/client"
42+
"sigs.k8s.io/controller-runtime/pkg/controller"
3543

3644
v1alpha1 "github.com/cert-manager/issuer-lib/api/v1alpha1"
3745
"github.com/cert-manager/issuer-lib/conditions"
@@ -278,3 +286,290 @@ func TestCombinedControllerTemporaryFailedCertificateRequestRetrigger(t *testing
278286
})
279287
}
280288
}
289+
290+
func TestCombinedControllerTiming(t *testing.T) { //nolint:tparallel
291+
t.Parallel()
292+
293+
t.Log(
294+
"Tests to show that the CertificateRequest controller and Issuer controller call the Check and Sign functions at the correct times",
295+
)
296+
297+
fieldOwner := "failed-certificate-request-should-retrigger-issuer"
298+
299+
ctx := testcontext.ForTest(t)
300+
kubeClients := testresource.KubeClients(t, nil)
301+
302+
type simulatedCheckResult struct {
303+
err error
304+
}
305+
type simulatedSignResult struct {
306+
cert []byte
307+
err error
308+
}
309+
310+
type simulatedResult struct {
311+
*simulatedCheckResult
312+
*simulatedSignResult
313+
expectedSinceLastResult time.Duration
314+
}
315+
316+
type testcase struct {
317+
name string
318+
maxRetryDuration time.Duration
319+
results []simulatedResult
320+
}
321+
322+
testcases := []testcase{
323+
{
324+
name: "single-error-for-issuer-and-certificate-request",
325+
maxRetryDuration: 1 * time.Hour,
326+
results: []simulatedResult{
327+
{
328+
simulatedCheckResult: &simulatedCheckResult{err: fmt.Errorf("[error message]")},
329+
expectedSinceLastResult: 0,
330+
},
331+
{
332+
simulatedCheckResult: &simulatedCheckResult{err: nil},
333+
expectedSinceLastResult: 200 * time.Millisecond,
334+
},
335+
{
336+
simulatedSignResult: &simulatedSignResult{cert: nil, err: fmt.Errorf("[error message]")},
337+
expectedSinceLastResult: 0,
338+
},
339+
{
340+
simulatedSignResult: &simulatedSignResult{cert: []byte("cert"), err: nil},
341+
expectedSinceLastResult: 200 * time.Millisecond,
342+
},
343+
},
344+
},
345+
{
346+
name: "double-error-for-issuer-and-certificate-request",
347+
maxRetryDuration: 1 * time.Hour,
348+
results: []simulatedResult{
349+
{
350+
simulatedCheckResult: &simulatedCheckResult{err: fmt.Errorf("[error message]")},
351+
expectedSinceLastResult: 0,
352+
},
353+
{
354+
simulatedCheckResult: &simulatedCheckResult{err: fmt.Errorf("[error message]")},
355+
expectedSinceLastResult: 200 * time.Millisecond,
356+
},
357+
{
358+
simulatedCheckResult: &simulatedCheckResult{err: nil},
359+
expectedSinceLastResult: 400 * time.Millisecond,
360+
},
361+
{
362+
simulatedSignResult: &simulatedSignResult{cert: nil, err: fmt.Errorf("[error message]")},
363+
expectedSinceLastResult: 0,
364+
},
365+
{
366+
simulatedSignResult: &simulatedSignResult{cert: nil, err: fmt.Errorf("[error message]")},
367+
expectedSinceLastResult: 200 * time.Millisecond,
368+
},
369+
{
370+
simulatedSignResult: &simulatedSignResult{cert: []byte("cert"), err: nil},
371+
expectedSinceLastResult: 400 * time.Millisecond,
372+
},
373+
},
374+
},
375+
{
376+
name: "single-error-for-issuer-and-certificate-request-reaching-max-retry-duration",
377+
maxRetryDuration: 300 * time.Millisecond, // should cause temporary CertificateRequest errors to fail permanently
378+
results: []simulatedResult{
379+
{
380+
simulatedCheckResult: &simulatedCheckResult{err: fmt.Errorf("[error message]")},
381+
expectedSinceLastResult: 0,
382+
},
383+
{
384+
simulatedCheckResult: &simulatedCheckResult{err: nil},
385+
expectedSinceLastResult: 200 * time.Millisecond,
386+
},
387+
{
388+
simulatedSignResult: &simulatedSignResult{cert: nil, err: fmt.Errorf("[error message]")},
389+
expectedSinceLastResult: 0,
390+
},
391+
},
392+
},
393+
{
394+
name: "single-pending-error-for-issuer-and-certificate-request-reaching-max-retry-duration",
395+
maxRetryDuration: 300 * time.Millisecond, // should cause temporary CertificateRequest errors to fail permanently
396+
results: []simulatedResult{
397+
{
398+
simulatedCheckResult: &simulatedCheckResult{err: fmt.Errorf("[error message]")},
399+
expectedSinceLastResult: 0,
400+
},
401+
{
402+
simulatedCheckResult: &simulatedCheckResult{err: nil},
403+
expectedSinceLastResult: 200 * time.Millisecond,
404+
},
405+
{
406+
simulatedSignResult: &simulatedSignResult{cert: nil, err: signer.PendingError{Err: fmt.Errorf("[error message]")}},
407+
expectedSinceLastResult: 0,
408+
},
409+
{
410+
simulatedSignResult: &simulatedSignResult{cert: []byte("ok"), err: nil},
411+
expectedSinceLastResult: 200 * time.Millisecond,
412+
},
413+
},
414+
},
415+
{
416+
name: "fail-issuer-permanently",
417+
maxRetryDuration: 1 * time.Hour,
418+
results: []simulatedResult{
419+
{
420+
simulatedCheckResult: &simulatedCheckResult{err: signer.PermanentError{Err: fmt.Errorf("[error message]")}},
421+
expectedSinceLastResult: 0,
422+
},
423+
},
424+
},
425+
{
426+
name: "trigger-issuer-error-then-recover",
427+
maxRetryDuration: 1 * time.Hour,
428+
results: []simulatedResult{
429+
{
430+
simulatedCheckResult: &simulatedCheckResult{err: nil},
431+
expectedSinceLastResult: 0,
432+
},
433+
{
434+
simulatedSignResult: &simulatedSignResult{cert: nil, err: signer.IssuerError{Err: fmt.Errorf("[error message]")}},
435+
expectedSinceLastResult: 0,
436+
},
437+
{
438+
simulatedCheckResult: &simulatedCheckResult{err: nil},
439+
expectedSinceLastResult: 200 * time.Millisecond,
440+
},
441+
{
442+
simulatedSignResult: &simulatedSignResult{cert: []byte("ok"), err: nil},
443+
expectedSinceLastResult: 0,
444+
},
445+
},
446+
},
447+
}
448+
449+
for _, tc := range testcases {
450+
tc := tc
451+
452+
t.Run(tc.name, func(t *testing.T) {
453+
resultsMutex := sync.Mutex{}
454+
resultsIndex := 0
455+
results := tc.results
456+
durations := make([]time.Time, len(results))
457+
errorCh := make(chan error)
458+
done := make(chan struct{})
459+
460+
ctx := setupControllersAPIServerAndClient(t, ctx, kubeClients,
461+
func(mgr ctrl.Manager) controllerInterface {
462+
return &CombinedController{
463+
IssuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
464+
ClusterIssuerTypes: []v1alpha1.Issuer{&api.TestClusterIssuer{}},
465+
FieldOwner: fieldOwner,
466+
MaxRetryDuration: tc.maxRetryDuration,
467+
Check: func(_ context.Context, _ v1alpha1.Issuer) error {
468+
resultsMutex.Lock()
469+
defer resultsMutex.Unlock()
470+
defer func() { resultsIndex++ }()
471+
472+
if resultsIndex >= len(results)-1 {
473+
if resultsIndex > len(results)-1 {
474+
errorCh <- fmt.Errorf("too many calls to Check")
475+
return nil
476+
}
477+
defer close(done)
478+
}
479+
durations[resultsIndex] = time.Now()
480+
if results[resultsIndex].simulatedCheckResult == nil {
481+
errorCh <- fmt.Errorf("unexpected call to Check")
482+
return nil
483+
}
484+
return results[resultsIndex].simulatedCheckResult.err
485+
},
486+
Sign: func(_ context.Context, _ signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) {
487+
resultsMutex.Lock()
488+
defer resultsMutex.Unlock()
489+
defer func() { resultsIndex++ }()
490+
491+
if resultsIndex >= len(results)-1 {
492+
if resultsIndex > len(results)-1 {
493+
errorCh <- fmt.Errorf("too many calls to Sign")
494+
return signer.PEMBundle{}, nil
495+
}
496+
defer close(done)
497+
}
498+
durations[resultsIndex] = time.Now()
499+
if results[resultsIndex].simulatedSignResult == nil {
500+
errorCh <- fmt.Errorf("unexpected call to Sign")
501+
return signer.PEMBundle{}, nil
502+
}
503+
result := results[resultsIndex].simulatedSignResult
504+
return signer.PEMBundle{
505+
ChainPEM: result.cert,
506+
}, result.err
507+
},
508+
EventRecorder: record.NewFakeRecorder(100),
509+
510+
PreSetupWithManager: func(ctx context.Context, gvk schema.GroupVersionKind, mgr ctrl.Manager, b *builder.Builder) error {
511+
b.WithOptions(controller.Options{
512+
RateLimiter: workqueue.NewItemExponentialFailureRateLimiter(200*time.Millisecond, 5*time.Second),
513+
})
514+
return nil
515+
},
516+
}
517+
},
518+
)
519+
520+
t.Logf("Creating a namespace")
521+
namespace, cleanup := kubeClients.SetupNamespace(t, ctx)
522+
defer cleanup()
523+
524+
issuer := testutil.TestIssuer(
525+
"issuer-1",
526+
testutil.SetTestIssuerNamespace(namespace),
527+
)
528+
529+
cr := cmgen.CertificateRequest(
530+
"certificate-request-1",
531+
cmgen.SetCertificateRequestNamespace(namespace),
532+
cmgen.SetCertificateRequestCSR([]byte("doo")),
533+
cmgen.SetCertificateRequestIssuer(cmmeta.ObjectReference{
534+
Name: issuer.Name,
535+
Kind: issuer.Kind,
536+
Group: api.SchemeGroupVersion.Group,
537+
}),
538+
)
539+
540+
require.NoError(t, kubeClients.Client.Create(ctx, issuer))
541+
createApprovedCR(t, ctx, kubeClients.Client, clock.RealClock{}, cr)
542+
543+
<-done
544+
time.Sleep(1 * time.Second)
545+
select {
546+
case err := <-errorCh:
547+
assert.NoError(t, err)
548+
default:
549+
}
550+
551+
require.NoError(t, retry.RetryOnConflict(retry.DefaultRetry, func() error {
552+
err := kubeClients.Client.Get(ctx, client.ObjectKeyFromObject(cr), cr)
553+
if err != nil {
554+
return err
555+
}
556+
return kubeClients.Client.Delete(ctx, cr)
557+
}))
558+
require.NoError(t, retry.RetryOnConflict(retry.DefaultRetry, func() error {
559+
err := kubeClients.Client.Get(ctx, client.ObjectKeyFromObject(issuer), issuer)
560+
if err != nil {
561+
return err
562+
}
563+
return kubeClients.Client.Delete(ctx, issuer)
564+
}))
565+
566+
for i := 1; i < len(results); i++ {
567+
measuredDuration := durations[i].Sub(durations[i-1])
568+
expectedDuration := results[i].expectedSinceLastResult
569+
570+
require.True(t, expectedDuration-150*time.Millisecond < measuredDuration, "result %d: expected %v, got %v", i, expectedDuration, measuredDuration)
571+
require.True(t, expectedDuration+150*time.Millisecond > measuredDuration, "result %d: expected %v, got %v", i, expectedDuration, measuredDuration)
572+
}
573+
})
574+
}
575+
}

‎controllers/issuer_controller.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ type IssuerReconciler struct {
7373
// Clock is used to mock condition transition times in tests.
7474
Clock clock.PassiveClock
7575

76+
// PreSetupWithManager is an optional function that can be used to perform
77+
// additional setup before the controller is built and registered with the
78+
// manager.
79+
PreSetupWithManager func(context.Context, schema.GroupVersionKind, ctrl.Manager, *builder.Builder) error
80+
81+
// PostSetupWithManager is an optional function that can be used to perform
82+
// additional setup after the controller is built and registered with the
83+
// manager.
7684
PostSetupWithManager func(context.Context, schema.GroupVersionKind, ctrl.Manager, controller.Controller) error
7785
}
7886

@@ -265,6 +273,14 @@ func (r *IssuerReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manage
265273
nil,
266274
)
267275

276+
if r.PreSetupWithManager != nil {
277+
err := r.PreSetupWithManager(ctx, forObjectGvk, mgr, build)
278+
r.PreSetupWithManager = nil // free setup function
279+
if err != nil {
280+
return err
281+
}
282+
}
283+
268284
if controller, err := build.Build(r); err != nil {
269285
return err
270286
} else if r.PostSetupWithManager != nil {

0 commit comments

Comments
 (0)