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

spec changes for header and mirror based routing #2073

Closed
zachaller opened this issue Jun 3, 2022 · 3 comments
Closed

spec changes for header and mirror based routing #2073

zachaller opened this issue Jun 3, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@zachaller
Copy link
Collaborator

Summary

We are planning to add traffic mirror and header based routing which now puts a different set of requirements on Argo Rollouts to have the idea of precedence within the routes that argo rollouts creates. Most traffic routers such as Istio, Ambassador, and Trafiek and others have the concept of being able to control route precedence. So in order to implement the two mentioned features above I propose adding a new managedRoutes section to traffic routing that defines the route precedence as well as allows us to clean up our routes when we are done with the rollout.

      trafficRouting:
        managedRoutes: # The order of the routes here become the precedence in the upstream traffic manager
          - name: set-header-1
          - name: mirror-route-3
      steps:
        - setMirrorRoute:
            name: mirror-route-3
            percentage: 100
            match:
              - method:
                  exact: POST
                path:
                  prefix: /
        - setHeaderRoute:
            name: set-header-1
            match:
            - headerName: agent-1a
              headerValue:
                regex: firefox1(.*)

The above spec changes are at the very minimum required for the two mentioned features. However, in order to also support a more gitops friendly workflow I think changing the managed routes to also have the option to include the route that acts as the canary route a very useful feature so that we do not have to predefine our routes such as required with istio. Notice the canaryRoute: true

      trafficRouting:
        managedRoutes:
          - name: set-header-1
          - name: mirror-route-3
          - name: primary
            canaryRoute: true #This becomes the default/canary route in the upstream traffic router

Argo rollouts will then be able to place the routes that rollouts creates at a higher precedence than any user defined routes which allows the user to create their routes as if Argo Rollouts did not exists and we would then only create higher precedence routes as needed during the rollout. This allow for a very gitops friendly deployment of argo rollouts with argocd.

@zachaller
Copy link
Collaborator Author

closed by #2074

@zachaller
Copy link
Collaborator Author

zachaller commented Sep 13, 2022

Notes on trying to remove canaryRoute: true

  • Gateway API - when we are trying to place a new default route in front of the user defined one we will conflict and the resolution is this we will need our newer default route to match so need to figure out how these two resolutions work https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteRule

    The oldest Route based on creation timestamp.
    The Route appearing first in alphabetical order by “{namespace}/{name}”.
    

    A case for having canary true for gateway api is a user having multiple http Route Rules that they want to apply the canary backendRef's to so that they can specify different backend per path but still use canary from rollouts. This would also possibly need a removeOnRolloutComplete: false or externallyManaged: true option added to managed routes.

  • Ambassador - For those situations, a Mapping can explicitly specify the precedence. A Mapping with no precedence is assumed to have a precedence of 0; the higher the precedence value, the earlier the Mapping is attempted.
    If multiple Mappings have the same precedence, Ambassador Edge Stack's normal sorting determines the ordering within the precedence; however, there is no way that Ambassador Edge Stack can ever sort a Mapping with a lower precedence ahead of one at a higher precedence.

    Emissary-ingress sorts mappings such that those that are more highly constrained are evaluated before those less highly 
    constrained. The prefix length, the request method, and the constraint headers are all taken into account. These.   
    mechanisms, however, may not be sufficient to guarantee the correct ordering when regular expressions or highly 
    complex constraints are in play.
    
  • Istio - For istio one issue we run into is when we are placing new default routes ontop of the current default for say canarying if the configuration of that route has other configuration such as cores retries etc we do not know which route to pull that from. If we had a way to specify a route as a template we could use that however this would still not be gitops. We really probably need to move some of that config into the rollouts possibly.

@zachaller
Copy link
Collaborator Author

zachaller commented Oct 25, 2022

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

No branches or pull requests

1 participant