TRT-2895: Force close regressions

pkg/api/README.md

@@ -644,3 +644,65 @@ Updates an existing triage record.
 Endpoint: `DELETE /api/component_readiness/triages/{id}`
 
 Deletes a triage record.
+
+Endpoint: `POST /api/component_readiness/triages/{id}/force_close_regressions`
+
+Force closes the open regressions associated with a resolved triage that existed at its
+resolution time (opened at or before `resolved`). Force closed regressions are excluded from
+the regression reuse window (regressionHysteresisDays), so they are not reopened for unrelated
+failures. This prevents generic tests (for example "install should succeed") from staying open
+for weeks with false "pants on fire" or "failed fix" status.
+
+Each regression is closed at the triage's resolution time and records, directly on the
+regression row, that it was force closed, by which user, for what reason, and the triage that
+drove the action. The operation is idempotent: regressions that opened after the resolution
+time, or that are already closed, are left untouched.
+
+The triage must be resolved. If it is not, the endpoint returns `400 Bad Request` with the
+message "Cannot force-close regressions for an unresolved triage. Resolve the triage first."
+This is a write endpoint and requires the `write_endpoints` capability.
+
+### Request body
+
+| Field  | Type   | Description                                              | Required |
+|--------|--------|----------------------------------------------------------|----------|
+| reason | String | The reason the regressions are being force closed.       | Yes      |
+
+### Response
+
+| Field                   | Type            | Description                                                     |
+|-------------------------|-----------------|-----------------------------------------------------------------|
+| closed_regression_ids   | Array of number | IDs of the regressions that were open and got closed.           |
+| timestamp               | String (time)   | The closed time applied to the regressions (the resolution time). |
+
+The regression record returned by `GET /api/component_readiness/regressions/{id}` includes
+`force_closed`, `force_closed_by`, `force_closed_reason`, and `force_closed_by_triage_id`
+directly (no join is required, the data is stored on the regression).
+
+Endpoint: `GET /api/component_readiness/triages/{id}/force_close_preview`
+
+Previews (dry run) what `force_close_regressions` would do for a resolved triage, without
+modifying anything. Use it to review which regressions would close and to spot any that kept
+failing after the claimed resolution before committing. The triage must be resolved; otherwise
+the endpoint returns `400 Bad Request` with the same message as the force close endpoint.
+
+### Response
+
+| Field           | Type            | Description                                                          |
+|-----------------|-----------------|---------------------------------------------------------------------|
+| triage_id       | Number          | The triage being previewed.                                         |
+| resolved        | String (time)   | The triage's resolution time (the cutoff used for scoping).         |
+| would_close     | Array of object | Open regressions that existed at the resolution time (would close). |
+| would_not_close | Array of object | Regressions that opened after the resolution time (left untouched). |
+
+Each regression object in `would_close` / `would_not_close` includes:
+
+| Field                         | Type            | Description                                                     |
+|-------------------------------|-----------------|-----------------------------------------------------------------|
+| regression_id                 | Number          | The regression ID.                                              |
+| test_name                     | String          | The regressed test name.                                        |
+| variants                      | Array of string | The regression's variants.                                      |
+| opened                        | String (time)   | When the regression opened.                                     |
+| closed                        | String (time)   | When the regression closed, if already closed.                  |
+| last_failure_before_resolution| String (time)   | Most recent failing job run at or before the resolution time.   |
+| first_failure_after_resolution| String (time)   | Earliest failing job run after the resolution time, if any (a gap indicator that the test kept failing). |

pkg/api/componentreadiness/regressiontracker.go

@@ -3,6 +3,7 @@ package componentreadiness
 import (
 	"context"
 	"database/sql"
+	"errors"
 	"fmt"
 	"time"
 
@@ -16,6 +17,7 @@ import (
 	"github.com/openshift/sippy/pkg/db"
 	"github.com/openshift/sippy/pkg/db/models"
 	log "github.com/sirupsen/logrus"
+	"gorm.io/gorm"
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
@@ -36,6 +38,14 @@ type RegressionStore interface {
 	UpdateRegression(reg *models.TestRegression) error
 	// ResolveTriages sets the resolution time on any triages that no longer have active regressions
 	ResolveTriages() error
+	// ForceCloseRegressions closes the open regressions associated with the given resolved triage that
+	// existed at its resolution time, marking them force closed so they are excluded from the reuse window
+	// and not reopened for unrelated failures. Returns ErrTriageNotResolved if the triage is not resolved.
+	// It is idempotent.
+	ForceCloseRegressions(triageID uint, closedBy, reason string) (*ForceCloseResult, error)
+	// ForceClosePreview returns a dry-run of what ForceCloseRegressions would do for the given triage,
+	// without modifying anything. Returns ErrTriageNotResolved if the triage is not resolved.
+	ForceClosePreview(triageID uint) (*ForceClosePreview, error)
 	// MergeJobRuns upserts job runs for a regression, adding new ones and skipping duplicates.
 	MergeJobRuns(regressionID uint, jobRuns []models.RegressionJobRun) error
 	// UpsertRegressionView records that a regression was observed in a view, setting active=true.
@@ -59,9 +69,11 @@ func (prs *PostgresRegressionStore) ListCurrentRegressionsForRelease(release str
 	// List open regressions (no closed date), or those that closed within the last few days. This is to prevent flapping
 	// and return more accurate opened dates when a test is falling in / out of the report.
 	regressions := make([]*models.TestRegression, 0)
+	// Force closed regressions are excluded from the reuse window even if they closed recently, so they
+	// are not reopened for unrelated failures (TRT-2895).
 	q := prs.dbc.DB.Table(testRegressionsTable).
 		Where("release = ?", release).
-		Where("closed IS NULL OR closed > ?", time.Now().Add(-regressionHysteresisDays*24*time.Hour))
+		Where("closed IS NULL OR (closed > ? AND force_closed = false)", time.Now().Add(-regressionHysteresisDays*24*time.Hour))
 	res := q.Scan(&regressions)
 	return regressions, res.Error
 }
@@ -217,7 +229,7 @@ func (prs *PostgresRegressionStore) ResolveTriages() error {
 	subQuery := prs.dbc.DB.Table("triage_regressions tr").
 		Joins("JOIN test_regressions r ON tr.test_regression_id = r.id").
 		Where("tr.triage_id = triages.id").
-		Where("r.closed IS NULL OR r.closed > ?", hysteresisTime).
+		Where("r.closed IS NULL OR (r.closed > ? AND r.force_closed = false)", hysteresisTime).
 		Select("1")
 
 	res := prs.dbc.DB.Table("triages").
@@ -265,6 +277,232 @@ func (prs *PostgresRegressionStore) ResolveTriages() error {
 	return nil
 }
 
+// ErrTriageNotResolved is returned by force close operations when the triage has not been resolved.
+// Force closing needs a resolution time to scope which regressions to close and when to close them.
+var ErrTriageNotResolved = errors.New("triage is not resolved")
+
+// ForceCloseResult summarizes the outcome of a force close operation. It is returned by the API
+// so callers know which regressions were closed and at what time.
+type ForceCloseResult struct {
+	// ClosedRegressionIDs are the IDs of the regressions that were open and got closed by this call.
+	// On an idempotent repeat call (regressions already closed) this will be empty.
+	ClosedRegressionIDs []uint `json:"closed_regression_ids"`
+	// Timestamp is the closed time applied to the regressions (the triage's resolution time).
+	Timestamp time.Time `json:"timestamp"`
+}
+
+// RegressionFailureGap describes the failure timing around a triage's resolution for a single regression.
+// A non-nil FirstFailureAfterResolution means the test kept failing after the claimed resolution, a signal
+// the triage may have been resolved prematurely and force closing warrants a closer look.
+type RegressionFailureGap struct {
+	// LastFailureBeforeResolution is the most recent failing job run at or before the resolution time.
+	LastFailureBeforeResolution *time.Time `json:"last_failure_before_resolution,omitempty"`
+	// FirstFailureAfterResolution is the earliest failing job run after the resolution time, if any.
+	FirstFailureAfterResolution *time.Time `json:"first_failure_after_resolution,omitempty"`
+}
+
+// ForceClosePreviewRegression describes a single regression in a force close preview, including the failure
+// gap around the triage's resolution so a user can judge whether force closing is appropriate.
+type ForceClosePreviewRegression struct {
+	RegressionID uint           `json:"regression_id"`
+	TestName     string         `json:"test_name"`
+	Variants     pq.StringArray `json:"variants"`
+	Opened       time.Time      `json:"opened"`
+	// Closed is set when the regression is already closed.
+	Closed *time.Time `json:"closed,omitempty"`
+	// RegressionFailureGap is embedded so its fields are promoted into this object's JSON.
+	RegressionFailureGap
+}
+
+// ForceClosePreview is the dry-run result for force closing a triage's regressions. WouldClose lists the
+// open regressions that existed at the resolution time and would be closed; WouldNotClose lists regressions
+// that opened after the resolution time and would be left untouched.
+type ForceClosePreview struct {
+	TriageID      uint                          `json:"triage_id"`
+	Resolved      time.Time                     `json:"resolved"`
+	WouldClose    []ForceClosePreviewRegression `json:"would_close"`
+	WouldNotClose []ForceClosePreviewRegression `json:"would_not_close"`
+}
+
+// ForceCloseRegressions closes the open regressions associated with the given resolved triage that existed
+// at its resolution time (opened at or before triage.Resolved), marking them force closed so they are
+// excluded from the regression reuse window (regressionHysteresisDays) and never reopened for unrelated
+// failures (TRT-2895). Each regression is closed at the triage's resolution time and records who force
+// closed it and why, directly on the regression row. The triage must be resolved; otherwise
+// ErrTriageNotResolved is returned. It is idempotent: already-closed regressions are left untouched.
+func (prs *PostgresRegressionStore) ForceCloseRegressions(triageID uint, closedBy, reason string) (*ForceCloseResult, error) {
+	result := &ForceCloseResult{}
+
+	err := prs.dbc.DB.Transaction(func(tx *gorm.DB) error {
+		var triage models.Triage
+		if err := tx.First(&triage, triageID).Error; err != nil {
+			return fmt.Errorf("error loading triage %d for force close: %w", triageID, err)
+		}
+		if !triage.Resolved.Valid {
+			return ErrTriageNotResolved
+		}
+		closeTime := triage.Resolved.Time
+		result.Timestamp = closeTime
+
+		// Select only the regressions this triage actually resolved: those that existed at the
+		// resolution time (opened <= closeTime) and are still open (closed IS NULL). Filtering in the
+		// query rather than loading every regression ever associated with the triage keeps the action
+		// scoped, keeps it idempotent, and preserves the closed time of already-closed regressions.
+		var regIDsToClose []uint
+		if err := tx.Table(testRegressionsTable).
+			Joins("JOIN triage_regressions ON triage_regressions.test_regression_id = test_regressions.id").
+			Where("triage_regressions.triage_id = ?", triageID).
+			Where("test_regressions.closed IS NULL").
+			Where("test_regressions.opened <= ?", closeTime).
+			Pluck("test_regressions.id", &regIDsToClose).Error; err != nil {
+			return fmt.Errorf("error finding regressions to force close for triage %d: %w", triageID, err)
+		}
+		if len(regIDsToClose) == 0 {
+			return nil
+		}
+
+		// Update only the affected columns in a single statement to avoid rewriting the many2many
+		// triage associations.
+		updates := map[string]interface{}{
+			"closed":                    sql.NullTime{Valid: true, Time: closeTime},
+			"force_closed":              true,
+			"force_closed_by":           closedBy,
+			"force_closed_reason":       reason,
+			"force_closed_by_triage_id": triageID,
+		}
+		if err := tx.Model(&models.TestRegression{}).Where("id IN ?", regIDsToClose).Updates(updates).Error; err != nil {
+			return fmt.Errorf("error force closing regressions for triage %d: %w", triageID, err)
+		}
+		result.ClosedRegressionIDs = regIDsToClose
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	log.WithField("triageID", triageID).WithField("closedBy", closedBy).
+		WithField("closedRegressions", len(result.ClosedRegressionIDs)).Info("force closed regressions for triage")
+	return result, nil
+}
+
+// queryRegressionFailureGaps computes, for each of the given regressions, the last failing job run at
+// or before resolutionTime and the first failing job run after resolutionTime using the
+// regression_job_runs table. It runs a single grouped query per direction (rather than a query per
+// regression) to avoid an N+1 pattern. Regressions with no matching failing run are absent from the
+// corresponding map entry / field.
+func (prs *PostgresRegressionStore) queryRegressionFailureGaps(regressionIDs []uint, resolutionTime time.Time) (map[uint]RegressionFailureGap, error) {
+	gaps := make(map[uint]RegressionFailureGap, len(regressionIDs))
+	if len(regressionIDs) == 0 {
+		return gaps, nil
+	}
+
+	// gapRow captures the grouped aggregate (MAX/MIN start_time) per regression_id.
+	type gapRow struct {
+		RegressionID uint
+		FailureTime  sql.NullTime
+	}
+
+	// Last failing run at or before the resolution time, one row per regression.
+	var lastRows []gapRow
+	if err := prs.dbc.DB.Table("regression_job_runs").
+		Select("regression_id, MAX(start_time) AS failure_time").
+		Where("regression_id IN ? AND start_time <= ? AND test_failed = true", regressionIDs, resolutionTime).
+		Group("regression_id").
+		Scan(&lastRows).Error; err != nil {
+		return nil, fmt.Errorf("error querying last failures before resolution: %w", err)
+	}
+	for _, row := range lastRows {
+		if row.FailureTime.Valid {
+			t := row.FailureTime.Time
+			gap := gaps[row.RegressionID]
+			gap.LastFailureBeforeResolution = &t
+			gaps[row.RegressionID] = gap
+		}
+	}
+
+	// First failing run after the resolution time, one row per regression.
+	var firstRows []gapRow
+	if err := prs.dbc.DB.Table("regression_job_runs").
+		Select("regression_id, MIN(start_time) AS failure_time").
+		Where("regression_id IN ? AND start_time > ? AND test_failed = true", regressionIDs, resolutionTime).
+		Group("regression_id").
+		Scan(&firstRows).Error; err != nil {
+		return nil, fmt.Errorf("error querying first failures after resolution: %w", err)
+	}
+	for _, row := range firstRows {
+		if row.FailureTime.Valid {
+			t := row.FailureTime.Time
+			gap := gaps[row.RegressionID]
+			gap.FirstFailureAfterResolution = &t
+			gaps[row.RegressionID] = gap
+		}
+	}
+
+	return gaps, nil
+}
+
+// ForceClosePreview returns a dry-run of what ForceCloseRegressions would do for the given triage without
+// modifying anything. The triage must be resolved; otherwise ErrTriageNotResolved is returned. Each entry
+// includes the failure gap around the resolution time so callers can spot regressions that kept failing.
+func (prs *PostgresRegressionStore) ForceClosePreview(triageID uint) (*ForceClosePreview, error) {
+	var triage models.Triage
+	if err := prs.dbc.DB.First(&triage, triageID).Error; err != nil {
+		return nil, fmt.Errorf("error loading triage %d for force close preview: %w", triageID, err)
+	}
+	if !triage.Resolved.Valid {
+		return nil, ErrTriageNotResolved
+	}
+	resolved := triage.Resolved.Time
+	preview := &ForceClosePreview{TriageID: triageID, Resolved: resolved}
+
+	// Load only the regressions the preview reports on: those still open (would close / would not close
+	// depending on when they opened) or those opened after the resolution time. Already-closed
+	// regressions that opened before resolution are omitted (force closing would not change them), so we
+	// filter them out in the query rather than loading every regression ever associated with the triage.
+	var regs []models.TestRegression
+	if err := prs.dbc.DB.Table(testRegressionsTable).
+		Select("test_regressions.*").
+		Joins("JOIN triage_regressions ON triage_regressions.test_regression_id = test_regressions.id").
+		Where("triage_regressions.triage_id = ?", triageID).
+		Where("test_regressions.closed IS NULL OR test_regressions.opened > ?", resolved).
+		Find(&regs).Error; err != nil {
+		return nil, fmt.Errorf("error loading regressions for triage %d force close preview: %w", triageID, err)
+	}
+
+	regIDs := make([]uint, len(regs))
+	for i := range regs {
+		regIDs[i] = regs[i].ID
+	}
+	gaps, err := prs.queryRegressionFailureGaps(regIDs, resolved)
+	if err != nil {
+		return nil, err
+	}
+
+	for i := range regs {
+		reg := &regs[i]
+		entry := ForceClosePreviewRegression{
+			RegressionID:         reg.ID,
+			TestName:             reg.TestName,
+			Variants:             reg.Variants,
+			Opened:               reg.Opened,
+			RegressionFailureGap: gaps[reg.ID],
+		}
+		if reg.Closed.Valid {
+			c := reg.Closed.Time
+			entry.Closed = &c
+		}
+		switch {
+		case !reg.Closed.Valid && !reg.Opened.After(resolved):
+			// Open and existed at the resolution time: this is what force close would close.
+			preview.WouldClose = append(preview.WouldClose, entry)
+		case reg.Opened.After(resolved):
+			// Opened after the resolution time: force close would leave it untouched.
+			preview.WouldNotClose = append(preview.WouldNotClose, entry)
+		}
+	}
+	return preview, nil
+}
+
 // SyncRegressionsForReport compares regressed tests from a component report against known
 // regressions in the database, opening new ones, reopening recently closed ones, and updating
 // stats on existing ones. Returns the list of active regressions after sync.

pkg/db/migrations/000013_add_force_close_to_regressions.down.sql

@@ -0,0 +1,8 @@
+DROP INDEX IF EXISTS idx_test_regressions_force_closed;
+DROP INDEX IF EXISTS idx_test_regressions_force_closed_by_triage_id;
+
+ALTER TABLE test_regressions
+    DROP COLUMN IF EXISTS force_closed_by_triage_id,
+    DROP COLUMN IF EXISTS force_closed_reason,
+    DROP COLUMN IF EXISTS force_closed_by,
+    DROP COLUMN IF EXISTS force_closed;

pkg/db/migrations/000013_add_force_close_to_regressions.up.sql

@@ -0,0 +1,25 @@
+-- TRT-2895: Force close regressions.
+--
+-- Generic tests (e.g. "install should succeed") stay open for weeks because the
+-- 5-day regression reuse window (regressionHysteresisDays) reopens recently
+-- closed regressions for unrelated failures, causing false "pants on fire" /
+-- "failed fix" status. Force closing a resolved triage's regressions marks them
+-- so they are excluded from the reuse window and never reopened.
+--
+-- All force close metadata lives on test_regressions so a regression is
+-- self-contained: it records that it was force closed, by whom, why, and which
+-- triage drove the action. Existing rows default to not force closed.
+
+ALTER TABLE test_regressions
+    ADD COLUMN IF NOT EXISTS force_closed BOOLEAN NOT NULL DEFAULT false,
+    ADD COLUMN IF NOT EXISTS force_closed_by TEXT NOT NULL DEFAULT '',
+    ADD COLUMN IF NOT EXISTS force_closed_reason TEXT NOT NULL DEFAULT '',
+    ADD COLUMN IF NOT EXISTS force_closed_by_triage_id BIGINT;
+
+CREATE INDEX IF NOT EXISTS idx_test_regressions_force_closed_by_triage_id
+    ON test_regressions (force_closed_by_triage_id);
+
+-- Partial index so the reuse-window queries that exclude force closed regressions
+-- (ListCurrentRegressionsForRelease, ResolveTriages) can skip the force closed rows
+-- cheaply. Only the force closed rows are indexed.
+CREATE INDEX idx_test_regressions_force_closed ON test_regressions (force_closed) WHERE force_closed = true;