Make panic threshold configurable for cluster #5118
Conversation
Signed-off-by: Kateryna Nezdolii <[email protected]>
|
|
||
| if policy.Spec.CommonLbSettings != nil { | ||
| if lbSettings, err = buildCommonLbSettings(policy.Spec.CommonLbSettings); err != nil { | ||
| err = perr.WithMessage(err, "RateLimit") |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5118 +/- ##
==========================================
- Coverage 66.92% 66.90% -0.02%
==========================================
Files 210 210
Lines 32995 33006 +11
==========================================
+ Hits 22081 22083 +2
- Misses 9579 9587 +8
- Partials 1335 1336 +1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Kateryna Nezdolii <[email protected]>
Signed-off-by: Kateryna Nezdolii <[email protected]>
api/v1alpha1/shared_types.go
Outdated
| @@ -579,6 +579,22 @@ type ClusterSettings struct { | |||
| // | |||
| // +optional | |||
| HTTP2 *HTTP2Settings `json:"http2,omitempty"` | |||
|
|
|||
There was a problem hiding this comment.
my preference would be to add a PanicThreshold unit32 field inside HealthCheck supporting values between 0-100
gateway/api/v1alpha1/healthcheck_types.go
Line 17 in 0e52d06
Signed-off-by: Kateryna Nezdolii <[email protected]>
internal/ir/xds.go
Outdated
| @@ -2456,6 +2459,12 @@ func (h *HealthCheck) Validate() error { | |||
| } | |||
| } | |||
|
|
|||
| if h.PanicThreshold != nil { | |||
| if *h.PanicThreshold < 0 || *h.PanicThreshold > 100 { | |||
There was a problem hiding this comment.
realistically it cannot be < 0 due to uint32 type
There was a problem hiding this comment.
Error: internal/ir/xds.go:2463:6: SA4003: no value of type uint32 is less than 0 (staticcheck)
if *h.PanicThreshold < 0 || *h.PanicThreshold > 100 {
CI is smart :)
| @@ -1421,6 +1439,7 @@ func TestValidateHealthCheck(t *testing.T) { | |||
| }, | |||
| }, | |||
| &OutlierDetection{}, | |||
| ptr.To[uint32](10), | |||
There was a problem hiding this comment.
I should be able just to set it to nil
Signed-off-by: Kateryna Nezdolii <[email protected]>
|
there is a failure for conformance test |
There was a problem hiding this comment.
LGTM thanks !
as a follow up can you add a test case in https://github.com/envoyproxy/gateway/tree/main/internal/xds/translator/testdata/in/xds-ir
Signed-off-by: Kateryna Nezdolii <[email protected]>
feat(api): Make panic threshold configurable per backend/cluster
Fixes #4015
Release Notes: TBD