Add retries for Barbican #289
Conversation
octavia_f5/utils/cert_manager.py
Outdated
| (urllib_exc.NewConnectionError, | ||
| requests_exc.ConnectionError)), |
There was a problem hiding this comment.
- Why did you decide on these two exceptions? Where are they thrown?
- I don't think these exceptions can occur here. Because
get_certificatescallscert_parser.load_certificates_data(which is here), which catches all exceptions and raises only aCertificateRetrievalException. Maybe I'm missing something?
There was a problem hiding this comment.
Yes, good catch. I checked the source class of Barbican https://github.com/sapcc/octavia/blob/600b5bae7537bc2fcf36c597818bfc4b7d48de10/octavia/certificates/manager/barbican.py#L97 but did not see that exception will be wrapped.
octavia_f5/utils/cert_manager.py
Outdated
| (urllib_exc.NewConnectionError, | ||
| requests_exc.ConnectionError)), |
There was a problem hiding this comment.
I don't think these exceptions can occur here either. Because load_secret calls self.cert_manager.get_secret, which defaults to this function here, which catches all exceptions and raises only a CertificateRetrievalException. Maybe I'm missing something?
There was a problem hiding this comment.
Fixed, left comment to previous one.
octavia_f5/utils/cert_manager.py
Outdated
| @@ -33,6 +39,12 @@ def __init__(self): | |||
| invoke_on_load=True, | |||
| ).driver | |||
|
|
|||
| @tenacity.retry( | |||
| retry=tenacity.retry_if_exception_type( | |||
| (octavia_exc.CertificateRetrievalException)), | |||
There was a problem hiding this comment.
As far as I understand it from the Tenacity docs, this does not have to be a tuple, so you can leave out the parentheses here. But it's okay if you don't, I'm still approving this PR 👍
octavia_f5/utils/cert_manager.py
Outdated
| @@ -68,6 +80,12 @@ def get_certificates(self, obj, context=None): | |||
|
|
|||
| return certificates | |||
|
|
|||
| @tenacity.retry( | |||
| retry=tenacity.retry_if_exception_type( | |||
| (octavia_exc.CertificateRetrievalException)), | |||
This PR adds a simple retries mechanism for requests to Barbican. Hardcoded values were used to keep the same style for calls of this decorator: https://github.com/search?q=repo%3Asapcc%2Foctavia-f5-provider-driver%20%22tenacity.retry%22&type=code