fix: breaking orders of retry statuscodes (alibaba#32)

Signed-off-by: bitliu <[email protected]>
Hinsteny · Nov 7, 2022 · 9a69a03 · 9a69a03
1 parent 03f31b8
commit 9a69a03
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 7 deletions.
diff --git a/ingress/kube/annotations/retry.go b/ingress/kube/annotations/retry.go
@@ -20,7 +20,6 @@ import (
 	"github.com/gogo/protobuf/types"
 
 	networking "istio.io/api/networking/v1alpha3"
-	"istio.io/istio/pilot/pkg/util/sets"
 )
 
 const (
@@ -71,7 +70,8 @@ func (r retry) Parse(annotations Annotations, config *Ingress, _ *GlobalContext)
 	}
 
 	if retryOn, err := annotations.ParseStringASAP(retryOn); err == nil {
-		conditions := toSet(splitBySeparator(retryOn, ","))
+		extraConfigs := splitBySeparator(retryOn, ",")
+		conditions := toSet(extraConfigs)
 		if len(conditions) > 0 {
 			if conditions.Contains("off") {
 				retryConfig.retryCount = 0
@@ -88,7 +88,7 @@ func (r retry) Parse(annotations Annotations, config *Ingress, _ *GlobalContext)
 					stringBuilder.WriteString("non_idempotent,")
 				}
 				// Append the status codes.
-				statusCodes := convertStatusCodes(conditions)
+				statusCodes := convertStatusCodes(extraConfigs)
 				if len(statusCodes) > 0 {
 					stringBuilder.WriteString(retryStatusCode + ",")
 					for _, code := range statusCodes {
@@ -123,11 +123,11 @@ func needRetryConfig(annotations Annotations) bool {
 		annotations.HasASAP(retryOn)
 }
 
-func convertStatusCodes(set sets.Set) []string {
+func convertStatusCodes(statusCodes []string) []string {
 	var result []string
-	for condition := range set {
-		if strings.HasPrefix(condition, "http_") {
-			result = append(result, strings.TrimPrefix(condition, "http_"))
+	for _, statusCode := range statusCodes {
+		if strings.HasPrefix(statusCode, "http_") {
+			result = append(result, strings.TrimPrefix(statusCode, "http_"))
 		}
 	}
 	return result

diff --git a/ingress/kube/annotations/retry_test.go b/ingress/kube/annotations/retry_test.go
@@ -95,6 +95,16 @@ func TestRetryParse(t *testing.T) {
 				retryOn:         "5xx,retriable-status-codes,503,502,404",
 			},
 		},
+		{
+			input: map[string]string{
+				buildNginxAnnotationKey(retryOn): "timeout,http_505,http_503,http_502,http_404,http_403",
+			},
+			expect: &RetryConfig{
+				retryCount:      3,
+				perRetryTimeout: &types.Duration{},
+				retryOn:         "5xx,retriable-status-codes,505,503,502,404,403",
+			},
+		},
 	}
 
 	for _, inputCase := range inputCases {