diff --git a/cmd/sippy/seed_data.go b/cmd/sippy/seed_data.go index adc6817ed..0f9525195 100644 --- a/cmd/sippy/seed_data.go +++ b/cmd/sippy/seed_data.go @@ -125,7 +125,7 @@ type testCount struct { flake int } -var syntheticReleases = []string{"4.22", "4.21", "4.20", "4.19"} +var syntheticReleases = []string{"4.22", "4.21", "4.20", "4.19", "Presubmits"} var syntheticJobs = []syntheticJobDef{ { @@ -429,6 +429,11 @@ func seedSyntheticData(dbc *db.DB) error { return err } + if err := seedPresubmitData(dbc); err != nil { + return errors.WithMessage(err, "failed to seed presubmit data") + } + log.Info("Seeded presubmit/PR test data") + if err := createLabelsAndSymptoms(dbc); err != nil { return errors.WithMessage(err, "failed to create labels and symptoms") } @@ -770,6 +775,198 @@ func syncRegressions(dbc *db.DB) error { return nil } +func seedPresubmitData(dbc *db.DB) error { + now := time.Now().UTC().Truncate(time.Hour) + + var suite models.Suite + if err := dbc.DB.Where("name = ?", "synthetic").First(&suite).Error; err != nil { + return fmt.Errorf("failed to find suite: %w", err) + } + + // Look up existing test records to reuse + testNames := []string{ + "install should succeed: overall", + "[sig-network] Services should serve endpoints on same port and different protocol", + } + testsByName := map[string]uint{} + for _, name := range testNames { + var t models.Test + if err := dbc.DB.Where("name = ?", name).First(&t).Error; err != nil { + return fmt.Errorf("failed to find test %q: %w", name, err) + } + testsByName[name] = t.ID + } + + // Create presubmit ProwJobs + presubmitJobs := []models.ProwJob{ + { + Kind: models.ProwKind("presubmit"), + Name: "openshift-origin-ci-5.0-e2e-aws-ovn-upgrade", + Release: "Presubmits", + Variants: pq.StringArray{ + "Architecture:amd64", "FeatureSet:default", "Installer:ipi", + "LayeredProduct:none", "Network:ovn", "Platform:aws", + "Suite:unknown", "Topology:ha", "Upgrade:minor", + }, + }, + { + Kind: models.ProwKind("presubmit"), + Name: "openshift-origin-ci-5.0-e2e-gcp-ovn-amd64", + Release: "Presubmits", + Variants: pq.StringArray{ + "Architecture:amd64", "FeatureSet:default", "Installer:ipi", + "LayeredProduct:none", "Network:ovn", "Platform:gcp", + "Suite:parallel", "Topology:ha", "Upgrade:none", + }, + }, + } + + for i, pj := range presubmitJobs { + if err := dbc.DB.Create(&pj).Error; err != nil { + return fmt.Errorf("failed to create presubmit ProwJob %s: %w", pj.Name, err) + } + presubmitJobs[i] = pj + } + + // Create ProwPullRequests + prs := []models.ProwPullRequest{ + { + Org: "openshift", + Repo: "origin", + Number: 99001, + Author: "test-author-1", + Title: "Test PR 99001", + SHA: "abc123def456", + Link: "https://github.com/openshift/origin/pull/99001", + }, + { + Org: "openshift", + Repo: "origin", + Number: 99002, + Author: "test-author-2", + Title: "Test PR 99002", + SHA: "789abc012def", + Link: "https://github.com/openshift/origin/pull/99002", + }, + } + + for i, pr := range prs { + if err := dbc.DB.Create(&pr).Error; err != nil { + return fmt.Errorf("failed to create ProwPullRequest %d: %w", pr.Number, err) + } + prs[i] = pr + } + + // Create runs: 3 runs per job, PR 99001 gets job[0] runs, PR 99002 gets job[1] runs + type runInfo struct { + run models.ProwJobRun + prIdx int + } + var runs []runInfo + + for jobIdx, pj := range presubmitJobs { + for i := 0; i < 3; i++ { + timestamp := now.Add(-time.Duration(3-i) * 20 * time.Hour) + run := models.ProwJobRun{ + ProwJobID: pj.ID, + ProwJobRelease: "Presubmits", + Cluster: "build01", + Timestamp: timestamp, + Duration: 2 * time.Hour, + OverallResult: v1.JobTestFailure, + Failed: true, + } + if err := dbc.DB.Create(&run).Error; err != nil { + return fmt.Errorf("failed to create ProwJobRun: %w", err) + } + runs = append(runs, runInfo{run: run, prIdx: jobIdx}) + } + } + + // Link runs to PRs via join table + for _, ri := range runs { + jrpr := models.ProwJobRunProwPullRequest{ + ProwJobRunID: ri.run.ID, + ProwPullRequestID: prs[ri.prIdx].ID, + ProwJobRunRelease: "Presubmits", + ProwJobRunTimestamp: ri.run.Timestamp, + } + if err := dbc.DB.Create(&jrpr).Error; err != nil { + return fmt.Errorf("failed to create ProwJobRunProwPullRequest: %w", err) + } + } + + // Create test results with mixed statuses + installTestID := testsByName["install should succeed: overall"] + networkTestID := testsByName["[sig-network] Services should serve endpoints on same port and different protocol"] + + for _, ri := range runs { + // Failure result for install test + failResult := models.ProwJobRunTest{ + ProwJobRunID: ri.run.ID, + ProwJobID: ri.run.ProwJobID, + ProwJobRunRelease: "Presubmits", + ProwJobRunTimestamp: ri.run.Timestamp, + TestID: installTestID, + SuiteID: &suite.ID, + Status: 12, + Duration: 5.0, + CreatedAt: ri.run.Timestamp, + } + if err := dbc.DB.Create(&failResult).Error; err != nil { + return fmt.Errorf("failed to create failure ProwJobRunTest: %w", err) + } + + // Add output for the first failure only + if ri.prIdx == 0 && ri.run.Timestamp.Equal(runs[0].run.Timestamp) { + output := models.ProwJobRunTestOutput{ + ProwJobRunTestID: failResult.ID, + Output: "Expected install to succeed but got timeout after 30m", + ProwJobRunTestTimestamp: ri.run.Timestamp, + ProwJobRunTestRelease: "Presubmits", + } + if err := dbc.DB.Create(&output).Error; err != nil { + return fmt.Errorf("failed to create ProwJobRunTestOutput: %w", err) + } + } + + // Success result for install test (same test, different run aspect) + successResult := models.ProwJobRunTest{ + ProwJobRunID: ri.run.ID, + ProwJobID: ri.run.ProwJobID, + ProwJobRunRelease: "Presubmits", + ProwJobRunTimestamp: ri.run.Timestamp, + TestID: networkTestID, + SuiteID: &suite.ID, + Status: 1, + Duration: 3.0, + CreatedAt: ri.run.Timestamp, + } + if err := dbc.DB.Create(&successResult).Error; err != nil { + return fmt.Errorf("failed to create success ProwJobRunTest: %w", err) + } + + // Flake result for network test on a different test + flakeResult := models.ProwJobRunTest{ + ProwJobRunID: ri.run.ID, + ProwJobID: ri.run.ProwJobID, + ProwJobRunRelease: "Presubmits", + ProwJobRunTimestamp: ri.run.Timestamp, + TestID: installTestID, + SuiteID: &suite.ID, + Status: 13, + Duration: 4.0, + CreatedAt: ri.run.Timestamp, + } + if err := dbc.DB.Create(&flakeResult).Error; err != nil { + return fmt.Errorf("failed to create flake ProwJobRunTest: %w", err) + } + } + + log.Infof("Created presubmit seed data: %d jobs, %d PRs, %d runs", len(presubmitJobs), len(prs), len(runs)) + return nil +} + const syntheticViewsFile = "config/seed-views.yaml" // variantMapToArray converts a variant map to a pq.StringArray. diff --git a/pkg/api/prtestresults.go b/pkg/api/prtestresults.go index c9bdb4558..b945b6b29 100644 --- a/pkg/api/prtestresults.go +++ b/pkg/api/prtestresults.go @@ -1,324 +1,142 @@ package api import ( - "context" "fmt" "net/http" "strconv" "time" - "cloud.google.com/go/bigquery" - "cloud.google.com/go/civil" - "github.com/openshift/sippy/pkg/bigquery/bqlabel" - "github.com/pkg/errors" log "github.com/sirupsen/logrus" - "google.golang.org/api/iterator" - bq "github.com/openshift/sippy/pkg/bigquery" + sippyprocessingv1 "github.com/openshift/sippy/pkg/apis/sippyprocessing/v1" + "github.com/openshift/sippy/pkg/db" "github.com/openshift/sippy/pkg/util/param" ) -// PRTestResult represents a test result from a pull request job +// PRTestResult represents a test result from a pull request job run type PRTestResult struct { - ProwJobBuildID string `json:"prowjob_build_id"` - ProwJobName string `json:"prowjob_name"` - ProwJobURL string `json:"prowjob_url"` - PRSha string `json:"pr_sha"` - ProwJobStart time.Time `json:"prowjob_start"` - TestName string `json:"test_name"` - TestSuite string `json:"test_suite"` - Success bool `json:"success"` - Flaked bool `json:"flaked"` - FailureContent string `json:"failure_content"` + ProwJobRunID uint `json:"prow_job_run_id"` + ProwJobName string `json:"prow_job_name"` + ProwJobURL string `json:"prow_job_url"` + PRSha string `json:"pr_sha"` + ProwJobStart time.Time `json:"prow_job_start"` + TestName string `json:"test_name"` + TestSuite string `json:"test_suite"` + Status string `json:"status"` + Output string `json:"output,omitempty"` } -// GetPRTestResults fetches test failures for a specific pull request from BigQuery -// This queries both junit_pr and junit tables: -// - junit_pr: Contains results from normal presubmit jobs -// - junit: Contains results from /payload jobs (manually invoked jobs) -// Note: Only returns test failures (success = false), excluding flakes and passes -// includeSuccesses: Optional list of test name substrings to also include successes for -func GetPRTestResults(ctx context.Context, bqc *bq.Client, org, repo string, prNumber int, startDate, endDate time.Time, includeSuccesses []string) ([]PRTestResult, error) { - log.WithFields(log.Fields{ - "org": org, - "repo": repo, - "pr_number": prNumber, - "start_date": startDate.Format("2006-01-02"), - "end_date": endDate.Format("2006-01-02"), - "include_successes": includeSuccesses, - }).Info("querying test results for pull request") - - // Query junit_pr table (normal presubmit jobs) - log.Infof("querying junit_pr table for org=%s, repo=%s, pr_number=%d, start=%s, end=%s", - org, repo, prNumber, startDate.Format("2006-01-02"), endDate.Format("2006-01-02")) - queryPR := buildPRTestResultsQuery(bqc, org, repo, prNumber, startDate, endDate, "junit_pr", includeSuccesses) - bqc.ApplyQueryLabels(ctx, bqlabel.PRTestResults, queryPR) - resultsPR, err := executePRTestResultsQuery(ctx, queryPR) - if err != nil { - log.WithError(err).Error("error querying junit_pr table") - return nil, errors.Wrap(err, "failed to execute PR test results query for junit_pr table") +func testStatusString(status int) string { + switch sippyprocessingv1.TestStatus(status) { + case sippyprocessingv1.TestStatusSuccess: + return "success" + case sippyprocessingv1.TestStatusFlake: + return "flake" + case sippyprocessingv1.TestStatusFailure: + return "failure" + default: + return fmt.Sprintf("unknown(%d)", status) } - log.Infof("found %d test results from presubmit jobs (junit_pr)", len(resultsPR)) - - // Query junit table (/payload jobs) - log.Infof("querying junit table for org=%s, repo=%s, pr_number=%d, start=%s, end=%s", - org, repo, prNumber, startDate.Format("2006-01-02"), endDate.Format("2006-01-02")) - queryPayload := buildPRTestResultsQuery(bqc, org, repo, prNumber, startDate, endDate, "junit", includeSuccesses) - bqc.ApplyQueryLabels(ctx, bqlabel.PRTestResults, queryPayload) - resultsPayload, err := executePRTestResultsQuery(ctx, queryPayload) - if err != nil { - log.WithError(err).Error("error querying junit table") - return nil, errors.Wrap(err, "failed to execute PR test results query for junit table") - } - log.Infof("found %d test results from /payload jobs (junit)", len(resultsPayload)) - - // Combine results from both tables - allResults := append(resultsPR, resultsPayload...) - log.Infof("found %d total test results for PR %s/%s#%d", len(allResults), org, repo, prNumber) - return allResults, nil } -// buildPRTestResultsQuery constructs the BigQuery query to fetch test results for a PR -// junitTable should be either "junit_pr" (for normal presubmits) or "junit" (for /payload jobs) -// includeSuccesses: Optional list of test name substrings to also include successes for -func buildPRTestResultsQuery(bqc *bq.Client, org, repo string, prNumber int, startDate, endDate time.Time, junitTable string, includeSuccesses []string) *bigquery.Query { - // Query joins jobs and specified junit table, filtering by org/repo/pr_number - // Uses partitioning on prowjob_start and modified_time for efficiency - // Note: junit_pr contains normal presubmit jobs, junit contains /payload jobs - // - // The query de-dupes test results because junit tables model the XML directly, - // which means retried tests appear as multiple rows. We prefer: - // 1. Flakes (flake_count > 0) - test that failed then passed on retry - // 2. Successes (success_val > 0) - test that passed - // 3. Failures (else) - test that failed - - // Build the WHERE clause for including successes - // By default, only include failures (adjusted_flake_count = 0 AND adjusted_success_val = 0) - // If includeSuccesses is specified, also include successes for matching test names - whereClause := ` - AND ( - (deduped.adjusted_flake_count = 0 AND deduped.adjusted_success_val = 0)` +// GetPRTestResults fetches test results for a specific pull request from PostgreSQL. +func GetPRTestResults(dbc *db.DB, org, repo string, prNumber int, latestSHAOnly bool, startDate, endDate time.Time, includeSuccesses []string) ([]PRTestResult, error) { + log.WithFields(log.Fields{ + "org": org, + "repo": repo, + "pr_number": prNumber, + "latest_sha_only": latestSHAOnly, + "start_date": startDate.Format("2006-01-02"), + "end_date": endDate.Format("2006-01-02"), + }).Info("querying test results for pull request") - if len(includeSuccesses) > 0 { - whereClause += ` - OR ( - deduped.adjusted_success_val > 0 - AND (` - for i := range includeSuccesses { - if i > 0 { - whereClause += " OR " - } - whereClause += fmt.Sprintf("deduped.test_name LIKE @IncludeSuccess%d", i) + // Start from the PR side and use partition keys (release, timestamp) on + // prow_job_run_tests to allow PostgreSQL to prune partitions and avoid + // locking every partition in the table. + query := dbc.DB.Table("prow_pull_requests pp"). + Select(`pjr.id AS prow_job_run_id, + pj.name AS prow_job_name, + pjr.url AS prow_job_url, + pp.sha AS pr_sha, + pjr.timestamp AS prow_job_start, + t.name AS test_name, + COALESCE(s.name, '') AS test_suite, + pjrt.status, + COALESCE(pjrto.output, '') AS output`). + Joins("JOIN prow_job_run_prow_pull_requests jrpr ON jrpr.prow_pull_request_id = pp.id"). + Joins("JOIN prow_job_runs pjr ON pjr.id = jrpr.prow_job_run_id"). + Joins("JOIN prow_jobs pj ON pj.id = pjr.prow_job_id AND pj.release = 'Presubmits'"). + Joins("JOIN prow_job_run_tests pjrt ON pjrt.prow_job_run_id = pjr.id AND pjrt.prow_job_run_release = 'Presubmits' AND pjrt.prow_job_run_timestamp >= ? AND pjrt.prow_job_run_timestamp < ?", startDate, endDate). + Joins("JOIN tests t ON t.id = pjrt.test_id"). + Joins("LEFT JOIN suites s ON s.id = pjrt.suite_id"). + Joins("LEFT JOIN prow_job_run_test_outputs pjrto ON pjrto.prow_job_run_test_id = pjrt.id AND pjrto.prow_job_run_test_timestamp = pjrt.prow_job_run_timestamp AND pjrto.prow_job_run_test_release = pjrt.prow_job_run_release"). + Where("pp.org = ? AND pp.repo = ? AND pp.number = ?", org, repo, prNumber). + Where("pjr.timestamp >= ? AND pjr.timestamp < ?", startDate, endDate) + + if latestSHAOnly { + query = query.Where("pp.sha = (SELECT pp2.sha FROM prow_pull_requests pp2 JOIN prow_job_run_prow_pull_requests jrpr2 ON jrpr2.prow_pull_request_id = pp2.id JOIN prow_job_runs pjr2 ON pjr2.id = jrpr2.prow_job_run_id WHERE pp2.org = ? AND pp2.repo = ? AND pp2.number = ? ORDER BY pjr2.timestamp DESC LIMIT 1)", org, repo, prNumber) + } + + // By default only return failures (no flakes, no successes). + // include_successes adds flakes and successes for matching test names. + if len(includeSuccesses) == 0 { + query = query.Where("pjrt.status = ?", int(sippyprocessingv1.TestStatusFailure)) + } else { + conditions := dbc.DB.Where("pjrt.status = ?", int(sippyprocessingv1.TestStatusFailure)) + for _, pattern := range includeSuccesses { + conditions = conditions.Or("t.name LIKE ?", "%"+pattern+"%") } - whereClause += ` - ) - )` - } - whereClause += ` - )` - - queryString := fmt.Sprintf(` - WITH deduped_testcases AS ( - SELECT - junit.*, - ROW_NUMBER() OVER(PARTITION BY prowjob_build_id, file_path, test_name, testsuite ORDER BY - CASE - WHEN flake_count > 0 THEN 0 - WHEN success_val > 0 THEN 1 - ELSE 2 - END) AS row_num, - CASE - WHEN flake_count > 0 THEN 0 - ELSE success_val - END AS adjusted_success_val, - CASE - WHEN flake_count > 0 THEN 1 - ELSE 0 - END AS adjusted_flake_count - FROM - %s.%s AS junit - WHERE - junit.modified_time >= DATETIME(@StartDate) - AND junit.modified_time < DATETIME(@EndDate) - AND junit.skipped = false - ) - SELECT - jobs.prowjob_build_id, - jobs.prowjob_job_name AS prowjob_name, - jobs.prowjob_url, - jobs.pr_sha, - jobs.prowjob_start, - deduped.test_name, - deduped.testsuite, - CASE - WHEN deduped.adjusted_flake_count > 0 THEN TRUE - ELSE FALSE - END AS flaked, - CASE - WHEN deduped.adjusted_flake_count > 0 THEN TRUE - WHEN deduped.adjusted_success_val > 0 THEN TRUE - ELSE FALSE - END AS success, - deduped.failure_content - FROM - %s.jobs AS jobs - INNER JOIN - deduped_testcases AS deduped - ON - jobs.prowjob_build_id = deduped.prowjob_build_id - AND deduped.row_num = 1 - WHERE - jobs.org = @Org - AND jobs.repo = @Repo - AND jobs.pr_number = @PRNumber - AND jobs.prowjob_start >= DATETIME(@StartDate) - AND jobs.prowjob_start < DATETIME(@EndDate)%s - ORDER BY - jobs.prowjob_start DESC, - deduped.test_name ASC - `, bqc.Dataset, junitTable, bqc.Dataset, whereClause) - - query := bqc.BQ.Query(queryString) - query.Parameters = []bigquery.QueryParameter{ - { - Name: "Org", - Value: org, - }, - { - Name: "Repo", - Value: repo, - }, - { - Name: "PRNumber", - Value: strconv.Itoa(prNumber), - }, - { - Name: "StartDate", - Value: startDate, - }, - { - Name: "EndDate", - Value: endDate, - }, - } - - // Add parameters for includeSuccesses LIKE clauses - for i, testName := range includeSuccesses { - query.Parameters = append(query.Parameters, bigquery.QueryParameter{ - Name: fmt.Sprintf("IncludeSuccess%d", i), - Value: "%" + testName + "%", // Wrap in % for SQL LIKE partial matching + query = query.Where(conditions) + } + + query = query.Order("pjr.timestamp DESC, t.name ASC") + + type rawRow struct { + ProwJobRunID uint `gorm:"column:prow_job_run_id"` + ProwJobName string `gorm:"column:prow_job_name"` + ProwJobURL string `gorm:"column:prow_job_url"` + PRSha string `gorm:"column:pr_sha"` + ProwJobStart time.Time `gorm:"column:prow_job_start"` + TestName string `gorm:"column:test_name"` + TestSuite string `gorm:"column:test_suite"` + Status int `gorm:"column:status"` + Output string `gorm:"column:output"` + } + + var rows []rawRow + if err := query.Find(&rows).Error; err != nil { + return nil, fmt.Errorf("error querying PR test results: %w", err) + } + + results := make([]PRTestResult, 0, len(rows)) + for _, r := range rows { + results = append(results, PRTestResult{ + ProwJobRunID: r.ProwJobRunID, + ProwJobName: r.ProwJobName, + ProwJobURL: r.ProwJobURL, + PRSha: r.PRSha, + ProwJobStart: r.ProwJobStart, + TestName: r.TestName, + TestSuite: r.TestSuite, + Status: testStatusString(r.Status), + Output: r.Output, }) } - return query -} - -// executePRTestResultsQuery executes the BigQuery query and returns the results -func executePRTestResultsQuery(ctx context.Context, query *bigquery.Query) ([]PRTestResult, error) { - results := []PRTestResult{} - - it, err := query.Read(ctx) - if err != nil { - return nil, errors.Wrap(err, "error reading from bigquery") - } - - for { - var row []bigquery.Value - err := it.Next(&row) - if err == iterator.Done { - break - } - if err != nil { - return nil, errors.Wrap(err, "error iterating bigquery results") - } - - result, err := deserializePRTestResult(row, it.Schema) - if err != nil { - log.WithError(err).Warn("error deserializing row, skipping") - continue - } - - results = append(results, result) - } - + log.Infof("found %d test results for PR %s/%s#%d", len(results), org, repo, prNumber) return results, nil } -// deserializePRTestResult converts a BigQuery row into a PRTestResult -func deserializePRTestResult(row []bigquery.Value, schema bigquery.Schema) (PRTestResult, error) { - if len(row) != len(schema) { - return PRTestResult{}, fmt.Errorf("row length %d does not match schema length %d", len(row), len(schema)) - } - - result := PRTestResult{} - - for i, field := range schema { - switch field.Name { - case "prowjob_build_id": - if row[i] != nil { - result.ProwJobBuildID = row[i].(string) - } - case "prowjob_name": - if row[i] != nil { - result.ProwJobName = row[i].(string) - } - case "prowjob_url": - if row[i] != nil { - result.ProwJobURL = row[i].(string) - } - case "pr_sha": - if row[i] != nil { - result.PRSha = row[i].(string) - } - case "prowjob_start": - if row[i] != nil { - // BigQuery returns civil.DateTime for DATETIME columns - civilDT := row[i].(civil.DateTime) - layout := "2006-01-02T15:04:05" - parsedTime, err := time.Parse(layout, civilDT.String()) - if err != nil { - return PRTestResult{}, errors.Wrap(err, "failed to parse prowjob_start") - } - result.ProwJobStart = parsedTime - } - case "test_name": - if row[i] != nil { - result.TestName = row[i].(string) - } - case "testsuite": - if row[i] != nil { - result.TestSuite = row[i].(string) - } - case "flaked": - if row[i] != nil { - result.Flaked = row[i].(bool) - } - case "success": - if row[i] != nil { - result.Success = row[i].(bool) - } - case "failure_content": - if row[i] != nil { - result.FailureContent = row[i].(string) - } - } - } - - return result, nil -} - // PrintPRTestResultsJSON is the HTTP handler for /api/pull_requests/test_results -func PrintPRTestResultsJSON(w http.ResponseWriter, req *http.Request, bqc *bq.Client) { - // Parse and validate query parameters +func PrintPRTestResultsJSON(w http.ResponseWriter, req *http.Request, dbc *db.DB) { org := param.SafeRead(req, "org") if org == "" { - // Default to openshift. org = "openshift" } repo := param.SafeRead(req, "repo") if repo == "" { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ "code": http.StatusBadRequest, "message": "required parameter 'repo' is missing", }) @@ -327,7 +145,7 @@ func PrintPRTestResultsJSON(w http.ResponseWriter, req *http.Request, bqc *bq.Cl prNumberStr := param.SafeRead(req, "pr_number") if prNumberStr == "" { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ "code": http.StatusBadRequest, "message": "required parameter 'pr_number' is missing", }) @@ -336,80 +154,66 @@ func PrintPRTestResultsJSON(w http.ResponseWriter, req *http.Request, bqc *bq.Cl prNumber, err := strconv.Atoi(prNumberStr) if err != nil { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ "code": http.StatusBadRequest, "message": fmt.Sprintf("invalid pr_number: %v", err), }) return } - startDateStr := param.SafeRead(req, "start_date") - if startDateStr == "" { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ - "code": http.StatusBadRequest, - "message": "required parameter 'start_date' is missing (format: YYYY-MM-DD)", - }) - return - } + now := time.Now().UTC() + startDate := now.AddDate(0, 0, -14) + endDate := now.AddDate(0, 0, 1) - startDate, err := time.Parse("2006-01-02", startDateStr) - if err != nil { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ - "code": http.StatusBadRequest, - "message": fmt.Sprintf("invalid start_date format (expected YYYY-MM-DD): %v", err), - }) - return + startDateStr := param.SafeRead(req, "start_date") + if startDateStr != "" { + parsed, err := time.Parse("2006-01-02", startDateStr) + if err != nil { + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ + "code": http.StatusBadRequest, + "message": fmt.Sprintf("invalid start_date format (expected YYYY-MM-DD): %v", err), + }) + return + } + startDate = parsed } endDateStr := param.SafeRead(req, "end_date") - if endDateStr == "" { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ - "code": http.StatusBadRequest, - "message": "required parameter 'end_date' is missing (format: YYYY-MM-DD)", - }) - return - } - - endDate, err := time.Parse("2006-01-02", endDateStr) - if err != nil { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ - "code": http.StatusBadRequest, - "message": fmt.Sprintf("invalid end_date format (expected YYYY-MM-DD): %v", err), - }) - return + if endDateStr != "" { + parsed, err := time.Parse("2006-01-02", endDateStr) + if err != nil { + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ + "code": http.StatusBadRequest, + "message": fmt.Sprintf("invalid end_date format (expected YYYY-MM-DD): %v", err), + }) + return + } + // Make end_date inclusive + endDate = parsed.AddDate(0, 0, 1) } - // Validate date range if endDate.Before(startDate) { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ "code": http.StatusBadRequest, "message": "end_date must be after start_date", }) return } - // Add one day to end_date to make it inclusive - endDate = endDate.AddDate(0, 0, 1) - - // Limit date range to 30 days to prevent expensive queries - maxDuration := 30 * 24 * time.Hour - duration := endDate.Sub(startDate) - if duration > maxDuration { - RespondWithJSON(http.StatusBadRequest, w, map[string]interface{}{ + latestSHAOnly, err := param.ReadBool(req, "latest_sha_only", false) + if err != nil { + RespondWithJSON(http.StatusBadRequest, w, map[string]any{ "code": http.StatusBadRequest, - "message": fmt.Sprintf("date range too large: %d days (maximum is 30 days)", int(duration.Hours()/24)), + "message": fmt.Sprintf("invalid latest_sha_only: %v", err), }) return } - - // Parse optional include_successes parameter (multi-valued) includeSuccesses := req.URL.Query()["include_successes"] - // Execute query - results, err := GetPRTestResults(req.Context(), bqc, org, repo, prNumber, startDate, endDate, includeSuccesses) + results, err := GetPRTestResults(dbc, org, repo, prNumber, latestSHAOnly, startDate, endDate, includeSuccesses) if err != nil { log.WithError(err).Error("error fetching PR test results") - RespondWithJSON(http.StatusInternalServerError, w, map[string]interface{}{ + RespondWithJSON(http.StatusInternalServerError, w, map[string]any{ "code": http.StatusInternalServerError, "message": fmt.Sprintf("error fetching test results: %v", err), }) diff --git a/pkg/dataloader/prowloader/bigqueryjobs.go b/pkg/dataloader/prowloader/bigqueryjobs.go index f81817669..4f29957c2 100644 --- a/pkg/dataloader/prowloader/bigqueryjobs.go +++ b/pkg/dataloader/prowloader/bigqueryjobs.go @@ -1,6 +1,7 @@ package prowloader import ( + "fmt" "strconv" "strings" "time" @@ -128,11 +129,56 @@ func (pl *ProwLoader) fetchProwJobsFromOpenShiftBigQuery() ([]prow.ProwJob, []er filteredAnnotations[key] = value } + jobName := bqjr.JobName + jobType := bqjr.Type + + // Detect /payload sub-jobs: they have a releaseJobName annotation and PR data, + // and are run as periodic jobs. Transform them into presubmit-style records so + // they appear on presubmit UI pages. Skip aggregator jobs (they just orchestrate + // sub-jobs and don't have test results themselves). + if releaseJobName, ok := filteredAnnotations["releaseJobName"]; ok && + refs != nil && + !strings.HasPrefix(bqjr.JobName, "aggregator-") { + + // Normalize releaseJobName: the BQ query generator splits on comma + // to strip suffixes, so we must do the same here. + releaseJobName = strings.SplitN(releaseJobName, ",", 2)[0] + filteredAnnotations["releaseJobName"] = releaseJobName + + prNumber := bqjr.PRNumber.StringVal + org := bqjr.PROrg.StringVal + repo := bqjr.PRRepo.StringVal + + // Strip the PR number from the sub-job name to create a stable name + // shared across all PRs running the same canonical job. + // e.g. openshift-origin-31301-ci-5.0-... -> openshift-origin-ci-5.0-... + prPrefix := fmt.Sprintf("%s-%s-%s-", org, repo, prNumber) + stablePrefix := fmt.Sprintf("%s-%s-", org, repo) + stableName := strings.Replace(bqjr.JobName, prPrefix, stablePrefix, 1) + + if stableName == bqjr.JobName { + // PR number not found in job name pattern, use releaseJobName-based fallback + stableName = "payload-pr-" + releaseJobName + log.WithField("job", bqjr.JobName). + WithField("releaseJobName", releaseJobName). + Warningf("could not strip PR number from /payload sub-job name, using fallback") + } + + jobName = stableName + jobType = "presubmit" + + log.WithField("originalName", bqjr.JobName). + WithField("stableName", stableName). + WithField("releaseJobName", releaseJobName). + WithField("prNumber", prNumber). + Debugf("transformed /payload sub-job for presubmit ingestion") + } + prowJobs[bqjr.BuildID] = prow.ProwJob{ Spec: prow.ProwJobSpec{ - Type: bqjr.Type, + Type: jobType, Cluster: bqjr.Cluster, - Job: bqjr.JobName, + Job: jobName, Refs: refs, DecorationConfig: prow.DecorationConfig{ GCSConfiguration: prow.GCSConfiguration{ diff --git a/pkg/dataloader/prowloader/prow.go b/pkg/dataloader/prowloader/prow.go index cd9b9c6c3..9911b45e9 100644 --- a/pkg/dataloader/prowloader/prow.go +++ b/pkg/dataloader/prowloader/prow.go @@ -604,6 +604,20 @@ func (pl *ProwLoader) processProwJob(ctx context.Context, pj *prow.ProwJob) erro "buildID": pj.Status.BuildID, }) + // /payload sub-jobs carry a releaseJobName annotation and PR refs. They've + // already been transformed (name stabilized, type set to presubmit) during + // the BigQuery fetch. Route them straight to the Presubmits pseudorelease. + if _, ok := pj.Annotations["releaseJobName"]; ok && pj.Spec.Refs != nil { + if pl.releaseSet["Presubmits"] { + if err := pl.prowJobToJobRun(ctx, pj, "Presubmits"); err != nil { + err = errors.Wrapf(err, "error converting /payload sub-job to job run: %s", pj.Spec.Job) + pjLog.WithError(err).Warning("prow import error") + return err + } + } + return nil + } + // Synthetic release claims take priority over all other matching. if release, ok := pl.syntheticReleaseJobOverrides.Lookup(pj.Spec.Job); ok { @@ -958,15 +972,43 @@ func (pl *ProwLoader) prowJobToJobRun(ctx context.Context, pj *prow.ProwJob, rel } func (pl *ProwLoader) createOrUpdateProwJob(ctx context.Context, pj *prow.ProwJob, release string, pjLog *log.Entry) (*models.ProwJob, error) { + // For /payload sub-jobs, use the releaseJobName annotation for variant + // identification (the stable name won't match variant patterns, but the + // canonical periodic job name will). Then override the release variant + // to "Presubmits" since the canonical name would produce e.g. "5.0". + variantJobName := pj.Spec.Job + isPayloadPresubmit := false + if rjn, ok := pj.Annotations["releaseJobName"]; ok && pj.Spec.Refs != nil { + variantJobName = rjn + isPayloadPresubmit = true + } + + identifyVariants := func() pq.StringArray { + variants := pl.variantManager.IdentifyVariants(variantJobName) + if isPayloadPresubmit { + for i, v := range variants { + if _, isRelease := pl.config.Releases[v]; isRelease { + variants[i] = "Presubmits" + break + } + } + } + return variants + } + dbProwJob, foundProwJob := pl.prowJobCache[pj.Spec.Job] if !foundProwJob { pjLog.Info("creating new ProwJob") + testGridURL := "" + if !isPayloadPresubmit { + testGridURL = pl.generateTestGridURL(release, pj.Spec.Job).String() + } dbProwJob = &models.ProwJob{ Name: pj.Spec.Job, Kind: models.ProwKind(pj.Spec.Type), Release: release, - Variants: pl.variantManager.IdentifyVariants(pj.Spec.Job), - TestGridURL: pl.generateTestGridURL(release, pj.Spec.Job).String(), + Variants: identifyVariants(), + TestGridURL: testGridURL, } err := pl.dbc.DB.WithContext(ctx).Clauses(clause.OnConflict{UpdateAll: true}).Create(dbProwJob).Error if err != nil { @@ -975,7 +1017,7 @@ func (pl *ProwLoader) createOrUpdateProwJob(ctx context.Context, pj *prow.ProwJo pl.prowJobCache[pj.Spec.Job] = dbProwJob } else { saveDB := false - newVariants := pl.variantManager.IdentifyVariants(pj.Spec.Job) + newVariants := identifyVariants() if !reflect.DeepEqual(newVariants, []string(dbProwJob.Variants)) || dbProwJob.Kind != models.ProwKind(pj.Spec.Type) { dbProwJob.Kind = models.ProwKind(pj.Spec.Type) dbProwJob.Variants = newVariants @@ -985,7 +1027,10 @@ func (pl *ProwLoader) createOrUpdateProwJob(ctx context.Context, pj *prow.ProwJo dbProwJob.Release = release saveDB = true } - if len(dbProwJob.TestGridURL) == 0 { + if isPayloadPresubmit && dbProwJob.TestGridURL != "" { + dbProwJob.TestGridURL = "" + saveDB = true + } else if !isPayloadPresubmit && len(dbProwJob.TestGridURL) == 0 { dbProwJob.TestGridURL = pl.generateTestGridURL(release, pj.Spec.Job).String() if len(dbProwJob.TestGridURL) > 0 { saveDB = true diff --git a/pkg/sippyserver/server.go b/pkg/sippyserver/server.go index 266d756a5..257d46870 100644 --- a/pkg/sippyserver/server.go +++ b/pkg/sippyserver/server.go @@ -1322,11 +1322,7 @@ func (s *Server) jsonPullRequestsReportFromDB(w http.ResponseWriter, req *http.R } func (s *Server) jsonPullRequestTestResults(w http.ResponseWriter, req *http.Request) { - if s.bigQueryClient == nil { - failureResponse(w, http.StatusBadRequest, "pull request test results API is only available when google-service-account-credential-file is configured") - return - } - api.PrintPRTestResultsJSON(w, req, s.bigQueryClient) + api.PrintPRTestResultsJSON(w, req, s.db) } func (s *Server) jsonJobRunSummary(w http.ResponseWriter, req *http.Request) { @@ -2475,13 +2471,11 @@ func (s *Server) Serve() { HandlerFunc: s.jsonPullRequestsReportFromDB, }, { - EndpointPath: "/api/pull_requests/test_results", - Description: "Fetches test failures for a specific pull request from BigQuery (presubmits and /payload jobs). Optional: include_successes param to also return successes for matching test names", - Capabilities: []string{ComponentReadinessCapability}, - HandlerFunc: s.jsonPullRequestTestResults, - CacheTime: 1 * time.Hour, - RateLimitRequests: 20, - RateLimitPeriod: 1 * time.Hour, + EndpointPath: "/api/pull_requests/test_results", + Description: "Fetches test results for a specific pull request from PostgreSQL (presubmit and /payload jobs)", + Capabilities: []string{LocalDBCapability}, + HandlerFunc: s.jsonPullRequestTestResults, + CacheTime: 1 * time.Minute, }, { EndpointPath: "/api/repositories", diff --git a/test/e2e/pull_requests_test.go b/test/e2e/pull_requests_test.go new file mode 100644 index 000000000..95b498feb --- /dev/null +++ b/test/e2e/pull_requests_test.go @@ -0,0 +1,100 @@ +package e2e + +import ( + "testing" + + "github.com/openshift/sippy/pkg/api" + "github.com/openshift/sippy/test/e2e/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func prTestResultsPath(extra string) string { + return "/api/pull_requests/test_results?org=openshift&repo=origin&pr_number=99001" + extra +} + +func TestPRTestResultsDefaultFailuresOnly(t *testing.T) { + var results []api.PRTestResult + err := util.SippyGet(prTestResultsPath(""), &results) + require.NoError(t, err, "error making http request") + require.Greater(t, len(results), 0, "expected at least one result") + + for _, r := range results { + assert.Equal(t, "failure", r.Status, "default query should only return failures, got %s for test %s", r.Status, r.TestName) + } +} + +func TestPRTestResultsIncludeSuccesses(t *testing.T) { + var results []api.PRTestResult + err := util.SippyGet(prTestResultsPath("&include_successes=install"), &results) + require.NoError(t, err, "error making http request") + require.Greater(t, len(results), 0, "expected at least one result") + + statuses := map[string]int{} + for _, r := range results { + statuses[r.Status]++ + } + + assert.Greater(t, statuses["failure"], 0, "should still have failures") + // include_successes=install should match "install should succeed: overall" and return + // success and flake statuses for it alongside the failures + assert.Greater(t, statuses["success"]+statuses["flake"], 0, "should have successes or flakes for matching tests") +} + +func TestPRTestResultsMultiplePRs(t *testing.T) { + var results []api.PRTestResult + err := util.SippyGet("/api/pull_requests/test_results?org=openshift&repo=origin&pr_number=99002", &results) + require.NoError(t, err, "error making http request") + require.Greater(t, len(results), 0, "expected results for PR 99002") + + for _, r := range results { + assert.Contains(t, r.ProwJobName, "gcp", "PR 99002 should be linked to GCP job, got %s", r.ProwJobName) + } +} + +func TestPRTestResultsLatestSHAOnly(t *testing.T) { + var results []api.PRTestResult + err := util.SippyGet(prTestResultsPath("&latest_sha_only=true"), &results) + require.NoError(t, err, "error making http request") + require.Greater(t, len(results), 0, "expected results for latest SHA") + + // All results should share the same SHA (the one from the most recent job run) + sha := results[0].PRSha + for _, r := range results { + assert.Equal(t, sha, r.PRSha, "all results should have the same SHA when latest_sha_only=true") + } +} + +func TestPRTestResultsDefaultDateRange(t *testing.T) { + // Query without start_date/end_date should work (defaults to last 14 days) + var results []api.PRTestResult + err := util.SippyGet(prTestResultsPath(""), &results) + require.NoError(t, err, "error making http request without date params") + assert.Greater(t, len(results), 0, "expected results with default date range") +} + +func TestPRTestResultsMissingParams(t *testing.T) { + tests := []struct { + name string + path string + }{ + {"missing repo", "/api/pull_requests/test_results?org=openshift&pr_number=1"}, + {"missing pr_number", "/api/pull_requests/test_results?org=openshift&repo=origin"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var result any + err := util.SippyGet(tt.path, &result) + assert.Error(t, err, "expected error for %s", tt.name) + assert.Contains(t, err.Error(), "400", "expected 400 status code") + }) + } +} + +func TestPRTestResultsEmptyResults(t *testing.T) { + var results []api.PRTestResult + err := util.SippyGet("/api/pull_requests/test_results?org=openshift&repo=origin&pr_number=99999", &results) + require.NoError(t, err, "error making http request") + assert.Empty(t, results, "expected empty results for non-existent PR") +}