Remove -sf 7 from haproxy image entrypoint

[killianmuldoon]

Remove -sf 7 from the haproxy entrypoint. This arg caused an issue in the Cluster API e2e tests causing the load balancer to exit prematurely. More info on the CAPI issue: kubernetes-sigs/cluster-api#8641

[killianmuldoon]

Alternative we could just make the change to remove the 7 and leave the -sf arg. The -W arg is enough to enable workers to spin up when the config is updated, however, and this matches the fix in CAPI.

[killianmuldoon]

Ref: kubernetes-sigs/cluster-api#8685

[neolit123]

a couple of questions.

i don't remember how HA works in kind with haproxy.

if the haproxy config is prepared in advance post parsing the user node config and not iterated on each time a new CP node is added, wouldn't that mean that the -sf and -W flags are not needed - i.e. no dynamic reconfig?

else, if kind plans to support dynamic topology ALA 'kind add node', then it makes sense to keep these?

also, reading the docs, i do not understand the diff between -sf with and without a pid. what is it?

[killianmuldoon]

if the haproxy config is prepared in advance post parsing the user node config and not iterated on each time a new CP node is added, wouldn't that mean that the -sf and -W flags are not needed - i.e. no dynamic reconfig?

This I'm not sure about - but my assumption is you need to load the config twice - once to startup haproxy with the valid config, and a second time to update the existing config with control plane endpoints so a reload would be needed at that point.

also, reading the docs, i do not understand the diff between -sf with and without a pid. what is it?

Sorry - I misremembered the config. You need to do something like the below to get the pid dynamically:

haproxy -f /etc/haproxy.cfg -D -p /var/run/haproxy.pid -sf $(cat /var/run/haproxy.pid)

[aojea]

we don't really use -sf https://docs.haproxy.org/2.2/management.html , and having it harcoded to a number sounds even more suspicious that this was an oversight.

Kind just create the config in the container and send a HUP signal

kind/pkg/cluster/internal/create/actions/loadbalancer/loadbalancer.go

Line 94 in 5dfe280

if err := loadBalancerNode.Command("kill", "-s", "HUP", "1").Run(); err != nil {

but is not dynamically reconfigured, so this lgtm

[aojea]

interestingly -db does not look like something recommended in production @BenTheElder 😄

-db : disable background mode and multi-process mode. The process remains in
foreground. It is mainly used during development or during small tests, as
Ctrl-C is enough to stop the process. Never use it in an init script.

[killianmuldoon]

/retest

[stmcginnis]

/lgtm

Makes sense!

[aojea]

/approve

Thanks for following up and explain all the tests you have done and the conclusions

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, killianmuldoon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [aojea]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment