Skip to content
This repository was archived by the owner on Jul 11, 2023. It is now read-only.

Supporting assigning EIP to single node ASG instance.#232

Open
Magicloud wants to merge 1 commit intomasterfrom
eip
Open

Supporting assigning EIP to single node ASG instance.#232
Magicloud wants to merge 1 commit intomasterfrom
eip

Conversation

@Magicloud
Copy link
Contributor

@Magicloud Magicloud commented Jul 24, 2019 •
edited
Loading

Tested via single-node-asg-test example. Fixes #205.

Module iam-instance-profile is necessary for this change.

name: Pull request template
about: Make a PR to terraform-aws-foundation

Please include the following in your PR:

Please also note that these are not hard requirements, but merely serve to define
what maintainers are looking for in PR's. Including these will more likely lead
to your PR being reviewed and accepted.

  • Update the changelog
  • Make sure that modules and files are documented. This can be done inside the module and files.
  • Make sure that new modules directories contain a basic README.md file.
  • Make sure that the module is added to tests/main.tf
  • Make sure that the linting passes on CI.
  • Make sure that there is an up to date example for your code:
    - For new modules this would entail example code for how to use the module or some explanation in the module readme.
    - For new examples please provide a README explaining how to run the example. It's also ideal to provide a basic makefile to use the example as well.
  • Make sure that there is a manual CI trigger that can test the deployment.

@Magicloud Magicloud changed the title Tested via single-node-asg-test example. Fixes #205. Supporting assigning EIP to single node ASG instance. Aug 2, 2019
@Magicloud Magicloud requested a review from ketzacoatl August 2, 2019 10:45
ketzacoatl
ketzacoatl suggested changes Dec 9, 2019
View reviewed changes
Copy link
Contributor

@ketzacoatl ketzacoatl left a comment

Choose a reason for hiding this comment

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

@Magicloud, can you split the module updates from the other changes to the examples? those changes can be kept on a branch and we can review again when there's more time.

@Magicloud Magicloud force-pushed the eip branch 2 times, most recently from e160114 to 269149c Compare December 10, 2019 13:21
@Magicloud
Copy link
Contributor Author

Magicloud commented Dec 10, 2019

@ketzacoatl Latest PR contains the modification to single-node-asg only. Please review.

ketzacoatl
ketzacoatl suggested changes Dec 20, 2019
View reviewed changes
Copy link
Contributor

@ketzacoatl ketzacoatl left a comment

Choose a reason for hiding this comment

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

Please see the comments inline.

modules/single-node-asg/main.tf

module "init-install-awscli" {
source = "../init-snippet-install-awscli"
}
Copy link
Contributor

@ketzacoatl ketzacoatl Dec 11, 2019

Choose a reason for hiding this comment

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

Does this snippet make the module ubuntu-only? or would this still work with centos?

Copy link
Contributor Author

@Magicloud Magicloud Dec 23, 2019

Choose a reason for hiding this comment

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

Yes, Ubuntu only. I think it won't be hard to support CentOS.

Copy link
Contributor

@qrilka qrilka Apr 27, 2020

Choose a reason for hiding this comment

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

@Magicloud will we support CentOS?

Copy link
Contributor

@qrilka qrilka May 22, 2020

Choose a reason for hiding this comment

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

@Magicloud any update on this?

@qrilka
Copy link
Contributor

qrilka commented Apr 10, 2020

@Magicloud what's the status of this PR?

@Magicloud
Copy link
Contributor Author

Magicloud commented Apr 13, 2020

@qrilka Have not attended to this for a while. Last I checked, the function worked. But now it contains conflicts with master. Will repair.

qrilka
qrilka reviewed Apr 14, 2020
View reviewed changes
@Magicloud Magicloud force-pushed the eip branch 2 times, most recently from fa52cd5 to f092273 Compare April 17, 2020 13:56
@Magicloud
New function: single-node-asg module supports binding EIP by itself.
8e9e53e 
Since it is single node, binding an EIP to the instance is possible. And
it eases other things since the public interface is constant.

Add assign_eip variable to single-node-asg. If turns it on, an EIP will
be allocated, and assocated with the instance.

Scope VPC is specified in case the account does not have a default VPC.
@qrilka
Copy link
Contributor

qrilka commented Apr 27, 2020

This was tested in a couple of cases using the branch https://github.com/fpco/terraform-aws-foundation/tree/ec2-connect-tunnel

@qrilka qrilka mentioned this pull request Apr 28, 2020
Draft
7 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@qrilka qrilka qrilka left review comments

@ketzacoatl ketzacoatl ketzacoatl requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

add support for EIP to single-node-asg module

3 participants

@Magicloud @qrilka @ketzacoatl