2021-11-15 19:45:14 +00:00
|
|
|
// Copyright 2016-2021, Pulumi Corporation. All rights reserved.
|
2018-05-23 21:43:17 +00:00
|
|
|
|
|
|
|
package graph
|
|
|
|
|
|
|
|
import (
|
|
|
|
"testing"
|
|
|
|
|
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
|
|
|
mapset "github.com/deckarep/golang-set/v2"
|
2021-03-17 13:20:05 +00:00
|
|
|
"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/tokens"
|
2018-05-23 21:43:17 +00:00
|
|
|
"github.com/stretchr/testify/assert"
|
|
|
|
)
|
|
|
|
|
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
|
|
|
func TestDependencyGraph(t *testing.T) {
|
|
|
|
t.Parallel()
|
2018-05-23 21:43:17 +00:00
|
|
|
|
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
|
|
|
// Arrange.
|
|
|
|
providerA, providerARef := makeProvider("pkg", "providerA", "0")
|
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
|
|
|
|
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
|
|
|
a1 := &resource.State{URN: "a1", Provider: providerARef}
|
|
|
|
a2 := &resource.State{
|
|
|
|
URN: "a2",
|
|
|
|
Provider: providerARef,
|
|
|
|
Dependencies: []resource.URN{a1.URN},
|
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
|
|
|
}
|
2023-03-03 16:36:39 +00:00
|
|
|
|
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
|
|
|
providerB, providerBRef := makeProvider("pkg", "providerB", "1", a1.URN, a2.URN)
|
2022-03-04 08:17:41 +00:00
|
|
|
|
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
|
|
|
b1 := &resource.State{
|
|
|
|
URN: "b1",
|
|
|
|
Provider: providerBRef,
|
|
|
|
Dependencies: []resource.URN{a1.URN},
|
|
|
|
}
|
2018-05-23 21:43:17 +00:00
|
|
|
|
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
|
|
|
c1 := &resource.State{
|
|
|
|
URN: "c1",
|
|
|
|
Dependencies: []resource.URN{a2.URN},
|
|
|
|
}
|
2018-05-23 21:43:17 +00:00
|
|
|
|
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
|
|
|
providerD, providerDRef := makeProvider("pkg", "providerD", "2")
|
2018-05-23 21:43:17 +00:00
|
|
|
|
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
|
|
|
d1 := &resource.State{URN: "d1", Provider: providerDRef}
|
|
|
|
d2 := &resource.State{URN: "d2", Parent: d1.URN}
|
|
|
|
d3 := &resource.State{URN: "d3", Provider: providerDRef, DeletedWith: d1.URN}
|
2024-07-11 12:59:20 +00:00
|
|
|
d4 := &resource.State{URN: "d4", Parent: d2.URN}
|
|
|
|
d5 := &resource.State{URN: "d5", Parent: d3.URN}
|
2022-03-04 08:17:41 +00:00
|
|
|
|
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
|
|
|
e1 := &resource.State{URN: "e1"}
|
|
|
|
e2 := &resource.State{URN: "e2", Dependencies: []resource.URN{e1.URN}}
|
|
|
|
e3 := &resource.State{
|
|
|
|
URN: "e3",
|
|
|
|
PropertyDependencies: map[resource.PropertyKey][]resource.URN{
|
|
|
|
"e3Prop1": {e1.URN},
|
|
|
|
},
|
|
|
|
}
|
|
|
|
|
|
|
|
e4 := &resource.State{URN: "e4", Dependencies: []resource.URN{e3.URN}}
|
|
|
|
e5 := &resource.State{URN: "e5", DeletedWith: e3.URN}
|
2018-05-23 21:43:17 +00:00
|
|
|
|
|
|
|
dg := NewDependencyGraph([]*resource.State{
|
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
|
|
|
providerA,
|
|
|
|
a1,
|
|
|
|
a2,
|
|
|
|
providerB,
|
|
|
|
b1,
|
|
|
|
c1,
|
|
|
|
providerD,
|
|
|
|
d1,
|
|
|
|
d2,
|
|
|
|
d3,
|
2024-07-11 12:59:20 +00:00
|
|
|
d4,
|
|
|
|
d5,
|
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
|
|
|
e1,
|
|
|
|
e2,
|
|
|
|
e3,
|
|
|
|
e4,
|
|
|
|
e5,
|
2018-05-23 21:43:17 +00:00
|
|
|
})
|
|
|
|
|
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
|
|
|
t.Run("DependingOn", func(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
// Arrange.
|
|
|
|
cases := []struct {
|
|
|
|
name string
|
|
|
|
res *resource.State
|
|
|
|
ignore map[resource.URN]bool
|
|
|
|
includeChildren bool
|
|
|
|
expected []*resource.State
|
|
|
|
}{
|
|
|
|
{
|
|
|
|
name: "providerA",
|
|
|
|
res: providerA,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
a1, // a1's provider is providerA
|
|
|
|
a2, // a2's provider is providerA; a2 also depends on a1
|
|
|
|
providerB, // providerB depends on a1 and a2
|
|
|
|
b1, // b1 depends on a1 and providerB
|
|
|
|
c1, // c1 depends on a2
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "a1",
|
|
|
|
res: a1,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
a2, // a2 depends on a1
|
|
|
|
providerB, // providerB depends on a1 and a2
|
|
|
|
b1, // b1 depends on a1 and providerB
|
|
|
|
c1, // c1 depends on a2
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "a2",
|
|
|
|
res: a2,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
providerB, // providerB depends on a2
|
|
|
|
b1, // b1's provider is providerB
|
|
|
|
c1, // c1 depends on a2
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerB",
|
|
|
|
res: providerB,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
b1, // b1's provider is providerB
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "b1",
|
|
|
|
res: b1,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: nil,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "c1",
|
|
|
|
res: c1,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: nil,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerA ignorning a1",
|
|
|
|
res: providerA,
|
|
|
|
ignore: map[resource.URN]bool{a1.URN: true},
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
a2, // a2's provider is providerA
|
|
|
|
providerB, // providerB depends on a2
|
|
|
|
b1, // b1's provider is providerB
|
|
|
|
c1, // c1 depends on a2
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerA ignorning a2",
|
|
|
|
res: providerA,
|
|
|
|
ignore: map[resource.URN]bool{a2.URN: true},
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
a1, // a1's provider is providerA
|
|
|
|
providerB, // providerB depends on a1
|
|
|
|
b1, // b1's provider is providerB
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerA ignoring a1 and a2",
|
|
|
|
res: providerA,
|
|
|
|
ignore: map[resource.URN]bool{a1.URN: true, a2.URN: true},
|
|
|
|
includeChildren: false,
|
|
|
|
expected: nil,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "a1 ignoring a2",
|
|
|
|
res: a1,
|
|
|
|
ignore: map[resource.URN]bool{a2.URN: true},
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
providerB, // providerB depends on a1
|
|
|
|
b1, // b1's provider is providerB
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "a1 ignoring a2 and providerB",
|
|
|
|
res: a1,
|
|
|
|
ignore: map[resource.URN]bool{a2.URN: true, providerB.URN: true},
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
b1, // b1 depends on a1
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "a2 ignoring providerB",
|
|
|
|
res: a2,
|
|
|
|
ignore: map[resource.URN]bool{providerB.URN: true},
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
c1, // c1 depends on a2
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "b1 ignoring providerB",
|
|
|
|
res: b1,
|
|
|
|
ignore: map[resource.URN]bool{providerB.URN: true},
|
|
|
|
includeChildren: false,
|
|
|
|
expected: nil,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerD",
|
|
|
|
res: providerD,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
d1, // d1's provider is providerD
|
|
|
|
d3, // d3's provider is providerD; d3 is deleted with d1
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerD including parent/child relationships",
|
|
|
|
res: providerD,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: true,
|
|
|
|
expected: []*resource.State{
|
|
|
|
d1, // d1's provider is providerD
|
|
|
|
d2, // d2 is a child of d1
|
|
|
|
d3, // d3's provider is providerD; d3 is deleted with d1
|
2024-07-11 12:59:20 +00:00
|
|
|
d4, // d4 is a child of d2
|
|
|
|
d5, // d5 is a child of d3
|
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
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerD ignoring d1, including parent/child relationships",
|
|
|
|
res: providerD,
|
|
|
|
ignore: map[resource.URN]bool{d1.URN: true},
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
d3, // d3's provider is providerD
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d1",
|
|
|
|
res: d1,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
d3, // d3 is deleted with d1
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d1 including parent/child relationships",
|
|
|
|
res: d1,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: true,
|
|
|
|
expected: []*resource.State{
|
|
|
|
d2, // d2 is a child of d1
|
|
|
|
d3, // d3 is deleted with d1
|
2024-07-11 12:59:20 +00:00
|
|
|
d4, // d4 is a child of d2
|
|
|
|
d5, // d5 is a child of d3
|
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
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d1 ignoring d3",
|
|
|
|
res: d1,
|
|
|
|
ignore: map[resource.URN]bool{d3.URN: true},
|
|
|
|
includeChildren: false,
|
|
|
|
expected: nil,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d2",
|
|
|
|
res: d2,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: nil,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e1",
|
|
|
|
res: e1,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
e2, // e2 depends on e3
|
|
|
|
e3, // e3 has a property dependency on e1
|
|
|
|
e4, // e4 depends on e3
|
|
|
|
e5, // e5 is deleted with e3
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e1 ignoring e3",
|
|
|
|
res: e1,
|
|
|
|
ignore: map[resource.URN]bool{e3.URN: true},
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
e2, // e2 depends on e3
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e3",
|
|
|
|
res: e3,
|
|
|
|
ignore: nil,
|
|
|
|
includeChildren: false,
|
|
|
|
expected: []*resource.State{
|
|
|
|
e4, // e4 depends on e3
|
|
|
|
e5, // e5 is deleted with e3
|
|
|
|
},
|
|
|
|
},
|
|
|
|
}
|
2018-09-27 22:49:08 +00:00
|
|
|
|
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 _, c := range cases {
|
|
|
|
c := c
|
|
|
|
t.Run(c.name, func(t *testing.T) {
|
|
|
|
t.Parallel()
|
2022-03-04 08:17:41 +00:00
|
|
|
|
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
|
|
|
// Act.
|
|
|
|
actual := dg.DependingOn(c.res, c.ignore, c.includeChildren)
|
2018-09-27 22:49:08 +00:00
|
|
|
|
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
|
|
|
// Assert.
|
|
|
|
assert.Equal(t, c.expected, actual)
|
|
|
|
})
|
|
|
|
}
|
2018-09-27 22:49:08 +00:00
|
|
|
})
|
|
|
|
|
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
|
|
|
t.Run("OnlyDependsOn", func(t *testing.T) {
|
|
|
|
t.Parallel()
|
2021-08-19 01:26:36 +00:00
|
|
|
|
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
|
|
|
// Arrange.
|
|
|
|
cases := []struct {
|
|
|
|
name string
|
|
|
|
res *resource.State
|
|
|
|
expected []*resource.State
|
|
|
|
}{}
|
2022-03-04 08:17:41 +00:00
|
|
|
|
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 _, c := range cases {
|
|
|
|
c := c
|
|
|
|
t.Run(c.name, func(t *testing.T) {
|
|
|
|
t.Parallel()
|
2021-08-19 01:26:36 +00:00
|
|
|
|
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
|
|
|
// Act.
|
|
|
|
actual := dg.OnlyDependsOn(c.res)
|
|
|
|
|
|
|
|
// Assert.
|
|
|
|
assert.Equal(t, c.expected, actual)
|
|
|
|
})
|
|
|
|
}
|
2021-08-19 01:26:36 +00:00
|
|
|
})
|
|
|
|
|
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
|
|
|
t.Run("DependenciesOf", func(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
// Arrange.
|
|
|
|
cases := []struct {
|
|
|
|
name string
|
|
|
|
res *resource.State
|
|
|
|
expected mapset.Set[*resource.State]
|
|
|
|
}{
|
|
|
|
{
|
|
|
|
name: "providerA",
|
|
|
|
res: providerA,
|
|
|
|
expected: mapset.NewSet[*resource.State](),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "a1",
|
|
|
|
res: a1,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
providerA, // a1's provider is providerA
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "a2",
|
|
|
|
res: a2,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
providerA, // a2's provider is providerA
|
|
|
|
a1, // a2 depends on a1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerB",
|
|
|
|
res: providerB,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
a1, // providerB depends on a1
|
|
|
|
a2, // providerB depends on a2
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "b1",
|
|
|
|
res: b1,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
providerB, // b1's provider is providerB
|
|
|
|
a1, // b1 depends on a1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "c1",
|
|
|
|
res: c1,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
a2, // c1 depends on a2
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerD",
|
|
|
|
res: providerD,
|
|
|
|
expected: mapset.NewSet[*resource.State](),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d1",
|
|
|
|
res: d1,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
providerD, // d1's provider is providerD
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d2",
|
|
|
|
res: d2,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
d1, // d2 is a child of d1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d3",
|
|
|
|
res: d3,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
providerD, // d3's provider is providerD
|
|
|
|
d1, // d3 is deleted with d1
|
|
|
|
d2, // d2 is a child of d1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e1",
|
|
|
|
res: e1,
|
|
|
|
expected: mapset.NewSet[*resource.State](),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e2",
|
|
|
|
res: e2,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
e1, // e2 depends on e1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e3",
|
|
|
|
res: e3,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
e1, // e3 has a property dependency on e1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e4",
|
|
|
|
res: e4,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
e3, // e4 depends on e3
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e5",
|
|
|
|
res: e5,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
e3, // e5 is deleted with e3
|
|
|
|
),
|
|
|
|
},
|
|
|
|
}
|
2021-08-19 01:26:36 +00:00
|
|
|
|
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 _, c := range cases {
|
|
|
|
c := c
|
|
|
|
t.Run(c.name, func(t *testing.T) {
|
|
|
|
t.Parallel()
|
2022-03-04 08:17:41 +00:00
|
|
|
|
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
|
|
|
// Act.
|
|
|
|
actual := dg.DependenciesOf(c.res)
|
2021-08-19 01:26:36 +00:00
|
|
|
|
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
|
|
|
// Assert.
|
|
|
|
if !c.expected.Equal(actual) {
|
|
|
|
assert.Failf(t, "expected and actual do not match", "expected: %v\nactual : %v", c.expected, actual)
|
|
|
|
}
|
|
|
|
})
|
|
|
|
}
|
2021-08-19 01:26:36 +00:00
|
|
|
})
|
|
|
|
|
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
|
|
|
t.Run("TransitiveDependenciesOf", func(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
// Arrange.
|
|
|
|
cases := []struct {
|
|
|
|
name string
|
|
|
|
res *resource.State
|
|
|
|
expected mapset.Set[*resource.State]
|
|
|
|
}{
|
|
|
|
{
|
|
|
|
name: "providerA",
|
|
|
|
res: providerA,
|
|
|
|
expected: mapset.NewSet[*resource.State](),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "a1",
|
|
|
|
res: a1,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
providerA, // a1's provider is providerA
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "a2",
|
|
|
|
res: a2,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
providerA, // a2's provider is providerA
|
|
|
|
a1, // a2 depends on a1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerB",
|
|
|
|
res: providerB,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
a1, // providerB depends on a1
|
|
|
|
a2, // providerB depends on a2
|
|
|
|
providerA, // (transitive) providerA is a1's and a2's provider
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "b1",
|
|
|
|
res: b1,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
providerB, // b1's provider is providerB
|
|
|
|
a1, // b1 depends on a1
|
|
|
|
a2, // (transitive) providerB depends on a2
|
|
|
|
providerA, // (transitive) providerA is a1's and a2's provider
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "c1",
|
|
|
|
res: c1,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
a2, // c1 depends on a2
|
|
|
|
providerA, // (transitive) providerA is a2's provider
|
|
|
|
a1, // (transitive) a2 depends on a1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerD",
|
|
|
|
res: providerD,
|
|
|
|
expected: mapset.NewSet[*resource.State](),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d1",
|
|
|
|
res: d1,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
providerD, // d1's provider is providerD
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d2",
|
|
|
|
res: d2,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
d1, // d2 is a child of d1
|
|
|
|
providerD, // (transitive) d1's provider is providerD
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d3",
|
|
|
|
res: d3,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
providerD, // d3's provider is providerD
|
|
|
|
d1, // d3 is deleted with d1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e1",
|
|
|
|
res: e1,
|
|
|
|
expected: mapset.NewSet[*resource.State](),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e2",
|
|
|
|
res: e2,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
e1, // e2 depends on e1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e3",
|
|
|
|
res: e3,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
e1, // e3 has a property dependency on e1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e4",
|
|
|
|
res: e4,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
e3, // e4 depends on e3
|
|
|
|
e1, // (transitive) e3 has a property dependency on e1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "e5",
|
|
|
|
res: e5,
|
|
|
|
expected: mapset.NewSet[*resource.State](
|
|
|
|
e3, // e5 is deleted with e3
|
|
|
|
e1, // (transitive) e3 has a property dependency on e1
|
|
|
|
),
|
|
|
|
},
|
|
|
|
}
|
2021-11-15 19:45:14 +00:00
|
|
|
|
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 _, c := range cases {
|
|
|
|
c := c
|
|
|
|
t.Run(c.name, func(t *testing.T) {
|
|
|
|
t.Parallel()
|
2022-03-04 08:17:41 +00:00
|
|
|
|
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
|
|
|
// Act.
|
|
|
|
actual := dg.TransitiveDependenciesOf(c.res)
|
2021-11-15 19:45:14 +00:00
|
|
|
|
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
|
|
|
// Assert.
|
|
|
|
if !c.expected.Equal(actual) {
|
|
|
|
assert.Failf(t, "expected and actual do not match", "expected: %v\nactual : %v", c.expected, actual)
|
|
|
|
}
|
|
|
|
})
|
|
|
|
}
|
2021-11-15 19:45:14 +00:00
|
|
|
})
|
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
|
|
|
|
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
|
|
|
providerF1, providerF1Ref := makeProvider("pkg", "providerF", "3")
|
|
|
|
providerF2, providerF2Ref := makeProvider("pkg", "providerF", "4")
|
|
|
|
providerF3, providerF3Ref := makeProvider("pkg", "providerF", "5")
|
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
|
|
|
|
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
|
|
|
fx1 := &resource.State{URN: "fx", Provider: providerF1Ref}
|
|
|
|
fx2 := &resource.State{URN: "fx", Provider: providerF2Ref}
|
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
|
|
|
|
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
|
|
|
fy := &resource.State{URN: "fy", Provider: providerF3Ref, Dependencies: []resource.URN{fx1.URN}}
|
|
|
|
fz := &resource.State{
|
|
|
|
URN: "fz",
|
|
|
|
Provider: providerF3Ref,
|
|
|
|
PropertyDependencies: map[resource.PropertyKey][]resource.URN{
|
|
|
|
"fzProp1": {fx1.URN},
|
|
|
|
},
|
|
|
|
}
|
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
|
|
|
|
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
|
|
|
fw := &resource.State{
|
|
|
|
URN: "fw",
|
|
|
|
Provider: providerF3Ref,
|
|
|
|
DeletedWith: fx1.URN,
|
|
|
|
}
|
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
|
|
|
|
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
|
|
|
dgOnly := NewDependencyGraph([]*resource.State{
|
|
|
|
providerF1,
|
|
|
|
providerF2,
|
|
|
|
providerF3,
|
|
|
|
fx1,
|
|
|
|
fx2,
|
|
|
|
fy,
|
|
|
|
fz,
|
|
|
|
fw,
|
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
|
|
|
})
|
|
|
|
|
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
|
|
|
t.Run("OnlyDependsOn", func(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
// Arrange.
|
|
|
|
cases := []struct {
|
|
|
|
name string
|
|
|
|
res *resource.State
|
|
|
|
expected []*resource.State
|
|
|
|
}{
|
|
|
|
{
|
|
|
|
name: "providerF1",
|
|
|
|
res: providerF1,
|
|
|
|
expected: []*resource.State{
|
|
|
|
fx1,
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerF2",
|
|
|
|
res: providerF2,
|
|
|
|
expected: []*resource.State{
|
|
|
|
fx2,
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "providerF3",
|
|
|
|
res: providerF3,
|
|
|
|
expected: []*resource.State{
|
|
|
|
fy,
|
|
|
|
fz,
|
|
|
|
fw,
|
|
|
|
},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "fx1",
|
|
|
|
res: fx1,
|
|
|
|
expected: nil,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "fx2",
|
|
|
|
res: fx2,
|
|
|
|
expected: nil,
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "fy1",
|
|
|
|
res: fy,
|
|
|
|
expected: nil,
|
|
|
|
},
|
|
|
|
}
|
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
|
|
|
|
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 _, c := range cases {
|
|
|
|
c := c
|
|
|
|
t.Run(c.name, func(t *testing.T) {
|
|
|
|
t.Parallel()
|
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
|
|
|
|
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
|
|
|
// Act.
|
|
|
|
actual := dgOnly.OnlyDependsOn(c.res)
|
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
|
|
|
|
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
|
|
|
// Assert.
|
|
|
|
assert.Equal(t, c.expected, actual)
|
|
|
|
})
|
|
|
|
}
|
|
|
|
})
|
2024-07-11 12:59:20 +00:00
|
|
|
|
|
|
|
t.Run("ParentsOf", func(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
// Arrange.
|
|
|
|
cases := []struct {
|
|
|
|
name string
|
|
|
|
res *resource.State
|
|
|
|
expected []*resource.State
|
|
|
|
}{
|
|
|
|
{
|
|
|
|
name: "e2",
|
|
|
|
res: e2,
|
|
|
|
expected: []*resource.State{},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d2",
|
|
|
|
res: d2,
|
|
|
|
expected: []*resource.State{d1},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d4",
|
|
|
|
res: d4,
|
|
|
|
expected: []*resource.State{d2, d1},
|
|
|
|
},
|
|
|
|
{
|
|
|
|
name: "d5",
|
|
|
|
res: d5,
|
|
|
|
expected: []*resource.State{d3},
|
|
|
|
},
|
|
|
|
}
|
|
|
|
|
|
|
|
for _, c := range cases {
|
|
|
|
c := c
|
|
|
|
t.Run(c.name, func(t *testing.T) {
|
|
|
|
t.Parallel()
|
|
|
|
|
|
|
|
// Act.
|
|
|
|
actual := dg.ParentsOf(c.res)
|
|
|
|
|
|
|
|
// Assert.
|
|
|
|
assert.Equal(t, c.expected, actual)
|
|
|
|
})
|
|
|
|
}
|
|
|
|
})
|
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
|
|
|
}
|
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
|
|
|
|
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
|
|
|
func makeProvider(pkg, name, id string, deps ...resource.URN) (*resource.State, string) {
|
|
|
|
t := providers.MakeProviderType(tokens.Package(pkg))
|
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
|
|
|
|
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
|
|
|
provider := &resource.State{
|
|
|
|
Type: t,
|
|
|
|
URN: resource.NewURN("stack", "proj", "", t, name),
|
|
|
|
ID: resource.ID(id),
|
|
|
|
Inputs: resource.PropertyMap{},
|
|
|
|
Outputs: resource.PropertyMap{},
|
|
|
|
Dependencies: deps,
|
|
|
|
}
|
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
|
|
|
|
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
|
|
|
providerRef, err := providers.NewReference(provider.URN, provider.ID)
|
|
|
|
if err != nil {
|
|
|
|
panic(err)
|
|
|
|
}
|
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
|
|
|
|
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
|
|
|
return provider, providerRef.String()
|
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
|
|
|
}
|