Repository navigation
VR: fail the LB config apply when haproxy.cfg cannot be replaced - #14636
bhouse-nexthop wants to merge 1 commit into
Conversation
CsLoadBalancer installs the new HAProxy config with CsHelper.copy(), which logs an IOError and returns. When /etc/haproxy/haproxy.cfg cannot be written (read-only or immutable file, full filesystem), the router logs "Could not copy ... haproxy.cfg.new to ... haproxy.cfg", reloads HAProxy on the old file and reports success. The management server then records the load balancer change as applied although the router still runs the old config, and nothing is rolled back. Copy the file directly and raise when the copy fails, as is already done when the new config fails "haproxy -c". The apply then fails, the router is not reloaded, and the management server sees the failure. Signed-off-by: Brad House <bhouse@nexthop.ai>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.22 #14636 +/- ##
============================================
- Coverage 18.32% 18.31% -0.01%
+ Complexity 16644 16639 -5
============================================
Files 5939 5939
Lines 537768 537768
Branches 65929 65929
============================================
- Hits 98551 98504 -47
- Misses 428008 428063 +55
+ Partials 11209 11201 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Test report: load balancer change when the router can't replace haproxy.cfgSummary: with Environment
Steps and results
Evidence for A
Result: pass. The management server now sees the failed install, rolls the rule back, and the router keeps serving the old, consistent config. Once the file is writable again, the change goes through. Side note, not from this PR: the failed Not covered
|
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()installedhaproxy.cfg.newoverhaproxy.cfgwithCsHelper.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 failshaproxy -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:
IpTablesExecutor.process(), is not applied.haproxy.cfg.newis left in place. The next apply writes it again and retries the copy.Fixes: #14633
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
How Has This Been Tested?
systemvm/test/TestCsLoadBalancer.py, run with pytest on Python 3.12 (PYTHONPATH=../debian/opt/cloud/bin):PermissionError,process()raises, HAProxy isn't reloaded, no firewall rule is added, andhaproxy.cfgkeeps the old content.pycodestyle --max-line-length=179is clean on the new test. OnCsLoadBalancer.pyit reports only an E302 that is already there before this change.chattr +i /etc/haproxy/haproxy.cfgon the router,updateLoadBalancerRulereturned success, the router loggedCould 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 runchattr -iand check that the same update succeeds.