Merge pull request #94 from stefanprodan/fix-abtest-routing

Fix A/B Testing HTTP URI match conditions
fluxcd · Mar 11, 2019 · 3adfcc8 · 3adfcc8
2 parents 1cd0c49 + c720fee
commit 3adfcc8
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 7 deletions.
diff --git a/artifacts/ab-testing/canary.yaml b/artifacts/ab-testing/canary.yaml
@@ -40,7 +40,7 @@ spec:
             regex: "^(?!.*Chrome)(?=.*\bSafari\b).*$"
       - headers:
           cookie:
-            regex: "^(.*?;)?(user=test)(;.*)?$"
+            regex: "^(.*?;)?(type=insider)(;.*)?$"
     metrics:
     - name: istio_requests_total
       # minimum req success rate (non 5xx responses)
@@ -58,4 +58,4 @@ spec:
         url: http://flagger-loadtester.test/
         timeout: 5s
         metadata:
-          cmd: "hey -z 1m -q 10 -c 2 http://podinfo.test:9898/"
+          cmd: "hey -z 1m -q 10 -c 2 -H 'Cookie: type=insider' http://podinfo.test:9898/"
diff --git a/docs/gitbook/how-it-works.md b/docs/gitbook/how-it-works.md
@@ -347,7 +347,7 @@ You can enable A/B testing by specifying the HTTP match conditions and the numbe
     match:
       - headers:
           user-agent:
-            regex: "^(?!.*Chrome)(?=.*\bSafari\b).*$"
+            regex: "^(?!.*Chrome).*Safari.*"
       - headers:
           cookie:
             regex: "^(.*?;)?(user=test)(;.*)?$"

diff --git a/docs/gitbook/usage/ab-testing.md b/docs/gitbook/usage/ab-testing.md
@@ -72,7 +72,7 @@ spec:
     match:
       - headers:
           user-agent:
-            regex: "^(?!.*Chrome)(?=.*\bSafari\b).*$"
+            regex: "^(?!.*Chrome).*Safari.*"
       - headers:
           cookie:
             regex: "^(.*?;)?(type=insider)(;.*)?$"
@@ -93,7 +93,7 @@ spec:
         url: http://flagger-loadtester.test/
         timeout: 5s
         metadata:
-          cmd: "hey -z 1m -q 10 -c 2 http://podinfo.test:9898/"
+          cmd: "hey -z 1m -q 10 -c 2 -H 'Cookie: type=insider' http://podinfo.test:9898/"
 ```
 
 The above configuration will run an analysis for ten minutes targeting Safari users and those that have an insider cookie.

diff --git a/pkg/router/istio.go b/pkg/router/istio.go
@@ -93,7 +93,7 @@ func (ir *IstioRouter) Sync(canary *flaggerv1.Canary) error {
 	}
 
 	if len(canary.Spec.CanaryAnalysis.Match) > 0 {
-		canaryMatch := append(canary.Spec.Service.Match, canary.Spec.CanaryAnalysis.Match...)
+		canaryMatch := mergeMatchConditions(canary.Spec.CanaryAnalysis.Match, canary.Spec.Service.Match)
 		newSpec.Http = []istiov1alpha3.HTTPRoute{
 			{
 				Match:         canaryMatch,
@@ -274,7 +274,7 @@ func (ir *IstioRouter) SetRoutes(
 	// fix routing (A/B testing)
 	if len(canary.Spec.CanaryAnalysis.Match) > 0 {
 		// merge the common routes with the canary ones
-		canaryMatch := append(canary.Spec.Service.Match, canary.Spec.CanaryAnalysis.Match...)
+		canaryMatch := mergeMatchConditions(canary.Spec.CanaryAnalysis.Match, canary.Spec.Service.Match)
 		vsCopy.Spec.Http = []istiov1alpha3.HTTPRoute{
 			{
 				Match:         canaryMatch,
@@ -345,3 +345,16 @@ func addHeaders(canary *flaggerv1.Canary) (headers map[string]string) {
 
 	return
 }
+
+// mergeMatchConditions appends the URI match rules to canary conditions
+func mergeMatchConditions(canary, defaults []istiov1alpha3.HTTPMatchRequest) []istiov1alpha3.HTTPMatchRequest {
+	for i := range canary {
+		for _, d := range defaults {
+			if d.Uri != nil {
+				canary[i].Uri = d.Uri
+			}
+		}
+	}
+
+	return canary
+}