-
Notifications
You must be signed in to change notification settings - Fork 82
NGF: Update architecture overview #450
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
NGF: Update architecture overview #450
Conversation
✅ All required contributors have signed the F5 CLA for this PR. Thank you! |
I have hereby read the F5 CLA and agree to its terms |
3e366c8
to
d41b315
Compare
8319e75
to
e95d5f2
Compare
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.
looks like there might be some rebase/merging issues with other files included in the pr, but the gateway-architecture looks good to me! 🚀
Co-authored-by: Saylor Berman <[email protected]>
d41b315
to
c78fee7
Compare
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.
Generally LGTM, though I have some non-blocking suggestions:
I'd break the new block of text into separate paragraphs. It's a dense read at the moment, so line breaks would help a lot.
I like the new diagram (And the fact it's written with Mermaid): I think the Kubernetes API object could do with a lighter colour, however.
The black text on a dark purple background is difficult to read: with the size of the diagram rendered in the browser, we want to make sure that text is as legible as possible.
@ADubhlaoich One thing I'll note with colors, is that in the past we found it difficult to make Mermaid look good in both light and dark modes in browsers. It's an art to figure out, pun intended. |
Proposed changes
Problem: The Gateway architecture document does not reflect the new control plane/ data plane split
Solution: Update the architecture document to reflect the new architecture. This PR also converts the existing diagrams into mermaid from PNG files.
Closes nginx/nginx-gateway-fabric#3234
Checklist
Before merging a pull request, run through this checklist and mark each as complete.
README.md
andCHANGELOG.md
Footnotes
Potentially sensitive changes include anything involving code, personally identify information (PII), live URLs or significant amounts of new or revised documentation. Please refer to our style guide for guidance about placeholder content. ↩