-
Notifications
You must be signed in to change notification settings - Fork 96
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
Use state file for updating N+ upstreams #2897
Conversation
Problem: When splitting the data and control plane, we want to be able to maintain the ability to dynamically update upstreams without a reload if using NGINX Plus. With the current process, we write the upstreams into the nginx conf but don't reload, then call the API to actually do the update. Writing into the conf will trigger a reload from the agent once we split, however. There were also two bugs: - if metrics were disabled, the nginx plus client was not initialized, preventing API calls from occuring and instead a reload occurred - stream upstreams were not updated with the API Solution: Don't write the upstream servers into the nginx conf anymore when using NGINX Plus. Instead, utilize the `state` file option that the NGINX Plus API will populate with the upstream servers. This way we can just call the API and don't unintentionally reload by writing servers into the conf. Also added support for updating stream upstreams, and fixed the initialization bug.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2897 +/- ##
==========================================
+ Coverage 89.71% 89.74% +0.03%
==========================================
Files 109 109
Lines 11091 11150 +59
Branches 50 50
==========================================
+ Hits 9950 10007 +57
- Misses 1082 1083 +1
- Partials 59 60 +1 ☔ View full report in Codecov by Sentry. |
I thought we want to have the Separate Data and Control Plane work merged into a feature branch, was I mistaken? |
This is technically independent of that work, it's just something that has to be done before the split is complete. But it's not a part of that work. |
Problem: When splitting the data and control plane, we want to be able to maintain the ability to dynamically update upstreams without a reload if using NGINX Plus. With the current process, we write the upstreams into the nginx conf but don't reload, then call the API to actually do the update. Writing into the conf will trigger a reload from the agent once we split, however.
There were also two bugs:
Solution: Don't write the upstream servers into the nginx conf anymore when using NGINX Plus. Instead, utilize the
state
file option that the NGINX Plus API will populate with the upstream servers. This way we can just call the API and don't unintentionally reload by writing servers into the conf.Also added support for updating stream upstreams, and fixed the initialization bug.
Testing: Verified that the state file is set and populated when using NGINX Plus. With OSS, servers are still written to the config.
Closes #2841
Checklist
Before creating a PR, run through this checklist and mark each as complete.
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.