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

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

Merged
merged 2 commits into from
Jan 19, 2024
Merged

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

merged 2 commits into from
Jan 19, 2024

Conversation

frank-f
Copy link
Contributor

@frank-f frank-f commented Dec 6, 2023

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
Copy link
Member

kenyon commented Dec 6, 2023

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

@frank-f
Copy link
Contributor Author

frank-f commented Dec 7, 2023

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
Copy link
Contributor Author

frank-f commented Dec 7, 2023

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

@frank-f
Copy link
Contributor Author

frank-f commented Dec 7, 2023

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.

@bastelfreak bastelfreak added bug Something isn't working and removed tests-fail labels Jan 19, 2024
@bastelfreak bastelfreak merged commit 87b2a42 into voxpupuli:master Jan 19, 2024
14 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashes in VRRP instance names can cause fragment ordering troubles
3 participants