Skip to content

Add context to network layer install section#3441

Merged
knative-prow-robot merged 4 commits into
knative:mainfrom
snneji:issue-3377
Apr 13, 2021
Merged

Add context to network layer install section#3441
knative-prow-robot merged 4 commits into
knative:mainfrom
snneji:issue-3377

Conversation

@snneji
Copy link
Copy Markdown
Contributor

@snneji snneji commented Apr 8, 2021

Closes #3377

@google-cla google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label Apr 8, 2021
@knative-prow-robot knative-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 8, 2021
@RichardJJG
Copy link
Copy Markdown
Contributor

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2021
Copy link
Copy Markdown
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

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

A few terminology commens, but I'm okay with the current language. If you want this to go in as-is, reply with /hold cancel. If you decide to make further changes, that will remove the /lgtm label and you'll need a re-review (and also to cancel the hold).

/approve
/hold


## Install a networking layer

The networking layer in Knative Serving routes incoming and outgoing traffic for your Knative installation.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd sort of prefer to refer to this as "HTTP router" or "HTTP ingress" (it's more precise, and avoids confusion with e.g. Kubernetes CNI layer).

The networking layer in question here is responsible for incoming requests and the associated responses. It doesn't do anything for outbound traffic from the user containers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @evankanderson for your suggestions. Would it make sense for me to change the wording to something like:

The networking layer in Knative Serving is responsible for incoming requests and the associated responses for your Knative installation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yep, that makes sense, and avoids needing to rewrite a lot more docs.

Comment thread docs/install/install-serving-with-yaml.md Outdated
Comment thread docs/install/install-serving-with-yaml.md Outdated
@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 12, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evankanderson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 13, 2021
@evankanderson
Copy link
Copy Markdown
Member

/hold cancel

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 13, 2021
@knative-prow-robot knative-prow-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 13, 2021
@evankanderson
Copy link
Copy Markdown
Member

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 13, 2021
@knative-prow-robot knative-prow-robot merged commit cf2ee4f into knative:main Apr 13, 2021
@snneji snneji deleted the issue-3377 branch April 14, 2021 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add context as to why users need to add a Networking layer, add a recommendation for prototyping (development/install/any-kubernetes-cluster.md)

4 participants