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

[WIP] Add a ignoreRefreshChanges resource option #16015

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

lukehoban
Copy link
Member

Introduce a new ignoreRefreshChanges resource option which is used to indicate changes that should be ignored during refresh.

This allows users to indicate that they accept some level of "drift" between the Pulumi state and the real cloud provider state, and do not want to be told about it or reconcile it back into Pulumi state. This enables some parts of the cloud state to be intentionally managed "outside of Pulumi". It can also be used to work around "bugs" in providers where they are overly noisy in reporting changes from the cloud provider - like a lastViewTime property that changes freuently but doesn't represent true "drift" that the user wants to pay attention to.

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

The ignoreRefreshChanges option applies to both input and output properties of a resource. This is different from the behavior of ignoreChanges which only treats 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. This seems reasonable as a "special case" because users must in practice ignore all properties to ignore the "discard", since all properties have changes (are no longer present).

Because this resource option is used during refresh, but the program is not evaluated during refresh, the ignoreRefreshChanges resource option must be applied and then pulumi upd before the pulumi refresh can consider it. This is awkward in some cases where a refresh might need to happen before a pulumi up. So this PR also introduces a pulumi state ignore-refresh-changes <urn> --path <path> CLI command to directly set this option on a resource in the state (without doing a pulumi up).

Logically - the difference between the two resource options is:

  • ignoreChanges: Ignores cases where the program is different than the state for operations that are attempting to modify state based on what is in a Pulumi program.
  • ignoreRefreshChanges: Ignores cases where the cloud provider is different than the state for operations that are attepting to modify state based on what is in the cloud.

Questions:

  1. Is ignoreRefreshChanges the right name? Or ignoreDrift?
  2. Do we need a separate resource option, or should we just use ignoreChanges to also apply to refresh changes? This PR implements a separate resource option based on feedback that there could be use cases where you only want one or the other, but it is slighlty hard to come up with real-world examples, especially because ignoring refresh changes means it isn't necessarily "safe" to also be making changes via Pulumi (since those will inevitably silently conflict and show incorrect diffs relative to the actual diff that will apply in the cloud).
  3. Are there other "Read" scenarios that should consider ignoreRefreshChanges? Are any of the places that currently listen to ignoreChanges for Read operations actually places that logically should have been using this new/expanded resource option?

Still TODO:

  • Tests
  • YAML, .NET, Java
  • ProgramGen

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.
Introduce a new ignoreRefreshChanges resource option which is used to indicate changes that should be ignored during refresh.  Use this instead of interpreting ignoreChanges as applying to both update and refresh operations.
Ensures users can apply this to state without needing to do a `pulumi up`.
@lukehoban lukehoban force-pushed the lukehoban/ignoreRefreshChanges branch from a07ce96 to 8de2a41 Compare April 22, 2024 05:11
@pulumi-bot
Copy link
Contributor

pulumi-bot commented Apr 22, 2024

Changelog

[uncommitted] (2024-04-22)

Features

  • [engine] Add a new ignoreRefreshChanges resource option
    #16015

@t0yv0
Copy link
Member

t0yv0 commented Apr 25, 2024

We really need this for pulumi-aws pulumi/pulumi-aws#2246 and possibly more.

Couple of other thoughts:

  • looks like it does not apply to import, which I think is good; import should just have the user edit the program as necessary

  • the provider protocol overloads ignoreChanges for Create/Diff/Update but not for Read used for refresh; this is going to be continue to be confusing and unusable for Set-typed paths in bridged providers just like ignoreChanges is IgnoreChanges unable to ignore set element changes pulumi-terraform-bridge#1756 with the additional work required to fix - not only the bridge needs to solve it for sets, but we also need to extend the protocol so that bridge is allowed to interpret this option for refresh and take control

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