Delete 'next' CertificateRequests that failed in last issuance cycle

So that the issuance is retried

Signed-off-by: irbekrm <irbekrm@gmail.com>
This commit is contained in:
irbekrm 2021-06-22 07:07:22 +01:00
parent feb62b1fe5
commit fd61e1ccc7
3 changed files with 81 additions and 0 deletions

View File

@ -66,5 +66,6 @@ go_test(
"@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
"@io_k8s_apimachinery//pkg/runtime:go_default_library",
"@io_k8s_client_go//testing:go_default_library",
"@io_k8s_utils//clock/testing:go_default_library",
],
)

View File

@ -195,6 +195,11 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
return err
}
requests, err = c.deleteCurrentFailedRequests(ctx, requests...)
if err != nil {
return err
}
if len(requests) > 1 {
// TODO: we should handle this case better, but for now do nothing to
// avoid getting into loops where we keep creating multiple requests
@ -212,6 +217,37 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
return c.createNewCertificateRequest(ctx, crt, pk, nextRevision, nextPrivateKeySecret.Name)
}
func (c *controller) deleteCurrentFailedRequests(ctx context.Context, reqs ...*cmapi.CertificateRequest) ([]*cmapi.CertificateRequest, error) {
log := logf.FromContext(ctx)
var remaining []*cmapi.CertificateRequest
for _, req := range reqs {
log = logf.WithRelatedResource(log, req)
// Check if there are any 'current' CertificateRequests that
// failed during the previous issuance cycle. Those should be
// deleted so that a new one gets created and the issuance is
// re-tried. In practice no more than one CertificateRequest is
// expected at this point.
cond := apiutil.GetCertificateRequestCondition(req, cmapi.CertificateRequestConditionReady)
if cond == nil || cond.Status != cmmeta.ConditionFalse || cond.Reason != cmapi.CertificateRequestReasonFailed {
remaining = append(remaining, req)
continue
}
// TODO: once we have implemented exponential back off for
// Certificate failures, this should be changed accordingly.
now := c.clock.Now()
durationSinceFailure := now.Sub(cond.LastTransitionTime.Time)
if durationSinceFailure >= certificates.RetryAfterLastFailure {
if err := c.client.CertmanagerV1().CertificateRequests(req.Namespace).Delete(ctx, req.Name, metav1.DeleteOptions{}); err != nil {
return nil, err
}
continue
}
remaining = append(remaining, req)
}
return remaining, nil
}
func (c *controller) deleteRequestsWithoutRevision(ctx context.Context, reqs ...*cmapi.CertificateRequest) ([]*cmapi.CertificateRequest, error) {
log := logf.FromContext(ctx)
var remaining []*cmapi.CertificateRequest

View File

@ -21,12 +21,14 @@ import (
"fmt"
"reflect"
"testing"
"time"
"github.com/kr/pretty"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
coretesting "k8s.io/client-go/testing"
fakeclock "k8s.io/utils/clock/testing"
cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
@ -76,6 +78,14 @@ func TestProcessItem(t *testing.T) {
},
Spec: cmapi.CertificateSpec{CommonName: "test-bundle-2"}},
)
fixedNow := metav1.NewTime(time.Now())
fixedClock := fakeclock.NewFakeClock(fixedNow.Time)
failedCRCondition := cmapi.CertificateRequestCondition{
Type: cmapi.CertificateRequestConditionReady,
Status: cmmeta.ConditionFalse,
Reason: cmapi.CertificateRequestReasonFailed,
LastTransitionTime: &metav1.Time{Time: fixedNow.Time.Add(-1 * time.Hour)},
}
tests := map[string]struct {
// key that should be passed to ProcessItem.
// if not set, the 'namespace/name' of the 'Certificate' field will be used.
@ -515,6 +525,39 @@ func TestProcessItem(t *testing.T) {
),
},
},
"should recreate the CertificateRequest if the current 'next' CertificateRequest failed during previous issuance cycle": {
secrets: []runtime.Object{
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "exists"},
Data: map[string][]byte{corev1.TLSPrivateKeyKey: bundle1.privateKeyBytes},
},
},
certificate: gen.CertificateFrom(bundle1.certificate,
gen.SetCertificateNextPrivateKeySecretName("exists"),
gen.SetCertificateStatusCondition(cmapi.CertificateCondition{Type: cmapi.CertificateConditionIssuing, Status: cmmeta.ConditionTrue}),
gen.SetCertificateRevision(5),
),
requests: []runtime.Object{
gen.CertificateRequestFrom(bundle1.certificateRequest,
gen.SetCertificateRequestAnnotations(map[string]string{
cmapi.CertificateRequestPrivateKeyAnnotationKey: "exists",
cmapi.CertificateRequestRevisionAnnotationKey: "6",
}),
gen.AddCertificateRequestStatusCondition(failedCRCondition),
),
},
expectedEvents: []string{`Normal Requested Created new CertificateRequest resource "test-notrandom"`},
expectedActions: []testpkg.Action{
testpkg.NewAction(coretesting.NewDeleteAction(cmapi.SchemeGroupVersion.WithResource("certificaterequests"), "testns", "test")),
testpkg.NewCustomMatch(coretesting.NewCreateAction(cmapi.SchemeGroupVersion.WithResource("certificaterequests"), "testns",
gen.CertificateRequestFrom(bundle1.certificateRequest,
gen.SetCertificateRequestAnnotations(map[string]string{
cmapi.CertificateRequestPrivateKeyAnnotationKey: "exists",
cmapi.CertificateRequestRevisionAnnotationKey: "6",
}),
)), relaxedCertificateRequestMatcher),
},
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
@ -524,6 +567,7 @@ func TestProcessItem(t *testing.T) {
ExpectedEvents: test.expectedEvents,
ExpectedActions: test.expectedActions,
StringGenerator: func(i int) string { return "notrandom" },
Clock: fixedClock,
}
if test.certificate != nil {
builder.CertManagerObjects = append(builder.CertManagerObjects, test.certificate)