-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
provider/aws: Add resource_aws_volume_attachment #2050
Conversation
}, | ||
}, | ||
}) | ||
} | ||
|
||
func testAccCheckVolumeExists(n string, v *ec2.Volume) resource.TestCheckFunc { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgraded this test while I was here, since I needed a testAccCheckVolumeExists
method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📈 👍
I plan to review the feedback provided, and maybe make an adjustment or two. I suspect we can merge this afternoon or tomorrow 😄 |
That’s wonderful ! great ! -- On May 26, 2015 at 4:37:17 PM, Clint ([email protected]) wrote: I plan to review the feedback provided, and maybe make an adjustment or two. I suspect we can merge this afternoon or tomorrow — |
Create ebs volume lifecycle is just like any other aws resources. TF just needs to wait for availability of any resources (e.g. aws eip ressource needs an aws instance). Exist func must mean the ressource has been well created. |
@phinze refactored – take another look please ALSO – I think the acc. test leaks a volume, but I haven't had time to crack down on that. |
vID, iID, awsErr.Message(), awsErr.Code()) | ||
} | ||
return err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So now that we loop above, there's no eventual consistency here? No need to loop and wait for "attached" or anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean clean clean! 😻 One question, but if the answer is "nope" this LGTM. |
no need to wait for attachment; ebs volume needs just to be created above in the platform creation process. |
…chment * upstream/master: (21 commits) fix typo fix typo, use awslabs/aws-sdk-go Update CHANGELOG.md More internal links in template documentation. providers/aws: Requires ttl and records attributes if there isn't an ALIAS block. Condense switch fallthroughs into expr lists Fix docs for aws_route53_record params Update CHANGELOG.md provider/aws: Add IAM Server Certificate resource aws_db_instance docs updated per #2070 providers/aws: Adds link to AWS docs about RDS parameters. Downgrade middleman to 3.3.12 as 3.3.13 does not exist providers/aws: Clarifies db_security_group usage. "More more" no more! Indentation issue Export ARN in SQS queue and SNS topic / subscription; updated tests for new AWS SDK errors; updated documentation. Changed Required: false to Optional: true in the SNS topic schema Initial SNS support correct resource name in example added attributes reference section for AWS_EBS_VOLUME ...
1f5c038: now with documentation! |
just used this and it works great |
👍 x 💯 LGTM! |
Thx ! Julien
|
Thanks for the validation @mzupan ! Merging... |
provider/aws: Add resource_aws_volume_attachment
It seems that the volume is attached after the provisioning of the instance. Is it expected ? If yes, how can we managed the bootstrap of the volume ? |
It's right and the expected behavior during the instance boot. Envoyé de mon iPhone
|
Thanks. Do you have an example on how to manage the bootstrap of the volume ? (mkfs, etc etc) |
@Pryz one way would be to add a resource "aws_volume_attachment" "ebs_att" {
device_name = "/dev/sdh"
volume_id = "${aws_ebs_volume.example.id}"
instance_id = "${aws_instance.web.id}"
// this will run after the attachment is made
provisioner "remote-exec" {
connection {
host = "${aws_instance.web.public_ip}"
// ...
}
inline = [ "mkfs -t ext4 /dev/sdh" /* ... */ ]
}
} |
Cloudinit does the thing very well -- On July 27, 2015 at 4:39:32 PM, Paul Hinze ([email protected]) wrote: @Pryz one way would be to add a remote-exec provisioner to the volume attachment itself and give it connection details for the instance, something like this: resource "aws_volume_attachment" "ebs_att" { |
The proposition of @phinze is working. I really think that if we can do that directly with a "ebs_block_device" resource things would be more readable and easier to maintain. Also the @phinze's proposition involve code duplication (about the connection). @julienlavergne The issue I have with cloudinit is that I have a lot of different instances to maintain. Most of the time the only differences (from Terraform) are about EBS. So having a "generic" cloud-init/user-data and manage the EBS with ebs_block_device would be easier :) |
I understand this feature. -- On September 4, 2015 at 12:05:26 PM, Julien Fabre ([email protected]) wrote: The proposition of @phinze is working. I really think that if we can do that directly with a "ebs_block_device" resource things would be more readable and easier to maintain. Also the @phinze's proposition involve code duplication (about the connection). @julienlavergne The issue I have with cloudinit is that I have a lot of different instances to maintain. Most of the time the only differences (from Terraform) are about EBS. So having a "generic" cloud-init/user-data and manage the EBS with ebs_block_device would be easier :) — |
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. |
Allows you to attach an existing EBS volume to an existing Instance.
Usage:
Remaining: