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

Cannot use Input<> with ChartOpts #162

Closed
pgavlin opened this issue Aug 27, 2018 · 7 comments
Closed

Cannot use Input<> with ChartOpts #162

pgavlin opened this issue Aug 27, 2018 · 7 comments
Assignees
Labels
area/helm p1 Bugs severe enough to be the next item assigned to an engineer
Milestone

Comments

@pgavlin
Copy link
Member

pgavlin commented Aug 27, 2018

e.g. the outputs of another resource cannot be used to fill in a Helm chart's parameters.

@lukehoban lukehoban added this to the 0.17 milestone Aug 27, 2018
@hausdorff hausdorff modified the milestones: 0.17, 0.18 Sep 5, 2018
@hausdorff
Copy link
Contributor

hausdorff commented Sep 5, 2018

This turns out to be harder than we had thought -- the values in chart.values needs to be recursively resolved expanded before it is written to some temporary file, so that the helm CLI can pick it up. Let's move to 0.18, as this will not block adoption.

@hausdorff hausdorff changed the title Cannot use Input<> with Chart.values Cannot use Input<> with ChartOpts Sep 6, 2018
@hausdorff
Copy link
Contributor

Let's expand the scope of this issue to make all ChartOpts of type pulumi.Input

@hausdorff
Copy link
Contributor

Let's be sure to resolve #147 as part of this.

@hausdorff
Copy link
Contributor

Looking closer, this seems like it will need to involve a break to the API:

  1. If values has a pulumi.Input somewhere inside, we'll need to wait for that to resolve before shelling out to the helm CLI too.
  2. Since we can't shell out to helm until after all the inputs resolve, the underlying list of resources will need to become a pulumi.Output, which in turn implies that getResources will need to return a pulumi.Output as well.
  3. This breaks anyone calling it, as they will have to resolve the output before they can use it.

@joeduffy
Copy link
Member

joeduffy commented Oct 8, 2018

@hausdorff Any progress on this?

hausdorff added a commit that referenced this issue Oct 16, 2018
hausdorff added a commit that referenced this issue Oct 16, 2018
@hausdorff
Copy link
Contributor

After some consideration, @lukehoban and I now believe that any approach that requires any subset of ChartOpts fields to be Input<T> will break preview. We believe there is no remediation for this problem, and we believe this flaw is serious enough to not proceed with this feature.

Our reasoning:

  • We need to call pulumi.output on ChartOpts before we shell out to the helm CLI in order to resolve all the values that we'd pass to the Chart.
  • This implies that Chart#resources needs to be type Output<T>, since in general we have no ability to statically determine what resources will be output by helm's template expand without running it on the fully-resolved values.
  • This, in turn, implies that preview could (and, we believe, very often will) generate a preview that is drastically different from what will be executed, as we are essentially generating those resources inside a call to .apply.

At the same time, we believe this issue will come up a lot, so I'll be playing around with some other ideas about how we can smooth this flow out in the near future.

I'll leave the issue open for other input.

@lukehoban lukehoban modified the milestones: 0.18, 0.19 Oct 18, 2018
@lukehoban lukehoban assigned pgavlin and unassigned hausdorff Nov 8, 2018
@pgavlin
Copy link
Member Author

pgavlin commented Nov 14, 2018

Fixed in #241.

@pgavlin pgavlin closed this as completed Nov 14, 2018
@infin8x infin8x added the p1 Bugs severe enough to be the next item assigned to an engineer label Jul 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm p1 Bugs severe enough to be the next item assigned to an engineer
Projects
None yet
Development

No branches or pull requests

5 participants