Merge pull request #7813 from hashicorp/stop_calculating_spot_bids

stop calculating spot bids

builder/amazon/common/run_config.go

@@ -153,20 +153,6 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 			"block_duration_minutes must be multiple of 60"))
 	}
 
-	if c.SpotPrice == "auto" {
-		if c.SpotPriceAutoProduct == "" {
-			errs = append(errs, fmt.Errorf(
-				"spot_price_auto_product must be specified when spot_price is auto"))
-		}
-	}
-
-	if c.SpotPriceAutoProduct != "" {
-		if c.SpotPrice != "auto" {
-			errs = append(errs, fmt.Errorf(
-				"spot_price should be set to auto when spot_price_auto_product is specified"))
-		}
-	}
-
 	if c.SpotTags != nil {
 		if c.SpotPrice == "" || c.SpotPrice == "0" {
 			errs = append(errs, fmt.Errorf(

builder/amazon/common/run_config_test.go

@@ -117,7 +117,6 @@ func TestRunConfigPrepare_EnableT2UnlimitedBadWithSpotInstanceRequest(t *testing
 	c.InstanceType = "t2.micro"
 	c.EnableT2Unlimited = true
 	c.SpotPrice = "auto"
-	c.SpotPriceAutoProduct = "Linux/UNIX"
 	err := c.Prepare(nil)
 	if len(err) != 1 {
 		t.Fatalf("Should error if T2 Unlimited has been used in conjuntion with a Spot Price request")
@@ -127,19 +126,14 @@ func TestRunConfigPrepare_EnableT2UnlimitedBadWithSpotInstanceRequest(t *testing
 func TestRunConfigPrepare_SpotAuto(t *testing.T) {
 	c := testConfig()
 	c.SpotPrice = "auto"
-	if err := c.Prepare(nil); len(err) != 1 {
-		t.Fatalf("Should error if spot_price_auto_product is not set and spot_price is set to auto")
-	}
-
-	// Good - SpotPrice and SpotPriceAutoProduct are correctly set
-	c.SpotPriceAutoProduct = "foo"
 	if err := c.Prepare(nil); len(err) != 0 {
 		t.Fatalf("err: %s", err)
 	}
 
-	c.SpotPrice = ""
-	if err := c.Prepare(nil); len(err) != 1 {
-		t.Fatalf("Should error if spot_price is not set to auto and spot_price_auto_product is set")
+	// Shouldn't error (YET) even though SpotPriceAutoProduct is deprecated
+	c.SpotPriceAutoProduct = "Linux/Unix"
+	if err := c.Prepare(nil); len(err) != 0 {
+		t.Fatalf("err: %s", err)
 	}
 }
 

builder/amazon/common/step_run_spot_instance.go

@@ -6,14 +6,11 @@ import (
 	"fmt"
 	"io/ioutil"
 	"log"
-	"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/aws/aws-sdk-go/service/ec2/ec2iface"
 	"github.com/hashicorp/packer/common/random"
 	"github.com/hashicorp/packer/common/retry"
 	"github.com/hashicorp/packer/helper/communicator"
@@ -35,7 +32,6 @@ type StepRunSpotInstance struct {
 	InstanceType                      string
 	SourceAMI                         string
 	SpotPrice                         string
-	SpotPriceProduct                  string
 	SpotTags                          TagMap
 	SpotInstanceTypes                 []string
 	Tags                              TagMap
@@ -48,55 +44,6 @@ type StepRunSpotInstance struct {
 	spotRequest *ec2.SpotInstanceRequest
 }
 
-func (s *StepRunSpotInstance) CalculateSpotPrice(az string, ec2conn ec2iface.EC2API) (string, error) {
-	// Calculate the spot price for a given availability zone
-	spotPrice := s.SpotPrice
-
-	if spotPrice == "auto" {
-		// Detect the spot price
-		startTime := time.Now().Add(-1 * time.Hour)
-		resp, err := ec2conn.DescribeSpotPriceHistory(&ec2.DescribeSpotPriceHistoryInput{
-			InstanceTypes:       []*string{&s.InstanceType},
-			ProductDescriptions: []*string{&s.SpotPriceProduct},
-			AvailabilityZone:    &az,
-			StartTime:           &startTime,
-		})
-		if err != nil {
-			return "", fmt.Errorf("Error finding spot price: %s", err)
-		}
-
-		var price float64
-		for _, history := range resp.SpotPriceHistory {
-			log.Printf("[INFO] Candidate spot price: %s", *history.SpotPrice)
-			current, err := strconv.ParseFloat(*history.SpotPrice, 64)
-			if err != nil {
-				log.Printf("[ERR] Error parsing spot price: %s", err)
-				continue
-			}
-			if price == 0 || current < price {
-				price = current
-				if az == "" {
-					az = *history.AvailabilityZone
-				}
-			}
-		}
-		if price == 0 {
-			return "", fmt.Errorf("No candidate spot prices found!")
-		} else {
-			// Add 0.5 cents to minimum spot bid to ensure capacity will be available
-			// Avoids price-too-low error in active markets which can fluctuate
-			price = price + 0.005
-		}
-
-		spotPrice = strconv.FormatFloat(price, 'f', -1, 64)
-	}
-
-	s.SpotPrice = spotPrice
-
-	return spotPrice, nil
-
-}
-
 func (s *StepRunSpotInstance) CreateTemplateData(userData *string, az string,
 	state multistep.StateBag, marketOptions *ec2.LaunchTemplateInstanceMarketOptionsRequest) *ec2.RequestLaunchTemplateData {
 	// Convert the BlockDeviceMapping into a
@@ -213,17 +160,6 @@ func (s *StepRunSpotInstance) Run(ctx context.Context, state multistep.StateBag)
 	}
 	az := azConfig
 
-	ui.Message(fmt.Sprintf("Finding spot price for %s %s...",
-		s.SpotPriceProduct, s.InstanceType))
-	spotPrice, err := s.CalculateSpotPrice(az, ec2conn)
-	if err != nil {
-		state.Put("error", err)
-		ui.Error(err.Error())
-		return multistep.ActionHalt
-	}
-
-	ui.Message(fmt.Sprintf("Determined spot instance price of: %s.", spotPrice))
-
 	var instanceId string
 
 	ui.Say("Interpolating tags for spot instance...")
@@ -247,8 +183,10 @@ func (s *StepRunSpotInstance) Run(ctx context.Context, state multistep.StateBag)
 	// instance yet
 	ec2Tags.Report(ui)
 
-	spotOptions := ec2.LaunchTemplateSpotMarketOptionsRequest{
-		MaxPrice: &s.SpotPrice,
+	spotOptions := ec2.LaunchTemplateSpotMarketOptionsRequest{}
+	// The default is to set the maximum price to the OnDemand price.
+	if s.SpotPrice != "auto" {
+		spotOptions.SetMaxPrice(s.SpotPrice)
 	}
 	if s.BlockDurationMinutes != 0 {
 		spotOptions.BlockDurationMinutes = &s.BlockDurationMinutes

builder/amazon/common/step_run_spot_instance_test.go

@@ -2,7 +2,6 @@ package common
 
 import (
 	"bytes"
-	"strconv"
 	"testing"
 	"time"
 
@@ -102,7 +101,6 @@ func getBasicStep() *StepRunSpotInstance {
 		InstanceType:                      "t2.micro",
 		SourceAMI:                         "",
 		SpotPrice:                         "auto",
-		SpotPriceProduct:                  "Linux/UNIX",
 		SpotTags:                          TagMap(nil),
 		Tags:                              TagMap{},
 		VolumeTags:                        TagMap(nil),
@@ -112,20 +110,31 @@ func getBasicStep() *StepRunSpotInstance {
 
 	return &stepRunSpotInstance
 }
-func TestCalculateSpotPrice(t *testing.T) {
+
+func TestCreateTemplateData(t *testing.T) {
+	state := tStateSpot()
 	stepRunSpotInstance := getBasicStep()
-	// Set spot price and spot price product
-	stepRunSpotInstance.SpotPrice = "auto"
-	stepRunSpotInstance.SpotPriceProduct = "Linux/UNIX"
-	ec2conn := getMockConnSpot()
-	// state := tStateSpot()
-	spotPrice, err := stepRunSpotInstance.CalculateSpotPrice("", ec2conn)
-	if err != nil {
-		t.Fatalf("Should not have had an error calculating spot price")
+	template := stepRunSpotInstance.CreateTemplateData(aws.String("userdata"), "az", state,
+		&ec2.LaunchTemplateInstanceMarketOptionsRequest{})
+
+	// expected := []*ec2.LaunchTemplateInstanceNetworkInterfaceSpecificationRequest{
+	// 	&ec2.LaunchTemplateInstanceNetworkInterfaceSpecificationRequest{
+	// 		DeleteOnTermination: aws.Bool(true),
+	// 		DeviceIndex:         aws.Int64(0),
+	// 		Groups:              aws.StringSlice([]string{"sg-0b8984db72f213dc3"}),
+	// 		SubnetId:            aws.String("subnet-077fde4e"),
+	// 	},
+	// }
+	// if expected != template.NetworkInterfaces {
+	if template.NetworkInterfaces == nil {
+		t.Fatalf("Template should have contained a networkInterface object: recieved %#v", template.NetworkInterfaces)
 	}
-	sp, _ := strconv.ParseFloat(spotPrice, 64)
-	expected := 0.008500
-	if sp != expected { // 0.003500 (from spot history) + .005
-		t.Fatalf("Expected spot price of \"0.008500\", not %s", spotPrice)
+
+	// Rerun, this time testing that we set security group IDs
+	state.Put("subnet_id", "")
+	template = stepRunSpotInstance.CreateTemplateData(aws.String("userdata"), "az", state,
+		&ec2.LaunchTemplateInstanceMarketOptionsRequest{})
+	if template.NetworkInterfaces != nil {
+		t.Fatalf("Template shouldn't contain network interfaces object if subnet_id is unset.")
 	}
 }

builder/amazon/ebs/builder.go

@@ -64,6 +64,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 
 	// Accumulate any errors
 	var errs *packer.MultiError
+	var warns []string
+
 	errs = packer.MultiErrorAppend(errs, b.config.AccessConfig.Prepare(&b.config.ctx)...)
 	errs = packer.MultiErrorAppend(errs,
 		b.config.AMIConfig.Prepare(&b.config.AccessConfig, &b.config.ctx)...)
@@ -77,12 +79,20 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 				"you use an AMI that already has either SR-IOV or ENA enabled."))
 	}
 
+	if b.config.RunConfig.SpotPriceAutoProduct != "" {
+		warns = append(warns, "spot_price_auto_product is deprecated and no "+
+			"longer necessary for Packer builds. In future versions of "+
+			"Packer, inclusion of spot_price_auto_product will error your "+
+			"builds. Please take a look at our current documentation to "+
+			"understand how Packer requests Spot instances.")
+	}
+
 	if errs != nil && len(errs.Errors) > 0 {
-		return nil, errs
+		return warns, errs
 	}
 
 	packer.LogSecretFilter.Set(b.config.AccessKey, b.config.SecretKey, b.config.Token)
-	return nil, nil
+	return warns, nil
 }
 
 func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (packer.Artifact, error) {
@@ -121,7 +131,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			InstanceType:                      b.config.InstanceType,
 			SourceAMI:                         b.config.SourceAmi,
 			SpotPrice:                         b.config.SpotPrice,
-			SpotPriceProduct:                  b.config.SpotPriceAutoProduct,
 			SpotTags:                          b.config.SpotTags,
 			Tags:                              b.config.RunTags,
 			SpotInstanceTypes:                 b.config.SpotInstanceTypes,

builder/amazon/ebssurrogate/builder.go

@@ -64,6 +64,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 
 	// Accumulate any errors
 	var errs *packer.MultiError
+	var warns []string
 	errs = packer.MultiErrorAppend(errs, b.config.AccessConfig.Prepare(&b.config.ctx)...)
 	errs = packer.MultiErrorAppend(errs, b.config.RunConfig.Prepare(&b.config.ctx)...)
 	errs = packer.MultiErrorAppend(errs,
@@ -96,6 +97,14 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 				"you use an AMI that already has either SR-IOV or ENA enabled."))
 	}
 
+	if b.config.RunConfig.SpotPriceAutoProduct != "" {
+		warns = append(warns, "spot_price_auto_product is deprecated and no "+
+			"longer necessary for Packer builds. In future versions of "+
+			"Packer, inclusion of spot_price_auto_product will error your "+
+			"builds. Please take a look at our current documentation to "+
+			"understand how Packer requests Spot instances.")
+	}
+
 	if b.config.Architecture == "" {
 		b.config.Architecture = "x86_64"
 	}
@@ -110,11 +119,12 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		errs = packer.MultiErrorAppend(errs, errors.New(`The only valid ami_architecture values are "x86_64" and "arm64"`))
 	}
 	if errs != nil && len(errs.Errors) > 0 {
-		return nil, errs
+		return warns, errs
 	}
 
 	packer.LogSecretFilter.Set(b.config.AccessKey, b.config.SecretKey, b.config.Token)
-	return nil, nil
+
+	return warns, nil
 }
 
 func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (packer.Artifact, error) {
@@ -151,7 +161,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			InstanceType:                      b.config.InstanceType,
 			SourceAMI:                         b.config.SourceAmi,
 			SpotPrice:                         b.config.SpotPrice,
-			SpotPriceProduct:                  b.config.SpotPriceAutoProduct,
 			SpotInstanceTypes:                 b.config.SpotInstanceTypes,
 			SpotTags:                          b.config.SpotTags,
 			Tags:                              b.config.RunTags,

builder/amazon/ebsvolume/builder.go

@@ -59,6 +59,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 
 	// Accumulate any errors
 	var errs *packer.MultiError
+	var warns []string
 	errs = packer.MultiErrorAppend(errs, b.config.AccessConfig.Prepare(&b.config.ctx)...)
 	errs = packer.MultiErrorAppend(errs, b.config.RunConfig.Prepare(&b.config.ctx)...)
 	errs = packer.MultiErrorAppend(errs, b.config.launchBlockDevices.Prepare(&b.config.ctx)...)
@@ -81,12 +82,20 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 				"you use an AMI that already has either SR-IOV or ENA enabled."))
 	}
 
+	if b.config.RunConfig.SpotPriceAutoProduct != "" {
+		warns = append(warns, "spot_price_auto_product is deprecated and no "+
+			"longer necessary for Packer builds. In future versions of "+
+			"Packer, inclusion of spot_price_auto_product will error your "+
+			"builds. Please take a look at our current documentation to "+
+			"understand how Packer requests Spot instances.")
+	}
+
 	if errs != nil && len(errs.Errors) > 0 {
-		return nil, errs
+		return warns, errs
 	}
 
 	packer.LogSecretFilter.Set(b.config.AccessKey, b.config.SecretKey, b.config.Token)
-	return nil, nil
+	return warns, nil
 }
 
 func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (packer.Artifact, error) {
@@ -121,7 +130,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			InstanceType:                      b.config.InstanceType,
 			SourceAMI:                         b.config.SourceAmi,
 			SpotPrice:                         b.config.SpotPrice,
-			SpotPriceProduct:                  b.config.SpotPriceAutoProduct,
 			SpotInstanceTypes:                 b.config.SpotInstanceTypes,
 			SpotTags:                          b.config.SpotTags,
 			Tags:                              b.config.RunTags,

builder/amazon/instance/builder.go

@@ -126,6 +126,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 
 	// Accumulate any errors
 	var errs *packer.MultiError
+	var warns []string
 	errs = packer.MultiErrorAppend(errs, b.config.AccessConfig.Prepare(&b.config.ctx)...)
 	errs = packer.MultiErrorAppend(errs, b.config.BlockDevices.Prepare(&b.config.ctx)...)
 	errs = packer.MultiErrorAppend(errs,
@@ -163,11 +164,19 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 				"you use an AMI that already has either SR-IOV or ENA enabled."))
 	}
 
+	if b.config.RunConfig.SpotPriceAutoProduct != "" {
+		warns = append(warns, "spot_price_auto_product is deprecated and no "+
+			"longer necessary for Packer builds. In future versions of "+
+			"Packer, inclusion of spot_price_auto_product will error your "+
+			"builds. Please take a look at our current documentation to "+
+			"understand how Packer requests Spot instances.")
+	}
+
 	if errs != nil && len(errs.Errors) > 0 {
-		return nil, errs
+		return warns, errs
 	}
 	packer.LogSecretFilter.Set(b.config.AccessKey, b.config.SecretKey, b.config.Token)
-	return nil, nil
+	return warns, nil
 }
 
 func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (packer.Artifact, error) {
@@ -203,7 +212,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			SourceAMI:                b.config.SourceAmi,
 			SpotPrice:                b.config.SpotPrice,
 			SpotInstanceTypes:        b.config.SpotInstanceTypes,
-			SpotPriceProduct:         b.config.SpotPriceAutoProduct,
 			Tags:                     b.config.RunTags,
 			SpotTags:                 b.config.SpotTags,
 			UserData:                 b.config.UserData,

fix/fixer.go

@@ -43,6 +43,7 @@ func init() {
 		"hyperv-cpu-and-ram":         new(FizerHypervCPUandRAM),
 		"vmware-compaction":          new(FixerVMwareCompaction),
 		"clean-image-name":           new(FixerCleanImageName),
+		"spot-price-auto-product":    new(FixerAmazonSpotPriceProductDeprecation),
 	}
 
 	FixerOrder = []string{
@@ -67,5 +68,6 @@ func init() {
 		"vmware-compaction",
 		"hyperv-cpu-and-ram",
 		"clean-image-name",
+		"spot-price-auto-product",
 	}
 }

fix/fixer_amazon_spot_price_product.go

@@ -0,0 +1,60 @@
+package fix
+
+import (
+	"github.com/mitchellh/mapstructure"
+)
+
+// FixerAmazonSpotPriceProductDeprecation removes the deprecated "vhd_temp_path" setting
+// from Amazon builder templates
+type FixerAmazonSpotPriceProductDeprecation struct{}
+
+func (FixerAmazonSpotPriceProductDeprecation) Fix(input map[string]interface{}) (map[string]interface{}, error) {
+	// The type we'll decode into; we only care about builders
+	type template struct {
+		Builders []map[string]interface{}
+	}
+
+	// Decode the input into our structure, if we can
+	var tpl template
+	if err := mapstructure.Decode(input, &tpl); err != nil {
+		return nil, err
+	}
+
+	for _, builder := range tpl.Builders {
+		builderTypeRaw, ok := builder["type"]
+		if !ok {
+			continue
+		}
+
+		builderType, ok := builderTypeRaw.(string)
+		if !ok {
+			continue
+		}
+
+		buildersToFix := []string{"amazon-ebs", "amazon-ebssurrogate",
+			"amazon-ebsvolume", "amazon-instance"}
+
+		matched := false
+		for _, b := range buildersToFix {
+			if builderType == b {
+				matched = true
+				break
+			}
+		}
+		if !matched {
+			continue
+		}
+
+		_, ok = builder["spot_price_auto_product"]
+		if ok {
+			delete(builder, "spot_price_auto_product")
+		}
+	}
+
+	input["builders"] = tpl.Builders
+	return input, nil
+}
+
+func (FixerAmazonSpotPriceProductDeprecation) Synopsis() string {
+	return `Removes the deprecated "spot_price_auto_product" setting from Amazon builder templates`
+}

website/source/partials/builders/_aws-spot-docs.html.md

@@ -16,9 +16,15 @@
     this to `auto` for Packer to automatically discover the best spot price or
     to "0" to use an on demand instance (default).
 
--   `spot_price_auto_product` (string) - Required if `spot_price` is set to
-    `auto`. This tells Packer what sort of AMI you're launching to find the
-    best spot price. This must be one of: `Linux/UNIX`, `SUSE Linux`,
+-   `spot_price_auto_product` (string) - Deprecated. Prior to v1.4.3, was
+    required if `spot_price` is set to `auto`.
+
+    If you are using Packer v1.4.3 or later, simply remove this from your
+    template; it is no longer necessary based on recent changes to how Amazon
+    calculates spot prices.
+
+    Prior to version 1.4.3, This told Packer what sort of AMI you're launching
+    to find the best spot price. This must be one of: `Linux/UNIX`, `SUSE Linux`,
     `Windows`, `Linux/UNIX (Amazon VPC)`, `SUSE Linux (Amazon VPC)`,
     `Windows (Amazon VPC)`