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

add loadbalancer-hostname-only-ingress annotation #92

Merged
merged 3 commits into from
Mar 5, 2021

Conversation

0xch4z
Copy link
Contributor

@0xch4z 0xch4z commented Mar 3, 2021

This annotation will allow users to use short-circuit adding the underlying NodeBalancer's IP to the LoadBalancerStatus (see #87). This is now an opt-in feature as it's much quicker to expose the IPv4 as an interface for the service than waiting for the DNS A record to be propagated for the hostname.

This annotation will allow users to use short-circuit adding the
underlying NodeBalancer's IP to the LoadBalancerStatus (see linode#87). This
is now an opt-in feature as it's much quicker to expose the IPv4 as an
interface for the service than waiting for the DNS A record to be
propagated for the hostname.
@0xch4z 0xch4z requested review from phillc, thorn3r and sibucan March 3, 2021 20:32
Copy link
Contributor

@sibucan sibucan left a comment

Choose a reason for hiding this comment

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

Just a small comment about how the logic for choosing whether to show the NB IP or the hostname based on the annotation:

cloud/linode/loadbalancers.go Show resolved Hide resolved
Copy link
Contributor

@thorn3r thorn3r left a comment

Choose a reason for hiding this comment

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

This looks good. made a small suggestion for the docs just to clarify a bit

README.md Outdated Show resolved Hide resolved
Co-authored-by: Tim Horner <developer.thorner@gmail.com>
@0xch4z 0xch4z merged commit 720f13d into linode:master Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants