Add example with templated and bind mounted config file #68
Conversation
karlkfi
force-pushed
the
feature/config-file-example
branch
from
feaa4a8 to
49a1c5d
Compare
|
Added fixture and integration tests. |
| default = "8080" | ||
| } | ||
|
|
||
| variable "restart_policy" { |
There was a problem hiding this comment.
FYI in general we shouldn't really need so many variables for examples - often they should just be hardcoded. I know many of the existing examples overuse variables, but the variables should really only be for things which actually need to change like project ID.
There was a problem hiding this comment.
I just copied it from instance_with_attached_disk. Should I remove it here or in both places?
Unlike instance_with_attached_disk, I set a default here so it doesn't actually need to be defined.
There was a problem hiding this comment.
Just because you started with an example doesn't mean you need to copy it exactly - try to make it better as you add new ones at least. Either remove it here or both places if you want to do more.
| http_address = attribute('http_address') | ||
| port = attribute('http_port') | ||
|
|
||
| # Test disabled due to flappiness - because this makes live network requests, it is prone to sporadic failure. |
There was a problem hiding this comment.
Let's just remove this test if it's too flaky.
There was a problem hiding this comment.
This test was copied from instance_with_attached_disk, which exposes many of the same outputs specifically to enable this test. I haven't actually run this test at all. I assumed that either we should have it in both places or remove it in both places, but I wasn't sure if there was an issue in the backlog to clean up the technical debt.
I could probably add a retry loop to make it less flaky, but I don't know how to quickly judge the flakiness before PR merge.
Do you want me to remove it here AND remove the outputs it needed, or also remove it from the other example test(s), or try to make it more robust (and take longer to fail)?
There was a problem hiding this comment.
Incremental improvement, try to leave the codebase better than you found it. You can leave the old one but no need to copy over the commented code here. (Also go ahead and remove the outputs.)
Fixes a bug with terraform docs generation
karlkfi
force-pushed
the
feature/config-file-example
branch
from
0b8e48d to
715ccf6
Compare
|
Removed the unused www test (copied tech debt). |
| - `terraform apply` to apply the infrastructure changes | ||
| - `terraform destroy` to tear down the created infrastructure | ||
|
|
||
| ## Debugging |
There was a problem hiding this comment.
Thanks for adding this, nice touch!
Fixes #66