From 762c48e96b2956b075d93e0ab89cda3f7d701017 Mon Sep 17 00:00:00 2001 From: Stanislav Rossovskii Date: Tue, 23 Jun 2026 20:04:08 +0400 Subject: [PATCH] Fix scheduler cleanup edge cases --- agent/internal/config/config.go | 2 +- agent/internal/config/config_test.go | 23 +++++++++++++++ agent/internal/scheduler/scheduler.go | 25 ++++++++-------- agent/internal/scheduler/scheduler_test.go | 33 ++++++++++++++++++++++ web/src/app/globals.css | 6 ---- 5 files changed, 69 insertions(+), 20 deletions(-) diff --git a/agent/internal/config/config.go b/agent/internal/config/config.go index 722b57c..7a78665 100644 --- a/agent/internal/config/config.go +++ b/agent/internal/config/config.go @@ -217,7 +217,7 @@ func (c *Config) Validate() error { return fmt.Errorf("check %q: timeout must be > 0", ch.ID) } if ch.Interval.Duration < 0 { - return fmt.Errorf("check %q: interval must be > 0", ch.ID) + return fmt.Errorf("check %q: interval must not be negative", ch.ID) } ch.Cron = strings.TrimSpace(ch.Cron) hasInterval := ch.Interval.Duration > 0 diff --git a/agent/internal/config/config_test.go b/agent/internal/config/config_test.go index 3535a73..702a23e 100644 --- a/agent/internal/config/config_test.go +++ b/agent/internal/config/config_test.go @@ -123,6 +123,29 @@ func TestValidateCheckScheduleChoice(t *testing.T) { } } +func TestValidateNegativeIntervalMessage(t *testing.T) { + _, err := Load(writeTmp(t, ` +agent_id = "host-1" +state_dir = "/tmp/x" + +[server] +enabled = true +url = "http://localhost" + +[[checks]] +id = "c1" +command = "true" +interval = "-1s" +timeout = "5s" +`)) + if err == nil { + t.Fatal("expected negative interval error") + } + if !strings.Contains(err.Error(), "interval must not be negative") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestLoadResourceLimits(t *testing.T) { c, err := Load(writeTmp(t, ` agent_id = "host-1" diff --git a/agent/internal/scheduler/scheduler.go b/agent/internal/scheduler/scheduler.go index 76fbb7e..9e96ffe 100644 --- a/agent/internal/scheduler/scheduler.go +++ b/agent/internal/scheduler/scheduler.go @@ -202,13 +202,24 @@ func (w *checkWorker) Wait() { } func (w *checkWorker) loop(initial config.CheckConfig) { + cfg := initial + stoppedNotified := false + notifyStopped := func() { + if stoppedNotified { + return + } + stoppedNotified = true + if w.hooks.OnStopped != nil { + w.hooks.OnStopped(cfg.ID) + } + } defer func() { + notifyStopped() if w.onDone != nil { w.onDone(w) } w.wg.Done() }() - cfg := initial t := time.NewTimer(time.Hour) stopTimer(t) defer t.Stop() @@ -249,9 +260,6 @@ func (w *checkWorker) loop(initial config.CheckConfig) { } if !arm(cfg, true) { - if w.hooks.OnStopped != nil { - w.hooks.OnStopped(cfg.ID) - } return } @@ -261,18 +269,12 @@ func (w *checkWorker) loop(initial config.CheckConfig) { stopping = true ctxDone = nil if !running { - if w.hooks.OnStopped != nil { - w.hooks.OnStopped(cfg.ID) - } return } case <-stopCh: stopping = true stopCh = nil if !running { - if w.hooks.OnStopped != nil { - w.hooks.OnStopped(cfg.ID) - } return } case next := <-w.updateCh: @@ -307,9 +309,6 @@ func (w *checkWorker) loop(initial config.CheckConfig) { } } if stopping { - if w.hooks.OnStopped != nil { - w.hooks.OnStopped(cfg.ID) - } return } if runAfterCurrent { diff --git a/agent/internal/scheduler/scheduler_test.go b/agent/internal/scheduler/scheduler_test.go index f9658ad..019fcd6 100644 --- a/agent/internal/scheduler/scheduler_test.go +++ b/agent/internal/scheduler/scheduler_test.go @@ -2,6 +2,7 @@ package scheduler import ( "context" + "errors" "strings" "sync/atomic" "testing" @@ -107,6 +108,38 @@ func TestCronEmitsOnDueSlot(t *testing.T) { m.Wait() } +func TestCronArmErrorCallsStoppedHook(t *testing.T) { + old := parseCronSchedule + var calls int32 + parseCronSchedule = func(string) (cron.Schedule, error) { + if atomic.AddInt32(&calls, 1) == 1 { + return fakeCronSchedule{delay: 10 * time.Millisecond}, nil + } + return nil, errors.New("boom") + } + t.Cleanup(func() { parseCronSchedule = old }) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + out := make(chan Event, 10) + stopped := make(chan string, 1) + m := NewManager(ctx, "a1", out, Hooks{ + OnStopped: func(id string) { stopped <- id }, + }) + m.Update([]config.CheckConfig{mkCronCheck("cron", "true", 100*time.Millisecond)}) + + select { + case id := <-stopped: + if id != "cron" { + t.Fatalf("unexpected stopped id: %q", id) + } + case <-ctx.Done(): + t.Fatal("stopped hook was not called") + } + m.Stop() + m.Wait() +} + func TestNoOverlapSkips(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 600*time.Millisecond) defer cancel() diff --git a/web/src/app/globals.css b/web/src/app/globals.css index 3d44047..421b578 100644 --- a/web/src/app/globals.css +++ b/web/src/app/globals.css @@ -13,12 +13,6 @@ --font-mono: var(--font-geist-mono); } -:root[data-theme="dark"] { - --background: #0a0a0a; - --foreground: #ededed; - color-scheme: dark; -} - body { background: var(--background); color: var(--foreground);