pulumi / pulumi

Pulumi - Infrastructure as Code in any programming language 🚀
https://www.pulumi.com
Apache License 2.0
21.74k stars 1.12k forks source link

registerOutputs usage is not clear #2653

Open melbourne2991 opened 5 years ago

melbourne2991 commented 5 years ago

I'm a bit confused about the registerOutputs call and what actually is considered an output. The current documentation says this:

Components can define their own properties using registerOutputs. The Pulumi engine uses this information to display the logical outputs of the component. The call to registerOutputs also tells Pulumi that the resource is done registering children and should be considered fully constructed, so it is recommended that this method be called in all components even if no outputs need to be registered.

That seems to imply that to expose properties you should use registerOutputs. But looking at the source code there are many instances of properties being exposed, yet registerOutputs is called with nothing, or an empty object:

https://github.com/pulumi/pulumi-awsx/blob/88f9bbc90758b08515945d0b93c6cf0a3a24e481/nodejs/awsx/ec2/internetGateway.ts

It seems to me registerOutputs does two things, it signifies that the resource is done being created (as the docs say) and IF you pass something to it and that resource is a top level export Pulumi will only display the registered output properties in the cli/ui.

If passing outputs to it is purely a means of exposing data to the UI/CLI and has no bearing on anything else it may be helpful for the docs to be a bit clearer on that. Maybe rewording it like this (assuming my understanding is correct):

Components can expose properties to the UI and CLI by calling registerOutputs. The call to registerOutputs also tells Pulumi that the resource is done registering children and should be considered fully constructed, so it is recommended that this method be called in all components even if no outputs need to be registered.

swgillespie commented 5 years ago

I'd say - it's not even clear to us what the purpose of registerOutputs is!

At this point, it's basically only a UI gesture. The CLI interprets "registerOutputs" to mean "this resource has completed creation" and transitions the "creating..." text to "created" (and "updating..." to "updated", etc.). It does store the "outputs" in that resource's state, but state for component resources is basically meaningless anyway.

I can't speak for everyone but I think that registerOutputs is a bit of a design wart. We sort-of need it, because this is how stack outputs are implemented, but they don't make any sense at all for component resources other than Stack. @CyrusNajmabadi has taken to passing the empty object to registerOutputs unconditionally because it basically doesn't mean anything.

CyrusNajmabadi commented 5 years ago

@swgillespie Beat me too it. We definitely need to update our docs here. Our current thinking is the following:

  1. registerOutputs is needed for ComponentResources primarily as a UI affordance to let code tell the engine "the resource is finished constructing, let the user know that's teh case". It's optional to call, but nice to do. If you miss it you'll just see things like "creating..." in the UI until the program finally finishes. But otherwise, there is no ill effects.
  2. We do have a single place in the pulumi stack where we truly call 'registerOutputs' for a resource to expose actual 'output' data. As Sean mentions, it's for the root 'Stack' object, and it's how we actually do things like allow the Stack to report back all the exports of the app module.

By and large, we now think of almost every component resource (except the root 'Stack') as being just an 'aggregation/parenting' concept for other resources. Because they don't actually become a true physical resource in your cloud, and because they don't go through a 'provider' to actually interpret, they don't actually have the concept of either inputs or outputs. As such, we don't even pay attention to inputs on the way in (the base 'ComponentResource' class just ignores them now, and only has htem in the signature for back compat purposes), and we've decided that all-but-one ComponentResource has no outputs, and always just calls .registerOutputs() purely for the UI.

This is def a documentation problem thouhg. I'm going to keep this issue open to track fixing that up. Thanks for the feedback!

ellismg commented 5 years ago

One thing we should keep in mind (and may influence our stance on what sort of outputs to register from a component) is that we expect to do more work around querying an existing deployment by using it's deployment state file. Having components record interesting outputs may make queries easier to write, by allowing components to aggregate some of their child's outputs that are relevant to users of the components.

icereed commented 4 years ago

Is registerOutputs actually needed for dependency tracking of Pulumi?

boillodmanuel commented 4 years ago

Hello, I came out to the same issue here. :) In order to understand what is registerOutputs used for, I looked at the source code, and I'm surprised that my call to registerOutputs is totally useless since the (undocumented) initialize method (cf #3676) has appeared, at least for nodejs runtime :-)

Indeed, registerOutputs could be called only once and it has already been called by constructor (cf https://github.com/pulumi/pulumi/blob/9abcca345a6c024087b35152b0d6fca92baef191/sdk/nodejs/resource.ts#L782-L800). So, if there is an utility to call registerOutputs, it should be done in initialize method.

Doc to be updated:

Sytten commented 4 years ago

I also came here for the exact same reason, the doc should be updated.

alvis commented 3 years ago

So after a year and a half, it's still not clear about how to use registerOutputs. :/ @lukehoban would you mind giving some hints for us?

RobbieMcKinstry commented 2 years ago

This behavior recently defied my expectations as well.

RobbieMcKinstry commented 2 years ago

I'm writing my first Component in an attempt to reproduce https://github.com/pulumi/pulumi/issues/9704.

I read the documentation for creating Components.

Screen Shot 2022-06-13 at 1 43 03 PM

This line...

Component resources can define their own output properties by using register_outputs .

...in conjunction with the name register_outputs made me comfortable in the notion that fields registered as outputs would be available on instances of the Component, just like with resources.

As a result of the documentation, I wrote this program:

import * as pulumi from "@pulumi/pulumi";
import {Command} from "@pulumi/command/local";

class MyLS extends pulumi.ComponentResource {
    files: any;

    constructor(name: string, opts: any) {
        super("pkg:index:MyLS", name, {}, opts);
        const child_cmd = new Command(`${name}-child`, {
          create: "ls",
        }, {parent: this});

        // Expecting this function call to set the property "files" to stdout.
        this.registerOutputs({
          files: child_cmd.stdout,
        });
    }
}

const ls = new MyLS("my-component", {});
export const ls_output = ls.files;

I expected to see an output named ls_output with the results of my local Command. However, the value ls.files is undefined, confirmed by adding console.log(ls_output); at the end of the program. As I see it, this.registerOutputs implies the caller this will has an output files set after the function call. I think the naming is very misleading.

alex-hunt-materialize commented 2 years ago

It seems like registerOutputs would be used in the dependency tracking, but that doesn't seem to be the case. Is the purpose of a ComponentResource not to aggregate multiple resources for dependency use?

We want to be able to create some kind of AggregateResource, and list only that AggregateResource in the dependency list of another resource for it to depend on all sub-resources of the AggregateResource.

It is totally unclear from the docs how to do this.

MisinformedDNA commented 2 years ago

@alex-hunt-materialize ComponentResource is the aggregate resource. If you need an output, just set a property and retrieve it as needed. Ignore registerOutputs.

alex-hunt-materialize commented 2 years ago

@MisinformedDNA Thanks for the response. Does this mean we can just set the ComponentResource as the only thing in the dependency list of another resource, and that resource will then depend on all sub-resources in the ComponentResource? I haven't tried this in 6+ months, but I'm pretty sure that didn't work when I tried.

MisinformedDNA commented 2 years ago

@alex-hunt-materialize Yep, that should work. If it doesn't, create a new ticket or post to Slack.

RobbieMcKinstry commented 2 years ago

Just make sure you register each child resource as a member of the parent. Here's a TypeScript example:

class MyComponent extends pulumi.ComponentResource {
    constructor(name, opts) {
        super("pkg:index:MyComponent", name, {}, opts);
        let bucket = new aws.s3.Bucket(
            `${name}-bucket`, 
             {/*bucket args go here...*/}, 
            { parent: this }
        );
    }
}

The line { parent: this } is how the Pulumi engine knows to associate the bucket as a child of MyComponent. That's the "aggregation" step. Otherwise, your bucket will not have its lifecycle tied to the parent IIRC (i.e. destroying MyComponent won't destroy the bucket).

RobbieMcKinstry commented 1 year ago

We recently discussed this issue as a team.

The TL;DR is that we should improve the documentation of this function to clarify what it does and why. Ultimately, it's not required in most cases. However, it's used to update the progress bar in the CLI. Calling it sends a signal to the CLI TUI that this component has finished execution. Otherwise, the TUI will only mark the component has finished once the entire stack is finished.

There's another use case around multi-language components, but that's probably getting too deep in the weeds.

mklueh commented 1 year ago

Just make sure you register each child resource as a member of the parent. Here's a TypeScript example:

class MyComponent extends pulumi.ComponentResource {
    constructor(name, opts) {
        super("pkg:index:MyComponent", name, {}, opts);
        let bucket = new aws.s3.Bucket(
            `${name}-bucket`, 
             {/*bucket args go here...*/}, 
            { parent: this }
        );
    }
}

The line { parent: this } is how the Pulumi engine knows to associate the bucket as a child of MyComponent. That's the "aggregation" step. Otherwise, your bucket will not have its lifecycle tied to the parent IIRC (i.e. destroying MyComponent won't destroy the bucket).

Thank you, I was looking for some kind of registerComponent in the parent ComponentResource, not being aware that you register them the other way around by passing the parent into the child