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

Do not await during .get or import operations #2373

Merged
merged 2 commits into from
Apr 28, 2023

Conversation

lblackstone
Copy link
Member

@lblackstone lblackstone commented Apr 28, 2023

Proposed changes

Changes to the provider logic invalidated a check in the await Read method that was supposed to return the cluster state without await checks in the case of a .get or import. As a result, await logic was always executed against resources during a Read operation. This could lead to deadlocks and timeouts in cases where the await logic causes a cyclic dependency.

This change fixes the check so that the provider does not perform await logic on resources during a .get or import operation.

Related issues (optional)

Fix #2372
Related #1656

Changes to the provider logic invalidated a check in the await Read method that was supposed to return the cluster state without await checks in the case of a .get or import. As a result, await logic was always executed against resources during a Read operation. This could lead to deadlocks and timeouts in cases where the await logic causes a cyclic dependency.

This change fixes the check so that the provider does not perform await logic on resources during a .get or import operation.
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Copy link
Contributor

@rquitales rquitales left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for updating the test case. I've also confirmed that the tests fail without the code change.

@lblackstone lblackstone merged commit 13189ac into master Apr 28, 2023
17 checks passed
@lblackstone lblackstone deleted the lblackstone/read-await branch April 28, 2023 23:41
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.

daemonset that relies on kube-dns fails because managednodegroup failed to create
3 participants