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

Add skipAwait for Helm and YAML SDKs #1421

Closed
lukehoban opened this issue Jan 11, 2021 · 3 comments · Fixed by #1610
Closed

Add skipAwait for Helm and YAML SDKs #1421

lukehoban opened this issue Jan 11, 2021 · 3 comments · Fixed by #1610
Assignees
Labels
area/await-logic kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Milestone

Comments

@lukehoban
Copy link
Member

We should consider a global kubernetes:skipAwait setting to turn off all await logic for this package, providing an experience closer to the default Kubernetes API experience. This has significant downsides (outputs not populated, incorrect deployments not identified, etc.), but for many use cases is the initially "expected" behaviour.

We have support for individual resource-level skipAwait annotations today, but not a global setting.

We could also consider support for skipAwait on YAML and Helm components. It's common to want to not await readiness on Helm charts, and is hard to do currently (requires a transformation to add the skipAwait annotation to every child resource).

@lukehoban lukehoban added the needs-triage Needs attention from the triage team label Jan 11, 2021
@leezen leezen added kind/enhancement Improvements or new features area/await-logic and removed needs-triage Needs attention from the triage team labels Jan 21, 2021
@lblackstone
Copy link
Member

An alternative was proposed where we instead default await logic to warn rather than error on failure. In this case, we'd likely want to have a flag to opt into the current behavior for the CI use case where users may want to error rather than proceeding to any subsequent release steps.

@maxromanovsky
Copy link

Well... lack of skipAwait for Helm chart itself (which IMHO should support it natively in Pulumi as it's kind of first class citizen) prevents from quickly experimenting with charts. And typically charts are thirdparty resources, and while adopting/upgrading them it's crucial to fail quickly, then fix-deploy-repeat

@leezen leezen added this to the 0.56 milestone Apr 28, 2021
@leezen leezen removed this from the 0.56 milestone May 17, 2021
@infin8x infin8x added this to the 0.57 milestone May 18, 2021
@lblackstone
Copy link
Member

lblackstone commented May 27, 2021

An alternative was proposed where we instead default await logic to warn rather than error on failure.

I realized that changing the error to a warning has implications for resource dependencies. If the resource isn't actually ready yet, property values may not have been populated yet when downstream resources access them. Since this could lead to a lot of subtle bugs, I think it's better to provide a flag to selectively disable the await logic.

Actually, I'm not sure a provider-level flag makes sense because of this issue. Skipping await logic on a stack that uses resource outputs to populate other inputs can't be expected to work reliably. Providing that option for Helm and YAML specifically makes more sense because these component resource types are generally expected to be self contained (i.e., the resources should eventually resolve after they are created with no expectation of ordering).

@leezen leezen modified the milestones: 0.57, 0.58 Jun 8, 2021
@lblackstone lblackstone changed the title Provide a global kubernetes:skipAwait setting to turn off all await logic Add skipAwait for Helm and YAML SDKs Jun 8, 2021
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/await-logic kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants