Generate per-Ingress Gateway instead of reconciling the global Ingress Gateway when auto TLS is enabled

[ZhiminXiang]

Currently when auto TLS is enabled, the controller updates the global Gateway to configure TLS. This is not a good pattern because:

1. The global Gateway object could become very big, and exceeds the object size limit.
2. Currently multiple threads try to update the single Gateway, which could be easily conflicted.

This PR moves the TLS servers within a global Gateway to an independent Gateway, and makes VirtualService also attach to it.

This PR is backward compatible, and should not affect the ongoing traffic because from Istio's perspective, the Gateway related configurations are kept as the same.

This PR will not affect any behavior when auto TLS is disabled.

Fix knative/serving#4312

[zhanggbj]

Hi @ZhiminXiang ,
I'm wondering if it is possible to add a new feature flag for splitting gateway instead of using autoTLS flag. As in our user scenario,

• We have autoTLS disabled as we're generating Certificate with our own cert service.
• However, we do want to have splitting gateway as above as so far we may have 500~1000 host https entries in the single Gateway.

[ZhiminXiang]

@zhanggbj I am not sure very sure about your use case.
The Gateway split will be triggered if 1) IngressTLS is set or 2) auto TLS is enabled. So your use case does not fall into either of those 2?

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ZhiminXiang

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 [ZhiminXiang]

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

[ZhiminXiang]

/cc @tcnghia @nak3 @JRBANCEL

[zhanggbj]

@ZhiminXiang
In our case, autoTLS=false, as we are using our own Cert service (I guess this also may be true for others).
and ingressTLS is also empty, if I understand it correctly, you mean this one? https://github.com/knative-sandbox/net-istio/blob/master/vendor/knative.dev/networking/pkg/apis/networking/v1alpha1/ingress_types.go#L47

Here is what we have in knative ingress

spec:
  rules:
  - hosts:
    - blue.ceperfhttp-1.svc.cluster.local
    http:
      paths:
      - splits:
        - appendHeaders:
            Knative-Serving-Namespace: ceperfhttp-1
            Knative-Serving-Revision: blue-tnyrz-1
          percent: 100
          serviceName: blue-tnyrz-1
          serviceNamespace: ceperfhttp-1
          servicePort: 80
        timeout: 10m0s
    visibility: ClusterLocal
  - hosts:
    - blue.ceperfhttp-1.dev-serving.codeengine.dev.appdomain.cloud
    http:
      paths:
      - splits:
        - appendHeaders:
            Knative-Serving-Namespace: ceperfhttp-1
            Knative-Serving-Revision: blue-tnyrz-1
          percent: 100
          serviceName: blue-tnyrz-1
          serviceNamespace: ceperfhttp-1
          servicePort: 80
        timeout: 10m0s
    visibility: ExternalIP
  visibility: ExternalIP

but in facts, we have TLS configs in gateway

  - hosts:
    - '*.ceperfhttp-1.dev-serving.knative.dev.appdomain.cloud'
    port:
      name: https-ceperfhttp-1
      number: 443
      protocol: HTTPS
    tls:
      credentialName: ceperfhttp-1
      mode: SIMPLE
      subjectAltNames: []

[JRBANCEL]

I wonder if it might be more future proof to create a Gateway per ksvc, no matter if TLS is here or not.
This way, it is there, already scoped to the specific hosts. Because otherwise I worried we go from a global gateway to a custom one just because TLS was enabled.

[ZhiminXiang]

I consider the Gateway split as two parts: TLS Gateway split and non-TLS (or HTTP) Gateway Split.

It seems no doubt that we need TLS Gateway split as we don't want to keep reconciling the global Gateway.

For non-TLS Gateway split, I think we need more considerations. Currently we do not provide a way to configure the non-TLS behavior through Kingerss. So all Kingresses have the same HTTP behavior by default (unless users directly change the Gateway). From this perspective, I am a little concerned that splitting non-TLS Gateway may cause performance regression for the default behavior (as Istio pilot now needs merging Gateway).

[JRBANCEL]

How does this work though?
By global Gateway, I am assuming you mean a * Gateway?
In this case, the more specific Gateway will be picked and other ignored, no?

[ZhiminXiang]

The more specific Gateway (or Knative generated Gateway) only has the TLS configuration.
In this PR, I plan to just simply move the TLS configuration from the global Gateway to the more specific Gateway. So for the TLS part, there is no conflict when attaching VirtualService to both global and the more specific Gateway.

And by attaching VirtualService to the global Gateway, I want to retain the non-TLS behavior for the backward compatibility reason.

[JRBANCEL]

/lgtm

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-knative-sandbox-net-istio-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/ingress/ingress.go	80.5%	81.8%	1.3
pkg/reconciler/ingress/resources/gateway.go	86.6%	85.7%	-0.9

[JRBANCEL]

/lgtm