-
Notifications
You must be signed in to change notification settings - Fork 155
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
Panic replacing ec2 instance after user data is updated #2904
Comments
Outcomes from pairing on debuggins with @VenelinMartinov When calling Open questions:
This looks suspliciously related to tags_all cc: @t0yv0 |
Yeah. We have some temporarily placeholders for GetRawPlan instead of the correct impl. Any more context? A repro would be invaluable still. |
We could patch provider.tagsResourceInterceptor to be tolerant of nils but I wonder if there's an opportunity instead to get a little closer to TF behavior with populating RawPlan. |
Here's steps for reproing the issue:
|
Excellent, thank you! |
I can reproduce this as described on my machine. This is very interesting. There is a bridge issue somewhere around simpleDiffViaPlanState that gets activated when computing the Update plan for the Instance here. We try to emulate TF CLI by computing the Plan via:
The plan is non-nil. However it becomes nil after going through the SimpleDiff black box:
This trips up the expectations of the tags interceptor that is definitely expecting non-nil plans. We can quick-fix by adding a patch to the tags interceptor to introduce a nil check, but the above feels like bad bridge behavior. I'd like to step through this in a debugger quick. |
This seems to be a discrepancy between how bridged providers and TF do planning per pulumi/pulumi-terraform-bridge#1505 - a very special case of the above. The bridge calls res.SimpleDiff which is allowed to return In the case of TF reference implementation here: There is code that compensates for an empty diff and propagates RawPlan from PlanResourceChange to ApplyResourceChange, so that subsequently res.Apply has access to it. Therefore the provider safely assumes RawPlan is accessible, but in the Pulumi case it is nil and causes a panic. |
Making a surgical change to avoid nil pointer panics on accessing RawPlan, coming from the AWS panic in pulumi/pulumi-aws#2904 I have verified that this resolves 2904. --------- Co-authored-by: Ian Wahbe <[email protected]>
[Upstream v5.25.0 release notes](https://github.com/hashicorp/terraform-provider-aws/releases/tag/v5.25.0) Fixes #2983, #2904, #2971, #2900 - [x] Rebuild eks.Cluster patches; upstream moved to AWS SDK v2 for Go, patches needed updates as well - [x] Fix pulumi/pulumi-terraform-bridge#1523 in the bridge - [x] Update bridge to include pulumi/pulumi-terraform-bridge#1521 and pulumi/pulumi-terraform-bridge#1520 fixes affecting P1s in pulumi-aws - [x] Build a Pulumi test for EKS Cluster add-on removal -> turns out the property is a no-op, not needed
Fixed in v6.9.0 |
What happened?
I ran pulumi up on a project and got a panic during ec2 instance replacement.
I originally started with the
vm-aws-python
template and was updating the user data of the ec2 instance when I got the panic.Example
I can reliably reproduce the panic in the original project but I wasn't able to reproduce it in a new project.
Error in the output of
pulumi up
:I ran
pulumi stack export
to get the current state of the stack:The code which I am running
pulumi up
on is here.Let me know if you need anything else.
Output of
pulumi about
Additional context
No response
Contributing
Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).
The text was updated successfully, but these errors were encountered: