roachprod: load balancer postgres URL #121831
Conversation
|
This change is |
herkolategan
changed the title
herkolategan
requested review from
DarrylWong and
renatolabs
and removed request for
a team
pkg/roachprod/install/services.go
Outdated
| return &a, nil | ||
| } | ||
| } | ||
| return nil, nil |
There was a problem hiding this comment.
nit: should we just return a "load balancer not found" error here so that way callers of this function don't need to manually check if address is nil?
There was a problem hiding this comment.
Good point! done.
pkg/roachprod/install/expander.go
Outdated
| e.pgHosts, err = c.pghosts(ctx, l, allNodes(len(c.VMs))) | ||
| switch m[1] { | ||
| case ":L": | ||
| addresses, err := c.ListLoadBalancers(l) |
There was a problem hiding this comment.
Hrmm, don't the semantics of pghost get a bit murky with this implementation of pghost?
Say I create a load balancer for system and tenant in the cluster. What does it mean to say {pghost:L} in that case? Not sure if the List* functions are deterministic (w.r.t. ordering of LBs), in which case it could get even more confusing.
Maybe the "two load balancer" scenario is not common enough. It probably wouldn't hurt if pghost took a virtual cluster name parameter as well.
There was a problem hiding this comment.
True, this won't work well for multiple load balancers. Adding a tenant group to this.
| if err != nil || address == nil { | ||
| return "", err | ||
| } | ||
| loadBalancerURL := c.NodeURL(address.IP, address.Port, virtualClusterName, serviceMode, AuthRootCert) |
There was a problem hiding this comment.
Should we take auth as parameter? I know it will complicate matters for the expanders, but assuming root doesn't seem right either.
There was a problem hiding this comment.
Nice catch, yes we should pass auth. Done. Also updated to use the same auth as the other expanders.
herkolategan
force-pushed
the
hbl/roachprod-lb-expanders
branch
2 times, most recently
from
baf4be0 to
b2bd159
Compare
Add the ability to find a load balancer given the service port it serves. If no port (0) is specified the first load balancer found will be returned. Epic: None Release Note: None
Add ability to get the postgres URL for a load balancer given a service. Epic: None Release Note: None
Add "L" to the node expansion in `roachprod` (e.g., `test-cluster:L` vs. `test-cluster:1`). This allows users to pass "L" instead of a node range / number. It will return the load balancer IP or postgres URL (depending on the expander used) and will enable `workload` and other utilities to target the load balancer IP rather than a specific node. Epic: None Release Note: None
Epic: None Release Note: None
herkolategan
force-pushed
the
hbl/roachprod-lb-expanders
branch
from
b2bd159 to
8cb12bd
Compare
|
TFTR! bors r=DarrylWong |
Add "L" to the node expansion in
roachprod(e.g.,test-cluster:Lvs.test-cluster:1). This allows users to pass "L" instead of a node range /number. It will return the load balancer IP or postgres URL (depending on the
expander used) and will enable
workloadand other utilities to target the loadbalancer IP rather than a specific node.
Epic: None
Release Note: None