Issue #3530 Add valid_from and valid_until for aws_spot_instance_request #4018

saravanan30erd · 2018-04-03T09:05:54Z

No description provided.

…s_spot_instance_request

bflad

Hi @saravanan30erd! 👋 Thanks for this contribution. Can you please see the below and also implement an acceptance test?

bflad · 2018-04-03T13:28:11Z

aws/resource_aws_spot_instance_request.go

@@ -12,6 +12,8 @@ import (
 	"github.com/hashicorp/terraform/helper/schema"
 )

+const awsSpotInstanceRequestTimeLayout = "2006-01-02T15:04:05Z"


Instead of creating our own time layout, can we please use time.RFC3339 instead?

Changed the format to time.RFC3339 instead of custom layout.

bflad · 2018-04-03T13:29:53Z

aws/resource_aws_spot_instance_request.go

+				Type:     schema.TypeString,
+				Optional: true,
+				ForceNew: true,
+			}


Switching to RFC3339 gives us an easy plan-time validation function for both of these:

ValidateFunc: validation.ValidateRFC3339TimeString,

Added the validation function from the same package(validators.go).

bflad · 2018-04-03T13:31:03Z

aws/resource_aws_spot_instance_request.go

+		if err != nil {
+			return err
+		}
+		spotOpts.ValidFrom = &valid_from


Minor nitpick: we prefer to use the AWS SDK provided functions for these, e.g. aws.Time(valid_from)

Used the aws.Time function for pointer reference.

bflad · 2018-04-03T13:32:12Z

website/docs/r/spot_instance_request.html.markdown

@@ -61,6 +61,11 @@ Spot Instance Requests support all the same arguments as
  The duration period starts as soon as your Spot instance receives its instance ID. At the end of the duration period, Amazon EC2 marks the Spot instance for termination and provides a Spot instance termination notice, which gives the instance a two-minute warning before it terminates.
  Note that you can't specify an Availability Zone group or a launch group if you specify a duration.
 * `instance_interruption_behavior` - (Optional) Indicates whether a Spot instance stops or terminates when it is interrupted. Default is `terminate` as this is the current AWS behaviour.
+* `valid_until` - The end date and time of the request, in UTC ISO8601 format


Once we switch to RFC3339, we should link to its documentation, e.g. in [RFC3339 format](https://tools.ietf.org/html/rfc3339#section-5.8)

Updated the document with RFC3339 change.

saravanan30erd · 2018-04-06T15:36:02Z

Hi @bflad added the acceptance test for valid_until. All of the custom tests are passed. But the general test is failing.

make testacc TEST=./aws TESTARGS='-run=TestAccAWSSpotInstanceRequest_validUntil'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSSpotInstanceRequest_validUntil -timeout 120m
=== RUN TestAccAWSSpotInstanceRequest_validUntil
--- FAIL: TestAccAWSSpotInstanceRequest_validUntil (212.13s)
testing.go:518: Step 0 error: After applying this step, the plan was not empty:

	DIFF:

	DESTROY/CREATE: aws_spot_instance_request.foo
             .........
             valid_from:                      "0001-01-01T00:00:00Z" => "" (forces new resource)
	 valid_until:                     "2018-04-07T03:16:45Z" => "2018-04-07T03:16:45Z"

Since terraform always expect empty(i.e no changes) during terraform plan after terraform apply.
But here valid_from forcing it to create new resource, because the default value is current time.
In real time if we configure both valid_from and valid_until then terraform plan & destroy works properly, because terraform tracks the same resource due to the value in valid_from.

Not only this test, all the acceptance tests for aws_spot_instance_request are failing since the aws introduced these parameters(valid_from & valid_until).

make testacc TEST=./aws TESTARGS='-run=TestAccAWSSpotInstanceRequest_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSSpotInstanceRequest_basic -timeout 120m
=== RUN TestAccAWSSpotInstanceRequest_basic

--- FAIL: TestAccAWSSpotInstanceRequest_basic (212.76s)
testing.go:518: Step 0 error: After applying this step, the plan was not empty:

	DIFF:

	DESTROY/CREATE: aws_spot_instance_request.foo
            .............
            valid_from:                      "0001-01-01T00:00:00Z" => "" (forces new resource)
	  valid_until:                     "0001-01-01T00:00:00Z" => "" (forces new resource)

We can fix this by setting valid_from during acceptance test, but it seems test always expecting the spot request result as "fulfilled" and if we set valid_from, actual result of the request is "not-scheduled-yet", so the test is failing.

bflad · 2018-04-20T16:15:28Z

@saravanan30erd for now add Computed: true, to both attributes in the schema 👍

saravanan30erd · 2018-04-20T17:10:47Z

@bflad thanks. Changed valid_from and valid_until as computed attributes, so tests are also passed.

bflad

LGTM -- thanks!! 🚀

Test failure unrelated

Tests failed: 1 (1 new), passed: 9
=== RUN   TestAccAWSSpotInstanceRequestInterruptHibernate
--- PASS: TestAccAWSSpotInstanceRequestInterruptHibernate (53.78s)
=== RUN   TestAccAWSSpotInstanceRequestInterruptStop
--- PASS: TestAccAWSSpotInstanceRequestInterruptStop (75.38s)
=== RUN   TestAccAWSSpotInstanceRequest_vpc
--- PASS: TestAccAWSSpotInstanceRequest_vpc (92.12s)
=== RUN   TestAccAWSSpotInstanceRequest_NetworkInterfaceAttributes
--- PASS: TestAccAWSSpotInstanceRequest_NetworkInterfaceAttributes (110.35s)
=== RUN   TestAccAWSSpotInstanceRequest_withBlockDuration
--- PASS: TestAccAWSSpotInstanceRequest_withBlockDuration (112.70s)
=== RUN   TestAccAWSSpotInstanceRequest_validUntil
--- PASS: TestAccAWSSpotInstanceRequest_validUntil (138.11s)
=== RUN   TestAccAWSSpotInstanceRequest_withLaunchGroup
--- PASS: TestAccAWSSpotInstanceRequest_withLaunchGroup (178.15s)
=== RUN   TestAccAWSSpotInstanceRequest_SubnetAndSGAndPublicIpAddress
--- FAIL: TestAccAWSSpotInstanceRequest_SubnetAndSGAndPublicIpAddress (209.23s)
=== RUN   TestAccAWSSpotInstanceRequest_basic
--- PASS: TestAccAWSSpotInstanceRequest_basic (249.55s)
=== RUN   TestAccAWSSpotInstanceRequest_getPasswordData
--- PASS: TestAccAWSSpotInstanceRequest_getPasswordData (318.24s)

bflad · 2018-04-25T15:57:09Z

This has been released in version 1.16.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:30:22Z

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!

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

saravanan30erd changed the title ~~Issue #3530 Add parameters valid_from and valid_until for aws_spot_in…~~ Issue #3530 Add valid_from and valid_until for aws_spot_instance_request Apr 3, 2018

saravanan30erd mentioned this pull request Apr 3, 2018

request-canceled-and-instance-running for the instance created with terraform #3530

Closed

Issue hashicorp#3530 Add parameters valid_from and valid_until for aw…

10db145

…s_spot_instance_request

saravanan30erd force-pushed the issue-3530 branch from 34988be to 10db145 Compare April 3, 2018 09:25

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

bflad added enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. labels Apr 3, 2018

bflad reviewed Apr 3, 2018

View reviewed changes

change time format to RFC3339

fbdcfa8

ghost added size/L Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Apr 5, 2018

added acceptance test(for valid_until)

007e29e

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

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

added computed to true in schema

8ba5807

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

Merge branch 'master' into issue-3530

53c4ab1

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

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

bflad approved these changes Apr 20, 2018

View reviewed changes

bflad merged commit 53c4ab1 into hashicorp:master Apr 20, 2018

bflad added this to the v1.16.0 milestone Apr 20, 2018

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

Update CHANGELOG for #4018

6a378a9

saravanan30erd deleted the issue-3530 branch April 21, 2018 05:21

ghost locked and limited conversation to collaborators Apr 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #3530 Add valid_from and valid_until for aws_spot_instance_request #4018

Issue #3530 Add valid_from and valid_until for aws_spot_instance_request #4018

saravanan30erd commented Apr 3, 2018 •

edited

Loading

bflad left a comment

bflad Apr 3, 2018

saravanan30erd Apr 5, 2018

bflad Apr 3, 2018

saravanan30erd Apr 5, 2018

bflad Apr 3, 2018

saravanan30erd Apr 5, 2018

bflad Apr 3, 2018

saravanan30erd Apr 5, 2018

saravanan30erd commented Apr 6, 2018

bflad commented Apr 20, 2018

saravanan30erd commented Apr 20, 2018

bflad left a comment

bflad commented Apr 25, 2018

ghost commented Apr 6, 2020

Issue #3530 Add valid_from and valid_until for aws_spot_instance_request #4018

Issue #3530 Add valid_from and valid_until for aws_spot_instance_request #4018

Conversation

saravanan30erd commented Apr 3, 2018 • edited Loading

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

saravanan30erd commented Apr 6, 2018

bflad commented Apr 20, 2018

saravanan30erd commented Apr 20, 2018

bflad left a comment

Choose a reason for hiding this comment

bflad commented Apr 25, 2018

ghost commented Apr 6, 2020

saravanan30erd commented Apr 3, 2018 •

edited

Loading