search

ContainerSolutions / terraform-examples

Simple and idiomatic examples of various Terraform functions and features.
https://containersolutions.github.io/terraform-examples/
160 stars 52 forks source link

Adding EBS_volume to instance #91

Closed rodrigorras closed 2 years ago

rodrigorras commented 2 years ago

Link related issues:

The following short checklist should be used to make sure your PR is of good quality, and can be merged easily:

Reviewers: @ianmiell @ttarczynski @sanyer

rodrigorras commented 2 years ago

Thanks, @Tomasz Tarczynski @.***> , I will check that.

On Thu, Aug 12, 2021 at 6:01 AM Tomasz Tarczynski @.***> wrote:

@.**** requested changes on this pull request.

Would be good to add just some explanation about ami = (see the comment below) and all the rest looks good to me.

In aws/aws_ebs_volume/volume_attachment/main.tf https://github.com/ContainerSolutions/terraform-examples/pull/91#discussion_r687501962 :

+resource "aws_ebs_volume" "changeme_aws_ebs_volume" {

  • availability_zone = data.aws_availability_zones.changeme_az_list.names[0]
  • size = 5
  • type = "standard"
  • encrypted = false
  • tags = {
  • Name = "changeme_ebs_volume_tag"
  • } +}

  • +# Documentation: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/instance +# Explanation: The AWS 'aws_instance' resource is responsible for allocating an EC2 instance. It's needed as we want to deliver the +# the EBS volume to it. +resource "aws_instance" "changeme_aws_instance" {

  • instance_type = "t2.micro"
  • ami = "ami-0c2b8ca1dad447f8a"

I think this is the simplest way for defining the ami and it's good for this example. Could we add a short explanation here? Something like:

Explanation: AMI IDs are region-specific. This AMI ID is specific to the us-east-1 region. If you use a different region, you will need to change this ID.

ami = "ami-0c2b8ca1dad447f8a" # us-east-1 / Amazon Linux

Also it would make sense to create a separate example for: aws_instance with AMI ID lookup using data "aws_ami" as shown here: Basic Example Using AMI Lookup https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/instance#basic-example-using-ami-lookup

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ContainerSolutions/terraform-examples/pull/91#pullrequestreview-728324448, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABP7HL2BDLLAEVD56S762UTT4OEU7ANCNFSM5B42PK7A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

--

Rodrigo Rios | Junior Cloud Native Engineer

+55 21 99453 0031

De Ruijterkade 143 | Amsterdam, 1011 AC | The Netherlands

https://www.facebook.com/Container-Solutions-1493324030919931/

https://container-solutions.com/