From 4f9559cccec50d8d1a561c2f8ab01389da440c68 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Tue, 29 Sep 2026 14:17:53 -0400 Subject: [PATCH 1/4] fix: name the credential profile in the SSO login hint The hint passed the Account name as --profile, but Accounts and profiles differ (libnd uses libnd-view), so the suggested command failed. The pipeline section dropped the profile altogether. Closes #66 --- internal/ui/dashboard.go | 12 ++++++------ internal/ui/dashboard_test.go | 36 ++++++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/internal/ui/dashboard.go b/internal/ui/dashboard.go index 973e8e6..8889211 100644 --- a/internal/ui/dashboard.go +++ b/internal/ui/dashboard.go @@ -496,7 +496,7 @@ func renderStacksSection(proj state.ProjectState) string { } out := normalStyle.Render(" stacks") + "\n" if proj.StacksFetch.Err != nil { - out += renderFetchError(proj.StacksFetch.Err, proj.Account) + out += renderFetchError(proj.StacksFetch.Err, proj.Profile) } for _, s := range stacks { icon := s.Stoplight.String() @@ -550,7 +550,7 @@ func renderECSSection(proj state.ProjectState) string { } out := normalStyle.Render(" ecs") + "\n" if proj.ECSFetch.Err != nil { - out += renderFetchError(proj.ECSFetch.Err, proj.Account) + out += renderFetchError(proj.ECSFetch.Err, proj.Profile) } for _, s := range services { icon := s.Stoplight.String() @@ -572,12 +572,12 @@ func renderECSSection(proj state.ProjectState) string { // renderFetchError renders a failed fetch: the error itself, plus the login // hint when it looks like a credential problem rather than a service one. -func renderFetchError(err error, account string) string { +func renderFetchError(err error, profile string) string { out := iconRed.Render(stageIndent+"⚠ "+err.Error()) + "\n" if isAuthError(err) { loginCmd := "aws sso login" - if account != "" { - loginCmd = "aws sso login --profile " + account + if profile != "" { + loginCmd = "aws sso login --profile " + profile } out += hintStyle.Render(stageIndent+" run: "+loginCmd) + "\n" } @@ -615,7 +615,7 @@ func isAuthError(err error) bool { func (d Dashboard) renderStages(proj state.ProjectState, navList []navItem, navCursor int) string { p := proj.Pipeline if p.Err != nil && len(p.Stages) == 0 { - return renderFetchError(p.Err, p.Account) + return renderFetchError(p.Err, proj.Profile) } if len(p.Stages) == 0 { return staleStyle.Render(stageIndent+"no stage data") + "\n" diff --git a/internal/ui/dashboard_test.go b/internal/ui/dashboard_test.go index 2c0023c..6246259 100644 --- a/internal/ui/dashboard_test.go +++ b/internal/ui/dashboard_test.go @@ -94,6 +94,7 @@ func TestStacksSectionSurfacesFetchError(t *testing.T) { proj := state.ProjectState{ Name: "annex-ims", Account: "libnd", + Profile: "libnd-view", StacksFetch: state.FetchStatus{StaleAt: &staleAt, Err: errors.New("sso session has expired")}, } @@ -104,7 +105,7 @@ func TestStacksSectionSurfacesFetchError(t *testing.T) { if !strings.Contains(out, "sso session has expired") { t.Errorf("expected the error in the section, got %q", out) } - if !strings.Contains(out, "aws sso login --profile libnd") { + if !strings.Contains(out, "aws sso login --profile libnd-view") { t.Errorf("expected the login hint for an auth error, got %q", out) } } @@ -191,3 +192,36 @@ func TestProjectRowFlagsStackTrouble(t *testing.T) { } } } + +// The login hint names the credential profile, not the Account: they differ +// (Account libnd uses profile libnd-view), and the Account name isn't a +// profile aws can log in to. +func TestLoginHintNamesTheProfile(t *testing.T) { + staleAt := time.Now() + expired := state.FetchStatus{StaleAt: &staleAt, Err: errors.New("the SSO session has expired")} + proj := state.ProjectState{ + Name: "annex-ims", + Account: "libnd", + Profile: "libnd-view", + Pipeline: state.PipelineState{FetchStatus: expired}, + StacksFetch: expired, + ECSFetch: expired, + } + d := NewDashboard(state.NewFromProjects([]state.ProjectState{proj}), nil, context.Background()) + + for name, out := range map[string]string{ + "pipeline": d.renderStages(proj, d.buildNavList(), -1), + "stacks": renderStacksSection(proj), + "ecs": renderECSSection(proj), + } { + if !strings.Contains(out, "aws sso login --profile libnd-view") { + t.Errorf("%s: expected the libnd-view profile in the hint, got %q", name, out) + } + } + + // A project with no Account uses the default credential chain. + proj.Account, proj.Profile = "", "" + if out := renderStacksSection(proj); !strings.Contains(out, "aws sso login") || strings.Contains(out, "--profile") { + t.Errorf("default credentials: expected a plain aws sso login, got %q", out) + } +} From 393bf7ba6babcdc57b54f8cebab2b7db0b3fa70f Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Tue, 29 Sep 2026 14:19:29 -0400 Subject: [PATCH 2/4] fix: offer the stalled-stack fix at the configured stuck threshold fix.Plan used a hard-coded 30 minutes, so with stuck_threshold_minutes set lower a stack alerted as stuck well before the f key offered to cancel its update. Plan now takes the threshold; the dashboard gets it from config at start and on every reload. Closes #65 --- internal/config/config.go | 7 +++++++ internal/fix/approval_test.go | 3 ++- internal/fix/fix.go | 17 ++++++++--------- internal/fix/fix_test.go | 35 +++++++++++++++++++++++++---------- internal/poller/poller.go | 3 +-- internal/ui/dashboard.go | 16 +++++++++++++++- internal/ui/dashboard_test.go | 18 ++++++++++++++++++ main.go | 3 ++- 8 files changed, 78 insertions(+), 24 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 624632c..fac9420 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -8,6 +8,7 @@ import ( "path/filepath" "slices" "strings" + "time" "github.com/BurntSushi/toml" ) @@ -57,6 +58,12 @@ func (p Project) IsEnabled() bool { return p.Enabled == nil || *p.Enabled } +// StuckThreshold is how long a resource stays in trouble before it counts as +// Stuck: its webhook fires, and the fix key offers to cancel a stalled update. +func (s Settings) StuckThreshold() time.Duration { + return time.Duration(s.StuckThresholdMinutes) * time.Minute +} + // Webhook is an endpoint that receives stuck-resource events. Secret is // optional; when set, requests are signed with HMAC-SHA256 over the body. type Webhook struct { diff --git a/internal/fix/approval_test.go b/internal/fix/approval_test.go index e3705e8..4d1b8ea 100644 --- a/internal/fix/approval_test.go +++ b/internal/fix/approval_test.go @@ -3,6 +3,7 @@ package fix_test import ( "context" "testing" + "time" "github.com/ericdahl-dev/aws-green/internal/fix" "github.com/ericdahl-dev/aws-green/internal/health" @@ -62,7 +63,7 @@ func TestPlanApproval_reject(t *testing.T) { // Approvals are driven by their own keys; the smart fix must not approve // anything on the user's behalf. func TestPlan_doesNotApprove(t *testing.T) { - if plan := fix.Plan(approvalProject("tok-123")); plan != nil { + if plan := fix.Plan(approvalProject("tok-123"), 30*time.Minute); plan != nil { t.Errorf("expected no smart fix for a pending approval, got %+v", plan) } } diff --git a/internal/fix/fix.go b/internal/fix/fix.go index b8ccfb5..445176a 100644 --- a/internal/fix/fix.go +++ b/internal/fix/fix.go @@ -78,14 +78,15 @@ type FixPlan struct { // Plan inspects a ProjectState and returns the highest-priority FixPlan, or nil if nothing to fix. // Precedence: rollback-failed stacks → pipeline → other stacks → ECS. // Rollback-failed stacks rank above pipeline because restarting the pipeline would fail -// immediately until the stack is recovered. -func Plan(proj state.ProjectState) *FixPlan { +// immediately until the stack is recovered. A stack update counts as stalled once it +// has run longer than stalledAfter — the same stuck threshold that raises its alert. +func Plan(proj state.ProjectState, stalledAfter time.Duration) *FixPlan { var plan *FixPlan // 1. Rollback-failed stacks — must be resolved before any pipeline restart can succeed. for _, s := range proj.Stacks { if s.Status.RollbackFailed() { - if p := planStack(s); p != nil { + if p := planStack(s, stalledAfter); p != nil { plan = p break } @@ -100,7 +101,7 @@ func Plan(proj state.ProjectState) *FixPlan { // 3. Other stacks (stalled in-progress) if plan == nil { for _, s := range proj.Stacks { - if p := planStack(s); p != nil { + if p := planStack(s, stalledAfter); p != nil { plan = p break } @@ -159,9 +160,7 @@ func PlanApproval(proj state.ProjectState, approve bool) *FixPlan { } } -const stalledStackThreshold = 30 * time.Minute - -func planStack(s state.StackState) *FixPlan { +func planStack(s state.StackState, stalledAfter time.Duration) *FixPlan { // Stuck rollback — ContinueUpdateRollback if s.Status.RollbackFailed() { return &FixPlan{ @@ -171,8 +170,8 @@ func planStack(s state.StackState) *FixPlan { } } - // In-progress > 30 min — CancelUpdateStack - if s.StartedAt != nil && s.Status.Cancellable() && time.Since(*s.StartedAt) > stalledStackThreshold { + // Update running past the stuck threshold — CancelUpdateStack + if s.StartedAt != nil && s.Status.Cancellable() && time.Since(*s.StartedAt) > stalledAfter { elapsed := time.Since(*s.StartedAt).Round(time.Second) return &FixPlan{ Kind: KindCancelStackUpdate, diff --git a/internal/fix/fix_test.go b/internal/fix/fix_test.go index f037946..a1f6c43 100644 --- a/internal/fix/fix_test.go +++ b/internal/fix/fix_test.go @@ -20,7 +20,7 @@ func proj(pipeline health.Stoplight, stacks []state.StackState, ecs []state.ECSS func TestPlan_noActionWhenGreen(t *testing.T) { p := proj(health.StoplightGreen, nil, nil) - plan := fix.Plan(p) + plan := fix.Plan(p, 30*time.Minute) if plan != nil { t.Errorf("expected nil plan for green project, got %v", plan) } @@ -28,7 +28,7 @@ func TestPlan_noActionWhenGreen(t *testing.T) { func TestPlan_noActionWhenGrey(t *testing.T) { p := proj(health.StoplightGrey, nil, nil) - plan := fix.Plan(p) + plan := fix.Plan(p, 30*time.Minute) if plan != nil { t.Errorf("expected nil plan for grey project, got %v", plan) } @@ -36,7 +36,7 @@ func TestPlan_noActionWhenGrey(t *testing.T) { func TestPlan_restartFailedPipeline(t *testing.T) { p := proj(health.StoplightRed, nil, nil) - plan := fix.Plan(p) + plan := fix.Plan(p, 30*time.Minute) if plan == nil { t.Fatal("expected plan for red pipeline") } @@ -52,7 +52,7 @@ func TestPlan_forceDeployDownECS(t *testing.T) { p := proj(health.StoplightGreen, nil, []state.ECSServiceState{ {Name: "web", Cluster: "my-cluster", RunningCount: 0, DesiredCount: 3, Stoplight: health.StoplightRed}, }) - plan := fix.Plan(p) + plan := fix.Plan(p, 30*time.Minute) if plan == nil { t.Fatal("expected plan for red ECS service") } @@ -65,7 +65,7 @@ func TestPlan_forceDeployStalledECS(t *testing.T) { p := proj(health.StoplightGreen, nil, []state.ECSServiceState{ {Name: "web", Cluster: "my-cluster", ActiveDeployment: true, RunningCount: 2, DesiredCount: 2, Stoplight: health.StoplightYellow}, }) - plan := fix.Plan(p) + plan := fix.Plan(p, 30*time.Minute) if plan == nil { t.Fatal("expected plan for yellow ECS service") } @@ -78,7 +78,7 @@ func TestPlan_continueRollbackStack(t *testing.T) { p := proj(health.StoplightGreen, []state.StackState{ {Name: "my-stack", Status: "UPDATE_ROLLBACK_FAILED", Stoplight: health.StoplightRed}, }, nil) - plan := fix.Plan(p) + plan := fix.Plan(p, 30*time.Minute) if plan == nil { t.Fatal("expected plan for failed rollback stack") } @@ -92,7 +92,7 @@ func TestPlan_cancelStalledStack(t *testing.T) { p := proj(health.StoplightGreen, []state.StackState{ {Name: "my-stack", Status: "UPDATE_IN_PROGRESS", StartedAt: &stale, Stoplight: health.StoplightYellow}, }, nil) - plan := fix.Plan(p) + plan := fix.Plan(p, 30*time.Minute) if plan == nil { t.Fatal("expected plan for stalled stack") } @@ -109,7 +109,7 @@ func TestPlan_noCancelForStalledNonUpdate(t *testing.T) { p := proj(health.StoplightGreen, []state.StackState{ {Name: "my-stack", Status: status, StartedAt: &stale, Stoplight: status.Stoplight()}, }, nil) - if plan := fix.Plan(p); plan != nil { + if plan := fix.Plan(p, 30*time.Minute); plan != nil { t.Errorf("%s: expected no plan, got %v", status, plan.Kind) } } @@ -119,7 +119,7 @@ func TestPlan_pipelineTakesPrecedenceOverECS(t *testing.T) { p := proj(health.StoplightRed, nil, []state.ECSServiceState{ {Name: "web", Cluster: "c", RunningCount: 0, DesiredCount: 1, Stoplight: health.StoplightRed}, }) - plan := fix.Plan(p) + plan := fix.Plan(p, 30*time.Minute) if plan == nil { t.Fatal("expected plan") } @@ -138,7 +138,7 @@ func TestPlan_credentialsFromProject(t *testing.T) { Stoplight: health.StoplightRed, }, } - plan := fix.Plan(p) + plan := fix.Plan(p, 30*time.Minute) if plan == nil { t.Fatal("expected plan for red pipeline") } @@ -149,3 +149,18 @@ func TestPlan_credentialsFromProject(t *testing.T) { t.Errorf("expected Region=us-east-1, got %q", plan.Region) } } + +// The cancel fix is offered at the same threshold that makes the stack +// Stuck — the user's stuck_threshold_minutes — not a fixed 30 minutes. +func TestPlan_stalledStackUsesTheGivenThreshold(t *testing.T) { + started := time.Now().Add(-12 * time.Minute) + p := proj(health.StoplightGreen, []state.StackState{ + {Name: "my-stack", Status: "UPDATE_IN_PROGRESS", StartedAt: &started, Stoplight: health.StoplightYellow}, + }, nil) + if plan := fix.Plan(p, 10*time.Minute); plan == nil || plan.Kind != fix.KindCancelStackUpdate { + t.Errorf("12m into a 10m threshold: got %+v, want a cancel plan", plan) + } + if plan := fix.Plan(p, 15*time.Minute); plan != nil { + t.Errorf("12m into a 15m threshold: got %+v, want no plan", plan) + } +} diff --git a/internal/poller/poller.go b/internal/poller/poller.go index 8615fe5..893300f 100644 --- a/internal/poller/poller.go +++ b/internal/poller/poller.go @@ -276,8 +276,7 @@ func (p *Poller) poll(ctx context.Context, ch chan<- state.Snapshot) { p.mu.Lock() p.current = updated - threshold := time.Duration(cfg.Settings.StuckThresholdMinutes) * time.Minute - events := p.stuck.evaluate(updated, threshold, now) + events := p.stuck.evaluate(updated, cfg.Settings.StuckThreshold(), now) dispatcher := p.dispatcher p.mu.Unlock() diff --git a/internal/ui/dashboard.go b/internal/ui/dashboard.go index 8889211..74f9b7f 100644 --- a/internal/ui/dashboard.go +++ b/internal/ui/dashboard.go @@ -10,6 +10,7 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/ericdahl-dev/aws-green/internal/config" "github.com/ericdahl-dev/aws-green/internal/fix" "github.com/ericdahl-dev/aws-green/internal/health" "github.com/ericdahl-dev/aws-green/internal/state" @@ -76,6 +77,10 @@ type Dashboard struct { actionerFactory fix.ActionerFactory fixCtx context.Context + // stuckThreshold is how long a stack update runs before `f` offers to + // cancel it — the configured stuck threshold, so the fix appears when + // the alert does. + stuckThreshold time.Duration fixStatus fixState fixPlan *fix.FixPlan @@ -91,11 +96,20 @@ func NewDashboard(snap state.Snapshot, actionerFactory fix.ActionerFactory, ctx lastActivity: time.Now(), actionerFactory: actionerFactory, fixCtx: ctx, + stuckThreshold: config.DefaultStuckThresholdMinutes * time.Minute, } d.applySnapshot(snap) return d } +// SetStuckThreshold sets how long a stack update runs before the fix key +// offers to cancel it. Call it with the configured stuck threshold at start +// and after every config change. +func (d Dashboard) SetStuckThreshold(threshold time.Duration) Dashboard { + d.stuckThreshold = threshold + return d +} + // needsAttention reports whether a project should have its row opened for the // user: it is broken, moving, or waiting on a person. func needsAttention(s health.Stoplight) bool { @@ -312,7 +326,7 @@ func (d Dashboard) Update(msg tea.Msg) (Dashboard, tea.Cmd) { } case "f": if proj := d.selectedProject(); proj != nil { - if plan := fix.Plan(*proj); plan != nil { + if plan := fix.Plan(*proj, d.stuckThreshold); plan != nil { d.fixStatus = fixConfirming d.fixPlan = plan } diff --git a/internal/ui/dashboard_test.go b/internal/ui/dashboard_test.go index 6246259..fea7bb9 100644 --- a/internal/ui/dashboard_test.go +++ b/internal/ui/dashboard_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/ericdahl-dev/aws-green/internal/fix" "github.com/ericdahl-dev/aws-green/internal/health" "github.com/ericdahl-dev/aws-green/internal/state" ) @@ -225,3 +226,20 @@ func TestLoginHintNamesTheProfile(t *testing.T) { t.Errorf("default credentials: expected a plain aws sso login, got %q", out) } } + +// The `f` key offers to cancel a stalled update at the configured stuck +// threshold, the same point its stuck alert fires. +func TestFixKeyUsesTheStuckThreshold(t *testing.T) { + started := time.Now().Add(-12 * time.Minute) + snap := state.NewFromProjects([]state.ProjectState{{ + Name: "app", + Stacks: []state.StackState{ + {Name: "app-stack", Status: "UPDATE_IN_PROGRESS", StartedAt: &started, Stoplight: health.StoplightYellow}, + }, + }}) + d := NewDashboard(snap, nil, context.Background()).SetStuckThreshold(10 * time.Minute) + d, _ = d.Update(key("f")) + if d.fixStatus != fixConfirming || d.fixPlan == nil || d.fixPlan.Kind != fix.KindCancelStackUpdate { + t.Errorf("status=%v plan=%+v, want a cancel-update confirmation", d.fixStatus, d.fixPlan) + } +} diff --git a/main.go b/main.go index dd71124..b389098 100644 --- a/main.go +++ b/main.go @@ -94,6 +94,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case ui.ConfigChangedMsg: m.cfg = msg.Config + m.dashboard = m.dashboard.SetStuckThreshold(m.cfg.Settings.StuckThreshold()) m.poller.ReloadConfig(m.cfg, m.pollCtx, m.pollChWrite) m.fetching = true cmds = append(cmds, kickSpinner(m.spinner)) @@ -297,7 +298,7 @@ func main() { m := model{ screen: screenDashboard, - dashboard: ui.NewDashboard(p.Snapshot(), actionerFactory, ctx), + dashboard: ui.NewDashboard(p.Snapshot(), actionerFactory, ctx).SetStuckThreshold(cfg.Settings.StuckThreshold()), manage: ui.NewManage(cfg, discover.Run), cfg: cfg, pollCh: writeCh, From 975a6cee589045e4ac451cd35e2eda54eec336be Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Tue, 29 Sep 2026 14:20:13 -0400 Subject: [PATCH 3/4] fix: run one poll cycle at a time ForceRefresh and ReloadConfig started extra poll goroutines alongside the ticker's. Whichever finished last won, so a slow older cycle could overwrite a newer one. A cycle-long mutex, separate from the state lock so Snapshot() isn't held up by fetches, now serializes them. Closes #63 --- internal/poller/poller.go | 8 ++++++ internal/poller/poller_test.go | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/internal/poller/poller.go b/internal/poller/poller.go index 893300f..ae14517 100644 --- a/internal/poller/poller.go +++ b/internal/poller/poller.go @@ -35,6 +35,11 @@ type Poller struct { cfnFactory CFNClientFactory ecsFactory ECSClientFactory mu sync.Mutex + // pollMu serializes poll cycles. A refresh or reload can ask for a + // cycle while the ticker's is in flight; run together, whichever + // finished last would win, so an older cycle could overwrite a newer + // one. It is separate from mu so Snapshot() isn't blocked by fetches. + pollMu sync.Mutex current []state.ProjectState dispatcher *webhooks.Dispatcher stuck *stuckTracker @@ -242,6 +247,9 @@ func configuredServices(carried []state.ECSServiceState, cfgECS []config.ECSConf } func (p *Poller) poll(ctx context.Context, ch chan<- state.Snapshot) { + p.pollMu.Lock() + defer p.pollMu.Unlock() + // Take cfg and current state under the lock. p.cfg is the poller's own copy, // which ReloadConfig replaces wholesale rather than edits, so it is safe to // read for the rest of the cycle. diff --git a/internal/poller/poller_test.go b/internal/poller/poller_test.go index 98b53bb..37759d4 100644 --- a/internal/poller/poller_test.go +++ b/internal/poller/poller_test.go @@ -732,3 +732,54 @@ func TestPollerOwnsItsConfig(t *testing.T) { t.Errorf("poll saw an edit made outside ReloadConfig: fetched %v", got) } } + +// firstCallGatedFetcher holds its first fetch until released and answers it +// with a failure; later fetches answer at once with success. It stands in for +// a slow poll cycle that a refresh overtakes. +type firstCallGatedFetcher struct { + mu sync.Mutex + calls int + entered chan struct{} + release chan struct{} +} + +func (f *firstCallGatedFetcher) FetchPipeline(_ context.Context, name string) (state.PipelineState, error) { + f.mu.Lock() + f.calls++ + first := f.calls == 1 + f.mu.Unlock() + if first { + close(f.entered) + <-f.release + return pipelineData(name, health.StatusFailed), nil + } + return pipelineData(name, health.StatusSucceeded), nil +} + +// A refresh requested while a cycle is in flight must not race it: whatever +// the refresh saw is newer, and the slow cycle finishing last must not +// overwrite it. +func TestRefreshDoesNotRaceAnInFlightPoll(t *testing.T) { + cfg := loadConfig(t, ` +[[projects]] +name = "alpha" +[projects.pipeline] +name = "alpha-pipeline" +`) + f := &firstCallGatedFetcher{entered: make(chan struct{}), release: make(chan struct{})} + p := New(cfg, pipelineFactory(f), nil, nil) + ctx := context.Background() + ch := make(chan state.Snapshot, 4) + + go p.poll(ctx, ch) // slow cycle, will report red + <-f.entered + p.ForceRefresh(ctx, ch) // newer cycle, will report green + time.Sleep(50 * time.Millisecond) + close(f.release) + <-ch + <-ch + + if got := p.Snapshot().Projects[0].Pipeline.Stoplight; got != health.StoplightGreen { + t.Errorf("final state = %v, want the refresh's green, not the slow cycle's red", got) + } +} From e2e5c6ac0716aafe3ed0456703fc6d4bb9907879 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Tue, 29 Sep 2026 14:21:45 -0400 Subject: [PATCH 4/4] fix: apply a changed poll interval without a restart Start read the interval once, so a new poll_interval_seconds from the manage screen was ignored until restart. ReloadConfig now hands a changed interval to the running ticker. An unchanged one is not sent, so a run of edits can't keep resetting the ticker and postpone polling. Closes #64 --- internal/poller/poller.go | 22 ++++++++++++++- internal/poller/poller_test.go | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/internal/poller/poller.go b/internal/poller/poller.go index ae14517..b1d5561 100644 --- a/internal/poller/poller.go +++ b/internal/poller/poller.go @@ -39,7 +39,10 @@ type Poller struct { // cycle while the ticker's is in flight; run together, whichever // finished last would win, so an older cycle could overwrite a newer // one. It is separate from mu so Snapshot() isn't blocked by fetches. - pollMu sync.Mutex + pollMu sync.Mutex + // intervals hands a changed poll interval from ReloadConfig to the + // running ticker in Start. + intervals chan time.Duration current []state.ProjectState dispatcher *webhooks.Dispatcher stuck *stuckTracker @@ -75,6 +78,7 @@ func New(cfg *config.Config, factory ClientFactory, cfnFactory CFNClientFactory, dispatcher: webhooks.New(cfg.Webhooks), stuck: newStuckTracker(), clients: make(map[string]accountClients), + intervals: make(chan time.Duration, 1), now: time.Now, } } @@ -104,6 +108,8 @@ func (p *Poller) Start(ctx context.Context) (<-chan state.Snapshot, context.Canc select { case <-ctx.Done(): return + case d := <-p.intervals: + ticker.Reset(d) case <-ticker.C: p.poll(ctx, ch) } @@ -124,6 +130,9 @@ func (p *Poller) ReloadConfig(cfg *config.Config, ctx context.Context, ch chan<- cfg = cfg.Clone() p.mu.Lock() prev := p.current + if cfg.Settings.PollInterval != p.cfg.Settings.PollInterval { + p.setInterval(time.Duration(cfg.Settings.PollInterval) * time.Second) + } p.cfg = cfg p.dispatcher = webhooks.New(cfg.Webhooks) enabled := cfg.EnabledProjects() @@ -160,6 +169,17 @@ func (p *Poller) clientsFor(profile, region string) accountClients { return c } +// setInterval queues d for the running ticker, replacing any change it hasn't +// picked up yet. Only a changed interval is sent: resetting on every reload +// would let a run of edits postpone polling indefinitely. +func (p *Poller) setInterval(d time.Duration) { + select { + case <-p.intervals: + default: + } + p.intervals <- d +} + // prevProject returns the last known state for a project, matched on the // account-qualified identity. Matching by identity rather than by index keeps // carried-forward state correct when a project is enabled or disabled and the diff --git a/internal/poller/poller_test.go b/internal/poller/poller_test.go index 37759d4..40cd972 100644 --- a/internal/poller/poller_test.go +++ b/internal/poller/poller_test.go @@ -783,3 +783,54 @@ name = "alpha-pipeline" t.Errorf("final state = %v, want the refresh's green, not the slow cycle's red", got) } } + +// A new poll interval from a config reload applies to the running ticker. +func TestReloadConfigAppliesNewPollInterval(t *testing.T) { + cfg := loadConfig(t, twoProjectConfig) + cfg.Settings.PollInterval = 3600 + p := New(cfg, pipelineFactory(&fakePipelineFetcher{}), nil, nil) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + ch, stop := p.Start(ctx) + defer stop() + <-ch // the immediate first poll + + faster := cfg.Clone() + faster.Settings.PollInterval = 1 + p.ReloadConfig(faster, ctx, make(chan state.Snapshot, 4)) + + select { + case <-ch: + case <-time.After(3 * time.Second): + t.Fatal("no scheduled poll within 3s of switching to a 1s interval") + } +} + +// Saving edits that leave the interval alone must not reset the ticker, or a +// run of edits faster than the interval would stop scheduled polls entirely. +func TestReloadConfigKeepsTickerWhenIntervalUnchanged(t *testing.T) { + cfg := loadConfig(t, twoProjectConfig) + cfg.Settings.PollInterval = 1 + p := New(cfg, pipelineFactory(&fakePipelineFetcher{}), nil, nil) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + ch, stop := p.Start(ctx) + defer stop() + <-ch + + done := make(chan struct{}) + go func() { + defer close(done) + for i := 0; i < 10; i++ { + p.ReloadConfig(cfg, ctx, make(chan state.Snapshot, 4)) + time.Sleep(250 * time.Millisecond) + } + }() + select { + case <-ch: + case <-done: + t.Fatal("reloads with an unchanged interval held off every scheduled poll") + } +}