Handle and retry 'connection reset by peer' errors. (#746)

This commit is contained in:
Jean-Rémy Bancel 2019-10-04 14:58:08 -07:00 committed by Knative Prow Robot
parent 9353a99265
commit c3d4edcf0c
3 changed files with 55 additions and 24 deletions

View File

@ -40,13 +40,14 @@ func isDNSError(err error) bool {
return strings.Contains(msg, "no such host") || strings.Contains(msg, ":53")
}
func isTCPConnectRefuse(err error) bool {
func isConnectionRefused(err error) bool {
// The alternative for the string check is:
// errNo := (((err.(*url.Error)).Err.(*net.OpError)).Err.(*os.SyscallError).Err).(syscall.Errno)
// if errNo == syscall.Errno(0x6f) {...}
// But with assertions, of course.
if err != nil && strings.Contains(err.Error(), "connect: connection refused") {
return true
}
return false
return err != nil && strings.Contains(err.Error(), "connect: connection refused")
}
func isConnectionReset(err error) bool {
return err != nil && strings.Contains(err.Error(), "connection reset by peer")
}

View File

@ -19,6 +19,7 @@ limitations under the License.
package spoof
import (
"errors"
"net/http"
"testing"
)
@ -57,31 +58,57 @@ func TestDNSError(t *testing.T) {
}
}
func TestTCPConnectRefuse(t *testing.T) {
func TestConnectionRefused(t *testing.T) {
client := &http.Client{}
for _, tt := range []struct {
name string
url string
tcpRefuse bool
name string
url string
connRefused bool
}{{
name: "nothing listening",
url: "http://localhost:60001",
tcpRefuse: true,
name: "nothing listening",
url: "http://localhost:60001",
connRefused: true,
}, {
name: "dns error",
url: "http://this.url.does.not.exist",
tcpRefuse: false,
name: "dns error",
url: "http://this.url.does.not.exist",
connRefused: false,
}, {
name: "google.com",
url: "https://google.com",
tcpRefuse: false,
name: "google.com",
url: "https://google.com",
connRefused: false,
}} {
t.Run(tt.name, func(t *testing.T) {
req, _ := http.NewRequest("GET", tt.url, nil)
_, err := client.Do(req)
if tcpRefuse := isTCPConnectRefuse(err); tt.tcpRefuse != tcpRefuse {
t.Errorf("Expected tcpRefuse=%v, got %v", tt.tcpRefuse, tcpRefuse)
if connRefused := isConnectionRefused(err); tt.connRefused != connRefused {
t.Errorf("Expected connRefused=%v, got %v", tt.connRefused, connRefused)
}
})
}
}
func TestConnectionReset(t *testing.T) {
for _, tt := range []struct {
name string
err error
connReset bool
}{{
name: "error matching",
err: errors.New("read tcp 10.60.2.57:47882->104.154.144.94:80: read: connection reset by peer"),
connReset: true,
}, {
name: "error not matching",
err: errors.New("dial tcp: lookup this.url.does.not.exist on 127.0.0.1:53: no such host"),
connReset: false,
}, {
name: "nil error",
err: nil,
connReset: false,
}} {
t.Run(tt.name, func(t *testing.T) {
if connReset := isConnectionReset(tt.err); tt.connReset != connReset {
t.Errorf("Expected connReset=%v, got %v", tt.connReset, connReset)
}
})
}

View File

@ -214,19 +214,22 @@ func (sc *SpoofingClient) Poll(req *http.Request, inState ResponseChecker) (*Res
resp, err = sc.Do(req)
if err != nil {
if isTCPTimeout(err) {
sc.Logf("Retrying %s for TCP timeout %v", req.URL.String(), err)
sc.Logf("Retrying %s for TCP timeout: %v", req.URL, err)
return false, nil
}
// Retrying on DNS error, since we may be using xip.io or nip.io in tests.
if isDNSError(err) {
sc.Logf("Retrying %s for DNS error %v", req.URL.String(), err)
sc.Logf("Retrying %s for DNS error: %v", req.URL, err)
return false, nil
}
// Repeat the poll on `connection refused` errors, which are usually transient Istio errors.
if isTCPConnectRefuse(err) {
sc.Logf("Retrying %s for connection refused %v", req.URL.String(), err)
if isConnectionRefused(err) {
sc.Logf("Retrying %s for connection refused: %v", req.URL, err)
return false, nil
}
if isConnectionReset(err) {
sc.Logf("Retrying %s for connection reset: %v", req.URL, err)
}
return true, err
}