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

Document how to migrate from Ingress to Gateway API #25599

Merged
merged 10 commits into from Jun 14, 2023
Merged

Document how to migrate from Ingress to Gateway API #25599

merged 10 commits into from Jun 14, 2023

Conversation

nvibert
Copy link
Contributor

@nvibert nvibert commented May 22, 2023 •
edited by joestringer 

Document how to migrate from Ingress to Gateway API

@nvibert
first draft
9542985 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert
added files
cf1b012 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert
Added TLS termination migration
7e69b99 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 22, 2023
@nvibert nvibert marked this pull request as ready for review May 22, 2023 18:44
@nvibert nvibert requested review from a team as code owners May 22, 2023 18:44
@nvibert
Added note about ingress annotation migration
c7a7868 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@meyskens meyskens requested a review from youngnick May 25, 2023 08:51
youngnick
youngnick approved these changes May 25, 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.

LGTM, nice work @nvibert!

Documentation/network/servicemesh/ingress-to-gateway/ingress-to-gateway.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/ingress-to-gateway/tls-migration.rst Outdated Show resolved Hide resolved
@nvibert
Changes based on review
3031320 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert
Additional changes based on review
aab0d39 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
zacharysarah
zacharysarah requested changes Jun 7, 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 Good start! ✨ This PR needs extensive revision, but you've built some solid bones.

You have a good conversational style, which makes for good speaking/listening but translates poorly into writing. Focus on using clear, plain language.

There are several sections that don't make sense to me as written. Please rewrite them to be abundantly clear. Err on the side of over-explaining: it's easier to edit plain meanings down than it is to guess what you meant.

Use active voice in the present tense.

Documentation/network/servicemesh/ingress-to-gateway/http-migration.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/ingress-to-gateway/http-migration.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/ingress-to-gateway/http-migration.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/ingress-to-gateway/http-migration.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/ingress-to-gateway/http-migration.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/ingress-to-gateway/tls-migration.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/ingress-to-gateway/tls-migration.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/ingress-to-gateway/tls-migration.rst Show resolved Hide resolved
Documentation/network/servicemesh/ingress-to-gateway/tls-migration.rst Outdated Show resolved Hide resolved
Documentation/network/servicemesh/ingress-to-gateway/tls-migration.rst Outdated Show resolved Hide resolved
nvibert and others added 4 commits June 7, 2023 21:06
@nvibert @zacharysarah
Apply suggestions from code review
f0a0968 
Co-authored-by: ZSC <zacharysarah@users.noreply.github.com>
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert
Fixed couple minor things following review
03b54a5 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert
Fixed following review
4d0043c 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert
Linter fixed
5159c6e 
Signed-off-by: Nico Vibert <nicolas.vibert@isovalent.com>
@nvibert nvibert requested a review from zacharysarah June 8, 2023 13:41
@nvibert
Copy link
Contributor Author

nvibert commented Jun 8, 2023

I think I've updated all the fields that needed updating @zacharysarah - thanks for the thorough review!

zacharysarah
zacharysarah approved these changes Jun 12, 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 🌟

@youngnick youngnick added release-note/misc This PR makes changes that have no direct user impact. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jun 13, 2023
@youngnick
Copy link
Contributor

Marking as release-note-minor because this is a docs-only change.

Also marking ready-to-merge because docs only and fully reviewed.

@youngnick youngnick added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 13, 2023
@joestringer joestringer changed the title Pr/nvibert/ingresstogatewaydocs Document how to migrate from Ingress to Gateway API Jun 14, 2023
@joestringer joestringer merged commit 2980a31 into cilium:main Jun 14, 2023
46 of 49 checks passed
@ti-mo
Copy link
Contributor

ti-mo commented Jun 14, 2023

Please pay attention to commit history, this PR introduced 9 small fixup commits that should've been squashed into one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meyskens meyskens meyskens left review comments

@youngnick youngnick youngnick approved these changes

@joamaki joamaki joamaki approved these changes

@zacharysarah zacharysarah zacharysarah 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
7 participants
@nvibert @youngnick @ti-mo @joamaki @meyskens @zacharysarah @joestringer