From 1183f73656fb8a663d0173bd19bb239704036f3e Mon Sep 17 00:00:00 2001 From: Anna Veretennykova Date: Fri, 11 Sep 2026 13:57:19 +0300 Subject: [PATCH] fix: retry manifest reads from URLs on transient network errors (#4194) --- pkg/files/reader.go | 28 +++++++++++++---- pkg/files/reader_test.go | 65 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/pkg/files/reader.go b/pkg/files/reader.go index 9051ba1636e0..4e41830bd202 100644 --- a/pkg/files/reader.go +++ b/pkg/files/reader.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "github.com/aws/eks-anywhere/pkg/retrier" "golang.org/x/net/http/httpproxy" ) @@ -24,6 +25,7 @@ type Reader struct { embedFS embed.FS httpClient *http.Client userAgent string + retrier *retrier.Retrier } type ReaderOpt func(*Reader) @@ -47,6 +49,14 @@ func WithEKSAUserAgent(eksAComponent, version string) ReaderOpt { return WithUserAgent(eksaUserAgent(eksAComponent, version)) } +// WithRetrier allows to use a custom retrier for the http GET requests +// performed when reading a file from a url. This is only for testing. +func WithRetrier(retrier *retrier.Retrier) ReaderOpt { + return func(r *Reader) { + r.retrier = retrier + } +} + // WithRootCACerts configures the HTTP client's trusted CAs. Note that this will overwrite // the defaults so the host's trust will be ignored. This option is only for testing. func WithRootCACerts(certs []*x509.Certificate) ReaderOpt { @@ -99,6 +109,7 @@ func NewReader(opts ...ReaderOpt) *Reader { embedFS: embedFS, httpClient: client, userAgent: eksaUserAgent("unknown", "no-version"), + retrier: retrier.NewWithMaxRetries(5, 5*time.Second), } for _, o := range opts { @@ -131,13 +142,18 @@ func (r *Reader) readHttpFile(uri string) ([]byte, error) { } request.Header.Set("User-Agent", r.userAgent) - resp, err := r.httpClient.Do(request) - if err != nil { - return nil, fmt.Errorf("failed reading file from url [%s]: %v", uri, err) - } - defer resp.Body.Close() - data, err := io.ReadAll(resp.Body) + var data []byte + err = r.retrier.Retry(func() error { + resp, err := r.httpClient.Do(request) + if err != nil { + return err + } + defer resp.Body.Close() + + data, err = io.ReadAll(resp.Body) + return err + }) if err != nil { return nil, fmt.Errorf("failed reading file from url [%s]: %v", uri, err) } diff --git a/pkg/files/reader_test.go b/pkg/files/reader_test.go index d5103df3246a..a489d787d75a 100644 --- a/pkg/files/reader_test.go +++ b/pkg/files/reader_test.go @@ -11,6 +11,7 @@ import ( "net/url" "os" "sync" + "sync/atomic" "testing" "time" @@ -18,6 +19,7 @@ import ( "github.com/aws/eks-anywhere/internal/test" "github.com/aws/eks-anywhere/pkg/files" + "github.com/aws/eks-anywhere/pkg/retrier" ) //go:embed testdata @@ -95,6 +97,69 @@ func TestReaderReadFileHTTPSSuccess(t *testing.T) { test.AssertContentToFile(t, string(got), filePath) } +func TestReaderReadFileHTTPSRetriesOnTransientError(t *testing.T) { + g := NewWithT(t) + filePath := "testdata/file.yaml" + + var attempts int32 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if atomic.AddInt32(&attempts, 1) <= 2 { + // Simulate a transient network error by closing the connection + // before a response is sent back to the client. + hj, ok := w.(http.Hijacker) + g.Expect(ok).To(BeTrue()) + conn, _, err := hj.Hijack() + g.Expect(err).To(BeNil()) + conn.Close() + return + } + + fileContent, err := os.ReadFile(filePath) + g.Expect(err).To(BeNil()) + if _, err := w.Write(fileContent); err != nil { + t.Errorf("Failed writing response to http request: %s", err) + } + })) + t.Cleanup(func() { server.Close() }) + + uri := server.URL + "/" + filePath + + r := files.NewReader( + files.WithRootCACerts(serverCerts(g, server)), + files.WithRetrier(retrier.NewWithMaxRetries(5, 0)), + ) + got, err := r.ReadFile(uri) + g.Expect(err).To(BeNil()) + test.AssertContentToFile(t, string(got), filePath) + g.Expect(atomic.LoadInt32(&attempts)).To(Equal(int32(3))) +} + +func TestReaderReadFileHTTPSFailsAfterExhaustingRetries(t *testing.T) { + g := NewWithT(t) + filePath := "testdata/file.yaml" + + var attempts int32 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&attempts, 1) + hj, ok := w.(http.Hijacker) + g.Expect(ok).To(BeTrue()) + conn, _, err := hj.Hijack() + g.Expect(err).To(BeNil()) + conn.Close() + })) + t.Cleanup(func() { server.Close() }) + + uri := server.URL + "/" + filePath + + r := files.NewReader( + files.WithRootCACerts(serverCerts(g, server)), + files.WithRetrier(retrier.NewWithMaxRetries(3, 0)), + ) + _, err := r.ReadFile(uri) + g.Expect(err).NotTo(BeNil()) + g.Expect(atomic.LoadInt32(&attempts)).To(Equal(int32(3))) +} + func TestReaderReadFileHTTPSProxySuccess(t *testing.T) { t.Skip("Flaky (https://github.com/aws/eks-anywhere/issues/5775)")