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

Test CI failure #16224

Closed
wants to merge 8 commits into from
Closed

Test CI failure #16224

wants to merge 8 commits into from

Conversation

justinvp
Copy link
Member

For personal testing

lukehoban and others added 8 commits May 7, 2024 23:00
Makes two changes to the handling of ignoreChanges:
1. Stores the ignoreChanges configuration for each resource in the state file
2. Applies the ignoreChanges configuration from the state file as part of processing a refresh

This allows users to use `ignoreChanges` to prevent refresh diffs being reported when changes happen in the cloud provider that they do not want to bring back into their Pulumi state.

This can be used to acknowledge that part or all of a resource may change in the cloud provider, but the user is okay with this and doesn't want to be notified about it during refresh.

Importantly, this means that the diff won't be reported, but also that the changes won't be applied to state.

The ignoreChanges option when applied to a refresh operation applies to *both* input and output properties of the resource.  This is different from the behavior during a normal update operation, where ignoreChanges only applies to input properties.

The special "*" property path ignores all properties, but also ignores the case where the resource is no longer found in the cloud, and instead keeps the resource with all of it's existing input and output properties.

Because the program is not run for refresh operations, users must apply the `ignoreChanges` via a `pulumi up` first, and then future `pulumi refresh` or `pulumi refresh --preview-only` oeprations will ignore the specificd changes.  This is unfortunate, but the only option we have unless/until Pulumi fundamentally changes it's approach to refresh and decides to run the program as part of computing the resource state (and options) to use during the refresh.
Change refresh steps to compute a diff similar to what would happen if a `preview` were run immediately after the refresh using the inputs from the program, but inverted.

This doesn't change what is stored back into state, but does produce a diff that is more aligned with true "change to desired state".

This fixes several corner cases that are unfortuante in the prior implementation:
1. Changes to output-only proeprties - like `etag`s - are no longer shown as refresh diffs (but are updated in the state outputs).
2. IgnoreChanges now works to ignore property additions or changes (the changes are still updated in state so that future updates don't overwrite them again).

TODO:
- [ ] The diff display isn't updated yet - only the progres row display.
- [ ] Tests
The provider previously triggers replaces on every update, because it replaces on `prefix` but doesn't store it into outputs.  This *also* triggers the new refresh logic to report an update as part of the refresh, but it seems it is indeed just "broken" even outside of refresh, we just weren't testing multiple consecutive updates to these resources previously.
@pulumi-bot
Copy link
Contributor

Changelog

[uncommitted] (2024-05-18)

Features

  • [engine] Change pulumi refresh to report diff relative to desired state instead of relative to only output changes
    #16224

@justinvp justinvp closed this May 20, 2024
@justinvp justinvp deleted the justin/test branch May 20, 2024 13:22
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.

None yet

3 participants