cli: update generated haproxy config with readiness endpoint #23463
Conversation
|
This change is |
|
Will cherrypick this into 2.0. |
| @@ -107,6 +145,7 @@ listen psql | |||
| bind :26257 | |||
| mode tcp | |||
| balance roundrobin | |||
| {{range .}} server cockroach{{.Desc.NodeID}} {{.Desc.Address.AddressField}} check | |||
| option httpchk GET /health?ready=1 | |||
There was a problem hiding this comment.
Does httpchck work for HTTPS endpoints automatically or do you need to tweak this for secure clusters?
There was a problem hiding this comment.
Tested it with a secure cluster and it works fine.
pkg/cli/haproxy.go
Outdated
| @@ -75,7 +89,31 @@ func runGenHAProxyCmd(cmd *cobra.Command, args []string) error { | |||
| w = f | |||
| } | |||
|
|
|||
| err = configTemplate.Execute(w, nodeStatuses.Nodes) | |||
| fs := flag.NewFlagSet("haproxy", flag.ContinueOnError) | |||
There was a problem hiding this comment.
This logic could use a unit test
There was a problem hiding this comment.
Done, thanks for the suggestion. It caught a bug regarding not reinitializing state between loops.
|
Yeah, I did test this manually. |
There was a problem hiding this comment.
LGTM. If @mberhault is back online it'd be nice to have him confirm this, but it looks good to me.
pkg/cli/haproxy_test.go
Outdated
| Args: []string{"-http-port"}, | ||
| }, | ||
| } | ||
| expected := []haProxyNodeInfo{ |
There was a problem hiding this comment.
Nit, but the convention throughout most of the code base is to include the expected results next to the input that generates them, rather than in separate slices that readers have to mentally join together. You can leave this is as if you want, though.
There was a problem hiding this comment.
No problem! Updated.
Release note (cli change): The generated haproxy config has been extended with readiness checks.
Release note (cli change): The generated haproxy config has been
extended with readiness checks.