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

fix: unstick helm commands by removing helm secrets #154

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

colesnodgrass
Copy link
Member

  • fix https://github.com/airbytehq/airbyte-internal-issues/issues/8885
    • If the previous abctl local install attempt was cut short it could get stuck in the helm state pending-update. This prevents the next abctl local install command from succeeding. The error would be something similar to another operation (install/upgrade/rollback) is in progress
    • The abctl local install command will now check for this error, attempt to resolve it (by removing the helm secrets) and trying the install/upgrade again.
    • Additional information about why removing the helm secret bypassing this problem can be found here

@colesnodgrass colesnodgrass requested a review from a team as a code owner December 13, 2024 01:17
@colesnodgrass colesnodgrass changed the title fix: unstick stuck helm commands by removing helm secrets fix: unstick helm commands by removing helm secrets Dec 13, 2024
)

if err != nil {
if strings.Contains(err.Error(), "another operation (install/upgrade/rollback) is in progress") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't you need to check attempt count somewhere? I think you could check it here

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't need to check it here, but I need to check if it was successful outside of this loop.

Copy link
Member Author

@colesnodgrass colesnodgrass Dec 13, 2024

Choose a reason for hiding this comment

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

@abuchanan-airbyte I added a check after the for-loop to ensure that helmRelease was properly set (and a new unit-test as well). I don't care what the attemptCount value is, I only care that we attempted more than once (after removing the helm-secrets on the first failed attempt) and less an an infinite amount. I picked three attempts as it was one more than the number of attempts it should realistically take (two).

Additionally I added a new localerr.ErrHelmStuck error that will now display information to the end-user on how to fix this problem if the newly added (in this PR) functionality to fix this problem for them fails.

@colesnodgrass colesnodgrass merged commit 4dd803e into main Dec 13, 2024
2 checks passed
@colesnodgrass colesnodgrass deleted the cole/fix-helm-stuck-problem branch December 13, 2024 16:25
Copy link

sentry-io bot commented Dec 14, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ *fmt.wrapError: unable to install airbyte chart: unable to install helm: unable to build kubernetes objects from ... github.com/airbytehq/abctl/internal/trace in Sp... View Issue
  • ‼️ *fmt.wrapError: unable to install airbyte chart: unable to install helm: Kubernetes cluster unreachable: Get "htt... github.com/airbytehq/abctl/internal/trace in Sp... View Issue
  • ‼️ *fmt.wrapError: unable to install airbyte chart: unable to install helm: Unable to continue with update: Role "ai... github.com/airbytehq/abctl/internal/trace in Sp... View Issue

Did you find this useful? React with a 👍 or 👎

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.

3 participants