osm
osm copied to clipboard
Retries: consolidate retries into UpstreamTrafficSetting API
Please describe the Improvement and/or Feature Request
The UpstreamTrafficSetting API is used to configure settings for traffic directed to upstream services. Currently, circuit breaking and rate limiting are a part of the UpstreamTrafficSetting spec.
A long term goal has been to consolidate the retry policy into the same UpstreamTrafficSetting API, as it allows more fine-grained control over applying HTTP policies at the virtual_host or route level. Moreover, it simplifies various code paths as custom policies pertaining to upstream services would be readily available on the UpstreamTrafficSetting object, thereby removing the need for multiple informers, caches, events, internal APIs, etc.
As a proposal, I would like to have the existing RetryPolicy (alpha feature) merged into the UpstreamTrafficSetting API as follows:
apiVersion: policy.openservicemesh.io/v1alpha1
kind: UpstreamTrafficSetting
metadata:
name: foo
namespace: test
spec:
host: foo.test.svc.cluster.local
retries: # applied on virtual_host
retryOn: "5xx"
perTryTimeout: 1s
numRetries: 5
retryBackoffBaseInterval: 1s
sources:
- kind: ServiceAccount
namespace: curl
name: curl
httpRoutes: # applied on routes within virtual_host
- path: /foo
retries:
retryOn: "5xx"
perTryTimeout: 1s
numRetries: 5
retryBackoffBaseInterval: 1s
sources:
- kind: ServiceAccount
namespace: curl
name: curl
Scope (please mark with X where applicable)
- Retries [X]
@shalier As a part of #2018, I am going to add support to plumb the policy in UpstreamTrafficSetting at the inbound route and virtual_host level. Since retries and rate limiting policies are at the same level, it would be easier for you to update the retries code once the initial part of #2018 is done. However, you may independently start updating UpstreamTrafficSetting API schema as I don't intend to modify it further at the moment.
@shalier I wanted to also note that before proceeding with an implementation, it would make sense to verify if merging the Retries policy with the UpstreamTrafficSetting API is feasible. I have outlined a sketch in the description, but it does not imply the design is vetted.
This issue will be closed due to a long period of inactivity. If you would like this issue to remain open then please comment or update.
Issue closed due to inactivity.