nginx-gateway-fabric
nginx-gateway-fabric copied to clipboard
Remove zone size for invalid backend ref
Proposed changes
Remove zone size for invalid backend ref.
Problem: The invalid-backend-ref size of 32k is too small for some environments causing an inability of the gateway to program.
Solution: Do not specify zone size for invalid-backend-ref
so that NGINX does not share upstream state across backends. This is ok for this specific upstream because we will only ever proxy to one backend.
Testing: Adjusted unit test to match. Deployed NGF with HTTPRoutes and verified it correctly still generates zone sizes for normal upstreams, an upstream missing its endpoint (service is present but no deployment/pod), and invalid-backend-ref
:
upstream default_coffee_80 {
random two least_conn;
zone default_coffee_80 512k;
server 10.244.0.25:8080;
}
upstream default_tea_80 {
random two least_conn;
zone default_tea_80 512k;
server unix:/var/lib/nginx/nginx-502-server.sock;
}
upstream invalid-backend-ref {
random two least_conn;
server unix:/var/lib/nginx/nginx-500-server.sock;
}
Note: This PR does not test if the fix would work in the bug-submitter's environment. Recreating the exact test environment is probably not worth the effort.
Closes #1794
Checklist
Before creating a PR, run through this checklist and mark each as complete.
- [x] I have read the CONTRIBUTING doc
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] I have checked that all unit tests pass after adding my changes
- [ ] I have updated necessary documentation
- [x] I have rebased my branch onto main
- [x] I will ensure my PR is targeting the main branch and pulling from my branch from my own fork
Release notes
If this PR introduces a change that affects users and needs to be mentioned in the release notes, please add a brief note that summarizes the change.
Remove zone size for invalid-backend-ref
I went with @pleshakov 's suggestion:
One approach to increase the zone is to just not specify. In that case, NGINX will not share upstream state across backends. Which is ok for that upstream, since we only proxy to one destination.
Am I missing anything else?
Oops, by changing the constant invalidBackendZoneSize
which is used in the upstreams_test.go
my brain thought of that as me changing the test, but there was no actual change in code in the test file.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 86.71%. Comparing base (
4cb9578
) to head (18d635a
). Report is 1 commits behind head on main.
:exclamation: Current head 18d635a differs from pull request most recent head 80f1b06. Consider uploading reports for the commit 80f1b06 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## main #1931 +/- ##
==========================================
- Coverage 86.72% 86.71% -0.01%
==========================================
Files 88 88
Lines 5972 5971 -1
Branches 50 50
==========================================
- Hits 5179 5178 -1
Misses 741 741
Partials 52 52
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.