Adds CertificateRequest FailureTime #1979
Conversation
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
FailureTime of the CertificateRequest status Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
jetstack-bot
added
release-note
jetstack-bot
added
area/api
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
jetstack-bot
added
the
area/deploy
| defer func() { | ||
| if _, saveErr := c.updateCertificateRequestStatus(ctx, cr, crCopy); saveErr != nil { | ||
| err = utilerrors.NewAggregate([]error{saveErr, err}) | ||
| } | ||
| }() | ||
|
|
||
| // If the CertificateRequest has the conditon 'Failed' then set the | ||
| // FailureTime to `c.clock.Now()` | ||
| defer c.setFailureTime(crCopy) |
There was a problem hiding this comment.
Instead of including this here, can we include it as part of the updateCertificateRequestStatus func?
Alternatively, if we simply set the FailureTime at the point where we set the reason to Failed, we can be sure that both are recorded at the same time anyway (i.e. in the Recorder).
There was a problem hiding this comment.
(now that I've thought about it, the recorder probably is the most sensible place to set this.. not sure what you think?)
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
failed but < 1 hour Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
|
/unassign |
jetstack-bot
added
size/XL
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
JoshVanL
force-pushed
the
cr-failure-time
branch
from
2cc9c95
to
7e56ce1
Compare
| // If pending condition not already set then fire a Pending Event. This is to | ||
| // reduce strain on the API server and avoid rate limiting ourselves for | ||
| // Event creation. | ||
| if !apiutil.CertificateRequestHasCondition(cr, cmapi.CertificateRequestCondition{ |
There was a problem hiding this comment.
nit: we can use the new ReadyReason function you just added here to save some lines.. but not too important 😄
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoshVanL, munnerz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Adds FailureTime to CertificateRequet Status.
Time set to now on the CertificateRequest if failed and the FailureTime is not currently set.
The Certificate controller will re-try CertificateRequests that have failed and have a FailureTime of at least one hour in the past. If the FailureTime is less than one hour, the Certificate is rescheduled for syncing in one hour.
/assign @munnerz