Explore re-allowing multiple shadow deployments (for Istio only as Ambassador doesn't support)

[axsaucedo]

The ability to deploy multiple shadows was removed in 1.3.0 due to Ambassador not supporting multiple shadows, which means that now an error is thrown if more than 2 shadows are created. This issue encompasses exploring whether we can enable the ability to have more than 2 shadow models when istio is enabled but limit it when ambassador is enabled.

[RafalSkolasinski]

It seems that Istio also does not support more than one shadow (mirror): https://github.com/istio/istio/issues/13330

Should we close this one?

[axsaucedo]

@RafalSkolasinski as per discussion closing

[axsaucedo]

Revisiting this and tested to validate whether canary and shadow work together. It seems the limitation of multiple shadows are only on the envoy mirror, but if provided with a canary and shadow, both the canary gets the correct traffic and the shadow gets 100% of the traffic.

Example here:

apiVersion: machinelearning.seldon.io/v1alpha2
kind: SeldonDeployment
metadata:
  labels:
    app: seldon
  name: example
spec:
  name: canary-example
  predictors:
  - componentSpecs:
    - spec:
        containers:
        - image: seldonio/mock_classifier:1.11.2
          imagePullPolicy: IfNotPresent
          name: classifier
        terminationGracePeriodSeconds: 1
    graph:
      children: []
      endpoint:
        type: REST
      name: classifier
      type: MODEL
    name: main
    replicas: 1
    traffic: 75
  - componentSpecs:
    - spec:
        containers:
        - image: seldonio/mock_classifier:1.11.2
          imagePullPolicy: IfNotPresent
          name: classifier
        terminationGracePeriodSeconds: 1
    graph:
      children: []
      endpoint:
        type: REST
      name: classifier
      type: MODEL
    name: canary
    replicas: 1
    traffic: 25
  - componentSpecs:
    - spec:
        containers:
        - image: seldonio/mock_classifier:1.11.2
          imagePullPolicy: IfNotPresent
          name: classifier
        terminationGracePeriodSeconds: 1
    graph:
      children: []
      endpoint:
        type: REST
      name: classifier
      type: MODEL
    name: shadow
    replicas: 1
    shadow: true

[ooooona]

It seems that Istio also does not support more than one shadow (mirror): istio/istio#13330

Should we close this one?

Hi @RafalSkolasinski, I notice that istio 1.20 has supported multi shadow with mirrors, link: https://github.com/istio/istio/issues/46785. Is that possible to support multi shadow in Seldon?

I notice that if we set multi shadow in SeldonDeployment, it will always overwrite the previous one:

        if p.Shadow == true {
            //if there's a shadow then add a mirror section to the VirtualService

            vsvc.Spec.Http[0].Mirror = &istio_networking.Destination{
                Host:   pSvcFqdn,
                Subset: p.Name,
                Port: &istio_networking.PortSelector{
                    Number: uint32(ports[i].httpPort),
                },
            }

            vsvc.Spec.Http[1].Mirror = &istio_networking.Destination{
                Host:   pSvcFqdn,
                Subset: p.Name,
                Port: &istio_networking.PortSelector{
                    Number: uint32(ports[i].grpcPort),
                },
            }

            if p.Traffic > 0 {
                //if shadow predictor's traffic is greater than 0, set the mirror percentage (like https://istio.io/latest/docs/tasks/traffic-management/mirroring/#mirroring-traffic-to-v2) in VirtualService
                vsvc.Spec.Http[0].MirrorPercentage = &istio_networking.Percent{
                    Value: float64(p.Traffic),
                }

                vsvc.Spec.Http[1].MirrorPercentage = &istio_networking.Percent{
                    Value: float64(p.Traffic),
                }
            }

            continue
        }