Skip to content

Commit

Permalink
Merge pull request #28961 from sosiouxme/20240729-fix-ra-retries
Browse files Browse the repository at this point in the history
NO-JIRA: riskanalysis: improve request retries
  • Loading branch information
openshift-merge-bot[bot] authored Aug 1, 2024
2 parents 24c4565 + 5bb32d2 commit 04973c8
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 13 deletions.
17 changes: 8 additions & 9 deletions pkg/riskanalysis/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,7 @@ type raRequestLog struct {
// readWriteRiskAnalysis requests Risk Analysis from sippy, writes the results to disk, and returns the RA html to include in prow job output.
// If the request fails, it will try up to maxTries times before returning an error; an error means no RA data returned.
func (opt *Options) readWriteRiskAnalysis(inputBytes []byte) ([]byte, error) {
req, err := http.NewRequest("GET", opt.SippyURL, bytes.NewBuffer(inputBytes))
if err != nil {
logrus.WithError(err).Error("Error creating GET request during risk analysis")
return nil, err
}
req.Header.Set("Content-Type", "application/json")
riskAnalysisBytes, err := opt.requestRiskAnalysis(req, &http.Client{}, &realSleeper{})
riskAnalysisBytes, err := opt.requestRiskAnalysis(inputBytes, &http.Client{}, &realSleeper{})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -179,19 +173,24 @@ func (rs *realSleeper) Sleep(d time.Duration) {
}

// requestRiskAnalysis makes the http request(s) and records the timing and status for each
func (opt *Options) requestRiskAnalysis(req *http.Request, client *http.Client, sleepy sleeper) ([]byte, error) {
func (opt *Options) requestRiskAnalysis(inputBytes []byte, client *http.Client, sleepy sleeper) ([]byte, error) {
var resp *http.Response
var err error
reqLogs := []*raRequestLog{}
var finalReqLog *raRequestLog = nil // keep final log entry to amend before writing if needed
defer opt.writeRARequestLogs(&reqLogs) // write all failures or successes after processing
clientDoSuccess := false
for i := 1; i <= maxTries; i++ {
req, err := http.NewRequest("GET", opt.SippyURL, bytes.NewBuffer(inputBytes))
if err != nil {
logrus.WithError(err).Error("Error creating GET request during risk analysis")
return nil, err
}
req.Header.Set("Content-Type", "application/json")
reqLog := &raRequestLog{RequestCount: i, StartTime: time.Now()}
finalReqLog = reqLog
reqLogs = append(reqLogs, finalReqLog)
ctx, cancelFn := context.WithTimeout(req.Context(), 20*time.Second)
defer cancelFn()

logrus.Infof("Requesting risk analysis (attempt %d/%d) from: %s", i, maxTries, req.RequestURI)
resp, err = client.Do(req.WithContext(ctx))
Expand Down
8 changes: 4 additions & 4 deletions pkg/riskanalysis/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ func TestRequestRiskAnalysisSuccessAfterRetry(t *testing.T) {

tmp := t.TempDir()
opt := &Options{SippyURL: url, JUnitDir: tmp}
req, _ := http.NewRequest("GET", opt.SippyURL, nil)
raContent, err := opt.requestRiskAnalysis(req, client, &mockSleeper{})
// req, _ := http.NewRequest("GET", opt.SippyURL, nil)
raContent, err := opt.requestRiskAnalysis(make([]byte, 0), client, &mockSleeper{})
assert.NoError(t, err, "Failed to read the request content")
assert.Equal(t, reqContent, string(raContent))

Expand Down Expand Up @@ -98,8 +98,8 @@ func TestRequestRiskAnalysisAllRetriesFail(t *testing.T) {

tmp := t.TempDir()
opt := &Options{SippyURL: url, JUnitDir: tmp}
req, _ := http.NewRequest("GET", opt.SippyURL, nil)
_, err := opt.requestRiskAnalysis(req, client, &mockSleeper{})
// req, _ := http.NewRequest("GET", opt.SippyURL, nil)
_, err := opt.requestRiskAnalysis(make([]byte, 0), client, &mockSleeper{})
assert.Error(t, err, "Should fail to request RA content")

// Attempt to load JSON from the expected log file
Expand Down

0 comments on commit 04973c8

Please sign in to comment.