-
Notifications
You must be signed in to change notification settings - Fork 124
NGF: User guide for session persistence #1560
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
base: ngf-release-2.4
Are you sure you want to change the base?
NGF: User guide for session persistence #1560
Conversation
| </tr> | ||
| <tr> | ||
| <td> | ||
| <code>targetRefs</code><br/> | ||
| <em> | ||
| <a href="https://pkg.go.dev/sigs.k8s.io/gateway-api/apis/v1alpha2#LocalPolicyTargetReference"> | ||
| []sigs.k8s.io/gateway-api/apis/v1alpha2.LocalPolicyTargetReference | ||
| </a> | ||
| </em> | ||
| </td> | ||
| <td> | ||
| <p>TargetRefs identifies the API object(s) to apply the policy to. | ||
| Objects must be in the same namespace as the policy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this document was bit outdated but copy-pasted it from generated docs on my feature branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a task in our release process to update this before each release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay good. should remove this change then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seeing some weird diff for this file, I will squash these commits and remove this file when its all ready to merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive documentation for session persistence in NGINX Gateway Fabric, including a new user guide and updates to existing documentation. It explains both NGINX OSS session affinity using ip_hash and NGINX Plus cookie-based session persistence, while also documenting load balancing method configuration through the UpstreamSettingsPolicy resource.
- Adds a new session persistence user guide with practical examples for NGINX OSS and NGINX Plus
- Updates
UpstreamSettingsPolicydocumentation to include load balancing method configuration - Updates API reference to reflect new fields and removed obsolete sections
- Updates Gateway API compatibility and NGINX Plus features documentation
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| content/ngf/traffic-management/upstream-settings.md | Added documentation for configuring load balancing methods via UpstreamSettingsPolicy with examples for random two least_time=header and hash consistent methods |
| content/ngf/traffic-management/session-persistence.md | New comprehensive guide covering session persistence configuration for both NGINX OSS (ip_hash) and NGINX Plus (cookie-based), with working examples |
| content/ngf/reference/api.md | Updated API reference to include new load balancing fields, removed ObservabilityPolicy section, and updated Gateway API version references from v1alpha2 to v1 |
| content/ngf/overview/nginx-plus.md | Added session persistence and advanced load balancing methods to the list of NGINX Plus features |
| content/ngf/overview/gateway-api-compatibility.md | Updated to reflect that sessionPersistence is now supported for HTTPRoute and GRPCRoute |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ```bash | ||
| for i in $(seq 5); do | ||
| echo "Request #$i" | ||
| curl -s -H "Host: cafe.example.com" \ | ||
| http://localhost:8080/coffee \ | ||
| | grep -E 'Server (address|name)' | ||
| echo | ||
| done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't remember how we treat scripting like this in the past, because it probably doesn't work on windows. We did it at one point in our docs, not sure if we just left it or changed it.
@ADubhlaoich any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not a script to run, this can be run directly on commandLine so bash should be okay. I see how it can be confusing, this should be shell, like other documents
|
|
||
| ## Before you begin | ||
|
|
||
| - [Install]({{< ref "/ngf/install/" >}}) NGINX Gateway Fabric with experimental features enabled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem we're going to have with this guide is that part of it requires NGINX Plus, while part of it only requires NGINX OSS.
We probably need two different options depending on what users are able to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, mixing OSS and NGINX Plus in a single flow can be confusing. That's why I define the different sections on how to configure it for plus and oss. Splitting into two documents didn't make sense to me so that was the only option to convey the message while also showcasing difference in behavior.
I have mentioned that Plus is needed in call out section as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be affected by @ADubhlaoich's changes to installation guides being split out? May need multiple references, but I'll let him chime in.
5d35c64 to
946cf7f
Compare
Proposed changes
Checklist
Before sharing this pull request, I completed the following checklist:
Footnotes
Potentially sensitive information includes personally identify information (PII), authentication credentials, and live URLs. Refer to the style guide for guidance about placeholder content. ↩