Fix bug on downloading job logs (#34041)

Fix #34038

routers/common/actions.go

@@ -22,7 +22,7 @@ func DownloadActionsRunJobLogsWithIndex(ctx *context.Base, ctxRepo *repo_model.R
 	if err = runJobs.LoadRepos(ctx); err != nil {
 		return fmt.Errorf("LoadRepos: %w", err)
 	}
-	if 0 < jobIndex || jobIndex >= int64(len(runJobs)) {
+	if jobIndex < 0 || jobIndex >= int64(len(runJobs)) {
 		return util.NewNotExistErrorf("job index is out of range: %d", jobIndex)
 	}
 	return DownloadActionsRunJobLogs(ctx, ctxRepo, runJobs[jobIndex])

tests/integration/actions_log_test.go

@@ -31,7 +31,7 @@ func TestDownloadTaskLogs(t *testing.T) {
 	testCases := []struct {
 		treePath    string
 		fileContent string
-		outcome     *mockTaskOutcome
+		outcome     []*mockTaskOutcome
 		zstdEnabled bool
 	}{
 		{
@@ -46,21 +46,44 @@ jobs:
       runs-on: ubuntu-latest
       steps:
         - run: echo job1 with zstd enabled
+    job2:
+      runs-on: ubuntu-latest
+      steps:
+        - run: echo job2 with zstd enabled
 `,
-			outcome: &mockTaskOutcome{
+			outcome: []*mockTaskOutcome{
-				result: runnerv1.Result_RESULT_SUCCESS,
+				{
-				logRows: []*runnerv1.LogRow{
+					result: runnerv1.Result_RESULT_SUCCESS,
-					{
+					logRows: []*runnerv1.LogRow{
-						Time:    timestamppb.New(now.Add(1 * time.Second)),
+						{
-						Content: "  \U0001F433  docker create image",
+							Time:    timestamppb.New(now.Add(1 * time.Second)),
+							Content: "  \U0001F433  docker create image",
+						},
+						{
+							Time:    timestamppb.New(now.Add(2 * time.Second)),
+							Content: "job1 zstd enabled",
+						},
+						{
+							Time:    timestamppb.New(now.Add(3 * time.Second)),
+							Content: "\U0001F3C1  Job succeeded",
+						},
 					},
-					{
+				},
-						Time:    timestamppb.New(now.Add(2 * time.Second)),
+				{
-						Content: "job1 zstd enabled",
+					result: runnerv1.Result_RESULT_SUCCESS,
-					},
+					logRows: []*runnerv1.LogRow{
-					{
+						{
-						Time:    timestamppb.New(now.Add(3 * time.Second)),
+							Time:    timestamppb.New(now.Add(1 * time.Second)),
-						Content: "\U0001F3C1  Job succeeded",
+							Content: "  \U0001F433  docker create image",
+						},
+						{
+							Time:    timestamppb.New(now.Add(2 * time.Second)),
+							Content: "job2 zstd enabled",
+						},
+						{
+							Time:    timestamppb.New(now.Add(3 * time.Second)),
+							Content: "\U0001F3C1  Job succeeded",
+						},
 					},
 				},
 			},
@@ -78,21 +101,44 @@ jobs:
       runs-on: ubuntu-latest
       steps:
         - run: echo job1 with zstd disabled
+    job2:
+      runs-on: ubuntu-latest
+      steps:
+        - run: echo job2 with zstd disabled
 `,
-			outcome: &mockTaskOutcome{
+			outcome: []*mockTaskOutcome{
-				result: runnerv1.Result_RESULT_SUCCESS,
+				{
-				logRows: []*runnerv1.LogRow{
+					result: runnerv1.Result_RESULT_SUCCESS,
-					{
+					logRows: []*runnerv1.LogRow{
-						Time:    timestamppb.New(now.Add(4 * time.Second)),
+						{
-						Content: "  \U0001F433  docker create image",
+							Time:    timestamppb.New(now.Add(4 * time.Second)),
+							Content: "  \U0001F433  docker create image",
+						},
+						{
+							Time:    timestamppb.New(now.Add(5 * time.Second)),
+							Content: "job1 zstd disabled",
+						},
+						{
+							Time:    timestamppb.New(now.Add(6 * time.Second)),
+							Content: "\U0001F3C1  Job succeeded",
+						},
 					},
-					{
+				},
-						Time:    timestamppb.New(now.Add(5 * time.Second)),
+				{
-						Content: "job1 zstd disabled",
+					result: runnerv1.Result_RESULT_SUCCESS,
-					},
+					logRows: []*runnerv1.LogRow{
-					{
+						{
-						Time:    timestamppb.New(now.Add(6 * time.Second)),
+							Time:    timestamppb.New(now.Add(4 * time.Second)),
-						Content: "\U0001F3C1  Job succeeded",
+							Content: "  \U0001F433  docker create image",
+						},
+						{
+							Time:    timestamppb.New(now.Add(5 * time.Second)),
+							Content: "job2 zstd disabled",
+						},
+						{
+							Time:    timestamppb.New(now.Add(6 * time.Second)),
+							Content: "\U0001F3C1  Job succeeded",
+						},
 					},
 				},
 			},
@@ -124,54 +170,55 @@ jobs:
 				opts := getWorkflowCreateFileOptions(user2, repo.DefaultBranch, fmt.Sprintf("create %s", tc.treePath), tc.fileContent)
 				createWorkflowFile(t, token, user2.Name, repo.Name, tc.treePath, opts)
 
-				// fetch and execute task
+				// fetch and execute tasks
-				task := runner.fetchTask(t)
+				for jobIndex, outcome := range tc.outcome {
-				runner.execTask(t, task, tc.outcome)
+					task := runner.fetchTask(t)
+					runner.execTask(t, task, outcome)
 
-				// check whether the log file exists
+					// check whether the log file exists
-				logFileName := fmt.Sprintf("%s/%02x/%d.log", repo.FullName(), task.Id%256, task.Id)
+					logFileName := fmt.Sprintf("%s/%02x/%d.log", repo.FullName(), task.Id%256, task.Id)
-				if setting.Actions.LogCompression.IsZstd() {
+					if setting.Actions.LogCompression.IsZstd() {
-					logFileName += ".zst"
+						logFileName += ".zst"
+					}
+					_, err := storage.Actions.Stat(logFileName)
+					assert.NoError(t, err)
+
+					// download task logs and check content
+					runIndex := task.Context.GetFields()["run_number"].GetStringValue()
+					req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/actions/runs/%s/jobs/%d/logs", user2.Name, repo.Name, runIndex, jobIndex)).
+						AddTokenAuth(token)
+					resp := MakeRequest(t, req, http.StatusOK)
+					logTextLines := strings.Split(strings.TrimSpace(resp.Body.String()), "\n")
+					assert.Len(t, logTextLines, len(outcome.logRows))
+					for idx, lr := range outcome.logRows {
+						assert.Equal(
+							t,
+							fmt.Sprintf("%s %s", lr.Time.AsTime().Format("2006-01-02T15:04:05.0000000Z07:00"), lr.Content),
+							logTextLines[idx],
+						)
+					}
+
+					runID, _ := strconv.ParseInt(task.Context.GetFields()["run_id"].GetStringValue(), 10, 64)
+
+					jobs, err := actions_model.GetRunJobsByRunID(t.Context(), runID)
+					assert.NoError(t, err)
+					assert.Len(t, jobs, len(tc.outcome))
+					jobID := jobs[jobIndex].ID
+
+					// download task logs from API and check content
+					req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/actions/jobs/%d/logs", user2.Name, repo.Name, jobID)).
+						AddTokenAuth(token)
+					resp = MakeRequest(t, req, http.StatusOK)
+					logTextLines = strings.Split(strings.TrimSpace(resp.Body.String()), "\n")
+					assert.Len(t, logTextLines, len(outcome.logRows))
+					for idx, lr := range outcome.logRows {
+						assert.Equal(
+							t,
+							fmt.Sprintf("%s %s", lr.Time.AsTime().Format("2006-01-02T15:04:05.0000000Z07:00"), lr.Content),
+							logTextLines[idx],
+						)
+					}
 				}
-				_, err := storage.Actions.Stat(logFileName)
-				assert.NoError(t, err)
-
-				// download task logs and check content
-				runIndex := task.Context.GetFields()["run_number"].GetStringValue()
-				req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/actions/runs/%s/jobs/0/logs", user2.Name, repo.Name, runIndex)).
-					AddTokenAuth(token)
-				resp := MakeRequest(t, req, http.StatusOK)
-				logTextLines := strings.Split(strings.TrimSpace(resp.Body.String()), "\n")
-				assert.Len(t, logTextLines, len(tc.outcome.logRows))
-				for idx, lr := range tc.outcome.logRows {
-					assert.Equal(
-						t,
-						fmt.Sprintf("%s %s", lr.Time.AsTime().Format("2006-01-02T15:04:05.0000000Z07:00"), lr.Content),
-						logTextLines[idx],
-					)
-				}
-
-				runID, _ := strconv.ParseInt(task.Context.GetFields()["run_id"].GetStringValue(), 10, 64)
-
-				jobs, err := actions_model.GetRunJobsByRunID(t.Context(), runID)
-				assert.NoError(t, err)
-				assert.Len(t, jobs, 1)
-				jobID := jobs[0].ID
-
-				// download task logs from API and check content
-				req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/actions/jobs/%d/logs", user2.Name, repo.Name, jobID)).
-					AddTokenAuth(token)
-				resp = MakeRequest(t, req, http.StatusOK)
-				logTextLines = strings.Split(strings.TrimSpace(resp.Body.String()), "\n")
-				assert.Len(t, logTextLines, len(tc.outcome.logRows))
-				for idx, lr := range tc.outcome.logRows {
-					assert.Equal(
-						t,
-						fmt.Sprintf("%s %s", lr.Time.AsTime().Format("2006-01-02T15:04:05.0000000Z07:00"), lr.Content),
-						logTextLines[idx],
-					)
-				}
-
 				resetFunc()
 			})
 		}