This reverts commit 641960b6. The reason we decided to revert this is
that we are unsure about the implications of adding the
scheduledWorkQueue.Forget call. The new Forget call is left untested,
and it makes us nervous not to know exactly if it works as intended.
The "Forget" memory leak that we are reverting now is the cause of a
tiny fraction of the overall memory leakage that was fixed in the PR
in the scheduler itself. Reverting this means that some goroutines will
be leaked, but only when a Certificate gets removed and never recreated
with the same name.
Signed-off-by: Maël Valais <mael@vls.dev>
The log message:
multiple CertificateRequests found for the 'next' revision 2,
skipping issuance until no more duplicate.
can be better phrased as:
multiple CertificateRequests are found for the 'next' revision 2,
issuance is skipped until there are no more duplicates.
Signed-off-by: Maël Valais <mael@vls.dev>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
Injecting the whole Gatherer struct was not necessary for testing
since DataForCertificate is now fully unit-tested. With that, we
can mock the Gatherer.Evaluate function. Since there is no reason
to inject a full Gatherer object into the trigger controller, I chose
to inject a simple policies.Func. I named the function "shouldReissue"
since this is exactly what this function does.
I also refactored the test cases to use the same gen.Certificate
that we use in the rest of the codebase.
Signed-off-by: Maël Valais <mael@vls.dev>
As Maartje mentioned, it doesn't make sense to return backoff = true
while returning a delay of 0. Also, use time.UTC instead of time.Local.
Signed-off-by: Maël Valais <mael.valais@gmail.com>
The trigger_controller_test.go has many unrelated test cases and I
thought it would be good to have more tightly scoped functions that are
easy to review (and most importantly, the unit tests are easy to
review).
Signed-off-by: Maël Valais <mael.valais@gmail.com>
