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

gateway-api: Add header modifier and splitting examples #25186

Merged
merged 7 commits into from May 26, 2023
Merged

gateway-api: Add header modifier and splitting examples #25186

merged 7 commits into from May 26, 2023

Conversation

nvibert
Copy link
Contributor

@nvibert nvibert commented Apr 28, 2023 •
edited

Updating Gateway API docs with a couple of use cases:

  • Traffic Splitting
  • HTTP Header Request Modification

@nvibert nvibert requested review from a team as code owners April 28, 2023 08:33
@nvibert nvibert requested review from youngnick and zacharysarah and removed request for a team April 28, 2023 08:33
@maintainer-s-little-helper
Copy link

Commit 5721577e20619c89a4e408d47dad2aa187c26cfa does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Apr 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Apr 28, 2023
@sayboras sayboras changed the title Header Modifier and Splitting use cases gateway-api: Add header modifier and splitting examples Apr 28, 2023
@sayboras sayboras added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. labels Apr 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 28, 2023
zacharysarah
zacharysarah requested changes Apr 29, 2023
View reviewed changes
Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

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

@nvibert 👋🏻 Thanks for these additions. The sections for headers and splitting look good, with some changes for clarity and style.

The new page for deploying the Echo app needs to follow the format of other service mesh installations: prerequisites, installation (or deployment), and examples.

Documentation/network/servicemesh/echo-app.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/gateway-api/header.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/gateway-api/header.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/gateway-api/header.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/gateway-api/header.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/gateway-api/splitting.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/gateway-api/splitting.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/gateway-api/splitting.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/gateway-api/splitting.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/gateway-api/splitting.rst Outdated Show resolved Hide resolved
youngnick
youngnick approved these changes May 16, 2023
View reviewed changes
Copy link
Contributor

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

Took me a while to find the right spot in the deploy preview, but this LGTM from a functionality and features point of view.

zacharysarah
zacharysarah approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

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

@nvibert Thanks for making the changes! ⭐

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 17, 2023
@nvibert
Copy link
Contributor Author

nvibert commented May 17, 2023

/test

@nvibert nvibert force-pushed the pr/nvibert/gatewayapi-docs branch from a40eb21 to 3104fbc Compare May 17, 2023 09:22
@aanm aanm changed the base branch from master to main May 17, 2023 17:00
@aanm aanm requested a review from a team as a code owner May 17, 2023 17:00
@aanm aanm requested a review from tommyp1ckles May 17, 2023 17:00
@aanm
Copy link
Member

aanm commented May 17, 2023

/test

tommyp1ckles
tommyp1ckles approved these changes May 18, 2023
View reviewed changes
Documentation/network/servicemesh/echo-app.rst Outdated

We will use a deployment made of echo servers.

The application will reply to the client and, in the body of the reply, will include information about the pod and node receiving the original request.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Pod, Node should be capitalized

@maintainer-s-little-helper
Copy link

Commit 847cfee83edd228c6667c74f81b4ca3fc51a3818 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 18, 2023
@maintainer-s-little-helper
Copy link

Commit 847cfee83edd228c6667c74f81b4ca3fc51a3818 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@aditighag
Copy link
Member

Hey @nvibert The PR was created against the master branch, but we recently switches to main. André tried to fix the base branch, but for some reason it was created as a merge commit. Can you rebase your PR against main, and do a force push?

@aditighag aditighag removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 18, 2023
@nvibert nvibert force-pushed the pr/nvibert/gatewayapi-docs branch from 5471fab to c629a8d Compare May 19, 2023 09:01
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 19, 2023
@nvibert nvibert force-pushed the pr/nvibert/gatewayapi-docs branch from c629a8d to 10c8640 Compare May 19, 2023 10:27
@nvibert
Copy link
Contributor Author

nvibert commented May 19, 2023

Hey @nvibert The PR was created against the master branch, but we recently switches to main. André tried to fix the base branch, but for some reason it was created as a merge commit. Can you rebase your PR against main, and do a force push?

Thanks @aditighag - done 👍 - hopefully this is OK to merge now. Sorry if I made a mistake earlier ; I am not great with Git 🤦‍♂️

@nvibert nvibert force-pushed the pr/nvibert/gatewayapi-docs branch 3 times, most recently from f98cc6b to 6d2dc55 Compare May 25, 2023 15:07
nvibert and others added 7 commits May 25, 2023 16:35
@nvibert
Header Modifier and Splitting use cases
f15b60f 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert
Fix title overline issue
c951dbd 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert
Fix title underline issue
de61ee2 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert @zacharysarah
Update Documentation/network/servicemesh/gateway-api/header.rst
b0f045b 
Co-authored-by: ZSC <zacharysarah@users.noreply.github.com>
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert @zacharysarah
Update Documentation/network/servicemesh/gateway-api/header.rst
73057ea 
Co-authored-by: ZSC <zacharysarah@users.noreply.github.com>
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert @zacharysarah
Apply suggestions from code review
d221f9e 
Co-authored-by: ZSC <zacharysarah@users.noreply.github.com>
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert
Capitalized Pods and Nodes.
441bb3c 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert nvibert force-pushed the pr/nvibert/gatewayapi-docs branch from 6d2dc55 to 441bb3c Compare May 25, 2023 15:35
@chancez chancez added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 25, 2023
@chancez
Copy link
Contributor

chancez commented May 25, 2023

PR is just docs, docs related CI checks are passing. LGTM.

@squeed squeed merged commit 8599937 into cilium:main May 26, 2023
44 of 45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zacharysarah zacharysarah zacharysarah approved these changes

@tommyp1ckles tommyp1ckles tommyp1ckles approved these changes

@youngnick youngnick youngnick approved these changes

Assignees
No one assigned
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@nvibert @aanm @aditighag @chancez @zacharysarah @tommyp1ckles @youngnick @squeed @sayboras