From 72acaca3ab69d563670531c36ca5f8382317876b Mon Sep 17 00:00:00 2001 From: Tirefire <84106878+tire-fire@users.noreply.github.com> Date: Tue, 27 Oct 2026 00:08:16 +0000 Subject: [PATCH 1/3] Clear stale Redis results at round start --- engine/engine.go | 13 +++++---- engine/engine_test.go | 68 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/engine/engine.go b/engine/engine.go index 5d8e164..da8f2d2 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -410,11 +410,14 @@ func (se *ScoringEngine) rvb() error { slog.Debug("Box configuration", "name", box.Name, "runners", len(box.Runners)) } - // Clear any stale tasks from previous rounds before enqueuing new ones - staleTasks := se.RedisClient.LLen(ctx, "tasks").Val() - if staleTasks > 0 { - slog.Warn("Clearing stale tasks from queue", "count", staleTasks, "round", se.CurrentRound) - se.RedisClient.Del(ctx, "tasks") + // Clear any stale tasks and results before enqueuing new ones. After a + // restart mid-round the next round reuses the round ID, so leftover + // 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) + se.RedisClient.Del(ctx, key) + } } // 1) Enqueue diff --git a/engine/engine_test.go b/engine/engine_test.go index 98ed8bb..e5988a7 100644 --- a/engine/engine_test.go +++ b/engine/engine_test.go @@ -585,3 +585,71 @@ func TestEnsureCredentialsSeeded_HotReload(t *testing.T) { require.NoError(t, se.ResetCredentials(team1.ID, "reload_linux.credlist", "admin")) assert.Equal(t, "pass1_updated", credMap(team1.ID, "reload_linux.credlist")["alice"]) } + +func TestRvb_IgnoresStaleResultsFromRestartedRound(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + t.Setenv("REDIS_ADDR", "localhost:6379") + + redis, pg := testutil.StartContainers(t) + + ctx := context.Background() + redis.Client.FlushDB(ctx) + require.NoError(t, pg.DB.ResetScores()) + + team := pg.CreateTestTeam(t, "Team Stale", "01") + + engine := NewTestEngine(t, redis, pg, 3) + engine.CurrentRound = 1 + + svc := &mockRunner{Service: checks.Service{Display: "web", Name: "box01-web", ServiceType: "Mock", Points: 10}} + engine.Config.Box = []config.Box{{Name: "box01", IP: "10.0.0.1", Runners: []checks.Runner{svc}}} + + // A failed result left behind by a round that was cut short by a restart. + 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) + } + } + }() + + err := engine.rvb() + close(stopRunner) + <-runnerDone + require.NoError(t, err) + + round, err := pg.DB.GetLastRound() + require.NoError(t, err) + var teamChecks []db.ServiceCheckSchema + for _, check := range round.Checks { + if check.TeamID == team.ID { + teamChecks = append(teamChecks, check) + } + } + require.Len(t, teamChecks, 1) + assert.True(t, teamChecks[0].Result, "stale failed result from the restarted round must not be scored") +} From a208e0fde4aae42302554d4d5a23d2d06ccc97be Mon Sep 17 00:00:00 2001 From: Tirefire <84106878+tire-fire@users.noreply.github.com> Date: Tue, 27 Oct 2026 00:08:17 +0000 Subject: [PATCH 2/3] Add AttemptDelay and event-wide attempt defaults --- README.md | 6 +++ engine/checks/checks.go | 22 ++++++++ engine/config/attempt_config_test.go | 79 ++++++++++++++++++++++++++++ engine/config/config.go | 29 ++++++++++ engine/engine.go | 1 + engine/task.go | 1 + runner/runner.go | 19 +++++++ runner/runner_test.go | 27 ++++++++++ 8 files changed, 184 insertions(+) create mode 100644 engine/config/attempt_config_test.go create mode 100644 runner/runner_test.go diff --git a/README.md b/README.md index 1b295f2..fe07a34 100644 --- a/README.md +++ b/README.md @@ -265,8 +265,12 @@ Points = 5 # Points per successful check (default: 1) Timeout = 30 # Check timeout in seconds (default: Delay/2) SlaThreshold = 5 # Consecutive failures before SLA penalty (default: 5) SlaPenalty = 50 # Points deducted for SLA violation (default: SlaThreshold * Points; 0 counts SLAs without deducting; not per-check) +Attempts = 1 # Tries before a check fails (default: 1) +AttemptDelay = 0 # Seconds to wait before retrying a failed attempt (default: 0) ``` +Every attempt, plus the waits between them, must fit in the shortest round: `Attempts * Timeout + (Attempts - 1) * AttemptDelay` must be less than `Delay - Jitter`. + #### UI Settings ```toml @@ -357,6 +361,8 @@ All checks support these common properties: | `timeout` | Check timeout in seconds | Global default | | `slathreshold` | Failures before SLA penalty | Global default | | `slapenalty` | Points deducted on SLA violation | Global default | +| `attempts` | Tries before the check fails | Global default | +| `attemptdelay` | Seconds to wait before retrying a failed attempt | Global default | | `credlists` | Array of credlist names for authentication | None | | `disabled` | Disable this check | false | | `launchtime` | Start checking at this time | Immediate | diff --git a/engine/checks/checks.go b/engine/checks/checks.go index bab0887..be07f1f 100644 --- a/engine/checks/checks.go +++ b/engine/checks/checks.go @@ -22,6 +22,8 @@ type Runner interface { GetType() string GetName() string GetAttempts() int + GetAttemptDelay() int + GetTimeout() int GetCredlists() []string SetTaskCredentials(creds []TaskCredential) } @@ -42,6 +44,7 @@ type Service struct { Target string `toml:",omitempty"` // Target is the IP address or hostname for the box ServiceType string `toml:",omitempty"` // ServiceType is the name of the Runner that checks the service Attempts int `toml:",omitempty"` // Attempts is the number of times the service has been checked + AttemptDelay int `toml:",omitempty"` // AttemptDelay is the number of seconds to wait before retrying a failed attempt TaskCredentials []TaskCredential `toml:"-"` // Credentials from task payload (set per-task, not from config) } @@ -75,6 +78,25 @@ func (service *Service) GetAttempts() int { return service.Attempts } +func (service *Service) GetAttemptDelay() int { + return service.AttemptDelay +} + +func (service *Service) GetTimeout() int { + return service.Timeout +} + +// SetAttemptDefaults fills Attempts and AttemptDelay from the event-wide +// defaults when the service doesn't set them. +func (service *Service) SetAttemptDefaults(attempts int, attemptDelay int) { + if service.Attempts == 0 { + service.Attempts = attempts + } + if service.AttemptDelay == 0 { + service.AttemptDelay = attemptDelay + } +} + func (service *Service) GetCredlists() []string { return service.CredLists } diff --git a/engine/config/attempt_config_test.go b/engine/config/attempt_config_test.go new file mode 100644 index 0000000..55a00b9 --- /dev/null +++ b/engine/config/attempt_config_test.go @@ -0,0 +1,79 @@ +package config + +import ( + "testing" + + "github.com/dbaseqp/Quotient/engine/checks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "pgregory.net/rapid" +) + +func attemptTestConfig(delay, jitter, timeout, attempts, attemptDelay int, svc checks.Service) *ConfigSettings { + svc.Port = 80 + return &ConfigSettings{ + RequiredSettings: RequiredConfig{ + EventName: "Test Event", + EventType: "rvb", + DBConnectURL: "postgres://test:test@localhost:5432/test", + BindAddress: "0.0.0.0:8080", + }, + MiscSettings: MiscConfig{ + Delay: delay, + Jitter: jitter, + Timeout: timeout, + Attempts: attempts, + AttemptDelay: attemptDelay, + }, + Box: []Box{{ + Name: "box", + IP: "10.100.1_.10", + Tcp: []*checks.Tcp{{Service: svc}}, + }}, + } +} + +// TestPropertyAttemptBudget verifies a check is accepted exactly when all its +// attempts and the delays between them fit in the shortest round. +func TestPropertyAttemptBudget(t *testing.T) { + rapid.Check(t, func(t *rapid.T) { + delay := rapid.IntRange(20, 300).Draw(t, "delay") + jitter := rapid.IntRange(1, delay/2).Draw(t, "jitter") + timeout := rapid.IntRange(1, delay-jitter-1).Draw(t, "timeout") + attempts := rapid.IntRange(1, 5).Draw(t, "attempts") + attemptDelay := rapid.IntRange(0, 200).Draw(t, "attemptDelay") + + conf := attemptTestConfig(delay, jitter, timeout, attempts, attemptDelay, checks.Service{}) + err := checkConfig(conf) + + fits := attempts*timeout+(attempts-1)*attemptDelay < delay-jitter + if fits { + require.NoError(t, err) + check := conf.Box[0].Runners[0] + assert.Equal(t, attempts, check.GetAttempts()) + assert.Equal(t, attemptDelay, check.GetAttemptDelay()) + } else { + require.Error(t, err) + assert.Contains(t, err.Error(), "must be smaller than delay minus jitter") + } + }) +} + +func TestAttemptDefaultsDoNotOverrideService(t *testing.T) { + conf := attemptTestConfig(120, 10, 30, 2, 45, checks.Service{Attempts: 1, AttemptDelay: 5}) + require.NoError(t, checkConfig(conf)) + check := conf.Box[0].Runners[0] + assert.Equal(t, 1, check.GetAttempts()) + assert.Equal(t, 5, check.GetAttemptDelay()) +} + +func TestAttemptsDefaultToOne(t *testing.T) { + conf := attemptTestConfig(120, 10, 30, 0, 0, checks.Service{}) + require.NoError(t, checkConfig(conf)) + assert.Equal(t, 1, conf.Box[0].Runners[0].GetAttempts()) +} + +func TestNegativeAttemptSettingsRejected(t *testing.T) { + require.Error(t, checkConfig(attemptTestConfig(120, 10, 30, -1, 0, checks.Service{}))) + require.Error(t, checkConfig(attemptTestConfig(120, 10, 30, 2, -1, checks.Service{}))) +} diff --git a/engine/config/config.go b/engine/config/config.go index 350d5de..5002c7a 100644 --- a/engine/config/config.go +++ b/engine/config/config.go @@ -130,6 +130,8 @@ type MiscConfig struct { Points int Timeout int SlaThreshold int + Attempts int + AttemptDelay int // seconds between attempts of a failing check // SlaPenalty is a pointer so an explicit 0 (count SLAs without deducting // points) can be told apart from an omitted key. SlaPenalty *int @@ -357,6 +359,13 @@ func checkConfig(conf *ConfigSettings) error { errResult = errors.Join(errResult, errors.New("timeout must be smaller than delay minus jitter")) } + if conf.MiscSettings.Attempts < 0 { + errResult = errors.Join(errResult, errors.New("Attempts must not be negative")) + } + if conf.MiscSettings.AttemptDelay < 0 { + errResult = errors.Join(errResult, errors.New("AttemptDelay must not be negative")) + } + if conf.MiscSettings.Points == 0 { conf.MiscSettings.Points = 1 } @@ -461,6 +470,9 @@ func checkConfig(conf *ConfigSettings) error { } for _, checks := range checkSets { for _, check := range checks { + if setter, ok := check.(interface{ SetAttemptDefaults(int, int) }); ok { + setter.SetAttemptDefaults(conf.MiscSettings.Attempts, conf.MiscSettings.AttemptDelay) + } if err := check.Verify( conf.Box[i].Name, conf.Box[i].IP, @@ -471,6 +483,9 @@ func checkConfig(conf *ConfigSettings) error { ); err != nil { errResult = errors.Join(errResult, err) } + if err := checkAttemptBudget(check, conf.MiscSettings.Delay-conf.MiscSettings.Jitter); err != nil { + errResult = errors.Join(errResult, err) + } if _, exists := runnerNames[check.GetName()]; exists { errResult = errors.Join(errResult, fmt.Errorf("duplicate check name found: %s", check.GetName())) } else { @@ -516,6 +531,20 @@ func getRunners[T checks.Runner](arr []T) []checks.Runner { return out } +// checkAttemptBudget rejects a check whose attempts, each running to its +// timeout, plus the delays between them can't finish in the shortest round. +func checkAttemptBudget(check checks.Runner, shortestRound int) error { + attempts, delay := check.GetAttempts(), check.GetAttemptDelay() + if attempts < 1 || delay < 0 { + return fmt.Errorf("%s: Attempts must be at least 1 and AttemptDelay not negative", check.GetName()) + } + if budget := attempts*check.GetTimeout() + (attempts-1)*delay; budget >= shortestRound { + return fmt.Errorf("%s: %d attempts with timeout %ds and delay %ds take up to %ds, which must be smaller than delay minus jitter (%ds)", + check.GetName(), attempts, check.GetTimeout(), delay, budget, shortestRound) + } + return nil +} + // Returns a flat list of all checks across all boxes. func (conf *ConfigSettings) AllChecks() []checks.Runner { var out []checks.Runner diff --git a/engine/engine.go b/engine/engine.go index da8f2d2..193c1fa 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -452,6 +452,7 @@ func (se *ScoringEngine) rvb() error { RoundID: se.CurrentRound, Deadline: se.NextRoundStartTime, Attempts: r.GetAttempts(), + AttemptDelay: r.GetAttemptDelay(), CheckData: data, // the entire specialized struct } diff --git a/engine/task.go b/engine/task.go index 2a03b58..a7a4385 100644 --- a/engine/task.go +++ b/engine/task.go @@ -19,6 +19,7 @@ type Task struct { Deadline time.Time `json:"deadline"` RoundID uint `json:"round_id"` Attempts int `json:"attempts"` + AttemptDelay int `json:"attempt_delay"` // seconds to wait before retrying a failed attempt CheckData json.RawMessage `json:"check_data"` Credentials []Credential `json:"credentials,omitempty"` } diff --git a/runner/runner.go b/runner/runner.go index 5fa4954..34b78eb 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -232,6 +232,9 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta if result.Status || time.Now().After(task.Deadline) { break } + if i < task.Attempts-1 && !waitForRetry(ctx, time.Duration(task.AttemptDelay)*time.Second, task.Deadline) { + break + } } // Marshal and store result @@ -256,3 +259,19 @@ func handleTask(ctx context.Context, rdb *redis.Client, runner checks.Runner, ta // Use a longer TTL for completed tasks to ensure they're visible in the UI rdb.Set(ctx, taskKey, statusJSON, 3*time.Minute) } + +// waitForRetry waits delay before the next attempt and reports whether that +// attempt can still start before the round deadline. +func waitForRetry(ctx context.Context, delay time.Duration, deadline time.Time) bool { + if !time.Now().Add(delay).Before(deadline) { + return false + } + timer := time.NewTimer(delay) + defer timer.Stop() + select { + case <-ctx.Done(): + return false + case <-timer.C: + return true + } +} diff --git a/runner/runner_test.go b/runner/runner_test.go new file mode 100644 index 0000000..f3f463a --- /dev/null +++ b/runner/runner_test.go @@ -0,0 +1,27 @@ +package main + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestWaitForRetryWaitsDelay(t *testing.T) { + start := time.Now() + assert.True(t, waitForRetry(context.Background(), 50*time.Millisecond, start.Add(time.Second))) + assert.GreaterOrEqual(t, time.Since(start), 50*time.Millisecond) +} + +func TestWaitForRetrySkipsWhenDeadlineTooClose(t *testing.T) { + start := time.Now() + assert.False(t, waitForRetry(context.Background(), time.Second, start.Add(500*time.Millisecond))) + assert.Less(t, time.Since(start), 100*time.Millisecond, "should not sleep when no retry fits") +} + +func TestWaitForRetryStopsOnCancel(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + assert.False(t, waitForRetry(ctx, time.Second, time.Now().Add(time.Minute))) +} From ecb2f5955edf4f85cbb709b192842576c21fe76c Mon Sep 17 00:00:00 2001 From: Tirefire <84106878+tire-fire@users.noreply.github.com> Date: Tue, 27 Oct 2026 00:12:34 +0000 Subject: [PATCH 3/3] Lowercase attempt config error strings --- engine/config/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/config/config.go b/engine/config/config.go index 5002c7a..a38f084 100644 --- a/engine/config/config.go +++ b/engine/config/config.go @@ -360,10 +360,10 @@ func checkConfig(conf *ConfigSettings) error { } if conf.MiscSettings.Attempts < 0 { - errResult = errors.Join(errResult, errors.New("Attempts must not be negative")) + errResult = errors.Join(errResult, errors.New("attempts must not be negative")) } if conf.MiscSettings.AttemptDelay < 0 { - errResult = errors.Join(errResult, errors.New("AttemptDelay must not be negative")) + errResult = errors.Join(errResult, errors.New("attempt delay must not be negative")) } if conf.MiscSettings.Points == 0 {