Skip to content

Commit b15f8d2

Browse files
committed
Add timeouts for volume plugin ops
This protects the daemon from volume plugins that are slow or deadlocked. Signed-off-by: Brian Goff <[email protected]>
1 parent 274538c commit b15f8d2

2 files changed

Lines changed: 63 additions & 13 deletions

File tree

pkg/plugins/client.go

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ package plugins
22

33
import (
44
"bytes"
5+
"context"
56
"encoding/json"
67
"io"
78
"io/ioutil"
89
"net/http"
910
"net/url"
1011
"time"
1112

13+
"github.com/docker/docker/pkg/ioutils"
1214
"github.com/docker/docker/pkg/plugins/transport"
1315
"github.com/docker/go-connections/sockets"
1416
"github.com/docker/go-connections/tlsconfig"
@@ -82,16 +84,33 @@ type Client struct {
8284
requestFactory transport.RequestFactory
8385
}
8486

87+
// RequestOpts is the set of options that can be passed into a request
88+
type RequestOpts struct {
89+
Timeout time.Duration
90+
}
91+
92+
// WithRequestTimeout sets a timeout duration for plugin requests
93+
func WithRequestTimeout(t time.Duration) func(*RequestOpts) {
94+
return func(o *RequestOpts) {
95+
o.Timeout = t
96+
}
97+
}
98+
8599
// Call calls the specified method with the specified arguments for the plugin.
86100
// It will retry for 30 seconds if a failure occurs when calling.
87-
func (c *Client) Call(serviceMethod string, args interface{}, ret interface{}) error {
101+
func (c *Client) Call(serviceMethod string, args, ret interface{}) error {
102+
return c.CallWithOptions(serviceMethod, args, ret)
103+
}
104+
105+
// CallWithOptions is just like call except it takes options
106+
func (c *Client) CallWithOptions(serviceMethod string, args interface{}, ret interface{}, opts ...func(*RequestOpts)) error {
88107
var buf bytes.Buffer
89108
if args != nil {
90109
if err := json.NewEncoder(&buf).Encode(args); err != nil {
91110
return err
92111
}
93112
}
94-
body, err := c.callWithRetry(serviceMethod, &buf, true)
113+
body, err := c.callWithRetry(serviceMethod, &buf, true, opts...)
95114
if err != nil {
96115
return err
97116
}
@@ -128,18 +147,31 @@ func (c *Client) SendFile(serviceMethod string, data io.Reader, ret interface{})
128147
return nil
129148
}
130149

131-
func (c *Client) callWithRetry(serviceMethod string, data io.Reader, retry bool) (io.ReadCloser, error) {
150+
func (c *Client) callWithRetry(serviceMethod string, data io.Reader, retry bool, reqOpts ...func(*RequestOpts)) (io.ReadCloser, error) {
132151
var retries int
133152
start := time.Now()
134153

154+
var opts RequestOpts
155+
for _, o := range reqOpts {
156+
o(&opts)
157+
}
158+
135159
for {
136160
req, err := c.requestFactory.NewRequest(serviceMethod, data)
137161
if err != nil {
138162
return nil, err
139163
}
140164

165+
cancelRequest := func() {}
166+
if opts.Timeout > 0 {
167+
var ctx context.Context
168+
ctx, cancelRequest = context.WithTimeout(req.Context(), opts.Timeout)
169+
req = req.WithContext(ctx)
170+
}
171+
141172
resp, err := c.http.Do(req)
142173
if err != nil {
174+
cancelRequest()
143175
if !retry {
144176
return nil, err
145177
}
@@ -157,6 +189,7 @@ func (c *Client) callWithRetry(serviceMethod string, data io.Reader, retry bool)
157189
if resp.StatusCode != http.StatusOK {
158190
b, err := ioutil.ReadAll(resp.Body)
159191
resp.Body.Close()
192+
cancelRequest()
160193
if err != nil {
161194
return nil, &statusError{resp.StatusCode, serviceMethod, err.Error()}
162195
}
@@ -176,7 +209,11 @@ func (c *Client) callWithRetry(serviceMethod string, data io.Reader, retry bool)
176209
// old way...
177210
return nil, &statusError{resp.StatusCode, serviceMethod, string(b)}
178211
}
179-
return resp.Body, nil
212+
return ioutils.NewReadCloserWrapper(resp.Body, func() error {
213+
err := resp.Body.Close()
214+
cancelRequest()
215+
return err
216+
}), nil
180217
}
181218
}
182219

volume/drivers/proxy.go

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,19 @@ package volumedrivers
44

55
import (
66
"errors"
7+
"time"
78

9+
"github.com/docker/docker/pkg/plugins"
810
"github.com/docker/docker/volume"
911
)
1012

13+
const (
14+
longTimeout = 2 * time.Minute
15+
shortTimeout = 1 * time.Minute
16+
)
17+
1118
type client interface {
12-
Call(string, interface{}, interface{}) error
19+
CallWithOptions(string, interface{}, interface{}, ...func(*plugins.RequestOpts)) error
1320
}
1421

1522
type volumeDriverProxy struct {
@@ -33,7 +40,8 @@ func (pp *volumeDriverProxy) Create(name string, opts map[string]string) (err er
3340

3441
req.Name = name
3542
req.Opts = opts
36-
if err = pp.Call("VolumeDriver.Create", req, &ret); err != nil {
43+
44+
if err = pp.CallWithOptions("VolumeDriver.Create", req, &ret, plugins.WithRequestTimeout(longTimeout)); err != nil {
3745
return
3846
}
3947

@@ -59,7 +67,8 @@ func (pp *volumeDriverProxy) Remove(name string) (err error) {
5967
)
6068

6169
req.Name = name
62-
if err = pp.Call("VolumeDriver.Remove", req, &ret); err != nil {
70+
71+
if err = pp.CallWithOptions("VolumeDriver.Remove", req, &ret, plugins.WithRequestTimeout(shortTimeout)); err != nil {
6372
return
6473
}
6574

@@ -86,7 +95,8 @@ func (pp *volumeDriverProxy) Path(name string) (mountpoint string, err error) {
8695
)
8796

8897
req.Name = name
89-
if err = pp.Call("VolumeDriver.Path", req, &ret); err != nil {
98+
99+
if err = pp.CallWithOptions("VolumeDriver.Path", req, &ret, plugins.WithRequestTimeout(shortTimeout)); err != nil {
90100
return
91101
}
92102

@@ -117,7 +127,8 @@ func (pp *volumeDriverProxy) Mount(name string, id string) (mountpoint string, e
117127

118128
req.Name = name
119129
req.ID = id
120-
if err = pp.Call("VolumeDriver.Mount", req, &ret); err != nil {
130+
131+
if err = pp.CallWithOptions("VolumeDriver.Mount", req, &ret, plugins.WithRequestTimeout(longTimeout)); err != nil {
121132
return
122133
}
123134

@@ -147,7 +158,8 @@ func (pp *volumeDriverProxy) Unmount(name string, id string) (err error) {
147158

148159
req.Name = name
149160
req.ID = id
150-
if err = pp.Call("VolumeDriver.Unmount", req, &ret); err != nil {
161+
162+
if err = pp.CallWithOptions("VolumeDriver.Unmount", req, &ret, plugins.WithRequestTimeout(shortTimeout)); err != nil {
151163
return
152164
}
153165

@@ -172,7 +184,7 @@ func (pp *volumeDriverProxy) List() (volumes []*proxyVolume, err error) {
172184
ret volumeDriverProxyListResponse
173185
)
174186

175-
if err = pp.Call("VolumeDriver.List", req, &ret); err != nil {
187+
if err = pp.CallWithOptions("VolumeDriver.List", req, &ret, plugins.WithRequestTimeout(shortTimeout)); err != nil {
176188
return
177189
}
178190

@@ -201,7 +213,8 @@ func (pp *volumeDriverProxy) Get(name string) (volume *proxyVolume, err error) {
201213
)
202214

203215
req.Name = name
204-
if err = pp.Call("VolumeDriver.Get", req, &ret); err != nil {
216+
217+
if err = pp.CallWithOptions("VolumeDriver.Get", req, &ret, plugins.WithRequestTimeout(shortTimeout)); err != nil {
205218
return
206219
}
207220

@@ -228,7 +241,7 @@ func (pp *volumeDriverProxy) Capabilities() (capabilities volume.Capability, err
228241
ret volumeDriverProxyCapabilitiesResponse
229242
)
230243

231-
if err = pp.Call("VolumeDriver.Capabilities", req, &ret); err != nil {
244+
if err = pp.CallWithOptions("VolumeDriver.Capabilities", req, &ret, plugins.WithRequestTimeout(shortTimeout)); err != nil {
232245
return
233246
}
234247

0 commit comments

Comments
 (0)