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.

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

BRE-342 Implement retry mechanism with error handling to get secrets from KV #329

Merged
merged 7 commits into from
Oct 3, 2024

Conversation

urbinaalex17
Copy link
Contributor

@urbinaalex17 urbinaalex17 commented Sep 30, 2024

🎟️ Tracking

BRE-342

📔 Objective

Fix get-keyvault-secrets action intermittent failures by implementing retry mechanism with error handling to get secrets from Azure Key Vault.

📸 Screenshots

Known issues: https://github.com/bitwarden/server/actions/runs/11033476838/job/30644953330

image

In this execution we can clearly see that the retry mechanism was executed due to the execution time, 9 seconds compare to the 6 seconds when the step failed (RETRY_DELAY = 3000):

image

https://github.com/bitwarden/server/actions/runs/11167289676/job/31043388477

Vivaldi 2024-10-03 12 33 46

Multiple jobs were triggered:

image

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@urbinaalex17 urbinaalex17 self-assigned this Sep 30, 2024
@urbinaalex17 urbinaalex17 requested a review from a team as a code owner September 30, 2024 14:50
Copy link

github-actions bot commented Sep 30, 2024

Logo
Checkmarx One – Scan Summary & Details5e98459c-3a71-4108-922a-71707cdbce68

No New Or Fixed Issues Found

@urbinaalex17 urbinaalex17 changed the title BRE-342 Print error message for debuging purposes BRE-342 Implement retry mechanism with error handling to get secrets from KV Oct 2, 2024
…ive code into a helper function for cleaner retries
Copy link
Member

@vgrassia vgrassia left a comment

Choose a reason for hiding this comment

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

The PR body is missing all the information. Can you please fill it out?

@urbinaalex17 urbinaalex17 enabled auto-merge (squash) October 3, 2024 18:34
Copy link
Member

@vgrassia vgrassia left a comment

Choose a reason for hiding this comment

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

I was able to test with debug and finally get an error that retried successfully. This will work as a temporary fix until we can move on from using Azure Key Vaults.

@urbinaalex17 urbinaalex17 merged commit 525b3ea into main Oct 3, 2024
5 checks passed
@urbinaalex17 urbinaalex17 deleted the task/BRE-342 branch October 3, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants