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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not warn on changing outputs during refresh #16072

Open
t0yv0 opened this issue Apr 26, 2024 · 1 comment
Open

Do not warn on changing outputs during refresh #16072

t0yv0 opened this issue Apr 26, 2024 · 1 comment
Labels
area/engine Pulumi engine kind/enhancement Improvements or new features

Comments

@t0yv0
Copy link
Member

t0yv0 commented Apr 26, 2024

Hello!

  • Vote on this issue by adding a 馃憤 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

There are multiple instances where pulumi refresh shows warnings when resource outputs are changing. Can we consider a model here output changes are not warning worthy?

pulumi/pulumi-aws#3837 lists multiple AWS examples with several distinct root causes all presenting as un-actionable refresh warnings that, if ignored, lead to a good steady-state where pulumi up and subsequent pulumi refresh show no changes:

  • nil to [] output drift that also reproduces in TF but self-corrects through automatic refresh during apply and therefore remains unfixed over time
  • benign output changes like lastModified changes on a Lambda
  • ownership conflicts between inline attributes and sidecar resources (Role managedPolicyArns or RouteTable routes)
  • reordering of Set elements where TF understands Set but Pulumi engine diffing Read results does not

Trying to recover the rationale for warning on output changes, one was provided: it is possible that resource outputs might influence the program flow execution and be reused as inputs to other resources and stack outputs, which may cause a non-empty pulumi preview after the refresh; so in essence Pulumi conservatively attempts to anticipate this and put the user in control of whether they are going to accept the refreshed state or reject it. The anticipation is correct when the changing outputs are indeed used in the program, and incorrect when they are not, as in lastModified output is changing but remains unused.

If that is the rationale for the current behavior, here is an alternative possible implementation for refresh:

  1. save current state S0
  2. call provider Read method to obtain candidate state S1 from the cloud; do not attempt to diff or warn at this point
  3. run pulumi preview to rerun the program against the S1 state
  4. if the preview indicates no changes, accept S1 as the new state and succeed
  5. if the preview indicates changes, inform the user of the diff result as potential drift; the user decides whether to retain S0 or accept S1

Affected area/feature

@t0yv0 t0yv0 added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Apr 26, 2024
@t0yv0
Copy link
Member Author

t0yv0 commented Apr 26, 2024

This is the first draft of my thoughts here but still actively discussing entangled resources with @EronWright and correlating some K8S use cases to explore the design space, especially for entangled resource pairs as this is by far the most challenging bit to cleanly solve for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/engine Pulumi engine kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

2 participants