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

Fix issue with Network interfaces and an instance-level security groups #1189

Merged
merged 7 commits into from
Mar 13, 2015

Conversation

catsby
Copy link
Contributor

@catsby catsby commented Mar 12, 2015

This should fix #1188, pending current acceptance tests and a new one to account for the issue there.

The configuration I tested:

provider "aws" {
  region="us-west-2"
}

resource "aws_vpc" "foo" {
  cidr_block = "10.1.0.0/16"
}

resource "aws_security_group" "tf_test_foo" {
  name = "tf_test_foo"
  description = "foo"
  vpc_id="${aws_vpc.foo.id}"

  ingress {
    protocol = "icmp"
    from_port = -1
    to_port = -1
    cidr_blocks = ["0.0.0.0/0"]
  }
}

resource "aws_subnet" "foo" {
  cidr_block = "10.1.1.0/24"
  vpc_id = "${aws_vpc.foo.id}"
}

resource "aws_instance" "nat" {
  ami = "ami-21f78e11"
  instance_type = "t1.micro"
  security_groups = ["${aws_security_group.tf_test_foo.id}"]
  subnet_id = "${aws_subnet.foo.id}"
  associate_public_ip_address = true
  source_dest_check = false
  tags {
    Name = "testing-vpc-subnet-things"
  }
}

@catsby
Copy link
Contributor Author

catsby commented Mar 12, 2015

cc @phinze

@jaxxstorm
Copy link

I think you need to associate an elasticip as well:

resource "aws_eip" "nat" {
  instance = "${aws_instance.nat.id}"
  vpc = true
}

That was part of my config. It might not be relevant to the bug, but I know some of the stuff with API issue I read mentioned associating elasticips in there. Definitely worth sanity checking I think.

@mitchellh
Copy link
Contributor

LGTM. It is good we're finding these instance issues, thanks @jaxxstorm :)

@catsby
Copy link
Contributor Author

catsby commented Mar 12, 2015

@jaxxstorm k, trying that. Do you also have an internet gateway? I'm having issues there...

@jaxxstorm
Copy link

yep, I sure do:

resource "aws_internet_gateway" "gw" {
  vpc_id = "${var.vpc_id}"

  tags {
    Name = "${concat(var.vpc_name,"-gateway")}"
  }
}

@kendawg2
Copy link

@jaxxstorm I have this and it works:

resource "aws_internet_gateway" "gw" {
vpc_id = "${aws_vpc.prod.id}"

tags {
    Name = "${var.environment}-Internet Gateway"
}

}

Not really any different except I am not using concat() which really isn't needed for that anymore.

@catsby
Copy link
Contributor Author

catsby commented Mar 12, 2015

Updated the config example:

provider "aws" {
  region="us-west-2"
}

resource "aws_internet_gateway" "gw" {
    vpc_id = "${aws_vpc.foo.id}"
    tags {
        Name = "main"
    }
}

resource "aws_vpc" "foo" {
  cidr_block = "10.1.0.0/16"
}

resource "aws_security_group" "tf_test_foo" {
  name = "tf_test_foo"
  description = "foo"
  vpc_id="${aws_vpc.foo.id}"

  ingress {
    protocol = "icmp"
    from_port = -1
    to_port = -1
    cidr_blocks = ["0.0.0.0/0"]
  }
}

resource "aws_subnet" "foo" {
  cidr_block = "10.1.1.0/24"
  vpc_id = "${aws_vpc.foo.id}"
}

resource "aws_instance" "nat" {
  ami = "ami-21f78e11"
  instance_type = "t1.micro"
  security_groups = ["${aws_security_group.tf_test_foo.id}"]
  subnet_id = "${aws_subnet.foo.id}"
  associate_public_ip_address = true
  source_dest_check = false
  tags {
    Name = "testing-vpc-subnet-things"
  }
}

resource "aws_eip" "nat" {
  instance = "${aws_instance.nat.id}"
  vpc = true
}

Things seem to work as expected. I'm going to make an acceptance test then get this merged!

@phinze
Copy link
Contributor

phinze commented Mar 12, 2015

LGTM

@phinze
Copy link
Contributor

phinze commented Mar 12, 2015

(Actually we're debugging an issue with the destroys in the test... stand by...)

terraform_test_graph

const testAccInstanceNetworkInstanceSecurityGroups = `
resource "aws_internet_gateway" "gw" {
vpc_id = "${aws_vpc.foo.id}"
depends_on = ["aws_eip.foo_eip"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, two things: (1) remove this dependency

@phinze
Copy link
Contributor

phinze commented Mar 13, 2015

Logic for new dependency suggestions:

  • Internet Gateway must be present for instance with public IP address to launch in a VPC, and that instance must be gone by the time the IGW is deleted
  • Internet Gateway must be present for EIP to be attached in a VPC, and that EIP must be detached before IGW is deleted

catsby added 2 commits March 13, 2015 14:39
* master:
  provider/aws: Fix encoding bug with AWS Instance
  minor style cleanups
  Tags Schema
  Added Tagging
  Added vpc refactor in aws sdk go
  Removed additional variable for print, added for debugging
  Using hashicorp/aws-sdk-go
  Changed things around as suggested by @catsby
  Refactor with Acceptance Tests
  VPC Refactor
  First refactor
  Added Connection to config
@catsby
Copy link
Contributor Author

catsby commented Mar 13, 2015

@phinze I think we're good here now, yeah?

@phinze
Copy link
Contributor

phinze commented Mar 13, 2015

@catsby Yep this is good to go.

catsby added a commit that referenced this pull request Mar 13, 2015
Fix issue with Network interfaces and an instance-level security groups
@catsby catsby merged commit 9c7f597 into master Mar 13, 2015
@catsby catsby deleted the b-aws-instance-sec-groups branch March 13, 2015 21:37
@ghost
Copy link

ghost commented May 4, 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 May 4, 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.

Network interfaces and an instance-level security groups may not be specified on the same request
5 participants