Avoid messing up fragment ordering if instance name contains dashes

voxpupuli / puppet-keepalived

Puppet Module to manage Keepalived

https://forge.puppet.com/puppet/keepalived

Apache License 2.0

49 stars 152 forks source link

Avoid messing up fragment ordering if instance name contains dashes #303

Closed frank-f closed 9 months ago

frank-f commented 11 months ago

Pull Request (PR) description

Config fragments are called 100-$NAME-key (Note: parts are separated by dashes). Now if the following conditions are met, ordering will be messed up:

$NAME contains dashes
you're trying to set up more than one VRRP instance
instance names contain unequal amounts of dashes
one name is a substring of the other

I created another "ordersafe" variable that does not contain dashes. Also I noticed that $_name could still contain colons, slashes, ... because regsubst was just removing the first occurrence. The G flag will fix this, while we're here.

This Pull Request (PR) fixes the following issues

Fixes #302

kenyon commented 11 months ago

I guess the Ubuntu 22.04 test failure is related, not sure why though.

frank-f commented 11 months ago

I don't see how those could be related to my changes. All I can see in the test output is

Dec 06 16:50:32 ubuntu2204-64-puppet8.example.com Keepalived[1185]: Warning - keepalived has no configuration to run

I also don't see any way for me to debug this. Other tests show the contents of keepalived.conf before starting it, but the Ubuntu tests don't. 🤷🏻‍♂️

frank-f commented 11 months ago

BTW, the same tests also fail with the same error in PR #299

frank-f commented 11 months ago

While deploying to our environment today I noticed, that unicast_peers was not working. The second commit fixes that.

However, it will not fix the Ubuntu checks. Those have been broken since Ubuntu 22.04 support was added with PR #297.