Update the instruction on how to use cluster local gateway by houshengbo · Pull Request #2090 · knative/docs

houshengbo · 2020-01-09T16:02:22Z

Fixes #1402

Proposed Changes

This PR adds the instruction of updating the configmap config-istio under the namespace knative-serving, if the service name for the local gateway is not cluster-local-gateway.

tcnghia · 2020-01-09T17:38:02Z

/lgtm

abrennan89

Some initial review comments

abrennan89 · 2020-01-13T19:53:35Z

 installation method is not ensured. For a production-ready installation, see the `helm` installation method above.

+If you follow either of the above steps, your service and deployment for the local gateway are both named `cluster-local-gateway`,
+and you do not need to update gateway configmap `config-istio` under `knative-serving` namespace, because Knative Serving


New sentence for readability;
"You do not need to update the config-istio configmap because Knative Serving can use the local gateway cluster-local-gateway by default."

abrennan89 · 2020-01-13T19:55:57Z

+and you do not need to update gateway configmap `config-istio` under `knative-serving` namespace, because Knative Serving
+can by default use the local gateway with the name `cluster-local-gateway`.
+
+However, if you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you


Maybe a new heading here to call attention to the user story / task?
e.g. Updating the config-istio configmap to use a non-default local gateway

@samodell wdyt?

+1 to that idea!

I'd remove "However" here

abrennan89 · 2020-01-13T19:57:11Z

+can by default use the local gateway with the name `cluster-local-gateway`.
+
+However, if you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you
+need to update gateway configmap `config-istio` under `knative-serving` namespace. Run the following command:


Run the following command

Replace X field with the custom service

Split this into numbered steps to make it more like a procedure?

samodell · 2020-01-13T20:58:07Z

+and you do not need to update gateway configmap `config-istio` under `knative-serving` namespace, because Knative Serving
+can by default use the local gateway with the name `cluster-local-gateway`.
+
+However, if you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you


+1 to that idea!

samodell · 2020-01-13T20:58:23Z

+can by default use the local gateway with the name `cluster-local-gateway`.
+
+However, if you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you
+need to update gateway configmap `config-istio` under `knative-serving` namespace. Run the following command:


samodell · 2020-01-13T21:00:12Z

 **Note:** This method is only for development purposes. The production readiness of the above 
 installation method is not ensured. For a production-ready installation, see the `helm` installation method above.

+If you follow either of the above steps, your service and deployment for the local gateway are both named `cluster-local-gateway`,


Instead of "either of the above steps," let's clarify the exact types of installs we're referring to -- "If you installed Istio via METHOD or METHOD..."

samodell · 2020-01-21T17:51:43Z

Hi @houshengbo , can you respond to the review comments when you have some time?

carieshmarie

Thanks, Vincent. Looking good - just have one tiny nit - can you incorporate my requested change?

Thanks!

carieshmarie · 2020-02-27T00:27:58Z

+
+### Updating the `config-istio` configmap to use a non-default local gateway

 However, if you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you


Suggested change

However, if you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you

If you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you

Done. @carieshmarie

abrennan89 · 2020-03-06T17:43:24Z

@@ -261,6 +261,54 @@ kubectl apply -f https://raw.githubusercontent.com/knative/serving/master/third_
 **Note:** This method is only for development purposes. The production readiness of the above 


No need to overstate, use something more succinct, e.g.

Note: This installation method is not production ready and is for development purposes only. For a more stable method, use Helm. For more information about the Helm installation method, see [link to helm docs].

abrennan89 · 2020-03-06T17:45:01Z

 installation method is not ensured. For a production-ready installation, see the `helm` installation method above.

+After you install the cluster local gateway, your service and deployment for the local gateway are both named `cluster-local-gateway`.
+You do not need to update the `config-istio` configmap under `knative-serving` namespace, because Knative Serving can use


"under the knative-serving namespace"

"the local gateway cluster-local-gateway by default"

abrennan89 · 2020-03-06T17:45:37Z

+
+### Updating the `config-istio` configmap to use a non-default local gateway
+
+#### Update Gateway Configmap


remove this heading, there's no need to have two headings below each other with no text in between

abrennan89 · 2020-03-06T17:46:07Z

+
+#### Update Gateway Configmap
+
+If you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you


"a custom service"

abrennan89 · 2020-03-06T17:46:23Z

+#### Update Gateway Configmap
+
+If you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you
+need to update gateway configmap `config-istio` under `knative-serving` namespace.


"the knative-serving namespace"

abrennan89 · 2020-03-06T17:47:35Z

+If you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you
+need to update gateway configmap `config-istio` under `knative-serving` namespace.
+
+1. Run the following command:


Doesn't add anything, instead say something like

Edit the config-istio configmap:

abrennan89 · 2020-03-06T17:48:36Z

+```
+
+2. Replace the `local-gateway.knative-serving.cluster-local-gateway` field with the custom service. If you name both
+of the service and deployment after `custom-local-gateway` under the namespace `istio-system`, it should be updated to:


remove 'of' and 'after'

abrennan89 · 2020-03-06T17:49:19Z

+
+#### Update Knative Gateway
+
+If both of the custom service and deployment are labeled with `custom: custom-local-gateway`, not the default


remove 'of'

abrennan89 · 2020-03-06T17:49:49Z

+custom: custom-local-gateway
+```
+
+If there is a change in service ports (compared with that of


abrennan89

Some minor comments

abrennan89

For some reason I can't comment on the configuring DNS part, so I'll comment here.

"Knative dispatches to different services based on their hostname, so it greatly
simplifies things to have DNS properly configured. For this, we must look up the
external IP address that Istio received. This can be done with the following
command:"

Maybe update to something like...
"You must configure your DNS settings to allow Knative to dispatch services based on their hostname.

To do this, you must look up the external IP address received by Istio by entering the following command:"

Some other minor comments in the doc too, but otherwise lgtm! Thanks Vincent.

abrennan89 · 2020-03-19T16:13:41Z


-**Note:** This method is only for development purposes. The production readiness of the above 
-installation method is not ensured. For a production-ready installation, see the `helm` installation method above.
+__Note:__ This installation method is not production ready and is for development purposes only. For a more stable method,


Why change the note format to __ here?

abrennan89 · 2020-03-19T16:14:45Z

+```
+
+If both the custom service and deployment are labeled with `custom: custom-local-gateway`, not the default
+`istio: cluster-local-gateway`, you need to update gateway instance `cluster-local-gateway` in `knative-serving` namespace:


the knative-serving namespace

houshengbo · 2020-03-19T17:23:48Z

@abrennan89 Thx, I just updated the changes.

tcnghia · 2020-03-24T16:44:26Z

/lgtm
/approve

abrennan89 · 2020-06-11T18:54:30Z

@houshengbo is this PR still WIP?

houshengbo · 2020-06-12T15:07:19Z

@abrennan89 I have just rebased and submitted.

nak3 · 2020-06-12T15:38:37Z

@houshengbo
The PR is almost same with https://knative.dev/development/serving/setting-up-custom-ingress-gateway/, it is just for cluster-local-gateway.
Is it better to add it in https://knative.dev/development/serving/setting-up-custom-ingress-gateway/ ?

houshengbo · 2020-06-12T20:44:41Z

@nak3 It is similar, but it is about cluster-local-gateway.
We do not have a separate page for cluster-local-gateway. I am not sure if it should be added into custom ingress gateway.

abrennan89

@houshengbo a few more minor suggestions that you should be able to just accept as commits, then this should be OK to merge.

@duglin do you want to take a quick look to make sure this addresses the linked issue?
Thanks.

abrennan89 · 2020-06-30T18:07:28Z

+If you create a custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you
+need to update gateway configmap `config-istio` under the `knative-serving` namespace.
+
+1. Edit the 1config-istio1 configmap:


Suggested change

1. Edit the 1config-istio1 configmap:

1. Edit the `config-istio` configmap:

abrennan89 · 2020-06-30T18:07:58Z

+```
+
+If both the custom service and deployment are labeled with `custom: custom-local-gateway`, not the default
+`istio: cluster-local-gateway`, you need to update gateway instance `cluster-local-gateway` in the `knative-serving` namespace:


Suggested change

`istio: cluster-local-gateway`, you need to update gateway instance `cluster-local-gateway` in the `knative-serving` namespace:

`istio: cluster-local-gateway`, you must update gateway instance `cluster-local-gateway` in the `knative-serving` namespace:

abrennan89 · 2020-06-30T18:09:39Z

+Knative dispatches to different services based on their hostname, so it greatly
+simplifies things to have DNS properly configured. For this, we must look up the
+external IP address that Istio received. This can be done with the following
+command:


Suggested change

Knative dispatches to different services based on their hostname, so it greatly

simplifies things to have DNS properly configured. For this, we must look up the

external IP address that Istio received. This can be done with the following

command:

Knative dispatches to different services based on their hostname, so it is recommended to have DNS properly configured.

To do this, begin by looking up the external IP address that Istio received:

abrennan89 · 2020-06-30T18:11:45Z

+This external IP can be used with your DNS provider with a wildcard `A` record;
+however, for a basic functioning DNS setup (not suitable for production!) this
+external IP address can be used with `xip.io` in the `config-domain` ConfigMap
+in `knative-serving`. You can edit this with the following command:


Suggested change

This external IP can be used with your DNS provider with a wildcard `A` record;

however, for a basic functioning DNS setup (not suitable for production!) this

external IP address can be used with `xip.io` in the `config-domain` ConfigMap

in `knative-serving`. You can edit this with the following command:

This external IP can be used with your DNS provider with a wildcard `A` record. However, for a basic non-production set up, this external IP address can be used with `xip.io` in the `config-domain` ConfigMap

in `knative-serving`.

You can edit this by using the following command:

duglin · 2020-07-01T00:58:07Z

 ```

+After you install the cluster local gateway, your service and deployment for the local gateway are both named `cluster-local-gateway`.
+You do not need to update the `config-istio` configmap under the `knative-serving` namespace, because Knative Serving can use


This sentence, while I assume is true, feels a bit out of place because it assumes the reader wanted to update the configMap - but why would they? A newbie wouldn't have even thought about doing this so I don't think there's any need to mention it. I think you can remove it.

duglin · 2020-07-01T00:59:36Z

+kubectl edit configmap config-istio -n knative-serving
+```
+
+2. Replace the `local-gateway.knative-serving.cluster-local-gateway` field with the custom service. If you name both


I'd add As an example, in front of the If you name both... sentence so make it clear that custom-local-gateway is just an example.

duglin · 2020-07-01T01:13:16Z

+custom-local-gateway.istio-system.svc.cluster.local
+```
+
+If both the custom service and deployment are labeled with `custom: custom-local-gateway`, not the default


I might have missed it, but why would they have this label? Did we talk about it some place?

I should surely add "As an example" at the beginning. This demonstrates how users should change the label of the cluster-local-gateway into something else.

houshengbo · 2020-07-20T15:42:56Z

@abrennan89 @duglin I have changed the PR based on the comments.
Thank you.

abrennan89 · 2020-07-28T14:45:34Z

/lgtm
/approve

Thanks @houshengbo !

knative-prow-robot · 2020-07-28T14:45:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abrennan89, houshengbo, tcnghia

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:

~~OWNERS~~ [abrennan89,tcnghia]

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

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jan 9, 2020

knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 9, 2020

knative-prow-robot requested review from abrennan89 and samodell January 9, 2020 16:02

knative-prow-robot assigned tcnghia Jan 9, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 9, 2020

abrennan89 suggested changes Jan 13, 2020

View reviewed changes

samodell suggested changes Jan 13, 2020

View reviewed changes

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 26, 2020

abrennan89 approved these changes Feb 26, 2020

View reviewed changes

carieshmarie suggested changes Feb 27, 2020

View reviewed changes

knative-prow-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 2, 2020

mattmoor reviewed Mar 2, 2020

View reviewed changes

abrennan89 reviewed Mar 6, 2020

View reviewed changes

abrennan89 suggested changes Mar 6, 2020

View reviewed changes

abrennan89 reviewed Mar 19, 2020

View reviewed changes

abrennan89 mentioned this pull request Jun 11, 2020

Add abrennan89 to doc-approvers #2579

Merged

3 tasks

knative-prow-robot added approved and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 12, 2020

knative-prow-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 16, 2020

abrennan89 suggested changes Jun 30, 2020

View reviewed changes

abrennan89 requested a review from duglin June 30, 2020 18:19

duglin reviewed Jul 1, 2020

View reviewed changes

Vincent Hou added 9 commits July 20, 2020 11:15

Update the instruction on how to use cluster local gateway

8cc1e93

Update the explanation of updating local gateway based on comments

4828bf3

Fix the minor issue

9dd4491

Add the doc on updating Knative gateway

f747569

Fixed this PR based on latest comments

ea92f7b

Change the desciption for DNS configuration

7f01208

Fix broken links and remove vendor specific section

a24c63b

Refactoring the doc

f1cf607

Change the PR based on comments

1433df3

knative-prow-robot assigned abrennan89 Jul 28, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2020

abrennan89 added this to the 0.17 docs release milestone Jul 28, 2020

knative-prow-robot merged commit 35e5cdb into knative:master Jul 28, 2020


		### Updating the `config-istio` configmap to use a non-default local gateway

		However, if you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you

	However, if you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you
	If you create custom service and deployment for local gateway with a name other than `cluster-local-gateway`, you

		@@ -261,6 +261,54 @@ kubectl apply -f https://raw.githubusercontent.com/knative/serving/master/third_
		Note: This method is only for development purposes. The production readiness of the above


		### Updating the `config-istio` configmap to use a non-default local gateway

		#### Update Gateway Configmap


		#### Update Knative Gateway

		If both of the custom service and deployment are labeled with `custom: custom-local-gateway`, not the default

	1. Edit the 1config-istio1 configmap:
	1. Edit the `config-istio` configmap:

	`istio: cluster-local-gateway`, you need to update gateway instance `cluster-local-gateway` in the `knative-serving` namespace:
	`istio: cluster-local-gateway`, you must update gateway instance `cluster-local-gateway` in the `knative-serving` namespace:

Conversation

houshengbo commented Jan 9, 2020

Proposed Changes

Uh oh!

tcnghia commented Jan 9, 2020

Uh oh!

abrennan89 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samodell commented Jan 21, 2020

Uh oh!

carieshmarie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abrennan89 Mar 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abrennan89 left a comment

Choose a reason for hiding this comment

Uh oh!

abrennan89 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

houshengbo commented Mar 19, 2020

Uh oh!

tcnghia commented Mar 24, 2020

Uh oh!

abrennan89 commented Jun 11, 2020

Uh oh!

houshengbo commented Jun 12, 2020

Uh oh!

nak3 commented Jun 12, 2020

Uh oh!

houshengbo commented Jun 12, 2020

Uh oh!

abrennan89 left a comment

Choose a reason for hiding this comment

abrennan89 Mar 6, 2020 •

edited

Loading