Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spot price optional for SIR, SFR #4424

Merged
merged 1 commit into from
May 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion aws/resource_aws_spot_fleet_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ func resourceAwsSpotFleetRequest() *schema.Resource {
},
"spot_price": {
Type: schema.TypeString,
Required: true,
Optional: true,
ForceNew: true,
},
"terminate_instances_with_expiration": {
Expand Down
106 changes: 106 additions & 0 deletions aws/resource_aws_spot_fleet_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,31 @@ func TestAccAWSSpotFleetRequest_overriddingSpotPrice(t *testing.T) {
})
}

func TestAccAWSSpotFleetRequest_withoutSpotPrice(t *testing.T) {
var sfr ec2.SpotFleetRequestConfig
rName := acctest.RandString(10)
rInt := acctest.RandInt()

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSSpotFleetRequestDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSSpotFleetRequestConfigWithoutSpotPrice(rName, rInt),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckAWSSpotFleetRequestExists(
"aws_spot_fleet_request.foo", &sfr),
resource.TestCheckResourceAttr(
"aws_spot_fleet_request.foo", "spot_request_state", "active"),
resource.TestCheckResourceAttr(
"aws_spot_fleet_request.foo", "launch_specification.#", "2"),
),
},
},
})
}

func TestAccAWSSpotFleetRequest_diversifiedAllocation(t *testing.T) {
var sfr ec2.SpotFleetRequestConfig
rName := acctest.RandString(10)
Expand Down Expand Up @@ -1511,6 +1536,87 @@ resource "aws_spot_fleet_request" "foo" {
`, rName, rInt, rInt, rName)
}

func testAccAWSSpotFleetRequestConfigWithoutSpotPrice(rName string, rInt int) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is in great shape -- only item is that this new test configuration does not look like its being called. If you fix that up then we should be able to get this in 👍

return fmt.Sprintf(`
resource "aws_key_pair" "debugging" {
key_name = "tmp-key-%s"
public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD3F6tyPEFEzV0LX3X8BsXdMsQz1x2cEikKDEY0aIj41qgxMCP/iteneqXSIFZBp5vizPvaoIR3Um9xK7PGoW8giupGn+EPuxIA4cDM4vzOqOkiMPhz5XK0whEjkVzTo4+S0puvDZuwIsdiW9mxhJc7tgBNL0cYlWSYVkz4G/fslNfRPW5mYAM49f4fhtxPb5ok4Q2Lg9dPKVHO/Bgeu5woMc7RY0p1ej6D4CKFE6lymSDJpW0YHX/wqE9+cfEauh7xZcG0q9t2ta6F6fmX0agvpFyZo8aFbXeUBr7osSCJNgvavWbM/06niWrOvYX2xwWdhXmXSrbX8ZbabVohBK41 phodgson@thoughtworks.com"
}

resource "aws_iam_policy" "test-policy" {
name = "test-policy-%d"
path = "/"
description = "Spot Fleet Request ACCTest Policy"
policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [{
"Effect": "Allow",
"Action": [
"ec2:DescribeImages",
"ec2:DescribeSubnets",
"ec2:RequestSpotInstances",
"ec2:TerminateInstances",
"ec2:DescribeInstanceStatus",
"iam:PassRole"
],
"Resource": ["*"]
}]
}
EOF
}

resource "aws_iam_policy_attachment" "test-attach" {
name = "test-attachment-%d"
roles = ["${aws_iam_role.test-role.name}"]
policy_arn = "${aws_iam_policy.test-policy.arn}"
}

resource "aws_iam_role" "test-role" {
name = "test-role-%s"
assume_role_policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "",
"Effect": "Allow",
"Principal": {
"Service": [
"spotfleet.amazonaws.com",
"ec2.amazonaws.com"
]
},
"Action": "sts:AssumeRole"
}
]
}
EOF
}

resource "aws_spot_fleet_request" "foo" {
iam_fleet_role = "${aws_iam_role.test-role.arn}"
target_capacity = 2
valid_until = "2019-11-04T20:44:20Z"
terminate_instances_with_expiration = true
wait_for_fulfillment = true
launch_specification {
instance_type = "m1.small"
ami = "ami-516b9131"
key_name = "${aws_key_pair.debugging.key_name}"
availability_zone = "us-west-2a"
}
launch_specification {
instance_type = "m3.large"
ami = "ami-d06a90b0"
key_name = "${aws_key_pair.debugging.key_name}"
availability_zone = "us-west-2a"
}
depends_on = ["aws_iam_policy_attachment.test-attach"]
}
`, rName, rInt, rInt, rName)
}

func testAccAWSSpotFleetRequestConfigDiversifiedAllocation(rName string, rInt int) string {
return fmt.Sprintf(`
resource "aws_key_pair" "debugging" {
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_spot_instance_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func resourceAwsSpotInstanceRequest() *schema.Resource {

s["spot_price"] = &schema.Schema{
Type: schema.TypeString,
Required: true,
Optional: true,
ForceNew: true,
}
s["spot_type"] = &schema.Schema{
Expand Down
62 changes: 62 additions & 0 deletions aws/resource_aws_spot_instance_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,31 @@ func TestAccAWSSpotInstanceRequest_validUntil(t *testing.T) {
})
}

func TestAccAWSSpotInstanceRequest_withoutSpotPrice(t *testing.T) {
var sir ec2.SpotInstanceRequest
rInt := acctest.RandInt()

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSSpotInstanceRequestDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSSpotInstanceRequestConfig_withoutSpotPrice(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSpotInstanceRequestExists(
"aws_spot_instance_request.foo", &sir),
testAccCheckAWSSpotInstanceRequestAttributesCheckSIRWithoutSpot(&sir),
resource.TestCheckResourceAttr(
"aws_spot_instance_request.foo", "spot_bid_status", "fulfilled"),
resource.TestCheckResourceAttr(
"aws_spot_instance_request.foo", "spot_request_state", "active"),
),
},
},
})
}

func TestAccAWSSpotInstanceRequest_SubnetAndSGAndPublicIpAddress(t *testing.T) {
var sir ec2.SpotInstanceRequest
rInt := acctest.RandInt()
Expand Down Expand Up @@ -374,6 +399,19 @@ func testAccCheckAWSSpotInstanceRequestAttributesValidUntil(
}
}

func testAccCheckAWSSpotInstanceRequestAttributesCheckSIRWithoutSpot(
sir *ec2.SpotInstanceRequest) resource.TestCheckFunc {
return func(s *terraform.State) error {
if *sir.State != "active" {
return fmt.Errorf("Unexpected request state: %s", *sir.State)
}
if *sir.Status.Code != "fulfilled" {
return fmt.Errorf("Unexpected bid status: %s", *sir.State)
}
return nil
}
}

func testAccCheckAWSSpotInstanceRequest_InstanceAttributes(
sir *ec2.SpotInstanceRequest, rInt int) resource.TestCheckFunc {
return func(s *terraform.State) error {
Expand Down Expand Up @@ -547,6 +585,30 @@ func testAccAWSSpotInstanceRequestConfigValidUntil(rInt int, validUntil string)
}`, rInt, validUntil)
}

func testAccAWSSpotInstanceRequestConfig_withoutSpotPrice(rInt int) string {
return fmt.Sprintf(`
resource "aws_key_pair" "debugging" {
key_name = "tmp-key-%d"
public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD3F6tyPEFEzV0LX3X8BsXdMsQz1x2cEikKDEY0aIj41qgxMCP/iteneqXSIFZBp5vizPvaoIR3Um9xK7PGoW8giupGn+EPuxIA4cDM4vzOqOkiMPhz5XK0whEjkVzTo4+S0puvDZuwIsdiW9mxhJc7tgBNL0cYlWSYVkz4G/fslNfRPW5mYAM49f4fhtxPb5ok4Q2Lg9dPKVHO/Bgeu5woMc7RY0p1ej6D4CKFE6lymSDJpW0YHX/wqE9+cfEauh7xZcG0q9t2ta6F6fmX0agvpFyZo8aFbXeUBr7osSCJNgvavWbM/06niWrOvYX2xwWdhXmXSrbX8ZbabVohBK41 phodgson@thoughtworks.com"
}

resource "aws_spot_instance_request" "foo" {
ami = "ami-4fccb37f"
instance_type = "m1.small"
key_name = "${aws_key_pair.debugging.key_name}"

# no spot price so AWS *should* default max bid to current on-demand price

# we wait for fulfillment because we want to inspect the launched instance
# and verify termination behavior
wait_for_fulfillment = true

tags {
Name = "terraform-test"
}
}`, rInt)
}

func testAccAWSSpotInstanceRequestConfig_withLaunchGroup(rInt int) string {
return fmt.Sprintf(`
resource "aws_key_pair" "debugging" {
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/launch_configuration.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ The following arguments are supported:
instance. See [Block Devices](#block-devices) below for details.
* `ephemeral_block_device` - (Optional) Customize Ephemeral (also known as
"Instance Store") volumes on the instance. See [Block Devices](#block-devices) below for details.
* `spot_price` - (Optional) The price to use for reserving spot instances.
* `spot_price` - (Optional; Default: On-demand price) The maximum price to use for reserving spot instances.
* `placement_tenancy` - (Optional) The tenancy of the instance. Valid values are
`"default"` or `"dedicated"`, see [AWS's Create Launch Configuration](http://docs.aws.amazon.com/AutoScaling/latest/APIReference/API_CreateLaunchConfiguration.html)
for more details
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/spot_fleet_request.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ across different markets and instance types.
what you can specify. See the list of officially supported inputs in the
[reference documentation](http://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_SpotFleetLaunchSpecification.html). Any normal [`aws_instance`](instance.html) parameter that corresponds to those inputs may be used.

* `spot_price` - (Required) The bid price per unit hour.
* `spot_price` - (Optional; Default: On-demand price) The maximum bid price per unit hour.
* `wait_for_fulfillment` - (Optional; Default: false) If set, Terraform will
wait for the Spot Request to be fulfilled, and will throw an error if the
timeout of 10m is reached.
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/spot_instance_request.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ resource "aws_spot_instance_request" "cheap_worker" {
Spot Instance Requests support all the same arguments as
[`aws_instance`](instance.html), with the addition of:

* `spot_price` - (Required) The price to request on the spot market.
* `spot_price` - (Optional; Default: On-demand price) The maximum price to request on the spot market.
* `wait_for_fulfillment` - (Optional; Default: false) If set, Terraform will
wait for the Spot Request to be fulfilled, and will throw an error if the
timeout of 10m is reached.
Expand Down