Fix default installation without sidecar injection

[guillaumeblaquiere]

The default installation doesn't work as described in the documentation.

I fixed it like this

• manifest apply no longer exists in istioctl CLI.
• Add runAsRoot to allow targetPort creation bellow 1024.

[knative-prow-robot]

Welcome @guillaumeblaquiere! It looks like this is your first PR to knative/docs 🎉

[knative-prow-robot]

Hi @guillaumeblaquiere. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

[nak3]

Thank you @guillaumeblaquiere

This change makes change for v1.6+, but actually our CI still does not run against Istio 1.6 or later 😢 What should we do for this @JRBANCEL @tcnghia ?

/area network

[knative-prow-robot]

@nak3: The label(s) area/network cannot be applied, because the repository doesn't have them

In response to this:

Thank you @guillaumeblaquiere

This change makes change for v1.6+, but actually our CI still does not run against Istio 1.6 or later 😢 What should we do for this @JRBANCEL @tcnghia ?

/area network

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.

[nak3]

/area networking

[knative-prow-robot]

@nak3: The label(s) area/networking cannot be applied, because the repository doesn't have them

In response to this:

/area networking

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.

[JRBANCEL]

Why do we need this?
Last time I installed Istio (1.6.x), it opens ports in the 1024+ range and the service maps the real port to it.
For example, port 80 is mapped to 8080.
Is this not the case in 1.5?

[guillaumeblaquiere]

I tested early in august, with version 1.6.x it worked (I prepared a demo for the GDG Cloud Melbourne). Recently, I tested the 1.7 version (for a demo to the GDG Nantes (a city in France), tonight: 6pm CEST) and it didn't work. I fixed like this. There might be a better/nicer way.

[julz]

better than runAsRoot is to modify the configuration so that the requested targetPorts are high enough not to require root. Modifying the ports section of Istio-minimal-operator.yaml to look like the below should work with 1.7:

            - name: status-port
              port: 15021
              targetPort: 15021
            - name: http2
              port: 80
              targetPort: 8080
            - name: https
              port: 443
              targetPort: 8443

[guillaumeblaquiere]

@julz, I committed a new version thank to your comment after having testing it!

[mpetason]

/ok-to-test

[upodroid]

I spent several hours debugging around till I noticed the cluster-local-gateway can't bind to privileged ports.

Fresh knative install with istio 1.7

Can we get this merged ASAP?

[nak3]

@JRBANCEL @tcnghia @ZhiminXiang I was also asked about the status of Istio 1.7 for Knative. I am wondering that we should merge this for 1.7 and put a note about the issue we have seen in CI? (I am happy to send a follow up PR.) We may also want to update istio-latest test to 1.7, though.

[tcnghia]

+1 on that.

I think at this point I'd go with testing 1.7 in latest with reduced parallelism in place. stable will still be older but at least we have some coverage.

[julz]

FWIW we've seen some big improvements in our load testing of knative with istio 1.7 with this fix applied: istio/istio#26918. It's not in 1.7, but it should be in 1.7.1 in a few days. The tl;dr is there was a race when Endpoints get created before Service that can sometimes cause the istio endpoint cache never to get updated, and this situation seems to occur in knative semi-regularly under load. Maybe this may even fix the problems at higher parallelism, if we're lucky, but either way - fyi.

[guillaumeblaquiere]

Any updates on this? Any helps/comments that I can bring to you?

[nak3]

I was thinking that we should wait for start testing Istio 1.7.1 which will include istio/istio#26985 but maybe we don't need to wait for it?

/lgtm
/assign @tcnghia @ZhiminXiang @JRBANCEL
/hold

[ZhiminXiang]

I think I may miss some context. What Istio versions could this PR apply to?

[nak3]

It depends on istioctl version which users use. But without this PR, installation of istio 1.7 always fail. So this PR is kind of applying to Istio 1.7.

[JRBANCEL]

I don't think we need to start testing 1.7 since with or without this change Istio until 1.6 included works and this unblocks 1.7.

I don't see any risk or downside of this change (except that i might create some downtime but even Istio doesn't seem to care about this 😞)

[tcnghia]

/unhold

(there is no objection to JR suggestion #2775 (comment) )

[tcnghia]

/lgtm

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: guillaumeblaquiere, tcnghia

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:

• OWNERS [tcnghia]

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