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

fix virtual service destination routes when match is set #1668

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

luisr-escobar
Copy link

@luisr-escobar luisr-escobar commented Jun 27, 2024

primary destination is duplicated on both virtual service routes when setting match rules as per the istio a/b testing tutorial:

No weights set on analysis object:

  analysis:
    interval: 15s
    iterations: 2
    match:
    - headers:
        x-header-1:
          exact: Test_from_DEVs

The following is generated:

  http:
  - match:
    - headers:
        x-header-1:
          exact: Test_from_DEVs
    route:
    - destination:
        host: podinfo-primary
      weight: 100
    - destination:
        host: podinfo-canary
      weight: 0
  - route:
     - destination:
         host: podinfo-primary
       weight: 100

analysis values with weigths:

  analysis:
    interval: 15s
    match:
    - headers:
        x-header-1:
          exact: Test_from_DEVs
    maxWeight: 50
    stepWeight: 50
    threshold: 2

generates:

  http:
  - match:
    - headers:
        x-header-1:
          exact: Test_from_DEVs
    route:
    - destination:
        host: podinfo-primary
      weight: 50
    - destination:
        host: podinfo-canary
      weight: 50
  - route:
     - destination:
         host: podinfo-primary
       weight: 50

Logic comes from here and here

this PR makes canary the default destination from the canary route when neither maxWeight or stepWeight are set on the canary

  http:
  - match:
    - headers:
        x-header-1:
          exact: Test_from_DEVs
    route:
    - destination:
        host: podinfo-primary
      weight: 0
    - destination:
        host: podinfo-canary
      weight: 100
  - route:
    - destination:
        host: podinfo-primary
       weight: 100

Additionally as per istio docs If there is only one destination in a rule, it will receive all traffic so it probably makes sense to get rid of the weights altogether and only leave one destination on each route?

…sis config

primary destination is duplicated on both virtual service routes when setting match rules as per the [istio a/b testing](https://docs.flagger.app/tutorials/istio-ab-testing) tutorial:
```yaml
  http:
  - match:
    - headers:
        x-header-1:
          exact: Test_from_DEVs
    route:
    - destination:
        host: podinfo-primary
      weight: 50
    - destination:
        host: podinfo-canary
      weight: 50
  - route:
     - destination:
         host: podinfo-primary
        weight: 50
```
this PR removes the primary destination from canary route to generate and update the virtual service without the duplicate destination:
```yaml
  http:
  - match:
    - headers:
        x-header-1:
          exact: Test_from_DEVs
    route:
    - destination:
        host: podinfo-canary
      weight: 50
  - route:
    - destination:
        host: podinfo-primary
       weight: 50
```

Signed-off-by: luisr-escobar <[email protected]>
@luisr-escobar
Copy link
Author

luisr-escobar commented Jun 28, 2024

@stefanprodan / @aryan9600 any chance one of you can take a look at this PR?

@aryan9600
Copy link
Member

.analysis.match and .analysis.stepWeight are not compatible with each other. the former is used for A/B testing while the latter is used for canary releases. if you'd like to combine both A/B testing with canary releases, you can do so by configuring session affinity: https://docs.flagger.app/tutorials/istio-progressive-delivery#session-affinity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants