From 2fc4bb4a45251d37699a115e7fe0a50995eb5974 Mon Sep 17 00:00:00 2001 From: Brad Ison Date: Tue, 14 Apr 2020 15:40:17 +0200 Subject: [PATCH] Fix flaky circuit breaker test (#7043) The `TestCircuitBreaker` test is very flaky. It relies on examining the percentage of requests that succeed or fail to a service with circuit breaking configured, and ensuring the numbers fall within a given range. Unfortunately, the percentages tend to vary widely from run to run, so the test fails as often as it passes. This simplifies the test by only verifying that both successes and failures occur instead of checking the percentage of each. --- .../trafficmanagement/circuitbreaker_test.go | 18 +---- .../scripts/trip_circuitbreaker.txt | 76 ++++++------------- 2 files changed, 25 insertions(+), 69 deletions(-) diff --git a/tests/trafficmanagement/circuitbreaker_test.go b/tests/trafficmanagement/circuitbreaker_test.go index 9e118a12b7..6a01f4612f 100644 --- a/tests/trafficmanagement/circuitbreaker_test.go +++ b/tests/trafficmanagement/circuitbreaker_test.go @@ -27,22 +27,8 @@ func TestCircuitBreaker(t *testing.T) { Add( istioio.Script{Input: istioio.Path("scripts/circuitbreaker_test_setup.txt")}, istioio.MultiPodWait("istio-io-circuitbreaker"), - istioio.Script{ - Input: istioio.Evaluate(istioio.Path("scripts/trip_circuitbreaker.txt"), map[string]interface{}{ - "isSnippet": false, - "inputTerminalFlag": "", - "beforeCircuitBreakVerify": " 2>&1 | verify_circuit_breaking 60 100 0 40", - "afterCircuitBreakVerify": " 2>&1 | verify_circuit_breaking 20 80 20 80", - "outputRedirectionCmd": "2>&1", - }), - SnippetInput: istioio.Evaluate(istioio.Path("scripts/trip_circuitbreaker.txt"), map[string]interface{}{ - "isSnippet": true, - "inputTerminalFlag": "-it", - "beforeCircuitBreakVerify": "", - "afterCircuitBreakVerify": "", - "outputRedirectionCmd": "", - }), - }). + istioio.Script{Input: istioio.Path("scripts/trip_circuitbreaker.txt")}, + ). Defer(istioio.Script{Input: istioio.Path("scripts/circuitbreaker_test_cleanup.txt")}). Build()) } diff --git a/tests/trafficmanagement/scripts/trip_circuitbreaker.txt b/tests/trafficmanagement/scripts/trip_circuitbreaker.txt index e2e6f1f61a..630beacdfe 100644 --- a/tests/trafficmanagement/scripts/trip_circuitbreaker.txt +++ b/tests/trafficmanagement/scripts/trip_circuitbreaker.txt @@ -20,72 +20,42 @@ set -o pipefail # $snippet test_fortio_httpbin_interaction.sh syntax="bash" outputis="text" $ FORTIO_POD=$(kubectl get pods -n istio-io-circuitbreaker | grep fortio | awk '{ print $1 }') -$ kubectl -n istio-io-circuitbreaker exec {{ .inputTerminalFlag }} $FORTIO_POD -c fortio /usr/bin/fortio -- load -curl http://httpbin:8000/get +$ kubectl -n istio-io-circuitbreaker exec $FORTIO_POD -c fortio -- /usr/bin/fortio load -curl http://httpbin:8000/get # $verify verifier="contains" 200 OK # $endsnippet -function ruby_eval() { - ruby -e "p $1" -} - -# check_percentage_bounds take 4 arguments as input. It must be invoked as follows +# FIXME / TODO: These tests previously relied on checking that the +# percentage of 200 and 503 responses fell within a given range. That +# turned out to be flaky, so for now they are only checking that both +# 200 and 503 responses are recorded, and ignoring the number of each. +# That should be fixed at some point. # -# $ check_percentage_bounds \ -# -# -# Percentage of requests with status 200 must be between "percentage-lower-bound-200" and -# "percentage-upper-bound-200" inclusive. Same for 503. If bounds are breached then the -# function exits with status 1 -function check_percentage_bounds() { - local lower_bounds=( ["200"]=${1:-"100"} ["503"]=${3:-"0"} ) - local upper_bounds=( ["200"]=${2:-"100"} ["503"]=${4:-"0"} ) - - readarray -t code_lines < <(grep -E "^Code ([1-5][0-9]{2,}) : [0-9]+ \(([0-9\.]+) %\)$") - - for code_line in "${code_lines[@]}"; do - local status_code=$(echo $code_line | cut -d' ' -f2) - local percentage_req=$(echo $code_line | cut -d' ' -f5 | sed "s/[\(]//g") - - local status_lower_bound=${lower_bounds[$status_code]} - local status_upper_bound=${upper_bounds[$status_code]} - - local is_lower_bound_breached=$(ruby_eval "$percentage_req < $status_lower_bound") - local is_upper_bound_breached=$(ruby_eval "$percentage_req > $status_upper_bound") - - if [[ $is_lower_bound_breached == "true" || $is_upper_bound_breached == "true" ]]; then - echo "either lower bound or upper bound is breached" - echo "status=$status_code actual=$percentage_req, low=$status_lower_bound high=$status_upper_bound" - exit 1 - fi - done -} - -function verify_circuit_breaking() { - local fortio_output=`cat` # preserve output - - echo "$fortio_output" | check_percentage_bounds $1 $2 $3 $4 - if (($? != 0)); then - exit 1; - fi - - echo "$fortio_output" -} +# Original PR: https://github.com/istio/istio.io/pull/6609 +# Temporary fix: https://github.com/istio/istio.io/pull/7043 +# Issue: https://github.com/istio/istio.io/issues/7074 # $snippet almost_trip_circuit_breaker.sh syntax="bash" outputis="text" -$ kubectl -n istio-io-circuitbreaker exec {{ .inputTerminalFlag }} $FORTIO_POD -c fortio /usr/bin/fortio -- \ - load -c 2 -qps 0 -n 20 -loglevel warning http://httpbin:8000/get {{ .beforeCircuitBreakVerify }} +$ kubectl -n istio-io-circuitbreaker exec $FORTIO_POD -c fortio -- /usr/bin/fortio \ + load -c 2 -qps 0 -n 20 -loglevel warning http://httpbin:8000/get +# $verify verifier="contains" +Code 200 : +# $verify verifier="contains" +Code 503 : # $endsnippet - # $snippet trip_circuit_breaker.sh syntax="bash" outputis="text" -$ kubectl -n istio-io-circuitbreaker exec {{ .inputTerminalFlag }} $FORTIO_POD -c fortio /usr/bin/fortio -- \ - load -c 3 -qps 0 -n 30 -loglevel warning http://httpbin:8000/get {{ .afterCircuitBreakVerify }} +$ kubectl -n istio-io-circuitbreaker exec $FORTIO_POD -c fortio -- /usr/bin/fortio \ + load -c 3 -qps 0 -n 30 -loglevel warning http://httpbin:8000/get +# $verify verifier="contains" +Code 200 : +# $verify verifier="contains" +Code 503 : # $endsnippet # $snippet print_statistics_after_tripping.sh syntax="bash" outputis="text" -$ kubectl -n istio-io-circuitbreaker exec {{ .inputTerminalFlag }} $FORTIO_POD -c istio-proxy -- \ - pilot-agent request GET stats | grep httpbin | grep pending {{ .outputRedirectionCmd }} +$ kubectl -n istio-io-circuitbreaker exec $FORTIO_POD -c istio-proxy -- \ + pilot-agent request GET stats | grep httpbin | grep pending # $verify cluster.outbound|8000||httpbin.istio-io-circuitbreaker.svc.cluster.local.circuit_breakers.default.rq_pending_open: ? cluster.outbound|8000||httpbin.istio-io-circuitbreaker.svc.cluster.local.circuit_breakers.high.rq_pending_open: ?