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.

Sign up for GitHub

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

Jump to bottom

NGINX Plus: dynamic upstream reloads support #1469

Merged
merged 1 commit into from
Jan 24, 2024
Merged

NGINX Plus: dynamic upstream reloads support #1469

merged 1 commit into from
Jan 24, 2024

Conversation

sjberman
Copy link
Contributor

Problem: One of the benefits of using NGINX Plus is the ability to dynamically update upstream servers using the API. We currently only perform nginx reloads to update upstream servers, which can be a disruptive process.

Solution: If using NGINX Plus, we'll now use the N+ API to update upstream servers. This reduces the amount of reloads that we have to perform, specifically when endpoints change (scaled, for example) with no other changes.

Testing: Verified that state file exists and is updated properly when apps are scaled.

Closes #1332

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@sjberman sjberman requested review from a team as code owners January 16, 2024 17:39
@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request helm-chart Relates to helm chart labels Jan 16, 2024
pleshakov
pleshakov reviewed Jan 17, 2024
View reviewed changes
internal/mode/static/handler.go Outdated Show resolved Hide resolved
internal/mode/static/handler.go Show resolved Hide resolved
internal/mode/static/metrics/collectors/nginx.go Show resolved Hide resolved
internal/mode/static/nginx/conf/nginx-plus.conf Outdated Show resolved Hide resolved
internal/mode/static/nginx/config/upstreams_template.go Outdated Show resolved Hide resolved
internal/mode/static/state/resolver/resolver.go Outdated Show resolved Hide resolved
internal/mode/static/handler.go Outdated Show resolved Hide resolved
site/content/overview/nginx-plus.md Outdated Show resolved Hide resolved
internal/mode/static/nginx/config/upstreams_template.go Outdated Show resolved Hide resolved
internal/mode/static/nginx/runtime/manager.go Outdated Show resolved Hide resolved
kate-osborn
kate-osborn requested changes Jan 17, 2024
View reviewed changes
internal/mode/static/handler.go Outdated Show resolved Hide resolved
internal/mode/static/manager.go Show resolved Hide resolved
internal/mode/static/nginx/config/upstreams.go Outdated Show resolved Hide resolved
internal/mode/static/state/resolver/resolver.go Outdated Show resolved Hide resolved
internal/mode/static/state/store_test.go Show resolved Hide resolved
@pleshakov pleshakov mentioned this pull request Jan 18, 2024
Open
@sjberman sjberman marked this pull request as draft January 18, 2024 23:26
@github-actions github-actions bot removed the helm-chart Relates to helm chart label Jan 18, 2024
@sjberman sjberman marked this pull request as ready for review January 19, 2024 17:47
kate-osborn
kate-osborn requested changes Jan 19, 2024
View reviewed changes
internal/mode/static/handler.go Show resolved Hide resolved
internal/mode/static/handler.go Show resolved Hide resolved
internal/mode/static/nginx/config/http/config.go Outdated Show resolved Hide resolved
internal/mode/static/nginx/runtime/manager.go Show resolved Hide resolved
internal/mode/static/state/change_processor.go Outdated Show resolved Hide resolved
internal/mode/static/state/change_processor.go Outdated Show resolved Hide resolved
internal/mode/static/state/store_test.go Show resolved Hide resolved
ADubhlaoich
ADubhlaoich approved these changes Jan 22, 2024
View reviewed changes
Copy link
Contributor

@ADubhlaoich ADubhlaoich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, with some edit suggestions.

The (Albeit small) NGINX Plus document reeks of marketing jargon to me, but it's hard to avoid it without completely rewriting it.

kate-osborn
kate-osborn approved these changes Jan 22, 2024
View reviewed changes
Copy link
Contributor

@kate-osborn kate-osborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 🚀

pleshakov
pleshakov reviewed Jan 22, 2024
View reviewed changes
internal/mode/static/handler.go Outdated Show resolved Hide resolved
internal/mode/static/handler.go Show resolved Hide resolved
internal/mode/static/handler.go Outdated Show resolved Hide resolved
internal/mode/static/handler_test.go Show resolved Hide resolved
internal/mode/static/nginx/runtime/manager.go Show resolved Hide resolved
internal/mode/static/nginx/runtime/manager.go Outdated Show resolved Hide resolved
internal/framework/events/loop.go Outdated Show resolved Hide resolved
internal/mode/static/state/change_processor.go Show resolved Hide resolved
pleshakov
pleshakov approved these changes Jan 23, 2024
View reviewed changes
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@sjberman
NGINX Plus: dynamic upstream reloads support
10e4959 
Problem: One of the benefits of using NGINX Plus is the ability to dynamically update upstream servers using the API. We currently only perform nginx reloads to update upstream servers, which can be a disruptive process.

Solution: If using NGINX Plus, we'll now use the N+ API to update upstream servers. This reduces the amount of reloads that we have to perform, specifically when endpoints change (scaled, for example) with no other changes.
@sjberman sjberman enabled auto-merge (squash) January 24, 2024 15:24
@sjberman sjberman merged commit d65385d into nginxinc:main Jan 24, 2024
26 checks passed
@sjberman sjberman deleted the enh/plus-dynamic-reloads branch January 24, 2024 15:29
@sjberman sjberman restored the enh/plus-dynamic-reloads branch July 24, 2024 19:39
@sjberman sjberman deleted the enh/plus-dynamic-reloads branch July 24, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pleshakov pleshakov pleshakov approved these changes

@ADubhlaoich ADubhlaoich ADubhlaoich approved these changes

@kate-osborn kate-osborn kate-osborn approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
NGINX Gateway Fabric
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NGINX+ Dynamic Upstream Reloads
4 participants
@sjberman @pleshakov @ADubhlaoich @kate-osborn