Allow splitting traffic to same FQDN using multiple traffic split policies matching different routes

[addozhang]

Please describe the Improvement and/or Feature Request

Scope (please mark with X where applicable)

• New Functionality [ ]
• Install [ ]
• SMI Traffic Access Policy [ ]
• SMI Traffic Specs Policy [ ]
• SMI Traffic Split Policy [ ]
• Permissive Traffic Policy [ ]
• Ingress [ ]
• Egress [ ]
• Envoy Control Plane [X]
• CLI Tool [ ]
• Metrics [ ]
• Certificate Management [ ]
• Sidecar Injection [ ]
• Logging [ ]
• Debugging [ ]
• Tests [ ]
• CI System [ ]
• Project Release [ ]

Possible use cases

In 0.7.0, it works fine. But not work in 0.8.0.

I guess it relates with logic https://github.com/openservicemesh/osm/blob/main/pkg/catalog/routes.go#L89.

- apiVersion: split.smi-spec.io/v1alpha2
  kind: TrafficSplit
  metadata:
    labels:
      target: boot
    name: boot-gray
  spec:
    backends:
    - service: boot-prod
      weight: 100
    service: boot
- apiVersion: split.smi-spec.io/v1alpha2
  kind: TrafficSplit
  metadata:
    labels:
      target: boot
    name: boot-gray-282
  spec:
    backends:
    - service: boot-gray
      weight: 100
    service: boot

I think the logic of 'apex service' should be that if 'existing', append to existing WeightedClusters set (make apexServices variable as map, key is MeshService, and value is OutboundTrafficPolicy).

Current logic limits one service can have ONLY one traffice split.

[shashankram]

What's the use case to have multiple traffic splits for the same FQDN? The check you see above is present to ensure conflicting traffic split policies for the same root service are not configured.

To avoid misconfiguration, it is advisable to list all the backends for the root service in the same traffic split policy. I don't see a good use case to support multiple traffic split policies corresponding to the root service. The reason this might be working in v0.7 is because if no checks in place, doesn't mean it was the intended behavior.

@michelleN , do you see a use case for multiple traffic splits for the same root service? Eventually, such ambiguity must be resolved using a validating webhook for SMI.

[addozhang]

@shashankram I think the split is not only applicable for canary release. Canary release usually has traffic splitted between two versions, and is implemented by route. Widely, route can be used among more than two versions, like bellow.

Some times, the environment resources are offten limited. And services may have multi versions being tested in one environment. Assuming that we have a flow A-B-C, and some of them have multi versions: A(A-v1,A-v2, A-main), B-main, C(C-v1, c-v2, c-main), main means the default/main branch.
The route is header with v1 will fall into v1 flow, and v2 is v2, v3 is v3.
Then for FQDN, the will be two traffic splits for service A, one is for v1 and another for v2.

We can call this 'flow dyeing', and the colorful flow likes swiming lane. The traffic may be splited two, maybe more.

I think the route is NOT ONLY for canary/AB/blue-gray testing only. It should be more widely used.

[shashankram]

@shashankram I think the split is not only applicable for canary release. Canary release usually has traffic splitted between two versions, and is implemented by route. Widely, route can be used among more than two versions, like bellow.

Some times, the environment resources are offten limited. And services may have multi versions being tested in one environment. Assuming that we have a flow A-B-C, and some of them have multi versions: A(A-v1,A-v2, A-main), B-main, C(C-v1, c-v2, c-main), main means the default/main branch.
The route is header with v1 will fall into v1 flow, and v2 is v2, v3 is v3.
Then for FQDN, the will be two traffic splits for service A, one is for v1 and another for v2.

We can call this 'flow dyeing', and the colorful flow likes swiming lane. The traffic may be splited two, maybe more.

I think the route is NOT ONLY for canary/AB/blue-gray testing only. It should be more widely used.

Traffic splitting in SMI is simple, you pick the root service's FQDN for which you want to split traffic, and specify the backends to split traffic to. When you decide to change your traffic splitting configuration, you simply update the existing traffic policy spec corresponding to the root service. This is simple and easy to configure.

Allowing multiple traffic split for the same service FQDN opens the door for misconfiguration, and this can become hard for users to spot and debug.

Currently, with v1alpha2 of TrafficSplit, only canary splitting is possible. With v1alpha4 of TrafficSplit, A/B testing with route matching on traffic splitting will be supported. When route matching can be linked with traffic splitting, you can specify a route match on headers, methods etc.

To be able to do traffic splitting based on routes matching, the following 2 issues need to be resolved:
#2368
#2369

@addozhang let me know if this clarifies your request.

[addozhang]

Totally agrees with "Traffic splitting in SMI is simple, you pick the root service's FQDN for which you want to split traffic".
But I think what we should do is not closing the door, it is asking a verification check to avoiding miconfiguration for users insteadl.

The logic in v0.7.0 is ignoring multi splits for one FQDN checing, and combine them at last. That approach is better, and it follows the envoy routing design.

[shashankram]

The logic in v0.7.0 is ignoring multi splits for one FQDN checing, and combine them at last. That approach is better, and it follows the envoy routing design.

SMI is not meant to follow Envoy routing design. OSM implements SMI with Envoy as the proxy. Envoy allowing something does not imply SMI should behave one way or the other.

As I mentioned before, route matching with traffic splitting will be available with v1alpha4 of SMI traffic splitting.

Is there a reason you can't specify all the backends in a single traffic split resource?

[addozhang]

In v1alpha4 split, will it support mult splits for same FQDN?

[shashankram]

In v1alpha4 split, will it support mult splits for same FQDN?

As it stands this is something we haven't planned to support. Could you provide sample SMI traffic split configurations for your use case? I don't follow the example in #2759 (comment).

[shashankram]

@addozhang , I imagine with v1alpha4 of traffic split something like the following will be supported, which will allow splitting same FQDN differently based on routes. I think that's what you are looking for (but not supported with v1alpha2 of split).

kind: TrafficSplit
metadata:
  name: split-1
spec:
  service: foo.bar.svc.cluster.local
  matches:
  - kind: HTTPRouteGroup
    name: route-1
  backends:
  - service: foo-v1
    weight: 10
  - service: foo-v2
    weight: 90
---
kind: TrafficSplit
metadata:
  name: split-2
spec:
  service: foo.bar.svc.cluster.local
  matches:
  - kind: HTTPRouteGroup
    name: route-2
  backends:
  - service: foo-v2
    weight: 100

[shashankram]

@addozhang , I imagine with v1alpha4 of traffic split something like the following will be supported, which will allow splitting same FQDN differently based on routes. I think that's what you are looking for (but not supported with v1alpha2 of split).

kind: TrafficSplit
metadata:
  name: split-1
spec:
  service: foo.bar.svc.cluster.local
  matches:
  - kind: HTTPRouteGroup
    name: route-1
  backends:
  - service: foo-v1
    weight: 10
  - service: foo-v2
    weight: 90
---
kind: TrafficSplit
metadata:
  name: split-2
spec:
  service: foo.bar.svc.cluster.local
  matches:
  - kind: HTTPRouteGroup
    name: route-2
  backends:
  - service: foo-v2
    weight: 100

ref: #2368

[addozhang]

@shashankram yes, that's it. Currently we are developping a UI to implement routes configuring via management TrafficSplit.
As code snippet mentioned in issue description, we create TrafficSplit for each backends individually. In v0.7.0, they are combined to route finally.

[github-actions]

This issue will be closed due to a long period of inactivity. If you would like this issue to remain open then please comment or update.

[github-actions]

Added default label size/needed. Please consider re-labeling this issue appropriately.

[github-actions]

This issue will be closed due to a long period of inactivity. If you would like this issue to remain open then please comment or update.

[github-actions]

Issue closed due to inactivity.