Add outlier detection failure matcher#3741
Conversation
|
😊 Welcome @Samarth6840! This is either your first contribution to the Istio api repo, or it's been You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
istio-testing
added
size/S
|
Hi @Samarth6840. Thanks for your PR. I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
aa6b8ce to
0ab9dbe
Compare
|
/ok-to-test |
istio-testing
added
ok-to-test
| // applicable in k8s environments with few pods per service. | ||
| int32 min_health_percent = 5; | ||
|
|
||
| // Specifies additional HTTP response status codes that should be treated as |
There was a problem hiding this comment.
@Samarth6840 : The comment says "in addition to" but Envoy's error_matcher is an override, could you please fix the comment to reflect that ? You can find the exact behavior at https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/upstreams/http/v3/http_protocol_options.proto#extensions-upstreams-http-v3-httpprotocoloptions-outlierdetection
kfaseela
added
the
release-notes-none
|
/test release-notes |
|
@ramaraochavali : This API looks good to me, may be in future we can have some MeshConfig level defaults as well, but this does not have to be blocked. I have also pushed the implementation PR. Could you please approve the API if it looks good? |
…tinationRule
Wires outlierDetectionHttpErrorCodes from DestinationRule.OutlierDetection
into Envoy's HttpProtocolOptions.OutlierDetection.error_matcher.
API: istio/api#3741
Resolves: istio#60828
Signed-off-by: Faseela K <faseela.k@est.tech>
| int32 min_health_percent = 5; | ||
|
|
||
| // Specifies HTTP response status codes that should be treated as | ||
| // outlier detection errors. Overrides the default 5xx behavior. |
There was a problem hiding this comment.
This and next statement are contradicting to each other? when you specify this field and consecutive5xxErrors both - I assume both put together are treated as outliers?
There was a problem hiding this comment.
@ramaraochavali : Could you please take a look if the updates description looks clearer?
Co-authored-by: Faseela K <k.faseela@gmail.com>
| // Specifies the HTTP response status codes that are treated as outlier | ||
| // detection errors. If specified, only responses with one of these status | ||
| // codes will be treated as errors by outlier detection. If not specified, | ||
| // only 5xx responses are treated as errors. |
There was a problem hiding this comment.
the outliers are detected based on consecutive5xx and consecutiveGatewayErrors fields.
|
|
||
| // Specifies the HTTP response status codes that are treated as outlier | ||
| // detection errors. If specified, only responses with one of these status | ||
| // codes will be treated as errors by outlier detection. If not specified, | ||
| // only 5xx responses are treated as errors. | ||
| // | ||
| // Values must be in the range [100, 599]. | ||
| // | ||
| // +protoc-gen-crd:list-value-validation:Minimum=100 | ||
| // +protoc-gen-crd:list-value-validation:Maximum=599 | ||
| repeated uint32 outlier_detection_http_error_codes = 10; |
There was a problem hiding this comment.
| // Specifies the HTTP response status codes that are treated as outlier | |
| // detection errors. If specified, only responses with one of these status | |
| // codes will be treated as errors by outlier detection. If not specified, | |
| // only 5xx responses are treated as errors. | |
| // | |
| // Values must be in the range [100, 599]. | |
| // | |
| // +protoc-gen-crd:list-value-validation:Minimum=100 | |
| // +protoc-gen-crd:list-value-validation:Maximum=599 | |
| repeated uint32 outlier_detection_http_error_codes = 10; | |
| // Specifies the HTTP response status codes that are treated as outlier | |
| // detection errors. If specified, only responses with one of these status | |
| // codes will be treated as errors by outlier detection. If not specified, | |
| // only 5xx responses are treated as errors. | |
| // | |
| // Note: Host ejection is still driven by the `consecutive5xxErrors` and | |
| // `consecutiveGatewayErrors` thresholds; this field only redefines which | |
| // HTTP status codes are counted as errors toward those thresholds. | |
| // | |
| // Values must be in the range [100, 599]. | |
| // | |
| // +protoc-gen-crd:list-value-validation:Minimum=100 | |
| // +protoc-gen-crd:list-value-validation:Maximum=599 | |
| repeated uint32 outlier_detection_http_error_codes = 10; |
|
@Samarth6840 : please use the updated description I have given , and also run make gen. |
5 participants
What this PR does
Adds
outlier_detection_http_error_codestoOutlierDetectionin DestinationRule,allowing users to specify additional HTTP status codes (beyond the default 5xx) to
be treated as outlier detection failures.
Maps to Envoy's new
outlier_detection.failure_matcherinHttpProtocolOptions(envoyproxy/envoy#39947).
Follows the same simplification pattern as
consecutiveGatewayErrors/consecutive5xxErrors(#1189) — a plain repeated field rather than exposingEnvoy's raw MatchPredicate, consistent with Istio's existing approach (e.g.
VirtualService's
retryOn).Related
Implements the API portion of istio/istio#60828
Next steps
Wiring into
istio/istio'scluster_traffic_policy.go(HttpProtocolOptionssection) will follow in a separate PR.