Handle component child / component dependent registration races when expanding deletion deps #7833
Assignees
Labels
customer/feedback
Feedback from customers
kind/bug
Some behavior is incorrect or out of spec
size/M
Estimated effort to complete (up to 5 days).
Comments
pgavlin
added
kind/bug
pgavlin
added a commit
that referenced
this issue
Aug 26, 2021
Consider the following Pulumi program:
```typescript
class MyResource extends pulumi.CustomResource {
// ...
}
class MyComponent extends pulumi.ComponentResource {
constructor(name: string) {
super("myComponent", name);
this.child = new MyResource("child", {}, {parent: this});
}
}
// Create a component resource. The constructor returns promptly, but
// the registration of the component itself happens asynchronously.
// Similarly, its child resource's constructor returns promptly, but
// its registration happens asynchronously. The SDK guarantees that the
// latter happens after the former by waiting on the parent's URN prior
// to registration (note that this is necessary in order for the runtime
// to fill out the `parent` field of the registration request).
const component = new MyComponent("component");
// Create a resource that depends on the component resource. The
// registration of this resource happens asynchronously, and races with
// the asynchronous registration of the component's child.
const res = new MyResource("resource", {}, {dependsOn: component});
```
Due to the races described in the comments, even when all of a
component's children are constructed prior to exiting the component's
constructor (e.g. not in applies, thens, etc.) we cannot guarantee that
the children have been registered prior to any resources that depend on
the component. Because of this, the approach that #7788 takes for
expanding dependencies on components to dependencies on their children
is not necessarily reliable, as it only adds children that appear prior
to dependents in the snapshot.
These changes adjust the dependency graph to add all children of a
component that would not create cycles in the dependency graph.
Fixes #7833.
pgavlin
added a commit
that referenced
this issue
Aug 26, 2021
Consider the following Pulumi program:
```typescript
class MyResource extends pulumi.CustomResource {
// ...
}
class MyComponent extends pulumi.ComponentResource {
constructor(name: string) {
super("myComponent", name);
this.child = new MyResource("child", {}, {parent: this});
}
}
// Create a component resource. The constructor returns promptly, but
// the registration of the component itself happens asynchronously.
// Similarly, its child resource's constructor returns promptly, but
// its registration happens asynchronously. The SDK guarantees that the
// latter happens after the former by waiting on the parent's URN prior
// to registration (note that this is necessary in order for the runtime
// to fill out the `parent` field of the registration request).
const component = new MyComponent("component");
// Create a resource that depends on the component resource. The
// registration of this resource happens asynchronously, and races with
// the asynchronous registration of the component's child.
const res = new MyResource("resource", {}, {dependsOn: component});
```
Due to the races described in the comments, even when all of a
component's children are constructed prior to exiting the component's
constructor (e.g. not in applies, thens, etc.) we cannot guarantee that
the children have been registered prior to any resources that depend on
the component. Because of this, the approach that #7788 takes for
expanding dependencies on components to dependencies on their children
is not necessarily reliable, as it only adds children that appear prior
to dependents in the snapshot.
These changes adjust the dependency graph to add all children of a
component that would not create cycles in the dependency graph.
Fixes #7833.
pgavlin
added a commit
that referenced
this issue
Aug 26, 2021
Consider the following Pulumi program:
```typescript
class MyResource extends pulumi.CustomResource {
// ...
}
class MyComponent extends pulumi.ComponentResource {
constructor(name: string) {
super("myComponent", name);
this.child = new MyResource("child", {}, {parent: this});
}
}
// Create a component resource. The constructor returns promptly, but
// the registration of the component itself happens asynchronously.
// Similarly, its child resource's constructor returns promptly, but
// its registration happens asynchronously. The SDK guarantees that the
// latter happens after the former by waiting on the parent's URN prior
// to registration (note that this is necessary in order for the runtime
// to fill out the `parent` field of the registration request).
const component = new MyComponent("component");
// Create a resource that depends on the component resource. The
// registration of this resource happens asynchronously, and races with
// the asynchronous registration of the component's child.
const res = new MyResource("resource", {}, {dependsOn: component});
```
Due to the races described in the comments, even when all of a
component's children are constructed prior to exiting the component's
constructor (e.g. not in applies, thens, etc.) we cannot guarantee that
the children have been registered prior to any resources that depend on
the component. Because of this, the approach that #7788 takes for
expanding dependencies on components to dependencies on their children
is not necessarily reliable, as it only adds children that appear prior
to dependents in the snapshot.
These changes adjust the dependency graph to add all children of a
component that would not create cycles in the dependency graph.
Fixes #7833.
|
A brief update: although this is certainly a problem that we need to fix, it is unlikely to trip in practice, and isn't quite as urgent as we had originally thought. In order for the ordering problems listed in this issue to arise, it must be possible for the registration of a component's children to race with the registration of a resource that depends on the component. While this is possible within the Pulumi model--particularly for in-process components--the existing implementations of |
pgavlin
removed
the
p1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
See #7791 (comment) for details.
The text was updated successfully, but these errors were encountered: