bhouse-nexthop opened a new pull request, #14636:
URL: https://github.com/apache/cloudstack/pull/14636
### Description
This PR makes the virtual router fail a load balancer config apply when it
can't install the new HAProxy config, instead of reporting success.
`CsLoadBalancer.process()` installed `haproxy.cfg.new` over `haproxy.cfg`
with `CsHelper.copy()`, which logs a failed copy and returns. If the file
couldn't be replaced (an immutable or read-only file, or a full filesystem),
the router reloaded HAProxy on the old file and the config run ended
successfully. The management server then treated the change as applied and
didn't roll it back.
The copy is now done with `shutil.copy2()` directly. When it fails, the
error is logged and an exception is raised, in the same way as for a config
that fails `haproxy -c`. HAProxy isn't reloaded, the config run fails, and the
management server gets a failed answer. `CsHelper.copy()` itself is unchanged,
since its other callers rely on it not raising.
What happens after the raise is the same as for an invalid config today:
- The rest of the iptables pass in that config run, which comes after the
load balancer in `IpTablesExecutor.process()`, is not applied.
- `haproxy.cfg.new` is left in place. The next apply writes it again and
retries the copy.
Fixes: #14633
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] Build/CI
- [x] Test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
### How Has This Been Tested?
- New `systemvm/test/TestCsLoadBalancer.py`, run with pytest on Python 3.12
(`PYTHONPATH=../debian/opt/cloud/bin`):
- a changed config is installed, HAProxy is reloaded once, and the
firewall rules are added;
- when the copy raises `PermissionError`, `process()` raises, HAProxy
isn't reloaded, no firewall rule is added, and `haproxy.cfg` keeps the old
content.
- The second test fails without this change, because the copy error is
swallowed.
- `pycodestyle --max-line-length=179` is clean on the new test. On
`CsLoadBalancer.py` it reports only an E302 that is already there before this
change.
- The bug was found in a lab: with `chattr +i /etc/haproxy/haproxy.cfg` on
the router, `updateLoadBalancerRule` returned success, the router logged `Could
not copy ...` and kept the old config. This change has not been run on a router
yet. To test it live: repeat that step and check that the update fails, the
rule keeps its old values, and HAProxy keeps running on the old config; then
run `chattr -i` and check that the same update succeeds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
