From a3f791be1799b427dd2a6abd70cb0530cc492cc0 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 16 Dec 2020 16:57:59 -0500 Subject: [PATCH] First pass A/B testing Signed-off-by: Kevin Dorosh --- pkg/apis/gloo/v1/types.go | 20 ++++++ pkg/apis/gloo/v1/zz_generated.deepcopy.go | 51 +++++++++++++- pkg/router/gloo.go | 40 ++++++++++- pkg/router/gloo_test.go | 39 +++++++++++ test/gloo/test-canary.sh | 83 ++++++++++++++++++++++- 5 files changed, 228 insertions(+), 5 deletions(-) diff --git a/pkg/apis/gloo/v1/types.go b/pkg/apis/gloo/v1/types.go index 54dfb572..f372284a 100644 --- a/pkg/apis/gloo/v1/types.go +++ b/pkg/apis/gloo/v1/types.go @@ -25,7 +25,27 @@ type Route struct { } type Matcher struct { + // only one of Prefix, Exact, Regex may be nonempty Prefix string `json:"prefix,omitempty"` + Exact string `json:"exact,omitempty"` + Regex string `json:"regex,omitempty"` + + Headers []HeaderMatcher `json:"headers,omitempty"` + QueryParameterMatchers []QueryParameterMatcher `json:"queryParameters,omitempty"` + Methods []string `json:"methods,omitempty"` +} + +type HeaderMatcher struct { + Name string `json:"name,omitempty"` + Value string `json:"value,omitempty"` + Regex bool `json:"regex,omitempty"` + InvertMatch bool `json:"invertMatch,omitempty"` +} + +type QueryParameterMatcher struct { + Name string `json:"name,omitempty"` + Value string `json:"value,omitempty"` + Regex bool `json:"regex,omitempty"` } type RouteAction struct { diff --git a/pkg/apis/gloo/v1/zz_generated.deepcopy.go b/pkg/apis/gloo/v1/zz_generated.deepcopy.go index 6e111a0c..44a00d9a 100644 --- a/pkg/apis/gloo/v1/zz_generated.deepcopy.go +++ b/pkg/apis/gloo/v1/zz_generated.deepcopy.go @@ -41,9 +41,40 @@ func (in *Destination) DeepCopy() *Destination { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HeaderMatcher) DeepCopyInto(out *HeaderMatcher) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HeaderMatcher. +func (in *HeaderMatcher) DeepCopy() *HeaderMatcher { + if in == nil { + return nil + } + out := new(HeaderMatcher) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Matcher) DeepCopyInto(out *Matcher) { *out = *in + if in.Headers != nil { + in, out := &in.Headers, &out.Headers + *out = make([]HeaderMatcher, len(*in)) + copy(*out, *in) + } + if in.QueryParameterMatchers != nil { + in, out := &in.QueryParameterMatchers, &out.QueryParameterMatchers + *out = make([]QueryParameterMatcher, len(*in)) + copy(*out, *in) + } + if in.Methods != nil { + in, out := &in.Methods, &out.Methods + *out = make([]string, len(*in)) + copy(*out, *in) + } return } @@ -78,6 +109,22 @@ func (in *MultiDestination) DeepCopy() *MultiDestination { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *QueryParameterMatcher) DeepCopyInto(out *QueryParameterMatcher) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new QueryParameterMatcher. +func (in *QueryParameterMatcher) DeepCopy() *QueryParameterMatcher { + if in == nil { + return nil + } + out := new(QueryParameterMatcher) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResourceRef) DeepCopyInto(out *ResourceRef) { *out = *in @@ -100,7 +147,9 @@ func (in *Route) DeepCopyInto(out *Route) { if in.Matchers != nil { in, out := &in.Matchers, &out.Matchers *out = make([]Matcher, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } in.Action.DeepCopyInto(&out.Action) return diff --git a/pkg/router/gloo.go b/pkg/router/gloo.go index 5578d368..0563cd0c 100644 --- a/pkg/router/gloo.go +++ b/pkg/router/gloo.go @@ -51,10 +51,11 @@ func (gr *GlooRouter) Reconcile(canary *flaggerv1.Canary) error { newSpec := gloov1.RouteTableSpec{ Routes: []gloov1.Route{ { - // eventually inherit from parent, used for A/B rollouts too? Matchers: []gloov1.Matcher{ { - Prefix: "/", + Prefix: "/", + Headers: getHeaderMatchers(canary), + Methods: getMethods(canary), }, }, Action: gloov1.RouteAction{ @@ -194,7 +195,9 @@ func (gr *GlooRouter) SetRoutes( // eventually inherit from parent, used for A/B rollouts too? Matchers: []gloov1.Matcher{ { - Prefix: "/", + Prefix: "/", + Headers: getHeaderMatchers(canary), + Methods: getMethods(canary), }, }, Action: gloov1.RouteAction{ @@ -235,3 +238,34 @@ func (gr *GlooRouter) SetRoutes( func (gr *GlooRouter) Finalize(_ *flaggerv1.Canary) error { return nil } + +func getHeaderMatchers(canary *flaggerv1.Canary) []gloov1.HeaderMatcher { + var headerMatchers []gloov1.HeaderMatcher + for _, match := range canary.GetAnalysis().Match { + for s, stringMatch := range match.Headers { + h := gloov1.HeaderMatcher{ + Name: s, + Value: stringMatch.Exact, + } + if stringMatch.Regex != "" { + h = gloov1.HeaderMatcher{ + Name: s, + Value: stringMatch.Regex, + Regex: true, + } + } + headerMatchers = append(headerMatchers, h) + } + } + return headerMatchers +} + +func getMethods(canary *flaggerv1.Canary) []string { + var methods []string + for _, match := range canary.GetAnalysis().Match { + if stringMatch := match.Method; stringMatch != nil { + methods = append(methods, stringMatch.Exact) + } + } + return methods +} diff --git a/pkg/router/gloo_test.go b/pkg/router/gloo_test.go index f1159693..dcf1a6c4 100644 --- a/pkg/router/gloo_test.go +++ b/pkg/router/gloo_test.go @@ -37,6 +37,7 @@ func TestGlooRouter_Sync(t *testing.T) { kubeClient: mocks.kubeClient, } + // init err := router.Reconcile(mocks.canary) require.NoError(t, err) @@ -47,6 +48,25 @@ func TestGlooRouter_Sync(t *testing.T) { assert.Len(t, dests, 2) assert.Equal(t, uint32(100), dests[0].Weight) assert.Equal(t, uint32(0), dests[1].Weight) + + // test headers update + cd, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{}) + require.NoError(t, err) + + cdClone := cd.DeepCopy() + cdClone.Spec.Analysis.Iterations = 5 + cdClone.Spec.Analysis.Match = newTestABTest().Spec.Analysis.Match + canary, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Update(context.TODO(), cdClone, metav1.UpdateOptions{}) + require.NoError(t, err) + + // apply change + err = router.Reconcile(canary) + require.NoError(t, err) + + rt, err = router.glooClient.GatewayV1().RouteTables("default").Get(context.TODO(), "podinfo", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, "x-user-type", rt.Spec.Routes[0].Matchers[0].Headers[0].Name) + assert.Equal(t, "test", rt.Spec.Routes[0].Matchers[0].Headers[0].Value) } func TestGlooRouter_SetRoutes(t *testing.T) { @@ -90,6 +110,25 @@ func TestGlooRouter_SetRoutes(t *testing.T) { assert.Equal(t, uint32(p), pRoute.Weight) assert.Equal(t, uint32(c), cRoute.Weight) + + cd, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{}) + require.NoError(t, err) + + // test update to A/B + cdClone := cd.DeepCopy() + cdClone.Spec.Analysis.Iterations = 5 + cdClone.Spec.Analysis.Match = newTestABTest().Spec.Analysis.Match + canary, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Update(context.TODO(), cdClone, metav1.UpdateOptions{}) + require.NoError(t, err) + + // test set routes for A/B + err = router.SetRoutes(canary, 0, 100, false) + require.NoError(t, err) + + rt, err = router.glooClient.GatewayV1().RouteTables("default").Get(context.TODO(), "podinfo", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, "x-user-type", rt.Spec.Routes[0].Matchers[0].Headers[0].Name) + assert.Equal(t, "test", rt.Spec.Routes[0].Matchers[0].Headers[0].Value) } func TestGlooRouter_GetRoutes(t *testing.T) { diff --git a/test/gloo/test-canary.sh b/test/gloo/test-canary.sh index 6a64b853..f14e79d5 100755 --- a/test/gloo/test-canary.sh +++ b/test/gloo/test-canary.sh @@ -19,7 +19,7 @@ echo '>>> Installing load tester' kubectl apply -k ${REPO_ROOT}/kustomize/tester kubectl -n test rollout status deployment/flagger-loadtester -echo '>>> Initializing canary' +echo '>>> Initialising canary' kubectl apply -f ${REPO_ROOT}/test/e2e-workload.yaml >>>>>>> Initial commit:test/e2e-gloo-tests.sh @@ -127,4 +127,85 @@ until ${ok}; do fi done +echo '>>> Waiting for canary finalization' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test get canary/podinfo | grep 'Succeeded' && ok=true || ok=false + sleep 5 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n ingress-nginx logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + echo '✔ Canary promotion test passed' + +cat <>> Triggering A/B testing' +kubectl -n test set image deployment/podinfo podinfod=stefanprodan/podinfo:3.1.2 + +echo '>>> Waiting for A/B testing promotion' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test describe deployment/podinfo-primary | grep '3.1.2' && ok=true || ok=false + sleep 10 + kubectl -n gloo-system logs deployment/flagger --tail 1 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n gloo-system logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + +echo '✔ A/B testing promotion test passed' + +kubectl -n gloo-system logs deployment/flagger + +echo '✔ All tests passed' \ No newline at end of file