Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide delegate virtual service to simplify ingress / reduce need for cluster-local-gateway #1003

Open
treyhyde opened this issue Oct 19, 2022 · 8 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@treyhyde
Copy link

treyhyde commented Oct 19, 2022

Originally mentioned here: https://knative.slack.com/archives/C012AK2FPK7/p1660149435405279

One overriding concern I have that limits my knative deployments is the pseudo requirement to pass through so many proxies due to the VirtualService config. I have to pass the traffic from the ingress gateway to the cluster local gateway then to the activator/service. It would be awesome if we can reconfigure the knative generated VirtualService to skip the need for the cluster local gateway.

The -mesh and -ingress VSs seem identical except for the gateway, any reason this isn't one VS with two gateways? In any case, if the only VS that needed updating was a delegate VS we could wire that right in from [our] top level VS and skip that cluster local gateway. That same delegate VS could be used in the existing -ingress and -mesh VS definitions.

As an example, I'll use a VS to delegate a single route to a ksvc.

apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:
  name: example
spec:
  hosts:
  - api.example.com
  gateways:
    - "ops/ingress"
  http:
    - name: "something"
       match:
       - uri:
           prefix: "/something"
       delegate:
         name: something
         namespace: this
   - name: someksvc
      match:
       - uri:
           prefix: "/somethingelse"
      rewrite:
        # Rewrite the original host header to the host header of  service
        # in order to redirect requests to  service.
        authority: ksvc-name.this.svc.cluster.local
      route:
        - destination:
            host: cluster-local-gateway.istio-system.svc.cluster.local
            port:
              number: 80

if Knative would add a VirtualService that is delegatable, the ingress proxy could communicate directly with activator/service without the need of the cluster-local-gateway and the extra hops, the extra network $$ and the extra configuration madness.

Every ksvc gets two VirtualServices.

someksvc-ingress               ["knative-serving/cluster-local-gateway"]   ["someksvc.this","someksvc.this.svc","someksvc.this.svc.cluster.local"]                                       7d2h
someksvc-mesh                  ["mesh"]                                    ["someksvc.this","someksvc.this.svc","someksvc.this.svc.cluster.local"]                                       7d2h

Knative appears to edit both of those VSs for every change. This services can not be used for delegates because they contain host names. If a delegate VS is added with no hosts, we can delegate a route and have the ingress gateway connect to the service directly without the hop through the cluster local gateway. That same delegate VS could then be added as a route for the -ingress and -mesh gateways and only THAT updated every time knative needs to swap between a service and activator.

@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 18, 2023
@dprotaso
Copy link
Contributor

/help
/reopen

@knative-prow knative-prow bot reopened this Feb 17, 2023
@knative-prow
Copy link

knative-prow bot commented Feb 17, 2023

@dprotaso: Reopened this issue.

In response to this:

/help
/reopen

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.

@dprotaso dprotaso added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Feb 17, 2023
@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 18, 2023
@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 20, 2023
@ReToCode
Copy link
Member

/remove-lifecycle stale

@knative-prow knative-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 22, 2023
@dprotaso dprotaso added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Jun 2, 2023
@joke
Copy link

joke commented Feb 7, 2024

on #1255 @dprotaso ask me comment with some use case:

I pretty much think the istio documentation on delegate services is a pretty good use case. I made smaller alterations:

In an micro services environment it is quite common that a single API is composed of multiple knative services. Routing is done based on paths segments of the API.

This delegate approach could be used instead of the work around with the local gateway as described here.

apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: bookinfo
spec:
  hosts:
  - "bookinfo.com"
  gateways:
  - mygateway
  http:
  - match:
    - uri:
        prefix: "/productpage"
    delegate:
       name: productpage
  - match:
    - uri:
        prefix: "/reviews"
    delegate:
        name: reviews

The actual (knative) services must not contain a hosts specification for delegation to work.

apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: productpage
  namespace: nsA
spec:
  http:
  - match:
     - uri:
        prefix: "/productpage/v1/"
    route:
    - destination:
        host: productpage-v1.nsA.svc.cluster.local
  - route:
    - destination:
        host: productpage.nsA.svc.cluster.local
apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: reviews
  namespace: nsB
spec:
  http:
  - route:
    - destination:
        host: reviews.nsB.svc.cluster.local

For knative istio itself could use delegation for its *-ingress and *-mesh virtual services.
The identical parts of each virtual service could be combined into a single base virtual service.
The *-ingress and *-mesh could just delegate to this base virtual service and just add the hosts section.

Regards
Joke

@treyhyde
Copy link
Author

This is still a bit of a sticking point on how to do larger volume knative rollouts but trying to eliminate the extra hops / latency / data xfer associated with going through extra proxies that don't seem to be adding value.

@treyhyde
Copy link
Author

A similar discussion happened in kserve/kserve#1371 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

4 participants