Set "disable-gateway-port-translation" label on local gateway Service #636
Conversation
This was added to Istio in istio/istio#33021. It will likely be available in 1.9.next and 1.10.next, but if the label is present with an older Istio version its not a problem - it will just not have any impact. What this does is avoid the ambiguities we currently have with the Gateway service selection. Currently, if the local gateway is created before the external gateway, we will be broken because the "port 80" Gateway will match the local gateway and get translated to port 8081, when it should be translated to port 8080 of the external gateway. What this label does is tell Istio that the local gateway service is not eligible for this port translation; as a result, Gateways on port 80 will never be mapped to port 8081. Instead, we expect that port 8081 will be used explicitly in the Gateway, as we do here. tl;dr: * Before: NACK if services created in wrong order * After: service order does not matter Note: the "experimental" label may be a red flag - this was intended to be a scary label to dissuade any *other* than Knative from using it. The label was made specifically to support Knative.
|
@howardjohn: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
knative-prow-robot
added
the
size/XS
|
Hi @howardjohn. Thanks for your PR. I'm waiting for a knative-sandbox member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
knative-prow-robot
added
the
needs-ok-to-test
Codecov Report
@@ Coverage Diff @@
## main #636 +/- ##
=======================================
Coverage 80.19% 80.19%
=======================================
Files 18 18
Lines 1116 1116
=======================================
Hits 895 895
Misses 138 138
Partials 83 83 Continue to review full report at Codecov.
|
|
/ok-to-test |
knative-prow-robot
added
ok-to-test
|
/cc @arturenault @julz @dprotaso |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arturenault, howardjohn, nak3 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 |
knative-prow-robot
added
the
approved
|
@howardjohn when does the feature flag switch and we'll be able to drop this label? |
|
Its not so much a feature flag - the label is basically enabling a non-standard behavior specifically to support knative doing a non-standard thing because we don't want to break knative. Once http://gateway-api.org/ is used this problem will go away entirely (see https://docs.google.com/document/d/1gDhj058q2PKKI9SWpxE23ExEXdOkLiueT8JA_oEJ8Mo/edit# for long context on why); the changes we are making there may be backported to the standard Istio Gateway API but there are no current plans to do so. If we did it would be 1.11+. Do you typically allow multiple versions of Istio with a single Knative version? |
There's tests against different versions - ie. #648 But we've been shipping what we deem 'stable' in our release yamls - which is 1.9.x now |
|
Does this mean when installing Istio to support Knative, we need a specific installation instruction? Here is out current installation instruction https://github.com/knative/docs/blob/mkdocs/docs/admin/install/installing-istio.md which seems does not include this extra label. |
|
No, the label is on the knative-local-gateway service which is installed by knative |
This was added to Istio in istio/istio#33021. It
will likely be available in 1.9.next and 1.10.next, but if the label is
present with an older Istio version its not a problem - it will just not
have any impact.
What this does is avoid the ambiguities we currently have with the
Gateway service selection. Currently, if the local gateway is created
before the external gateway, we will be broken because the "port 80"
Gateway will match the local gateway and get translated to port 8081,
when it should be translated to port 8080 of the external gateway.
What this label does is tell Istio that the local gateway service is not
eligible for this port translation; as a result, Gateways on port 80
will never be mapped to port 8081. Instead, we expect that port 8081
will be used explicitly in the Gateway, as we do here.
tl;dr:
Note: the "experimental" label may be a red flag - this was intended to
be a scary label to dissuade any other than Knative from using it. The
label was made specifically to support Knative.
Changes
/kind
Fixes #
Release Note
Docs