Repository: kubernetes-sigs/prow

Issue:
Prow does not re-trigger complex workflows consisting of muliple workflows
This is essentially a continuation of https://github.com/kubernetes-sigs/prow/issues/276.
We still see workflows that are not properly re-triggered by prow.
I have a theory on this. Prow looks for [runs with status=failure](https://github.com/kubernetes-sigs/prow/blob/9af35b74cda4122cd97ea5d4be4e689f5855609d/pkg/github/client.go#L2058), but our "run" consists of multiple workflows. Here is what I found from curling the API and checking specifically a run that had failed:

```json
{
  "id": 13359118667,
  "name": "E2E Test pipeline",
  "node_id": "WFR_kwLOCfWxyM8AAAADHEP5Sw",
  "head_branch": "dependabot/go_modules/main/kubernetes-52f6fc2930",
  "head_sha": "3f245d6ad3fbe09b1431ae4b0ae9f2fc8a4ed458",
  "path": ".github/workflows/pipeline.yml",
  "display_title": "🌱 Bump the kubernetes group to v0.31.6",
  "run_number": 1677,
  "event": "pull_request",
  "status": "completed",
  "conclusion": "cancelled",
  ...
  "jobs_url": "https://api.github.com/repos/metal3-io/baremetal-operator/actions/runs/13359118667/jobs",
```

So the status is "completed" with conclusion "cancelled". This would not be picked up as a failed run by prow.
Note that there is a link to the jobs that are part of the run! From this it is possible to get the individual jobs and their status/conclusion.
For example:

```json
  {
    "id": 37306075829,
    "run_id": 13359118667,
    "workflow_name": "E2E Test pipeline",
    "head_branch": "dependabot/go_modules/main/kubernetes-52f6fc2930",
    "run_url": "https://api.github.com/repos/metal3-io/baremetal-operator/actions/runs/13359118667",
    "run_attempt": 1,
    "node_id": "CR_kwDOCfWxyM8AAAAIr52KtQ",
    "head_sha": "3f245d6ad3fbe09b1431ae4b0ae9f2fc8a4ed458",
    "url": "https://api.github.com/repos/metal3-io/baremetal-operator/actions/jobs/37306075829",
    "html_url": "https://github.com/metal3-io/baremetal-operator/actions/runs/13359118667/job/37306075829",
    "status": "completed",
    "conclusion": "failure",
    "created_at": "2025-02-16T21:53:30Z",
    "started_at": "2025-02-16T21:54:41Z",
    "completed_at": "2025-02-16T22:19:44Z",
    "name": "e2e-test (redfish-virtualmedia) / E2E test",
    ...
```

The status here is also "completed" but here we can see that the conclusion is failed. This would be picked up as failed by Prow.

TL;DR: Prow needs to re-trigger any workflow that has status == `completed` AND conclusion != `success`


Discussion:

Solution patch:
diff --git a/pkg/github/client.go b/pkg/github/client.go
index 5115f3d5d..1197d8715 100644
--- a/pkg/github/client.go
+++ b/pkg/github/client.go
@@ -2055,7 +2055,8 @@ func (c *client) GetFailedActionRunsByHeadBranch(org, repo, branchName, headSHA
 		Path: fmt.Sprintf("/repos/%s/%s/actions/runs", org, repo),
 	}
 	query := u.Query()
-	query.Add("status", "failure")
+	// Filter for the specific head SHA
+	query.Add("head_sha", headSHA)
 	// setting the OR condition to get both PR and PR target workflows, as well
 	// as workflows called via another workflow using workflow_call (matrix workflows)
 	query.Add("event", "pull_request OR pull_request_target OR workflow_call")
@@ -2072,9 +2073,15 @@ func (c *client) GetFailedActionRunsByHeadBranch(org, repo, branchName, headSHA
 
 	prRuns := []WorkflowRun{}
 
-	// keep only the runs matching the current PR headSHA
+	// We only want to get failed workflows.
+	// Note: The query parameter "status" is overloaded and used for both status and conclusion.
+	// See https://docs.github.com/en/rest/actions/workflow-runs?apiVersion=2022-11-28#list-workflow-runs-for-a-workflow
+	// This makes it hard to use directly. Instead, we loop through the runs and check them individually.
+	// A successful workflow will have status "completed" and conclusion "success".
+	// A failed workflow also have status "completed", but the conclusion can be either "failure" or "cancelled".
+	// We only want completed jobs that are not successful.
 	for _, run := range runs.WorkflowRuns {
-		if run.HeadSha == headSHA {
+		if run.Status == "completed" && run.Conclusion != "success" {
 			prRuns = append(prRuns, run)
 		}
 	}



Test patch:
diff --git a/pkg/github/client_test.go b/pkg/github/client_test.go
index 760c0770b..8a6314e54 100644
--- a/pkg/github/client_test.go
+++ b/pkg/github/client_test.go
@@ -381,6 +381,116 @@ func TestGetPullRequest(t *testing.T) {
 	}
 }
 
+func TestGetFailedActionRunsByHeadBranch(t *testing.T) {
+	const (
+		org     = "k8s"
+		repo    = "kuber"
+		branch  = "main"
+		headSHA = "123abc"
+	)
+	var (
+		failedRun       = WorkflowRun{HeadSha: headSHA, Status: "completed", Conclusion: "failure"}
+		successfulRun   = WorkflowRun{HeadSha: headSHA, Status: "completed", Conclusion: "success"}
+		secondFailedRun = WorkflowRun{HeadSha: headSHA, Status: "completed", Conclusion: "failure"}
+		cancelledRun    = WorkflowRun{HeadSha: headSHA, Status: "completed", Conclusion: "cancelled"}
+	)
+	testCases := []struct {
+		name string
+		// queryResponse is the response from the GitHub API
+		queryResponse WorkflowRuns
+		// expectedRuns is the expected result after filtering the queryResponse
+		expectedRuns []WorkflowRun
+	}{
+		{
+			name: "single matching run",
+			queryResponse: WorkflowRuns{
+				WorkflowRuns: []WorkflowRun{failedRun},
+			},
+			expectedRuns: []WorkflowRun{failedRun},
+		},
+		{
+			name: "multiple runs, one matching",
+			queryResponse: WorkflowRuns{
+				WorkflowRuns: []WorkflowRun{failedRun, successfulRun},
+			},
+			expectedRuns: []WorkflowRun{failedRun},
+		},
+		{
+			name: "multiple runs, two matching",
+			queryResponse: WorkflowRuns{
+				WorkflowRuns: []WorkflowRun{failedRun, successfulRun, secondFailedRun},
+			},
+			expectedRuns: []WorkflowRun{failedRun, secondFailedRun},
+		},
+		{
+			name: "cancelled run",
+			queryResponse: WorkflowRuns{
+				WorkflowRuns: []WorkflowRun{cancelledRun},
+			},
+			expectedRuns: []WorkflowRun{cancelledRun},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				if r.Method != http.MethodGet {
+					t.Errorf("Expected GET method, got %s", r.Method)
+				}
+				expectedPath := fmt.Sprintf("/repos/%s/%s/actions/runs", org, repo)
+				if r.URL.Path != expectedPath {
+					t.Errorf("Expected path %s, got %s", expectedPath, r.URL.Path)
+				}
+
+				// Check query parameters
+				query := r.URL.Query()
+				if query.Get("head_sha") != headSHA {
+					t.Errorf("Expected query parameter head_sha=%s, got %s", headSHA, query.Get("head_sha"))
+				}
+				expectedEvent := "pull_request OR pull_request_target OR workflow_call"
+				if query.Get("event") != "pull_request OR pull_request_target OR workflow_call" {
+					t.Errorf("Expected query parameter event=%q, got %q", expectedEvent, query.Get("event"))
+				}
+				if query.Get("branch") != branch {
+					t.Errorf("Expected query parameter branch=%s, got %s", branch, query.Get("branch"))
+				}
+
+				// Prepare a response based on the test case
+				b, err := json.Marshal(&tc.queryResponse)
+				if err != nil {
+					t.Fatalf("Unexpected error marshalling JSON: %v", err)
+				}
+				w.Header().Set("Content-Type", "application/json")
+				fmt.Fprint(w, string(b))
+			}))
+			defer ts.Close()
+
+			c := getClient(ts.URL)
+			runs, err := c.GetFailedActionRunsByHeadBranch(org, repo, branch, headSHA)
+			if err != nil {
+				t.Errorf("Did not expect error, got %v", err)
+			}
+
+			// Check if the returned runs match the expected runs
+			if len(runs) != len(tc.expectedRuns) {
+				t.Errorf("Expected %d runs, got %d", len(tc.expectedRuns), len(runs))
+			}
+			for _, run := range runs {
+				found := false
+				for _, expectedRun := range tc.expectedRuns {
+					if run.Status == expectedRun.Status && run.Conclusion == expectedRun.Conclusion && run.HeadSha == expectedRun.HeadSha {
+						found = true
+					}
+				}
+				if !found {
+					t.Errorf("Run %v not found in expected result %v", run, tc.expectedRuns)
+				}
+			}
+		})
+	}
+
+}
+
 func TestGetPullRequestChanges(t *testing.T) {
 	ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodGet {