Adds retry support to methods called by the operator #58
Conversation
| "github.com/giantswarm/certctl/service/pki" | ||
| "github.com/giantswarm/certctl/service/token" | ||
| "github.com/giantswarm/certificatetpr" | ||
| microerror "github.com/giantswarm/microkit/error" | ||
| ) | ||
|
|
||
| // SetupPKI creates a PKI backend and policy if one does not exists for the cluster. | ||
| const ( | ||
| setupPKIMaxElapsedTime = 30 * time.Second |
There was a problem hiding this comment.
The timeouts are short because the timeout for the secrets watch is only 90 seconds. The operator needs to issue all certs in that time.
| operation := func() error { | ||
| err := s.SetupPKI(cert) | ||
| if err != nil { | ||
| s.Logger.Log("info", "failed to setup PKI - retrying") |
There was a problem hiding this comment.
This error handling is a bit uncommon. I would prefer returning the error here masked like we usually do and do logging additionally if desired. Below should be returned nil. Looks strange that the error is always returned. In case SetupPKI does weird things we forward the weirdness here. Lets not do this.
There was a problem hiding this comment.
My thinking was that SetupPKI would have the normal logging. In the wait wrapper I just wanted to indicate that a retry was taking place.
But it is different to our normal error handling and I get the point about returning the error. I'll take another look.
Fixes #10
This is some boyscouting I did a while back I want to finish off. It adds retry support to the methods called in the addFunc and deleteFunc.