fix terraform resource paths, use templatefile, update metal version #96

displague · 2021-08-13T23:47:37Z

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 displague/terraform-metal-tinkerbell#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

displague · 2021-08-13T23:49:45Z

deploy/terraform/main.tf

@@ -52,27 +52,27 @@ resource "null_resource" "tink_directory" {
  }

  provisioner "file" {
-    source      = "../../setup.sh"
+    source      = "${path.module}/../../setup.sh"


I was doubtful that this would work through the ../../, but it did:

https://github.com/displague/terraform-metal-tinkerbell/blob/main/main.tf#L2

root@tink-provisioner:~# ls -la /root/tink/ total 40 drwxr-xr-x 3 root root 4096 Aug 13 22:29 . drwx------ 6 root root 4096 Aug 13 23:24 .. -rwxr-xr-x 1 root root 771 Aug 13 22:29 current_versions.sh drwxr-xr-x 6 root root 4096 Aug 13 22:31 deploy -rwxr-xr-x 1 root root 2800 Aug 13 22:29 generate-env.sh -rw-r--r-- 1 root root 6 Aug 13 22:29 .nat_interface -rwxr-xr-x 1 root root 13594 Aug 13 22:29 setup.sh

jacobweinstock · 2021-08-27T04:28:33Z

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 · 2021-09-15T11:03:59Z

@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.

jacobweinstock

Hey @displague, thanks for updating this. I'm not able to get this working. I'm seeing this below. Any ideas?

❯ terraform apply  
╷
│ Error: Invalid function argument
│ 
│   on main.tf line 102, in data "cloudinit_config" "setup":
│  102:       COMPOSE_ZIP = filebase64("${path.module}/compose.zip")
│     ├────────────────
│     │ path.module is "."
│ 
│ Invalid value for "path" parameter: no file exists at ./compose.zip; this function works only with files that are distributed as part of the configuration source code, so if this file will be created by a resource in this configuration you must instead obtain this result from an attribute of that resource.
╵

 ~/repos/tinkerbell/sandbox/deploy/terraform  pr/displague/96:terraform-paths                                                                                                                                                                                                                                                                                                                                      ▼  09:26:33 AM 
❯ terraform version
Terraform v1.0.4
on darwin_amd64
+ provider registry.terraform.io/equinix/metal v3.1.0
+ provider registry.terraform.io/hashicorp/archive v2.2.0
+ provider registry.terraform.io/hashicorp/cloudinit v2.2.0
+ provider registry.terraform.io/hashicorp/null v2.1.2
+ provider registry.terraform.io/hashicorp/template v2.1.2

Your version of Terraform is out of date! The latest version
is 1.0.6. You can update by downloading from https://www.terraform.io/downloads.html

displague · 2021-09-17T13:42:44Z

@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 · 2021-09-17T13:54:21Z

@jacobweinstock pushed. Thanks for taking a look!

displague · 2021-09-17T14:09:43Z

@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 · 2021-10-05T15:38:55Z

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

displague · 2021-10-05T15:45:25Z

@jacobweinstock was there talk of dismantling sandbox?

jacobweinstock · 2021-10-05T17:10:10Z

@jacobweinstock was there talk of dismantling sandbox?

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

nshalman · 2021-10-19T15:49:29Z

@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 · 2021-11-30T16:33:27Z

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

displague · 2021-12-06T21:37:44Z

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

cprivitere

We should update the documentation to reference metro instead of facility, but looks good otherwise. I'll be making at least some of those documentation changes in the next few days.

jacobweinstock

Hey @displague, really sorry for the terribly delayed response. This looks good. Only a doc update suggestion.

When I tested this I needed to change lines 45 and 97 in the terraform quickstart doc.
metal device reboot -i $(terraform show -json | jq -r '.values.root_module.resources[1].values.id') -> metal device reboot -i $(terraform show -json | jq -r '.values.root_module.resources[3].values.id')

- fix terraform resource paths (use path.module) - use cloud-config (avoiding ssh provisioners) - use templatefile (template_file resource is deprecated) - update metal version Signed-off-by: Marques Johansson <mjohansson@equinix.com>

Signed-off-by: Marques Johansson <mjohansson@equinix.com>

deploy/terraform/cloud-config.cfg

Signed-off-by: Marques Johansson <mjohansson@equinix.com>

displague · 2022-03-22T18:17:37Z

@jacobweinstock Rebased and updated

mmlb · 2022-03-22T19:19:52Z

deploy/terraform/main.tf

+data "archive_file" "compose" {
+  type        = "zip"
+  source_dir  = "${path.module}/../compose"
+  output_path = "${path.module}/compose.zip"
 }


must we go with zip here to make this a module? Having to install unzip makes for a runtime failure (wan network issue for example...) vs using local-exec & tar the failure will happen early and solved "once".

zip is what is available today in the Terraform module. Taking advantage of what is available in Terraform reduces our dependence on local compression tools.

https://registry.terraform.io/providers/hashicorp/archive/latest/docs/data-sources/archive_file

hashicorp/terraform-provider-archive#29

We have other network dependencies that would fail with zip, be it package installs or Tink configuration.

zip is what is available today in the Terraform module. Taking advantage of what is available in Terraform reduces our dependence on local compression tools.

But with a remote/additional dependence on ubuntu mirrors for every provision. Its a once/now vs always/later tradeoff, once is better than always and now is better than later.Its very unlikely that tar is not around, and we can probably lean on git archive if that were to be an issue anyway, it has built in support for tar (plain tar, git requires gzip for gz compression).

registry.terraform.io/providers/hashicorp/archive/latest/docs/data-sources/archive_file

hashicorp/terraform-provider-archive#29

hashicorp/terraform-provider-archive#29 (comment) is pretty damning though(its likely not going to get tar.gz support and may go away all together). We'll always have to require installing unzip, even after this ubuntu goes EOL and the mirror url changes and we can't actually install (long time from now yes, but still...).

We have other network dependencies that would fail with zip, be it package installs or Tink configuration.

We don't install any other packages except for zip right? I mean specifically in userdata/cloud-config. I find it much easier to see status/track progress and debug issues using the remote-exec provisioner instead of userdata/cloud-init. I get rid of userdata completely in #126 because of this.

zip is the only hang up I have for this PR, it feels like a "it ain't broke so don't fix" (afaik at least). That being said I don't want to delay the rest of the PR further over issues that aren't super likely to happen or would be minor annoyances. So I'll 👍 it, you can keep the zip in or remove up to you :D

mmlb · 2022-03-30T16:29:02Z

deploy/terraform/main.tf

+data "archive_file" "compose" {
+  type        = "zip"
+  source_dir  = "${path.module}/../compose"
+  output_path = "${path.module}/compose.zip"
 }


zip is what is available today in the Terraform module. Taking advantage of what is available in Terraform reduces our dependence on local compression tools.

But with a remote/additional dependence on ubuntu mirrors for every provision. Its a once/now vs always/later tradeoff, once is better than always and now is better than later.Its very unlikely that tar is not around, and we can probably lean on git archive if that were to be an issue anyway, it has built in support for tar (plain tar, git requires gzip for gz compression).

registry.terraform.io/providers/hashicorp/archive/latest/docs/data-sources/archive_file

hashicorp/terraform-provider-archive#29

hashicorp/terraform-provider-archive#29 (comment) is pretty damning though(its likely not going to get tar.gz support and may go away all together). We'll always have to require installing unzip, even after this ubuntu goes EOL and the mirror url changes and we can't actually install (long time from now yes, but still...).

We have other network dependencies that would fail with zip, be it package installs or Tink configuration.

We don't install any other packages except for zip right? I mean specifically in userdata/cloud-config. I find it much easier to see status/track progress and debug issues using the remote-exec provisioner instead of userdata/cloud-init. I get rid of userdata completely in #126 because of this.

zip is the only hang up I have for this PR, it feels like a "it ain't broke so don't fix" (afaik at least). That being said I don't want to delay the rest of the PR further over issues that aren't super likely to happen or would be minor annoyances. So I'll 👍 it, you can keep the zip in or remove up to you :D

displague commented Aug 13, 2021

View reviewed changes

displague marked this pull request as ready for review August 13, 2021 23:52

displague force-pushed the terraform-paths branch from 1febe6d to a5aad7a Compare September 15, 2021 11:00

jacobweinstock reviewed Sep 15, 2021

View reviewed changes

displague marked this pull request as draft September 17, 2021 14:09

displague force-pushed the terraform-paths branch 2 times, most recently from e0f9a5a to 822c24d Compare December 6, 2021 20:16

displague marked this pull request as ready for review December 6, 2021 21:35

displague force-pushed the terraform-paths branch from 822c24d to 644ecf7 Compare December 6, 2021 22:51

cprivitere approved these changes Dec 6, 2021

View reviewed changes

displague force-pushed the terraform-paths branch from 644ecf7 to 4e4decc Compare December 14, 2021 12:08

jeremytanner requested a review from jacobweinstock January 11, 2022 16:36

jacobweinstock force-pushed the terraform-paths branch from 78ebcf1 to 5178205 Compare February 4, 2022 21:47

jacobweinstock approved these changes Feb 7, 2022

View reviewed changes

displague added 2 commits March 22, 2022 14:09

make terraform config reusable as a module

af4a9ce

- fix terraform resource paths (use path.module) - use cloud-config (avoiding ssh provisioners) - use templatefile (template_file resource is deprecated) - update metal version Signed-off-by: Marques Johansson <mjohansson@equinix.com>

fix terraform compose.zip creation planning/timing

83ac80a

Signed-off-by: Marques Johansson <mjohansson@equinix.com>

displague mentioned this pull request Mar 22, 2022

Terraform love #126

Merged

3 tasks

displague commented Mar 22, 2022

View reviewed changes

deploy/terraform/cloud-config.cfg Outdated Show resolved Hide resolved

update terraform configuration to use metal_ports

48b06fa

Signed-off-by: Marques Johansson <mjohansson@equinix.com>

displague force-pushed the terraform-paths branch from 5178205 to 48b06fa Compare March 22, 2022 18:12

update Terraform quickstart guide to reboot correct node

a1c19d3

Signed-off-by: Marques Johansson <mjohansson@equinix.com>

mmlb reviewed Mar 22, 2022

View reviewed changes

mmlb approved these changes Mar 30, 2022

View reviewed changes

displague merged commit 467e0b5 into tinkerbell:main Mar 30, 2022

displague deleted the terraform-paths branch March 30, 2022 16:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix terraform resource paths, use templatefile, update metal version #96

fix terraform resource paths, use templatefile, update metal version #96

displague commented Aug 13, 2021 •

edited

displague Aug 13, 2021 •

edited

jacobweinstock commented Aug 27, 2021

displague commented Sep 15, 2021

jacobweinstock left a comment

displague commented Sep 17, 2021

displague commented Sep 17, 2021

displague commented Sep 17, 2021 •

edited

jacobweinstock commented Oct 5, 2021

displague commented Oct 5, 2021

jacobweinstock commented Oct 5, 2021

nshalman commented Oct 19, 2021

nshalman commented Nov 30, 2021

displague commented Dec 6, 2021

cprivitere left a comment

jacobweinstock left a comment

displague commented Mar 22, 2022

mmlb Mar 22, 2022 •

edited

displague Mar 30, 2022

displague Mar 30, 2022

mmlb Mar 30, 2022

mmlb Mar 30, 2022

fix terraform resource paths, use templatefile, update metal version #96

fix terraform resource paths, use templatefile, update metal version #96

Conversation

displague commented Aug 13, 2021 • edited

Description

Why is this needed

How Has This Been Tested?

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

Checklist:

displague Aug 13, 2021 • edited

Choose a reason for hiding this comment

jacobweinstock commented Aug 27, 2021

displague commented Sep 15, 2021

jacobweinstock left a comment

Choose a reason for hiding this comment

displague commented Sep 17, 2021

displague commented Sep 17, 2021

displague commented Sep 17, 2021 • edited

jacobweinstock commented Oct 5, 2021

displague commented Oct 5, 2021

jacobweinstock commented Oct 5, 2021

nshalman commented Oct 19, 2021

nshalman commented Nov 30, 2021

displague commented Dec 6, 2021

cprivitere left a comment

Choose a reason for hiding this comment

jacobweinstock left a comment

Choose a reason for hiding this comment

displague commented Mar 22, 2022

mmlb Mar 22, 2022 • edited

Choose a reason for hiding this comment

displague Mar 30, 2022

Choose a reason for hiding this comment

displague Mar 30, 2022

Choose a reason for hiding this comment

mmlb Mar 30, 2022

Choose a reason for hiding this comment

mmlb Mar 30, 2022

Choose a reason for hiding this comment

displague commented Aug 13, 2021 •

edited

displague Aug 13, 2021 •

edited

displague commented Sep 17, 2021 •

edited

mmlb Mar 22, 2022 •

edited