Streamlining reusing parameters on subsequent actions

[carolynvs]

Porter has the ability to remember a parameter that you used previously and then use it in subsequent actions. For example, remembering the value of the "resource-group" parameter that you specified on install and using it automatically for upgrade, or persisting tfstate for the terraform mixin.

The problem is that it's not clear how to configure it. We could streamline the common scenarios for persisting state with a bundle and make those configurations more straightforward.

I want to remember the value used for the parameter during install and reuse it later.

In the example below, here's what's required to persist a parameter and reuse it later. You need an output declared, and then a parameter that sources the output:

parameters:
- name: resource-group
  type: string
  path: /cnab/app/resource-group
  source:
    output: resource-group
  applyTo:
  - install
  - upgrade
  - uninstall

outputs:
- name: resource-group
  path: /cnab/app/resource-group

In the case where we just want to remember a parameter value from action to action, here is a potential shorthand:

parameters:
- name: resource-group
  type: string
  persistent: true

The previous syntax would still be supported for advanced scenarios but for the most common just marking a parameter as persistent is a nice simplification. Under the hood we will expand that setting out to the bundle.json configuration used today.

When a parameter is sourced from an output, porter automatically appends "applyTo: " when applyTo isn't specified. That is why I specified applyTo in the example above, to make the parameter visible to the user for install, and require that it's specified.

I want to generate a value during install and then reuse it later

In this example, we are persisted the tfstate generated by the bundle during install and reusing it for subsequent steps:

parameters:
  - name: tfstate
    type: file
    path: /cnab/app/terraform/terraform.tfstate
    source:
      output: tfstate

outputs:
  - name: tfstate
    type: file
    path: /cnab/app/terraform/terraform.tfstate

I suggest that we introduce a new section state to let the bundle author say that something needs to be kept track of, but not expose it to the user as something that they should be setting themselves.

state:
  - name: tfstate
    path: /cnab/app/terraform/terraform.tfstate

So instead of having the user think about tfstate as a parameter or output when really they never pass it in, or will care about it as an output, we declare it as bundle state. Data that we want to have available to the bundle, but the user doesn't interact with.

What do you think? Does this line up with how you think about reusing values between bundle actions?

[vdice]

A quick vote of support for both strategies -- the proposed refinements bring clarity and improved UX over the current situation. Like you've said, users can always go the current/classic route to have fine-grained control over which parameters have which sources and with certain action applicability, but I definitely think the persistent: true and state approaches cover a vast majority of use cases and the syntax is far easier to quickly comprehend.

[carolynvs]

When we do this, it would be great to clarify what's really required in porter explain. Required right now is a 1:1 translation from the cnab parameter required flag. But what it should indicate is "does the user need to supply this value?". In both cases above (and in general for parameters with a parameter source) is that they are not required. In fact they probably shouldn't be displayed in some circumstances.

For example in the bundle below, the tfstate and tfvars files are state parameters. They shouldn't be displayed at all.

porter explain -r carolynvs/tabby-cat-demo:v0.1.0
Name: tabby-cat-demo
Description: Demo bundle that deploys TabbyCat to Azure
Version: 0.1.0
Porter Version: v1.0.0-alpha.1-66-gf801cc84

No credentials defined

Parameters:
Name       Description                                     Type     Default     Required   Applies To
location   Azure location where the database is deployed   string   centralus   false      install
tfstate                                                    file     <nil>       true       uninstall,upgrade
tfvars                                                     file     <nil>       true       uninstall,upgrade

There is probably more we can do with the default column that either replaces required all together, or just makes things more clear when Porter can come up with a value on its own. The required column may be redundant if the answer is "if default doesn't have a value it's required". Something to think about.

[carolynvs]

I'd also like to see mixins be able to declare any state that they need so that the bundle author doesn't even need to declare state files generated by a mixin. The terraform mixin can tell porter about its state and add it automatically.

[carolynvs]

I've been working more with terraform lately in a bundle and am realizing that it doesn't work well in the install action due to how parameter sources work. If install fails, and you need to repeat it, the tfstate file isn't injected back into the bundle because the output wasn't captured from the failed run and isn't reinjected for the second try.

When we make state from previous actions into state, it can make it really hard to recover from a failed bundle run. If install fails and doesn't complete creating all its outputs, and those outputs are used as parameters to other actions, then you can get into a state where you can uninstall the bundle. For example, if I make a generated kubeconfig a parameter, then if install fails before kubeconfig is output, then when I try to uninstall (because everything is borked), the missing kubeconfig prevents uninstall from executing.

Calling state... state, not conflating it with parameters, and capturing the generated state regardless of action outcome should help us be more forgiving with missing state. Ideally steps should be able to skip or nope out when their required state isn't present. So the uninstall step would skip uninstalling a helm release when the kubeconfig isn't present. This would help us uninstall a partial install.