-
Notifications
You must be signed in to change notification settings - Fork 490
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
docs: update tcp guide for v1alpha2 #808
docs: update tcp guide for v1alpha2 #808
Conversation
57a188c
to
d7f1073
Compare
spec: | ||
parentRefs: | ||
- name: my-tcp-gateway | ||
sectionName: foo |
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 section name feels off. Or maybe it is a new thing so it feels that way. Just a comment and no action required.
site-src/v1alpha2/guides/tcp.md
Outdated
@@ -26,5 +22,5 @@ Please note the following: | |||
be resolved as per the [conflict resolution](/v1alpha2/concepts/guidelines#conflicts) guidelines. |
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.
Can we please explain how a route is tied to a specific listener via the sectionName based matching? That may not be obvious and it deserves special attention here because most users won't do the sectionName based attachment for HTTP.
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.
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.
Thanks for the work on this @shaneutt! This mostly LGTM, just a few nits. The line wrapping feels a bit inconsistent in this file, so if you want to update it to be consistently 80 as part of this PR that would be welcome (though not necessary).
94fe465
to
e5635c8
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.
was about to approve but some more nits
lgtm otherwise
013627a
to
c20ba93
Compare
/lgtm nice work @shaneutt. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hbagdi, shaneutt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
In commit c7bb1d6 (kubernetes-sigs#808), we documented how the tcp route match extension point can be used for metadata based tcp matching. Soon after, in commit e3450f7 (kubernetes-sigs#829), this extension point was removed, but the docs remained. Remove this section to reflect the current state of the API.
What type of PR is this?
/kind documentation
What this PR does / why we need it:
This updates the
TCP
guide forv1alpha2
.Which issue(s) this PR fixes:
This resolves the 5th task of #783
Does this PR introduce a user-facing change?: