maxmustermann/pulumi
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[sdk] Wait on remote component dependencies (#7541)

Browse source 
When a resource `dependsOn` a remote component, we were not correctly waiting on it, because we were skipping over waiting on comoponents, and only waiting on their custom resource children.  For remote components, we do not know the children, but waiting on the remote component will always wait on all children.

Co-authored-by: Mike Metral <1112768+metral@users.noreply.github.com>
This commit is contained in:
Luke Hoban 2021-07-16 16:11:34 -07:00 committed by GitHub
parent 28b1a25629
commit 0bcca3883e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 98 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -19,6 +19,9 @@ CHANGELOG
- [codegen/go] - Emit To[ElementType]Output methods for go enum output types
[#7499](https://github.com/pulumi/pulumi/pull/7499)
- [sdk/nodejs] - Wait on remote component dependencies
[#7541](https://github.com/pulumi/pulumi/pull/7541)
## 3.6.1 (2021-07-07)
### Improvements

54
sdk/dotnet/Pulumi/Deployment/Deployment_Prepare.cs
View file

@ -77,14 +77,14 @@ namespace Pulumi
// The list of all dependencies (implicit or explicit).
var allDirectDependencies = new HashSet<Resource>(explicitDirectDependencies);
var allDirectDependencyUrns = await GetAllTransitivelyReferencedCustomResourceUrnsAsync(explicitDirectDependencies).ConfigureAwait(false);
var allDirectDependencyUrns = await GetAllTransitivelyReferencedResourceUrnsAsync(explicitDirectDependencies).ConfigureAwait(false);
var propertyToDirectDependencyUrns = new Dictionary<string, HashSet<string>>();
foreach (var (propertyName, directDependencies) in propertyToDirectDependencies)
{
allDirectDependencies.AddRange(directDependencies);
var urns = await GetAllTransitivelyReferencedCustomResourceUrnsAsync(directDependencies).ConfigureAwait(false);
var urns = await GetAllTransitivelyReferencedResourceUrnsAsync(directDependencies).ConfigureAwait(false);
allDirectDependencyUrns.AddRange(urns);
propertyToDirectDependencyUrns[propertyName] = urns;
}
@ -123,32 +123,42 @@ namespace Pulumi
private static Task<ImmutableArray<Resource>> GatherExplicitDependenciesAsync(InputList<Resource> resources)
=> resources.ToOutput().GetValueAsync();
private static async Task<HashSet<string>> GetAllTransitivelyReferencedCustomResourceUrnsAsync(
private static async Task<HashSet<string>> GetAllTransitivelyReferencedResourceUrnsAsync(
HashSet<Resource> resources)
{
// Go through 'resources', but transitively walk through **Component** resources,
// collecting any of their child resources. This way, a Component acts as an
// aggregation really of all the reachable custom resources it parents. This walking
// will transitively walk through other child ComponentResources, but will stop when it
// hits custom resources. in other words, if we had:
// Go through 'resources', but transitively walk through **Component** resources, collecting any
// of their child resources. This way, a Component acts as an aggregation really of all the
// reachable resources it parents. This walking will stop when it hits custom resources.
//
// Comp1
// / \
// Cust1 Comp2
// / \
// Cust2 Cust3
// /
// Cust4
// This function also terminates at remote components, whose children are not known to the Node SDK directly.
// Remote components will always wait on all of their children, so ensuring we return the remote component
// itself here and waiting on it will accomplish waiting on all of it's children regardless of whether they
// are returned explicitly here.
//
// Then the transitively reachable custom resources of Comp1 will be [Cust1, Cust2,
// Cust3]. It will *not* include 'Cust4'.
// To do this, first we just get the transitively reachable set of resources (not diving
// into custom resources). In the above picture, if we start with 'Comp1', this will be
// [Comp1, Cust1, Comp2, Cust2, Cust3]
// In other words, if we had:
//
// Comp1
// / | \
// Cust1 Comp2 Remote1
// / \ \
// Cust2 Cust3 Comp3
// / \
// Cust4 Cust5
//
// Then the transitively reachable resources of Comp1 will be [Cust1, Cust2, Cust3, Remote1]. It
// will *not* include:
// * Cust4 because it is a child of a custom resource
// * Comp2 because it is a non-remote component resoruce
// * Comp3 and Cust5 because Comp3 is a child of a remote component resource
var transitivelyReachableResources = GetTransitivelyReferencedChildResourcesOfComponentResources(resources);
var transitivelyReachableCustomResources = transitivelyReachableResources.OfType<CustomResource>();
var transitivelyReachableCustomResources = transitivelyReachableResources.Where(res => {
switch (res) {
case CustomResource custom: return true;
case ComponentResource component: return component.remote;
default: return false; // Unreachable
}
});
var tasks = transitivelyReachableCustomResources.Select(r => r.Urn.GetValueAsync());
var urns = await Task.WhenAll(tasks).ConfigureAwait(false);
return new HashSet<string>(urns);

3
sdk/dotnet/Pulumi/Resources/ComponentResource.cs
View file

@ -14,6 +14,8 @@ namespace Pulumi
/// </summary>
public class ComponentResource : Resource
{
internal readonly bool remote;
/// <summary>
/// Creates and registers a new component resource. <paramref name="type"/> is the fully
/// qualified type token and <paramref name="name"/> is the "name" part to use in creating a
@ -47,6 +49,7 @@ namespace Pulumi
: base(type, name, custom: false, args ?? ResourceArgs.Empty, options ?? new ComponentResourceOptions(), remote)
#pragma warning restore RS0022 // Constructor make noninheritable base class inheritable
{
this.remote = remote;
}
/// <summary>

5
sdk/nodejs/resource.ts
View file

@ -803,6 +803,10 @@ export class ComponentResource<TData = any> extends Resource {
// tslint:disable-next-line:variable-name
private __registered = false;
/** @internal */
// tslint:disable-next-line:variable-name
public readonly __remote: boolean;
/**
* Returns true if the given object is an instance of CustomResource. This is designed to work even when
* multiple copies of the Pulumi SDK have been loaded into the same process.
@ -834,6 +838,7 @@ export class ComponentResource<TData = any> extends Resource {
// not correspond to a real piece of cloud infrastructure. As such, changes to it *itself*
// do not have any effect on the cloud side of things at all.
super(type, name, /*custom:*/ false, /*props:*/ remote || opts?.urn ? args : {}, opts, remote);
this.__remote = remote;
this.__registered = remote || !!opts?.urn;
this.__data = remote || opts?.urn ? Promise.resolve(<TData>{}) : this.initializeAndRegisterOutputs(args);
}

43
sdk/nodejs/runtime/resource.ts
View file

@ -539,13 +539,13 @@ async function prepareResource(label: string, res: Resource, custom: boolean, re
// The list of all dependencies (implicit or explicit).
const allDirectDependencies = new Set<Resource>(explicitDirectDependencies);
const allDirectDependencyURNs = await getAllTransitivelyReferencedCustomResourceURNs(explicitDirectDependencies);
const allDirectDependencyURNs = await getAllTransitivelyReferencedResourceURNs(explicitDirectDependencies);
const propertyToDirectDependencyURNs = new Map<string, Set<URN>>();
for (const [propertyName, directDependencies] of propertyToDirectDependencies) {
addAll(allDirectDependencies, directDependencies);
const urns = await getAllTransitivelyReferencedCustomResourceURNs(directDependencies);
const urns = await getAllTransitivelyReferencedResourceURNs(directDependencies);
addAll(allDirectDependencyURNs, urns);
propertyToDirectDependencyURNs.set(propertyName, urns);
}
@ -584,30 +584,41 @@ function addAll<T>(to: Set<T>, from: Set<T>) {
}
}
async function getAllTransitivelyReferencedCustomResourceURNs(resources: Set<Resource>) {
async function getAllTransitivelyReferencedResourceURNs(resources: Set<Resource>): Promise<Set<string>> {
// Go through 'resources', but transitively walk through **Component** resources, collecting any
// of their child resources. This way, a Component acts as an aggregation really of all the
// reachable custom resources it parents. This walking will transitively walk through other
// child ComponentResources, but will stop when it hits custom resources. in other words, if we
// had:
// reachable resources it parents. This walking will stop when it hits custom resources.
//
// Comp1
// / \
// Cust1 Comp2
// / \
// Cust2 Cust3
// /
// Cust4
// This function also terminates at remote components, whose children are not known to the Node SDK directly.
// Remote components will always wait on all of their children, so ensuring we return the remote component
// itself here and waiting on it will accomplish waiting on all of it's children regardless of whether they
// are returned explicitly here.
//
// Then the transitively reachable custom resources of Comp1 will be [Cust1, Cust2, Cust3]. It
// will *not* include `Cust4`.
// In other words, if we had:
//
// Comp1
// / | \
// Cust1 Comp2 Remote1
// / \ \
// Cust2 Cust3 Comp3
// / \
// Cust4 Cust5
//
// Then the transitively reachable resources of Comp1 will be [Cust1, Cust2, Cust3, Remote1].
// It will *not* include:
// * Cust4 because it is a child of a custom resource
// * Comp2 because it is a non-remote component resoruce
// * Comp3 and Cust5 because Comp3 is a child of a remote component resource
// To do this, first we just get the transitively reachable set of resources (not diving
// into custom resources). In the above picture, if we start with 'Comp1', this will be
// [Comp1, Cust1, Comp2, Cust2, Cust3]
const transitivelyReachableResources = await getTransitivelyReferencedChildResourcesOfComponentResources(resources);
const transitivelyReachableCustomResources = [...transitivelyReachableResources].filter(r => CustomResource.isInstance(r));
// Then we filter to only include Custom and Remote resources.
const transitivelyReachableCustomResources =
[...transitivelyReachableResources]
.filter(r => CustomResource.isInstance(r) || (r as ComponentResource).__remote);
const promises = transitivelyReachableCustomResources.map(r => r.urn.promise());
const urns = await Promise.all(promises);
return new Set<string>(urns);

10
tests/integration/integration_dotnet_test.go
View file

@ -380,12 +380,18 @@ func optsForConstructDotnet(t *testing.T, expectedResourceCount int, env ...stri
urns[string(res.URN.Name())] = res.URN
switch res.URN.Name() {
case "child-a", "child-b":
case "child-a":
for _, deps := range res.PropertyDependencies {
assert.Empty(t, deps)
}
case "child-b":
expected := []resource.URN{urns["a"]}
assert.ElementsMatch(t, expected, res.Dependencies)
assert.ElementsMatch(t, expected, res.PropertyDependencies["echo"])
case "child-c":
assert.Equal(t, []resource.URN{urns["child-a"]}, res.PropertyDependencies["echo"])
expected := []resource.URN{urns["a"], urns["child-a"]}
assert.ElementsMatch(t, expected, res.Dependencies)
assert.ElementsMatch(t, expected, res.PropertyDependencies["echo"])
case "a", "b", "c":
secretPropValue, ok := res.Outputs["secret"].(map[string]interface{})
assert.Truef(t, ok, "secret output was not serialized as a secret")

9
tests/integration/integration_go_test.go
View file

@ -474,10 +474,13 @@ func optsForConstructGo(t *testing.T, expectedResourceCount int, env ...string)
assert.Empty(t, deps)
}
case "child-b":
assert.Equal(t, []resource.URN{urns["a"]}, res.PropertyDependencies["echo"])
expected := []resource.URN{urns["a"]}
assert.ElementsMatch(t, expected, res.Dependencies)
assert.ElementsMatch(t, expected, res.PropertyDependencies["echo"])
case "child-c":
assert.ElementsMatch(t, []resource.URN{urns["child-a"], urns["a"]},
res.PropertyDependencies["echo"])
expected := []resource.URN{urns["a"], urns["child-a"]}
assert.ElementsMatch(t, expected, res.Dependencies)
assert.ElementsMatch(t, expected, res.PropertyDependencies["echo"])
case "a", "b", "c":
secretPropValue, ok := res.Outputs["secret"].(map[string]interface{})
assert.Truef(t, ok, "secret output was not serialized as a secret")

10
tests/integration/integration_nodejs_test.go
View file

@ -910,12 +910,18 @@ func optsForConstructNode(t *testing.T, expectedResourceCount int, env ...string
urns[string(res.URN.Name())] = res.URN
switch res.URN.Name() {
case "child-a", "child-b":
case "child-a":
for _, deps := range res.PropertyDependencies {
assert.Empty(t, deps)
}
case "child-b":
expected := []resource.URN{urns["a"]}
assert.ElementsMatch(t, expected, res.Dependencies)
assert.ElementsMatch(t, expected, res.PropertyDependencies["echo"])
case "child-c":
assert.Equal(t, []resource.URN{urns["child-a"]}, res.PropertyDependencies["echo"])
expected := []resource.URN{urns["a"], urns["child-a"]}
assert.ElementsMatch(t, expected, res.Dependencies)
assert.ElementsMatch(t, expected, res.PropertyDependencies["echo"])
case "a", "b", "c":
secretPropValue, ok := res.Outputs["secret"].(map[string]interface{})
assert.Truef(t, ok, "secret output was not serialized as a secret")

9
tests/integration/integration_python_test.go
View file

@ -587,10 +587,13 @@ func optsForConstructPython(t *testing.T, expectedResourceCount int, env ...stri
assert.Empty(t, deps)
}
case "child-b":
assert.Equal(t, []resource.URN{urns["a"]}, res.PropertyDependencies["echo"])
expected := []resource.URN{urns["a"]}
assert.ElementsMatch(t, expected, res.Dependencies)
assert.ElementsMatch(t, expected, res.PropertyDependencies["echo"])
case "child-c":
assert.ElementsMatch(t, []resource.URN{urns["child-a"], urns["a"]},
res.PropertyDependencies["echo"])
expected := []resource.URN{urns["a"], urns["child-a"]}
assert.ElementsMatch(t, expected, res.Dependencies)
assert.ElementsMatch(t, expected, res.PropertyDependencies["echo"])
case "a", "b", "c":
secretPropValue, ok := res.Outputs["secret"].(map[string]interface{})
assert.Truef(t, ok, "secret output was not serialized as a secret")