From e884df37936f61dc4d11864b6ab70d5432610358 Mon Sep 17 00:00:00 2001 From: Tirefire <84106878+tire-fire@users.noreply.github.com> Date: Sun, 8 Nov 2026 17:27:39 +0000 Subject: [PATCH 1/5] Add preflight rounds before competition start --- README.md | 2 + engine/checks/checks.go | 1 + engine/db/db.go | 3 +- engine/db/preflight.go | 38 +++++++++ engine/engine.go | 106 ++++++++++++++++++------ engine/preflight_test.go | 170 +++++++++++++++++++++++++++++++++++++++ engine/task.go | 1 + runner/runner.go | 7 +- www/api/admin.go | 30 +++++++ www/router.go | 1 + 10 files changed, 331 insertions(+), 28 deletions(-) create mode 100644 engine/db/preflight.go create mode 100644 engine/preflight_test.go diff --git a/README.md b/README.md index 79f098f0..30b0e3ca 100644 --- a/README.md +++ b/README.md @@ -271,6 +271,8 @@ AttemptDelay = 0 # Seconds to wait before retrying a failed attempt ( Every attempt, plus the waits between them, must fit in the shortest round: `Attempts * Timeout + (Attempts - 1) * AttemptDelay` must be less than `Delay - Jitter`. +In rvb events, rounds that run before an admin first starts the competition are **preflight rounds**. They run every check, but their results are stored apart and never count toward points, uptime or SLAs. Leave `StartPaused = false` to have preflight rounds run as soon as the engine is up. `GET /api/engine/preflight` (admin) returns the latest preflight round, so you can confirm every team is green before handing over. + #### UI Settings ```toml diff --git a/engine/checks/checks.go b/engine/checks/checks.go index be07f1f9..2a53e3eb 100644 --- a/engine/checks/checks.go +++ b/engine/checks/checks.go @@ -58,6 +58,7 @@ type Result struct { Points int `json:"points,omitempty"` ServiceType string `json:"service_type,omitempty"` RoundID uint `json:"round_id"` + Preflight bool `json:"preflight,omitempty"` // Added for runner visualization RunnerID string `json:"runner_id,omitempty"` diff --git a/engine/db/db.go b/engine/db/db.go index 707b9c3c..4fe582f6 100644 --- a/engine/db/db.go +++ b/engine/db/db.go @@ -55,7 +55,8 @@ func (d *DB) migrate() { // box schema must come first for automigrate to work &VulnSchema{}, &BoxSchema{}, &VectorSchema{}, &AttackSchema{}, &CompetitionStateSchema{}, // credential schemas for PCR management - &OriginalCredentialSchema{}, &CredentialSchema{}, &PCRHistorySchema{}) + &OriginalCredentialSchema{}, &CredentialSchema{}, &PCRHistorySchema{}, + &PreflightCheckSchema{}) if err != nil { log.Fatalln("Failed to auto migrate:", err) } diff --git a/engine/db/preflight.go b/engine/db/preflight.go new file mode 100644 index 00000000..1e8bdf8b --- /dev/null +++ b/engine/db/preflight.go @@ -0,0 +1,38 @@ +package db + +import ( + "time" +) + +// Preflight rounds are kept apart from round_schemas so they never count toward points, uptime or SLAs. +type PreflightCheckSchema struct { + ID uint `gorm:"primarykey"` + RoundID uint `gorm:"index"` + StartTime time.Time + TeamID uint + Team TeamSchema + ServiceName string + ServiceType string + Result bool + Error string + Debug string +} + +func (d *DB) CreatePreflightChecks(checks []PreflightCheckSchema) error { + return d.db.Create(&checks).Error +} + +func (d *DB) GetLastPreflightRound() (uint, error) { + var last uint + err := d.db.Model(&PreflightCheckSchema{}).Select("COALESCE(MAX(round_id), 0)").Scan(&last).Error + return last, err +} + +func (d *DB) GetLatestPreflightChecks() ([]PreflightCheckSchema, error) { + var checks []PreflightCheckSchema + err := d.db.Preload("Team"). + Where("round_id = (?)", d.db.Model(&PreflightCheckSchema{}).Select("MAX(round_id)")). + Order("team_id, service_name"). + Find(&checks).Error + return checks, err +} diff --git a/engine/engine.go b/engine/engine.go index b1bf8a47..2e95512e 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -32,6 +32,7 @@ type ScoringEngine struct { uptimeMu sync.RWMutex SlaPerService map[uint]map[string]int CurrentRound uint + PreflightRound uint // last preflight round run; see preflight() NextRoundStartTime time.Time CurrentRoundStartTime time.Time RedisClient *redis.Client @@ -103,6 +104,11 @@ func (se *ScoringEngine) Start() { } else { se.CurrentRound = uint(t.ID) + 1 } + if last, err := se.DB.GetLastPreflightRound(); err != nil { + slog.Error("failed to get last preflight round", "error", err) + } else { + se.PreflightRound = last + } se.uptimeMu.Lock() if err := se.DB.LoadUptimes(&se.UptimePerService); err != nil { @@ -144,16 +150,20 @@ func (se *ScoringEngine) Start() { default: } - slog.Info("Starting round", "round", se.CurrentRound) + preflight := se.Config.RequiredSettings.EventType == "rvb" && !se.competitionEverStarted() se.CurrentRoundStartTime = time.Now() se.NextRoundStartTime = time.Now().Add(time.Duration(se.Config.MiscSettings.Delay) * time.Second) - // run the round logic var err error - switch se.Config.RequiredSettings.EventType { - case "koth": + switch { + case preflight: + slog.Info("Starting preflight round", "preflight_round", se.PreflightRound+1) + err = se.preflight() + case se.Config.RequiredSettings.EventType == "koth": + slog.Info("Starting round", "round", se.CurrentRound) err = se.koth() - case "rvb": + case se.Config.RequiredSettings.EventType == "rvb": + slog.Info("Starting round", "round", se.CurrentRound) err = se.rvb() default: slog.Error("Unknown event type", "eventType", se.Config.RequiredSettings.EventType) @@ -163,11 +173,14 @@ func (se *ScoringEngine) Start() { return } - slog.Info(fmt.Sprintf("Round %d complete", se.CurrentRound)) - se.CurrentRound++ - - se.RedisClient.Publish(context.Background(), "events", "round_finish") - slog.Info(fmt.Sprintf("Round %d will start in %s, sleeping...", se.CurrentRound, time.Until(se.NextRoundStartTime).String())) + if preflight { + slog.Info(fmt.Sprintf("Preflight round %d complete", se.PreflightRound)) + } else { + slog.Info(fmt.Sprintf("Round %d complete", se.CurrentRound)) + se.CurrentRound++ + se.RedisClient.Publish(context.Background(), "events", "round_finish") + } + slog.Info(fmt.Sprintf("Next round will start in %s, sleeping...", time.Until(se.NextRoundStartTime).String())) select { case <-stop: return @@ -326,6 +339,16 @@ func (se *ScoringEngine) Paused() bool { return se.pausedLocked() } +// Stopping the competition after it starts doesn't bring preflight rounds back. +func (se *ScoringEngine) competitionEverStarted() bool { + start, err := se.DB.GetCompetitionStart() + if err != nil { + slog.Error("failed to read competition start; scoring the round", "error", err) + return true + } + return start != nil +} + // ResetScores resets the engine to the initial state and stops the engine func (se *ScoringEngine) ResetScores() error { slog.Info("Resetting scores and clearing Redis queues") @@ -367,8 +390,47 @@ func (se *ScoringEngine) koth() error { return nil } -// perform a round of rvb func (se *ScoringEngine) rvb() error { + results, err := se.runChecks(se.CurrentRound, false) + if err != nil { + return err + } + se.processCollectedResults(results) + return nil +} + +func (se *ScoringEngine) preflight() error { + round := se.PreflightRound + 1 + results, err := se.runChecks(round, true) + if err != nil { + return err + } + se.PreflightRound = round + if len(results) == 0 { + slog.Warn("No results collected for preflight round", "preflight_round", round) + return nil + } + + rows := make([]db.PreflightCheckSchema, 0, len(results)) + for _, result := range results { + rows = append(rows, db.PreflightCheckSchema{ + RoundID: round, + StartTime: se.CurrentRoundStartTime, + TeamID: result.TeamID, + ServiceName: sanitizeDBString(result.ServiceName), + ServiceType: result.ServiceType, + Result: result.Status, + Error: sanitizeDBString(result.Error), + Debug: sanitizeDBString(result.Debug), + }) + } + if err := se.DB.CreatePreflightChecks(rows); err != nil { + slog.Error("failed to save preflight round", "preflight_round", round, "error", err) + } + return nil +} + +func (se *ScoringEngine) runChecks(roundID uint, preflight bool) ([]checks.Result, error) { // reassign the next round start time with jitter // double the jitter and subtract it to get a random number between -jitter and jitter randomJitter := rand.Intn(2*se.Config.MiscSettings.Jitter) - se.Config.MiscSettings.Jitter // #nosec G404 -- non-crypto randomization for timing jitter @@ -382,18 +444,17 @@ func (se *ScoringEngine) rvb() error { defer events.Close() eventsChannel := events.Channel() - // do rvb stuff teams, err := se.DB.GetTeams() if err != nil { slog.Error("failed to get teams:", "error", err) - return err + return nil, err } runners := 0 ctx, cancel := context.WithTimeout(context.Background(), time.Until(se.NextRoundStartTime)) defer cancel() - slog.Debug("Starting service checks", "round", se.CurrentRound) + slog.Debug("Starting service checks", "round", roundID, "preflight", preflight) allRunners := se.Config.AllChecks() slog.Info("Service check configuration", "boxes", len(se.Config.Box), @@ -409,12 +470,11 @@ func (se *ScoringEngine) rvb() error { // results would otherwise be counted as this round's. for _, key := range []string{"tasks", "results"} { if stale := se.RedisClient.LLen(ctx, key).Val(); stale > 0 { - slog.Warn("Clearing stale entries from queue", "queue", key, "count", stale, "round", se.CurrentRound) + slog.Warn("Clearing stale entries from queue", "queue", key, "count", stale, "round", roundID, "preflight", preflight) se.RedisClient.Del(ctx, key) } } - // 1) Enqueue for _, team := range teams { if !team.Active { continue @@ -443,7 +503,8 @@ func (se *ScoringEngine) rvb() error { TeamIdentifier: team.Identifier, ServiceType: r.GetType(), ServiceName: r.GetName(), - RoundID: se.CurrentRound, + RoundID: roundID, + Preflight: preflight, Deadline: se.NextRoundStartTime, Attempts: r.GetAttempts(), AttemptDelay: r.GetAttemptDelay(), @@ -483,7 +544,6 @@ func (se *ScoringEngine) rvb() error { } slog.Info("Enqueued checks", "count", runners) - // 2) Collect results from Redis results := make([]checks.Result, 0, runners) timeoutCtx, cancel := context.WithTimeout(ctx, time.Until(se.NextRoundStartTime)) defer cancel() @@ -496,7 +556,7 @@ COLLECTION: slog.Info("Received message", "message", msg.Payload) if msg.Payload == "reset" { slog.Info("Reset event received, quitting...") - return fmt.Errorf("reset event received") + return nil, fmt.Errorf("reset event received") } else { continue } @@ -530,8 +590,8 @@ COLLECTION: slog.Error("Failed to unmarshal check result", "error", err) continue } - if result.RoundID != se.CurrentRound { - slog.Warn("Ignoring out of round result", "receivedRound", result.RoundID, "currentRound", se.CurrentRound) + if result.RoundID != roundID || result.Preflight != preflight { + slog.Warn("Ignoring out of round result", "receivedRound", result.RoundID, "receivedPreflight", result.Preflight, "currentRound", roundID, "currentPreflight", preflight) continue } results = append(results, result) @@ -540,9 +600,7 @@ COLLECTION: } } - // 3) Process all collected results - se.processCollectedResults(results) - return nil + return results, nil } func sanitizeDBString(s string) string { diff --git a/engine/preflight_test.go b/engine/preflight_test.go new file mode 100644 index 00000000..c0416d9d --- /dev/null +++ b/engine/preflight_test.go @@ -0,0 +1,170 @@ +package engine + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/dbaseqp/Quotient/engine/checks" + "github.com/dbaseqp/Quotient/engine/config" + "github.com/dbaseqp/Quotient/engine/db" + "github.com/dbaseqp/Quotient/tests/testutil" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func answerTasks(t *testing.T, redis *testutil.RedisContainer, seen chan<- Task, extra func(Task) []checks.Result) (stop func()) { + t.Helper() + ctx := context.Background() + done := make(chan struct{}) + finished := make(chan struct{}) + go func() { + defer close(finished) + for { + select { + case <-done: + return + default: + } + val, err := redis.Client.BLPop(ctx, time.Second, "tasks").Result() + if err != nil || len(val) < 2 { + continue + } + var task Task + if err := json.Unmarshal([]byte(val[1]), &task); err != nil { + continue + } + if seen != nil { + seen <- task + } + results := []checks.Result{} + if extra != nil { + results = extra(task) + } + results = append(results, checks.Result{ + TeamID: task.TeamID, ServiceName: task.ServiceName, ServiceType: task.ServiceType, + RoundID: task.RoundID, Preflight: task.Preflight, Status: true, Points: 10, + }) + for _, r := range results { + payload, _ := json.Marshal(r) + redis.Client.RPush(ctx, "results", payload) + } + } + }() + return func() { + close(done) + <-finished + } +} + +func newPreflightEngine(t *testing.T, redis *testutil.RedisContainer, pg *testutil.PostgresContainer) *ScoringEngine { + t.Helper() + se := NewTestEngine(t, redis, pg, 3) + last, err := pg.DB.GetLastPreflightRound() + require.NoError(t, err) + se.PreflightRound = last + svc := &mockRunner{Service: checks.Service{Display: "web", Name: "box01-web", ServiceType: "Mock", Points: 10}} + se.Config.Box = []config.Box{{Name: "box01", IP: "10.0.0.1", Runners: []checks.Runner{svc}}} + return se +} + +func teamPreflight(t *testing.T, pg *testutil.PostgresContainer, teamID uint) []db.PreflightCheckSchema { + t.Helper() + rows, err := pg.DB.GetLatestPreflightChecks() + require.NoError(t, err) + var mine []db.PreflightCheckSchema + for _, row := range rows { + if row.TeamID == teamID { + mine = append(mine, row) + } + } + return mine +} + +func TestPreflight_StoresResultsApartFromScoring(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + redis, pg := testutil.StartContainers(t) + ctx := context.Background() + redis.Client.FlushDB(ctx) + require.NoError(t, pg.DB.ResetScores()) + + team := pg.CreateTestTeam(t, "Team Preflight", "01") + se := newPreflightEngine(t, redis, pg) + firstRound := se.PreflightRound + 1 + + seen := make(chan Task, 100) + stop := answerTasks(t, redis, seen, nil) + require.NoError(t, se.preflight()) + require.NoError(t, se.preflight()) + stop() + close(seen) + + for task := range seen { + assert.True(t, task.Preflight, "preflight tasks are marked for the runner") + } + + assert.Equal(t, firstRound+1, se.PreflightRound) + rows := teamPreflight(t, pg, team.ID) + require.Len(t, rows, 1, "only the latest preflight round is returned") + assert.Equal(t, firstRound+1, rows[0].RoundID) + assert.True(t, rows[0].Result) + assert.Equal(t, "box01-web", rows[0].ServiceName) + assert.Equal(t, "Mock", rows[0].ServiceType) + assert.Equal(t, team.Name, rows[0].Team.Name) + + lastRound, err := pg.DB.GetLastRound() + require.NoError(t, err) + assert.Zero(t, lastRound.ID, "preflight rounds never create scored rounds") + assert.Equal(t, uint(1), se.CurrentRound, "preflight rounds don't advance the scored round") + assert.Empty(t, se.UptimePerService, "preflight rounds don't count toward uptime") + assert.Empty(t, se.SlaPerService, "preflight rounds don't count toward SLAs") +} + +func TestPreflight_IgnoresScoredRoundResults(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + redis, pg := testutil.StartContainers(t) + ctx := context.Background() + redis.Client.FlushDB(ctx) + require.NoError(t, pg.DB.ResetScores()) + + team := pg.CreateTestTeam(t, "Team Preflight Mixed", "01") + se := newPreflightEngine(t, redis, pg) + + stop := answerTasks(t, redis, nil, func(task Task) []checks.Result { + return []checks.Result{{ + TeamID: task.TeamID, ServiceName: task.ServiceName, ServiceType: task.ServiceType, + RoundID: task.RoundID, Preflight: false, Status: false, + }} + }) + require.NoError(t, se.preflight()) + stop() + + rows := teamPreflight(t, pg, team.ID) + require.Len(t, rows, 1) + assert.True(t, rows[0].Result, "a scored round's result must not be stored as preflight") +} + +func TestCompetitionStarted_StaysStartedAfterStop(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + redis, pg := testutil.StartContainers(t) + pg.ResetCompetitionState(t) + t.Cleanup(func() { pg.ResetCompetitionState(t) }) + se := NewTestEngine(t, redis, pg, 3) + + assert.False(t, se.competitionEverStarted()) + require.NoError(t, pg.DB.SetCompetitionStarted(true)) + assert.True(t, se.competitionEverStarted()) + require.NoError(t, pg.DB.SetCompetitionStarted(false)) + assert.True(t, se.competitionEverStarted(), "stopping after the first start doesn't return to preflight") +} diff --git a/engine/task.go b/engine/task.go index a7a43859..9177cae5 100644 --- a/engine/task.go +++ b/engine/task.go @@ -18,6 +18,7 @@ type Task struct { ServiceName string `json:"service_name"` Deadline time.Time `json:"deadline"` RoundID uint `json:"round_id"` + Preflight bool `json:"preflight,omitempty"` Attempts int `json:"attempts"` AttemptDelay int `json:"attempt_delay"` // seconds to wait before retrying a failed attempt CheckData json.RawMessage `json:"check_data"` diff --git a/runner/runner.go b/runner/runner.go index eaa55529..99b96bc9 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -159,15 +159,14 @@ func createRunner(task *engine.Task) (checks.Runner, error) { } func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, task *engine.Task) { - // Create a task key to identify the check - taskKey := fmt.Sprintf("task:%d:%d:%s:%s", task.RoundID, task.TeamID, task.ServiceType, task.ServiceName) + taskKey := fmt.Sprintf("task:%t:%d:%d:%s:%s", task.Preflight, task.RoundID, task.TeamID, task.ServiceType, task.ServiceName) - // Create a result result := checks.Result{ TeamID: task.TeamID, ServiceName: task.ServiceName, ServiceType: task.ServiceType, RoundID: task.RoundID, + Preflight: task.Preflight, Status: false, RunnerID: runnerID, StartTime: time.Now().Format(time.RFC3339), @@ -211,6 +210,7 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta result.ServiceName = task.ServiceName result.ServiceType = task.ServiceType result.RoundID = task.RoundID + result.Preflight = task.Preflight slog.Info("check result received", "round_id", result.RoundID, "team_id", result.TeamID, "service_type", result.ServiceType, "status", result.Status, "debug", result.Debug, "error", result.Error) @@ -223,6 +223,7 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta result.ServiceName = task.ServiceName result.ServiceType = task.ServiceType result.RoundID = task.RoundID + result.Preflight = task.Preflight slog.Warn("check timed out", "round_id", task.RoundID, "team_id", task.TeamID, "service_type", task.ServiceType) diff --git a/www/api/admin.go b/www/api/admin.go index 41d7a5c9..50e92c79 100644 --- a/www/api/admin.go +++ b/www/api/admin.go @@ -11,6 +11,7 @@ import ( "path/filepath" "regexp" "strings" + "time" ) var validIdentifierRegex = regexp.MustCompile(`^[0-9]{1,3}$`) @@ -163,6 +164,35 @@ func (a *API) GetEngine(w http.ResponseWriter, r *http.Request) { }) } +func (a *API) GetPreflight(w http.ResponseWriter, r *http.Request) { + rows, err := a.eng.DB.GetLatestPreflightChecks() + if err != nil { + slog.Error("failed to get preflight checks", "error", err) + WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": "Failed to retrieve preflight checks"}) + return + } + + type check struct { + TeamID uint `json:"team_id"` + TeamName string `json:"team_name"` + ServiceName string `json:"service_name"` + ServiceType string `json:"service_type"` + Result bool `json:"result"` + Error string `json:"error"` + Debug string `json:"debug"` + } + resp := struct { + Round uint `json:"round"` + StartTime *time.Time `json:"start_time"` + Checks []check `json:"checks"` + }{Checks: make([]check, 0, len(rows))} + for _, row := range rows { + resp.Round, resp.StartTime = row.RoundID, &row.StartTime + resp.Checks = append(resp.Checks, check{row.TeamID, row.Team.Name, row.ServiceName, row.ServiceType, row.Result, row.Error, row.Debug}) + } + WriteJSON(w, http.StatusOK, resp) +} + func (a *API) UpdateTeams(w http.ResponseWriter, r *http.Request) { type Form struct { Teams []struct { diff --git a/www/router.go b/www/router.go index 612238a4..7e021316 100644 --- a/www/router.go +++ b/www/router.go @@ -165,6 +165,7 @@ func (router *Router) Start() { mux.HandleFunc("GET /api/engine/reset", ADMINAUTH(a.ResetScores)) mux.HandleFunc("GET /api/engine", ADMINAUTH(a.GetEngine)) mux.HandleFunc("GET /api/engine/tasks", ADMINAUTH(a.GetActiveTasks)) + mux.HandleFunc("GET /api/engine/preflight", ADMINAUTH(a.GetPreflight)) mux.HandleFunc("POST /api/competition/start", ADMINAUTH(a.SetCompetitionStarted)) mux.HandleFunc("POST /api/admin/teams", ADMINAUTH(a.UpdateTeams)) mux.HandleFunc("GET /api/admin/teamchecks", ADMINAUTH(a.GetTeamChecks)) From 783e0100e8904acf82a15979d95be321c6ca60b7 Mon Sep 17 00:00:00 2001 From: Tirefire <84106878+tire-fire@users.noreply.github.com> Date: Mon, 9 Nov 2026 06:31:46 +0000 Subject: [PATCH 2/5] Address review findings --- README.md | 4 +- docs/custom-checks.md | 2 +- engine/db/competition.go | 15 +++ engine/db/db.go | 8 +- engine/db/preflight.go | 37 ++++--- engine/engine.go | 72 ++++++------- engine/engine_test.go | 129 +++-------------------- engine/preflight_test.go | 171 +++++++++++++++++++++++-------- runner/runner.go | 25 ++--- tests/integration/graphs_test.go | 2 +- tests/testutil/testutil.go | 7 +- www/api/admin.go | 14 +-- 12 files changed, 239 insertions(+), 247 deletions(-) diff --git a/README.md b/README.md index 30b0e3ca..d2c8e5e2 100644 --- a/README.md +++ b/README.md @@ -254,7 +254,7 @@ ShowDebugToBlueTeam = false # Show check debug info to teams Port = 80 # Server port (443 default with SSL) LogoImage = "/static/assets/quotient.svg" LogFile = "" # Optional log file path -StartPaused = true # Start with scoring paused +StartPaused = true # Start with the engine paused (no preflight or scored rounds until resumed) # Round timing Delay = 60 # Seconds between rounds (default: 60) @@ -271,7 +271,7 @@ AttemptDelay = 0 # Seconds to wait before retrying a failed attempt ( Every attempt, plus the waits between them, must fit in the shortest round: `Attempts * Timeout + (Attempts - 1) * AttemptDelay` must be less than `Delay - Jitter`. -In rvb events, rounds that run before an admin first starts the competition are **preflight rounds**. They run every check, but their results are stored apart and never count toward points, uptime or SLAs. Leave `StartPaused = false` to have preflight rounds run as soon as the engine is up. `GET /api/engine/preflight` (admin) returns the latest preflight round, so you can confirm every team is green before handing over. +In rvb events, rounds that run before an admin first starts the competition are **preflight rounds**. They run the same checks a scored round would (inactive teams, disabled team services and checks without seeded credentials are skipped alike), but their results are stored apart and never count toward points, uptime or SLAs. Leave `StartPaused = false` to have preflight rounds run as soon as the engine is up. `GET /api/engine/preflight` returns the latest preflight round, including one that collected no results, so you can confirm every team is green before handing over; it is admin-only because the stored error and debug text can contain credentials. Stopping the competition after it has started does not bring preflight rounds back; `GET /api/engine/reset` does, since it also forgets that the competition started. In custom checks the `ROUND` placeholder is the preflight round number during preflight and the scored round number afterwards, so both start at 1. #### UI Settings diff --git a/docs/custom-checks.md b/docs/custom-checks.md index 9e0b47f0..b1360fc3 100644 --- a/docs/custom-checks.md +++ b/docs/custom-checks.md @@ -8,7 +8,7 @@ Custom checks are defined under a `[[box.custom]]` section in `event.conf`. The | Placeholder | Description | |-------------|-------------| -| `ROUND` | Current round number | +| `ROUND` | Current round number. During preflight rounds (before the competition is started) this is the preflight round number, which also starts at 1, so a script must not assume it is unique across preflight and scored rounds | | `TARGET` | Hostname or IP address (with team identifier substituted) | | `TEAMIDENTIFIER` | The team's unique identifier (e.g., "01") | | `USERNAME` | A username from the configured credlists | diff --git a/engine/db/competition.go b/engine/db/competition.go index c4b607c2..7886fafc 100644 --- a/engine/db/competition.go +++ b/engine/db/competition.go @@ -66,3 +66,18 @@ func (d *DB) GetCompetitionStart() (*time.Time, error) { } return state.StartedAt, nil } + +// CompetitionEverStarted is true once the competition has been started at +// least once, even if it was stopped since. Rows written before StartedAt +// existed only carry Started, so both are consulted. +func (d *DB) CompetitionEverStarted() (bool, error) { + var state CompetitionStateSchema + result := d.db.First(&state) + if result.Error != nil { + if errors.Is(result.Error, gorm.ErrRecordNotFound) { + return false, nil + } + return false, result.Error + } + return state.Started || state.StartedAt != nil, nil +} diff --git a/engine/db/db.go b/engine/db/db.go index 4fe582f6..b03deb60 100644 --- a/engine/db/db.go +++ b/engine/db/db.go @@ -56,7 +56,7 @@ func (d *DB) migrate() { &VulnSchema{}, &BoxSchema{}, &VectorSchema{}, &AttackSchema{}, &CompetitionStateSchema{}, // credential schemas for PCR management &OriginalCredentialSchema{}, &CredentialSchema{}, &PCRHistorySchema{}, - &PreflightCheckSchema{}) + &PreflightRoundSchema{}, &PreflightCheckSchema{}) if err != nil { log.Fatalln("Failed to auto migrate:", err) } @@ -148,9 +148,11 @@ func (d *DB) AddTeams(conf *config.ConfigSettings) error { return nil } +// ResetScores returns the event to the state before any round ran: scored and +// preflight rounds are dropped and the competition is no longer started, so the +// engine runs preflight rounds again. func (d *DB) ResetScores() error { - // truncate servicecheckschemas, slaschemas, and roundschemas with cascade - if err := d.db.Exec("TRUNCATE TABLE service_check_schemas, round_schemas, sla_schemas CASCADE").Error; err != nil { + if err := d.db.Exec("TRUNCATE TABLE service_check_schemas, round_schemas, sla_schemas, preflight_check_schemas, preflight_round_schemas, competition_state_schemas CASCADE").Error; err != nil { return err } diff --git a/engine/db/preflight.go b/engine/db/preflight.go index 1e8bdf8b..8b48d40b 100644 --- a/engine/db/preflight.go +++ b/engine/db/preflight.go @@ -1,14 +1,22 @@ package db import ( + "errors" "time" + + "gorm.io/gorm" ) // Preflight rounds are kept apart from round_schemas so they never count toward points, uptime or SLAs. +type PreflightRoundSchema struct { + ID uint + StartTime time.Time + Checks []PreflightCheckSchema `gorm:"foreignKey:RoundID"` +} + type PreflightCheckSchema struct { ID uint `gorm:"primarykey"` RoundID uint `gorm:"index"` - StartTime time.Time TeamID uint Team TeamSchema ServiceName string @@ -18,21 +26,26 @@ type PreflightCheckSchema struct { Debug string } -func (d *DB) CreatePreflightChecks(checks []PreflightCheckSchema) error { - return d.db.Create(&checks).Error +func (d *DB) CreatePreflightRound(round PreflightRoundSchema) error { + return d.db.Create(&round).Error } -func (d *DB) GetLastPreflightRound() (uint, error) { +// GetLastPreflightRoundID is 0 when no preflight round has been stored. +func (d *DB) GetLastPreflightRoundID() (uint, error) { var last uint - err := d.db.Model(&PreflightCheckSchema{}).Select("COALESCE(MAX(round_id), 0)").Scan(&last).Error + err := d.db.Model(&PreflightRoundSchema{}).Select("COALESCE(MAX(id), 0)").Scan(&last).Error return last, err } -func (d *DB) GetLatestPreflightChecks() ([]PreflightCheckSchema, error) { - var checks []PreflightCheckSchema - err := d.db.Preload("Team"). - Where("round_id = (?)", d.db.Model(&PreflightCheckSchema{}).Select("MAX(round_id)")). - Order("team_id, service_name"). - Find(&checks).Error - return checks, err +// GetLatestPreflightRound returns the zero round when none has been stored. +func (d *DB) GetLatestPreflightRound() (PreflightRoundSchema, error) { + var round PreflightRoundSchema + err := d.db. + Preload("Checks", func(tx *gorm.DB) *gorm.DB { return tx.Order("team_id, service_name") }). + Preload("Checks.Team"). + Order("id desc").First(&round).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return PreflightRoundSchema{}, nil + } + return round, err } diff --git a/engine/engine.go b/engine/engine.go index 2e95512e..b5cc88ea 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -32,7 +32,6 @@ type ScoringEngine struct { uptimeMu sync.RWMutex SlaPerService map[uint]map[string]int CurrentRound uint - PreflightRound uint // last preflight round run; see preflight() NextRoundStartTime time.Time CurrentRoundStartTime time.Time RedisClient *redis.Client @@ -104,11 +103,6 @@ func (se *ScoringEngine) Start() { } else { se.CurrentRound = uint(t.ID) + 1 } - if last, err := se.DB.GetLastPreflightRound(); err != nil { - slog.Error("failed to get last preflight round", "error", err) - } else { - se.PreflightRound = last - } se.uptimeMu.Lock() if err := se.DB.LoadUptimes(&se.UptimePerService); err != nil { @@ -150,20 +144,20 @@ func (se *ScoringEngine) Start() { default: } - preflight := se.Config.RequiredSettings.EventType == "rvb" && !se.competitionEverStarted() + started, err := se.DB.CompetitionEverStarted() + if err != nil { + slog.Error("failed to read competition start", "error", err) + return + } se.CurrentRoundStartTime = time.Now() se.NextRoundStartTime = time.Now().Add(time.Duration(se.Config.MiscSettings.Delay) * time.Second) - var err error switch { - case preflight: - slog.Info("Starting preflight round", "preflight_round", se.PreflightRound+1) + case se.Config.RequiredSettings.EventType == "rvb" && !started: err = se.preflight() case se.Config.RequiredSettings.EventType == "koth": - slog.Info("Starting round", "round", se.CurrentRound) err = se.koth() case se.Config.RequiredSettings.EventType == "rvb": - slog.Info("Starting round", "round", se.CurrentRound) err = se.rvb() default: slog.Error("Unknown event type", "eventType", se.Config.RequiredSettings.EventType) @@ -173,13 +167,6 @@ func (se *ScoringEngine) Start() { return } - if preflight { - slog.Info(fmt.Sprintf("Preflight round %d complete", se.PreflightRound)) - } else { - slog.Info(fmt.Sprintf("Round %d complete", se.CurrentRound)) - se.CurrentRound++ - se.RedisClient.Publish(context.Background(), "events", "round_finish") - } slog.Info(fmt.Sprintf("Next round will start in %s, sleeping...", time.Until(se.NextRoundStartTime).String())) select { case <-stop: @@ -339,16 +326,6 @@ func (se *ScoringEngine) Paused() bool { return se.pausedLocked() } -// Stopping the competition after it starts doesn't bring preflight rounds back. -func (se *ScoringEngine) competitionEverStarted() bool { - start, err := se.DB.GetCompetitionStart() - if err != nil { - slog.Error("failed to read competition start; scoring the round", "error", err) - return true - } - return start != nil -} - // ResetScores resets the engine to the initial state and stops the engine func (se *ScoringEngine) ResetScores() error { slog.Info("Resetting scores and clearing Redis queues") @@ -387,35 +364,47 @@ func (se *ScoringEngine) koth() error { // enginePauseWg.Wait() // do koth stuff + se.finishScoredRound() return nil } +// perform a round of rvb func (se *ScoringEngine) rvb() error { + slog.Info("Starting round", "round", se.CurrentRound) results, err := se.runChecks(se.CurrentRound, false) if err != nil { return err } se.processCollectedResults(results) + se.finishScoredRound() return nil } +func (se *ScoringEngine) finishScoredRound() { + slog.Info(fmt.Sprintf("Round %d complete", se.CurrentRound)) + se.CurrentRound++ + se.RedisClient.Publish(context.Background(), "events", "round_finish") +} + +// preflight runs the same checks as rvb but stores the results apart from +// scoring. The round row is written even when no results arrived so a timed-out +// round is visible to admins. func (se *ScoringEngine) preflight() error { - round := se.PreflightRound + 1 - results, err := se.runChecks(round, true) + last, err := se.DB.GetLastPreflightRoundID() + if err != nil { + return err + } + round := db.PreflightRoundSchema{ID: last + 1, StartTime: se.CurrentRoundStartTime} + slog.Info("Starting preflight round", "preflight_round", round.ID) + results, err := se.runChecks(round.ID, true) if err != nil { return err } - se.PreflightRound = round if len(results) == 0 { - slog.Warn("No results collected for preflight round", "preflight_round", round) - return nil + slog.Warn("No results collected for preflight round", "preflight_round", round.ID) } - - rows := make([]db.PreflightCheckSchema, 0, len(results)) for _, result := range results { - rows = append(rows, db.PreflightCheckSchema{ - RoundID: round, - StartTime: se.CurrentRoundStartTime, + round.Checks = append(round.Checks, db.PreflightCheckSchema{ TeamID: result.TeamID, ServiceName: sanitizeDBString(result.ServiceName), ServiceType: result.ServiceType, @@ -424,9 +413,10 @@ func (se *ScoringEngine) preflight() error { Debug: sanitizeDBString(result.Debug), }) } - if err := se.DB.CreatePreflightChecks(rows); err != nil { - slog.Error("failed to save preflight round", "preflight_round", round, "error", err) + if err := se.DB.CreatePreflightRound(round); err != nil { + slog.Error("failed to save preflight round", "preflight_round", round.ID, "error", err) } + slog.Info(fmt.Sprintf("Preflight round %d complete", round.ID)) return nil } diff --git a/engine/engine_test.go b/engine/engine_test.go index dba96a5e..d7b440d7 100644 --- a/engine/engine_test.go +++ b/engine/engine_test.go @@ -298,49 +298,9 @@ func TestRvb_EnqueuesTasksAndCollectsResults(t *testing.T) { }, } - // Start a goroutine that consumes tasks and pushes results - stopRunner := make(chan struct{}) - runnerDone := make(chan struct{}) - go func() { - defer close(runnerDone) - for { - select { - case <-stopRunner: - return - default: - val, err := redis.Client.BLPop(ctx, time.Second, "tasks").Result() - if err != nil { - continue - } - if len(val) < 2 { - continue - } - - var task Task - if err := json.Unmarshal([]byte(val[1]), &task); err != nil { - t.Logf("Failed to unmarshal task: %v", err) - continue - } - - // Push a successful result - result := checks.Result{ - TeamID: task.TeamID, - ServiceName: task.ServiceName, - ServiceType: task.ServiceType, - RoundID: task.RoundID, - Status: true, - Points: 10, - } - resultJSON, _ := json.Marshal(result) - redis.Client.RPush(ctx, "results", resultJSON) - } - } - }() - - // Run one round + stop := fakeRunner(t, redis, nil) err := engine.rvb() - close(stopRunner) - <-runnerDone + stop() require.NoError(t, err) @@ -446,53 +406,18 @@ func TestRvb_HandlesMultipleServices(t *testing.T) { }, } - // Runner goroutine - returns different points based on service - stopRunner := make(chan struct{}) - runnerDone := make(chan struct{}) - go func() { - defer close(runnerDone) - for { - select { - case <-stopRunner: - return - default: - val, err := redis.Client.BLPop(ctx, time.Second, "tasks").Result() - if err != nil { - continue - } - if len(val) < 2 { - continue - } - - var task Task - require.NoError(t, json.Unmarshal([]byte(val[1]), &task)) - - // Return points based on service name - points := 10 - switch task.ServiceName { - case "box01-ssh": - points = 5 - case "box01-dns": - points = 15 - } - - result := checks.Result{ - TeamID: task.TeamID, - ServiceName: task.ServiceName, - ServiceType: task.ServiceType, - RoundID: task.RoundID, - Status: true, - Points: points, - } - resultJSON, _ := json.Marshal(result) - redis.Client.RPush(ctx, "results", resultJSON) - } + stop := fakeRunner(t, redis, func(task Task) []checks.Result { + results := passResult(task) + switch task.ServiceName { + case "box01-ssh": + results[0].Points = 5 + case "box01-dns": + results[0].Points = 15 } - }() - + return results + }) err := engine.rvb() - close(stopRunner) - <-runnerDone + stop() require.NoError(t, err) @@ -656,35 +581,9 @@ func TestRvb_IgnoresStaleResultsFromRestartedRound(t *testing.T) { stale, _ := json.Marshal(checks.Result{TeamID: team.ID, ServiceName: "box01-web", ServiceType: "Mock", RoundID: 1, Status: false}) require.NoError(t, redis.Client.RPush(ctx, "results", stale).Err()) - stopRunner := make(chan struct{}) - runnerDone := make(chan struct{}) - go func() { - defer close(runnerDone) - for { - select { - case <-stopRunner: - return - default: - val, err := redis.Client.BLPop(ctx, time.Second, "tasks").Result() - if err != nil || len(val) < 2 { - continue - } - var task Task - if err := json.Unmarshal([]byte(val[1]), &task); err != nil { - continue - } - result, _ := json.Marshal(checks.Result{ - TeamID: task.TeamID, ServiceName: task.ServiceName, ServiceType: task.ServiceType, - RoundID: task.RoundID, Status: true, Points: 10, - }) - redis.Client.RPush(ctx, "results", result) - } - } - }() - + stop := fakeRunner(t, redis, nil) err := engine.rvb() - close(stopRunner) - <-runnerDone + stop() require.NoError(t, err) round, err := pg.DB.GetLastRound() diff --git a/engine/preflight_test.go b/engine/preflight_test.go index c0416d9d..a176c824 100644 --- a/engine/preflight_test.go +++ b/engine/preflight_test.go @@ -3,8 +3,8 @@ package engine import ( "context" "encoding/json" + "sync/atomic" "testing" - "time" "github.com/dbaseqp/Quotient/engine/checks" "github.com/dbaseqp/Quotient/engine/config" @@ -15,39 +15,44 @@ import ( "github.com/stretchr/testify/require" ) -func answerTasks(t *testing.T, redis *testutil.RedisContainer, seen chan<- Task, extra func(Task) []checks.Result) (stop func()) { +// passResult is the result fakeRunner pushes for a task when respond is nil. +func passResult(task Task) []checks.Result { + return []checks.Result{{ + TeamID: task.TeamID, ServiceName: task.ServiceName, ServiceType: task.ServiceType, + RoundID: task.RoundID, Preflight: task.Preflight, Status: true, Points: 10, + }} +} + +// fakeRunner answers every task on the "tasks" queue with respond(task) until +// stop is called. respond defaults to passResult. go-redis does not interrupt +// a blocking BLPop on context cancellation, so stop pushes a sentinel task that +// the goroutine consumes before it checks done. +func fakeRunner(t *testing.T, redis *testutil.RedisContainer, respond func(Task) []checks.Result) (stop func()) { t.Helper() + if respond == nil { + respond = passResult + } ctx := context.Background() done := make(chan struct{}) finished := make(chan struct{}) go func() { defer close(finished) for { + val, err := redis.Client.BLPop(ctx, 0, "tasks").Result() select { case <-done: return default: } - val, err := redis.Client.BLPop(ctx, time.Second, "tasks").Result() if err != nil || len(val) < 2 { continue } var task Task if err := json.Unmarshal([]byte(val[1]), &task); err != nil { + t.Logf("Failed to unmarshal task: %v", err) continue } - if seen != nil { - seen <- task - } - results := []checks.Result{} - if extra != nil { - results = extra(task) - } - results = append(results, checks.Result{ - TeamID: task.TeamID, ServiceName: task.ServiceName, ServiceType: task.ServiceType, - RoundID: task.RoundID, Preflight: task.Preflight, Status: true, Points: 10, - }) - for _, r := range results { + for _, r := range respond(task) { payload, _ := json.Marshal(r) redis.Client.RPush(ctx, "results", payload) } @@ -55,6 +60,7 @@ func answerTasks(t *testing.T, redis *testutil.RedisContainer, seen chan<- Task, }() return func() { close(done) + redis.Client.RPush(ctx, "tasks", "stop") <-finished } } @@ -62,25 +68,22 @@ func answerTasks(t *testing.T, redis *testutil.RedisContainer, seen chan<- Task, func newPreflightEngine(t *testing.T, redis *testutil.RedisContainer, pg *testutil.PostgresContainer) *ScoringEngine { t.Helper() se := NewTestEngine(t, redis, pg, 3) - last, err := pg.DB.GetLastPreflightRound() - require.NoError(t, err) - se.PreflightRound = last svc := &mockRunner{Service: checks.Service{Display: "web", Name: "box01-web", ServiceType: "Mock", Points: 10}} se.Config.Box = []config.Box{{Name: "box01", IP: "10.0.0.1", Runners: []checks.Runner{svc}}} return se } -func teamPreflight(t *testing.T, pg *testutil.PostgresContainer, teamID uint) []db.PreflightCheckSchema { +func teamPreflight(t *testing.T, pg *testutil.PostgresContainer, teamID uint) (db.PreflightRoundSchema, []db.PreflightCheckSchema) { t.Helper() - rows, err := pg.DB.GetLatestPreflightChecks() + round, err := pg.DB.GetLatestPreflightRound() require.NoError(t, err) var mine []db.PreflightCheckSchema - for _, row := range rows { + for _, row := range round.Checks { if row.TeamID == teamID { mine = append(mine, row) } } - return mine + return round, mine } func TestPreflight_StoresResultsApartFromScoring(t *testing.T) { @@ -95,23 +98,25 @@ func TestPreflight_StoresResultsApartFromScoring(t *testing.T) { team := pg.CreateTestTeam(t, "Team Preflight", "01") se := newPreflightEngine(t, redis, pg) - firstRound := se.PreflightRound + 1 - seen := make(chan Task, 100) - stop := answerTasks(t, redis, seen, nil) + var scoredTasks atomic.Int64 + stop := fakeRunner(t, redis, func(task Task) []checks.Result { + if !task.Preflight { + scoredTasks.Add(1) + } + return passResult(task) + }) require.NoError(t, se.preflight()) require.NoError(t, se.preflight()) stop() - close(seen) - for task := range seen { - assert.True(t, task.Preflight, "preflight tasks are marked for the runner") - } + assert.Zero(t, scoredTasks.Load(), "preflight tasks are marked for the runner") - assert.Equal(t, firstRound+1, se.PreflightRound) - rows := teamPreflight(t, pg, team.ID) - require.Len(t, rows, 1, "only the latest preflight round is returned") - assert.Equal(t, firstRound+1, rows[0].RoundID) + round, rows := teamPreflight(t, pg, team.ID) + assert.Equal(t, uint(2), round.ID, "only the latest preflight round is returned") + assert.Equal(t, se.CurrentRoundStartTime.Unix(), round.StartTime.Unix()) + require.Len(t, rows, 1) + assert.Equal(t, round.ID, rows[0].RoundID) assert.True(t, rows[0].Result) assert.Equal(t, "box01-web", rows[0].ServiceName) assert.Equal(t, "Mock", rows[0].ServiceType) @@ -138,33 +143,111 @@ func TestPreflight_IgnoresScoredRoundResults(t *testing.T) { team := pg.CreateTestTeam(t, "Team Preflight Mixed", "01") se := newPreflightEngine(t, redis, pg) - stop := answerTasks(t, redis, nil, func(task Task) []checks.Result { - return []checks.Result{{ + stop := fakeRunner(t, redis, func(task Task) []checks.Result { + scored := checks.Result{ TeamID: task.TeamID, ServiceName: task.ServiceName, ServiceType: task.ServiceType, RoundID: task.RoundID, Preflight: false, Status: false, - }} + } + return append([]checks.Result{scored}, passResult(task)...) }) require.NoError(t, se.preflight()) stop() - rows := teamPreflight(t, pg, team.ID) + _, rows := teamPreflight(t, pg, team.ID) require.Len(t, rows, 1) assert.True(t, rows[0].Result, "a scored round's result must not be stored as preflight") } -func TestCompetitionStarted_StaysStartedAfterStop(t *testing.T) { +func TestPreflight_StoresRoundWithoutResults(t *testing.T) { if testing.Short() { t.Skip("skipping integration test in short mode") } redis, pg := testutil.StartContainers(t) - pg.ResetCompetitionState(t) - t.Cleanup(func() { pg.ResetCompetitionState(t) }) - se := NewTestEngine(t, redis, pg, 3) + redis.Client.FlushDB(context.Background()) + require.NoError(t, pg.DB.ResetScores()) + + pg.CreateTestTeam(t, "Team Preflight Timeout", "01") + se := newPreflightEngine(t, redis, pg) + se.Config.MiscSettings.Delay = 2 + + // No runner answers, so the round times out with nothing collected. + require.NoError(t, se.preflight()) + + round, err := pg.DB.GetLatestPreflightRound() + require.NoError(t, err) + assert.Equal(t, uint(1), round.ID, "a round that collected nothing is still the latest round") + assert.Empty(t, round.Checks) +} + +func TestCompetitionEverStarted_StaysStartedAfterStop(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + _, pg := testutil.StartContainers(t) + require.NoError(t, pg.DB.ResetScores()) + t.Cleanup(func() { require.NoError(t, pg.DB.ResetScores()) }) + + started, err := pg.DB.CompetitionEverStarted() + require.NoError(t, err) + assert.False(t, started) - assert.False(t, se.competitionEverStarted()) require.NoError(t, pg.DB.SetCompetitionStarted(true)) - assert.True(t, se.competitionEverStarted()) + started, err = pg.DB.CompetitionEverStarted() + require.NoError(t, err) + assert.True(t, started) + require.NoError(t, pg.DB.SetCompetitionStarted(false)) - assert.True(t, se.competitionEverStarted(), "stopping after the first start doesn't return to preflight") + started, err = pg.DB.CompetitionEverStarted() + require.NoError(t, err) + assert.True(t, started, "stopping after the first start doesn't return to preflight") +} + +func TestCompetitionEverStarted_LegacyRowWithoutStartedAt(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + _, pg := testutil.StartContainers(t) + require.NoError(t, pg.DB.ResetScores()) + t.Cleanup(func() { require.NoError(t, pg.DB.ResetScores()) }) + + // Rows written before StartedAt existed: the UI treats them as started, so + // the engine must not run preflight rounds against them. + require.NoError(t, pg.DB.SetCompetitionStarted(true)) + pg.Exec(t, "UPDATE competition_state_schemas SET started_at = NULL") + + started, err := pg.DB.CompetitionEverStarted() + require.NoError(t, err) + assert.True(t, started) + assert.True(t, pg.DB.GetCompetitionStarted()) +} + +func TestResetScores_ReentersPreflight(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + redis, pg := testutil.StartContainers(t) + redis.Client.FlushDB(context.Background()) + require.NoError(t, pg.DB.ResetScores()) + t.Cleanup(func() { require.NoError(t, pg.DB.ResetScores()) }) + + pg.CreateTestTeam(t, "Team Preflight Reset", "01") + se := newPreflightEngine(t, redis, pg) + + stop := fakeRunner(t, redis, nil) + require.NoError(t, se.preflight()) + stop() + require.NoError(t, pg.DB.SetCompetitionStarted(true)) + + require.NoError(t, se.ResetScores()) + + started, err := pg.DB.CompetitionEverStarted() + require.NoError(t, err) + assert.False(t, started, "reset returns the engine to preflight rounds") + round, err := pg.DB.GetLatestPreflightRound() + require.NoError(t, err) + assert.Zero(t, round.ID, "reset drops the previous event's preflight rounds") } diff --git a/runner/runner.go b/runner/runner.go index 99b96bc9..0ef36560 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -203,14 +203,11 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta // Run the check in a goroutine go runner.Run(task.TeamID, task.TeamIdentifier, task.RoundID, resultsChan) - // Wait for either result or deadline + // Wait for either result or deadline. Only the fields a check produces + // are taken from it; task identity and runner metadata stay as set above. select { - case result = <-resultsChan: - result.TeamID = task.TeamID - result.ServiceName = task.ServiceName - result.ServiceType = task.ServiceType - result.RoundID = task.RoundID - result.Preflight = task.Preflight + case r := <-resultsChan: + result.Status, result.Error, result.Debug, result.Points, result.Target = r.Status, r.Error, r.Debug, r.Points, r.Target slog.Info("check result received", "round_id", result.RoundID, "team_id", result.TeamID, "service_type", result.ServiceType, "status", result.Status, "debug", result.Debug, "error", result.Error) @@ -219,11 +216,6 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta result.Status = false result.Debug = "round ended before check completed" result.Error = "timeout" - result.TeamID = task.TeamID - result.ServiceName = task.ServiceName - result.ServiceType = task.ServiceType - result.RoundID = task.RoundID - result.Preflight = task.Preflight slog.Warn("check timed out", "round_id", task.RoundID, "team_id", task.TeamID, "service_type", task.ServiceType) @@ -238,7 +230,8 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta } } - // Marshal and store result + result.EndTime = time.Now().Format(time.RFC3339) + result.StatusText = map[bool]string{true: "success", false: "failed"}[result.Status] resultJSON, err := json.Marshal(result) if err != nil { slog.Error("failed to marshal result", "error", err) @@ -253,12 +246,8 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta slog.Info("successfully pushed result", "round_id", result.RoundID, "team_id", result.TeamID, "service_type", result.ServiceType, "status", result.Status) - // Update the task key with the final result status - result.EndTime = time.Now().Format(time.RFC3339) - result.StatusText = map[bool]string{true: "success", false: "failed"}[result.Status] - statusJSON, _ = json.Marshal(result) // Use a longer TTL for completed tasks to ensure they're visible in the UI - rdb.Set(ctx, taskKey, statusJSON, 3*time.Minute) + rdb.Set(ctx, taskKey, resultJSON, 3*time.Minute) } // waitForRetry waits delay before the next attempt and reports whether that diff --git a/tests/integration/graphs_test.go b/tests/integration/graphs_test.go index e9ae8e6d..1d56127c 100644 --- a/tests/integration/graphs_test.go +++ b/tests/integration/graphs_test.go @@ -52,7 +52,7 @@ func TestGraphsMarkOnlyCallersOwnSeries(t *testing.T) { eng := engine.NewTestEngine(t, redis, pg, 3) a := api.NewAPI(eng.Config, eng) require.NoError(t, pg.DB.SetCompetitionStarted(true)) - t.Cleanup(func() { pg.ResetCompetitionState(t) }) + t.Cleanup(func() { require.NoError(t, pg.DB.ResetScores()) }) suffix := time.Now().UnixNano() teams := make([]db.TeamSchema, 3) diff --git a/tests/testutil/testutil.go b/tests/testutil/testutil.go index 40e58ce0..60427279 100644 --- a/tests/testutil/testutil.go +++ b/tests/testutil/testutil.go @@ -72,16 +72,15 @@ func startPostgres(t *testing.T) *PostgresContainer { return &PostgresContainer{DB: db.Connect(connString), connString: connString} } -// ResetCompetitionState forgets that the competition ever started, which the -// DB layer never does on its own: StartedAt is kept once set. -func (p *PostgresContainer) ResetCompetitionState(t *testing.T) { +// Exec runs raw SQL for test setups the DB layer has no API for. +func (p *PostgresContainer) Exec(t *testing.T, sql string) { t.Helper() conn, err := gorm.Open(postgres.Open(p.connString), &gorm.Config{}) require.NoError(t, err) sqlDB, err := conn.DB() require.NoError(t, err) defer sqlDB.Close() //nolint:errcheck - require.NoError(t, conn.Exec("DELETE FROM competition_state_schemas").Error) + require.NoError(t, conn.Exec(sql).Error) } // testTeamCounter provides unique team IDs across test runs diff --git a/www/api/admin.go b/www/api/admin.go index 50e92c79..211f2231 100644 --- a/www/api/admin.go +++ b/www/api/admin.go @@ -165,10 +165,10 @@ func (a *API) GetEngine(w http.ResponseWriter, r *http.Request) { } func (a *API) GetPreflight(w http.ResponseWriter, r *http.Request) { - rows, err := a.eng.DB.GetLatestPreflightChecks() + round, err := a.eng.DB.GetLatestPreflightRound() if err != nil { - slog.Error("failed to get preflight checks", "error", err) - WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": "Failed to retrieve preflight checks"}) + slog.Error("failed to get preflight round", "error", err) + WriteJSON(w, http.StatusInternalServerError, map[string]any{"error": "Failed to retrieve preflight round"}) return } @@ -185,9 +185,11 @@ func (a *API) GetPreflight(w http.ResponseWriter, r *http.Request) { Round uint `json:"round"` StartTime *time.Time `json:"start_time"` Checks []check `json:"checks"` - }{Checks: make([]check, 0, len(rows))} - for _, row := range rows { - resp.Round, resp.StartTime = row.RoundID, &row.StartTime + }{Round: round.ID, Checks: make([]check, 0, len(round.Checks))} + if round.ID != 0 { + resp.StartTime = &round.StartTime + } + for _, row := range round.Checks { resp.Checks = append(resp.Checks, check{row.TeamID, row.Team.Name, row.ServiceName, row.ServiceType, row.Result, row.Error, row.Debug}) } WriteJSON(w, http.StatusOK, resp) From e3af4278df98d563cd7e954c17fed90002073776 Mon Sep 17 00:00:00 2001 From: Tirefire <84106878+tire-fire@users.noreply.github.com> Date: Mon, 9 Nov 2026 06:47:40 +0000 Subject: [PATCH 3/5] Address round-2 review findings --- README.md | 2 +- engine/db/competition.go | 15 ------- engine/db/db.go | 8 ++-- engine/engine.go | 30 +++++++------ engine/preflight_test.go | 75 +++++++++++++++++++------------- tests/integration/graphs_test.go | 5 ++- 6 files changed, 70 insertions(+), 65 deletions(-) diff --git a/README.md b/README.md index d2c8e5e2..84e6a3e6 100644 --- a/README.md +++ b/README.md @@ -271,7 +271,7 @@ AttemptDelay = 0 # Seconds to wait before retrying a failed attempt ( Every attempt, plus the waits between them, must fit in the shortest round: `Attempts * Timeout + (Attempts - 1) * AttemptDelay` must be less than `Delay - Jitter`. -In rvb events, rounds that run before an admin first starts the competition are **preflight rounds**. They run the same checks a scored round would (inactive teams, disabled team services and checks without seeded credentials are skipped alike), but their results are stored apart and never count toward points, uptime or SLAs. Leave `StartPaused = false` to have preflight rounds run as soon as the engine is up. `GET /api/engine/preflight` returns the latest preflight round, including one that collected no results, so you can confirm every team is green before handing over; it is admin-only because the stored error and debug text can contain credentials. Stopping the competition after it has started does not bring preflight rounds back; `GET /api/engine/reset` does, since it also forgets that the competition started. In custom checks the `ROUND` placeholder is the preflight round number during preflight and the scored round number afterwards, so both start at 1. +In rvb events, rounds that run while the competition is not started and no scored round exists yet are **preflight rounds**. They run the same checks a scored round would (inactive teams, disabled team services and checks without seeded credentials are skipped alike), but their results are stored apart and never count toward points, uptime or SLAs. Leave `StartPaused = false` to have preflight rounds run as soon as the engine is up. `GET /api/engine/preflight` returns the latest preflight round, including one that collected no results, so you can confirm every team is green before handing over; it is admin-only because the stored error and debug text can contain credentials. Stopping a competition that has already scored rounds does not bring preflight rounds back; `GET /api/engine/reset` while stopped (or stopping after a reset) does, since it drops the scored rounds. Injects keep their anchor: a reset does not forget when the competition first started. In custom checks the `ROUND` placeholder is the preflight round number during preflight and the scored round number afterwards, so both start at 1. #### UI Settings diff --git a/engine/db/competition.go b/engine/db/competition.go index 7886fafc..c4b607c2 100644 --- a/engine/db/competition.go +++ b/engine/db/competition.go @@ -66,18 +66,3 @@ func (d *DB) GetCompetitionStart() (*time.Time, error) { } return state.StartedAt, nil } - -// CompetitionEverStarted is true once the competition has been started at -// least once, even if it was stopped since. Rows written before StartedAt -// existed only carry Started, so both are consulted. -func (d *DB) CompetitionEverStarted() (bool, error) { - var state CompetitionStateSchema - result := d.db.First(&state) - if result.Error != nil { - if errors.Is(result.Error, gorm.ErrRecordNotFound) { - return false, nil - } - return false, result.Error - } - return state.Started || state.StartedAt != nil, nil -} diff --git a/engine/db/db.go b/engine/db/db.go index b03deb60..0fc6261a 100644 --- a/engine/db/db.go +++ b/engine/db/db.go @@ -148,11 +148,11 @@ func (d *DB) AddTeams(conf *config.ConfigSettings) error { return nil } -// ResetScores returns the event to the state before any round ran: scored and -// preflight rounds are dropped and the competition is no longer started, so the -// engine runs preflight rounds again. +// ResetScores drops scored and preflight rounds so the engine re-enters +// preflight once the competition is stopped. Competition state is kept: the +// start anchor of imported injects survives a reset. func (d *DB) ResetScores() error { - if err := d.db.Exec("TRUNCATE TABLE service_check_schemas, round_schemas, sla_schemas, preflight_check_schemas, preflight_round_schemas, competition_state_schemas CASCADE").Error; err != nil { + if err := d.db.Exec("TRUNCATE TABLE service_check_schemas, round_schemas, sla_schemas, preflight_check_schemas, preflight_round_schemas CASCADE").Error; err != nil { return err } diff --git a/engine/engine.go b/engine/engine.go index b5cc88ea..15d11460 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -144,21 +144,19 @@ func (se *ScoringEngine) Start() { default: } - started, err := se.DB.CompetitionEverStarted() - if err != nil { - slog.Error("failed to read competition start", "error", err) - return - } se.CurrentRoundStartTime = time.Now() se.NextRoundStartTime = time.Now().Add(time.Duration(se.Config.MiscSettings.Delay) * time.Second) - switch { - case se.Config.RequiredSettings.EventType == "rvb" && !started: - err = se.preflight() - case se.Config.RequiredSettings.EventType == "koth": + var err error + switch se.Config.RequiredSettings.EventType { + case "koth": err = se.koth() - case se.Config.RequiredSettings.EventType == "rvb": - err = se.rvb() + case "rvb": + if se.preflightPending() { + err = se.preflight() + } else { + err = se.rvb() + } default: slog.Error("Unknown event type", "eventType", se.Config.RequiredSettings.EventType) } @@ -386,9 +384,15 @@ func (se *ScoringEngine) finishScoredRound() { se.RedisClient.Publish(context.Background(), "events", "round_finish") } +// preflightPending is true while the competition is not started and no scored +// round exists. GetCompetitionStarted reports a read error as not started, +// which fails closed into an unscored preflight round. +func (se *ScoringEngine) preflightPending() bool { + return se.CurrentRound == 1 && !se.DB.GetCompetitionStarted() +} + // preflight runs the same checks as rvb but stores the results apart from -// scoring. The round row is written even when no results arrived so a timed-out -// round is visible to admins. +// scoring. func (se *ScoringEngine) preflight() error { last, err := se.DB.GetLastPreflightRoundID() if err != nil { diff --git a/engine/preflight_test.go b/engine/preflight_test.go index a176c824..bed7bd62 100644 --- a/engine/preflight_test.go +++ b/engine/preflight_test.go @@ -3,8 +3,10 @@ package engine import ( "context" "encoding/json" + "sync" "sync/atomic" "testing" + "time" "github.com/dbaseqp/Quotient/engine/checks" "github.com/dbaseqp/Quotient/engine/config" @@ -15,7 +17,6 @@ import ( "github.com/stretchr/testify/require" ) -// passResult is the result fakeRunner pushes for a task when respond is nil. func passResult(task Task) []checks.Result { return []checks.Result{{ TeamID: task.TeamID, ServiceName: task.ServiceName, ServiceType: task.ServiceType, @@ -24,8 +25,8 @@ func passResult(task Task) []checks.Result { } // fakeRunner answers every task on the "tasks" queue with respond(task) until -// stop is called. respond defaults to passResult. go-redis does not interrupt -// a blocking BLPop on context cancellation, so stop pushes a sentinel task that +// stop is called (also at test cleanup). go-redis does not interrupt a +// blocking BLPop on context cancellation, so stop pushes a sentinel task that // the goroutine consumes before it checks done. func fakeRunner(t *testing.T, redis *testutil.RedisContainer, respond func(Task) []checks.Result) (stop func()) { t.Helper() @@ -58,11 +59,16 @@ func fakeRunner(t *testing.T, redis *testutil.RedisContainer, respond func(Task) } } }() - return func() { - close(done) - redis.Client.RPush(ctx, "tasks", "stop") - <-finished + var once sync.Once + stop = func() { + once.Do(func() { + close(done) + redis.Client.RPush(ctx, "tasks", "stop") + <-finished + }) } + t.Cleanup(stop) + return stop } func newPreflightEngine(t *testing.T, redis *testutil.RedisContainer, pg *testutil.PostgresContainer) *ScoringEngine { @@ -106,6 +112,7 @@ func TestPreflight_StoresResultsApartFromScoring(t *testing.T) { } return passResult(task) }) + se.CurrentRoundStartTime = time.Now().Truncate(time.Microsecond) require.NoError(t, se.preflight()) require.NoError(t, se.preflight()) stop() @@ -114,7 +121,7 @@ func TestPreflight_StoresResultsApartFromScoring(t *testing.T) { round, rows := teamPreflight(t, pg, team.ID) assert.Equal(t, uint(2), round.ID, "only the latest preflight round is returned") - assert.Equal(t, se.CurrentRoundStartTime.Unix(), round.StartTime.Unix()) + assert.True(t, se.CurrentRoundStartTime.Equal(round.StartTime), "round start time %s stored as %s", se.CurrentRoundStartTime, round.StartTime) require.Len(t, rows, 1) assert.Equal(t, round.ID, rows[0].RoundID) assert.True(t, rows[0].Result) @@ -180,48 +187,45 @@ func TestPreflight_StoresRoundWithoutResults(t *testing.T) { assert.Empty(t, round.Checks) } -func TestCompetitionEverStarted_StaysStartedAfterStop(t *testing.T) { +func TestPreflightPending_StopReturnsToPreflightOnlyBeforeScoring(t *testing.T) { if testing.Short() { t.Skip("skipping integration test in short mode") } - _, pg := testutil.StartContainers(t) + redis, pg := testutil.StartContainers(t) require.NoError(t, pg.DB.ResetScores()) - t.Cleanup(func() { require.NoError(t, pg.DB.ResetScores()) }) + t.Cleanup(func() { pg.Exec(t, "DELETE FROM competition_state_schemas") }) + se := newPreflightEngine(t, redis, pg) - started, err := pg.DB.CompetitionEverStarted() - require.NoError(t, err) - assert.False(t, started) + assert.True(t, se.preflightPending()) require.NoError(t, pg.DB.SetCompetitionStarted(true)) - started, err = pg.DB.CompetitionEverStarted() - require.NoError(t, err) - assert.True(t, started) + assert.False(t, se.preflightPending()) + se.CurrentRound = 2 require.NoError(t, pg.DB.SetCompetitionStarted(false)) - started, err = pg.DB.CompetitionEverStarted() - require.NoError(t, err) - assert.True(t, started, "stopping after the first start doesn't return to preflight") + assert.False(t, se.preflightPending(), "stopping after a scored round doesn't return to preflight") + + se.CurrentRound = 1 + assert.True(t, se.preflightPending(), "stopping before any scored round returns to preflight") } -func TestCompetitionEverStarted_LegacyRowWithoutStartedAt(t *testing.T) { +func TestPreflightPending_LegacyRowWithoutStartedAt(t *testing.T) { if testing.Short() { t.Skip("skipping integration test in short mode") } - _, pg := testutil.StartContainers(t) + redis, pg := testutil.StartContainers(t) require.NoError(t, pg.DB.ResetScores()) - t.Cleanup(func() { require.NoError(t, pg.DB.ResetScores()) }) + t.Cleanup(func() { pg.Exec(t, "DELETE FROM competition_state_schemas") }) + se := newPreflightEngine(t, redis, pg) // Rows written before StartedAt existed: the UI treats them as started, so // the engine must not run preflight rounds against them. require.NoError(t, pg.DB.SetCompetitionStarted(true)) pg.Exec(t, "UPDATE competition_state_schemas SET started_at = NULL") - started, err := pg.DB.CompetitionEverStarted() - require.NoError(t, err) - assert.True(t, started) - assert.True(t, pg.DB.GetCompetitionStarted()) + assert.False(t, se.preflightPending()) } func TestResetScores_ReentersPreflight(t *testing.T) { @@ -232,21 +236,30 @@ func TestResetScores_ReentersPreflight(t *testing.T) { redis, pg := testutil.StartContainers(t) redis.Client.FlushDB(context.Background()) require.NoError(t, pg.DB.ResetScores()) - t.Cleanup(func() { require.NoError(t, pg.DB.ResetScores()) }) + t.Cleanup(func() { pg.Exec(t, "DELETE FROM competition_state_schemas") }) pg.CreateTestTeam(t, "Team Preflight Reset", "01") se := newPreflightEngine(t, redis, pg) stop := fakeRunner(t, redis, nil) require.NoError(t, se.preflight()) - stop() require.NoError(t, pg.DB.SetCompetitionStarted(true)) + startedAt, err := pg.DB.GetCompetitionStart() + require.NoError(t, err) + require.NotNil(t, startedAt) + require.NoError(t, se.rvb()) + stop() + assert.False(t, se.preflightPending()) + require.NoError(t, pg.DB.SetCompetitionStarted(false)) + assert.False(t, se.preflightPending(), "stopping alone doesn't drop the scored round") require.NoError(t, se.ResetScores()) + assert.True(t, se.preflightPending(), "reset while stopped returns the engine to preflight rounds") - started, err := pg.DB.CompetitionEverStarted() + afterReset, err := pg.DB.GetCompetitionStart() require.NoError(t, err) - assert.False(t, started, "reset returns the engine to preflight rounds") + require.NotNil(t, afterReset, "reset keeps the competition start, the anchor of imported injects") + assert.True(t, startedAt.Equal(*afterReset)) round, err := pg.DB.GetLatestPreflightRound() require.NoError(t, err) assert.Zero(t, round.ID, "reset drops the previous event's preflight rounds") diff --git a/tests/integration/graphs_test.go b/tests/integration/graphs_test.go index 1d56127c..cf556cad 100644 --- a/tests/integration/graphs_test.go +++ b/tests/integration/graphs_test.go @@ -52,7 +52,10 @@ func TestGraphsMarkOnlyCallersOwnSeries(t *testing.T) { eng := engine.NewTestEngine(t, redis, pg, 3) a := api.NewAPI(eng.Config, eng) require.NoError(t, pg.DB.SetCompetitionStarted(true)) - t.Cleanup(func() { require.NoError(t, pg.DB.ResetScores()) }) + t.Cleanup(func() { + require.NoError(t, pg.DB.ResetScores()) + pg.Exec(t, "DELETE FROM competition_state_schemas") + }) suffix := time.Now().UnixNano() teams := make([]db.TeamSchema, 3) From fd0dceaca54c749dc210eeeddfab40ac6076044b Mon Sep 17 00:00:00 2001 From: Tirefire <84106878+tire-fire@users.noreply.github.com> Date: Mon, 9 Nov 2026 06:59:34 +0000 Subject: [PATCH 4/5] Address round-3 review findings --- README.md | 2 +- docs/custom-checks.md | 2 +- engine/engine.go | 17 ++++- engine/engine_test.go | 9 +-- engine/preflight_test.go | 75 +++++++++++++++-------- tests/integration/graphs_test.go | 2 +- tests/integration/inject_schedule_test.go | 2 + tests/testutil/testutil.go | 7 +++ 8 files changed, 78 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 84e6a3e6..fcb4d8f4 100644 --- a/README.md +++ b/README.md @@ -271,7 +271,7 @@ AttemptDelay = 0 # Seconds to wait before retrying a failed attempt ( Every attempt, plus the waits between them, must fit in the shortest round: `Attempts * Timeout + (Attempts - 1) * AttemptDelay` must be less than `Delay - Jitter`. -In rvb events, rounds that run while the competition is not started and no scored round exists yet are **preflight rounds**. They run the same checks a scored round would (inactive teams, disabled team services and checks without seeded credentials are skipped alike), but their results are stored apart and never count toward points, uptime or SLAs. Leave `StartPaused = false` to have preflight rounds run as soon as the engine is up. `GET /api/engine/preflight` returns the latest preflight round, including one that collected no results, so you can confirm every team is green before handing over; it is admin-only because the stored error and debug text can contain credentials. Stopping a competition that has already scored rounds does not bring preflight rounds back; `GET /api/engine/reset` while stopped (or stopping after a reset) does, since it drops the scored rounds. Injects keep their anchor: a reset does not forget when the competition first started. In custom checks the `ROUND` placeholder is the preflight round number during preflight and the scored round number afterwards, so both start at 1. +In rvb events, rounds that run while the competition is not started and no scored round has been stored are **preflight rounds**. They run the same checks a scored round would (inactive teams, disabled team services and checks without seeded credentials are skipped alike), but their results are stored apart and never count toward points, uptime or SLAs. Leave `StartPaused = false` to have preflight rounds run as soon as the engine is up. `GET /api/engine/preflight` returns the latest preflight round, including one that collected no results, so you can confirm every team is green before handing over; it is admin-only because the stored error and debug text can contain credentials. Stopping a competition that has stored scored rounds does not bring preflight rounds back; `GET /api/engine/reset` while stopped (or stopping before the first scored round is stored) re-enters preflight, since the decision is made from the stored rounds. Injects keep their anchor: a reset does not forget when the competition first started. In custom checks the `ROUND` placeholder is the preflight round number during preflight and the scored round number afterwards, so both start at 1. #### UI Settings diff --git a/docs/custom-checks.md b/docs/custom-checks.md index b1360fc3..a5575208 100644 --- a/docs/custom-checks.md +++ b/docs/custom-checks.md @@ -8,7 +8,7 @@ Custom checks are defined under a `[[box.custom]]` section in `event.conf`. The | Placeholder | Description | |-------------|-------------| -| `ROUND` | Current round number. During preflight rounds (before the competition is started) this is the preflight round number, which also starts at 1, so a script must not assume it is unique across preflight and scored rounds | +| `ROUND` | Current round number. During preflight rounds (while the competition is not started and no scored round has been stored, so also after `GET /api/engine/reset` while stopped) this is the preflight round number, which also starts at 1, so a script must not assume it is unique across preflight and scored rounds | | `TARGET` | Hostname or IP address (with team identifier substituted) | | `TEAMIDENTIFIER` | The team's unique identifier (e.g., "01") | | `USERNAME` | A username from the configured credlists | diff --git a/engine/engine.go b/engine/engine.go index 15d11460..94ea533e 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -385,10 +385,21 @@ func (se *ScoringEngine) finishScoredRound() { } // preflightPending is true while the competition is not started and no scored -// round exists. GetCompetitionStarted reports a read error as not started, -// which fails closed into an unscored preflight round. +// round has been stored. Both facts are read from the DB every round rather +// than from CurrentRound, which also advances on rounds that stored nothing and +// is reseeded on restart. GetCompetitionStarted reports a read error as not +// started, and a GetLastRound error counts as no stored round: on a read error +// this iteration runs a preflight round, delaying scoring by one round. func (se *ScoringEngine) preflightPending() bool { - return se.CurrentRound == 1 && !se.DB.GetCompetitionStarted() + if se.DB.GetCompetitionStarted() { + return false + } + last, err := se.DB.GetLastRound() + if err != nil { + slog.Error("failed to get last round, running a preflight round", "error", err) + return true + } + return last.ID == 0 } // preflight runs the same checks as rvb but stores the results apart from diff --git a/engine/engine_test.go b/engine/engine_test.go index d7b440d7..ca228695 100644 --- a/engine/engine_test.go +++ b/engine/engine_test.go @@ -298,9 +298,8 @@ func TestRvb_EnqueuesTasksAndCollectsResults(t *testing.T) { }, } - stop := fakeRunner(t, redis, nil) + fakeRunner(t, redis, nil) err := engine.rvb() - stop() require.NoError(t, err) @@ -406,7 +405,7 @@ func TestRvb_HandlesMultipleServices(t *testing.T) { }, } - stop := fakeRunner(t, redis, func(task Task) []checks.Result { + fakeRunner(t, redis, func(task Task) []checks.Result { results := passResult(task) switch task.ServiceName { case "box01-ssh": @@ -417,7 +416,6 @@ func TestRvb_HandlesMultipleServices(t *testing.T) { return results }) err := engine.rvb() - stop() require.NoError(t, err) @@ -581,9 +579,8 @@ func TestRvb_IgnoresStaleResultsFromRestartedRound(t *testing.T) { stale, _ := json.Marshal(checks.Result{TeamID: team.ID, ServiceName: "box01-web", ServiceType: "Mock", RoundID: 1, Status: false}) require.NoError(t, redis.Client.RPush(ctx, "results", stale).Err()) - stop := fakeRunner(t, redis, nil) + fakeRunner(t, redis, nil) err := engine.rvb() - stop() require.NoError(t, err) round, err := pg.DB.GetLastRound() diff --git a/engine/preflight_test.go b/engine/preflight_test.go index bed7bd62..8bf1f473 100644 --- a/engine/preflight_test.go +++ b/engine/preflight_test.go @@ -3,7 +3,6 @@ package engine import ( "context" "encoding/json" - "sync" "sync/atomic" "testing" "time" @@ -25,10 +24,10 @@ func passResult(task Task) []checks.Result { } // fakeRunner answers every task on the "tasks" queue with respond(task) until -// stop is called (also at test cleanup). go-redis does not interrupt a -// blocking BLPop on context cancellation, so stop pushes a sentinel task that -// the goroutine consumes before it checks done. -func fakeRunner(t *testing.T, redis *testutil.RedisContainer, respond func(Task) []checks.Result) (stop func()) { +// test cleanup. go-redis does not interrupt a blocking BLPop on context +// cancellation, so cleanup pushes a sentinel task that the goroutine consumes +// before it checks done. +func fakeRunner(t *testing.T, redis *testutil.RedisContainer, respond func(Task) []checks.Result) { t.Helper() if respond == nil { respond = passResult @@ -59,16 +58,11 @@ func fakeRunner(t *testing.T, redis *testutil.RedisContainer, respond func(Task) } } }() - var once sync.Once - stop = func() { - once.Do(func() { - close(done) - redis.Client.RPush(ctx, "tasks", "stop") - <-finished - }) - } - t.Cleanup(stop) - return stop + t.Cleanup(func() { + close(done) + redis.Client.RPush(ctx, "tasks", "stop") + <-finished + }) } func newPreflightEngine(t *testing.T, redis *testutil.RedisContainer, pg *testutil.PostgresContainer) *ScoringEngine { @@ -106,7 +100,7 @@ func TestPreflight_StoresResultsApartFromScoring(t *testing.T) { se := newPreflightEngine(t, redis, pg) var scoredTasks atomic.Int64 - stop := fakeRunner(t, redis, func(task Task) []checks.Result { + fakeRunner(t, redis, func(task Task) []checks.Result { if !task.Preflight { scoredTasks.Add(1) } @@ -115,7 +109,6 @@ func TestPreflight_StoresResultsApartFromScoring(t *testing.T) { se.CurrentRoundStartTime = time.Now().Truncate(time.Microsecond) require.NoError(t, se.preflight()) require.NoError(t, se.preflight()) - stop() assert.Zero(t, scoredTasks.Load(), "preflight tasks are marked for the runner") @@ -150,7 +143,7 @@ func TestPreflight_IgnoresScoredRoundResults(t *testing.T) { team := pg.CreateTestTeam(t, "Team Preflight Mixed", "01") se := newPreflightEngine(t, redis, pg) - stop := fakeRunner(t, redis, func(task Task) []checks.Result { + fakeRunner(t, redis, func(task Task) []checks.Result { scored := checks.Result{ TeamID: task.TeamID, ServiceName: task.ServiceName, ServiceType: task.ServiceType, RoundID: task.RoundID, Preflight: false, Status: false, @@ -158,7 +151,6 @@ func TestPreflight_IgnoresScoredRoundResults(t *testing.T) { return append([]checks.Result{scored}, passResult(task)...) }) require.NoError(t, se.preflight()) - stop() _, rows := teamPreflight(t, pg, team.ID) require.Len(t, rows, 1) @@ -194,7 +186,8 @@ func TestPreflightPending_StopReturnsToPreflightOnlyBeforeScoring(t *testing.T) redis, pg := testutil.StartContainers(t) require.NoError(t, pg.DB.ResetScores()) - t.Cleanup(func() { pg.Exec(t, "DELETE FROM competition_state_schemas") }) + pg.ResetCompetitionState(t) + t.Cleanup(func() { pg.ResetCompetitionState(t) }) se := newPreflightEngine(t, redis, pg) assert.True(t, se.preflightPending()) @@ -202,14 +195,43 @@ func TestPreflightPending_StopReturnsToPreflightOnlyBeforeScoring(t *testing.T) require.NoError(t, pg.DB.SetCompetitionStarted(true)) assert.False(t, se.preflightPending()) - se.CurrentRound = 2 + _, err := pg.DB.CreateRound(db.RoundSchema{ID: 1, StartTime: time.Now()}) + require.NoError(t, err) require.NoError(t, pg.DB.SetCompetitionStarted(false)) assert.False(t, se.preflightPending(), "stopping after a scored round doesn't return to preflight") - se.CurrentRound = 1 + require.NoError(t, pg.DB.ResetScores()) assert.True(t, se.preflightPending(), "stopping before any scored round returns to preflight") } +func TestPreflightPending_IgnoresRoundCounterAfterEmptyRound(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + redis, pg := testutil.StartContainers(t) + redis.Client.FlushDB(context.Background()) + require.NoError(t, pg.DB.ResetScores()) + pg.ResetCompetitionState(t) + t.Cleanup(func() { pg.ResetCompetitionState(t) }) + + pg.CreateTestTeam(t, "Team Preflight Empty", "01") + se := newPreflightEngine(t, redis, pg) + se.Config.MiscSettings.Delay = 2 + + // Round 1 runs without a runner, so it stores nothing; only the in-memory + // counter moves on. + require.NoError(t, pg.DB.SetCompetitionStarted(true)) + require.NoError(t, se.rvb()) + assert.Equal(t, uint(2), se.CurrentRound) + last, err := pg.DB.GetLastRound() + require.NoError(t, err) + require.Zero(t, last.ID, "a round without results is not stored") + + require.NoError(t, pg.DB.SetCompetitionStarted(false)) + assert.True(t, se.preflightPending(), "stopping before any scored round was stored returns to preflight") +} + func TestPreflightPending_LegacyRowWithoutStartedAt(t *testing.T) { if testing.Short() { t.Skip("skipping integration test in short mode") @@ -217,7 +239,8 @@ func TestPreflightPending_LegacyRowWithoutStartedAt(t *testing.T) { redis, pg := testutil.StartContainers(t) require.NoError(t, pg.DB.ResetScores()) - t.Cleanup(func() { pg.Exec(t, "DELETE FROM competition_state_schemas") }) + pg.ResetCompetitionState(t) + t.Cleanup(func() { pg.ResetCompetitionState(t) }) se := newPreflightEngine(t, redis, pg) // Rows written before StartedAt existed: the UI treats them as started, so @@ -236,19 +259,19 @@ func TestResetScores_ReentersPreflight(t *testing.T) { redis, pg := testutil.StartContainers(t) redis.Client.FlushDB(context.Background()) require.NoError(t, pg.DB.ResetScores()) - t.Cleanup(func() { pg.Exec(t, "DELETE FROM competition_state_schemas") }) + pg.ResetCompetitionState(t) + t.Cleanup(func() { pg.ResetCompetitionState(t) }) pg.CreateTestTeam(t, "Team Preflight Reset", "01") se := newPreflightEngine(t, redis, pg) - stop := fakeRunner(t, redis, nil) + fakeRunner(t, redis, nil) require.NoError(t, se.preflight()) require.NoError(t, pg.DB.SetCompetitionStarted(true)) startedAt, err := pg.DB.GetCompetitionStart() require.NoError(t, err) require.NotNil(t, startedAt) require.NoError(t, se.rvb()) - stop() assert.False(t, se.preflightPending()) require.NoError(t, pg.DB.SetCompetitionStarted(false)) diff --git a/tests/integration/graphs_test.go b/tests/integration/graphs_test.go index cf556cad..cfde26f0 100644 --- a/tests/integration/graphs_test.go +++ b/tests/integration/graphs_test.go @@ -53,8 +53,8 @@ func TestGraphsMarkOnlyCallersOwnSeries(t *testing.T) { a := api.NewAPI(eng.Config, eng) require.NoError(t, pg.DB.SetCompetitionStarted(true)) t.Cleanup(func() { + pg.ResetCompetitionState(t) require.NoError(t, pg.DB.ResetScores()) - pg.Exec(t, "DELETE FROM competition_state_schemas") }) suffix := time.Now().UnixNano() diff --git a/tests/integration/inject_schedule_test.go b/tests/integration/inject_schedule_test.go index db48bb9c..8850d6c3 100644 --- a/tests/integration/inject_schedule_test.go +++ b/tests/integration/inject_schedule_test.go @@ -17,6 +17,8 @@ func TestImportedInjectScheduleAnchorsOnceToActualStart(t *testing.T) { } _, pg := testutil.StartContainers(t) + pg.ResetCompetitionState(t) + t.Cleanup(func() { pg.ResetCompetitionState(t) }) openOffset := int64((30 * time.Minute) / time.Second) dueOffset := int64((90 * time.Minute) / time.Second) diff --git a/tests/testutil/testutil.go b/tests/testutil/testutil.go index 60427279..b421d066 100644 --- a/tests/testutil/testutil.go +++ b/tests/testutil/testutil.go @@ -83,6 +83,13 @@ func (p *PostgresContainer) Exec(t *testing.T, sql string) { require.NoError(t, conn.Exec(sql).Error) } +// ResetCompetitionState forgets that the competition ever started, which the +// DB layer never does on its own: StartedAt is kept once set. +func (p *PostgresContainer) ResetCompetitionState(t *testing.T) { + t.Helper() + p.Exec(t, "DELETE FROM competition_state_schemas") +} + // testTeamCounter provides unique team IDs across test runs var testTeamCounter atomic.Uint64 From 2a31d134d9a876321051d492c41eeb0a8c05da31 Mon Sep 17 00:00:00 2001 From: Tirefire <84106878+tire-fire@users.noreply.github.com> Date: Mon, 9 Nov 2026 07:06:33 +0000 Subject: [PATCH 5/5] Address round-4 review findings --- engine/db/rounds.go | 7 +++++++ engine/engine.go | 15 ++++++--------- engine/preflight_test.go | 23 ++++++----------------- tests/integration/graphs_test.go | 6 ++---- tests/integration/inject_schedule_test.go | 1 - tests/testutil/testutil.go | 6 ++++-- 6 files changed, 25 insertions(+), 33 deletions(-) diff --git a/engine/db/rounds.go b/engine/db/rounds.go index 54c0414c..7e4ced43 100644 --- a/engine/db/rounds.go +++ b/engine/db/rounds.go @@ -51,6 +51,13 @@ func (d *DB) GetLastRound() (RoundSchema, error) { return round, nil } +// HasScoredRound reports whether any scored round has been stored. +func (d *DB) HasScoredRound() (bool, error) { + var n int64 + err := d.db.Table("round_schemas").Limit(1).Count(&n).Error + return n > 0, err +} + func (d *DB) RefreshScoresMaterializedView() error { // Use concurrent refresh to avoid blocking reads return d.db.Exec("REFRESH MATERIALIZED VIEW CONCURRENTLY cumulative_scores").Error diff --git a/engine/engine.go b/engine/engine.go index 94ea533e..356023d0 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -384,22 +384,19 @@ func (se *ScoringEngine) finishScoredRound() { se.RedisClient.Publish(context.Background(), "events", "round_finish") } -// preflightPending is true while the competition is not started and no scored -// round has been stored. Both facts are read from the DB every round rather -// than from CurrentRound, which also advances on rounds that stored nothing and -// is reseeded on restart. GetCompetitionStarted reports a read error as not -// started, and a GetLastRound error counts as no stored round: on a read error -// this iteration runs a preflight round, delaying scoring by one round. +// preflightPending reads the DB rather than CurrentRound, which also advances +// on rounds that stored nothing and is reseeded on restart. A DB read error +// runs a preflight round, delaying scoring by one round. func (se *ScoringEngine) preflightPending() bool { if se.DB.GetCompetitionStarted() { return false } - last, err := se.DB.GetLastRound() + scored, err := se.DB.HasScoredRound() if err != nil { - slog.Error("failed to get last round, running a preflight round", "error", err) + slog.Error("failed to check for scored rounds, running a preflight round", "error", err) return true } - return last.ID == 0 + return !scored } // preflight runs the same checks as rvb but stores the results apart from diff --git a/engine/preflight_test.go b/engine/preflight_test.go index 8bf1f473..560cb7e1 100644 --- a/engine/preflight_test.go +++ b/engine/preflight_test.go @@ -25,28 +25,24 @@ func passResult(task Task) []checks.Result { // fakeRunner answers every task on the "tasks" queue with respond(task) until // test cleanup. go-redis does not interrupt a blocking BLPop on context -// cancellation, so cleanup pushes a sentinel task that the goroutine consumes -// before it checks done. +// cancellation, so cleanup pushes a "stop" sentinel instead. func fakeRunner(t *testing.T, redis *testutil.RedisContainer, respond func(Task) []checks.Result) { t.Helper() if respond == nil { respond = passResult } ctx := context.Background() - done := make(chan struct{}) finished := make(chan struct{}) go func() { defer close(finished) for { val, err := redis.Client.BLPop(ctx, 0, "tasks").Result() - select { - case <-done: - return - default: - } if err != nil || len(val) < 2 { continue } + if val[1] == "stop" { + return + } var task Task if err := json.Unmarshal([]byte(val[1]), &task); err != nil { t.Logf("Failed to unmarshal task: %v", err) @@ -59,7 +55,6 @@ func fakeRunner(t *testing.T, redis *testutil.RedisContainer, respond func(Task) } }() t.Cleanup(func() { - close(done) redis.Client.RPush(ctx, "tasks", "stop") <-finished }) @@ -187,7 +182,6 @@ func TestPreflightPending_StopReturnsToPreflightOnlyBeforeScoring(t *testing.T) redis, pg := testutil.StartContainers(t) require.NoError(t, pg.DB.ResetScores()) pg.ResetCompetitionState(t) - t.Cleanup(func() { pg.ResetCompetitionState(t) }) se := newPreflightEngine(t, redis, pg) assert.True(t, se.preflightPending()) @@ -213,16 +207,13 @@ func TestPreflightPending_IgnoresRoundCounterAfterEmptyRound(t *testing.T) { redis.Client.FlushDB(context.Background()) require.NoError(t, pg.DB.ResetScores()) pg.ResetCompetitionState(t) - t.Cleanup(func() { pg.ResetCompetitionState(t) }) pg.CreateTestTeam(t, "Team Preflight Empty", "01") se := newPreflightEngine(t, redis, pg) - se.Config.MiscSettings.Delay = 2 - // Round 1 runs without a runner, so it stores nothing; only the in-memory - // counter moves on. + // A round that stored nothing still advances the in-memory counter. require.NoError(t, pg.DB.SetCompetitionStarted(true)) - require.NoError(t, se.rvb()) + se.finishScoredRound() assert.Equal(t, uint(2), se.CurrentRound) last, err := pg.DB.GetLastRound() require.NoError(t, err) @@ -240,7 +231,6 @@ func TestPreflightPending_LegacyRowWithoutStartedAt(t *testing.T) { redis, pg := testutil.StartContainers(t) require.NoError(t, pg.DB.ResetScores()) pg.ResetCompetitionState(t) - t.Cleanup(func() { pg.ResetCompetitionState(t) }) se := newPreflightEngine(t, redis, pg) // Rows written before StartedAt existed: the UI treats them as started, so @@ -260,7 +250,6 @@ func TestResetScores_ReentersPreflight(t *testing.T) { redis.Client.FlushDB(context.Background()) require.NoError(t, pg.DB.ResetScores()) pg.ResetCompetitionState(t) - t.Cleanup(func() { pg.ResetCompetitionState(t) }) pg.CreateTestTeam(t, "Team Preflight Reset", "01") se := newPreflightEngine(t, redis, pg) diff --git a/tests/integration/graphs_test.go b/tests/integration/graphs_test.go index cfde26f0..36c2792b 100644 --- a/tests/integration/graphs_test.go +++ b/tests/integration/graphs_test.go @@ -49,13 +49,11 @@ func TestGraphsMarkOnlyCallersOwnSeries(t *testing.T) { redis, pg := testutil.StartContainers(t) require.NoError(t, pg.DB.ResetScores()) + pg.ResetCompetitionState(t) eng := engine.NewTestEngine(t, redis, pg, 3) a := api.NewAPI(eng.Config, eng) require.NoError(t, pg.DB.SetCompetitionStarted(true)) - t.Cleanup(func() { - pg.ResetCompetitionState(t) - require.NoError(t, pg.DB.ResetScores()) - }) + t.Cleanup(func() { assert.NoError(t, pg.DB.ResetScores()) }) suffix := time.Now().UnixNano() teams := make([]db.TeamSchema, 3) diff --git a/tests/integration/inject_schedule_test.go b/tests/integration/inject_schedule_test.go index 8850d6c3..1e85978e 100644 --- a/tests/integration/inject_schedule_test.go +++ b/tests/integration/inject_schedule_test.go @@ -18,7 +18,6 @@ func TestImportedInjectScheduleAnchorsOnceToActualStart(t *testing.T) { _, pg := testutil.StartContainers(t) pg.ResetCompetitionState(t) - t.Cleanup(func() { pg.ResetCompetitionState(t) }) openOffset := int64((30 * time.Minute) / time.Second) dueOffset := int64((90 * time.Minute) / time.Second) diff --git a/tests/testutil/testutil.go b/tests/testutil/testutil.go index b421d066..3f533a7d 100644 --- a/tests/testutil/testutil.go +++ b/tests/testutil/testutil.go @@ -83,11 +83,13 @@ func (p *PostgresContainer) Exec(t *testing.T, sql string) { require.NoError(t, conn.Exec(sql).Error) } -// ResetCompetitionState forgets that the competition ever started, which the -// DB layer never does on its own: StartedAt is kept once set. +// ResetCompetitionState forgets that the competition ever started, now and +// again at test cleanup, which the DB layer never does on its own: StartedAt is +// kept once set. func (p *PostgresContainer) ResetCompetitionState(t *testing.T) { t.Helper() p.Exec(t, "DELETE FROM competition_state_schemas") + t.Cleanup(func() { p.Exec(t, "DELETE FROM competition_state_schemas") }) } // testTeamCounter provides unique team IDs across test runs