ref(*): update mixin #3
Conversation
pkg/terraform/install_test.go
Outdated
|
|
||
| assert.Equal(t, "Install MySQL", step.Description) | ||
| assert.Equal(t, true, step.Init) |
There was a problem hiding this comment.
Is it safe to unconditionally init terraform every time? I don't know enough about it, so I'm just asking for my own curiosity.
There was a problem hiding this comment.
Good question. I think it's becoming apparent that we need to init every time, now that I'm reconciling terraform with how porter (or, generally, CNAB) bundles currently operate. If I'm not mistaken, each porter/CNAB action is a fresh container run using the invocation image, with fresh context, and therefore in tf-land, we do need to init each time.
For maintaining/re-using state, I've created #4 for use of remote backends. I'm not currently aware of a technique to re-use state in a CNAB context locally, yet. We need to research this.
Which is to say, I think at this point, we probably need to assume that every action needs to init first (and, thus, the utility of allowing user to specify whether or not to via the porter manifest is not really useful/applicable.)
There was a problem hiding this comment.
Oh so init is a client side action?
There was a problem hiding this comment.
init downloads all terraform modules required to run
There was a problem hiding this comment.
Hmmm, what if we ran it as part of the docker build instead of at runtime? Is that an option? Once we start passing in the full porter manifest to the mixin, it will know where the working directory is. So we can't do it in this PR, but soon.
There was a problem hiding this comment.
Oh, good idea! This might definitely be the best way to go, once porter/mixins support it. I'll file a ticket to revisit.
pkg/terraform/install_test.go
Outdated
| }, | ||
| // TODO: test Init: true (requires main test helper refactor to support one action issuing multiple commands) |
There was a problem hiding this comment.
oh yeah we really need to do this, Jeremy has an issue open for it getporter/porter#231
pkg/terraform/install_test.go
Outdated
|
|
||
| assert.Equal(t, "Install MySQL", step.Description) | ||
| assert.Equal(t, true, step.Init) |
There was a problem hiding this comment.
init downloads all terraform modules required to run
|
Merge at will! 🚢 |
* update with most recent porter <-> mixin data protocol * add new terraform options/args to actions
Signed-off-by: Vaughn Dice <vadice@microsoft.com>
|
Thanks @carolynvs ! The only blocker now for this PR is getporter/porter#266 (or a similar solution), as the bulk of these unit tests rely on the support mentioned in that PR/ticket. |
|
Ok, needed to run |
In general, this PR gets this mixin up to speed and in a working state, tested by the integration test added in getporter/porter#261