Add web auth, infinite-scroll, agent admission and review fixes across agent/server/web

agent/internal/scheduler/scheduler.go

@@ -2,6 +2,7 @@ package scheduler
 
 import (
 	"context"
+	"reflect"
 	"sync"
 	"time"
 
@@ -33,82 +34,316 @@ func (e Event) WireEvent() Event {
 }
 
 type Hooks struct {
-	OnSkipped func(checkID string)
-	OnResult  func(ev Event)
+	OnSkipped              func(checkID string)
+	OnResult               func(ev Event)
+	OnEventChannelFull     func()
+	OnResourceLimitFailure func(checkID string, resource string)
+	OnStopped              func(checkID string)
 }
 
-// Run starts one goroutine per check. Emits events to out. Blocks until ctx is done.
-func Run(ctx context.Context, agentID string, checks []config.CheckConfig, out chan<- Event, h Hooks) {
-	var wg sync.WaitGroup
-	for i := range checks {
-		c := checks[i]
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			runCheck(ctx, agentID, c, out, h)
-		}()
+// Manager owns per-check workers and applies hot config updates.
+type Manager struct {
+	ctx     context.Context
+	agentID string
+	out     chan<- Event
+	hooks   Hooks
+
+	mu       sync.Mutex
+	workers  map[string]*checkWorker
+	draining map[string]struct{}
+	pending  map[string]config.CheckConfig
+	all      []*checkWorker
+	stopped  bool
+}
+
+// NewManager creates a scheduler manager bound to the parent context.
+func NewManager(ctx context.Context, agentID string, out chan<- Event, h Hooks) *Manager {
+	return &Manager{
+		ctx:      ctx,
+		agentID:  agentID,
+		out:      out,
+		hooks:    h,
+		workers:  make(map[string]*checkWorker),
+		draining: make(map[string]struct{}),
+		pending:  make(map[string]config.CheckConfig),
 	}
-	wg.Wait()
 }
 
-func runCheck(ctx context.Context, agentID string, c config.CheckConfig, out chan<- Event, h Hooks) {
-	t := time.NewTicker(c.Interval.Duration)
-	defer t.Stop()
-	var running bool
-	var mu sync.Mutex
+// Update applies the current check set without killing in-flight runs.
+func (m *Manager) Update(checks []config.CheckConfig) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.stopped {
+		return
+	}
+	next := make(map[string]config.CheckConfig, len(checks))
+	for _, c := range checks {
+		next[c.ID] = c
+		if w, ok := m.workers[c.ID]; ok {
+			if _, draining := m.draining[c.ID]; draining {
+				m.pending[c.ID] = c
+			} else {
+				w.Update(c)
+			}
+			continue
+		}
+		id := c.ID
+		w := newCheckWorker(m.ctx, m.agentID, c, m.out, m.hooks, func(stopped *checkWorker) {
+			m.workerStopped(id, stopped)
+		})
+		m.workers[c.ID] = w
+		m.all = append(m.all, w)
+	}
+	for id, w := range m.workers {
+		if _, ok := next[id]; ok {
+			continue
+		}
+		m.draining[id] = struct{}{}
+		delete(m.pending, id)
+		w.Stop()
+	}
+}
 
-	exec := func() {
-		mu.Lock()
+// Stop prevents new runs and lets in-flight checks finish or observe ctx cancel.
+func (m *Manager) Stop() {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.stopped {
+		return
+	}
+	m.stopped = true
+	for _, w := range m.workers {
+		w.Stop()
+	}
+}
+
+// Wait blocks until all workers have drained.
+func (m *Manager) Wait() {
+	m.mu.Lock()
+	workers := append([]*checkWorker(nil), m.all...)
+	m.mu.Unlock()
+	for _, w := range workers {
+		w.Wait()
+	}
+}
+
+func (m *Manager) workerStopped(checkID string, w *checkWorker) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if current, ok := m.workers[checkID]; !ok || current != w {
+		return
+	}
+	delete(m.workers, checkID)
+	delete(m.draining, checkID)
+	if m.stopped {
+		delete(m.pending, checkID)
+		return
+	}
+	if cfg, ok := m.pending[checkID]; ok {
+		delete(m.pending, checkID)
+		next := newCheckWorker(m.ctx, m.agentID, cfg, m.out, m.hooks, func(stopped *checkWorker) {
+			m.workerStopped(checkID, stopped)
+		})
+		m.workers[checkID] = next
+		m.all = append(m.all, next)
+	}
+}
+
+type checkWorker struct {
+	ctx     context.Context
+	agentID string
+	out     chan<- Event
+	hooks   Hooks
+	onDone  func(*checkWorker)
+
+	updateCh chan config.CheckConfig
+	stopCh   chan struct{}
+	doneCh   chan struct{}
+	stopOnce sync.Once
+	wg       sync.WaitGroup
+}
+
+func newCheckWorker(ctx context.Context, agentID string, c config.CheckConfig, out chan<- Event, h Hooks, onDone func(*checkWorker)) *checkWorker {
+	w := &checkWorker{
+		ctx:      ctx,
+		agentID:  agentID,
+		out:      out,
+		hooks:    h,
+		onDone:   onDone,
+		updateCh: make(chan config.CheckConfig, 1),
+		stopCh:   make(chan struct{}),
+		doneCh:   make(chan struct{}, 1),
+	}
+	w.wg.Add(1)
+	go w.loop(c)
+	return w
+}
+
+func (w *checkWorker) Update(c config.CheckConfig) {
+	select {
+	case w.updateCh <- c:
+	default:
+		select {
+		case <-w.updateCh:
+		default:
+		}
+		w.updateCh <- c
+	}
+}
+
+func (w *checkWorker) Stop() {
+	w.stopOnce.Do(func() { close(w.stopCh) })
+}
+
+func (w *checkWorker) Wait() {
+	w.wg.Wait()
+}
+
+func (w *checkWorker) loop(initial config.CheckConfig) {
+	defer func() {
+		if w.onDone != nil {
+			w.onDone(w)
+		}
+		w.wg.Done()
+	}()
+	cfg := initial
+	t := time.NewTicker(cfg.Interval.Duration)
+	defer t.Stop()
+
+	running := false
+	stopping := false
+	runAfterCurrent := false
+	ctxDone := w.ctx.Done()
+	stopCh := w.stopCh
+
+	start := func(c config.CheckConfig) {
 		if running {
-			mu.Unlock()
-			if h.OnSkipped != nil {
-				h.OnSkipped(c.ID)
+			if w.hooks.OnSkipped != nil {
+				w.hooks.OnSkipped(c.ID)
 			}
 			return
 		}
 		running = true
-		mu.Unlock()
-		defer func() {
-			mu.Lock()
-			running = false
-			mu.Unlock()
+		go func() {
+			w.run(c)
+			w.doneCh <- struct{}{}
 		}()
-
-		res := runner.Run(ctx, c.Argv(), c.Timeout.Duration)
-		id, err := eventid.New()
-		if err != nil {
-			return
-		}
-		ev := Event{
-			EventID:              id,
-			CheckID:              c.ID,
-			ObservedAt:           time.Now().UTC(),
-			Status:               string(res.Status),
-			ExitCode:             res.ExitCode,
-			DurationMs:           res.DurationMs,
-			Output:               res.Output,
-			OutputTruncated:      res.OutputTruncated,
-			NotificationsEnabled: boolPtr(c.NotificationsOn()),
-			IncidentKey:          incidentKey(agentID, c),
-		}
-		if h.OnResult != nil {
-			h.OnResult(ev)
-		}
-		select {
-		case out <- ev:
-		case <-ctx.Done():
-		}
 	}
 
-	// Fire first run immediately so smoke tests don't wait a full interval.
-	go exec()
+	start(cfg)
 
 	for {
 		select {
-		case <-ctx.Done():
-			return
+		case <-ctxDone:
+			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:
+			if reflect.DeepEqual(cfg, next) {
+				continue
+			}
+			cfg = next
+			t.Reset(cfg.Interval.Duration)
+			if running {
+				runAfterCurrent = true
+			} else {
+				start(cfg)
+			}
+		case <-w.doneCh:
+			running = false
+			if !stopping {
+				select {
+				case <-stopCh:
+					stopping = true
+					stopCh = nil
+				default:
+				}
+			}
+			if stopping {
+				if w.hooks.OnStopped != nil {
+					w.hooks.OnStopped(cfg.ID)
+				}
+				return
+			}
+			if runAfterCurrent {
+				runAfterCurrent = false
+				for {
+					select {
+					case next := <-w.updateCh:
+						if !reflect.DeepEqual(cfg, next) {
+							cfg = next
+							t.Reset(cfg.Interval.Duration)
+						}
+					default:
+						start(cfg)
+						goto nextLoop
+					}
+				}
+			nextLoop:
+				continue
+			}
 		case <-t.C:
-			go exec()
+			if stopping {
+				continue
+			}
+			start(cfg)
+		}
+	}
+}
+
+func (w *checkWorker) run(c config.CheckConfig) {
+	res := runner.RunCommand(w.ctx, c.Command, c.Timeout.Duration, runner.ResourceLimits{
+		CPUTime:     c.ResourceLimits.CPUTime.Duration,
+		MemoryBytes: c.ResourceLimits.Memory.Bytes,
+		OpenFiles:   c.ResourceLimits.OpenFiles,
+	})
+	if res.ResourceLimitFailure != "" && w.hooks.OnResourceLimitFailure != nil {
+		w.hooks.OnResourceLimitFailure(c.ID, res.ResourceLimitFailure)
+	}
+	id, err := eventid.New()
+	if err != nil {
+		return
+	}
+	ev := Event{
+		EventID:              id,
+		CheckID:              c.ID,
+		ObservedAt:           time.Now().UTC(),
+		Status:               string(res.Status),
+		ExitCode:             res.ExitCode,
+		DurationMs:           res.DurationMs,
+		Output:               res.Output,
+		OutputTruncated:      res.OutputTruncated,
+		NotificationsEnabled: boolPtr(c.NotificationsOn()),
+		IncidentKey:          incidentKey(w.agentID, c),
+	}
+	if w.hooks.OnResult != nil {
+		w.hooks.OnResult(ev)
+	}
+	select {
+	case w.out <- ev:
+	case <-w.ctx.Done():
+	default:
+		if w.hooks.OnEventChannelFull != nil {
+			w.hooks.OnEventChannelFull()
+		}
+		select {
+		case w.out <- ev:
+		case <-w.ctx.Done():
+		case <-w.stopCh:
 		}
 	}
 }

agent/internal/scheduler/scheduler_test.go

@@ -2,6 +2,7 @@ package scheduler
 
 import (
 	"context"
+	"strings"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -9,6 +10,14 @@ import (
 	"github.com/monlet/agent/internal/config"
 )
 
+func runScheduler(ctx context.Context, agentID string, checks []config.CheckConfig, out chan<- Event, h Hooks) {
+	m := NewManager(ctx, agentID, out, h)
+	m.Update(checks)
+	<-ctx.Done()
+	m.Stop()
+	m.Wait()
+}
+
 func mkCheck(id, cmd string, interval, timeout time.Duration) config.CheckConfig {
 	return config.CheckConfig{
 		ID:       id,
@@ -22,7 +31,7 @@ func TestEmitsEvents(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond)
 	defer cancel()
 	out := make(chan Event, 10)
-	go Run(ctx, "a1", []config.CheckConfig{mkCheck("c1", "echo hi", 200*time.Millisecond, 100*time.Millisecond)}, out, Hooks{})
+	go runScheduler(ctx, "a1", []config.CheckConfig{mkCheck("c1", "echo hi", 200*time.Millisecond, 100*time.Millisecond)}, out, Hooks{})
 	select {
 	case ev := <-out:
 		if ev.CheckID != "c1" || ev.Status != "ok" {
@@ -43,9 +52,168 @@ func TestNoOverlapSkips(t *testing.T) {
 	var skipped int32
 	hooks := Hooks{OnSkipped: func(string) { atomic.AddInt32(&skipped, 1) }}
 	c := mkCheck("slow", "sleep 1", 100*time.Millisecond, 900*time.Millisecond)
-	go Run(ctx, "a1", []config.CheckConfig{c}, out, hooks)
+	go runScheduler(ctx, "a1", []config.CheckConfig{c}, out, hooks)
 	<-ctx.Done()
 	if atomic.LoadInt32(&skipped) == 0 {
 		t.Fatal("expected at least one skipped tick")
 	}
 }
+
+func TestReportsFullEventChannel(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+	out := make(chan Event, 1)
+	out <- Event{EventID: "occupied"}
+	var full int32
+	m := NewManager(ctx, "a1", out, Hooks{
+		OnEventChannelFull: func() { atomic.AddInt32(&full, 1) },
+	})
+	m.Update([]config.CheckConfig{mkCheck("c1", "echo hi", time.Hour, time.Second)})
+
+	deadline := time.Now().Add(time.Second)
+	for time.Now().Before(deadline) {
+		if atomic.LoadInt32(&full) > 0 {
+			break
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	if atomic.LoadInt32(&full) == 0 {
+		t.Fatal("expected full channel hook")
+	}
+	<-out
+	select {
+	case ev := <-out:
+		if ev.CheckID != "c1" {
+			t.Fatalf("unexpected event: %+v", ev)
+		}
+	case <-ctx.Done():
+		t.Fatal("blocked event was not delivered")
+	}
+	m.Stop()
+	m.Wait()
+}
+
+func TestManagerReloadChangedCheckWaitsForRunning(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+	out := make(chan Event, 10)
+	m := NewManager(ctx, "a1", out, Hooks{})
+	m.Update([]config.CheckConfig{mkCheck("c1", "sleep 0.3; echo old", time.Second, time.Second)})
+	time.Sleep(50 * time.Millisecond)
+	m.Update([]config.CheckConfig{mkCheck("c1", "echo new", time.Second, time.Second)})
+
+	var first, second Event
+	select {
+	case first = <-out:
+	case <-ctx.Done():
+		t.Fatal("no first event")
+	}
+	select {
+	case second = <-out:
+	case <-ctx.Done():
+		t.Fatal("no second event")
+	}
+	m.Stop()
+	m.Wait()
+
+	if !strings.Contains(first.Output, "old") {
+		t.Fatalf("first output: %q", first.Output)
+	}
+	if !strings.Contains(second.Output, "new") {
+		t.Fatalf("second output: %q", second.Output)
+	}
+}
+
+func TestManagerReloadRemovedCheckLetsRunningFinish(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 2*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{mkCheck("c1", "sleep 0.2; echo old", time.Second, time.Second)})
+	time.Sleep(50 * time.Millisecond)
+	m.Update(nil)
+
+	select {
+	case ev := <-out:
+		if !strings.Contains(ev.Output, "old") {
+			t.Fatalf("output: %q", ev.Output)
+		}
+	case <-ctx.Done():
+		t.Fatal("removed running check did not finish")
+	}
+	select {
+	case id := <-stopped:
+		if id != "c1" {
+			t.Fatalf("stopped id: %q", id)
+		}
+	case <-ctx.Done():
+		t.Fatal("removed check did not stop")
+	}
+	m.Stop()
+	m.Wait()
+}
+
+func TestManagerRemoveThenReaddSameCheckDoesNotOverlap(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
+	out := make(chan Event, 10)
+	m := NewManager(ctx, "a1", out, Hooks{})
+	m.Update([]config.CheckConfig{mkCheck("c1", "sleep 0.25; echo old", time.Second, time.Second)})
+	time.Sleep(50 * time.Millisecond)
+	m.Update(nil)
+	m.Update([]config.CheckConfig{mkCheck("c1", "echo new", time.Second, time.Second)})
+
+	var first, second Event
+	select {
+	case first = <-out:
+	case <-ctx.Done():
+		t.Fatal("no first event")
+	}
+	select {
+	case second = <-out:
+	case <-ctx.Done():
+		t.Fatal("no second event")
+	}
+	m.Stop()
+	m.Wait()
+
+	if !strings.Contains(first.Output, "old") {
+		t.Fatalf("first output: %q", first.Output)
+	}
+	if !strings.Contains(second.Output, "new") {
+		t.Fatalf("second output: %q", second.Output)
+	}
+}
+
+func TestManagerUsesLatestPendingReloadAfterRunningFinishes(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
+	out := make(chan Event, 10)
+	m := NewManager(ctx, "a1", out, Hooks{})
+	m.Update([]config.CheckConfig{mkCheck("c1", "sleep 0.25; echo old", time.Second, time.Second)})
+	time.Sleep(50 * time.Millisecond)
+	m.Update([]config.CheckConfig{mkCheck("c1", "echo intermediate", time.Second, time.Second)})
+	m.Update([]config.CheckConfig{mkCheck("c1", "echo latest", time.Second, time.Second)})
+
+	var first, second Event
+	select {
+	case first = <-out:
+	case <-ctx.Done():
+		t.Fatal("no first event")
+	}
+	select {
+	case second = <-out:
+	case <-ctx.Done():
+		t.Fatal("no second event")
+	}
+	m.Stop()
+	m.Wait()
+
+	if !strings.Contains(first.Output, "old") {
+		t.Fatalf("first output: %q", first.Output)
+	}
+	if strings.Contains(second.Output, "intermediate") || !strings.Contains(second.Output, "latest") {
+		t.Fatalf("second output: %q", second.Output)
+	}
+}