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

Check spot instance is running before trying to attach volumes #12459

Merged
merged 1 commit into from
Mar 7, 2017

Conversation

bodgit
Copy link
Contributor

@bodgit bodgit commented Mar 6, 2017

Currently, given the following manifest snippet:

resource "aws_spot_instance_request" "spot" {
  ...

  ebs_block_device {
    encrypted = true
    ...
  }
}

According to http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-spot-limits.html the Amazon API does not honour the encryption setting on additional volumes. This could be worked around with something like:

resource "aws_spot_instance_request" "spot" {
  wait_for_fulfillment = true
  ...
}

resource "aws_ebs_volume" "volume" {
  encrypted = true
  ...
}

resource "aws_volume_attachment" "attach" {
  volume_id   = "${aws_ebs_volume.volume.id}"
  instance_id = "${aws_spot_instance_request.spot.spot_instance_id}"
  ...
}

However waiting for the spot request to be fulfilled does not guarantee that the instance is running; quite often it's pending for ~ 30-40 seconds which causes the volume attachment to fail. This PR makes the volume attachment poll the instance API to ensure it's running before attempting to attach and fixes the issue.

I lifted the whole resource.StateChangeConf block from the aws_instance resource rather than write a copy of the same refresh function.

I have to include the following disclaimer to keep my employers legal team happy:

This contribution is provided 'as is' and without any warranty or guarantee of any kind, express or implied, including in relation to its quality, suitability for a particular purpose or non-infringement. To the extent permitted by law, in no event shall the creator of this contribution be liable for any claim, damage or other liability, whether arising in contract, tort or otherwise, arising out of or in connection with this contribution.

This covers the scenario of an instance created by a spot request. Using
Terraform we only know the spot request is fulfilled but the instance can
still be pending which causes the attachment to fail.
@stack72
Copy link
Contributor

stack72 commented Mar 7, 2017

Hi @bodgit

Thanks for the work here - this LGTM!

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVolumeAttachment_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/07 16:14:03 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVolumeAttachment_ -timeout 120m
=== RUN   TestAccAWSVolumeAttachment_basic
--- PASS: TestAccAWSVolumeAttachment_basic (131.14s)
=== RUN   TestAccAWSVolumeAttachment_skipDestroy
--- PASS: TestAccAWSVolumeAttachment_skipDestroy (145.10s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	276.266s

Paul

@stack72 stack72 merged commit 3d335e4 into hashicorp:master Mar 7, 2017
stack72 pushed a commit that referenced this pull request Mar 7, 2017
This covers the scenario of an instance created by a spot request. Using
Terraform we only know the spot request is fulfilled but the instance can
still be pending which causes the attachment to fail.
catsby added a commit that referenced this pull request Mar 7, 2017
* 'master' of github.com:hashicorp/terraform:
  provider/datadog: Update to datadog_monitor still used d.GetOk (#12497)
  Check instance is running before trying to attach (#12459)
  Fix aws_dms_replication_task diff for json with whitespace. (#12380)
  add "name" to exported attributes (#12483)
  provider/aws: Adding an acceptance test to for ForceNew on ecs_task_definition volumes
  provider/aws: (#10587) Changing volumes in ECS task definition should force new revision.
  provider/aws: Change aws_spot_fleet_request tests to use the correct hash values in test cases
  Small doc updates (#12165)
  Improve description of consul_catalog_entry (#12162)
  provider/aws: Only send iops when creating io1 devices. Fix docs (#12392)
  provider/google: initial commit for node pool resource (#11802)
  Fix spurious user_data diffs
  Properly handle 'vpc_security_group_ids', drop phantom 'security_groups'
  Default 'ebs_optimized' and 'monitoring' to false
yanndegat pushed a commit to yanndegat/terraform that referenced this pull request Mar 13, 2017
This covers the scenario of an instance created by a spot request. Using
Terraform we only know the spot request is fulfilled but the instance can
still be pending which causes the attachment to fail.
@ghost
Copy link

ghost commented Apr 16, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants