fix: allow this module to be used with a count argument
#47
Conversation
ctreatma
force-pushed
the
no-legacy-module
branch
2 times, most recently
from
931b76e
to
0c56d28
Compare
examples/deploy/outputs.tf
Outdated
| @@ -0,0 +1,4 @@ | |||
| output "nutanix_outputs" { | |||
| description = "Outputs of the Nutanix module" | |||
| value = module.nutanix | |||
There was a problem hiding this comment.
The README example of outputs will also need to be revised
ctreatma
changed the title
count argument
…le module directory
ctreatma
force-pushed
the
no-legacy-module
branch
from
0c56d28
to
d2aa3bd
Compare
| @@ -52,6 +52,12 @@ To accommodate deployment requirements, this module will create: | |||
|
|
|||
| You'll need [Terraform installed](https://developer.hashicorp.com/terraform/install) and an [Equinix Metal account](https://deploy.equinix.com/developers/docs/metal/identity-access-management/users/) with an [API key](https://deploy.equinix.com/developers/docs/metal/identity-access-management/api-keys/). | |||
|
|
|||
| The terraform configuration for deploying a proof-of-concept Nutanix cluster is in `examples/deploy`. Start by changing to that directory: | |||
There was a problem hiding this comment.
This sentence reads like "the princess is in another castle."
We could move the root .tf files into a module directory and state that this is a reusable module (link to deploy, link to registry for this module, link to reusable TF module documentation) and state that examples of that module are in examples, similar to the Fabric and NE (upcoming) TF Modules.
Doing so, the module and the example would inherit some of this README.md. What remains in the root would be significantly lighter, essentially, the princess is in another castle.
🍄 🏰 🎆
This tips my hat to how I'm currently thinking about equinix-labs/terraform-equinix-labs#47
(^ just for consideration - not review blocking)
There was a problem hiding this comment.
I think the dissonance of pointing the user at a different directory in order to run the thing is another indication that this sort of indirection--there's a module, but to actually run it you need to use the wrapper over here / you ran this config, but actually the interesting part was that config underneath--isn't desirable in our example/proof-of-concept modules. Tucking the actual module code in a subdirectory still leaves the user digging through layers to find the information they really want, and the only reason we really need to do that is do allow the module to be dynamically enabled in the Labs repo.
|
I converted this to a draft for now since it's not clear if the work done here runs counter to the purpose of our proof-of-concept modules. Once we have consensus on how the modules we deliver should be structured we can mark this as ready for review again. |
| @@ -154,7 +154,7 @@ resource "null_resource" "wait_for_firstboot" { | |||
| } | |||
|
|
|||
| provisioner "remote-exec" { | |||
| script = "scripts/firstboot-check.sh" | |||
| script = "${path.module}/scripts/firstboot-check.sh" | |||
We can't include this module in equinix-labs/terraform-equinix-labs if it includes explicit provider configuration because equinix-labs/terraform-equinix-labs uses a
countfor each of its submodules to enable users to opt in to any combination of those submodules. This PR removes the explicit provider configuration from the module itself and adds an example inexamples/deploythat configures the provider and can be used to run the now-reusable module. This will require updating the README to direct users to run theexamples/deploymodule; I haven't done that in this PR because #44 is still in flight.