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

pulumi refresh resulting in state integrity errors #16052

Open
Frassle opened this issue Apr 25, 2024 · 1 comment
Open

pulumi refresh resulting in state integrity errors #16052

Frassle opened this issue Apr 25, 2024 · 1 comment
Assignees
Labels
area/engine Pulumi engine impact/snapshot-integrity Snapshot integrity failure kind/bug Some behavior is incorrect or out of spec p1 Bugs severe enough to be the next item assigned to an engineer
Milestone

Comments

@Frassle
Copy link
Member

Frassle commented Apr 25, 2024

What happened?

Reported by a customer (see internal chat https://pulumi.slack.com/archives/CKJ4C845D/p1713941404085679 for details).

It appears a pulumi refresh run resulted in state being written out that failed integrity checks.

Example

Details on internal chat.

Output of pulumi about

N/A

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).

@Frassle Frassle added kind/bug Some behavior is incorrect or out of spec p1 Bugs severe enough to be the next item assigned to an engineer area/engine Pulumi engine impact/snapshot-integrity Snapshot integrity failure labels Apr 25, 2024
@justinvp justinvp added this to the 0.103 milestone Apr 26, 2024
@Frassle
Copy link
Member Author

Frassle commented Apr 29, 2024

Having looked at this more we think this is a failure in up not refresh.

github-merge-queue bot pushed a commit that referenced this issue May 3, 2024
A resource `A` depends on a resource `B` if:

1. `B` is `A`'s `Provider`.
2. `B` is `A`'s `Parent`.
3. `B` appears in `A`'s `Dependencies`.
4. `B` appears in one or more of `A`'s `PropertyDependencies`.
5. `B` is referenced by `A`'s `DeletedWith` field.

While cases 1, 2, and 3 (providers, parents, and dependencies) are
handled fairly consistently, there have been a number of cases where the
newer features of `PropertyDependencies` (case 4) and `DeletedWith`
(case 5) have been neglected. This commit addresses some of these
omissions. Specifically:

* When refreshing state, it's important that we remove URNs that point
to resources that we've identified as deleted. Presently we check
pointers to parents and dependencies, but not property dependencies or
`deletedWith`. This commit fixes these gaps where dangling URNs could
lurk.

* Linked to the above, this commit extends snapshot integrity checks to
include property dependencies and `deletedWith`. Some tests that
previously used now invalid states have to be repaired or removed as a
  result of this.

* Fixing snapshot integrity checking reveals that dependency graph
checks also fail to consider property dependencies and `deletedWith`.
This probably hasn't bitten us since property dependencies are currently
rolled up into dependencies (for temporary backwards compatibility) and
`deletedWith` is typically an optimisation (moreover, one that only
matters during deletes), so operations being parallelised due a
perceived lack of dependency still succeed. However, tests that
previously passed now fail as we can spot these races with our better
integrity checks. This commit thus fixes up dependency graph
construction and bulks out its test suite to cover the new cases.

These bugs were discovered as part of the investigation into #16052,
though they may not be directly responsible for it (though the issues
with dependency graphs are certainly a candidate).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/engine Pulumi engine impact/snapshot-integrity Snapshot integrity failure kind/bug Some behavior is incorrect or out of spec p1 Bugs severe enough to be the next item assigned to an engineer
Projects
None yet
Development

No branches or pull requests

3 participants