From 2133171f4e81cc84ea0da2aaf5e7ee8b15ced9f8 Mon Sep 17 00:00:00 2001 From: BLasan Date: Thu, 15 Jun 2023 09:21:38 +0530 Subject: [PATCH 01/16] Add: Backend Retry Configuration in CR --- .../internal/oasparser/envoyconf/routes_configs.go | 2 +- .../oasparser/envoyconf/routes_with_clusters.go | 13 +++++++++++-- adapter/internal/oasparser/model/http_route.go | 2 ++ .../operator/apis/dp/v1alpha1/backend_types.go | 1 + .../operator/apis/dp/v1alpha1/resolvedbackend.go | 1 + .../config/crd/bases/dp.wso2.com_backends.yaml | 3 +++ helm-charts/crds/dp.wso2.com_backends.yaml | 3 +++ 7 files changed, 22 insertions(+), 3 deletions(-) diff --git a/adapter/internal/oasparser/envoyconf/routes_configs.go b/adapter/internal/oasparser/envoyconf/routes_configs.go index a8cfa4381..f157f6f6c 100644 --- a/adapter/internal/oasparser/envoyconf/routes_configs.go +++ b/adapter/internal/oasparser/envoyconf/routes_configs.go @@ -108,7 +108,7 @@ func generateRouteAction(apiType string, routeConfig *model.EndpointConfig, rate commonRetryPolicy := &routev3.RetryPolicy{ RetryOn: retryPolicyRetriableStatusCodes, NumRetries: &wrapperspb.UInt32Value{ - Value: 0, + Value: uint32(routeConfig.RetryConfig.Count), // If not set to 0, default value 1 will be }, RetriableStatusCodes: retryConfig.StatusCodes, diff --git a/adapter/internal/oasparser/envoyconf/routes_with_clusters.go b/adapter/internal/oasparser/envoyconf/routes_with_clusters.go index c8eac7021..b3846c0bd 100644 --- a/adapter/internal/oasparser/envoyconf/routes_with_clusters.go +++ b/adapter/internal/oasparser/envoyconf/routes_with_clusters.go @@ -171,8 +171,17 @@ 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) + + // set retry count if endpoint retry config is available + if endpoint.Config != nil && endpoint.Config.RetryConfig != nil { + routeParams.routeConfig.RetryConfig = &model.RetryConfig{ + Count: endpoint.Config.RetryConfig.Count, + } + } + + 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) diff --git a/adapter/internal/oasparser/model/http_route.go b/adapter/internal/oasparser/model/http_route.go index 585434c57..168534ed5 100644 --- a/adapter/internal/oasparser/model/http_route.go +++ b/adapter/internal/oasparser/model/http_route.go @@ -71,6 +71,7 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR if outputRatelimitPolicy != nil { ratelimitPolicy = concatRateLimitPolicies(*outputRatelimitPolicy, nil) } + var backendRetry int32 for _, rule := range httpRoute.Spec.Rules { var endPoints []Endpoint @@ -247,6 +248,7 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR timeoutInMillis = resolvedBackend.Timeout.RouteTimeoutSeconds * 1000 idleTimeoutInSeconds = resolvedBackend.Timeout.RouteIdleTimeoutSeconds } + backendRetry = resolvedBackend.Retry endPoints = append(endPoints, GetEndpoints(backendName, httpRouteParams.BackendMapping)...) for _, security := range resolvedBackend.Security { switch security.Type { diff --git a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go index e773dfba3..feb879f02 100644 --- a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go +++ b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go @@ -67,6 +67,7 @@ type BackendSpec struct { // +optional // Timeout congifuration for the backend Timeout *Timeout `json:"timeout,omitempty"` + Retry int32 `json:"retry,omitempty"` } // Timeout defines the timeout configurations diff --git a/adapter/internal/operator/apis/dp/v1alpha1/resolvedbackend.go b/adapter/internal/operator/apis/dp/v1alpha1/resolvedbackend.go index fb815d2c5..708f938b5 100644 --- a/adapter/internal/operator/apis/dp/v1alpha1/resolvedbackend.go +++ b/adapter/internal/operator/apis/dp/v1alpha1/resolvedbackend.go @@ -30,6 +30,7 @@ type ResolvedBackend struct { Security []ResolvedSecurityConfig CircuitBreaker *CircuitBreaker Timeout *Timeout + Retry int32 } // ResolvedTLSConfig defines enpoint TLS configurations diff --git a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml index 402c7e0c5..26c10168b 100644 --- a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml +++ b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml @@ -74,6 +74,9 @@ spec: - ws - wss type: string + retry: + format: int32 + type: integer security: items: description: SecurityConfig defines enpoint security configurations diff --git a/helm-charts/crds/dp.wso2.com_backends.yaml b/helm-charts/crds/dp.wso2.com_backends.yaml index f5079166d..cc56af687 100644 --- a/helm-charts/crds/dp.wso2.com_backends.yaml +++ b/helm-charts/crds/dp.wso2.com_backends.yaml @@ -90,6 +90,9 @@ spec: - ws - wss type: string + retry: + format: int32 + type: integer security: items: description: SecurityConfig defines enpoint security configurations From bcc2d33e77265c10dba281447b4144924dc628da Mon Sep 17 00:00:00 2001 From: BLasan Date: Thu, 15 Jun 2023 09:34:13 +0530 Subject: [PATCH 02/16] Fix: Retry count setting when resolving the backend --- adapter/internal/operator/utils/utils.go | 1 + 1 file changed, 1 insertion(+) diff --git a/adapter/internal/operator/utils/utils.go b/adapter/internal/operator/utils/utils.go index c5bc760a3..c3acefbba 100644 --- a/adapter/internal/operator/utils/utils.go +++ b/adapter/internal/operator/utils/utils.go @@ -326,6 +326,7 @@ func GetResolvedBackend(ctx context.Context, client k8client.Client, RouteIdleTimeoutSeconds: backend.Spec.Timeout.RouteIdleTimeoutSeconds, } } + resolvedBackend.Retry = backend.Spec.Retry 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) From 8cb06bbad07469e59783617858f1c73e448c36d1 Mon Sep 17 00:00:00 2001 From: BLasan Date: Thu, 15 Jun 2023 09:49:22 +0530 Subject: [PATCH 03/16] update backend cr --- .../operator/config/crd/bases/dp.wso2.com_backends.yaml | 2 +- helm-charts/crds/dp.wso2.com_backends.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml index 26c10168b..fc166198c 100644 --- a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml +++ b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml @@ -74,7 +74,7 @@ spec: - ws - wss type: string - retry: + retryCount: format: int32 type: integer security: diff --git a/helm-charts/crds/dp.wso2.com_backends.yaml b/helm-charts/crds/dp.wso2.com_backends.yaml index cc56af687..fa174a541 100644 --- a/helm-charts/crds/dp.wso2.com_backends.yaml +++ b/helm-charts/crds/dp.wso2.com_backends.yaml @@ -90,7 +90,7 @@ spec: - ws - wss type: string - retry: + retryCount: format: int32 type: integer security: From 3b3ad03649fa95a40485df29b83c2abd124b717b Mon Sep 17 00:00:00 2001 From: BLasan Date: Thu, 15 Jun 2023 22:35:23 +0530 Subject: [PATCH 04/16] Fix: Route config overriding issue --- .../oasparser/envoyconf/routes_configs.go | 7 ++-- .../envoyconf/routes_with_clusters.go | 11 +++++-- .../oasparser/model/adapter_internal_api.go | 5 +-- .../internal/oasparser/model/http_route.go | 32 +++++++++++++++++-- .../apis/dp/v1alpha1/backend_types.go | 12 +++++-- .../apis/dp/v1alpha1/resolvedbackend.go | 2 +- .../crd/bases/dp.wso2.com_backends.yaml | 17 ++++++++-- adapter/internal/operator/utils/utils.go | 8 ++++- helm-charts/crds/dp.wso2.com_backends.yaml | 17 ++++++++-- 9 files changed, 91 insertions(+), 20 deletions(-) diff --git a/adapter/internal/oasparser/envoyconf/routes_configs.go b/adapter/internal/oasparser/envoyconf/routes_configs.go index f157f6f6c..f90e9dad2 100644 --- a/adapter/internal/oasparser/envoyconf/routes_configs.go +++ b/adapter/internal/oasparser/envoyconf/routes_configs.go @@ -104,22 +104,23 @@ func generateRouteAction(apiType string, routeConfig *model.EndpointConfig, rate 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: uint32(routeConfig.RetryConfig.Count), // If not set to 0, default value 1 will be }, - RetriableStatusCodes: retryConfig.StatusCodes, + RetriableStatusCodes: routeConfig.RetryConfig.StatusCodes, RetryBackOff: &routev3.RetryPolicy_RetryBackOff{ BaseInterval: &durationpb.Duration{ - Nanos: int32(retryConfig.BaseIntervalInMillis) * 1000, + Nanos: int32(routeConfig.RetryConfig.BaseIntervalInMillis) * 1000, }, }, } action.Route.RetryPolicy = commonRetryPolicy + fmt.Println("Action Retry: ", action.Route.RetryPolicy.NumRetries) } + return action } diff --git a/adapter/internal/oasparser/envoyconf/routes_with_clusters.go b/adapter/internal/oasparser/envoyconf/routes_with_clusters.go index b3846c0bd..296406cdf 100644 --- a/adapter/internal/oasparser/envoyconf/routes_with_clusters.go +++ b/adapter/internal/oasparser/envoyconf/routes_with_clusters.go @@ -174,10 +174,17 @@ func CreateRoutesWithClusters(adapterInternalAPI model.AdapterInternalAPI, inter routeParams := genRouteCreateParams(&adapterInternalAPI, resource, vHost, basePath, clusterName, *operationalReqInterceptors, *operationalRespInterceptorVal, organizationID, false) + fmt.Println("Retry Cluster Routes: ", endpoint.Config.RetryConfig.Count) + fmt.Println("BaseTimeline Cluster Routes: ", endpoint.Config.RetryConfig.BaseIntervalInMillis) + // set retry count if endpoint retry config is available if endpoint.Config != nil && endpoint.Config.RetryConfig != nil { - routeParams.routeConfig.RetryConfig = &model.RetryConfig{ - Count: endpoint.Config.RetryConfig.Count, + routeParams.routeConfig = &model.EndpointConfig{ + RetryConfig: &model.RetryConfig{ + Count: endpoint.Config.RetryConfig.Count, + StatusCodes: endpoint.Config.RetryConfig.StatusCodes, + BaseIntervalInMillis: endpoint.Config.RetryConfig.BaseIntervalInMillis, + }, } } diff --git a/adapter/internal/oasparser/model/adapter_internal_api.go b/adapter/internal/oasparser/model/adapter_internal_api.go index a3196b0ab..fc9b1b2e2 100644 --- a/adapter/internal/oasparser/model/adapter_internal_api.go +++ b/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 diff --git a/adapter/internal/oasparser/model/http_route.go b/adapter/internal/oasparser/model/http_route.go index 168534ed5..f00672c04 100644 --- a/adapter/internal/oasparser/model/http_route.go +++ b/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,12 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR disableScopes := true disableAuthentications := false + config := config.ReadConfigs() + retryConfig := config.Envoy.Upstream.Retry + + backendRetryCount := retryConfig.MaxRetryCount + statusCodes := retryConfig.StatusCodes + baseIntervalInMillis := retryConfig.BaseIntervalInMillis var authScheme *dpv1alpha1.Authentication if outputAuthScheme != nil { @@ -71,7 +78,6 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR if outputRatelimitPolicy != nil { ratelimitPolicy = concatRateLimitPolicies(*outputRatelimitPolicy, nil) } - var backendRetry int32 for _, rule := range httpRoute.Spec.Rules { var endPoints []Endpoint @@ -84,6 +90,7 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR var scopes []string var timeoutInMillis uint32 var idleTimeoutInSeconds uint32 + isRetryConfig := false isRouteTimeout := false for _, filter := range rule.Filters { hasPolicies = true @@ -248,7 +255,19 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR timeoutInMillis = resolvedBackend.Timeout.RouteTimeoutSeconds * 1000 idleTimeoutInSeconds = resolvedBackend.Timeout.RouteIdleTimeoutSeconds } - backendRetry = resolvedBackend.Retry + + if resolvedBackend.Retry != nil { + isRetryConfig = true + if resolvedBackend.Retry.MaxRetryCount > 0 { + backendRetryCount = resolvedBackend.Retry.MaxRetryCount + } + if resolvedBackend.Retry.StatusCodes != nil && len(resolvedBackend.Retry.StatusCodes) > 0 { + statusCodes = append(statusCodes, resolvedBackend.Retry.StatusCodes...) + } + if resolvedBackend.Retry.BaseIntervalInMillis > 0 { + baseIntervalInMillis = resolvedBackend.Retry.BaseIntervalInMillis + } + } endPoints = append(endPoints, GetEndpoints(backendName, httpRouteParams.BackendMapping)...) for _, security := range resolvedBackend.Security { switch security.Type { @@ -294,7 +313,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) diff --git a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go index feb879f02..9be09a627 100644 --- a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go +++ b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go @@ -66,8 +66,8 @@ type BackendSpec struct { CircuitBreaker *CircuitBreaker `json:"circuitBreaker,omitempty"` // +optional // Timeout congifuration for the backend - Timeout *Timeout `json:"timeout,omitempty"` - Retry int32 `json:"retry,omitempty"` + Timeout *Timeout `json:"timeout,omitempty"` + Retry *RetryConfig `json:"retry,omitempty"` } // Timeout defines the timeout configurations @@ -98,6 +98,14 @@ type CircuitBreaker struct { type RetryBudget struct { BudgetPercent uint64 `json:"budgetPercent,omitempty"` MinRetryConcurrency uint32 `json:"minRetryConcurrency,omitempty"` + // RetryCount int32 `json:"retryCount,omitempty"` +} + +// RetryConfig defines retry configurations +type RetryConfig struct { + MaxRetryCount uint32 `json:"maxRetryCount,omitempty"` + BaseIntervalInMillis uint32 `json:"baseIntervalInMillis,omitempty"` + StatusCodes []uint32 `json:"statusCodes,omitempty"` } // Service holds host and port information for the service diff --git a/adapter/internal/operator/apis/dp/v1alpha1/resolvedbackend.go b/adapter/internal/operator/apis/dp/v1alpha1/resolvedbackend.go index 708f938b5..9de459c8f 100644 --- a/adapter/internal/operator/apis/dp/v1alpha1/resolvedbackend.go +++ b/adapter/internal/operator/apis/dp/v1alpha1/resolvedbackend.go @@ -30,7 +30,7 @@ type ResolvedBackend struct { Security []ResolvedSecurityConfig CircuitBreaker *CircuitBreaker Timeout *Timeout - Retry int32 + Retry *RetryConfig } // ResolvedTLSConfig defines enpoint TLS configurations diff --git a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml index fc166198c..16a49a87f 100644 --- a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml +++ b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml @@ -74,9 +74,20 @@ spec: - ws - wss type: string - retryCount: - format: int32 - type: integer + retry: + properties: + baseIntervalInMillis: + format: int32 + type: integer + maxRetryCount: + format: int32 + type: integer + statusCodes: + items: + format: int32 + type: integer + type: array + type: object security: items: description: SecurityConfig defines enpoint security configurations diff --git a/adapter/internal/operator/utils/utils.go b/adapter/internal/operator/utils/utils.go index c3acefbba..1044668d2 100644 --- a/adapter/internal/operator/utils/utils.go +++ b/adapter/internal/operator/utils/utils.go @@ -326,7 +326,13 @@ func GetResolvedBackend(ctx context.Context, client k8client.Client, RouteIdleTimeoutSeconds: backend.Spec.Timeout.RouteIdleTimeoutSeconds, } } - resolvedBackend.Retry = backend.Spec.Retry + if backend.Spec.Retry != nil { + resolvedBackend.Retry = dpv1alpha1.Retry{ + MaxRetryCount: backend.Spec.Retry.MaxRetryCount, + BaseIntervalInMillis: backend.Spec.Retry.BaseIntervalInMillis, + 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) diff --git a/helm-charts/crds/dp.wso2.com_backends.yaml b/helm-charts/crds/dp.wso2.com_backends.yaml index fa174a541..fbb5caf1a 100644 --- a/helm-charts/crds/dp.wso2.com_backends.yaml +++ b/helm-charts/crds/dp.wso2.com_backends.yaml @@ -90,9 +90,20 @@ spec: - ws - wss type: string - retryCount: - format: int32 - type: integer + retry: + properties: + baseIntervalInMillis: + format: int32 + type: integer + maxRetryCount: + format: int32 + type: integer + statusCodes: + items: + format: int32 + type: integer + type: array + type: object security: items: description: SecurityConfig defines enpoint security configurations From 4a3e2ff373fbb457bb7388e99d3b8135c1234b82 Mon Sep 17 00:00:00 2001 From: BLasan Date: Tue, 20 Jun 2023 11:22:51 +0530 Subject: [PATCH 05/16] Fix: Status codes getting appended --- .../config/crd/bases/dp.wso2.com_backends.yaml | 1 + developer/tryout/samples/sample-backend.yaml | 11 +++++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml index 16a49a87f..16f1a3573 100644 --- a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml +++ b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml @@ -75,6 +75,7 @@ spec: - wss type: string retry: + description: RetryConfig defines retry configurations properties: baseIntervalInMillis: format: int32 diff --git a/developer/tryout/samples/sample-backend.yaml b/developer/tryout/samples/sample-backend.yaml index f15ba417d..e2544a9ea 100644 --- a/developer/tryout/samples/sample-backend.yaml +++ b/developer/tryout/samples/sample-backend.yaml @@ -21,12 +21,11 @@ kind: Backend metadata: name: insecure-backend spec: - circuitBreaker: - thresholds: - - maxConnections: 100 - maxPendingRequests: 100 - maxRequests: 100 - maxRetries: 3 + retry: + maxRetryCount: 3 + baseIntervalInMillis: 2000 + statusCodes: + - 504 services: - host: backend port: 3000 From 9178a4c1a9b956240506e8b4f4da9b677f761f91 Mon Sep 17 00:00:00 2001 From: BLasan Date: Tue, 20 Jun 2023 11:56:43 +0530 Subject: [PATCH 06/16] Make Route Retry Policy optional --- .../oasparser/envoyconf/routes_with_clusters.go | 3 --- adapter/internal/oasparser/model/http_route.go | 13 +++++-------- adapter/internal/operator/utils/utils.go | 2 +- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/adapter/internal/oasparser/envoyconf/routes_with_clusters.go b/adapter/internal/oasparser/envoyconf/routes_with_clusters.go index 296406cdf..c3d2c7d2d 100644 --- a/adapter/internal/oasparser/envoyconf/routes_with_clusters.go +++ b/adapter/internal/oasparser/envoyconf/routes_with_clusters.go @@ -174,9 +174,6 @@ func CreateRoutesWithClusters(adapterInternalAPI model.AdapterInternalAPI, inter routeParams := genRouteCreateParams(&adapterInternalAPI, resource, vHost, basePath, clusterName, *operationalReqInterceptors, *operationalRespInterceptorVal, organizationID, false) - fmt.Println("Retry Cluster Routes: ", endpoint.Config.RetryConfig.Count) - fmt.Println("BaseTimeline Cluster Routes: ", endpoint.Config.RetryConfig.BaseIntervalInMillis) - // set retry count if endpoint retry config is available if endpoint.Config != nil && endpoint.Config.RetryConfig != nil { routeParams.routeConfig = &model.EndpointConfig{ diff --git a/adapter/internal/oasparser/model/http_route.go b/adapter/internal/oasparser/model/http_route.go index f00672c04..4ea3f97ae 100644 --- a/adapter/internal/oasparser/model/http_route.go +++ b/adapter/internal/oasparser/model/http_route.go @@ -21,7 +21,6 @@ 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" @@ -55,12 +54,6 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR disableScopes := true disableAuthentications := false - config := config.ReadConfigs() - retryConfig := config.Envoy.Upstream.Retry - - backendRetryCount := retryConfig.MaxRetryCount - statusCodes := retryConfig.StatusCodes - baseIntervalInMillis := retryConfig.BaseIntervalInMillis var authScheme *dpv1alpha1.Authentication if outputAuthScheme != nil { @@ -92,6 +85,9 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR var idleTimeoutInSeconds uint32 isRetryConfig := false isRouteTimeout := false + var backendRetryCount uint32 + var statusCodes []uint32 + var baseIntervalInMillis uint32 for _, filter := range rule.Filters { hasPolicies = true switch filter.Type { @@ -234,6 +230,7 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR return fmt.Errorf("no backendref were provided") } var securityConfig []EndpointSecurity + isRetryConfigDefined := false for _, backend := range rule.BackendRefs { backendName := types.NamespacedName{ Name: string(backend.Name), @@ -262,7 +259,7 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR backendRetryCount = resolvedBackend.Retry.MaxRetryCount } if resolvedBackend.Retry.StatusCodes != nil && len(resolvedBackend.Retry.StatusCodes) > 0 { - statusCodes = append(statusCodes, resolvedBackend.Retry.StatusCodes...) + statusCodes = resolvedBackend.Retry.StatusCodes } if resolvedBackend.Retry.BaseIntervalInMillis > 0 { baseIntervalInMillis = resolvedBackend.Retry.BaseIntervalInMillis diff --git a/adapter/internal/operator/utils/utils.go b/adapter/internal/operator/utils/utils.go index 1044668d2..4a759cc6f 100644 --- a/adapter/internal/operator/utils/utils.go +++ b/adapter/internal/operator/utils/utils.go @@ -327,7 +327,7 @@ func GetResolvedBackend(ctx context.Context, client k8client.Client, } } if backend.Spec.Retry != nil { - resolvedBackend.Retry = dpv1alpha1.Retry{ + resolvedBackend.Retry = &dpv1alpha1.RetryConfig{ MaxRetryCount: backend.Spec.Retry.MaxRetryCount, BaseIntervalInMillis: backend.Spec.Retry.BaseIntervalInMillis, StatusCodes: backend.Spec.Retry.StatusCodes, From c1a03552edf0af1e88f349413d3652103302d154 Mon Sep 17 00:00:00 2001 From: BLasan Date: Tue, 20 Jun 2023 13:38:53 +0530 Subject: [PATCH 07/16] Remove: Unwanted logs --- adapter/internal/oasparser/envoyconf/routes_configs.go | 5 +---- adapter/internal/operator/apis/dp/v1alpha1/backend_types.go | 6 ++++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/adapter/internal/oasparser/envoyconf/routes_configs.go b/adapter/internal/oasparser/envoyconf/routes_configs.go index f90e9dad2..d8980abf1 100644 --- a/adapter/internal/oasparser/envoyconf/routes_configs.go +++ b/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{ @@ -88,6 +85,7 @@ func generateRouteAction(apiType string, routeConfig *model.EndpointConfig, rate }, UpgradeConfigs: getUpgradeConfig(apiType), MaxStreamDuration: getMaxStreamDuration(apiType), + IdleTimeout: durationpb.New(time.Duration(routeConfig.IdleTimeoutInSeconds) * time.Second), ClusterSpecifier: &routev3.RouteAction_ClusterHeader{ ClusterHeader: clusterHeaderName, }, @@ -118,7 +116,6 @@ func generateRouteAction(apiType string, routeConfig *model.EndpointConfig, rate }, } action.Route.RetryPolicy = commonRetryPolicy - fmt.Println("Action Retry: ", action.Route.RetryPolicy.NumRetries) } return action diff --git a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go index 9be09a627..2a8ae0814 100644 --- a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go +++ b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go @@ -66,8 +66,10 @@ type BackendSpec struct { CircuitBreaker *CircuitBreaker `json:"circuitBreaker,omitempty"` // +optional // Timeout congifuration for the backend - Timeout *Timeout `json:"timeout,omitempty"` - Retry *RetryConfig `json:"retry,omitempty"` + Timeout *Timeout `json:"timeout,omitempty"` + + // +optional + Retry *RetryConfig `json:"retry,omitempty"` } // Timeout defines the timeout configurations From 2ca3bc6790c8c505233cd82b6a641ffa9663d1d7 Mon Sep 17 00:00:00 2001 From: BLasan Date: Thu, 22 Jun 2023 10:29:10 +0530 Subject: [PATCH 08/16] Add: Retry configurations in Sample CRDs --- developer/tryout/samples/sample-backend.yaml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/developer/tryout/samples/sample-backend.yaml b/developer/tryout/samples/sample-backend.yaml index e2544a9ea..ded629e75 100644 --- a/developer/tryout/samples/sample-backend.yaml +++ b/developer/tryout/samples/sample-backend.yaml @@ -21,11 +21,6 @@ kind: Backend metadata: name: insecure-backend spec: - retry: - maxRetryCount: 3 - baseIntervalInMillis: 2000 - statusCodes: - - 504 services: - host: backend port: 3000 @@ -46,6 +41,12 @@ spec: maxRouteTimeoutSeconds: 60 routeIdleTimeoutSeconds: 10 routeTimeoutSeconds: 40 + retry: + maxRetryCount: 4 + baseIntervalInMillis: 2000 + statusCodes: + - 504 + - 500 services: - host: test-backend-svc port: 3000 From 431c5e07908e63c75c82530288cecaebebfe1531 Mon Sep 17 00:00:00 2001 From: BLasan Date: Thu, 22 Jun 2023 10:38:19 +0530 Subject: [PATCH 09/16] Remove external hostname from the sample CR --- developer/tryout/samples/sample-backend.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer/tryout/samples/sample-backend.yaml b/developer/tryout/samples/sample-backend.yaml index ded629e75..d35fabc00 100644 --- a/developer/tryout/samples/sample-backend.yaml +++ b/developer/tryout/samples/sample-backend.yaml @@ -57,7 +57,7 @@ metadata: name: test-backend-svc spec: type: ExternalName - externalName: benura-4.local + externalName: ports: - name: http port: 3000 From 2a54d06f5d13f15b9173f970a49fcd486002ad70 Mon Sep 17 00:00:00 2001 From: BLasan Date: Wed, 28 Jun 2023 19:47:15 +0530 Subject: [PATCH 10/16] Remove: Retry Config from RouteActions Add validations to the Backend Retry configs --- .../oasparser/envoyconf/routes_configs.go | 19 ------------------- .../internal/oasparser/model/http_route.go | 5 ++++- .../apis/dp/v1alpha1/backend_types.go | 14 ++++++++++---- .../crd/bases/dp.wso2.com_backends.yaml | 12 ++++++++++-- .../operator/controllers/dp/api_controller.go | 11 +++++++++++ adapter/internal/operator/utils/utils.go | 6 +++--- adapter/pkg/logging/logging_constant.go | 12 ++++++++++++ developer/tryout/samples/sample-backend.yaml | 6 +++--- helm-charts/crds/dp.wso2.com_backends.yaml | 13 +++++++++++-- 9 files changed, 64 insertions(+), 34 deletions(-) diff --git a/adapter/internal/oasparser/envoyconf/routes_configs.go b/adapter/internal/oasparser/envoyconf/routes_configs.go index d8980abf1..3eef03004 100644 --- a/adapter/internal/oasparser/envoyconf/routes_configs.go +++ b/adapter/internal/oasparser/envoyconf/routes_configs.go @@ -99,25 +99,6 @@ func generateRouteAction(apiType string, routeConfig *model.EndpointConfig, rate 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. - commonRetryPolicy := &routev3.RetryPolicy{ - RetryOn: retryPolicyRetriableStatusCodes, - NumRetries: &wrapperspb.UInt32Value{ - Value: uint32(routeConfig.RetryConfig.Count), - // If not set to 0, default value 1 will be - }, - RetriableStatusCodes: routeConfig.RetryConfig.StatusCodes, - RetryBackOff: &routev3.RetryPolicy_RetryBackOff{ - BaseInterval: &durationpb.Duration{ - Nanos: int32(routeConfig.RetryConfig.BaseIntervalInMillis) * 1000, - }, - }, - } - action.Route.RetryPolicy = commonRetryPolicy - } - return action } diff --git a/adapter/internal/oasparser/model/http_route.go b/adapter/internal/oasparser/model/http_route.go index 4ea3f97ae..c902a20e0 100644 --- a/adapter/internal/oasparser/model/http_route.go +++ b/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 { @@ -87,6 +89,7 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR 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 @@ -230,7 +233,7 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR return fmt.Errorf("no backendref were provided") } var securityConfig []EndpointSecurity - isRetryConfigDefined := false + // isRetryConfigDefined := false for _, backend := range rule.BackendRefs { backendName := types.NamespacedName{ Name: string(backend.Name), diff --git a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go index 2a8ae0814..e51f86e20 100644 --- a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go +++ b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go @@ -100,14 +100,20 @@ type CircuitBreaker struct { type RetryBudget struct { BudgetPercent uint64 `json:"budgetPercent,omitempty"` MinRetryConcurrency uint32 `json:"minRetryConcurrency,omitempty"` - // RetryCount int32 `json:"retryCount,omitempty"` } // RetryConfig defines retry configurations type RetryConfig struct { - MaxRetryCount uint32 `json:"maxRetryCount,omitempty"` - BaseIntervalInMillis uint32 `json:"baseIntervalInMillis,omitempty"` - StatusCodes []uint32 `json:"statusCodes,omitempty"` + // +kubebuilder:default=5 + // MaxRetry defines the maximum number of retries + Count uint32 `json:"count"` + // +kubebuilder:default=25 + // BaseIntervalMillis defines the base interval in milliseconds + BaseIntervalMillis uint32 `json:"baseIntervalMillis"` + // +optional + // +kubebuilder:validation:"minimum=401,maximum=598 + // StatusCodes defines the list of status codes to retry + StatusCodes []uint32 `json:"statusCodes,omitempty"` } // Service holds host and port information for the service diff --git a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml index 16f1a3573..9882c4ef3 100644 --- a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml +++ b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml @@ -77,17 +77,25 @@ spec: retry: description: RetryConfig defines retry configurations properties: - baseIntervalInMillis: + baseIntervalMillis: + default: 25 + description: BaseIntervalMillis defines the base interval in milliseconds format: int32 type: integer - maxRetryCount: + count: + default: 5 + 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: diff --git a/adapter/internal/operator/controllers/dp/api_controller.go b/adapter/internal/operator/controllers/dp/api_controller.go index e11588398..1fbfb580c 100644 --- a/adapter/internal/operator/controllers/dp/api_controller.go +++ b/adapter/internal/operator/controllers/dp/api_controller.go @@ -876,6 +876,17 @@ func (apiReconciler *APIReconciler) getAPIsForScope(obj k8client.Object) []recon func (apiReconciler *APIReconciler) getAPIsForBackend(obj k8client.Object) []reconcile.Request { ctx := context.Background() backend, ok := obj.(*dpv1alpha1.Backend) + retryConfig := backend.Spec.Retry + if retryConfig != nil { + if int32(retryConfig.Count) < 0 { + loggers.LoggerAPKOperator.ErrorC(logging.GetErrorByCode(3117, utils.NamespacedName(backend).String())) + } + for _, statusCode := range retryConfig.StatusCodes { + if statusCode > 598 || statusCode < 401 { + loggers.LoggerAPKOperator.ErrorC(logging.GetErrorByCode(3116, utils.NamespacedName(backend).String())) + } + } + } if !ok { loggers.LoggerAPKOperator.ErrorC(logging.GetErrorByCode(2624, backend)) return []reconcile.Request{} diff --git a/adapter/internal/operator/utils/utils.go b/adapter/internal/operator/utils/utils.go index 4a759cc6f..71591b255 100644 --- a/adapter/internal/operator/utils/utils.go +++ b/adapter/internal/operator/utils/utils.go @@ -328,9 +328,9 @@ func GetResolvedBackend(ctx context.Context, client k8client.Client, } if backend.Spec.Retry != nil { resolvedBackend.Retry = &dpv1alpha1.RetryConfig{ - MaxRetryCount: backend.Spec.Retry.MaxRetryCount, - BaseIntervalInMillis: backend.Spec.Retry.BaseIntervalInMillis, - StatusCodes: backend.Spec.Retry.StatusCodes, + Count: backend.Spec.Retry.Count, + BaseIntervalMillis: backend.Spec.Retry.BaseIntervalMillis, + StatusCodes: backend.Spec.Retry.StatusCodes, } } if backend.Spec.TLS != nil { diff --git a/adapter/pkg/logging/logging_constant.go b/adapter/pkg/logging/logging_constant.go index 085503509..758585227 100644 --- a/adapter/pkg/logging/logging_constant.go +++ b/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, + }, } diff --git a/developer/tryout/samples/sample-backend.yaml b/developer/tryout/samples/sample-backend.yaml index d35fabc00..c88e18554 100644 --- a/developer/tryout/samples/sample-backend.yaml +++ b/developer/tryout/samples/sample-backend.yaml @@ -42,8 +42,8 @@ spec: routeIdleTimeoutSeconds: 10 routeTimeoutSeconds: 40 retry: - maxRetryCount: 4 - baseIntervalInMillis: 2000 + count: 4 + baseIntervalMillis: 2000 statusCodes: - 504 - 500 @@ -57,7 +57,7 @@ metadata: name: test-backend-svc spec: type: ExternalName - externalName: + externalName: benura-4.local ports: - name: http port: 3000 diff --git a/helm-charts/crds/dp.wso2.com_backends.yaml b/helm-charts/crds/dp.wso2.com_backends.yaml index fbb5caf1a..14e229b7a 100644 --- a/helm-charts/crds/dp.wso2.com_backends.yaml +++ b/helm-charts/crds/dp.wso2.com_backends.yaml @@ -91,18 +91,27 @@ spec: - wss type: string retry: + description: RetryConfig defines retry configurations properties: - baseIntervalInMillis: + baseIntervalMillis: + default: 25 + description: BaseIntervalMillis defines the base interval in milliseconds format: int32 type: integer - maxRetryCount: + count: + default: 5 + 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: From 8f63a3fbf50210bf96ee3570ffc36515f4bdd9c7 Mon Sep 17 00:00:00 2001 From: BLasan Date: Wed, 28 Jun 2023 23:23:34 +0530 Subject: [PATCH 11/16] Remove: RetryConfig from RouteParams --- .../oasparser/envoyconf/routes_with_clusters.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/adapter/internal/oasparser/envoyconf/routes_with_clusters.go b/adapter/internal/oasparser/envoyconf/routes_with_clusters.go index c3d2c7d2d..7e4c08d73 100644 --- a/adapter/internal/oasparser/envoyconf/routes_with_clusters.go +++ b/adapter/internal/oasparser/envoyconf/routes_with_clusters.go @@ -174,17 +174,6 @@ func CreateRoutesWithClusters(adapterInternalAPI model.AdapterInternalAPI, inter routeParams := genRouteCreateParams(&adapterInternalAPI, resource, vHost, basePath, clusterName, *operationalReqInterceptors, *operationalRespInterceptorVal, organizationID, false) - // set retry count if endpoint retry config is available - if endpoint.Config != nil && endpoint.Config.RetryConfig != nil { - routeParams.routeConfig = &model.EndpointConfig{ - RetryConfig: &model.RetryConfig{ - Count: endpoint.Config.RetryConfig.Count, - StatusCodes: endpoint.Config.RetryConfig.StatusCodes, - BaseIntervalInMillis: endpoint.Config.RetryConfig.BaseIntervalInMillis, - }, - } - } - routeP, err := createRoutes(routeParams) if err != nil { logger.LoggerXds.ErrorC(logging.GetErrorByCode(2231, adapterInternalAPI.GetTitle(), adapterInternalAPI.GetVersion(), resource.GetPath(), err.Error())) From b3ed6a532332ed02f72c77fd541d7b3b62cc5567 Mon Sep 17 00:00:00 2001 From: BLasan Date: Thu, 29 Jun 2023 19:27:02 +0530 Subject: [PATCH 12/16] Remove: Unwanted files Remove: Unwanted codes --- .../internal/oasparser/model/http_route.go | 3 +-- .../apis/dp/v1alpha1/backend_types.go | 1 - adapter/internal/operator/utils/utils.go | 2 -- .../tryout/samples/sample-application.yaml | 27 ------------------- .../tryout/samples/sample-organization.yaml | 27 ------------------- .../tryout/samples/sample-subscription.yaml | 27 ------------------- 6 files changed, 1 insertion(+), 86 deletions(-) delete mode 100644 developer/tryout/samples/sample-application.yaml delete mode 100644 developer/tryout/samples/sample-organization.yaml delete mode 100644 developer/tryout/samples/sample-subscription.yaml diff --git a/adapter/internal/oasparser/model/http_route.go b/adapter/internal/oasparser/model/http_route.go index c902a20e0..57023f6e7 100644 --- a/adapter/internal/oasparser/model/http_route.go +++ b/adapter/internal/oasparser/model/http_route.go @@ -233,7 +233,6 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR return fmt.Errorf("no backendref were provided") } var securityConfig []EndpointSecurity - // isRetryConfigDefined := false for _, backend := range rule.BackendRefs { backendName := types.NamespacedName{ Name: string(backend.Name), @@ -261,7 +260,7 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR if resolvedBackend.Retry.MaxRetryCount > 0 { backendRetryCount = resolvedBackend.Retry.MaxRetryCount } - if resolvedBackend.Retry.StatusCodes != nil && len(resolvedBackend.Retry.StatusCodes) > 0 { + if len(resolvedBackend.Retry.StatusCodes) > 0 { statusCodes = resolvedBackend.Retry.StatusCodes } if resolvedBackend.Retry.BaseIntervalInMillis > 0 { diff --git a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go index e51f86e20..5576680b8 100644 --- a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go +++ b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go @@ -111,7 +111,6 @@ type RetryConfig struct { // BaseIntervalMillis defines the base interval in milliseconds BaseIntervalMillis uint32 `json:"baseIntervalMillis"` // +optional - // +kubebuilder:validation:"minimum=401,maximum=598 // StatusCodes defines the list of status codes to retry StatusCodes []uint32 `json:"statusCodes,omitempty"` } diff --git a/adapter/internal/operator/utils/utils.go b/adapter/internal/operator/utils/utils.go index 71591b255..270a17660 100644 --- a/adapter/internal/operator/utils/utils.go +++ b/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,7 +317,6 @@ 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, diff --git a/developer/tryout/samples/sample-application.yaml b/developer/tryout/samples/sample-application.yaml deleted file mode 100644 index d15bfa4d0..000000000 --- a/developer/tryout/samples/sample-application.yaml +++ /dev/null @@ -1,27 +0,0 @@ -# # -------------------------------------------------------------------- -# # Copyright (c) 2023, WSO2 LLC. (http://wso2.com) All Rights Reserved. -# # -# # Licensed under the Apache License, Version 2.0 (the "License"); -# # you may not use this file except in compliance with the License. -# # You may obtain a copy of the License at -# # -# # http://www.apache.org/licenses/LICENSE-2.0 -# # -# # Unless required by applicable law or agreed to in writing, software -# # distributed under the License is distributed on an "AS IS" BASIS, -# # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# # See the License for the specific language governing permissions and -# # limitations under the License. -# # ----------------------------------------------------------------------- -# apiVersion: cp.wso2.com/v1alpha1 -# kind: Application -# metadata: -# name: sample-application-uuid -# namespace: apk -# spec: -# name: sample-application -# owner: admin -# policy: 30ReqPerMin -# attributes: -# keys: -# organization: carbon.super \ No newline at end of file diff --git a/developer/tryout/samples/sample-organization.yaml b/developer/tryout/samples/sample-organization.yaml deleted file mode 100644 index 9e10e88c4..000000000 --- a/developer/tryout/samples/sample-organization.yaml +++ /dev/null @@ -1,27 +0,0 @@ -# # -------------------------------------------------------------------- -# # Copyright (c) 2023, WSO2 LLC. (http://wso2.com) All Rights Reserved. -# # -# # Licensed under the Apache License, Version 2.0 (the "License"); -# # you may not use this file except in compliance with the License. -# # You may obtain a copy of the License at -# # -# # http://www.apache.org/licenses/LICENSE-2.0 -# # -# # Unless required by applicable law or agreed to in writing, software -# # distributed under the License is distributed on an "AS IS" BASIS, -# # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# # See the License for the specific language governing permissions and -# # limitations under the License. -# # ----------------------------------------------------------------------- -# apiVersion: cp.wso2.com/v1alpha1 -# kind: Organization -# metadata: -# name: org1 -# spec: -# displayName: org1 -# enabled: true -# name: org1 -# organizationClaimValue: org1 -# serviceListingNamespaces: -# - '*' -# uuid: 01edb285-6304-1b20-a090-4d02067ed56e diff --git a/developer/tryout/samples/sample-subscription.yaml b/developer/tryout/samples/sample-subscription.yaml deleted file mode 100644 index 8313b15e4..000000000 --- a/developer/tryout/samples/sample-subscription.yaml +++ /dev/null @@ -1,27 +0,0 @@ -# # -------------------------------------------------------------------- -# # Copyright (c) 2023, WSO2 LLC. (http://wso2.com) All Rights Reserved. -# # -# # Licensed under the Apache License, Version 2.0 (the "License"); -# # you may not use this file except in compliance with the License. -# # You may obtain a copy of the License at -# # -# # http://www.apache.org/licenses/LICENSE-2.0 -# # -# # Unless required by applicable law or agreed to in writing, software -# # distributed under the License is distributed on an "AS IS" BASIS, -# # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# # See the License for the specific language governing permissions and -# # limitations under the License. -# # ----------------------------------------------------------------------- -# apiVersion: cp.wso2.com/v1alpha1 -# kind: Subscription -# metadata: -# name: subscription-uuid -# namespace: apk -# spec: -# policyId: businessPlan1 -# apiRef: http-bin-api-uuid -# applicationRef: sample-application-uuid -# subscriptionStatus: UNBLOCKED -# subscriber: apk-user -# organization: carbon.super \ No newline at end of file From 032fa49fa3f22267f383704b289eff313a3701c2 Mon Sep 17 00:00:00 2001 From: BLasan Date: Fri, 30 Jun 2023 10:35:43 +0530 Subject: [PATCH 13/16] Fix: HttpRoute Errors --- adapter/internal/oasparser/model/http_route.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/adapter/internal/oasparser/model/http_route.go b/adapter/internal/oasparser/model/http_route.go index 57023f6e7..272131ec0 100644 --- a/adapter/internal/oasparser/model/http_route.go +++ b/adapter/internal/oasparser/model/http_route.go @@ -257,14 +257,14 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR if resolvedBackend.Retry != nil { isRetryConfig = true - if resolvedBackend.Retry.MaxRetryCount > 0 { - backendRetryCount = resolvedBackend.Retry.MaxRetryCount + if resolvedBackend.Retry.Count > 0 { + backendRetryCount = resolvedBackend.Retry.Count } if len(resolvedBackend.Retry.StatusCodes) > 0 { statusCodes = resolvedBackend.Retry.StatusCodes } - if resolvedBackend.Retry.BaseIntervalInMillis > 0 { - baseIntervalInMillis = resolvedBackend.Retry.BaseIntervalInMillis + if resolvedBackend.Retry.BaseIntervalMillis > 0 { + baseIntervalInMillis = resolvedBackend.Retry.BaseIntervalMillis } } endPoints = append(endPoints, GetEndpoints(backendName, httpRouteParams.BackendMapping)...) From 33b15cc01dda213d73bddc78838ff7c08b7d38b7 Mon Sep 17 00:00:00 2001 From: BLasan Date: Fri, 30 Jun 2023 10:49:43 +0530 Subject: [PATCH 14/16] Fix: Memory nil issue --- adapter/internal/oasparser/envoyconf/routes_configs.go | 1 - 1 file changed, 1 deletion(-) diff --git a/adapter/internal/oasparser/envoyconf/routes_configs.go b/adapter/internal/oasparser/envoyconf/routes_configs.go index 3eef03004..48cc8f86e 100644 --- a/adapter/internal/oasparser/envoyconf/routes_configs.go +++ b/adapter/internal/oasparser/envoyconf/routes_configs.go @@ -85,7 +85,6 @@ func generateRouteAction(apiType string, routeConfig *model.EndpointConfig, rate }, UpgradeConfigs: getUpgradeConfig(apiType), MaxStreamDuration: getMaxStreamDuration(apiType), - IdleTimeout: durationpb.New(time.Duration(routeConfig.IdleTimeoutInSeconds) * time.Second), ClusterSpecifier: &routev3.RouteAction_ClusterHeader{ ClusterHeader: clusterHeaderName, }, From bdf4732add5d45a6f7d6b71d1fbe68dff94604c7 Mon Sep 17 00:00:00 2001 From: BLasan Date: Fri, 30 Jun 2023 10:49:58 +0530 Subject: [PATCH 15/16] Add: Validation to the retry configs --- .../operator/apis/dp/v1alpha1/backend_webhook.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/adapter/internal/operator/apis/dp/v1alpha1/backend_webhook.go b/adapter/internal/operator/apis/dp/v1alpha1/backend_webhook.go index 6f62f6de8..54d179730 100644 --- a/adapter/internal/operator/apis/dp/v1alpha1/backend_webhook.go +++ b/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"}, From 10ed01f5e2bc424c08e5102e8702c5c4db6fc11d Mon Sep 17 00:00:00 2001 From: BLasan Date: Fri, 30 Jun 2023 11:59:17 +0530 Subject: [PATCH 16/16] Resolve: Merge conflicts --- adapter/config/default_config.go | 4 +--- adapter/config/types.go | 21 ++++++------------- .../oasparser/envoyconf/routes_configs.go | 9 ++++++++ .../oasparser/model/adapter_internal_api.go | 6 ------ .../internal/oasparser/model/http_route.go | 8 ++----- .../apis/dp/v1alpha1/backend_types.go | 4 ++-- .../crd/bases/dp.wso2.com_backends.yaml | 4 ++-- .../operator/controllers/dp/api_controller.go | 11 ---------- helm-charts/crds/dp.wso2.com_backends.yaml | 4 ++-- 9 files changed, 24 insertions(+), 47 deletions(-) diff --git a/adapter/config/default_config.go b/adapter/config/default_config.go index 80e80d2bd..650188bbc 100644 --- a/adapter/config/default_config.go +++ b/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, diff --git a/adapter/config/types.go b/adapter/config/types.go index 08c9270b6..b7378d2aa 100644 --- a/adapter/config/types.go +++ b/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 { diff --git a/adapter/internal/oasparser/envoyconf/routes_configs.go b/adapter/internal/oasparser/envoyconf/routes_configs.go index 48cc8f86e..f075d9577 100644 --- a/adapter/internal/oasparser/envoyconf/routes_configs.go +++ b/adapter/internal/oasparser/envoyconf/routes_configs.go @@ -94,6 +94,15 @@ func generateRouteAction(apiType string, routeConfig *model.EndpointConfig, rate action.Route.IdleTimeout = durationpb.New(time.Duration(routeConfig.IdleTimeoutInSeconds) * time.Second) } + if routeConfig != nil && routeConfig.RetryConfig != nil { + retryPolicy := &routev3.RetryPolicy{ + RetryBackOff: &routev3.RetryPolicy_RetryBackOff{ + BaseInterval: durationpb.New(time.Duration(routeConfig.RetryConfig.BaseIntervalInMillis) * time.Millisecond), + }, + } + action.Route.RetryPolicy = retryPolicy + } + if ratelimitCriteria != nil && ratelimitCriteria.level != "" { action.Route.RateLimits = generateRateLimitPolicy(ratelimitCriteria) } diff --git a/adapter/internal/oasparser/model/adapter_internal_api.go b/adapter/internal/oasparser/model/adapter_internal_api.go index fc9b1b2e2..106a2afd0 100644 --- a/adapter/internal/oasparser/model/adapter_internal_api.go +++ b/adapter/internal/oasparser/model/adapter_internal_api.go @@ -402,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 { diff --git a/adapter/internal/oasparser/model/http_route.go b/adapter/internal/oasparser/model/http_route.go index 272131ec0..49dd2919f 100644 --- a/adapter/internal/oasparser/model/http_route.go +++ b/adapter/internal/oasparser/model/http_route.go @@ -257,15 +257,11 @@ func (swagger *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwapiv1b1.HTTPR if resolvedBackend.Retry != nil { isRetryConfig = true - if resolvedBackend.Retry.Count > 0 { - backendRetryCount = resolvedBackend.Retry.Count - } + backendRetryCount = resolvedBackend.Retry.Count + baseIntervalInMillis = resolvedBackend.Retry.BaseIntervalMillis if len(resolvedBackend.Retry.StatusCodes) > 0 { statusCodes = resolvedBackend.Retry.StatusCodes } - if resolvedBackend.Retry.BaseIntervalMillis > 0 { - baseIntervalInMillis = resolvedBackend.Retry.BaseIntervalMillis - } } endPoints = append(endPoints, GetEndpoints(backendName, httpRouteParams.BackendMapping)...) for _, security := range resolvedBackend.Security { diff --git a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go index 5576680b8..5967f95c0 100644 --- a/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go +++ b/adapter/internal/operator/apis/dp/v1alpha1/backend_types.go @@ -104,10 +104,10 @@ type RetryBudget struct { // RetryConfig defines retry configurations type RetryConfig struct { - // +kubebuilder:default=5 + // +kubebuilder:default=1 // MaxRetry defines the maximum number of retries Count uint32 `json:"count"` - // +kubebuilder:default=25 + // +kubebuilder:default=2000 // BaseIntervalMillis defines the base interval in milliseconds BaseIntervalMillis uint32 `json:"baseIntervalMillis"` // +optional diff --git a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml index 9882c4ef3..84e6e08af 100644 --- a/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml +++ b/adapter/internal/operator/config/crd/bases/dp.wso2.com_backends.yaml @@ -78,12 +78,12 @@ spec: description: RetryConfig defines retry configurations properties: baseIntervalMillis: - default: 25 + default: 2000 description: BaseIntervalMillis defines the base interval in milliseconds format: int32 type: integer count: - default: 5 + default: 1 description: MaxRetry defines the maximum number of retries format: int32 type: integer diff --git a/adapter/internal/operator/controllers/dp/api_controller.go b/adapter/internal/operator/controllers/dp/api_controller.go index 1fbfb580c..e11588398 100644 --- a/adapter/internal/operator/controllers/dp/api_controller.go +++ b/adapter/internal/operator/controllers/dp/api_controller.go @@ -876,17 +876,6 @@ func (apiReconciler *APIReconciler) getAPIsForScope(obj k8client.Object) []recon func (apiReconciler *APIReconciler) getAPIsForBackend(obj k8client.Object) []reconcile.Request { ctx := context.Background() backend, ok := obj.(*dpv1alpha1.Backend) - retryConfig := backend.Spec.Retry - if retryConfig != nil { - if int32(retryConfig.Count) < 0 { - loggers.LoggerAPKOperator.ErrorC(logging.GetErrorByCode(3117, utils.NamespacedName(backend).String())) - } - for _, statusCode := range retryConfig.StatusCodes { - if statusCode > 598 || statusCode < 401 { - loggers.LoggerAPKOperator.ErrorC(logging.GetErrorByCode(3116, utils.NamespacedName(backend).String())) - } - } - } if !ok { loggers.LoggerAPKOperator.ErrorC(logging.GetErrorByCode(2624, backend)) return []reconcile.Request{} diff --git a/helm-charts/crds/dp.wso2.com_backends.yaml b/helm-charts/crds/dp.wso2.com_backends.yaml index 14e229b7a..63b73e307 100644 --- a/helm-charts/crds/dp.wso2.com_backends.yaml +++ b/helm-charts/crds/dp.wso2.com_backends.yaml @@ -94,12 +94,12 @@ spec: description: RetryConfig defines retry configurations properties: baseIntervalMillis: - default: 25 + default: 2000 description: BaseIntervalMillis defines the base interval in milliseconds format: int32 type: integer count: - default: 5 + default: 1 description: MaxRetry defines the maximum number of retries format: int32 type: integer