Skip to content

Commit 872c4af

Browse files
committed
feat(STONEINTG-1677): consolidated Gitlab status reporting
- add HTTP 422 as unrecoverable across all three forge reporters - add ReportConsolidatedStatus to reporter.go GitLab posts a single "X/Y passed" status when scenario count exceeds the per-pipeline job threshold; GitHub and Forgejo are no-ops - propagate IsOptional through TestReport so optional failures do not set the consolidated status to failed - add threshold config via GITLAB_MAX_INDIVIDUAL_STATUSES env var (default 50) with adapter routing logic - unit tests for consolidated status, transition error suppression, and threshold routing Assisted by: Cursor AI Signed-off-by: jcullina <jcullina@redhat.com>
1 parent 3e5f7eb commit 872c4af

9 files changed

Lines changed: 456 additions & 46 deletions

File tree

internal/controller/statusreport/statusreport_adapter.go

Lines changed: 60 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -525,23 +525,26 @@ func (a *Adapter) iterateIntegrationTestStatusDetailsInStatusReport(reporter sta
525525
return nil
526526
}
527527

528+
// Build the per-scenario test reports. These are always needed — for comment
529+
// text in both paths and for per-scenario commit statuses in the individual path.
530+
var allReports []status.TestReport
528531
for _, integrationTestStatusDetail := range integrationTestStatusDetails {
529-
// set isFinalStatus to true if there is at least one integration test status is in final status, which means the comment for integration test might not be updated again to have only one comment for each component
530532
if integrationTestStatusDetail.Status.IsFinal() {
531533
isFinalStatus = true
532534
}
533535
testReport, reportErr := status.GenerateTestReport(a.context, a.client, *integrationTestStatusDetail, testedSnapshot, componentNameOrPrGroup)
534536
if reportErr != nil {
535537
a.logger.Error(reportErr, fmt.Sprintf("failed to generate test report for integration test scenario %s/%s",
536538
testedSnapshot.Namespace, integrationTestStatusDetail.ScenarioName))
537-
if writeErr := status.WriteSnapshotReportStatus(a.context, a.client, testedSnapshot, srs); writeErr != nil { // try to write what was already written
539+
if writeErr := status.WriteSnapshotReportStatus(a.context, a.client, testedSnapshot, srs); writeErr != nil {
538540
return fmt.Errorf("failed to generate test report AND write snapshot report status metadata: %w", e.Join(reportErr, writeErr))
539541
}
540542
return fmt.Errorf("failed to generate test report: %w", reportErr)
541543
}
544+
allReports = append(allReports, *testReport)
545+
542546
// split passed and failing integration test report in gitlab comment to show failing tests on top
543547
if integrationTestStatusDetail.Status == intgteststat.IntegrationTestStatusTestPassed {
544-
// generate comment for passed integration test with short text which has pipelinerun link but without task run details
545548
commentForEachTest, err := status.FormatCommentForSuccessfulTest(testReport.Summary, testReport.ShortText)
546549
if err != nil {
547550
return fmt.Errorf("failed to generate comment for status of integration test scenario %s/%s : %w", testedSnapshot.Namespace, testReport.ScenarioName, err)
@@ -554,38 +557,70 @@ func (a *Adapter) iterateIntegrationTestStatusDetailsInStatusReport(reporter sta
554557
}
555558
commentForFailingIntegrationTests = append(commentForFailingIntegrationTests, commentForEachTest)
556559
}
560+
}
557561

558-
if srs.IsNewer(integrationTestStatusDetail.ScenarioName, destinationSnapshot.Name, integrationTestStatusDetail.LastUpdateTime) {
559-
a.logger.Info("Integration Test contains new status updates", "scenario.Name", integrationTestStatusDetail.ScenarioName, "destinationSnapshot.Name", destinationSnapshot.Name, "testedSnapshot", testedSnapshot.Name)
560-
561-
} else {
562-
//integration test contains no changes
563-
a.logger.Info("Integration Test doen't contain new status updates", "scenario.Name", integrationTestStatusDetail.ScenarioName)
564-
continue
565-
}
562+
// When the number of scenarios exceeds the configured threshold for GitLab or
563+
// Forgejo reporters, post a single consolidated commit status instead of one
564+
// per scenario to avoid exhausting the provider's per-pipeline job limit.
565+
useConsolidated := reporter.GetReporterName() == status.GitLabProvider &&
566+
len(integrationTestStatusDetails) > status.MaxIndividualStatuses
566567

567-
if statusCode, reportStatusErr := reporter.ReportStatus(a.context, *testReport); reportStatusErr != nil {
568-
if integrationTestStatusDetail.Status.IsFinal() && integrationTestStatusDetail.TestPipelineRunName != "" {
569-
a.logger.Error(reportStatusErr, fmt.Sprintf("failed to report status to git provider for completed integration pipelinerun %s/%s, then finalizer test.appstudio.openshift.io/pipelinerun might not be removed from it later", testedSnapshot.Namespace, integrationTestStatusDetail.TestPipelineRunName))
570-
}
568+
if useConsolidated {
569+
a.logger.Info("Scenario count exceeds threshold, using consolidated commit status",
570+
"scenarioCount", len(integrationTestStatusDetails), "threshold", status.MaxIndividualStatuses,
571+
"reporter", reporter.GetReporterName())
571572

573+
if statusCode, reportErr := reporter.ReportConsolidatedStatus(a.context, allReports); reportErr != nil {
572574
if reporter.ReturnCodeIsUnrecoverable(statusCode) {
573-
a.logger.Error(reportStatusErr, fmt.Sprintf("failed to report status to git provider for integration pipelinerun %s/%s, the statusCode %d is not easily recoverable", testedSnapshot.Namespace, integrationTestStatusDetail.TestPipelineRunName, statusCode))
575+
a.logger.Error(reportErr, "consolidated status report failed with unrecoverable error, skipping",
576+
"statusCode", statusCode)
574577
return nil
575-
} else {
576-
return fmt.Errorf("failed to update status: %w", reportStatusErr)
577578
}
579+
return fmt.Errorf("failed to report consolidated status: %w", reportErr)
578580
}
579581

580-
if writeErr := status.WriteSnapshotReportStatus(a.context, a.client, testedSnapshot, srs); writeErr != nil { // try to write what was already written
581-
return fmt.Errorf("failed to report status AND write snapshot report status metadata: %w", writeErr)
582+
// Mark all scenarios as reported so they are not re-reported on the next reconcile
583+
for _, detail := range integrationTestStatusDetails {
584+
srs.SetLastUpdateTime(detail.ScenarioName, destinationSnapshot.Name, detail.LastUpdateTime)
585+
}
586+
if writeErr := status.WriteSnapshotReportStatus(a.context, a.client, testedSnapshot, srs); writeErr != nil {
587+
return fmt.Errorf("failed to write snapshot report status after consolidated status: %w", writeErr)
582588
}
589+
} else {
590+
// Per-scenario path: post one commit status per scenario.
591+
for i, integrationTestStatusDetail := range integrationTestStatusDetails {
592+
testReport := allReports[i]
583593

584-
a.logger.Info("Successfully report integration test status for snapshot",
585-
"testedSnapshot.Name", testedSnapshot.Name,
586-
"destinationSnapshot.Name", destinationSnapshot.Name,
587-
"testStatus", integrationTestStatusDetail.Status)
588-
srs.SetLastUpdateTime(integrationTestStatusDetail.ScenarioName, destinationSnapshot.Name, integrationTestStatusDetail.LastUpdateTime)
594+
if srs.IsNewer(integrationTestStatusDetail.ScenarioName, destinationSnapshot.Name, integrationTestStatusDetail.LastUpdateTime) {
595+
a.logger.Info("Integration Test contains new status updates", "scenario.Name", integrationTestStatusDetail.ScenarioName, "destinationSnapshot.Name", destinationSnapshot.Name, "testedSnapshot", testedSnapshot.Name)
596+
} else {
597+
a.logger.Info("Integration Test doen't contain new status updates", "scenario.Name", integrationTestStatusDetail.ScenarioName)
598+
continue
599+
}
600+
601+
if statusCode, reportStatusErr := reporter.ReportStatus(a.context, testReport); reportStatusErr != nil {
602+
if integrationTestStatusDetail.Status.IsFinal() && integrationTestStatusDetail.TestPipelineRunName != "" {
603+
a.logger.Error(reportStatusErr, fmt.Sprintf("failed to report status to git provider for completed integration pipelinerun %s/%s, then finalizer test.appstudio.openshift.io/pipelinerun might not be removed from it later", testedSnapshot.Namespace, integrationTestStatusDetail.TestPipelineRunName))
604+
}
605+
606+
if reporter.ReturnCodeIsUnrecoverable(statusCode) {
607+
a.logger.Error(reportStatusErr, fmt.Sprintf("failed to report status to git provider for integration pipelinerun %s/%s, the statusCode %d is not easily recoverable", testedSnapshot.Namespace, integrationTestStatusDetail.TestPipelineRunName, statusCode))
608+
return nil
609+
} else {
610+
return fmt.Errorf("failed to update status: %w", reportStatusErr)
611+
}
612+
}
613+
614+
if writeErr := status.WriteSnapshotReportStatus(a.context, a.client, testedSnapshot, srs); writeErr != nil {
615+
return fmt.Errorf("failed to report status AND write snapshot report status metadata: %w", writeErr)
616+
}
617+
618+
a.logger.Info("Successfully report integration test status for snapshot",
619+
"testedSnapshot.Name", testedSnapshot.Name,
620+
"destinationSnapshot.Name", destinationSnapshot.Name,
621+
"testStatus", integrationTestStatusDetail.Status)
622+
srs.SetLastUpdateTime(integrationTestStatusDetail.ScenarioName, destinationSnapshot.Name, integrationTestStatusDetail.LastUpdateTime)
623+
}
589624
}
590625

591626
// update integration test status comment for gitlab and forgejo reporters when comment is not disabled

status/mock_reporter.go

Lines changed: 25 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

status/reporter.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ type TestReport struct {
6060
ShortText string
6161
// test status
6262
Status intgteststat.IntegrationTestStatus
63+
// IsOptional is true when the scenario is optional (failure does not block promotion)
64+
IsOptional bool
6365
// short summary of test results
6466
Summary string
6567
// time when test started
@@ -79,6 +81,11 @@ type ReporterInterface interface {
7981
GetReporterName() string
8082
// Update status of the integration test
8183
ReportStatus(context.Context, TestReport) (int, error)
84+
// ReportConsolidatedStatus posts a single aggregated commit status summarising
85+
// all integration test results. Used when the per-scenario count exceeds the
86+
// git provider's per-pipeline job limit. Reporters that do not support this
87+
// (e.g. GitHub) should return (http.StatusCreated, nil).
88+
ReportConsolidatedStatus(context.Context, []TestReport) (int, error)
8289
// Is the return code a recoverable error
8390
ReturnCodeIsUnrecoverable(statusCode int) bool
8491
// Update status comment in the gitlab merge request, implemented and used in reporter_gitlab.go

status/reporter_forgejo.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,13 @@ func (r *ForgejoReporter) DeleteExistingComments(commentIDs []int64) (int, error
385385
return lastStatusCode, nil
386386
}
387387

388+
// blank implementation to satisfy ReporterInterface — consolidated status
389+
// reporting is not yet needed for Forgejo due to its small user base.
390+
// Implement when a per-pipeline job limit becomes a practical concern.
391+
func (r *ForgejoReporter) ReportConsolidatedStatus(_ context.Context, _ []TestReport) (int, error) {
392+
return http.StatusCreated, nil
393+
}
394+
388395
// ReportStatus reports test result to forgejo
389396
func (r *ForgejoReporter) ReportStatus(ctx context.Context, report TestReport) (int, error) {
390397
var statusCode = 0

status/reporter_github.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,12 @@ func (r *GitHubReporter) UpdateStatusInComment(arg0, arg1 string, arg2 bool) (in
634634
return http.StatusCreated, nil
635635
}
636636

637+
// blank implementation to satisfy ReporterInterface — GitHub does not have a
638+
// per-pipeline job limit that requires status consolidation.
639+
func (r *GitHubReporter) ReportConsolidatedStatus(_ context.Context, _ []TestReport) (int, error) {
640+
return http.StatusCreated, nil
641+
}
642+
637643
// Initialize github reporter. Must be called before updating status
638644
func (r *GitHubReporter) Initialize(ctx context.Context, snapshot *applicationapiv1alpha1.Snapshot) (int, error) {
639645
var statusCode int
@@ -714,5 +720,7 @@ func (r *GitHubReporter) ReportStatus(ctx context.Context, report TestReport) (i
714720
}
715721

716722
func (r *GitHubReporter) ReturnCodeIsUnrecoverable(statusCode int) bool {
717-
return statusCode == http.StatusForbidden || statusCode == http.StatusUnauthorized || statusCode == http.StatusBadRequest || statusCode == http.StatusNotFound
723+
return statusCode == http.StatusForbidden || statusCode == http.StatusUnauthorized ||
724+
statusCode == http.StatusBadRequest || statusCode == http.StatusNotFound ||
725+
statusCode == http.StatusUnprocessableEntity
718726
}

0 commit comments

Comments
 (0)