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

Write Succeeded condition only after adding droplet info into BuildWorkload status #3497

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

pbusko
Copy link
Contributor

@pbusko pbusko commented Sep 30, 2024

Is there a related GitHub Issue?

no

What is this change about?

If an error is thrown during the generation of droplet status, the BuildWorkload object ends up in a deadlock situation, where each subsequent reconciliation loop exits early and no droplet info is written.

Does this PR introduce a breaking change?

no

Acceptance Steps

Tag your pair, your PM, and/or team

@danail-branekov
Copy link
Member

danail-branekov commented Oct 2, 2024

Hey @pbusko

Thanks for the PR. Would you be willing to also backfill a test that verifies the fix? Looks good otherwise

@pbusko pbusko force-pushed the fix-broken-state-kpack branch 2 times, most recently from f6d91e5 to d00d15e Compare October 4, 2024 08:49
@pbusko
Copy link
Contributor Author

pbusko commented Oct 4, 2024

Hey @pbusko

Thanks for the PR. Would you be willing to also backfill a test that verifies the fix? Looks good otherwise

a test has been added

Copy link
Member

@danail-branekov danail-branekov left a comment

Choose a reason for hiding this comment

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

Minor comments on the test, looks good otherwise

@pbusko
Copy link
Contributor Author

pbusko commented Oct 9, 2024

Minor comments on the test, looks good otherwise

4ea98eb

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