Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sdk/go] Track rehydrated components as dependencies #12494

Merged
merged 1 commit into from Mar 24, 2023

Conversation

justinvp
Copy link
Member

@justinvp justinvp commented Mar 24, 2023

When expanding dependencies, local component resources act as aggregations of their descendants; rather than adding the component resource itself, each child resource is added as a dependency. Remote component resources, on the other hand, are added directly to the set, as they naturally act as aggregations of their children with respect to dependencies: the construction of a remote component always waits on the construction of its children.

// addDependency adds a dependency on the given resource to the set of deps.
//
// The behavior of this method depends on whether or not the resource is a custom resource, a local component resource,
// or a remote component resource:
//
// - Custom resources are added directly to the set, as they are "real" nodes in the dependency graph.
// - Local component resources act as aggregations of their descendents. Rather than adding the component resource
// itself, each child resource is added as a dependency.
// - Remote component resources are added directly to the set, as they naturally act as aggregations of their children
// with respect to dependencies: the construction of a remote component always waits on the construction of its
// children.
//
// 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

However, rehydrated components (i.e. instances of a component resource that have been recreated from a URN, typically via deserialization of a resource reference) aren't marked as a remote component and won't have any children from the SDK's perspective. And because of that, when their outputs are used, there won't be any dependencies kept for those outputs.

This change fixes rehydrated components to be marked as remote components, in the same way as "dependency resources" are marked as remote components, so that they are kept as a dependency.

This addresses cases where a component resource method is implemented in Go and the method returns results derived from the component's outputs. Inside the implementation of the provider's Call, the implementation will get a rehydrated instance of the component. Without this fix, the dependency on that component will not be kept in the returned output.

Here's a concrete example:

func (c *Component) GetMessage(args *ComponentGetMessageArgs) (*ComponentGetMessageResult, error) {
return &ComponentGetMessageResult{
Message: pulumi.Sprintf("%s %s, %s!", c.First, c.Second, args.Name),
}, nil
}

The above is a method implemented in a component provider. The c *Component receiver will be a rehydrated instance of the component. The method uses pulumi.Sprintf to create a result that combines some of the component's outputs. But the combined output that is returned won't have the dependency on the component kept, since the rehydrated component is a component and doesn't have any children. With the fix, the dependency will be kept and serialized as expected.

Part of #12471

@pulumi-bot
Copy link
Contributor

Changelog

[uncommitted] (2023-03-24)

Bug Fixes

  • [sdk/go] Track rehydrated components as dependencies
    #12494

sdk/go/pulumi/context.go Outdated Show resolved Hide resolved
@justinvp justinvp requested a review from a team March 24, 2023 18:43
sdk/go/pulumi/context.go Outdated Show resolved Hide resolved
@justinvp justinvp force-pushed the justin/rehydratedcomp_go branch 2 times, most recently from 71ec791 to 663d27a Compare March 24, 2023 19:08
Copy link
Contributor

@dixler dixler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I have a better understanding how the SDK views local components after reading this. I have some nitpicks that should help future readability from the POV of someone trying to skim this for the first time.

sdk/go/pulumi/context.go Outdated Show resolved Hide resolved
sdk/go/pulumi/context.go Outdated Show resolved Hide resolved
sdk/go/pulumi/context.go Outdated Show resolved Hide resolved
When expanding dependencies, local component resources act as aggregations of their descendants; rather than adding the component resource itself, each child resource is added as a dependency. Remote component resources, on the other hand, are added directly to the set, as they naturally act as aggregations of their children with respect to dependencies: the construction of a remote component always waits on the construction of its children.

https://github.com/pulumi/pulumi/blob/46ccb5a22caaece6895d828cd019f3886eb7902a/sdk/go/pulumi/rpc.go#L65-L91

However, rehydrated components (i.e. instances of a component resource that have been recreated from a URN, typically via deserialization of a resource reference) won't have any children from the SDK's perspective. And because of that, they aren't currently kept as a dependency.

This change fixes rehydrated components to be marked as remote components, in the same way as "dependency resources" are marked as remote components, so that they are kept as a dependency.
@justinvp
Copy link
Member Author

Thanks for the feedback! I've addressed your comments.

Copy link
Contributor

@dixler dixler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me. Thanks for updatingmodifying the comments!

@justinvp
Copy link
Member Author

bors merge p=1

@bors
Copy link
Contributor

bors bot commented Mar 24, 2023

Build succeeded:

@bors bors bot merged commit 20ee7fc into master Mar 24, 2023
42 checks passed
@bors bors bot deleted the justin/rehydratedcomp_go branch March 24, 2023 22:15
bors bot added a commit that referenced this pull request Mar 24, 2023
12498: Freeze version for 3.59.1 release r=justinvp a=justinvp

#12494 is in the merge queue. Once it merges and this PR is approved, I'll merge this PR.

Co-authored-by: Justin Van Patten <jvp@justinvp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants