Skip to content

Commit 4ea3460

Browse files
committed
Add rudimentary grace period support
This is important to make the backend runner able to support partial traces; the default CommandContext setup always sigkills, and we need to sigterm things like rmem to get them to dump a partial result. Seems to work; could do with some testing, but how?, and refactoring, but how?. This isn't yet supported in the machine node, mainly because that doesn't quite support the service.Runner idiom yet. See golang/go#22757.
1 parent 3300905 commit 4ea3460

File tree

5 files changed

+142
-21
lines changed

5 files changed

+142
-21
lines changed

internal/app/backend/backend.go

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ import (
1313
"io"
1414
"io/ioutil"
1515
"os"
16+
"time"
17+
18+
"github.com/MattWindsor91/c4t/internal/act"
1619

1720
"github.com/MattWindsor91/c4t/internal/helper/errhelp"
1821

@@ -60,6 +63,12 @@ const (
6063
flagDryRun = "dry-run"
6164
flagDryRunShort = "d"
6265
usageDryRun = "if true, print any external commands run instead of running them"
66+
flagTimeout = "timeout"
67+
flagTimeoutShort = "t"
68+
usageTimeout = "`DURATION` to wait before trying to stop the backend"
69+
flagGrace = "grace"
70+
flagGraceShort = "g"
71+
usageGrace = "`DURATION` to wait between sigterm and sigkill when timing out"
6372
)
6473

6574
// App is the c4-backend app.
@@ -83,6 +92,8 @@ func flags() []c.Flag {
8392
&c.GenericFlag{Name: flagBackendIDGlob, Aliases: []string{flagBackendIDGlobShort}, Usage: usageBackendIDGlob, Value: &id.ID{}},
8493
&c.GenericFlag{Name: flagBackendStyleGlob, Aliases: []string{flagBackendStyleGlobShort}, Usage: usageBackendStyleGlob, Value: &id.ID{}},
8594
&c.BoolFlag{Name: flagDryRun, Aliases: []string{flagDryRunShort}, Usage: usageDryRun},
95+
&c.DurationFlag{Name: flagTimeout, Aliases: []string{flagTimeoutShort}, Usage: usageTimeout},
96+
&c.DurationFlag{Name: flagGrace, Aliases: []string{flagGraceShort}, Usage: usageGrace},
8697
}
8798
return append(ownFlags, stdflag.ActRunnerCliFlags()...)
8899
}
@@ -93,50 +104,60 @@ func run(ctx *c.Context, outw io.Writer, errw io.Writer) error {
93104
return fmt.Errorf("while getting config: %w", err)
94105
}
95106
c4f := stdflag.ActRunnerFromCli(ctx, errw)
96-
cri := criteriaFromCli(ctx)
97-
fn, err := inputNameFromCli(ctx)
107+
108+
td, err := ioutil.TempDir("", "c4t-backend")
98109
if err != nil {
99110
return err
100111
}
101-
arch := idFromCli(ctx, flagArchID)
102112

103-
bspec, b, err := getBackend(cfg, cri)
113+
fn, err := inputNameFromCli(ctx)
104114
if err != nil {
105115
return err
106116
}
107117

108-
in, err := backend.InputFromFile(ctx.Context, fn, c4f)
118+
bspec, b, err := getBackend(cfg, criteriaFromCli(ctx))
109119
if err != nil {
110120
return err
111121
}
112122

113-
td, err := ioutil.TempDir("", "c4t-backend")
123+
j, err := jobFromCli(ctx, fn, c4f, bspec, td)
114124
if err != nil {
115125
return err
116126
}
127+
perr := runParseAndDump(ctx, outw, b, j, makeRunner(ctx, errw))
128+
derr := os.RemoveAll(td)
129+
return errhelp.FirstError(perr, derr)
130+
}
131+
132+
func jobFromCli(ctx *c.Context, fn string, c4f *act.Runner, bspec *backend.Spec, td string) (backend.LiftJob, error) {
133+
in, err := backend.InputFromFile(ctx.Context, fn, c4f)
134+
if err != nil {
135+
return backend.LiftJob{}, err
136+
}
137+
117138
j := backend.LiftJob{
118139
Backend: bspec,
119-
Arch: arch,
140+
Arch: idFromCli(ctx, flagArchID),
120141
In: in,
121142
Out: backend.LiftOutput{Dir: td, Target: backend.ToStandalone},
122143
}
123-
xr := makeRunner(ctx, errw)
124-
perr := runParseAndDump(ctx, outw, b, j, xr)
125-
derr := os.RemoveAll(td)
126-
return errhelp.FirstError(perr, derr)
144+
return j, nil
127145
}
128146

129147
func makeRunner(ctx *c.Context, errw io.Writer) service.Runner {
130148
// TODO(@MattWindsor91): the backend logic isn't very resilient against having external commands not run.
131149
if ctx.Bool(flagDryRun) {
132150
return srvrun.DryRunner{Writer: errw}
133151
}
134-
return srvrun.NewExecRunner(srvrun.StderrTo(errw))
152+
// TODO(@MattWindsor91): use grace in the rest of c4t
153+
return srvrun.NewExecRunner(srvrun.StderrTo(errw), srvrun.WithGrace(ctx.Duration(flagGrace)))
135154
}
136155

137156
func runParseAndDump(ctx *c.Context, outw io.Writer, b backend2.Backend, j backend.LiftJob, xr service.Runner) error {
138157
var o obs.Obs
139-
if err := runAndParse(ctx.Context, b, j, &o, xr); err != nil {
158+
159+
to := ctx.Duration(flagTimeout)
160+
if err := runAndParse(ctx.Context, to, b, j, &o, xr); err != nil {
140161
return err
141162
}
142163

@@ -145,9 +166,10 @@ func runParseAndDump(ctx *c.Context, outw io.Writer, b backend2.Backend, j backe
145166
return e.Encode(o)
146167
}
147168

148-
func runAndParse(ctx context.Context, b backend2.Backend, j backend.LiftJob, o *obs.Obs, xr service.Runner) error {
149-
// TODO(@MattWindsor91): deduplicate with runAndParseBin?.
150-
r, err := b.Lift(ctx, j, xr)
169+
func runAndParse(ctx context.Context, to time.Duration, b backend2.Backend, j backend.LiftJob, o *obs.Obs, xr service.Runner) error {
170+
// TODO(@MattWindsor91): clean this function up, eg making a separate struct...
171+
172+
r, err := liftWithTimeout(ctx, to, b, j, xr)
151173
if err != nil {
152174
return err
153175
}
@@ -164,6 +186,17 @@ func runAndParse(ctx context.Context, b backend2.Backend, j backend.LiftJob, o *
164186
return nil
165187
}
166188

189+
func liftWithTimeout(ctx context.Context, to time.Duration, b backend2.Backend, j backend.LiftJob, xr service.Runner) (recipe.Recipe, error) {
190+
cf := func() {}
191+
if to != 0 {
192+
ctx, cf = context.WithTimeout(ctx, to)
193+
}
194+
defer cf()
195+
196+
// TODO(@MattWindsor91): deduplicate with runAndParseBin?.
197+
return b.Lift(ctx, j, xr)
198+
}
199+
167200
func parseFile(ctx context.Context, b backend2.Backend, j backend.LiftJob, o *obs.Obs, fname string) error {
168201
f, err := os.Open(fname)
169202
if err != nil {

internal/helper/srvrun/dry.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package srvrun
88
import (
99
"context"
1010
"io"
11+
"time"
1112

1213
"github.com/MattWindsor91/c4t/internal/model/service"
1314
)
@@ -27,6 +28,11 @@ func (d DryRunner) WithStderr(_ io.Writer) service.Runner {
2728
return d
2829
}
2930

31+
// WithGrace just returns the same runner, ignoring the override.
32+
func (d DryRunner) WithGrace(_ time.Duration) service.Runner {
33+
return d
34+
}
35+
3036
// Run dumps r to the dry runner's writer.
3137
func (d DryRunner) Run(_ context.Context, r service.RunInfo) error {
3238
_, err := io.WriteString(d, r.String()+"\n")

internal/helper/srvrun/exec.go

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,19 @@ package srvrun
88
import (
99
"context"
1010
"io"
11+
"os"
1112
"os/exec"
13+
"runtime"
14+
"time"
1215

1316
"github.com/MattWindsor91/c4t/internal/model/service"
1417
)
1518

1619
// ExecRunner represents runs of services using exec.
1720
type ExecRunner struct {
18-
errw io.Writer
19-
outw io.Writer
21+
errw io.Writer
22+
outw io.Writer
23+
grace time.Duration
2024
}
2125

2226
// NewExecRunner constructs an exec-based runner using the options in os.
@@ -28,22 +32,68 @@ func NewExecRunner(os ...ExecOption) *ExecRunner {
2832

2933
// WithStderr returns a new ExecRunner with standard error rerouted to w.
3034
func (e ExecRunner) WithStderr(w io.Writer) service.Runner {
31-
return ExecRunner{outw: e.outw, errw: w}
35+
StderrTo(w)(&e)
36+
return e
37+
}
38+
39+
// WithGrace returns a new ExecRunner with the timeout grace period set as d.
40+
func (e ExecRunner) WithGrace(d time.Duration) service.Runner {
41+
WithGrace(d)(&e)
42+
return e
3243
}
3344

3445
// WithStdout returns a new ExecRunner with standard output rerouted to w.
3546
func (e ExecRunner) WithStdout(w io.Writer) service.Runner {
36-
return ExecRunner{outw: w, errw: e.errw}
47+
e.outw = w
48+
return e
49+
}
50+
51+
func (e ExecRunner) hasGrace() bool {
52+
return runtime.GOOS != "windows" && 0 < e.grace
3753
}
3854

3955
// Run runs the command specified by r using exec, on context ctx.
4056
func (e ExecRunner) Run(ctx context.Context, r service.RunInfo) error {
57+
if e.hasGrace() {
58+
return e.runWithGrace(ctx, r)
59+
}
4160
c := exec.CommandContext(ctx, r.Cmd, r.Args...)
4261
c.Stderr = e.errw
4362
c.Stdout = e.outw
4463
return c.Run()
4564
}
4665

66+
func (e ExecRunner) runWithGrace(ctx context.Context, r service.RunInfo) error {
67+
cdone := make(chan struct{})
68+
defer close(cdone)
69+
70+
c := exec.Command(r.Cmd, r.Args...)
71+
c.Stderr = e.errw
72+
c.Stdout = e.outw
73+
if err := c.Start(); err != nil {
74+
return err
75+
}
76+
77+
// https://github.com/golang/go/issues/22757#issuecomment-345009730
78+
go func() {
79+
select {
80+
case <-cdone:
81+
return
82+
case <-ctx.Done():
83+
}
84+
85+
t := time.AfterFunc(e.grace, func() {
86+
_ = c.Process.Kill()
87+
})
88+
_ = c.Process.Signal(os.Interrupt)
89+
90+
<-cdone
91+
t.Stop()
92+
}()
93+
94+
return c.Wait()
95+
}
96+
4797
// ExecOption is the type of options used when constructing a ExecRunner.
4898
type ExecOption func(*ExecRunner)
4999

@@ -62,3 +112,13 @@ func StderrTo(w io.Writer) ExecOption {
62112
l.errw = w
63113
}
64114
}
115+
116+
// WithGrace sets a timeout grace period of d.
117+
//
118+
// If an ExecRunner's context closes and it has a timeout grace period, and the OS supports it, it will SIGTERM the
119+
// program, wait d, and then SIGKILL.
120+
func WithGrace(d time.Duration) ExecOption {
121+
return func(l *ExecRunner) {
122+
l.grace = d
123+
}
124+
}

internal/model/service/mocks/Runner.go

Lines changed: 19 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/model/service/run_info.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
"io"
1111
"strings"
12+
"time"
1213
)
1314

1415
// Runner is the interface of things that can run, or pretend to run, services.
@@ -19,6 +20,9 @@ type Runner interface {
1920
// WithStderr should return a new runner with the standard error overridden to w.
2021
WithStderr(w io.Writer) Runner
2122

23+
// WithGrace should return a new runner with the timeout grace period set to d.
24+
WithGrace(d time.Duration) Runner
25+
2226
// Run runs r using context ctx.
2327
Run(ctx context.Context, r RunInfo) error
2428
}

0 commit comments

Comments
 (0)