diff --git a/.changelog/24695.txt b/.changelog/24695.txt new file mode 100644 index 000000000000..7d97765724ba --- /dev/null +++ b/.changelog/24695.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_instance: Prevent error `InvalidParameterCombination: The parameter GroupName within placement information cannot be specified when instanceInterruptionBehavior is set to 'STOP'` when using a launch template that sets `instance_interruption_behavior` to `stop` +``` diff --git a/internal/service/ec2/ec2_instance.go b/internal/service/ec2/ec2_instance.go index f2cdf93ec9b6..e976a0198cd9 100644 --- a/internal/service/ec2/ec2_instance.go +++ b/internal/service/ec2/ec2_instance.go @@ -1989,62 +1989,6 @@ func blockDeviceIsRoot(bd *ec2.InstanceBlockDeviceMapping, instance *ec2.Instanc aws.StringValue(bd.DeviceName) == aws.StringValue(instance.RootDeviceName) } -func fetchLaunchTemplateAMI(specs []interface{}, conn *ec2.EC2) (string, error) { - if len(specs) < 1 { - return "", errors.New("Cannot fetch AMI for blank launch template.") - } - - spec := specs[0].(map[string]interface{}) - - idValue, idOk := spec["id"] - nameValue, nameOk := spec["name"] - - request := &ec2.DescribeLaunchTemplateVersionsInput{} - - if idOk && idValue != "" { - request.LaunchTemplateId = aws.String(idValue.(string)) - } else if nameOk && nameValue != "" { - request.LaunchTemplateName = aws.String(nameValue.(string)) - } - - var isLatest bool - defaultFilter := []*ec2.Filter{ - { - Name: aws.String("is-default-version"), - Values: aws.StringSlice([]string{"true"}), - }, - } - if v, ok := spec["version"]; ok && v != "" { - switch v { - case LaunchTemplateVersionDefault: - request.Filters = defaultFilter - case LaunchTemplateVersionLatest: - isLatest = true - default: - request.Versions = []*string{aws.String(v.(string))} - } - } - - dltv, err := conn.DescribeLaunchTemplateVersions(request) - if err != nil { - return "", err - } - - var ltData *ec2.ResponseLaunchTemplateData - if isLatest { - index := len(dltv.LaunchTemplateVersions) - 1 - ltData = dltv.LaunchTemplateVersions[index].LaunchTemplateData - } else { - ltData = dltv.LaunchTemplateVersions[0].LaunchTemplateData - } - - if ltData.ImageId != nil { - return *ltData.ImageId, nil - } - - return "", nil -} - func FetchRootDeviceName(conn *ec2.EC2, amiID string) (*string, error) { if amiID == "" { return nil, errors.New("Cannot fetch root device name for blank AMI ID.") @@ -2292,21 +2236,24 @@ func readBlockDeviceMappingsFromConfig(d *schema.ResourceData, conn *ec2.EC2) ([ } var amiID string - if v, ok := d.GetOk("launch_template"); ok { - var err error - amiID, err = fetchLaunchTemplateAMI(v.([]interface{}), conn) + + if v, ok := d.GetOk("launch_template"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + launchTemplateData, err := findLaunchTemplateData(conn, expandLaunchTemplateSpecification(v.([]interface{})[0].(map[string]interface{}))) + if err != nil { return nil, err } + + amiID = aws.StringValue(launchTemplateData.ImageId) } - // AMI id from attributes overrides ami from launch template + // AMI from configuration overrides the one from the launch template. if v, ok := d.GetOk("ami"); ok { amiID = v.(string) } if amiID == "" { - return nil, errors.New("`ami` must be set or provided via launch template") + return nil, errors.New("`ami` must be set or provided via `launch_template`") } if dn, err := FetchRootDeviceName(conn, amiID); err == nil { @@ -2508,8 +2455,21 @@ func buildInstanceOpts(d *schema.ResourceData, meta interface{}) (*awsInstanceOp opts.InstanceType = aws.String(v.(string)) } - if v, ok := d.GetOk("launch_template"); ok { - opts.LaunchTemplate = expandLaunchTemplateSpecification(v.([]interface{})) + var instanceInterruptionBehavior string + + if v, ok := d.GetOk("launch_template"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + launchTemplateSpecification := expandLaunchTemplateSpecification(v.([]interface{})[0].(map[string]interface{})) + launchTemplateData, err := findLaunchTemplateData(conn, launchTemplateSpecification) + + if err != nil { + return nil, err + } + + opts.LaunchTemplate = launchTemplateSpecification + + if launchTemplateData.InstanceMarketOptions != nil && launchTemplateData.InstanceMarketOptions.SpotOptions != nil { + instanceInterruptionBehavior = aws.StringValue(launchTemplateData.InstanceMarketOptions.SpotOptions.InstanceInterruptionBehavior) + } } instanceType := d.Get("instance_type").(string) @@ -2563,7 +2523,6 @@ func buildInstanceOpts(d *schema.ResourceData, meta interface{}) (*awsInstanceOp // aws_spot_instance_request. They represent the same data. :-| opts.Placement = &ec2.Placement{ AvailabilityZone: aws.String(d.Get("availability_zone").(string)), - GroupName: aws.String(d.Get("placement_group").(string)), } if v, ok := d.GetOk("placement_partition_number"); ok { @@ -2572,7 +2531,11 @@ func buildInstanceOpts(d *schema.ResourceData, meta interface{}) (*awsInstanceOp opts.SpotPlacement = &ec2.SpotPlacement{ AvailabilityZone: aws.String(d.Get("availability_zone").(string)), - GroupName: aws.String(d.Get("placement_group").(string)), + } + + if v := d.Get("placement_group").(string); instanceInterruptionBehavior == "" || instanceInterruptionBehavior == ec2.InstanceInterruptionBehaviorTerminate { + opts.Placement.GroupName = aws.String(v) + opts.SpotPlacement.GroupName = aws.String(v) } if v := d.Get("tenancy").(string); v != "" { @@ -3071,29 +3034,26 @@ func flattenInstanceMaintenanceOptions(apiObject *ec2.InstanceMaintenanceOptions return tfMap } -func expandLaunchTemplateSpecification(specs []interface{}) *ec2.LaunchTemplateSpecification { - if len(specs) < 1 { +func expandLaunchTemplateSpecification(tfMap map[string]interface{}) *ec2.LaunchTemplateSpecification { + if tfMap == nil { return nil } - spec := specs[0].(map[string]interface{}) + apiObject := &ec2.LaunchTemplateSpecification{} - idValue, idOk := spec["id"] - nameValue, nameOk := spec["name"] - - result := &ec2.LaunchTemplateSpecification{} - - if idOk && idValue != "" { - result.LaunchTemplateId = aws.String(idValue.(string)) - } else if nameOk && nameValue != "" { - result.LaunchTemplateName = aws.String(nameValue.(string)) + // DescribeLaunchTemplates returns both name and id but LaunchTemplateSpecification + // allows only one of them to be set. + if v, ok := tfMap["id"]; ok && v != "" { + apiObject.LaunchTemplateId = aws.String(v.(string)) + } else if v, ok := tfMap["name"]; ok && v != "" { + apiObject.LaunchTemplateName = aws.String(v.(string)) } - if v, ok := spec["version"]; ok && v != "" { - result.Version = aws.String(v.(string)) + if v, ok := tfMap["version"].(string); ok && v != "" { + apiObject.Version = aws.String(v) } - return result + return apiObject } func flattenInstanceLaunchTemplate(conn *ec2.EC2, instanceID, previousLaunchTemplateVersion string) ([]interface{}, error) { @@ -3178,6 +3138,43 @@ func findInstanceLaunchTemplateVersion(conn *ec2.EC2, id string) (string, error) return launchTemplateVersion, nil } +func findLaunchTemplateData(conn *ec2.EC2, launchTemplateSpecification *ec2.LaunchTemplateSpecification) (*ec2.ResponseLaunchTemplateData, error) { + input := &ec2.DescribeLaunchTemplateVersionsInput{} + + if v := aws.StringValue(launchTemplateSpecification.LaunchTemplateId); v != "" { + input.LaunchTemplateId = aws.String(v) + } else if v := aws.StringValue(launchTemplateSpecification.LaunchTemplateName); v != "" { + input.LaunchTemplateName = aws.String(v) + } + + var latestVersion bool + + if v := aws.StringValue(launchTemplateSpecification.Version); v != "" { + switch v { + case LaunchTemplateVersionDefault: + input.Filters = BuildAttributeFilterList(map[string]string{ + "is-default-version": "true", + }) + case LaunchTemplateVersionLatest: + latestVersion = true + default: + input.Versions = aws.StringSlice([]string{v}) + } + } + + output, err := FindLaunchTemplateVersions(conn, input) + + if err != nil { + return nil, fmt.Errorf("reading EC2 Launch Template versions: %w", err) + } + + if latestVersion { + return output[len(output)-1].LaunchTemplateData, nil + } + + return output[0].LaunchTemplateData, nil +} + // findLaunchTemplateNameAndVersions returns the specified launch template's name, default version and latest version. func findLaunchTemplateNameAndVersions(conn *ec2.EC2, id string) (string, string, string, error) { lt, err := FindLaunchTemplateByID(conn, id) diff --git a/internal/service/ec2/ec2_instance_test.go b/internal/service/ec2/ec2_instance_test.go index 63169f581991..cd8533e7a6cb 100644 --- a/internal/service/ec2/ec2_instance_test.go +++ b/internal/service/ec2/ec2_instance_test.go @@ -3209,6 +3209,29 @@ func TestAccEC2Instance_LaunchTemplate_swapIDAndName(t *testing.T) { }) } +func TestAccEC2Instance_LaunchTemplate_spotAndStop(t *testing.T) { + var v ec2.Instance + resourceName := "aws_instance.test" + launchTemplateResourceName := "aws_launch_template.test" + rName := sdkacctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_templateSpotAndStop(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + resource.TestCheckResourceAttrPair(resourceName, "launch_template.0.id", launchTemplateResourceName, "id"), + ), + }, + }, + }) +} + func TestAccEC2Instance_GetPasswordData_falseToTrue(t *testing.T) { var before, after ec2.Instance resourceName := "aws_instance.test" @@ -7646,3 +7669,35 @@ resource "aws_instance" "test" { } `, rName)) } + +func testAccInstanceConfig_templateSpotAndStop(rName string) string { + return acctest.ConfigCompose( + acctest.ConfigLatestAmazonLinuxHVMEBSAMI(), + acctest.AvailableEC2InstanceTypeForRegion("t3.micro", "t2.micro", "t1.micro", "m1.small"), + fmt.Sprintf(` +resource "aws_launch_template" "test" { + name = %[1]q + image_id = data.aws_ami.amzn-ami-minimal-hvm-ebs.id + instance_type = data.aws_ec2_instance_type_offering.available.instance_type + + instance_market_options { + market_type = "spot" + + spot_options { + instance_interruption_behavior = "stop" + spot_instance_type = "persistent" + } + } +} + +resource "aws_instance" "test" { + launch_template { + name = aws_launch_template.test.name + } + + tags = { + Name = %[1]q + } +} +`, rName)) +} diff --git a/internal/service/ec2/find.go b/internal/service/ec2/find.go index 6fd4e5cc25f9..c029b57f7dc1 100644 --- a/internal/service/ec2/find.go +++ b/internal/service/ec2/find.go @@ -4185,7 +4185,7 @@ func FindLaunchTemplateVersion(conn *ec2.EC2, input *ec2.DescribeLaunchTemplateV return nil, err } - if len(output) == 0 || output[0] == nil { + if len(output) == 0 || output[0] == nil || output[0].LaunchTemplateData == nil { return nil, tfresource.NewEmptyResultError(input) } @@ -4213,7 +4213,7 @@ func FindLaunchTemplateVersions(conn *ec2.EC2, input *ec2.DescribeLaunchTemplate return !lastPage }) - if tfawserr.ErrCodeEquals(err, errCodeInvalidLaunchTemplateIdNotFound, errCodeInvalidLaunchTemplateIdVersionNotFound) { + if tfawserr.ErrCodeEquals(err, errCodeInvalidLaunchTemplateIdNotFound, errCodeInvalidLaunchTemplateNameNotFoundException, errCodeInvalidLaunchTemplateIdVersionNotFound) { return nil, &resource.NotFoundError{ LastError: err, LastRequest: input,