Merge pull request #1324 from BLasan/backend-retry

Add: Backend Retry Configuration in CR #1324

adapter/config/default_config.go

@@ -71,9 +71,7 @@ var defaultConfig = &Config{
 				HealthyThreshold:   2,
 			},
 			Retry: upstreamRetry{
-				MaxRetryCount:        5,
-				BaseIntervalInMillis: 25,
-				StatusCodes:          []uint32{504},
+				StatusCodes: []uint32{504},
 			},
 			DNS: upstreamDNS{
 				DNSRefreshRate: 5000,

adapter/config/types.go

@@ -186,12 +186,11 @@ type payloadPassingToEnforcer struct {
 // Envoy Upstream Related Configurations
 type envoyUpstream struct {
 	// UpstreamTLS related Configuration
-	TLS      upstreamTLS
-	Timeouts upstreamTimeout
-	Health   upstreamHealth
-	DNS      upstreamDNS
-	Retry    upstreamRetry
-	HTTP2    upstreamHTTP2Options
+	TLS    upstreamTLS
+	Health upstreamHealth
+	DNS    upstreamDNS
+	Retry  upstreamRetry
+	HTTP2  upstreamHTTP2Options
 }
 
 // Envoy Downstream Related Configurations
@@ -214,12 +213,6 @@ type upstreamTLS struct {
 	DisableSslVerification bool
 }
 
-type upstreamTimeout struct {
-	RouteTimeoutInSeconds     uint32
-	MaxRouteTimeoutInSeconds  uint32
-	RouteIdleTimeoutInSeconds uint32
-}
-
 type upstreamHealth struct {
 	Timeout            int32
 	Interval           int32
@@ -238,9 +231,7 @@ type upstreamHTTP2Options struct {
 }
 
 type upstreamRetry struct {
-	MaxRetryCount        uint32
-	BaseIntervalInMillis uint32
-	StatusCodes          []uint32
+	StatusCodes []uint32
 }
 
 type security struct {

adapter/internal/oasparser/envoyconf/routes_configs.go

@@ -31,7 +31,6 @@ import (
 	"github.com/envoyproxy/go-control-plane/pkg/wellknown"
 	"github.com/golang/protobuf/proto"
 	"github.com/golang/protobuf/ptypes/any"
-	"github.com/wso2/apk/adapter/config"
 	logger "github.com/wso2/apk/adapter/internal/loggers"
 	"github.com/wso2/apk/adapter/internal/oasparser/constants"
 	"github.com/wso2/apk/adapter/internal/oasparser/model"
@@ -77,8 +76,6 @@ func generateRouteMatch(routeRegex string) *routev3.RouteMatch {
 }
 
 func generateRouteAction(apiType string, routeConfig *model.EndpointConfig, ratelimitCriteria *ratelimitCriteria) (action *routev3.Route_Route) {
-
-	config := config.ReadConfigs()
 	action = &routev3.Route_Route{
 		Route: &routev3.RouteAction{
 			HostRewriteSpecifier: &routev3.RouteAction_AutoHostRewrite{
@@ -97,29 +94,19 @@ func generateRouteAction(apiType string, routeConfig *model.EndpointConfig, rate
 		action.Route.IdleTimeout = durationpb.New(time.Duration(routeConfig.IdleTimeoutInSeconds) * time.Second)
 	}
 
-	if ratelimitCriteria != nil && ratelimitCriteria.level != "" {
-		action.Route.RateLimits = generateRateLimitPolicy(ratelimitCriteria)
-	}
-
 	if routeConfig != nil && routeConfig.RetryConfig != nil {
-		// Retry configs are always added via headers. This is to update the
-		// default retry back-off base interval, which cannot be updated via headers.
-		retryConfig := config.Envoy.Upstream.Retry
-		commonRetryPolicy := &routev3.RetryPolicy{
-			RetryOn: retryPolicyRetriableStatusCodes,
-			NumRetries: &wrapperspb.UInt32Value{
-				Value: 0,
-				// If not set to 0, default value 1 will be
-			},
-			RetriableStatusCodes: retryConfig.StatusCodes,
+		retryPolicy := &routev3.RetryPolicy{
 			RetryBackOff: &routev3.RetryPolicy_RetryBackOff{
-				BaseInterval: &durationpb.Duration{
-					Nanos: int32(retryConfig.BaseIntervalInMillis) * 1000,
-				},
+				BaseInterval: durationpb.New(time.Duration(routeConfig.RetryConfig.BaseIntervalInMillis) * time.Millisecond),
 			},
 		}
-		action.Route.RetryPolicy = commonRetryPolicy
+		action.Route.RetryPolicy = retryPolicy
+	}
+
+	if ratelimitCriteria != nil && ratelimitCriteria.level != "" {
+		action.Route.RateLimits = generateRateLimitPolicy(ratelimitCriteria)
 	}
+
 	return action
 }
 

adapter/internal/oasparser/envoyconf/routes_with_clusters.go

@@ -171,8 +171,10 @@ func CreateRoutesWithClusters(adapterInternalAPI model.AdapterInternalAPI, inter
 			interceptorCerts, vHost, organizationID, apiRequestInterceptor, apiResponseInterceptor, resource)
 		clusters = append(clusters, clustersI...)
 		endpoints = append(endpoints, endpointsI...)
-		routeP, err := createRoutes(genRouteCreateParams(&adapterInternalAPI, resource, vHost, basePath, clusterName, *operationalReqInterceptors, *operationalRespInterceptorVal, organizationID,
-			false))
+		routeParams := genRouteCreateParams(&adapterInternalAPI, resource, vHost, basePath, clusterName, *operationalReqInterceptors, *operationalRespInterceptorVal, organizationID,
+			false)
+
+		routeP, err := createRoutes(routeParams)
 		if err != nil {
 			logger.LoggerXds.ErrorC(logging.GetErrorByCode(2231, adapterInternalAPI.GetTitle(), adapterInternalAPI.GetVersion(), resource.GetPath(), err.Error()))
 			return nil, nil, nil, fmt.Errorf("error while creating routes. %v", err)

adapter/internal/oasparser/model/adapter_internal_api.go

@@ -149,8 +149,9 @@ type EndpointConfig struct {
 
 // RetryConfig holds the parameters for retries done by apk to the EndpointCluster
 type RetryConfig struct {
-	Count       int32    `mapstructure:"count"`
-	StatusCodes []uint32 `mapstructure:"statusCodes"`
+	Count                int32    `mapstructure:"count"`
+	StatusCodes          []uint32 `mapstructure:"statusCodes"`
+	BaseIntervalInMillis int32    `mapstructure:"baseIntervalInMillis"`
 }
 
 // CircuitBreakers holds the parameters for retries done by apk to the EndpointCluster
@@ -401,12 +402,6 @@ func (endpoint *Endpoint) GetAuthorityHeader() string {
 
 func (retryConfig *RetryConfig) validateRetryConfig() {
 	conf := config.ReadConfigs()
-	maxConfigurableCount := conf.Envoy.Upstream.Retry.MaxRetryCount
-	if retryConfig.Count > int32(maxConfigurableCount) || retryConfig.Count < 0 {
-		logger.LoggerOasparser.Errorf("Retry count for the API must be within the range 0 - %v."+
-			"Reconfiguring retry count as %v", maxConfigurableCount, maxConfigurableCount)
-		retryConfig.Count = int32(maxConfigurableCount)
-	}
 	var validStatusCodes []uint32
 	for _, statusCode := range retryConfig.StatusCodes {
 		if statusCode > 598 || statusCode < 401 {

adapter/internal/oasparser/model/http_route.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	"github.com/google/uuid"
+	"github.com/wso2/apk/adapter/config"
 	"github.com/wso2/apk/adapter/internal/loggers"
 	"github.com/wso2/apk/adapter/internal/oasparser/constants"
 	dpv1alpha1 "github.com/wso2/apk/adapter/internal/operator/apis/dp/v1alpha1"
@@ -54,6 +55,7 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR
 
 	disableScopes := true
 	disableAuthentications := false
+	config := config.ReadConfigs()
 
 	var authScheme *dpv1alpha1.Authentication
 	if outputAuthScheme != nil {
@@ -83,7 +85,12 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR
 		var scopes []string
 		var timeoutInMillis uint32
 		var idleTimeoutInSeconds uint32
+		isRetryConfig := false
 		isRouteTimeout := false
+		var backendRetryCount uint32
+		var statusCodes []uint32
+		statusCodes = append(statusCodes, config.Envoy.Upstream.Retry.StatusCodes...)
+		var baseIntervalInMillis uint32
 		for _, filter := range rule.Filters {
 			hasPolicies = true
 			switch filter.Type {
@@ -251,6 +258,15 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR
 					timeoutInMillis = resolvedBackend.Timeout.RouteTimeoutSeconds * 1000
 					idleTimeoutInSeconds = resolvedBackend.Timeout.RouteIdleTimeoutSeconds
 				}
+
+				if resolvedBackend.Retry != nil {
+					isRetryConfig = true
+					backendRetryCount = resolvedBackend.Retry.Count
+					baseIntervalInMillis = resolvedBackend.Retry.BaseIntervalMillis
+					if len(resolvedBackend.Retry.StatusCodes) > 0 {
+						statusCodes = resolvedBackend.Retry.StatusCodes
+					}
+				}
 				endPoints = append(endPoints, GetEndpoints(backendName, httpRouteParams.BackendMapping)...)
 				for _, security := range resolvedBackend.Security {
 					switch security.Type {
@@ -296,7 +312,14 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR
 					MaxConnectionPools: int32(circuitBreaker.MaxConnectionPools),
 				}
 			}
-			if isRouteTimeout || circuitBreaker != nil {
+			if isRetryConfig {
+				endpointConfig.RetryConfig = &RetryConfig{
+					Count:                int32(backendRetryCount),
+					StatusCodes:          statusCodes,
+					BaseIntervalInMillis: int32(baseIntervalInMillis),
+				}
+			}
+			if isRouteTimeout || circuitBreaker != nil || isRetryConfig {
 				resource.endpoints.Config = endpointConfig
 			}
 			resource.endpointSecurity = utils.GetPtrSlice(securityConfig)

adapter/internal/operator/apis/dp/v1alpha1/backend_types.go

@@ -67,6 +67,9 @@ type BackendSpec struct {
 	// +optional
 	// Timeout congifuration for the backend
 	Timeout *Timeout `json:"timeout,omitempty"`
+
+	// +optional
+	Retry *RetryConfig `json:"retry,omitempty"`
 }
 
 // Timeout defines the timeout configurations
@@ -99,6 +102,19 @@ type RetryBudget struct {
 	MinRetryConcurrency uint32 `json:"minRetryConcurrency,omitempty"`
 }
 
+// RetryConfig defines retry configurations
+type RetryConfig struct {
+	// +kubebuilder:default=1
+	// MaxRetry defines the maximum number of retries
+	Count uint32 `json:"count"`
+	// +kubebuilder:default=2000
+	// BaseIntervalMillis defines the base interval in milliseconds
+	BaseIntervalMillis uint32 `json:"baseIntervalMillis"`
+	// +optional
+	// StatusCodes defines the list of status codes to retry
+	StatusCodes []uint32 `json:"statusCodes,omitempty"`
+}
+
 // Service holds host and port information for the service
 type Service struct {
 	Host string `json:"host"`

adapter/internal/operator/apis/dp/v1alpha1/backend_webhook.go

@@ -89,6 +89,19 @@ func (r *Backend) validateBackendSpec() error {
 				circuitBreakers.MaxConnectionPools, "maxConnectionPools should be greater than 0"))
 		}
 	}
+	retryConfig := r.Spec.Retry
+	if retryConfig != nil {
+		if int32(retryConfig.Count) < 0 {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("retry").Child("count"), retryConfig.Count,
+				"retry count should be greater than or equal to 0"))
+		}
+		for _, statusCode := range retryConfig.StatusCodes {
+			if statusCode > 598 || statusCode < 401 {
+				allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("retry").Child("statusCodes"),
+					retryConfig.StatusCodes, "status code should be between 401 and 598"))
+			}
+		}
+	}
 	if len(allErrs) > 0 {
 		return apierrors.NewInvalid(
 			schema.GroupKind{Group: "dp.wso2.com", Kind: "Backend"},

adapter/internal/operator/apis/dp/v1alpha1/resolvedbackend.go

@@ -30,6 +30,7 @@ type ResolvedBackend struct {
 	Security       []ResolvedSecurityConfig
 	CircuitBreaker *CircuitBreaker
 	Timeout        *Timeout
+	Retry          *RetryConfig
 }
 
 // ResolvedTLSConfig defines enpoint TLS configurations

adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml

@@ -74,6 +74,29 @@ spec:
                 - ws
                 - wss
                 type: string
+              retry:
+                description: RetryConfig defines retry configurations
+                properties:
+                  baseIntervalMillis:
+                    default: 2000
+                    description: BaseIntervalMillis defines the base interval in milliseconds
+                    format: int32
+                    type: integer
+                  count:
+                    default: 1
+                    description: MaxRetry defines the maximum number of retries
+                    format: int32
+                    type: integer
+                  statusCodes:
+                    description: StatusCodes defines the list of status codes to retry
+                    items:
+                      format: int32
+                      type: integer
+                    type: array
+                required:
+                - baseIntervalMillis
+                - count
+                type: object
               security:
                 items:
                   description: SecurityConfig defines enpoint security configurations

adapter/internal/operator/utils/utils.go

@@ -308,7 +308,6 @@ func GetResolvedBackend(ctx context.Context, client k8client.Client,
 	}
 	resolvedBackend.Services = backend.Spec.Services
 	resolvedBackend.Protocol = backend.Spec.Protocol
-	resolvedBackend.CircuitBreaker = nil
 	if backend.Spec.CircuitBreaker != nil {
 		resolvedBackend.CircuitBreaker = &dpv1alpha1.CircuitBreaker{
 			MaxConnections:     backend.Spec.CircuitBreaker.MaxConnections,
@@ -318,14 +317,20 @@ func GetResolvedBackend(ctx context.Context, client k8client.Client,
 			MaxPendingRequests: backend.Spec.CircuitBreaker.MaxPendingRequests,
 		}
 	}
-	resolvedBackend.Timeout = nil
 	if backend.Spec.Timeout != nil {
 		resolvedBackend.Timeout = &dpv1alpha1.Timeout{
 			MaxRouteTimeoutSeconds:  backend.Spec.Timeout.MaxRouteTimeoutSeconds,
 			RouteTimeoutSeconds:     backend.Spec.Timeout.RouteTimeoutSeconds,
 			RouteIdleTimeoutSeconds: backend.Spec.Timeout.RouteIdleTimeoutSeconds,
 		}
 	}
+	if backend.Spec.Retry != nil {
+		resolvedBackend.Retry = &dpv1alpha1.RetryConfig{
+			Count:              backend.Spec.Retry.Count,
+			BaseIntervalMillis: backend.Spec.Retry.BaseIntervalMillis,
+			StatusCodes:        backend.Spec.Retry.StatusCodes,
+		}
+	}
 	if backend.Spec.TLS != nil {
 		resolvedTLSConfig.ResolvedCertificate, err = ResolveCertificate(ctx, client,
 			backend.Namespace, backend.Spec.TLS.CertificateInline, backend.Spec.TLS.ConfigMapRef, backend.Spec.TLS.SecretRef)

adapter/pkg/logging/logging_constant.go

@@ -102,6 +102,8 @@ const (
 	error3113 = 3113
 	error3114 = 3114
 	error3115 = 3115
+	error3116 = 3116
+	error3117 = 3117
 )
 
 // Mapper used to keep error details for error logs
@@ -467,4 +469,14 @@ var Mapper = map[int]ErrorDetails{
 		Message:   "Route Timeout cannot be greater than the Max value defined : %s",
 		Severity:  MAJOR,
 	},
+	error3116: {
+		ErrorCode: error3116,
+		Message:   "Invalid Status Codes for Retry: %s",
+		Severity:  MAJOR,
+	},
+	error3117: {
+		ErrorCode: error3117,
+		Message:   "Retry Count Should be greater than 0: %s",
+		Severity:  MAJOR,
+	},
 }