r/spot_instance_request: add import support + refactor tests

.changelog/12787.txt

@@ -0,0 +1,6 @@
+```release-note:enhancement
+resource/aws_spot_instance_request: Add import support
+```
+```release-note:enhancement
+resource/aws_spot_instance_request: Add plan time validation for `spot_type` and `block_duration_minutes`
+```

aws/resource_aws_eip_association_test.go

@@ -120,7 +120,7 @@ func TestAccAWSEIPAssociation_ec2Classic(t *testing.T) {
 
 func TestAccAWSEIPAssociation_spotInstance(t *testing.T) {
 	var a ec2.Address
-	rInt := acctest.RandInt()
+	rName := acctest.RandomWithPrefix("tf-acc-test")
 	resourceName := "aws_eip_association.test"
 
 	resource.ParallelTest(t, resource.TestCase{
@@ -129,7 +129,7 @@ func TestAccAWSEIPAssociation_spotInstance(t *testing.T) {
 		CheckDestroy:      testAccCheckAWSEIPAssociationDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccAWSEIPAssociationConfig_spotInstance(rInt),
+				Config: testAccAWSEIPAssociationConfig_spotInstance(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSEIPExists("aws_eip.test", &a),
 					testAccCheckAWSEIPAssociationExists(resourceName, &a),
@@ -419,7 +419,7 @@ resource "aws_eip_association" "test" {
 `)
 }
 
-func testAccAWSEIPAssociationConfig_spotInstance(rInt int) string {
+func testAccAWSEIPAssociationConfig_spotInstance(rName string) string {
 	return composeConfig(
 		testAccLatestAmazonLinuxHvmEbsAmiConfig(),
 		testAccAvailableEc2InstanceTypeForAvailabilityZone("aws_subnet.test.availability_zone", "t3.micro", "t2.micro"),
@@ -440,7 +440,7 @@ resource "aws_internet_gateway" "test" {
 }
 
 resource "aws_key_pair" "test" {
-  key_name   = "tmp-key-%d"
+  key_name   = %[1]q
   public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD3F6tyPEFEzV0LX3X8BsXdMsQz1x2cEikKDEY0aIj41qgxMCP/iteneqXSIFZBp5vizPvaoIR3Um9xK7PGoW8giupGn+EPuxIA4cDM4vzOqOkiMPhz5XK0whEjkVzTo4+S0puvDZuwIsdiW9mxhJc7tgBNL0cYlWSYVkz4G/fslNfRPW5mYAM49f4fhtxPb5ok4Q2Lg9dPKVHO/Bgeu5woMc7RY0p1ej6D4CKFE6lymSDJpW0YHX/wqE9+cfEauh7xZcG0q9t2ta6F6fmX0agvpFyZo8aFbXeUBr7osSCJNgvavWbM/06niWrOvYX2xwWdhXmXSrbX8ZbabVohBK41 [email protected]"
 }
 
@@ -461,7 +461,7 @@ resource "aws_eip_association" "test" {
   allocation_id = aws_eip.test.id
   instance_id   = aws_spot_instance_request.test.spot_instance_id
 }
-`, rInt))
+`, rName))
 }
 
 func testAccAWSEIPAssociationConfig_instance() string {

aws/resource_aws_spot_instance_request.go

@@ -3,10 +3,11 @@ package aws
 import (
 	"fmt"
 	"log"
+	"math/big"
+	"strconv"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -20,6 +21,9 @@ func resourceAwsSpotInstanceRequest() *schema.Resource {
 		Read:   resourceAwsSpotInstanceRequestRead,
 		Delete: resourceAwsSpotInstanceRequestDelete,
 		Update: resourceAwsSpotInstanceRequestUpdate,
+		Importer: &schema.ResourceImporter{
+			State: schema.ImportStatePassthrough,
+		},
 
 		Timeouts: &schema.ResourceTimeout{
 			Create: schema.DefaultTimeout(10 * time.Minute),
@@ -47,12 +51,20 @@ func resourceAwsSpotInstanceRequest() *schema.Resource {
 			s["spot_price"] = &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
+				Computed: true,
 				ForceNew: true,
+				DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
+					oldFloat, _ := strconv.ParseFloat(old, 64)
+					newFloat, _ := strconv.ParseFloat(new, 64)
+
+					return big.NewFloat(oldFloat).Cmp(big.NewFloat(newFloat)) == 0
+				},
 			}
 			s["spot_type"] = &schema.Schema{
-				Type:     schema.TypeString,
-				Optional: true,
-				Default:  "persistent",
+				Type:         schema.TypeString,
+				Optional:     true,
+				Default:      ec2.SpotInstanceTypePersistent,
+				ValidateFunc: validation.StringInSlice(ec2.SpotInstanceType_Values(), false),
 			}
 			s["wait_for_fulfillment"] = &schema.Schema{
 				Type:     schema.TypeBool,
@@ -77,20 +89,17 @@ func resourceAwsSpotInstanceRequest() *schema.Resource {
 				Computed: true,
 			}
 			s["block_duration_minutes"] = &schema.Schema{
-				Type:     schema.TypeInt,
-				Optional: true,
-				ForceNew: true,
+				Type:         schema.TypeInt,
+				Optional:     true,
+				ForceNew:     true,
+				ValidateFunc: validation.IntDivisibleBy(60),
 			}
 			s["instance_interruption_behaviour"] = &schema.Schema{
-				Type:     schema.TypeString,
-				Optional: true,
-				Default:  ec2.InstanceInterruptionBehaviorTerminate,
-				ForceNew: true,
-				ValidateFunc: validation.StringInSlice([]string{
-					ec2.InstanceInterruptionBehaviorTerminate,
-					ec2.InstanceInterruptionBehaviorStop,
-					ec2.InstanceInterruptionBehaviorHibernate,
-				}, false),
+				Type:         schema.TypeString,
+				Optional:     true,
+				Default:      ec2.InstanceInterruptionBehaviorTerminate,
+				ForceNew:     true,
+				ValidateFunc: validation.StringInSlice(ec2.InstanceInterruptionBehavior_Values(), false),
 			}
 			s["valid_from"] = &schema.Schema{
 				Type:         schema.TypeString,
@@ -155,19 +164,19 @@ func resourceAwsSpotInstanceRequestCreate(d *schema.ResourceData, meta interface
 	}
 
 	if v, ok := d.GetOk("valid_from"); ok {
-		valid_from, err := time.Parse(time.RFC3339, v.(string))
+		validFrom, err := time.Parse(time.RFC3339, v.(string))
 		if err != nil {
 			return err
 		}
-		spotOpts.ValidFrom = aws.Time(valid_from)
+		spotOpts.ValidFrom = aws.Time(validFrom)
 	}
 
 	if v, ok := d.GetOk("valid_until"); ok {
-		valid_until, err := time.Parse(time.RFC3339, v.(string))
+		validUntil, err := time.Parse(time.RFC3339, v.(string))
 		if err != nil {
 			return err
 		}
-		spotOpts.ValidUntil = aws.Time(valid_until)
+		spotOpts.ValidUntil = aws.Time(validUntil)
 	}
 
 	// Placement GroupName can only be specified when instanceInterruptionBehavior is not set or set to 'terminate'
@@ -248,7 +257,8 @@ func resourceAwsSpotInstanceRequestRead(d *schema.ResourceData, meta interface{}
 	if err != nil {
 		// If the spot request was not found, return nil so that we can show
 		// that it is gone.
-		if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "InvalidSpotInstanceRequestID.NotFound" {
+		if isAWSErr(err, "InvalidSpotInstanceRequestID.NotFound", "") {
+			log.Printf("[WARN] EC2 Spot Instance Request (%s) not found, removing from state", d.Id())
 			d.SetId("")
 			return nil
 		}
@@ -259,14 +269,15 @@ func resourceAwsSpotInstanceRequestRead(d *schema.ResourceData, meta interface{}
 
 	// If nothing was found, then return no state
 	if len(resp.SpotInstanceRequests) == 0 {
+		log.Printf("[WARN] EC2 Spot Instance Request (%s) not found, removing from state", d.Id())
 		d.SetId("")
 		return nil
 	}
 
 	request := resp.SpotInstanceRequests[0]
 
 	// if the request is cancelled or closed, then it is gone
-	if *request.State == "cancelled" || *request.State == "closed" {
+	if *request.State == ec2.SpotInstanceStateCancelled || *request.State == ec2.SpotInstanceStateClosed {
 		d.SetId("")
 		return nil
 	}
@@ -292,6 +303,11 @@ func resourceAwsSpotInstanceRequestRead(d *schema.ResourceData, meta interface{}
 	d.Set("instance_interruption_behaviour", request.InstanceInterruptionBehavior)
 	d.Set("valid_from", aws.TimeValue(request.ValidFrom).Format(time.RFC3339))
 	d.Set("valid_until", aws.TimeValue(request.ValidUntil).Format(time.RFC3339))
+	d.Set("spot_type", request.Type)
+	d.Set("spot_price", request.SpotPrice)
+	d.Set("key_name", request.LaunchSpecification.KeyName)
+	d.Set("instance_type", request.LaunchSpecification.InstanceType)
+	d.Set("ami", request.LaunchSpecification.ImageId)
 
 	return nil
 }
@@ -316,62 +332,64 @@ func readInstance(d *schema.ResourceData, meta interface{}) error {
 		return fmt.Errorf("no instances found")
 	}
 
-	// Set these fields for connection information
-	if instance != nil {
-		d.Set("public_dns", instance.PublicDnsName)
-		d.Set("public_ip", instance.PublicIpAddress)
-		d.Set("private_dns", instance.PrivateDnsName)
-		d.Set("private_ip", instance.PrivateIpAddress)
-
-		// set connection information
-		if instance.PublicIpAddress != nil {
-			d.SetConnInfo(map[string]string{
-				"type": "ssh",
-				"host": *instance.PublicIpAddress,
-			})
-		} else if instance.PrivateIpAddress != nil {
-			d.SetConnInfo(map[string]string{
-				"type": "ssh",
-				"host": *instance.PrivateIpAddress,
-			})
-		}
-		if err := readBlockDevices(d, instance, conn); err != nil {
-			return err
-		}
+	d.Set("public_dns", instance.PublicDnsName)
+	d.Set("public_ip", instance.PublicIpAddress)
+	d.Set("private_dns", instance.PrivateDnsName)
+	d.Set("private_ip", instance.PrivateIpAddress)
+	d.Set("source_dest_check", instance.SourceDestCheck)
+
+	// set connection information
+	if instance.PublicIpAddress != nil {
+		d.SetConnInfo(map[string]string{
+			"type": "ssh",
+			"host": *instance.PublicIpAddress,
+		})
+	} else if instance.PrivateIpAddress != nil {
+		d.SetConnInfo(map[string]string{
+			"type": "ssh",
+			"host": *instance.PrivateIpAddress,
+		})
+	}
+	if err := readBlockDevices(d, instance, conn); err != nil {
+		return err
+	}
 
-		var ipv6Addresses []string
-		if len(instance.NetworkInterfaces) > 0 {
-			for _, ni := range instance.NetworkInterfaces {
-				if *ni.Attachment.DeviceIndex == 0 {
-					d.Set("subnet_id", ni.SubnetId)
-					d.Set("primary_network_interface_id", ni.NetworkInterfaceId)
-					d.Set("associate_public_ip_address", ni.Association != nil)
-					d.Set("ipv6_address_count", len(ni.Ipv6Addresses))
-
-					for _, address := range ni.Ipv6Addresses {
-						ipv6Addresses = append(ipv6Addresses, *address.Ipv6Address)
-					}
+	var ipv6Addresses []string
+	if len(instance.NetworkInterfaces) > 0 {
+		for _, ni := range instance.NetworkInterfaces {
+			if *ni.Attachment.DeviceIndex == 0 {
+				d.Set("subnet_id", ni.SubnetId)
+				d.Set("primary_network_interface_id", ni.NetworkInterfaceId)
+				d.Set("associate_public_ip_address", ni.Association != nil)
+				d.Set("ipv6_address_count", len(ni.Ipv6Addresses))
+
+				for _, address := range ni.Ipv6Addresses {
+					ipv6Addresses = append(ipv6Addresses, *address.Ipv6Address)
 				}
 			}
-		} else {
-			d.Set("subnet_id", instance.SubnetId)
-			d.Set("primary_network_interface_id", "")
 		}
+	} else {
+		d.Set("subnet_id", instance.SubnetId)
+		d.Set("primary_network_interface_id", "")
+	}
 
-		if err := d.Set("ipv6_addresses", ipv6Addresses); err != nil {
-			log.Printf("[WARN] Error setting ipv6_addresses for AWS Spot Instance (%s): %s", d.Id(), err)
-		}
+	if err := d.Set("ipv6_addresses", ipv6Addresses); err != nil {
+		log.Printf("[WARN] Error setting ipv6_addresses for AWS Spot Instance (%s): %s", d.Id(), err)
+	}
 
-		if d.Get("get_password_data").(bool) {
-			passwordData, err := getAwsEc2InstancePasswordData(*instance.InstanceId, conn)
-			if err != nil {
-				return err
-			}
-			d.Set("password_data", passwordData)
-		} else {
-			d.Set("get_password_data", false)
-			d.Set("password_data", nil)
+	if err := readSecurityGroups(d, instance, conn); err != nil {
+		return err
+	}
+
+	if d.Get("get_password_data").(bool) {
+		passwordData, err := getAwsEc2InstancePasswordData(*instance.InstanceId, conn)
+		if err != nil {
+			return err
 		}
+		d.Set("password_data", passwordData)
+	} else {
+		d.Set("get_password_data", false)
+		d.Set("password_data", nil)
 	}
 
 	return nil
@@ -424,7 +442,7 @@ func SpotInstanceStateRefreshFunc(
 		})
 
 		if err != nil {
-			if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "InvalidSpotInstanceRequestID.NotFound" {
+			if isAWSErr(err, "InvalidSpotInstanceRequestID.NotFound", "") {
 				// Set this to nil as if we didn't find anything.
 				resp = nil
 			} else {