pulumi/pkg/resource/graph/dependency_graph.go

367 lines
13 KiB
Go
Raw Normal View History

// Copyright 2016-2021, Pulumi Corporation. All rights reserved.
package graph
import (
mapset "github.com/deckarep/golang-set/v2"
"github.com/pulumi/pulumi/pkg/v3/resource/deploy/providers"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
)
// DependencyGraph represents a dependency graph encoded within a resource snapshot.
type DependencyGraph struct {
index map[*resource.State]int // A mapping of resource pointers to indexes within the snapshot
resources []*resource.State // The list of resources, obtained from the snapshot
childrenOf map[resource.URN][]int // Pre-computed map of transitive children for each resource
}
// DependingOn returns a slice containing all resources that directly or indirectly
// depend upon the given resource. The returned slice is guaranteed to be in topological
// order with respect to the snapshot dependency graph.
//
// The time complexity of DependingOn is linear with respect to the number of resources.
//
// includeChildren adds children as another type of (transitive) dependency.
func (dg *DependencyGraph) DependingOn(res *resource.State,
all: Reformat with gofumpt Per team discussion, switching to gofumpt. [gofumpt][1] is an alternative, stricter alternative to gofmt. It addresses other stylistic concerns that gofmt doesn't yet cover. [1]: https://github.com/mvdan/gofumpt See the full list of [Added rules][2], but it includes: - Dropping empty lines around function bodies - Dropping unnecessary variable grouping when there's only one variable - Ensuring an empty line between multi-line functions - simplification (`-s` in gofmt) is always enabled - Ensuring multi-line function signatures end with `) {` on a separate line. [2]: https://github.com/mvdan/gofumpt#Added-rules gofumpt is stricter, but there's no lock-in. All gofumpt output is valid gofmt output, so if we decide we don't like it, it's easy to switch back without any code changes. gofumpt support is built into the tooling we use for development so this won't change development workflows. - golangci-lint includes a gofumpt check (enabled in this PR) - gopls, the LSP for Go, includes a gofumpt option (see [installation instrutions][3]) [3]: https://github.com/mvdan/gofumpt#installation This change was generated by running: ```bash gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error) ``` The following files were manually tweaked afterwards: - pkg/cmd/pulumi/stack_change_secrets_provider.go: one of the lines overflowed and had comments in an inconvenient place - pkg/cmd/pulumi/destroy.go: `var x T = y` where `T` wasn't necessary - pkg/cmd/pulumi/policy_new.go: long line because of error message - pkg/backend/snapshot_test.go: long line trying to assign three variables in the same assignment I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 16:36:39 +00:00
ignore map[resource.URN]bool, includeChildren bool,
) []*resource.State {
// This implementation relies on the detail that snapshots are stored in a valid
// topological order.
var dependents []*resource.State
dependentSet := make(map[resource.URN]bool)
cursorIndex, ok := dg.index[res]
contract.Assertf(ok, "could not determine index for resource %s", res.URN)
dependentSet[res.URN] = true
Implement first-class providers. (#1695) ### First-Class Providers These changes implement support for first-class providers. First-class providers are provider plugins that are exposed as resources via the Pulumi programming model so that they may be explicitly and multiply instantiated. Each instance of a provider resource may be configured differently, and configuration parameters may be source from the outputs of other resources. ### Provider Plugin Changes In order to accommodate the need to verify and diff provider configuration and configure providers without complete configuration information, these changes adjust the high-level provider plugin interface. Two new methods for validating a provider's configuration and diffing changes to the same have been added (`CheckConfig` and `DiffConfig`, respectively), and the type of the configuration bag accepted by `Configure` has been changed to a `PropertyMap`. These changes have not yet been reflected in the provider plugin gRPC interface. We will do this in a set of follow-up changes. Until then, these methods are implemented by adapters: - `CheckConfig` validates that all configuration parameters are string or unknown properties. This is necessary because existing plugins only accept string-typed configuration values. - `DiffConfig` either returns "never replace" if all configuration values are known or "must replace" if any configuration value is unknown. The justification for this behavior is given [here](https://github.com/pulumi/pulumi/pull/1695/files#diff-a6cd5c7f337665f5bb22e92ca5f07537R106) - `Configure` converts the config bag to a legacy config map and configures the provider plugin if all config values are known. If any config value is unknown, the underlying plugin is not configured and the provider may only perform `Check`, `Read`, and `Invoke`, all of which return empty results. We justify this behavior becuase it is only possible during a preview and provides the best experience we can manage with the existing gRPC interface. ### Resource Model Changes Providers are now exposed as resources that participate in a stack's dependency graph. Like other resources, they are explicitly created, may have multiple instances, and may have dependencies on other resources. Providers are referred to using provider references, which are a combination of the provider's URN and its ID. This design addresses the need during a preview to refer to providers that have not yet been physically created and therefore have no ID. All custom resources that are not themselves providers must specify a single provider via a provider reference. The named provider will be used to manage that resource's CRUD operations. If a resource's provider reference changes, the resource must be replaced. Though its URN is not present in the resource's dependency list, the provider should be treated as a dependency of the resource when topologically sorting the dependency graph. Finally, `Invoke` operations must now specify a provider to use for the invocation via a provider reference. ### Engine Changes First-class providers support requires a few changes to the engine: - The engine must have some way to map from provider references to provider plugins. It must be possible to add providers from a stack's checkpoint to this map and to register new/updated providers during the execution of a plan in response to CRUD operations on provider resources. - In order to support updating existing stacks using existing Pulumi programs that may not explicitly instantiate providers, the engine must be able to manage the "default" providers for each package referenced by a checkpoint or Pulumi program. The configuration for a "default" provider is taken from the stack's configuration data. The former need is addressed by adding a provider registry type that is responsible for managing all of the plugins required by a plan. In addition to loading plugins froma checkpoint and providing the ability to map from a provider reference to a provider plugin, this type serves as the provider plugin for providers themselves (i.e. it is the "provider provider"). The latter need is solved via two relatively self-contained changes to plan setup and the eval source. During plan setup, the old checkpoint is scanned for custom resources that do not have a provider reference in order to compute the set of packages that require a default provider. Once this set has been computed, the required default provider definitions are conjured and prepended to the checkpoint's resource list. Each resource that requires a default provider is then updated to refer to the default provider for its package. While an eval source is running, each custom resource registration, resource read, and invoke that does not name a provider is trapped before being returned by the source iterator. If no default provider for the appropriate package has been registered, the eval source synthesizes an appropriate registration, waits for it to complete, and records the registered provider's reference. This reference is injected into the original request, which is then processed as usual. If a default provider was already registered, the recorded reference is used and no new registration occurs. ### SDK Changes These changes only expose first-class providers from the Node.JS SDK. - A new abstract class, `ProviderResource`, can be subclassed and used to instantiate first-class providers. - A new field in `ResourceOptions`, `provider`, can be used to supply a particular provider instance to manage a `CustomResource`'s CRUD operations. - A new type, `InvokeOptions`, can be used to specify options that control the behavior of a call to `pulumi.runtime.invoke`. This type includes a `provider` field that is analogous to `ResourceOptions.provider`.
2018-08-07 00:50:29 +00:00
isDependent := func(candidate *resource.State) bool {
Fix a dependency graph bug during DBR. (#3329) The dependency graph used to determine the set of resources that depend on a resource being DBR'd is constructured from the list of resource states present in the old snapshot. However, the dependencies of resources that are present in both the old snapshot and the current plan can be different, which in turn can cause the engine to make incorrect decisions during DBR with respect to which resources need to be replaced. For example, consider the following program: ``` var resA = new Resource("a", {dbr: "foo"}); var resB = new Resource("b", {dbr: resA.prop}); ``` If this program is then changed to: ``` var resB = new Resource("b", {dbr: "<literal value of resA.prop>"}); var resA = new Resource("a", {dbr: "bar"}); ``` The engine will first decide to make no changes to "b", as its input property values have not changed. "b" has changed, however, such that it no longer has a dependency on "a". The engine will then decide to DBR "a". In the process, it will determine that it first needs to delete "b", because the state for "b" that is used when calculating "a"'s dependents does not reflect the changes made during the plan. To fix this issue, we rely on the observation that dependents can only have been _removed_ from the base dependency graph: for a dependent to have been added, it would have had to have been registered prior to the root--a resource it depends on--which is not a valid operation. This means that any resources that depend on the root must not yet have been registered, which in turn implies that resources that have already been registered must not depend on the root. Thus, we ignore these resources if they are encountered while walking the old dependency graph to determine the set of dependents.
2019-10-13 00:22:13 +00:00
if ignore[candidate.URN] {
return false
}
if includeChildren && dependentSet[candidate.Parent] {
return true
}
for _, dependency := range candidate.Dependencies {
if dependentSet[dependency] {
return true
}
}
Better handle property dependencies and `deletedWith` (#16088) 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).
2024-05-03 17:08:06 +00:00
for _, deps := range candidate.PropertyDependencies {
for _, dep := range deps {
if dependentSet[dep] {
return true
}
}
}
if candidate.DeletedWith != "" && dependentSet[candidate.DeletedWith] {
return true
}
Implement first-class providers. (#1695) ### First-Class Providers These changes implement support for first-class providers. First-class providers are provider plugins that are exposed as resources via the Pulumi programming model so that they may be explicitly and multiply instantiated. Each instance of a provider resource may be configured differently, and configuration parameters may be source from the outputs of other resources. ### Provider Plugin Changes In order to accommodate the need to verify and diff provider configuration and configure providers without complete configuration information, these changes adjust the high-level provider plugin interface. Two new methods for validating a provider's configuration and diffing changes to the same have been added (`CheckConfig` and `DiffConfig`, respectively), and the type of the configuration bag accepted by `Configure` has been changed to a `PropertyMap`. These changes have not yet been reflected in the provider plugin gRPC interface. We will do this in a set of follow-up changes. Until then, these methods are implemented by adapters: - `CheckConfig` validates that all configuration parameters are string or unknown properties. This is necessary because existing plugins only accept string-typed configuration values. - `DiffConfig` either returns "never replace" if all configuration values are known or "must replace" if any configuration value is unknown. The justification for this behavior is given [here](https://github.com/pulumi/pulumi/pull/1695/files#diff-a6cd5c7f337665f5bb22e92ca5f07537R106) - `Configure` converts the config bag to a legacy config map and configures the provider plugin if all config values are known. If any config value is unknown, the underlying plugin is not configured and the provider may only perform `Check`, `Read`, and `Invoke`, all of which return empty results. We justify this behavior becuase it is only possible during a preview and provides the best experience we can manage with the existing gRPC interface. ### Resource Model Changes Providers are now exposed as resources that participate in a stack's dependency graph. Like other resources, they are explicitly created, may have multiple instances, and may have dependencies on other resources. Providers are referred to using provider references, which are a combination of the provider's URN and its ID. This design addresses the need during a preview to refer to providers that have not yet been physically created and therefore have no ID. All custom resources that are not themselves providers must specify a single provider via a provider reference. The named provider will be used to manage that resource's CRUD operations. If a resource's provider reference changes, the resource must be replaced. Though its URN is not present in the resource's dependency list, the provider should be treated as a dependency of the resource when topologically sorting the dependency graph. Finally, `Invoke` operations must now specify a provider to use for the invocation via a provider reference. ### Engine Changes First-class providers support requires a few changes to the engine: - The engine must have some way to map from provider references to provider plugins. It must be possible to add providers from a stack's checkpoint to this map and to register new/updated providers during the execution of a plan in response to CRUD operations on provider resources. - In order to support updating existing stacks using existing Pulumi programs that may not explicitly instantiate providers, the engine must be able to manage the "default" providers for each package referenced by a checkpoint or Pulumi program. The configuration for a "default" provider is taken from the stack's configuration data. The former need is addressed by adding a provider registry type that is responsible for managing all of the plugins required by a plan. In addition to loading plugins froma checkpoint and providing the ability to map from a provider reference to a provider plugin, this type serves as the provider plugin for providers themselves (i.e. it is the "provider provider"). The latter need is solved via two relatively self-contained changes to plan setup and the eval source. During plan setup, the old checkpoint is scanned for custom resources that do not have a provider reference in order to compute the set of packages that require a default provider. Once this set has been computed, the required default provider definitions are conjured and prepended to the checkpoint's resource list. Each resource that requires a default provider is then updated to refer to the default provider for its package. While an eval source is running, each custom resource registration, resource read, and invoke that does not name a provider is trapped before being returned by the source iterator. If no default provider for the appropriate package has been registered, the eval source synthesizes an appropriate registration, waits for it to complete, and records the registered provider's reference. This reference is injected into the original request, which is then processed as usual. If a default provider was already registered, the recorded reference is used and no new registration occurs. ### SDK Changes These changes only expose first-class providers from the Node.JS SDK. - A new abstract class, `ProviderResource`, can be subclassed and used to instantiate first-class providers. - A new field in `ResourceOptions`, `provider`, can be used to supply a particular provider instance to manage a `CustomResource`'s CRUD operations. - A new type, `InvokeOptions`, can be used to specify options that control the behavior of a call to `pulumi.runtime.invoke`. This type includes a `provider` field that is analogous to `ResourceOptions.provider`.
2018-08-07 00:50:29 +00:00
if candidate.Provider != "" {
ref, err := providers.ParseReference(candidate.Provider)
contract.AssertNoErrorf(err, "cannot parse provider reference %q", candidate.Provider)
Implement first-class providers. (#1695) ### First-Class Providers These changes implement support for first-class providers. First-class providers are provider plugins that are exposed as resources via the Pulumi programming model so that they may be explicitly and multiply instantiated. Each instance of a provider resource may be configured differently, and configuration parameters may be source from the outputs of other resources. ### Provider Plugin Changes In order to accommodate the need to verify and diff provider configuration and configure providers without complete configuration information, these changes adjust the high-level provider plugin interface. Two new methods for validating a provider's configuration and diffing changes to the same have been added (`CheckConfig` and `DiffConfig`, respectively), and the type of the configuration bag accepted by `Configure` has been changed to a `PropertyMap`. These changes have not yet been reflected in the provider plugin gRPC interface. We will do this in a set of follow-up changes. Until then, these methods are implemented by adapters: - `CheckConfig` validates that all configuration parameters are string or unknown properties. This is necessary because existing plugins only accept string-typed configuration values. - `DiffConfig` either returns "never replace" if all configuration values are known or "must replace" if any configuration value is unknown. The justification for this behavior is given [here](https://github.com/pulumi/pulumi/pull/1695/files#diff-a6cd5c7f337665f5bb22e92ca5f07537R106) - `Configure` converts the config bag to a legacy config map and configures the provider plugin if all config values are known. If any config value is unknown, the underlying plugin is not configured and the provider may only perform `Check`, `Read`, and `Invoke`, all of which return empty results. We justify this behavior becuase it is only possible during a preview and provides the best experience we can manage with the existing gRPC interface. ### Resource Model Changes Providers are now exposed as resources that participate in a stack's dependency graph. Like other resources, they are explicitly created, may have multiple instances, and may have dependencies on other resources. Providers are referred to using provider references, which are a combination of the provider's URN and its ID. This design addresses the need during a preview to refer to providers that have not yet been physically created and therefore have no ID. All custom resources that are not themselves providers must specify a single provider via a provider reference. The named provider will be used to manage that resource's CRUD operations. If a resource's provider reference changes, the resource must be replaced. Though its URN is not present in the resource's dependency list, the provider should be treated as a dependency of the resource when topologically sorting the dependency graph. Finally, `Invoke` operations must now specify a provider to use for the invocation via a provider reference. ### Engine Changes First-class providers support requires a few changes to the engine: - The engine must have some way to map from provider references to provider plugins. It must be possible to add providers from a stack's checkpoint to this map and to register new/updated providers during the execution of a plan in response to CRUD operations on provider resources. - In order to support updating existing stacks using existing Pulumi programs that may not explicitly instantiate providers, the engine must be able to manage the "default" providers for each package referenced by a checkpoint or Pulumi program. The configuration for a "default" provider is taken from the stack's configuration data. The former need is addressed by adding a provider registry type that is responsible for managing all of the plugins required by a plan. In addition to loading plugins froma checkpoint and providing the ability to map from a provider reference to a provider plugin, this type serves as the provider plugin for providers themselves (i.e. it is the "provider provider"). The latter need is solved via two relatively self-contained changes to plan setup and the eval source. During plan setup, the old checkpoint is scanned for custom resources that do not have a provider reference in order to compute the set of packages that require a default provider. Once this set has been computed, the required default provider definitions are conjured and prepended to the checkpoint's resource list. Each resource that requires a default provider is then updated to refer to the default provider for its package. While an eval source is running, each custom resource registration, resource read, and invoke that does not name a provider is trapped before being returned by the source iterator. If no default provider for the appropriate package has been registered, the eval source synthesizes an appropriate registration, waits for it to complete, and records the registered provider's reference. This reference is injected into the original request, which is then processed as usual. If a default provider was already registered, the recorded reference is used and no new registration occurs. ### SDK Changes These changes only expose first-class providers from the Node.JS SDK. - A new abstract class, `ProviderResource`, can be subclassed and used to instantiate first-class providers. - A new field in `ResourceOptions`, `provider`, can be used to supply a particular provider instance to manage a `CustomResource`'s CRUD operations. - A new type, `InvokeOptions`, can be used to specify options that control the behavior of a call to `pulumi.runtime.invoke`. This type includes a `provider` field that is analogous to `ResourceOptions.provider`.
2018-08-07 00:50:29 +00:00
if dependentSet[ref.URN()] {
return true
}
}
return false
}
// The dependency graph encoded directly within the snapshot is the reverse of
// the graph that we actually want to operate upon. Edges in the snapshot graph
// originate in a resource and go to that resource's dependencies.
//
// The `DependingOn` is simpler when operating on the reverse of the snapshot graph,
// where edges originate in a resource and go to resources that depend on that resource.
// In this graph, `DependingOn` for a resource is the set of resources that are reachable from the
// given resource.
//
// To accomplish this without building up an entire graph data structure, we'll do a linear
// scan of the resource list starting at the requested resource and ending at the end of
// the list. All resources that depend directly or indirectly on `res` are prepended
// onto `dependents`.
for i := cursorIndex + 1; i < len(dg.resources); i++ {
Implement first-class providers. (#1695) ### First-Class Providers These changes implement support for first-class providers. First-class providers are provider plugins that are exposed as resources via the Pulumi programming model so that they may be explicitly and multiply instantiated. Each instance of a provider resource may be configured differently, and configuration parameters may be source from the outputs of other resources. ### Provider Plugin Changes In order to accommodate the need to verify and diff provider configuration and configure providers without complete configuration information, these changes adjust the high-level provider plugin interface. Two new methods for validating a provider's configuration and diffing changes to the same have been added (`CheckConfig` and `DiffConfig`, respectively), and the type of the configuration bag accepted by `Configure` has been changed to a `PropertyMap`. These changes have not yet been reflected in the provider plugin gRPC interface. We will do this in a set of follow-up changes. Until then, these methods are implemented by adapters: - `CheckConfig` validates that all configuration parameters are string or unknown properties. This is necessary because existing plugins only accept string-typed configuration values. - `DiffConfig` either returns "never replace" if all configuration values are known or "must replace" if any configuration value is unknown. The justification for this behavior is given [here](https://github.com/pulumi/pulumi/pull/1695/files#diff-a6cd5c7f337665f5bb22e92ca5f07537R106) - `Configure` converts the config bag to a legacy config map and configures the provider plugin if all config values are known. If any config value is unknown, the underlying plugin is not configured and the provider may only perform `Check`, `Read`, and `Invoke`, all of which return empty results. We justify this behavior becuase it is only possible during a preview and provides the best experience we can manage with the existing gRPC interface. ### Resource Model Changes Providers are now exposed as resources that participate in a stack's dependency graph. Like other resources, they are explicitly created, may have multiple instances, and may have dependencies on other resources. Providers are referred to using provider references, which are a combination of the provider's URN and its ID. This design addresses the need during a preview to refer to providers that have not yet been physically created and therefore have no ID. All custom resources that are not themselves providers must specify a single provider via a provider reference. The named provider will be used to manage that resource's CRUD operations. If a resource's provider reference changes, the resource must be replaced. Though its URN is not present in the resource's dependency list, the provider should be treated as a dependency of the resource when topologically sorting the dependency graph. Finally, `Invoke` operations must now specify a provider to use for the invocation via a provider reference. ### Engine Changes First-class providers support requires a few changes to the engine: - The engine must have some way to map from provider references to provider plugins. It must be possible to add providers from a stack's checkpoint to this map and to register new/updated providers during the execution of a plan in response to CRUD operations on provider resources. - In order to support updating existing stacks using existing Pulumi programs that may not explicitly instantiate providers, the engine must be able to manage the "default" providers for each package referenced by a checkpoint or Pulumi program. The configuration for a "default" provider is taken from the stack's configuration data. The former need is addressed by adding a provider registry type that is responsible for managing all of the plugins required by a plan. In addition to loading plugins froma checkpoint and providing the ability to map from a provider reference to a provider plugin, this type serves as the provider plugin for providers themselves (i.e. it is the "provider provider"). The latter need is solved via two relatively self-contained changes to plan setup and the eval source. During plan setup, the old checkpoint is scanned for custom resources that do not have a provider reference in order to compute the set of packages that require a default provider. Once this set has been computed, the required default provider definitions are conjured and prepended to the checkpoint's resource list. Each resource that requires a default provider is then updated to refer to the default provider for its package. While an eval source is running, each custom resource registration, resource read, and invoke that does not name a provider is trapped before being returned by the source iterator. If no default provider for the appropriate package has been registered, the eval source synthesizes an appropriate registration, waits for it to complete, and records the registered provider's reference. This reference is injected into the original request, which is then processed as usual. If a default provider was already registered, the recorded reference is used and no new registration occurs. ### SDK Changes These changes only expose first-class providers from the Node.JS SDK. - A new abstract class, `ProviderResource`, can be subclassed and used to instantiate first-class providers. - A new field in `ResourceOptions`, `provider`, can be used to supply a particular provider instance to manage a `CustomResource`'s CRUD operations. - A new type, `InvokeOptions`, can be used to specify options that control the behavior of a call to `pulumi.runtime.invoke`. This type includes a `provider` field that is analogous to `ResourceOptions.provider`.
2018-08-07 00:50:29 +00:00
candidate := dg.resources[i]
if isDependent(candidate) {
dependents = append(dependents, candidate)
dependentSet[candidate.URN] = true
}
}
return dependents
}
State: fix panic when deleting non-unique Provider (#15322) # Description In https://github.com/pulumi/pulumi/commit/30f59eb30a9b519766aa0e3d3e463a2b4667466b, we made sure that we don't delete multiple resources, when a user is prompted to choose between multiple resources with the same URN. If there are non-unique URNs, we currently assume that we can just go ahead and delete it, as there's still another resource that will fulfill the requirements. However that is not true if the dependent is a provider. Providers are identified by a {URN, ID} tuple. Therefore even though after the deletion of a provider with a specific URN there's still another one with the same URN left, this is not good enough to fulfill the dependency requirements. If we don't have a unique URN, check again to make sure there's no provider dependency on the condemned resource. If there is one, we either error out, or delete the dependencies depending on the options passed in. Note that I haven't managed to reproduce this by just issuing pulumi commands, but I did by editing the state in a way to reproduce the same issue. One unanswered question for me is how the deleted cluster in the original issue ended up lingering in the state. While this PR fixes the panic, and will make it easier for the user to actually delete the cluster from the state (it's going to suggest `--target-dependents` which will do the right thing), it doesn't address that bit. Fixes https://github.com/pulumi/pulumi/issues/15166 ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. --> --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-02-05 16:21:38 +00:00
// OnlyDependsOn returns a slice containing all resources that directly or indirectly
// depend upon *only* the given resource. Resources that also depend on another resource with
// the same URN will not be included in the returned slice. The returned slice is guaranteed
// to be in topological order with respect to the snapshot dependency graph.
//
// The time complexity of OnlyDependsOn is linear with respect to the number of resources.
func (dg *DependencyGraph) OnlyDependsOn(res *resource.State) []*resource.State {
// This implementation relies on the detail that snapshots are stored in a valid
// topological order.
var dependents []*resource.State
dependentSet := make(map[resource.URN][]resource.ID)
nonDependentSet := make(map[resource.URN][]resource.ID)
cursorIndex, ok := dg.index[res]
contract.Assertf(ok, "could not determine index for resource %s", res.URN)
dependentSet[res.URN] = []resource.ID{res.ID}
isDependent := func(candidate *resource.State) bool {
if res.URN == candidate.URN && res.ID == candidate.ID {
return false
}
if len(dependentSet[candidate.Parent]) > 0 && len(nonDependentSet[candidate.Parent]) == 0 {
return true
}
for _, dependency := range candidate.Dependencies {
if len(dependentSet[dependency]) == 1 && len(nonDependentSet[dependency]) == 0 {
return true
}
}
Better handle property dependencies and `deletedWith` (#16088) 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).
2024-05-03 17:08:06 +00:00
for _, deps := range candidate.PropertyDependencies {
for _, dep := range deps {
if len(dependentSet[dep]) == 1 && len(nonDependentSet[dep]) == 0 {
return true
}
}
}
if candidate.DeletedWith != "" {
if len(dependentSet[candidate.DeletedWith]) == 1 && len(nonDependentSet[candidate.DeletedWith]) == 0 {
return true
}
}
State: fix panic when deleting non-unique Provider (#15322) # Description In https://github.com/pulumi/pulumi/commit/30f59eb30a9b519766aa0e3d3e463a2b4667466b, we made sure that we don't delete multiple resources, when a user is prompted to choose between multiple resources with the same URN. If there are non-unique URNs, we currently assume that we can just go ahead and delete it, as there's still another resource that will fulfill the requirements. However that is not true if the dependent is a provider. Providers are identified by a {URN, ID} tuple. Therefore even though after the deletion of a provider with a specific URN there's still another one with the same URN left, this is not good enough to fulfill the dependency requirements. If we don't have a unique URN, check again to make sure there's no provider dependency on the condemned resource. If there is one, we either error out, or delete the dependencies depending on the options passed in. Note that I haven't managed to reproduce this by just issuing pulumi commands, but I did by editing the state in a way to reproduce the same issue. One unanswered question for me is how the deleted cluster in the original issue ended up lingering in the state. While this PR fixes the panic, and will make it easier for the user to actually delete the cluster from the state (it's going to suggest `--target-dependents` which will do the right thing), it doesn't address that bit. Fixes https://github.com/pulumi/pulumi/issues/15166 ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. --> --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-02-05 16:21:38 +00:00
if candidate.Provider != "" {
ref, err := providers.ParseReference(candidate.Provider)
contract.AssertNoErrorf(err, "cannot parse provider reference %q", candidate.Provider)
for _, id := range dependentSet[ref.URN()] {
if id == ref.ID() {
return true
}
}
}
return false
}
// The dependency graph encoded directly within the snapshot is the reverse of
// the graph that we actually want to operate upon. Edges in the snapshot graph
// originate in a resource and go to that resource's dependencies.
//
// The `OnlyDependsOn` is simpler when operating on the reverse of the snapshot graph,
// where edges originate in a resource and go to resources that depend on that resource.
// In this graph, `OnlyDependsOn` for a resource is the set of resources that are reachable from the
// given resource, and only from the given resource.
//
// To accomplish this without building up an entire graph data structure, we'll do a linear
// scan of the resource list starting at the requested resource and ending at the end of
// the list. All resources that depend directly or indirectly on `res` are prepended
// onto `dependents`.
//
// We also walk through the the list of resources before the requested resource, as resources
// sorted later could still be dependent on the requested resource.
for i := 0; i < cursorIndex; i++ {
candidate := dg.resources[i]
nonDependentSet[candidate.URN] = append(nonDependentSet[candidate.URN], candidate.ID)
}
for i := cursorIndex + 1; i < len(dg.resources); i++ {
candidate := dg.resources[i]
if isDependent(candidate) {
dependents = append(dependents, candidate)
dependentSet[candidate.URN] = append(dependentSet[candidate.URN], candidate.ID)
} else {
nonDependentSet[candidate.URN] = append(nonDependentSet[candidate.URN], candidate.ID)
}
}
return dependents
}
Better handle property dependencies and `deletedWith` (#16088) 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).
2024-05-03 17:08:06 +00:00
// DependenciesOf returns a set of resources upon which the given resource
// depends directly. This includes the resource's provider, parent, any
// resources in the `Dependencies` list, any resources in the
// `PropertyDependencies` map, and any resource referenced by the `DeletedWith`
// field.
func (dg *DependencyGraph) DependenciesOf(res *resource.State) mapset.Set[*resource.State] {
set := mapset.NewSet[*resource.State]()
dependentUrns := make(map[resource.URN]bool)
for _, dep := range res.Dependencies {
dependentUrns[dep] = true
}
Better handle property dependencies and `deletedWith` (#16088) 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).
2024-05-03 17:08:06 +00:00
for _, deps := range res.PropertyDependencies {
for _, dep := range deps {
dependentUrns[dep] = true
}
}
if res.DeletedWith != "" {
dependentUrns[res.DeletedWith] = true
}
if res.Provider != "" {
ref, err := providers.ParseReference(res.Provider)
contract.AssertNoErrorf(err, "cannot parse provider reference %q", res.Provider)
dependentUrns[ref.URN()] = true
}
cursorIndex, ok := dg.index[res]
contract.Assertf(ok, "could not determine index for resource %s", res.URN)
for i := cursorIndex - 1; i >= 0; i-- {
candidate := dg.resources[i]
// Include all resources that are dependencies of the resource
if dependentUrns[candidate.URN] {
set.Add(candidate)
// If the dependency is a component, all transitive children of the dependency that are before this
// resource in the topological sort are also implicitly dependencies. This is necessary because for remote
// components, the dependencies will not include the transitive set of children directly, but will include
// the parent component. We must walk that component's children here to ensure they are treated as
// dependencies. Transitive children of the dependency that are after the resource in the topological sort
// are not included as this could lead to cycles in the dependency order.
if !candidate.Custom {
for _, transitiveCandidateIndex := range dg.childrenOf[candidate.URN] {
if transitiveCandidateIndex < cursorIndex {
set.Add(dg.resources[transitiveCandidateIndex])
}
}
}
}
// Include the resource's parent, as the resource depends on it's parent existing.
if candidate.URN == res.Parent {
set.Add(candidate)
}
}
return set
}
fix panic with --continue-on-error on delete after failed create (#16261) For deleting resources when `--continue-on-error` is given, we look up a failed resources transitive dependencies in the dependency graph. This is used to skip those resources from being destroyed to keep the dependency chains correctly intact. The errors for looking these dependencies up are recorded in the step executor, whenever a resource execution fails. This includes errors from resources that are supposed to be created. However when a resource fails to create that resource does not exist in the dependency graph at all, so we cannot look it up, and we get a panic because we try to access a nil pointer. When deleting resources we do not need to care about resources that failed earlier, so we can just clear the errored steps before going ahead with the deletes. Fixes https://github.com/pulumi/pulumi/issues/16258 ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2024-05-30 17:18:34 +00:00
// Contains returns whether the given resource is in the dependency graph.
func (dg *DependencyGraph) Contains(res *resource.State) bool {
_, ok := dg.index[res]
return ok
}
Better handle property dependencies and `deletedWith` (#16088) 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).
2024-05-03 17:08:06 +00:00
// `TransitiveDependenciesOf` calculates the set of resources upon which the
// given resource depends, directly or indirectly. This includes the resource's
// provider, parent, any resources in the `Dependencies` list, any resources in
// the `PropertyDependencies` map, and any resource referenced by the
// `DeletedWith` field.
//
// This function is linear in the number of resources in the `DependencyGraph`.
func (dg *DependencyGraph) TransitiveDependenciesOf(r *resource.State) mapset.Set[*resource.State] {
dependentProviders := make(map[resource.URN]struct{})
urns := make(map[resource.URN]*node, len(dg.resources))
for _, r := range dg.resources {
urns[r.URN] = &node{resource: r}
}
// Linearity is due to short circuiting in the traversal.
markAsDependency(r.URN, urns, dependentProviders)
// This will only trigger if (urn, node) is a provider. The check is implicit
// in the set lookup.
for urn := range urns {
if _, ok := dependentProviders[urn]; ok {
markAsDependency(urn, urns, dependentProviders)
}
}
dependencies := mapset.NewSet[*resource.State]()
for _, r := range urns {
if r.marked {
dependencies.Add(r.resource)
}
}
Better handle property dependencies and `deletedWith` (#16088) 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).
2024-05-03 17:08:06 +00:00
// We don't want to include `r` as its own dependency.
dependencies.Remove(r)
return dependencies
}
// ChildrenOf returns a slice containing all resources that are children of the given resource.
func (dg *DependencyGraph) ChildrenOf(res *resource.State) []*resource.State {
children := make([]*resource.State, 0)
for _, childIndex := range dg.childrenOf[res.URN] {
children = append(children, dg.resources[childIndex])
}
return children
}
// ParentsOf returns a slice containing all resources that are parents of the given resource.
func (dg *DependencyGraph) ParentsOf(res *resource.State) []*resource.State {
parents := make([]*resource.State, 0)
// The resources in dg.resources are topologically sorted, so when we walk backwards and we match a parent,
// we know we have yet to see that parent's parent (if it exists). We know it's safe to terminate when we've
// traversed the full set in reverse.
for i := len(dg.resources) - 1; i >= 0; i-- {
if dg.resources[i].URN == res.Parent {
parents = append(parents, dg.resources[i])
res = dg.resources[i]
}
}
return parents
}
Better handle property dependencies and `deletedWith` (#16088) 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).
2024-05-03 17:08:06 +00:00
// Mark a resource and its provider, parent, dependencies, property
// dependencies, and deletion dependencies, as a dependency. This is a helper
// function for `TransitiveDependenciesOf`.
func markAsDependency(urn resource.URN, urns map[resource.URN]*node, dependedProviders map[resource.URN]struct{}) {
r := urns[urn]
for {
r.marked = true
if r.resource.Provider != "" {
ref, err := providers.ParseReference(r.resource.Provider)
contract.AssertNoErrorf(err, "cannot parse provider reference %q", r.resource.Provider)
dependedProviders[ref.URN()] = struct{}{}
}
for _, dep := range r.resource.Dependencies {
markAsDependency(dep, urns, dependedProviders)
}
Better handle property dependencies and `deletedWith` (#16088) 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).
2024-05-03 17:08:06 +00:00
for _, deps := range r.resource.PropertyDependencies {
for _, dep := range deps {
markAsDependency(dep, urns, dependedProviders)
}
}
if r.resource.DeletedWith != "" {
markAsDependency(r.resource.DeletedWith, urns, dependedProviders)
}
Better handle property dependencies and `deletedWith` (#16088) 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).
2024-05-03 17:08:06 +00:00
// If the resource's parent is already marked, we don't need to continue to
// traverse. All nodes above its parent will have already been marked. This
// is a property of the set of resources being topologically sorted.
if p, ok := urns[r.resource.Parent]; ok && !p.marked {
r = p
} else {
break
}
}
}
// NewDependencyGraph creates a new DependencyGraph from a list of resources.
// The resources should be in topological order with respect to their dependencies, including
// parents appearing before children.
func NewDependencyGraph(resources []*resource.State) *DependencyGraph {
index := make(map[*resource.State]int)
childrenOf := make(map[resource.URN][]int)
urnIndex := make(map[resource.URN]int)
for idx, res := range resources {
index[res] = idx
urnIndex[res.URN] = idx
parent := res.Parent
for parent != "" {
childrenOf[parent] = append(childrenOf[parent], idx)
parent = resources[urnIndex[parent]].Parent
}
}
return &DependencyGraph{index, resources, childrenOf}
}
// A node in a graph.
type node struct {
marked bool
resource *resource.State
}