Cooperate/GitHub chunqi faultinjection #20

spring-young · 2024-01-02T03:44:31Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

feat: FaultInjection #12

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., design docs, usage docs, etc.:

Eikykun

/ptal

Eikykun · 2024-01-02T08:21:14Z

pkg/apis/ctrlmesh/constants/constants.go

 	EnvEnableApiServerCircuitBreaker = "ENABLE_API_SERVER_BREAKER"
 	EnvEnableRestCircuitBreaker      = "ENABLE_REST_BREAKER"
+	EnvEnableRestFaultInjection     = "ENABLE_REST_Fault_Injection"


Env variables need to be capitalized.

Eikykun · 2024-01-02T08:33:12Z

pkg/apis/ctrlmesh/v1alpha1/faultinjection_types.go

@@ -55,41 +57,114 @@ type HTTPFaultInjectionDelay struct {
 type HTTPFaultInjectionAbort struct {
 	// HttpStatus is used to indicate the HTTP status code to
 	// return to the caller.
-	HttpStatus int32 `json:"httpStatus,omitempty"`
+	HttpStatus HTTPSTATUS `json:"httpStatus,omitempty"`


Typo: In word HTTPSTATUS
HTTPSTATUS -> int
we can get http status code from net/http/status.go.

Eikykun · 2024-01-02T08:34:46Z

pkg/apis/ctrlmesh/v1alpha1/faultinjection_types.go

+const (
+	StatusOK                   HTTPSTATUS = 200
+	StatusBadRequest           HTTPSTATUS = 400
+	StatusUnauthorized         HTTPSTATUS = 401
+	StatusForbidden            HTTPSTATUS = 403
+	StatusNotFound             HTTPSTATUS = 404
+	StatusMethodNotAllowed     HTTPSTATUS = 405
+	StatusNotAcceptable        HTTPSTATUS = 406
+	StatusConflict             HTTPSTATUS = 409
+	StatusUnsupportedMediaType HTTPSTATUS = 415
+	StatusUnprocessableEntity  HTTPSTATUS = 422
+	StatusTooManyRequests      HTTPSTATUS = 429
+	StatusInternalServerError  HTTPSTATUS = 500
+	StatusServiceUnavailable   HTTPSTATUS = 503
+	StatusGatewayTimeout       HTTPSTATUS = 504
+)


Unnecessary definitions.

Eikykun · 2024-01-02T08:36:14Z

pkg/apis/ctrlmesh/v1alpha1/faultinjection_types.go

+// RestRule defines the target rest resource of the limiting policy
+type MultiRestRule struct {


modify the comment, plz

Eikykun · 2024-01-02T08:37:05Z

pkg/apis/ctrlmesh/v1alpha1/faultinjection_types.go

+// LimitingSnapshot defines the snapshot of the whole faultinjection policy
+type FaultInjectionSnapshot struct {


modify the comment, plz

Eikykun · 2024-01-02T08:43:04Z

pkg/manager/controllers/circuitbreaker/circuitbreaker_controller_test.go

@@ -72,7 +72,7 @@ var circuitBreaker = &ctrlmeshv1alpha1.CircuitBreaker{
 	Spec: ctrlmeshv1alpha1.CircuitBreakerSpec{
 		Selector: &metav1.LabelSelector{
 			MatchLabels: map[string]string{
-				"test": "test",
+				"test": "test2",


Why edit it?

Eikykun · 2024-01-02T08:46:34Z

pkg/manager/controllers/faultinjection/faultinject_controller.go

+	if resp != nil && resp.Msg != nil && !resp.Msg.Success {
+		return fmt.Errorf("fail to disable pod [%s, %s] circuit breaker config, %s", podIp, name, resp.Msg.Message)
+	}
+	klog.Infof("pod[ip=%s] faultinjection %s was disabled", podIp, name)


"circuit breaker config" -> faultinjection config?

Eikykun · 2024-01-02T09:05:41Z

pkg/proxy/faultinjection/manager.go

+		realEndPointUrl, err := url.Parse(req.Header.Get("Mesh-Real-Endpoint"))
+		logger.Info("receive proxy-host:%s, proxy-method:%s, Mesh-Real-Endpoint:%s", realEndPointUrl.Host, req.Method, req.Header.Get("Mesh-Real-Endpoint"))
+		if err != nil || realEndPointUrl == nil {
+			logger.Error(err, "Request Header Mesh-Real-Endpoint Parse Error")
+			http.Error(w, fmt.Sprintf("Can not find real endpoint in header %s", err), http.StatusInternalServerError)
+			return
+		}
+		// if enableRestFaultInjection {
+		// 	result := pkgfi.ValidateRest(realEndPointUrl.Host, req.Method)
+		// 	if result.Allowed == false {
+		// 		logger.Error("ErrorTProxy: %s %s  ValidateTrafficIntercept NOPASSED ,checkresult:\t%s", realEndPointUrl.Host, req.Method, result.Reason)
+		// 		http.Error(w, fmt.Sprintf("Forbidden by  ValidateTrafficIntercept breaker, %s, %s", result.Message, result.Reason), http.StatusForbidden)
+		// 		return
+		// 	}
+		// }
+
+		// ValidateRest check
+		logger.Info("start ValidateRest checkrule %s %s", realEndPointUrl.Host, req.Method)
+		validateresult := validator.ValidateRest(req.Header.Get("Mesh-Real-Endpoint"), req.Method)
+		if !validateresult.Allowed {
+			logger.Info("ErrorTProxy: %s %s  ValidateRest NOPASSED ,checkresult:%t, validateresultReason:%s", req.Header.Get("Mesh-Real-Endpoint"), req.Method, validateresult.Allowed, validateresult.Reason)
+			// http.Error(w, fmt.Sprintf("Forbidden by circuit ValidateRest breaker, %s, %s", validateresult.Message, validateresult.Reason), http.StatusForbidden)
+			// return
+		}
+		logger.Info("TProxy: %s %s ValidateRest check PASSED", realEndPointUrl.Host, req.Method)
+


withFaultInjection only takes effect for apiserver requests.
donot validate rest here

add http proxy in main.go

Eikykun · 2024-01-02T09:12:20Z

pkg/apis/ctrlmesh/v1alpha1/faultinjection_types.go

+var (
+	FaultInjectionStatusOpened FaultInjectionState = "Opened"
+	FaultInjectionStatusClosed FaultInjectionState = "Closed"


Opend and Closed generally indicate the fuse state. What does this mean for FaultInjection? I don't think FaultInjection needs to have a snapshot state.

A "Fault Injection snapshot" records whether an error code exception has been triggered; it is a snapshot of the occurrence of the exception.

github-actions · 2024-01-03T07:02:22Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

spring-young · 2024-01-03T07:19:00Z

I have read the CLA Document and I hereby sign the CLA

…ection' into cooperate/github-chunqi-faultinjection

codecov · 2024-01-03T08:15:57Z

Codecov Report

Attention: 401 lines in your changes are missing coverage. Please review.

Comparison is base (3b5f3ce) 36.23% compared to head (128a21f) 42.86%.

Files	Patch %	Lines
pkg/proxy/faultinjection/manager.go	46.47%	141 Missing and 11 partials ⚠️
...ntrollers/faultinjection/faultinject_controller.go	67.12%	54 Missing and 17 partials ⚠️
...anager/controllers/faultinjection/event_handler.go	44.70%	42 Missing and 5 partials ⚠️
pkg/manager/controllers/faultinjection/cache.go	40.00%	37 Missing and 2 partials ⚠️
pkg/proxy/faultinjection/store.go	63.88%	36 Missing and 3 partials ⚠️
pkg/proxy/http/http_proxy.go	57.44%	16 Missing and 4 partials ⚠️
pkg/proxy/faultinjection/lease.go	51.72%	13 Missing and 1 partial ⚠️
pkg/proxy/grpcserver/faultinject_handler.go	0.00%	9 Missing ⚠️
...anager/controllers/circuitbreaker/event_handler.go	50.00%	4 Missing and 1 partial ⚠️
pkg/proxy/grpcserver/throttling_handler.go	66.66%	2 Missing and 1 partial ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #20      +/-   ##
==========================================
+ Coverage   36.23%   42.86%   +6.62%     
==========================================
  Files          16       26      +10     
  Lines        1904     2788     +884     
==========================================
+ Hits          690     1195     +505     
- Misses       1147     1485     +338     
- Partials       67      108      +41

Flag	Coverage Δ
unittests	`42.86% <55.54%> (+6.62%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

…ection' into cooperate/github-chunqi-faultinjection

Eikykun

/lgtm

init faultinjection api

f2df7c1

spring-young force-pushed the cooperate/github-chunqi-faultinjection branch from d4d0097 to d30208c Compare January 2, 2024 07:55

Eikykun reviewed Jan 2, 2024

View reviewed changes

spring-young force-pushed the cooperate/github-chunqi-faultinjection branch 2 times, most recently from 9b8db36 to 33e3fbc Compare January 3, 2024 03:22

spring-young added 3 commits January 3, 2024 11:36

feat:add fault injection

0180f30

feat:add fault injection test

9b1061f

feat:add fault injection effective time

4a7aeef

spring-young force-pushed the cooperate/github-chunqi-faultinjection branch from 33e3fbc to 9174b18 Compare January 3, 2024 03:37

fix:delete snapshot

3981a95

spring-young force-pushed the cooperate/github-chunqi-faultinjection branch from 9174b18 to 3981a95 Compare January 3, 2024 06:11

Eikykun changed the base branch from 231206-faultinject to main January 3, 2024 06:52

merge main

03e4da9

Eikykun assigned spring-young Jan 3, 2024

Eikykun added the enhancement New feature or request label Jan 3, 2024

Eikykun linked an issue Jan 3, 2024 that may be closed by this pull request

feat: FaultInjection #12

Closed

spring-young added 3 commits January 3, 2024 16:03

fix:fix CI

8c0ffb6

Merge remote-tracking branch 'origin/cooperate/github-chunqi-faultinj…

08c40d0

…ection' into cooperate/github-chunqi-faultinjection

fix:add license

d38b6d3

Eikykun and others added 8 commits January 3, 2024 17:49

merge main and fix some issue

a22ee74

fix:fix abort

68687b2

Merge remote-tracking branch 'origin/cooperate/github-chunqi-faultinj…

0c8a6f6

…ection' into cooperate/github-chunqi-faultinjection

fix:fix CI

8893e8f

fix: circuitbreaker ut

9c12f0e

fix: faultinjection ut

f09867a

fix: effective time range

14d26fc

issue fix and add some test cases

74546e8

Eikykun and others added 4 commits January 4, 2024 16:58

fix ut

a58a624

fix: faultinjection ui

628e380

fix ut

2e08670

fix ut

128a21f

Eikykun approved these changes Jan 4, 2024

View reviewed changes

Eikykun merged commit 24cb737 into main Jan 4, 2024

github-actions bot locked and limited conversation to collaborators Jan 4, 2024

Eikykun deleted the cooperate/github-chunqi-faultinjection branch January 9, 2024 04:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cooperate/GitHub chunqi faultinjection #20

Cooperate/GitHub chunqi faultinjection #20

spring-young commented Jan 2, 2024 •

edited by Eikykun

Loading

Eikykun left a comment

Eikykun Jan 2, 2024

spring-young Jan 2, 2024

Eikykun Jan 2, 2024

Eikykun Jan 2, 2024

Eikykun Jan 2, 2024

Eikykun Jan 2, 2024

Eikykun Jan 2, 2024

Eikykun Jan 2, 2024

Eikykun Jan 2, 2024

spring-young Jan 2, 2024

Eikykun Jan 2, 2024

spring-young Jan 2, 2024

github-actions bot commented Jan 3, 2024 •

edited

Loading

spring-young commented Jan 3, 2024

codecov bot commented Jan 3, 2024 •

edited

Loading

Eikykun left a comment

		// RestRule defines the target rest resource of the limiting policy
		type MultiRestRule struct {

		// LimitingSnapshot defines the snapshot of the whole faultinjection policy
		type FaultInjectionSnapshot struct {

Cooperate/GitHub chunqi faultinjection #20

Cooperate/GitHub chunqi faultinjection #20

Conversation

spring-young commented Jan 2, 2024 • edited by Eikykun Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., design docs, usage docs, etc.:

Eikykun left a 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

github-actions bot commented Jan 3, 2024 • edited Loading

spring-young commented Jan 3, 2024

codecov bot commented Jan 3, 2024 • edited Loading

Codecov Report

Eikykun left a comment

Choose a reason for hiding this comment

spring-young commented Jan 2, 2024 •

edited by Eikykun

Loading

github-actions bot commented Jan 3, 2024 •

edited

Loading

codecov bot commented Jan 3, 2024 •

edited

Loading