Skip to content
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

sap_ha_pacemaker_cluster: Fix haproxy and minor lint issues #898

Merged
merged 3 commits into from
Nov 29, 2024

Conversation

ja9fuchs
Copy link
Contributor

ansible-lint

  • Jinja whitespace fixes
  • noqa added to one task in role sap_ha_install_anydb_ibmdb2 that includes tabs in the code on purpose

haproxy config fix (GCP and IBM Cloud VS)
RHEL: in haproxy 2.4.17 the default systemd service unit was enhanced with an auto-include of the default config file directory using the CFGDIR variable. When creating the unit template for the resources, our role replaced the Environment definition and the CFGDIR variable definition was removed, but not the newly added -f $CFGDIR in the various Exec lines. This broke the start of the unit and resources.

The solution in this PR changes the replaced patterns in the config to make sure the CFGDIR pieces are removed on all lines.

Original excerpt of the unit file section:

[Service]
EnvironmentFile=-/etc/sysconfig/haproxy
Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid" "CFGDIR=/etc/haproxy/conf.d"
ExecStartPre=/usr/sbin/haproxy -f $CONFIG -f $CFGDIR -c -q $OPTIONS
ExecStart=/usr/sbin/haproxy -Ws -f $CONFIG -f $CFGDIR -p $PIDFILE $OPTIONS
ExecReload=/usr/sbin/haproxy -f $CONFIG -f $CFGDIR -c -q $OPTIONS

New config excerpt of the template copy:

[Service]
EnvironmentFile=-/etc/sysconfig/haproxy
Environment="CONFIG=/etc/haproxy/haproxy-%i.cfg" "PIDFILE=/run/haproxy-%i.pid" 
ExecStartPre=/usr/sbin/haproxy -f $CONFIG -c -q $OPTIONS
ExecStart=/usr/sbin/haproxy -Ws -f $CONFIG -p $PIDFILE $OPTIONS
ExecReload=/usr/sbin/haproxy -f $CONFIG -c -q $OPTIONS

The task now replaces the patterns in a loop with a human readable label.

Resulting output:

changed: [localhost] => (item=Replace haproxy.cfg with haproxy-%i.cfg)
changed: [localhost] => (item=Replace haproxy.pid with haproxy-%i.pid)
changed: [localhost] => (item=Delete definition of CFGDIR)
changed: [localhost] => (item=Remove "-f $CFGDIR")

This is only applied to the newly created template, which is a copy of the original service unit. The original remains untouched as before.

-- only locally tested --

Copy link
Contributor

@marcelmamula marcelmamula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from change perspective, but I cannot test it myself.

Copy link
Member

@sean-freeman sean-freeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍 Verification test performed and HAProxy is responding to the Load Balancer Health Check Probe

@ja9fuchs ja9fuchs merged commit 288a0ed into sap-linuxlab:dev Nov 29, 2024
3 of 4 checks passed
@ja9fuchs ja9fuchs deleted the fix-haproxy branch November 29, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants