-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(helm) - Enable configuring service type LoadBalancer with a Static IP #1689
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
Conversation
Signed-off-by: David Mutia <[email protected]>
@mutiadavid Thanks for raising the change and its a valid one. Can you make the following changes so that we can merge this? |
Signed-off-by: David Mutia <[email protected]>
cmdstats_map were on the hotpath and are needed only to update the command stats. Instead, I introduced the stats withing the CommandId itself that we lookup anyways. Also, it removes fragile dependency on naked command name char* pointers. Signed-off-by: Roman Gershman <[email protected]>
@Pothulapati Please let me know whether the merge from main the repository added is what you meant by rebase. Thanks. |
@mutiadavid this PR fails our unit tests. |
@Pothulapati it's possible that our tests are broken due to recent changes with TLS. You must pass |
@romange @Pothulapati Changes to that file haven't been committed to the repo. |
@mutiadavid I'm trying to fix the issue in CI with #1696, Will update you once its merged, for you to rebase. |
@mutiadavid Fix just got merged: #1696. Can you rebase your branch? Thanks! |
@Pothulapati I've merged the updates from main. Thanks for fixing the issue. |
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.
LGTM! Thanks for the contribution @mutiadavid
…ic IP (#1689) * Enable configuring service type LoadBalancer IP Signed-off-by: David Mutia <[email protected]> * Add tests for loadBalancerIP Signed-off-by: David Mutia <[email protected]> * chore: get rid of cmdstats_map (#1687) cmdstats_map were on the hotpath and are needed only to update the command stats. Instead, I introduced the stats withing the CommandId itself that we lookup anyways. Also, it removes fragile dependency on naked command name char* pointers. Signed-off-by: Roman Gershman <[email protected]> --------- Signed-off-by: David Mutia <[email protected]> Signed-off-by: Roman Gershman <[email protected]> Co-authored-by: Roman Gershman <[email protected]>
Enable configuring service type LoadBalancer with a Static IP.
This is very helpful when using cloud providers such as Azure Kubernetes Service (AKS) and you want to assign a static IP to the dragonfly service.