Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add retries for Barbican #289

Merged
merged 4 commits into from
Mar 3, 2025
+18 −0
Merged

Add retries for Barbican #289

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
da20033
Add retries for Barbican
velp Feb 25, 2025
10bc79a
Fix exception handling
velp Feb 25, 2025
beae816
Fix lint
velp Feb 25, 2025
e2a3326
Remove tuple
velp Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions octavia_f5/utils/cert_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
# under the License.

import hashlib
import tenacity
from requests import exceptions as requests_exc
from urllib3 import exceptions as urllib_exc

from oslo_config import cfg
from oslo_context import context as oslo_context
Expand All @@ -23,6 +26,10 @@
from octavia_f5.restclient.as3objects import certificate as m_cert

CONF = cfg.CONF
RETRY_ATTEMPTS = 15
RETRY_INITIAL_DELAY = 1
RETRY_BACKOFF = 1
RETRY_MAX = 5


class CertManagerWrapper(object):
Expand All @@ -33,6 +40,13 @@ def __init__(self):
invoke_on_load=True,
).driver

@tenacity.retry(
retry=tenacity.retry_if_exception_type(
(urllib_exc.NewConnectionError,
requests_exc.ConnectionError)),
Copy link
Collaborator

@BenjaminLudwigSAP BenjaminLudwigSAP Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Why did you decide on these two exceptions? Where are they thrown?
  • I don't think these exceptions can occur here. Because get_certificates calls cert_parser.load_certificates_data (which is here), which catches all exceptions and raises only a CertificateRetrievalException. Maybe I'm missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

wait=tenacity.wait_incrementing(
RETRY_INITIAL_DELAY, RETRY_BACKOFF, RETRY_MAX),
stop=tenacity.stop_after_attempt(RETRY_ATTEMPTS))
def get_certificates(self, obj, context=None):
"""Fetches certificates and creates dict out of octavia objects

Expand Down Expand Up @@ -68,6 +82,13 @@ def get_certificates(self, obj, context=None):

return certificates

@tenacity.retry(
retry=tenacity.retry_if_exception_type(
(urllib_exc.NewConnectionError,
requests_exc.ConnectionError)),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, left comment to previous one.

wait=tenacity.wait_incrementing(
RETRY_INITIAL_DELAY, RETRY_BACKOFF, RETRY_MAX),
stop=tenacity.stop_after_attempt(RETRY_ATTEMPTS))
def load_secret(self, project_id, secret_ref):
"""Loads secrets from secret store

Expand Down
Loading