-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: Create EIP association for EC2 #372
feat: Create EIP association for EC2 #372
Conversation
@antonbabenko can you do the review, please? |
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.
Almost great, but a few comments to make it more flexible.
@bryantbiggs WDYT?
instance = try( | ||
aws_instance.this[0].id, | ||
aws_instance.ignore_ami[0].id, | ||
null, |
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.
null, |
# Elastic IP | ||
################################################################################ | ||
|
||
resource "aws_eip" "public" { |
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.
resource "aws_eip" "public" { | |
resource "aws_eip" "this" { |
Why call it "public"? Can it be of another type?
|
||
tags = merge(var.tags, var.eip_public_tags) | ||
|
||
depends_on = [ |
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.
There is no need in "depends_on" since it is already having references in "instance" argument.
@@ -85,6 +85,8 @@ module "ec2_complete" { | |||
} | |||
] | |||
|
|||
create_eip_public = true |
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.
Let's add a way to "create EIP", "bring your own EIP" and use aws_eip_association to manage the relations.
# Elastic IP | ||
################################################################################ | ||
|
||
output "eip_public_id" { |
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.
all of these variables and outputs should be stripped of public
- there is no notion of a private elastic IP, its just an elastic IP (eip) and those are public
This PR has been automatically marked as stale because it has been open 30 days |
This PR was automatically closed because of stale in 10 days |
I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Description
Create a Public Elastic IP binding for EC2 instances.
Motivation and Context
With this change it is possible for the EC2 instance to be turned off and when turned on again the Public IP remains the same.
Implementation result:
Breaking Changes
How Has This Been Tested?
examples/*
to demonstrate and validate my change(s)examples/*
projectspre-commit run -a
on my pull request