FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

automatically set ELB_LIST if not provided by wjordan · Pull Request #40 · aws-samples/aws-codedeploy-samples · GitHub

/ aws-codedeploy-samples Public

automatically set ELB_LIST if not provided - #40

Open
wjordan wants to merge 2 commits into
aws-samples:masterfrom
wjordan:elb_list
ConversationCommits (2)ChecksFiles changed
Open

automatically set ELB_LIST if not provided#40
wjordan wants to merge 2 commits into
aws-samples:masterfrom
wjordan:elb_list

Conversation

wjordan commented May 10, 2016

Copy link
Copy Markdown

This PR removes the need to manually set ELB_LIST when not using an auto-balancer, and removes the step in the README.md where this is required.

In deregister_from_elb.sh, if ELB_LIST is empty this PR calls the existing get_elb_list function to automatically find all load balancers the instance is currently registered to, then writes this list to a temp file (/tmp/elblist).

In register_with_elb.sh, if ELB_LIST is empty this PR reads the list of previously-registered load balancers from the temp file (/tmp/elblist).

The behavior is unchanged when an auto-scaling group is used.

yubangxi commented May 10, 2016

Copy link
Copy Markdown
Contributor

Hi,

Thanks for the PR.

Although it might not be the best practice, there could be multiple applications running on the same instance but behind different ELB at the same time. It could be error-prone to automatically deregister and register instance from all ELBs it's bund to. I think it would be helpful if we provide this feature as an opt-in option instead of default behavior.

wjordan commented May 12, 2016

Copy link
Copy Markdown
Author

I've updated the PR to make this feature opt-in.
Also, I found that this feature requires a fix to the get_elb_list function, which I've added in a separate PR #41.

wjordan mentioned this pull request May 12, 2016
Open

surybala commented May 25, 2016

Copy link
Copy Markdown
Contributor

Could you please confirm this contribution is under the terms of the Apache 2.0 license. Thanks.

wjordan commented May 31, 2016

Copy link
Copy Markdown
Author

Confirmed, Apache 2.0 license applies to this contribution.

default50 mentioned this pull request Jun 14, 2016
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL