Merge pull request #6603 from whitewindmills/fix-httpserver

Fix the issue of infinite loop caused by connection failure

Author: karmada-bot

pkg/webhook/interpreter/http.go

@@ -97,8 +97,17 @@ func (wh *Webhook) writeResponse(w io.Writer, response Response) {
 // writeResourceInterpreterResponse writes ar to w.
 func (wh *Webhook) writeResourceInterpreterResponse(w io.Writer, interpreterContext configv1alpha1.ResourceInterpreterContext) {
 	if err := json.NewEncoder(w).Encode(interpreterContext); err != nil {
-		klog.Errorf("unable to encode the response: %v", err)
-		wh.writeResponse(w, Errored(http.StatusInternalServerError, err))
+		klog.Errorf("unable to encode and write the response: %v", err)
+		// Since the `ar configv1alpha1.ResourceInterpreterContext` is a clear and legal object,
+		// it should not have problem to be marshalled into bytes.
+		// The error here is probably caused by the abnormal HTTP connection,
+		// e.g., broken pipe, so we can only write the error response once,
+		// to avoid endless circular calling.
+		// More info: https://github.com/kubernetes-sigs/controller-runtime/pull/1930
+		serverError := Errored(http.StatusInternalServerError, err)
+		if err = json.NewEncoder(w).Encode(configv1alpha1.ResourceInterpreterContext{Response: &serverError.ResourceInterpreterResponse}); err != nil {
+			klog.Errorf("still unable to encode and write the InternalServerError response: %v", err)
+		}
 	} else {
 		response := interpreterContext.Response
 		if response.Successful {

pkg/webhook/interpreter/http_test.go

@@ -59,24 +59,13 @@ func (m *mockBody) Close() error {
 	return nil
 }
 
-// limitedBadResponseWriter is a custom io.Writer implementation that simulates
-// write errors for a specified number of attempts. After a certain number of failures,
-// it allows the write operation to succeed.
-type limitedBadResponseWriter struct {
-	failCount   int
-	maxFailures int
+// brokenWriter implements the io.Writer interface.
+type brokenWriter struct {
+	http.ResponseWriter
 }
 
-// Write simulates writing data to the writer. It forces an error response for
-// a limited number of attempts, specified by maxFailures. Once failCount reaches
-// maxFailures, it allows the write to succeed.
-func (b *limitedBadResponseWriter) Write(p []byte) (n int, err error) {
-	if b.failCount < b.maxFailures {
-		b.failCount++
-		return 0, errors.New("forced write error")
-	}
-	// After reaching maxFailures, allow the write to succeed to stop the infinite loop.
-	return len(p), nil
+func (bw *brokenWriter) Write(_ []byte) (int, error) {
+	return 0, fmt.Errorf("mock: write: broken pipe")
 }
 
 func TestServeHTTP(t *testing.T) {
@@ -294,20 +283,14 @@ func TestWriteResourceInterpreterResponse(t *testing.T) {
 			},
 		},
 		{
-			name:        "WriteResourceInterpreterResponse_FailedToWrite_WriterReachedMaxFailures",
+			name:        "should never run into circular calling if the writer has broken",
 			mockHandler: &HTTPMockHandler{},
 			res: configv1alpha1.ResourceInterpreterContext{
 				Response: &configv1alpha1.ResourceInterpreterResponse{},
 			},
-			rec: &limitedBadResponseWriter{maxFailures: 3},
-			verify: func(writer io.Writer, _ *configv1alpha1.ResourceInterpreterResponse) error {
-				data, ok := writer.(*limitedBadResponseWriter)
-				if !ok {
-					return fmt.Errorf("expected writer of type limitedBadResponseWriter but got %T", writer)
-				}
-				if data.failCount != data.maxFailures {
-					return fmt.Errorf("expected %d write failures, got %d", data.maxFailures, data.failCount)
-				}
+			rec: &brokenWriter{},
+			verify: func(_ io.Writer, _ *configv1alpha1.ResourceInterpreterResponse) error {
+				// reaching here means not running into circular calling
 				return nil
 			},
 		},