Adding https checks for HAproxy and Galera

[wejdross]

What this PR does / why we need it:

• This change ensures that Haproxy sends traffic only to nodes with wsred_ready=1 metric set. It queries prometheus endpoint and parses config. Buffer size for haproxy was raised from 16kB to 300kB, otherwise Haproxy can't "see" metrics.

Checklist

• Chart Version bumped
• I have run make docs
• Variables are documented in the values.yaml using the format required by Helm-Docs.
• PR contains the label that identifies the chart, e.g. chart/<chart-name>
• PR contains the label that identifies the type of change, which is one of
  [ bug, enhancement, documentation, change, breaking, dependency ]

[TheBigLee]

I thought about that approach for a while now, and I don't think this is a good idea to use the metrics exporter for this:

• The result from the exporter can get "huge" for really big instances with loads of databases and tables and the 300k in tune.bufsize might not be enough.
• It puts a lot of strain on the haproxy if he has to parse the result every time for that mysql_global_status_wsrep_ready string. Especially for big clusters with loads of metrics
• It puts unnecessary strain on the cluster as it queries the endpoint every second.
• If the exporter fails (or the query is too slow or too big), the whole instance goes down.

[wejdross]

so what do You suggest? should we write custom sidecar? how is that solution better?

[TheBigLee]

so what do You suggest? should we write custom sidecar? how is that solution better?
Yes and using a script to determine if the galera node is up and running.

Also using mysql_global_status_wsrep_ready might not be enough to determine if a node is ready to accept connections. Afaik this value is true as well if the node is in the DONOR state. However, the node is blocked during that state and can't accept queries. Which again will result in downtimes as haproxy assumes the node is up.

[wejdross]

I took info from docs:

Whether the server is ready to accept queries. If this status is OFF, almost all of the queries will fail with:
ERROR 1047 (08S01) Unknown Command

Ready to receive queries sounds like it should be ready to receive queries :D
But if You don't like this solution, what do You suggest? custom sidecar parsing metrics and exposing http endpoint with statuses 200/503?

the only reason why I like solution with sidecar is this buffer which is hard to estimate and 244kB is not that small amout. but saving memory in haproxy would cause that we need to run 3 sidecars on each database engine consuming resources as well.

It's Your call @TheBigLee I'm OK with both solutions

[zugao]

Can this be some kind of an alternative solution ?

[wejdross]

Can this be some kind of an alternative solution ?

unfortunately not, whole thing is to detect when galera is ready, not mariadb itself :/

[zugao]

Can this be some kind of an alternative solution ?

unfortunately not, whole thing is to detect when galera is ready, not mariadb itself :/

But MariaDB will not respond ok if galera cluster is not working on that node no?

[TheBigLee]

Can this be some kind of an alternative solution ?

unfortunately not, whole thing is to detect when galera is ready, not mariadb itself :/

But MariaDB will not respond ok if galera cluster is not working on that node no?

It can actually. MariaDB can respond ok but not accept queries (eg. when in DONOR mode)
Furthermore, this is a haproxy enterprise feature.

[zugao]

Unfortunately I have limited knowledge with reverse proxies / load balancers such as HAProxy and Nginx. Nevertheless I took some time and read relevant docs and some blogs. There are 2 facts:

1. HAProxy supports running a script to check on galera nodes.
2. Increasing the tune.bufsize is strongly not recommended.

Now my 2 cents on these:

1. Since this approach does not imply changing shady parameters I definitely would go with it, especially in our case where we might have huge databases. Now how we want to do it - via a simple script or full blown web server, we can discuss and decide as a team.
2. From the docs it seems that changing the value of this parameter implies that other parameters have to be adjusted as well, otherwise nasty things may happen. My main concern is that later on we might have hidden issues because of this parameter which could take us an unbelievable amount of time to figure out the cause.

Conclusion:
I don't see this improvement as a serious reason to change a controversial parameter with 20x the default value, the risk is not justified. Either we do it properly or don't do it at all.

@Kidswiss we might also want to hear from you on this sensitive topic.

[Kidswiss]

I agree with the consent here, given that the output of the exporter is so huge and we need to adjust not recommended settings, we should look for an alternative.

IMHO a script check would be the optimal solution here, the script could connect to the instance, query the wsrep status (or other relevant things, like donor status) and return back the readiness of a given node.

I would not go with webserver that exports that information again. Then we'll have yet another long running thing that could break. Also the indirect nature of that approach would probably introduce new edge cases.

[wejdross]

Short summary:

• extended script logic to galera and galerak8s
• added Dockerfile that builds haproxy image with mysql command required to our check
• tested it locally with helm template and it looks good
• tested logic in dev cluster by editing deployments, not by installing new ones

I'd be very happy to get some critic as I'm very inexperienced in helm charts development and I don't even know what should I look for.

[Kidswiss]

I'm also no helm expert.

Just one nit, the rest looks good.

[Kidswiss]

Can't helm read from a file and insert the string here?

Having the script separate in a readable form would greatly help the reviewability/maintainability of this script.

[TheBigLee]

+1 on that. I would try to do this via file if possible.

[wejdross]

As a compromise I fixed formatting - wdyt?

[Kidswiss]

A separate file would make it easier to check with shellcheck.

There's a way to read files in helm: https://helm.sh/docs/chart_template_guide/accessing_files/

Something like this should do the trick:

data:
  script.sh: |-
        {{ .Files.Get "script.sh" }}

[TheBigLee]

+1 on that. I would try to do this via file if possible.

[TheBigLee]

The script doesn't seem to work in the current state:
Haproxy logs the following error:

Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: Enter password: ERROR 2002 (HY000): Can't connect to MySQL server on '11.33.187.2' (115)
[WARNING]  (1) : Process 21639 exited with code 1 (Exit)

[zugao]

I would set the version of the helm chart to 2.4.0.

[TheBigLee]

Suggested change

	echo Error: wsrep_ready is not ON, actual status is: $WSREP_READY
	echo Error: wsrep_ready is not ON, actual status is: $wsrep_ready

[TheBigLee]

Please run your script through shellcheck when the script is finished to catch some bash specific issues.