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

implement the support for Traefik setMirror #2890

Open
GiuseppeChiesa-TomTom opened this issue Jul 19, 2023 · 9 comments
Open

implement the support for Traefik setMirror #2890

GiuseppeChiesa-TomTom opened this issue Jul 19, 2023 · 9 comments
Assignees

Comments

@GiuseppeChiesa-TomTom
Copy link

Summary

As per docs available here, the native Traefik integration enable 2 patterns: weighted traffic routing and mirrored traffic routing.
However, the mirrored traffic pattern is currently not completely supported

Use Cases

In our use-case, we would like to have a pre-warmed fleet of pods in the canary replicaset. For this reason we are seeking the possibility to mirror the traffic from the stable replicaset to the canary one.
With TraefikService CRD this is possible (see docs above for an example), and we would like to leverage this capability to achieve something like:

  strategy:
    canary:
      analysis:
        templates:
          - templateName: response-time
        # delay starting analysis after the pods are available
        startingStep: 2 
        args:
          - name: service-name
            value: deployment-service-preview
      stableService: deployment-service-active
      canaryService: deployment-service-preview
      trafficRouting:
        traefik:
          mirrorTraefikServiceName: traefik-mirroring-service
      steps:
        # scale the new fleet of pods to same as current one
        - setCanaryScale: 
            weight: 100
        # wait for some traffic to warm-up the new pods
        - pause: { duration: 5m } # no traffic but analysis is running
        - setWeight: 20 # start offering response from the new replicaset
        [...]

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.

@GiuseppeChiesa-TomTom GiuseppeChiesa-TomTom added the enhancement New feature or request label Jul 19, 2023
@GiuseppeChiesa-TomTom GiuseppeChiesa-TomTom changed the title implement the support for Traefik Native mirrorTraefikServiceName implement the support for Traefik mirrorTraefikServiceName Jul 19, 2023
@Philipp-Plotnikov
Copy link
Contributor

@GiuseppeChiesa-TomTom thank you for the created issue.
I can suggest several ways how to add support.

  1. Simply to add the support direct in argo-rollouts
  2. Create the seperate plugin for Traefik support but I think in the future we will need to delete the current Traefik support from argo-rollouts repository to have single implementation. Like we do here https://github.com/argoproj-labs/rollouts-plugin-trafficrouter-gatewayapi

I prefer second variant as plugin system was created for that and the future fixes and changes for Traefik support will be easier and quicker

Also I can implement Traefik mirror support

@zachaller and @kostis-codefresh what do you think ?

@zachaller
Copy link
Collaborator

zachaller commented Jul 19, 2023

I think that docs are just a typo actually there is no other mention of that field in the code anywhere. I would actually start with something like enabling the SetMirror support for trafiek https://argo-rollouts.readthedocs.io/en/stable/features/traffic-management/#traffic-routing-mirroring-traffic-to-canary I think that would get you want you want?

@GiuseppeChiesa-TomTom
Copy link
Author

GiuseppeChiesa-TomTom commented Jul 24, 2023

I've tried the suggested approach via setMirrorRoute but I get an error mentioning that is supported by Istio only:

Name:            echo-server-my-application
Namespace:       default
Status:          ✖ Degraded
Message:         InvalidSpec: The Rollout "echo-server-my-application" is invalid: spec.strategy.steps[2].setMirrorRoute: Invalid value: v1alpha1.SetMirrorRoute{Name:"warmup", Match:[]v1alpha1.RouteMatch{v1alpha1.RouteMatch{Method:(*v1alpha1.StringMatch)(0xc0010f8840), Path:(*v1alpha1.StringMatch)(nil), Headers:map[string]v1alpha1.StringMatch(nil)}}, Percentage:(*int32)(0xc000f69464)}: SetMirrorRoute requires TrafficRouting, supports Istio only
Strategy:        Canary
  Step:          7/9
  SetWeight:     50
  ActualWeight:  0
Images:          ealen/echo-server:0.6.0 (stable)
Replicas:
  Desired:       1
  Current:       1
  Updated:       1
  Ready:         1
  Available:     1

NAME                                                   KIND        STATUS      AGE  INFO
⟳ echo-server-my-application                           Rollout     ✖ Degraded  53m  
└──# revision:1                                                                     
   └──⧉ echo-server-my-application-647bcbcd6           ReplicaSet  ✔ Healthy   53m  stable
      └──□ echo-server-my-application-647bcbcd6-r4t7j  Pod         ✔ Running   53m  ready:1/1

my configuration was the following:

      trafficRouting:
        managedRoutes:
          - name: warmup
        traefik:
          weightedTraefikServiceName: traefik-mirroring-service
      steps:
        - setCanaryScale:
            weight: 100
        - pause: { duration: 120 }
        - setMirrorRoute:
            name: warmup
            percentage: 100
            match:
              - method:
                  exact: GET
        - pause: { duration: 120 }
        - setWeight: 20
        - pause: { duration: 20 }
        - setWeight: 50
        - pause: { duration: 20 }
        - setWeight: 100

at this point I believe it would be really nice to have the native support for Traefik extended to support mirroring.
Traefik is very convenient for the simplicity in management and configuration compared with Istio.

@kostis-codefresh
Copy link
Member

@GiuseppeChiesa-TomTom I updated the docs here #2904

@Philipp-Plotnikov
Copy link
Contributor

@zachaller I looked at code and traefik doesn't have implemented method for SetMirrororRoute
We need to implement SetMirrororRoute and RemoveManagedRoutes and change validation to pass with SetMirrororRoute when we use Traefik

@zachaller
Copy link
Collaborator

@Philipp-Plotnikov yup that is correct, and then also update the Readme in the repo to show alpha support.

@Philipp-Plotnikov
Copy link
Contributor

Philipp-Plotnikov commented Jul 26, 2023

@zachaller if it is ok, can I make it ? Who should assign me ?

@zachaller zachaller changed the title implement the support for Traefik mirrorTraefikServiceName implement the support for Traefik set Mirror Jul 26, 2023
@zachaller zachaller changed the title implement the support for Traefik set Mirror implement the support for Traefik setMirror Jul 26, 2023
@Philipp-Plotnikov
Copy link
Contributor

@GiuseppeChiesa-TomTom I created PR for that, you can see progress here
#2948

@github-actions
Copy link
Contributor

This issue is stale because it has been open 60 days with no activity.

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

No branches or pull requests

4 participants