New Resource: aws_launch_template #2927

kl4w · 2018-01-10T16:18:22Z

To address #2505

I've created an initial LaunchTemplate resource but unsure how to handle versions and would like input/help.

Creating a new launch template is also conceptually creating the first LaunchTemplateVersion.
If a new LaunchTemplateVersion resource is to be created, it seems weird that it may/should also update the defaultVersion and latestVersion attributes of the LaunchTemplate. Deleting a launch template also deletes all existing versions.

I was thinking about combining the concept of LaunchTemplate and LaunchTemplateVersion such that an update of the LaunchTemplate creates a LaunchTemplateVersion. This results in a single LaunchTemplate in the resulting state. The question is how to handle the versioning of the launch template and how to handle the sourceVersion parameter when creating a new LaunchTemplateVersion.

Combining LaunchTemplate and LaunchTemplateVersion also means that it does not allow as much freedom managing versions, and may mean that creating any resource using the LaunchTemplate may have to use the latestVersion.

Another idea that I was throwing around was create both the LaunchTemplate and LaunchTemplateVersion resources but remove the launchTemplateData attribute from the LaunchTemplate. This would mean that the LaunchTemplate is used more as a versioning reference, and the data is always held in the LaunchTemplateVersion resource. However this would also mean that both resources would be required in order to create a useful launch template.

Thoughts/ideas?

danieladams456 · 2018-01-11T19:22:23Z

I don't think having to configure two resources (LaunchTemplate and LaunchTemplateVersion) would be too bad. The main thing I would want to do is be able to use Lambda or some other automated process to make a new version of the Launch Template and update the default version to the new without making a re-run of Terraform think it needs to change something. This would let me do initial infrastructure creation through Terraform and automated maintenance through Lambda.

Having a boolean manageVersions or some other similar method would seem to achieve that. I don't know if Terraform can store attributes in state that are for informational purposes only but not engage on a feedback loop for changing resources based on them? If not, then maybe that boolean would cause version not to be stored in state so Terraform wouldn't see any changes.

bflad · 2018-01-11T19:31:06Z

Terraform core provides a lifecycle configuration block for all resources that allows you to ignore certain attributes you don't want to track: https://www.terraform.io/docs/configuration/resources.html#ignore_changes

e.g. this will create an ECS service with a desired count of 1 (or import with whatever value) and ignore any changes

resource "aws_ecs_service" "example" {
  # other configuration
  desired_count = 1

  lifecycle {
    ignore_changes = ["desired_count"]
  }
}

danieladams456 · 2018-01-11T19:35:49Z

@bflad Is that only for explicitly defined fields or computed values as well?

As long as this feature is implemented in a way that the default version attribute can be ignored with this methodology, that sounds great to me.

kl4w · 2018-01-24T03:14:42Z

Would appreciate some eyes on this PR so far.

make testacc TEST=./aws TESTARGS='-run=TestAccAWSLaunchTemplate_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLaunchTemplate_ -timeout 120m
=== RUN   TestAccAWSLaunchTemplate_basic
--- PASS: TestAccAWSLaunchTemplate_basic (21.18s)
=== RUN   TestAccAWSLaunchTemplate_data
--- PASS: TestAccAWSLaunchTemplate_data (21.27s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	42.490s

kl4w · 2018-03-01T15:48:31Z

I think this is ready to remove to the WIP from this PR

kl4w · 2018-03-02T15:29:23Z

after rebase and added documentation:

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLaunchTemplate_ -timeout 120m
=== RUN   TestAccAWSLaunchTemplate_basic
--- PASS: TestAccAWSLaunchTemplate_basic (12.32s)
=== RUN   TestAccAWSLaunchTemplate_data
--- PASS: TestAccAWSLaunchTemplate_data (10.72s)
=== RUN   TestAccAWSLaunchTemplate_update
--- PASS: TestAccAWSLaunchTemplate_update (19.42s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	42.505s```

kl4w · 2018-03-26T14:12:06Z

rebase again

make testacc TEST=./aws TESTARGS='-run=TestAccAWSLaunchTemplate_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLaunchTemplate_ -timeout 120m
=== RUN   TestAccAWSLaunchTemplate_basic
--- PASS: TestAccAWSLaunchTemplate_basic (10.54s)
=== RUN   TestAccAWSLaunchTemplate_data
--- PASS: TestAccAWSLaunchTemplate_data (10.51s)
=== RUN   TestAccAWSLaunchTemplate_update
--- PASS: TestAccAWSLaunchTemplate_update (18.99s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	40.084s

Currently occasional cpu spike cause the t2 instance to go down and new instances are unable to bootstrap (upgrades) Until hashicorp/terraform-provider-aws#2927 becomes available, move all critical instances on non-metered cpu instances types

bflad

Hi @kl4w! 👋 Thank you very much for this contribution! Overall this pull request is in great shape (don't let the number of comments distract from that 😄 ). You did a fantastic job implementing the crazy nested schema here.

Please check out the feedback and let me know if you have any questions or do not have time to implement any of them.

Aside: For this resource especially, I would also suggest running the acceptance testing with TF_SCHEMA_PANIC_ON_ERROR=1 just in case you missed setting any of the API attributes properly (types, etc.) back into the Terraform state, but it looks like you did a really good job returning errors for d.Set().

bflad · 2018-04-05T20:01:56Z

aws/resource_aws_launch_template.go

+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+const awsSpotInstanceTimeLayout = "2006-01-02T15:04:05Z"


We should use RFC3339 instead of our own custom time layout. 👍

bflad · 2018-04-05T20:03:37Z

aws/resource_aws_launch_template.go

+			"description": {
+				Type:     schema.TypeString,
+				Optional: true,
+				ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {


This can be simplified with:

ValidateFunc: validation.StringLenBetween(0, 255),

bflad · 2018-04-05T20:05:33Z

aws/resource_aws_launch_template.go

+				},
+			},
+
+			"client_token": {


This attribute is not necessary (nor would I recommend) exposing. 👍

bflad · 2018-04-05T20:08:42Z

aws/resource_aws_launch_template.go

+							Optional: true,
+						},
+						"ebs": {
+							Type:     schema.TypeSet,


To simplify the implementation and future maintenance, we can use schema.TypeList here 👍

bflad · 2018-04-05T20:12:12Z

aws/resource_aws_launch_template.go

+			},
+
+			"instance_market_options": {
+				Type:     schema.TypeSet,


To simply the implementation and future maintenance, this can be schema.TypeList 👍

bflad · 2018-04-05T20:47:00Z

aws/resource_aws_launch_template.go

+	s := []interface{}{}
+	for _, v := range m {
+		mapping := map[string]interface{}{
+			"device_name":  *v.DeviceName,


Rather than risk panics on nil dereferencing, we should switch anything using *thing to the appropriate SDK method that will set it to the zero value for the type: e.g. aws.StringValue(v.DeviceName) This should repeated everywhere below.

bflad · 2018-04-05T20:51:33Z

aws/resource_aws_launch_template_test.go

+					resource.TestCheckResourceAttr(resName, "default_version", "1"),
+					resource.TestCheckResourceAttr(resName, "latest_version", "1"),
+				),
+			},


Let's add a TestStep here that imports the resource and ensures the Terraform state matches what is expected 👍

{ ResourceName: resName, ImportState: true, ImportStateVerify: true, },

bflad · 2018-04-05T20:52:00Z

aws/resource_aws_launch_template_test.go

+					resource.TestCheckResourceAttr(resName, "vpc_security_group_ids.#", "1"),
+					resource.TestCheckResourceAttr(resName, "tag_specifications.#", "1"),
+				),
+			},


Let's add a TestStep here that imports the resource and ensures the Terraform state matches what is expected 👍

{ ResourceName: resName, ImportState: true, ImportStateVerify: true, },

bflad · 2018-04-05T20:52:45Z

aws/resource_aws_launch_template_test.go

+					resource.TestCheckResourceAttr(resName, "latest_version", "2"),
+					resource.TestCheckResourceAttrSet(resName, "image_id"),
+				),
+			},


Let's add a TestStep here that imports the resource and ensures the Terraform state matches what is expected 👍

{ ResourceName: resName, ImportState: true, ImportStateVerify: true, },

bflad · 2018-04-05T20:54:24Z

aws/resource_aws_launch_template_test.go

+			}
+		}
+
+		ae, ok := err.(awserr.Error)


This can be simplified with if isAWSErr(err, "InvalidLaunchTemplateId.NotFound", "") { 👍 (replacing the middle with an SDK constant if available)

danieladams456 · 2018-04-06T00:19:48Z

To echo @bflad, thank-you very much! This will definitely be useful.

bflad · 2018-04-13T14:19:02Z

aws/resource_aws_launch_template.go

@@ -277,34 +279,31 @@ func resourceAwsLaunchTemplate() *schema.Resource {
 							Optional: true,
 						},
 						"security_groups": {
-							Type:     schema.TypeSet,


These (security_groups, ipv6_addresses, and ipv4_addresses) will likely need to remain Type: TypeSet (just needed the Set: schema.HashString removed) as the API likely does not return them in a consistent order. We'll find out!

bflad · 2018-04-13T14:20:46Z

aws/resource_aws_launch_template.go

@@ -358,21 +362,19 @@ func resourceAwsLaunchTemplate() *schema.Resource {
 			},

 			"security_group_names": {
-				Type:     schema.TypeSet,


Similar note here about security_group_names and vpc_security_group_ids -- we shall see!

bflad · 2018-04-13T14:22:03Z

aws/resource_aws_launch_template.go

-				"volume_type":           *v.Ebs.VolumeType,
+				"delete_on_termination": aws.BoolValue(v.Ebs.DeleteOnTermination),
+				"encrypted":             aws.BoolValue(v.Ebs.Encrypted),
+				"volume_size":           aws.Int64Value(v.Ebs.VolumeSize),


Any of these aws.Int64Value() calls will need to be wrapped in int(aws.Int64Value()) since Terraform's TypeInt is a regular int

kl4w · 2018-04-13T14:29:46Z

Made modifications based off of the review. Can someone please advise on the import? I'm having test failures after adding the import step verification

Without import

make testacc TEST=./aws TESTARGS='-run=TestAccAWSLaunchTemplate_'
gofmt -w $(find . -name '*.go' |grep -v vendor)
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLaunchTemplate_ -timeout 120m
=== RUN   TestAccAWSLaunchTemplate_basic
--- PASS: TestAccAWSLaunchTemplate_basic (13.15s)
=== RUN   TestAccAWSLaunchTemplate_data
--- PASS: TestAccAWSLaunchTemplate_data (11.08s)
=== RUN   TestAccAWSLaunchTemplate_update
--- PASS: TestAccAWSLaunchTemplate_update (21.39s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws

With import:

make testacc TEST=./aws TESTARGS='-run=TestAccAWSLaunchTemplate_'
gofmt -w $(find . -name '*.go' |grep -v vendor)
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLaunchTemplate_ -timeout 120m
=== RUN   TestAccAWSLaunchTemplate_basic
--- PASS: TestAccAWSLaunchTemplate_basic (14.27s)
=== RUN   TestAccAWSLaunchTemplate_data
--- FAIL: TestAccAWSLaunchTemplate_data (12.74s)
	testing.go:518: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.

		(map[string]string) {
		}


		(map[string]string) (len=5) {
		 (string) (len=12) "monitoring.#": (string) (len=1) "1",
		 (string) (len=20) "monitoring.0.enabled": (string) (len=4) "true",
		 (string) (len=11) "ram_disk_id": (string) (len=12) "ari-a12bc3de",
		 (string) (len=24) "vpc_security_group_ids.#": (string) (len=1) "1",
		 (string) (len=24) "vpc_security_group_ids.0": (string) (len=4) "test"
		}

=== RUN   TestAccAWSLaunchTemplate_update
--- FAIL: TestAccAWSLaunchTemplate_update (23.05s)
	testing.go:518: Step 2 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.

		(map[string]string) {
		}


		(map[string]string) (len=5) {
		 (string) (len=12) "monitoring.#": (string) (len=1) "1",
		 (string) (len=20) "monitoring.0.enabled": (string) (len=4) "true",
		 (string) (len=11) "ram_disk_id": (string) (len=12) "ari-a12bc3de",
		 (string) (len=24) "vpc_security_group_ids.#": (string) (len=1) "1",
		 (string) (len=24) "vpc_security_group_ids.0": (string) (len=4) "test"
		}

FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	50.102s

bflad · 2018-04-13T14:38:00Z

@kl4w generally that means d.Set() is not being called correctly for those attributes during read. 😄

bflad · 2018-04-13T14:40:31Z

aws/resource_aws_launch_template.go

+	d.Set("instance_type", ltData.InstanceType)
+	d.Set("kernel_id", ltData.KernelId)
+	d.Set("key_name", ltData.KeyName)
+	d.Set("monitoring", ltData.Monitoring)


This needs to be updated to err check (since its non-scalar now) as well as correctly handle the new nested structure:

if err := d.Set("monitoring", getMonitoring(ltData.Monitoring)); err != nil { return err }

ahhh makes sense. Thank you. I'll make those changes now.

bflad · 2018-04-13T14:42:33Z

aws/resource_aws_launch_template.go

+	d.Set("kernel_id", ltData.KernelId)
+	d.Set("key_name", ltData.KeyName)
+	d.Set("monitoring", ltData.Monitoring)
+	d.Set("ram_dist_id", ltData.RamDiskId)


Typo: 😅 d.Set("ram_disk_id", ltData.RamDiskId) (FYI you can find these by running the acceptance testing with TF_SCHEMA_PANIC_ON_ERROR=1 as well as the import testing that noticed it)

kl4w · 2018-04-13T19:13:23Z

TF_SCHEMA_PANIC_ON_ERROR=1 make testacc TEST=./aws TESTARGS='-run=TestAccAWSLaunchTemplate_'
gofmt -w $(find . -name '*.go' |grep -v vendor)
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLaunchTemplate_ -timeout 120m
=== RUN   TestAccAWSLaunchTemplate_importBasic
--- PASS: TestAccAWSLaunchTemplate_importBasic (14.62s)
=== RUN   TestAccAWSLaunchTemplate_importData
--- PASS: TestAccAWSLaunchTemplate_importData (15.01s)
=== RUN   TestAccAWSLaunchTemplate_basic
--- PASS: TestAccAWSLaunchTemplate_basic (12.00s)
=== RUN   TestAccAWSLaunchTemplate_data
--- PASS: TestAccAWSLaunchTemplate_data (13.97s)
=== RUN   TestAccAWSLaunchTemplate_update
--- PASS: TestAccAWSLaunchTemplate_update (21.29s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws

bflad · 2018-04-13T19:26:22Z

Do you have a second to update the monitoring documentation as well? Running testing now

bflad · 2018-04-13T20:18:17Z

website/docs/r/launch_template.html.markdown

+
+  key_name = "test"
+
+  monitoring = true


I'll fix this up post merge 👍

ahh thanks. I was going to update the documentation as well on this branch. Do you want me to hold off?

bflad

@kl4w you are about to make a large amount of people happy! Great work on this and thanks for sticking through the crazy schema issues! 🚀 💯 🥇

5 tests passed (all tests)
=== RUN   TestAccAWSLaunchTemplate_data
--- PASS: TestAccAWSLaunchTemplate_data (6.54s)
=== RUN   TestAccAWSLaunchTemplate_basic
--- PASS: TestAccAWSLaunchTemplate_basic (6.60s)
=== RUN   TestAccAWSLaunchTemplate_importBasic
--- PASS: TestAccAWSLaunchTemplate_importBasic (7.32s)
=== RUN   TestAccAWSLaunchTemplate_importData
--- PASS: TestAccAWSLaunchTemplate_importData (7.35s)
=== RUN   TestAccAWSLaunchTemplate_update
--- PASS: TestAccAWSLaunchTemplate_update (11.30s)

bflad · 2018-04-18T18:19:23Z

This has been released in version 1.15.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-04-06T17:39:49Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

bflad added new-resource Introduces a new resource. service/ec2 Issues and PRs that pertain to the ec2 service. labels Jan 11, 2018

radeksimko changed the title ~~[WIP] create launch template resource~~ [WIP] New Resource: aws_launch_template Jan 16, 2018

kl4w force-pushed the add_launch_template_resource branch from 06830e6 to 8675237 Compare January 24, 2018 03:12

kl4w force-pushed the add_launch_template_resource branch from 95af11a to 4b30fd6 Compare January 24, 2018 03:19

kl4w force-pushed the add_launch_template_resource branch from 4b30fd6 to f99fe1b Compare February 26, 2018 14:15

ghost added the size/XXL Managed by automation to categorize the size of a PR. label Feb 26, 2018

bflad added this to the v1.12.0 milestone Feb 28, 2018

bflad self-requested a review February 28, 2018 18:38

ghost added the size/XXL Managed by automation to categorize the size of a PR. label Mar 1, 2018

kl4w changed the title ~~[WIP] New Resource: aws_launch_template~~ New Resource: aws_launch_template Mar 1, 2018

bflad mentioned this pull request Mar 1, 2018

Add support for kms key id in the block device mapping of launch configurations #3592

Closed

bflad modified the milestones: v1.12.0, v1.13.0 Mar 23, 2018

kl4w added 5 commits March 26, 2018 09:21

create launch template resource

fde2107

implement update function

237439a

name cannot be less than 3 chars

50eae6d

add an update test

aabb541

add documentation for launch template resource

4a26f38

kl4w force-pushed the add_launch_template_resource branch from ac7bc33 to 4a26f38 Compare March 26, 2018 14:02

ghost added the size/XXL Managed by automation to categorize the size of a PR. label Mar 26, 2018

george-angel mentioned this pull request Mar 26, 2018

cfssl instance use m5 instance family utilitywarehouse/tf_kube_aws#27

Merged

bflad modified the milestones: v1.13.0, v1.14.0 Mar 29, 2018

bflad self-assigned this Apr 5, 2018

bflad reviewed Apr 5, 2018

View reviewed changes

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Apr 5, 2018

bflad modified the milestones: v1.14.0, v1.15.0 Apr 6, 2018

modifications after review

053aaf3

ghost added the size/XXL Managed by automation to categorize the size of a PR. label Apr 13, 2018

bflad reviewed Apr 13, 2018

View reviewed changes

add missing pieces to correct import

fa28f9a

ghost added the size/XXL Managed by automation to categorize the size of a PR. label Apr 13, 2018

bflad reviewed Apr 13, 2018

View reviewed changes

bflad approved these changes Apr 13, 2018

View reviewed changes

bflad removed the waiting-response Maintainers are waiting on response from community or contributor. label Apr 13, 2018

bflad merged commit 621189d into hashicorp:master Apr 13, 2018

bflad added a commit that referenced this pull request Apr 13, 2018

Update CHANGELOG for #2927

0ea1b06

gordonbondon mentioned this pull request Apr 19, 2018

Feature: Add support for launch_template in aws_instance resource #4264

Closed

WheresWardy mentioned this pull request Apr 19, 2018

Add support for launch templates to aws_spot_fleet_request #4267

Closed

kl4w deleted the add_launch_template_resource branch April 20, 2018 13:56

ghost locked and limited conversation to collaborators Apr 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: aws_launch_template #2927

New Resource: aws_launch_template #2927

kl4w commented Jan 10, 2018

danieladams456 commented Jan 11, 2018

bflad commented Jan 11, 2018

danieladams456 commented Jan 11, 2018

kl4w commented Jan 24, 2018

kl4w commented Mar 1, 2018

kl4w commented Mar 2, 2018 •

edited

Loading

kl4w commented Mar 26, 2018

bflad left a comment

bflad Apr 5, 2018

bflad Apr 5, 2018

bflad Apr 5, 2018

bflad Apr 5, 2018

bflad Apr 5, 2018

bflad Apr 5, 2018

bflad Apr 5, 2018

bflad Apr 5, 2018

bflad Apr 5, 2018

bflad Apr 5, 2018

danieladams456 commented Apr 6, 2018

bflad Apr 13, 2018

bflad Apr 13, 2018

bflad Apr 13, 2018

kl4w commented Apr 13, 2018

bflad commented Apr 13, 2018

bflad Apr 13, 2018

kl4w Apr 13, 2018

bflad Apr 13, 2018

kl4w commented Apr 13, 2018

bflad commented Apr 13, 2018

bflad Apr 13, 2018

kl4w Apr 13, 2018

bflad left a comment

bflad commented Apr 18, 2018

ghost commented Apr 6, 2020

New Resource: aws_launch_template #2927

New Resource: aws_launch_template #2927

Conversation

kl4w commented Jan 10, 2018

danieladams456 commented Jan 11, 2018

bflad commented Jan 11, 2018

danieladams456 commented Jan 11, 2018

kl4w commented Jan 24, 2018

kl4w commented Mar 1, 2018

kl4w commented Mar 2, 2018 • edited Loading

kl4w commented Mar 26, 2018

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danieladams456 commented Apr 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kl4w commented Apr 13, 2018

bflad commented Apr 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kl4w commented Apr 13, 2018

bflad commented Apr 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bflad left a comment

Choose a reason for hiding this comment

bflad commented Apr 18, 2018

ghost commented Apr 6, 2020

kl4w commented Mar 2, 2018 •

edited

Loading