API: Support FaultInjection in BackendTrafficPolicy #2304

tmsnan · 2023-12-15T06:20:38Z

What type of PR is this?

API: Support FaultInjection in BackendTrafficPolicy
Provide basic fault injection capabilities：
Supports injecting fixed time delay or abort errors by percentage

Example

---
apiVersion: gateway.envoyproxy.io/v1alpha1
kind: BackendTrafficPolicy
metadata:
  name: example-policy-with-fault-injection
spec:
 [...]
  faultInjection: # optional
    abort: # optional
      statusCode: # required
      percentage: # default 100
    delay: # optional
      fixedDelay: # required
      percentage: # default 100
  
[...]

Relates to #2124

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

github-actions · 2023-12-15T06:20:52Z

🚀 Thank you for contributing to the Envoy Gateway project! 🚀

Before merging, please ensure to follow the process below:

Requesting Reviews:

cc @envoyproxy/gateway-reviewers team for an initial review.
After the initial review, reviewers should request the @envoyproxy/gateway-maintainers team for further review.

Review Approval:

Your PR needs to receive at least two approvals.
At least one approval must come from a member of the gateway-maintainers team.

NOTE: Once your PR is under review, please do not rebase and force push it. Otherwise, it will force your reviewers to review the PR from scratch rather than simply look at your latest changes.

What's more, you can help expedite the processing of your PR by

Ensuring you have self-reviewed your work according to the project's Contribution Guidelines.
If your PR addresses a specific issue, make sure to mention it in the PR description.
Respond promptly if there are any test failures or suggestions for improvements that we comment on.

github-actions · 2023-12-15T06:23:39Z

🚀 Deployed on https://gateway-pr-2304-preview--eg-preview.netlify.app

codecov · 2023-12-15T06:23:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (64d7152) 64.54% compared to head (fcd4f79) 64.53%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2304      +/-   ##
==========================================
- Coverage   64.54%   64.53%   -0.02%     
==========================================
  Files         112      112              
  Lines       15949    15949              
==========================================
- Hits        10295    10293       -2     
- Misses       5007     5009       +2     
  Partials      647      647

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

arkodg · 2023-12-15T19:43:14Z

api/v1alpha1/backendtrafficpolicy_types.go

@@ -65,6 +65,10 @@ type BackendTrafficPolicySpec struct {
 	//
 	// +optional
 	TCPKeepalive *TCPKeepalive `json:"tcpKeepalive,omitempty"`
+
+	// FaultInjection defines the fault injection policy to be applied. Support delays and aborts.


Suggested change

// FaultInjection defines the fault injection policy to be applied. Support delays and aborts.

// FaultInjection defines the fault injection policy to be applied. This configuration can be used to

// inject delays and abort requests to mimic failure scenarios such as service failures and overloads.

arkodg · 2023-12-15T19:46:46Z

api/v1alpha1/fault_injection.go

+// FaultInjection defines the fault injection policy to be applied. Support delays and aborts.
+type FaultInjection struct {
+
+	// If specified, the delay will inject a fixed delay into the request


Suggested change

// If specified, the delay will inject a fixed delay into the request

// If specified, a delay will be injected into the request

arkodg · 2023-12-15T19:48:04Z

api/v1alpha1/fault_injection.go

+	// +optional
+	Delay *DelayConfig `json:"delay,omitempty"`
+
+	// If specified, the abort will abort the request with the specified HTTP status code


Suggested change

// If specified, the abort will abort the request with the specified HTTP status code

// If specified, the request will be aborted if it meets the configuration criteria.

arkodg · 2023-12-15T19:48:21Z

api/v1alpha1/fault_injection.go

+	// If specified, the delay will inject a fixed delay into the request
+	//
+	// +optional
+	Delay *DelayConfig `json:"delay,omitempty"`


Suggested change

Delay *DelayConfig `json:"delay,omitempty"`

Delay *FaultInjectionDelay `json:"delay,omitempty"`

arkodg · 2023-12-15T19:48:31Z

api/v1alpha1/fault_injection.go

+	// If specified, the abort will abort the request with the specified HTTP status code
+	//
+	// +optional
+	Abort *AbortConfig `json:"abort,omitempty"`


Suggested change

Abort *AbortConfig `json:"abort,omitempty"`

Abort *FaultInjectionAbort `json:"abort,omitempty"`

arkodg · 2023-12-15T19:50:19Z

api/v1alpha1/fault_injection.go

+	// +required
+	FixedDelay *metav1.Duration `json:"fixedDelay"`
+
+	// Percentage specifies the percentage of requests to be delayed. Default 100%, if set 0, no requests will be delayed.


so we can only express int e.g. 1%, 2%, but not fractions such as 0.5% ?

arkodg · 2023-12-15T19:51:28Z

api/v1alpha1/fault_injection.go

+	// +required
+	StatusCode *int `json:"statusCode"`
+
+	// Percentage specifies the percentage of requests to be aborted. Default 100%, if set 0, no requests will be aborted.


same as https://github.com/envoyproxy/gateway/pull/2304/files#r1428419030

arkodg · 2023-12-15T19:53:59Z

api/v1alpha1/fault_injection.go

+
+// AbortConfig defines the abort fault injection configuration
+type AbortConfig struct {
+	// StatusCode specifies the HTTP/GRPC status code to be returned


grpc status is different from http status

one way to define this is

status: type: HTTP http: 200

or define httpStatus & grpcStatus and use CEL to make sure both cannot be simultaneously defined

done, used CEL

Signed-off-by: zhaonan <[email protected]>

shawnh2 · 2023-12-18T12:03:57Z

api/v1alpha1/fault_injection.go

+	// Percentage specifies the percentage of requests to be delayed. Default 100%, if set 0, no requests will be delayed. Accuracy to 0.0001%.
+	// +optional
+	// +kubebuilder:default=100
+	Percentage *float64 `json:"percentage"`


omitempty?

shawnh2 · 2023-12-18T12:04:54Z

api/v1alpha1/fault_injection.go

+// +union
+//
+// +kubebuilder:validation:XValidation:rule=" !(has(self.httpStatus) && has(self.grpcStatus)) && (has(self.httpStatus) || has(self.grpcStatus))",message="httpStatus and grpcStatus cannot be simultaneously defined."
+type FaultInjectionAbort struct {


also missing omitempty for optional fields

shawnh2 · 2023-12-18T12:06:05Z

tools/make/kube.mk

@@ -30,7 +30,7 @@ CONTROLLERGEN_OBJECT_FLAGS :=  object:headerFile="$(ROOT_DIR)/tools/boilerplate/
 .PHONY: manifests
 manifests: $(tools/controller-gen) generate-gwapi-manifests ## Generate WebhookConfiguration and CustomResourceDefinition objects.
 	@$(LOG_TARGET)
-	$(tools/controller-gen) crd paths="./..." output:crd:artifacts:config=charts/gateway-helm/crds/generated
+	$(tools/controller-gen) crd:allowDangerousTypes=true paths="./..." output:crd:artifacts:config=charts/gateway-helm/crds/generated


why set this to be true?

because of float field

api/v1alpha1/fault_injection.go

arkodg · 2023-12-18T18:45:30Z

internal/infrastructure/kubernetes/proxy/testdata/deployments/enable-coredump.yaml

@@ -0,0 +1,220 @@
+apiVersion: apps/v1


this shouldnt be part of this PR

Done, my bad.

arkodg · 2023-12-18T18:51:51Z

api/v1alpha1/fault_injection.go

+// FaultInjectionAbort defines the abort fault injection configuration
+// +union
+//
+// +kubebuilder:validation:XValidation:rule=" !(has(self.httpStatus) && has(self.grpcStatus)) && (has(self.httpStatus) || has(self.grpcStatus))",message="httpStatus and grpcStatus cannot be simultaneously defined."


should this be !(has(self.httpStatus) && has(self.grpcStatus)) ?

yeah，Add verification that at least one of httpStatus and grpcStatus is set

Signed-off-by: zhaonan <[email protected]>

arkodg · 2023-12-19T22:51:03Z

api/v1alpha1/fault_injection.go

+// +union
+//
+// +kubebuilder:validation:XValidation:rule=" !(has(self.httpStatus) && has(self.grpcStatus)) ",message="httpStatus and grpcStatus cannot be simultaneously defined."
+// +kubebuilder:validation:XValidation:rule=" has(self.httpStatus) || has(self.grpcStatus) ",message="httpStatus and grpcStatus are set at least one."


is L45 needed ?

yeah，an error type must be set(not provide default behavior for abort fault)，https://github.com/envoyproxy/envoy/blob/6b152468de33832161951348aac5e2219935f258/api/envoy/extensions/filters/http/fault/v3/fault.proto#L41C1-L52C4

arkodg

LGTM thanks for introducing this API
adding a minor non blocking comment

Signed-off-by: zhaonan <[email protected]>

shawnh2

lgtm

Xunzhuo

Looks good, thanks

Xunzhuo · 2023-12-20T03:44:39Z

/retest

tmsnan requested a review from a team as a code owner December 15, 2023 06:20

github-actions bot temporarily deployed to pull request December 15, 2023 06:23 Inactive

arkodg reviewed Dec 15, 2023

View reviewed changes

Xunzhuo added the road-to-ga label Dec 18, 2023

zhaonan added 2 commits December 18, 2023 19:47

API: Support FaultInjection in BackendTrafficPolicy

8015f1e

Signed-off-by: zhaonan <[email protected]>

fix api and update cel

c5221fe

Signed-off-by: zhaonan <[email protected]>

tmsnan force-pushed the dev-add-fault-injection branch from c197ba1 to c5221fe Compare December 18, 2023 11:59

github-actions bot temporarily deployed to pull request December 18, 2023 12:02 Inactive

shawnh2 reviewed Dec 18, 2023

View reviewed changes

arkodg reviewed Dec 18, 2023

View reviewed changes

github-actions bot temporarily deployed to pull request December 19, 2023 02:35 Inactive

fix fault injection api

c5fcea9

Signed-off-by: zhaonan <[email protected]>

tmsnan force-pushed the dev-add-fault-injection branch from 661b177 to c5fcea9 Compare December 19, 2023 02:40

Merge branch 'main' into dev-add-fault-injection

efc14de

github-actions bot temporarily deployed to pull request December 19, 2023 02:44 Inactive

arkodg reviewed Dec 19, 2023

View reviewed changes

arkodg previously approved these changes Dec 19, 2023

View reviewed changes

arkodg requested review from a team December 19, 2023 22:51

arkodg requested a review from shawnh2 December 19, 2023 22:51

merge main to solve conflict

7fddc46

Signed-off-by: zhaonan <[email protected]>

zirain previously approved these changes Dec 20, 2023

View reviewed changes

tmsnan dismissed stale reviews from zirain and arkodg via 7fddc46 December 20, 2023 02:51

shawnh2 approved these changes Dec 20, 2023

View reviewed changes

Merge branch 'main' into dev-add-fault-injection

fcd4f79

zirain approved these changes Dec 20, 2023

View reviewed changes

Xunzhuo approved these changes Dec 20, 2023

View reviewed changes

zirain merged commit c2f88f0 into envoyproxy:main Dec 21, 2023
18 checks passed

tmsnan deleted the dev-add-fault-injection branch December 21, 2023 03:17

tmsnan mentioned this pull request Dec 26, 2023

feat(translator): Implement fault injection API #2365

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API: Support FaultInjection in BackendTrafficPolicy #2304

API: Support FaultInjection in BackendTrafficPolicy #2304

tmsnan commented Dec 15, 2023 •

edited

Loading

github-actions bot commented Dec 15, 2023

github-actions bot commented Dec 15, 2023 •

edited

Loading

codecov bot commented Dec 15, 2023 •

edited

Loading

arkodg Dec 15, 2023

arkodg Dec 15, 2023

arkodg Dec 15, 2023

arkodg Dec 15, 2023

arkodg Dec 15, 2023

arkodg Dec 15, 2023

tmsnan Dec 18, 2023

arkodg Dec 15, 2023

arkodg Dec 15, 2023

arkodg Dec 15, 2023

arkodg Dec 16, 2023

tmsnan Dec 18, 2023

shawnh2 Dec 18, 2023

shawnh2 Dec 18, 2023

shawnh2 Dec 18, 2023

tmsnan Dec 19, 2023

shawnh2 Dec 19, 2023

arkodg Dec 18, 2023

tmsnan Dec 19, 2023

arkodg Dec 18, 2023 •

edited

Loading

tmsnan Dec 19, 2023

arkodg Dec 19, 2023

tmsnan Dec 20, 2023

arkodg left a comment

shawnh2 left a comment

Xunzhuo left a comment

Xunzhuo commented Dec 20, 2023

	// FaultInjection defines the fault injection policy to be applied. Support delays and aborts.
	// FaultInjection defines the fault injection policy to be applied. This configuration can be used to
	// inject delays and abort requests to mimic failure scenarios such as service failures and overloads.

	// If specified, the delay will inject a fixed delay into the request
	// If specified, a delay will be injected into the request

	// If specified, the abort will abort the request with the specified HTTP status code
	// If specified, the request will be aborted if it meets the configuration criteria.

	Delay *DelayConfig `json:"delay,omitempty"`
	Delay *FaultInjectionDelay `json:"delay,omitempty"`

	Abort *AbortConfig `json:"abort,omitempty"`
	Abort *FaultInjectionAbort `json:"abort,omitempty"`

API: Support FaultInjection in BackendTrafficPolicy #2304

API: Support FaultInjection in BackendTrafficPolicy #2304

Conversation

tmsnan commented Dec 15, 2023 • edited Loading

github-actions bot commented Dec 15, 2023

github-actions bot commented Dec 15, 2023 • edited Loading

codecov bot commented Dec 15, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arkodg Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arkodg left a comment

Choose a reason for hiding this comment

shawnh2 left a comment

Choose a reason for hiding this comment

Xunzhuo left a comment

Choose a reason for hiding this comment

Xunzhuo commented Dec 20, 2023

tmsnan commented Dec 15, 2023 •

edited

Loading

github-actions bot commented Dec 15, 2023 •

edited

Loading

codecov bot commented Dec 15, 2023 •

edited

Loading

arkodg Dec 18, 2023 •

edited

Loading