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

A/B testing support for Gloo Edge ingress controller #758

Closed
wants to merge 16 commits into from

Conversation

kdorosh
Copy link
Contributor

@kdorosh kdorosh commented Dec 21, 2020

Resolves #707 , which is related to solo-io/gloo#3726

The approach here is to replace Gloo UpstreamGroups in the flagger implementation with Gloo RouteTables, which additionally support header/method matching used in A/B testing. For this to work, we needed to add and leverage the new inheritablePathMatcher field on Gloo RouteTables, thus these changes will only work in Gloo Edge 1.6.0-beta22 and higher. (Gloo Edge 1.6.0 release is slated for end of year)

Unit and integration tests all pass locally. Still running through the documented Gloo guide to ensure it's up to date, otherwise this PR is ready to be looked over. Going to mark as draft until I have finished reviewing the guide.

Appreciate the attention to developer experience and developer guide, thanks!


![Flagger Gloo Ingress Controller](https://raw.githubusercontent.com/weaveworks/flagger/master/docs/diagrams/flagger-gloo-overview.png)

## Prerequisites

Flagger requires a Kubernetes cluster **v1.11** or newer and Gloo ingress **1.3.5** or newer.
This guide was written for Flagger version **1.5.0** or higher. Prior versions of Flagger used Gloo upstream groups to handle
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does flagger versioning work? According to semver this is a new feature, so I put 1.5.0 here; please let me know what to put if that's not accurate.

@kdorosh kdorosh force-pushed the gloo_route_table branch 4 times, most recently from d6c4bd6 to 67c2845 Compare December 21, 2020 22:58
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
Signed-off-by: Kevin Dorosh <kcdorosh@gmail.com>
This guide was written for Flagger version **1.5.0** or higher. Prior versions of Flagger used Gloo upstream groups to handle
canaries, but newer versions of Flagger use Gloo route tables to handle canaries as well as A/B testing.

Flagger requires a Kubernetes cluster **v1.11** or newer and Gloo Edge ingress **1.6.0** or newer.

Install Gloo with Helm v3:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note, the guide will not work as written until 1.6.0 is released in two weeks, as helm install installs the latest 1.5 release by default.

I can provide the --version flag to the helm install command if we want to hard-code the guide to a 1.6 version (for now, a beta). Thoughts?

@kdorosh kdorosh marked this pull request as ready for review December 21, 2020 23:11
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kdorosh this looks great thanks! I think we should hold this off until there is a stable release of Goo with this feature.

How are Goo users suppose to upgrade, does it imply downtime? It would be nice for Gloo users to have a section in docs on how to upgrade from Flagger v.1.4 and Gloo 1.5 to Flagger v.1.5 and Gloo Edge 1.6.

@stefanprodan stefanprodan deleted the branch fluxcd:master December 22, 2020 14:37
@stefanprodan
Copy link
Member

@kdorosh please open a PR agains main branch, I've delete master.

@kdorosh
Copy link
Contributor Author

kdorosh commented Dec 22, 2020

replaced by #765

w.r.t. #758 (review):

I think we should hold this off until there is a stable release of Goo with this feature.

1.6.0-beta22 is a release of Gloo that is live, and demonstrates the functionality that will be in the long-term-support version of 1.6 Gloo. I'm happy to come back and update to version tested against in two weeks, but would really like to avoid rebasing my PR on top of changes to this repo again (so far, twice: once for circle ci -> GH actions, and once for master -> main branch).

How are Goo users suppose to upgrade, does it imply downtime? It would be nice for Gloo users to have a section in docs on how to upgrade from Flagger v.1.4 and Gloo 1.5 to Flagger v.1.5 and Gloo Edge 1.6.

What do you imply by downtime? Time where you cannot run a canary release?

If so, the only downtime is for the 1.4.2 -> 1.5 flagger upgrade. Gloo 1.6 works with both upstream groups and route tables, so both old and new flagger versions will work.

The only change required of the user to use 1.5+ versions of flagger is to point their virtual service(s) toward a route table (to be generated by flagger) rather than an upstream group. The new guide demonstrates this when creating the new virtual service upfront. Happy to add a note to the new PR to clarify this (what would you want it to say?).

@stefanprodan
Copy link
Member

Gloo 1.6 works with both upstream groups and route tables, so both old and new flagger versions will work.

All good then, thanks!

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