Skip to content

Commit 786b7e2

Browse files
committed
Report Running to SCM only after supervisor startup succeeds
Execute starts Run in a goroutine and immediately sends svc.Running, so any failure inside Run's synchronous startup path (logger init, instance UID, auth, local OpAMP server, collector start) is reported only after SCM has already been told the service is up. Run now accepts an optional startedCh that it closes after sv.Start returns successfully. The Windows service handler selects on this channel and errCh, staying in StartPending until startup is confirmed and only then transitioning to Running. A 30s WaitHint is attached to the initial StartPending so SCM doesn't time out while real startup runs.
1 parent aa2e17a commit 786b7e2

3 files changed

Lines changed: 34 additions & 4 deletions

File tree

superv/command.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,5 +289,5 @@ func runSupervisor(cmd *cobra.Command, _ []string) error {
289289
ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
290290
defer stop()
291291

292-
return Run(ctx, cfg, events)
292+
return Run(ctx, cfg, events, nil)
293293
}

superv/run.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,14 @@ import (
3434

3535
// Run starts the supervisor and blocks until ctx is cancelled.
3636
// The caller controls the lifecycle: cancelling ctx triggers graceful shutdown.
37-
func Run(ctx context.Context, cfg config.Config, events []func(*zap.Logger)) error {
37+
//
38+
// If startedCh is non-nil, it is closed once synchronous startup has completed
39+
// successfully — i.e. after sv.Start returns and before Run blocks waiting for
40+
// ctx. Startup failures are reported via the returned error; startedCh is left
41+
// untouched. Callers that need to distinguish "started" from "failed during
42+
// startup" should select on both startedCh and the channel they use to receive
43+
// Run's return value.
44+
func Run(ctx context.Context, cfg config.Config, events []func(*zap.Logger), startedCh chan<- struct{}) error {
3845
logger, err := initLogger(cfg.Logging, cfg.Debug)
3946
if err != nil {
4047
return fmt.Errorf("failed to create logger: %w", err)
@@ -91,6 +98,10 @@ func Run(ctx context.Context, cfg config.Config, events []func(*zap.Logger)) err
9198
return fmt.Errorf("failed to start supervisor: %w", err)
9299
}
93100

101+
if startedCh != nil {
102+
close(startedCh)
103+
}
104+
94105
<-ctx.Done()
95106

96107
shutdownCtx, cancelShutdown := context.WithTimeout(context.Background(), cfg.Shutdown.GracefulTimeout)

superv/service_windows.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,11 @@ func NewSvcHandler() svc.Handler {
5555
type supervisorService struct{}
5656

5757
func (s *supervisorService) Execute(_ []string, r <-chan svc.ChangeRequest, changes chan<- svc.Status) (bool, uint32) {
58-
changes <- svc.Status{State: svc.StartPending}
58+
// Give SCM a generous WaitHint so it doesn't decide the service is hung
59+
// while Run is doing its synchronous startup (auth, OpAMP, collector).
60+
// Ideally we'd also send periodic StartPending heartbeats with a monotonically
61+
// increasing CheckPoint for full spec compliance.
62+
changes <- svc.Status{State: svc.StartPending, WaitHint: uint32((30 * time.Second).Milliseconds())}
5963

6064
elog, err := eventlog.Open(eventLogSourceName)
6165
if err != nil {
@@ -76,10 +80,25 @@ func (s *supervisorService) Execute(_ []string, r <-chan svc.ChangeRequest, chan
7680
defer cancel()
7781

7882
errCh := make(chan error, 1)
83+
startedCh := make(chan struct{})
7984
go func() {
80-
errCh <- Run(ctx, cfg, events)
85+
errCh <- Run(ctx, cfg, events, startedCh)
8186
}()
8287

88+
// Stay in StartPending until Run signals that synchronous startup has
89+
// completed, or returns early with a startup error. Reporting Running to
90+
// SCM before this point would falsely advertise the service as healthy.
91+
select {
92+
case <-startedCh:
93+
case err := <-errCh:
94+
if err != nil {
95+
_ = elog.Error(serviceStartError, "Startup error: "+err.Error())
96+
return true, 1
97+
}
98+
// Run returned nil before signaling start — treat as a clean exit.
99+
return false, 0
100+
}
101+
83102
changes <- svc.Status{
84103
State: svc.Running,
85104
Accepts: svc.AcceptStop | svc.AcceptShutdown,

0 commit comments

Comments
 (0)