-
Notifications
You must be signed in to change notification settings - Fork 480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
apis: add implementation for GEP-1731 HTTPRoute Retries #3301
Changes from all commits
3eec49c
a947b20
0668b8d
68cfd22
c9e6e48
7f1d02f
5d24b39
aaf0ecb
1993647
d342f6b
c1f074c
86f5d87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -296,6 +296,14 @@ type HTTPRouteRule struct { | |
// +optional | ||
Timeouts *HTTPRouteTimeouts `json:"timeouts,omitempty"` | ||
|
||
// Retry defines the configuration for when to retry an HTTP request. | ||
// | ||
// Support: Extended | ||
// | ||
// +optional | ||
// <gateway:experimental> | ||
Retry *HTTPRouteRetry `json:"retry,omitempty"` | ||
|
||
// SessionPersistence defines and configures session persistence | ||
// for the route rule. | ||
// | ||
|
@@ -361,6 +369,95 @@ type HTTPRouteTimeouts struct { | |
BackendRequest *Duration `json:"backendRequest,omitempty"` | ||
} | ||
|
||
// HTTPRouteRetry defines retry configuration for an HTTPRoute. | ||
// | ||
// Implementations SHOULD retry on connection errors (disconnect, reset, timeout, | ||
// TCP failure) if a retry stanza is configured. | ||
type HTTPRouteRetry struct { | ||
// Codes defines the HTTP response status codes for which a backend request | ||
// should be retried. | ||
// | ||
// Support: Extended | ||
// | ||
// +optional | ||
Codes []HTTPRouteRetryStatusCode `json:"codes,omitempty"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If at some point we want to include strings here, do we add a new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly what I had been thinking if that we end up wanting that in the future! Per above comment on connection error variance between implementations, a |
||
|
||
// Attempts specifies the maxmimum number of times an individual request | ||
// from the gateway to a backend should be retried. | ||
// | ||
// If the maximum number of retries has been attempted without a successful | ||
// response from the backend, the Gateway MUST return an error. | ||
// | ||
// When this field is unspecified, the number of times to attempt to retry | ||
// a backend request is implementation-specific. | ||
// | ||
// Support: Extended | ||
// | ||
// +optional | ||
Attempts *int `json:"attempts,omitempty"` | ||
|
||
// Backoff specifies the minimum duration a Gateway should wait between | ||
// retry attempts and is represented in Gateway API Duration formatting. | ||
// | ||
// For example, setting the `rules[].retry.backoff` field to the value | ||
// `100ms` will cause a backend request to first be retried approximately | ||
// 100 milliseconds after timing out or receiving a response code configured | ||
// to be retryable. | ||
// | ||
// An implementation MAY use an exponential or alternative backoff strategy | ||
// for subsequent retry attempts, MAY cap the maximum backoff duration to | ||
// some amount greater than the specified minimum, and MAY add arbitrary | ||
// jitter to stagger requests, as long as unsuccessful backend requests are | ||
// not retried before the configured minimum duration. | ||
// | ||
// If a Request timeout (`rules[].timeouts.request`) is configured on the | ||
// route, the entire duration of the initial request and any retry attempts | ||
// MUST not exceed the Request timeout duration. If any retry attempts are | ||
// still in progress when the Request timeout duration has been reached, | ||
// these SHOULD be canceled if possible and the Gateway MUST immediately | ||
// return a timeout error. | ||
// | ||
// If a BackendRequest timeout (`rules[].timeouts.backendRequest`) is | ||
// configured on the route, any retry attempts which reach the configured | ||
// BackendRequest timeout duration without a response SHOULD be canceled if | ||
// possible and the Gateway should wait for at least the specified backoff | ||
// duration before attempting to retry the backend request again. | ||
// | ||
// If a BackendRequest timeout is _not_ configured on the route, retry | ||
// attempts MAY time out after an implementation default duration, or MAY | ||
// remain pending until a configured Request timeout or implementation | ||
// default duration for total request time is reached. | ||
// | ||
// When this field is unspecified, the time to wait between retry attempts | ||
// is implementation-specific. | ||
// | ||
// Support: Extended | ||
// | ||
// +optional | ||
Backoff *Duration `json:"backoff,omitempty"` | ||
} | ||
|
||
// HTTPRouteRetryStatusCode defines an HTTP response status code for | ||
// which a backend request should be retried. | ||
// | ||
// Implementations MUST support the following status codes as retryable: | ||
// | ||
// * 500 | ||
// * 502 | ||
// * 503 | ||
// * 504 | ||
// | ||
// Implementations MAY support specifying additional discrete values in the | ||
// 500-599 range. | ||
// | ||
// Implementations MAY support specifying discrete values in the 400-499 range, | ||
// which are often inadvisable to retry. | ||
// | ||
// +kubebuilder:validation:Minimum:=400 | ||
// +kubebuilder:validation:Maximum:=599 | ||
mikemorris marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// <gateway:experimental> | ||
type HTTPRouteRetryStatusCode int | ||
|
||
// PathMatchType specifies the semantics of how HTTP paths should be compared. | ||
// Valid PathMatchType values, along with their support levels, are: | ||
// | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason not to make this configurable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Primarily because there's way too much variance in how different implementations define, default, bucket or even allow these to be configurable for this to be generally implementable within extended conformance, and secondarily because it's a simpler UX for what is often a good practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FTR I think that makes sense, @mikemorris.