seldon-core icon indicating copy to clipboard operation
seldon-core copied to clipboard

enh: Adding annotation and UrlRewrites property to configure istio routes

Open brightsparc opened this issue 3 years ago • 2 comments
trafficstars

What this PR does / why we need it: Enables providing custom configuration to may url prefix to rewrite configuration for istio VirtualService.

In particular it allows creating the VirtualSerivce with a list of prefixes/model-endpoints/v2/model-one that maps to an endpoints /v2/model-1 which might not have the same model name eg:

apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:
  name: seldon-predictor
spec:
  gateways:
  -istio-system/seldon-gateway
  hosts:
  - "*"
  http:
  - route:
    - destination:
        host: seldon-predictor.canary.svc.cluster.local
        port:
          number: 8000
    match:
    - uri:
        prefix: /model-endpoints/v2/model-one
    rewrite:
      uri: /v2/model-1

Which issue(s) this PR fixes: Fixes #3470

Special notes for your reviewer: This is an example of SeldonDeployment configuration that includes the new annotations as well as optional urlRewrites dictionary to map specific routes inside the container:

apiVersion: machinelearning.seldon.io/v1
kind: SeldonDeployment
metadata:
  name: example-seldon
  annotations:
    "seldon.io/istio-host": "seldon.io"
    "seldon.io/istio-prefix": "/model-endpoints/v2"
    "seldon.io/istio-rewrite": "/v2"
spec:
  protocol: seldon
  urlRewrites:
    "/model-endpoints/v2/model-one": "/v2/model-1"
    "/model-endpoints/v2/model-two": "/v2/model-2"
  predictors:
  - componentSpecs:
    - spec:
        containers:
        - image: seldonio/mock_classifier:{VERSION}
          name: classifier
    graph:
      name: classifier
      type: MODEL
    name: model
    replicas: 1

brightsparc avatar Aug 24 '22 23:08 brightsparc

Hi @brightsparc. Thanks for your PR.

I'm waiting for a SeldonIO or todo 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 jenkins-x/lighthouse repository.

seldondev avatar Aug 24 '22 23:08 seldondev

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please assign majolo You can assign the PR to them by writing /assign @majolo in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

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

seldondev avatar Aug 24 '22 23:08 seldondev

We plan to close this unless you have time to review further @brightsparc

ukclivecox avatar Jun 20 '23 10:06 ukclivecox

We plan to close this unless you have time to review further @brightsparc

I have worked around this further up the stack, fine to close.

brightsparc avatar Jun 20 '23 23:06 brightsparc

Thanks @brightsparc

ukclivecox avatar Jun 21 '23 06:06 ukclivecox