fix terraform resource paths, use templatefile, update metal version

displague commented 3 years ago

Description

Updates the EM provider to the latest
Uses EM Metros
Uses path.module in all local file references (this is a TF best practice and is required in order to consume this config as a module)
replaces use of the deprecated template data source with the templatefile function
adds the .terraform.lock.hcl file, another TF best practice
use cloud-config (avoiding ssh provisioners)
use metal_port resources instead of metal_device_network_type and metal_vlan_port_attachment resources (https://registry.terraform.io/providers/equinix/metal/latest/docs/guides/network_types).

Why is this needed

I needed these changes to pursue work in https://github.com/displague/terraform-metal-tinkerbell/pull/1 The project itself creates a Terraform module (which could be submitted to the Terraform registry) by thinly wrapping the sandbox project configs.

In the linked PR, I am attempted to use the Tinkerbell Terraform Provider to work with the created provisioner node. I do not want to do anything but terraform apply to have an EM node execute a Tinkerbell workflow.

Fixes: #

How Has This Been Tested?

Deployed via https://github.com/displague/terraform-metal-tinkerbell

How are existing users impacted? What migration steps/scripts do we need?

Users will have to terraform init -upgrade, which is typical. Users may also need a newer version of Terraform if they are running TF <0.12.

Checklist:

I have:

[ ] updated the documentation and/or roadmap (if required)
[ ] added unit or e2e tests
[ ] provided instructions on how to upgrade

jacobweinstock commented 3 years ago

Hey @displague, #90 just landed and has changed the sandbox significantly. Would you mind having a look at the changes and confirm if this change is still something you'd like to see?

displague commented 3 years ago

@jacobweinstock Yes, this is still needed. I've updated the PR (and the PR description) and verified that this terraform config works when run from the sandbox/deploy/terraform directory.

displague commented 3 years ago

@jacobweinstock I'll push up a fix to work around some Terraform limitations here. This works as-is on the second apply because compose.zip is created in the first failing pass. I was looking at this problem specifically but missed it in my testing.

The work-around will involve a locals block which will insert a transitive phase in the planning, requiring the compose.zip to be created before attempting to use it. (the depends_on is not sufficient in this case)

displague commented 3 years ago

@jacobweinstock pushed. Thanks for taking a look!

displague commented 3 years ago

@jacobweinstock looks like that last push resulted in a clean apply but an empty /root/sandbox/compose dir. I'll have to take another look at how to work with or around these Terraform behaviors.

jacobweinstock commented 3 years ago

Hey @displague, anything I can do to help with this?

displague commented 3 years ago

@jacobweinstock was there talk of dismantling sandbox?

jacobweinstock commented 3 years ago

@jacobweinstock was there talk of dismantling sandbox?

No, there wasn't. Why do you ask?

nshalman commented 2 years ago

@jacobweinstock was there talk of dismantling sandbox?

No, there wasn't. Why do you ask?

@displague Does that answer help with moving this PR forward?

nshalman commented 2 years ago

Is there a desire for someone else to pick this up?

displague commented 2 years ago

@nshalman @jacobweinstock This PR is working as expected now. Please give it another look.

displague commented 2 years ago

@jacobweinstock Rebased and updated

tinkerbell / playground