From 83ca993c154d56e03d6f95a3f8351c48b3ed3e29 Mon Sep 17 00:00:00 2001 From: Anusha Ragunathan Date: Mon, 21 Nov 2016 09:24:01 -0800 Subject: [PATCH] Add HTTP client timeout. Signed-off-by: Anusha Ragunathan --- api/server/router/plugin/backend.go | 2 +- api/server/router/plugin/plugin_routes.go | 14 ++++++++++- api/swagger.yaml | 7 +++++- api/types/client.go | 5 ++++ api/types/configs.go | 5 ++++ cli/command/plugin/enable.go | 23 +++++++++++++++--- client/interface.go | 2 +- client/plugin_enable.go | 11 +++++++-- client/plugin_enable_test.go | 5 ++-- client/plugin_install.go | 2 +- hack/validate/swagger-gen | 4 ++-- pkg/plugins/client.go | 29 ++++++++++++++++++----- plugin/backend_linux.go | 5 +++- plugin/backend_unsupported.go | 2 +- plugin/manager_linux.go | 2 +- plugin/v2/plugin.go | 1 + 16 files changed, 96 insertions(+), 23 deletions(-) diff --git a/api/server/router/plugin/backend.go b/api/server/router/plugin/backend.go index acbb1e408c..cc931fe411 100644 --- a/api/server/router/plugin/backend.go +++ b/api/server/router/plugin/backend.go @@ -11,7 +11,7 @@ import ( // Backend for Plugin type Backend interface { Disable(name string) error - Enable(name string) error + Enable(name string, config *enginetypes.PluginEnableConfig) error List() ([]enginetypes.Plugin, error) Inspect(name string) (enginetypes.Plugin, error) Remove(name string, config *enginetypes.PluginRmConfig) error diff --git a/api/server/router/plugin/plugin_routes.go b/api/server/router/plugin/plugin_routes.go index 1af8dad6c6..ffa05dc984 100644 --- a/api/server/router/plugin/plugin_routes.go +++ b/api/server/router/plugin/plugin_routes.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "encoding/json" "net/http" + "strconv" "strings" "github.com/docker/docker/api/server/httputils" @@ -56,7 +57,18 @@ func (pr *pluginRouter) createPlugin(ctx context.Context, w http.ResponseWriter, } func (pr *pluginRouter) enablePlugin(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - return pr.backend.Enable(vars["name"]) + if err := httputils.ParseForm(r); err != nil { + return err + } + + name := vars["name"] + timeout, err := strconv.Atoi(r.Form.Get("timeout")) + if err != nil { + return err + } + config := &types.PluginEnableConfig{Timeout: timeout} + + return pr.backend.Enable(name, config) } func (pr *pluginRouter) disablePlugin(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { diff --git a/api/swagger.yaml b/api/swagger.yaml index 28f099aecf..02e7b91f56 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -6307,7 +6307,7 @@ paths: summary: "Install a plugin" operationId: "PostPluginsPull" description: | - Pulls and installs a plugin. After the plugin is installed, it can be enabled using the [`POST /plugins/{name}/enable` endpoint](#operation/PostPluginEnable). + Pulls and installs a plugin. After the plugin is installed, it can be enabled using the [`POST /plugins/{name}/enable` endpoint](#operation/PostPluginsEnable). produces: - "application/json" responses: @@ -6430,6 +6430,11 @@ paths: description: "The name of the plugin. The `:latest` tag is optional, and is the default if omitted." required: true type: "string" + - name: "timeout" + in: "query" + description: "Set the HTTP client timeout (in seconds)" + type: "integer" + default: 0 tags: - "Plugins" /plugins/{name}/disable: diff --git a/api/types/client.go b/api/types/client.go index 78a000cfad..82dd4cd1c2 100644 --- a/api/types/client.go +++ b/api/types/client.go @@ -332,6 +332,11 @@ type PluginRemoveOptions struct { Force bool } +// PluginEnableOptions holds parameters to enable plugins. +type PluginEnableOptions struct { + Timeout int +} + // PluginInstallOptions holds parameters to install a plugin. type PluginInstallOptions struct { Disabled bool diff --git a/api/types/configs.go b/api/types/configs.go index 177a62bb4f..4086126232 100644 --- a/api/types/configs.go +++ b/api/types/configs.go @@ -59,3 +59,8 @@ type ExecConfig struct { type PluginRmConfig struct { ForceRemove bool } + +// PluginEnableConfig holds arguments for the plugin enable +type PluginEnableConfig struct { + Timeout int +} diff --git a/cli/command/plugin/enable.go b/cli/command/plugin/enable.go index 0fd8f469dd..d84da24668 100644 --- a/cli/command/plugin/enable.go +++ b/cli/command/plugin/enable.go @@ -3,6 +3,7 @@ package plugin import ( "fmt" + "github.com/docker/docker/api/types" "github.com/docker/docker/cli" "github.com/docker/docker/cli/command" "github.com/docker/docker/reference" @@ -10,20 +11,32 @@ import ( "golang.org/x/net/context" ) +type enableOpts struct { + timeout int + name string +} + func newEnableCommand(dockerCli *command.DockerCli) *cobra.Command { + var opts enableOpts + cmd := &cobra.Command{ Use: "enable PLUGIN", Short: "Enable a plugin", Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runEnable(dockerCli, args[0]) + opts.name = args[0] + return runEnable(dockerCli, &opts) }, } + flags := cmd.Flags() + flags.IntVar(&opts.timeout, "timeout", 0, "HTTP client timeout (in seconds)") return cmd } -func runEnable(dockerCli *command.DockerCli, name string) error { +func runEnable(dockerCli *command.DockerCli, opts *enableOpts) error { + name := opts.name + named, err := reference.ParseNamed(name) // FIXME: validate if err != nil { return err @@ -35,7 +48,11 @@ func runEnable(dockerCli *command.DockerCli, name string) error { if !ok { return fmt.Errorf("invalid name: %s", named.String()) } - if err := dockerCli.Client().PluginEnable(context.Background(), ref.String()); err != nil { + if opts.timeout < 0 { + return fmt.Errorf("negative timeout %d is invalid", opts.timeout) + } + + if err := dockerCli.Client().PluginEnable(context.Background(), ref.String(), types.PluginEnableOptions{Timeout: opts.timeout}); err != nil { return err } fmt.Fprintln(dockerCli.Out(), name) diff --git a/client/interface.go b/client/interface.go index d46720e6c7..0d722d9075 100644 --- a/client/interface.go +++ b/client/interface.go @@ -109,7 +109,7 @@ type NodeAPIClient interface { type PluginAPIClient interface { PluginList(ctx context.Context) (types.PluginsListResponse, error) PluginRemove(ctx context.Context, name string, options types.PluginRemoveOptions) error - PluginEnable(ctx context.Context, name string) error + PluginEnable(ctx context.Context, name string, options types.PluginEnableOptions) error PluginDisable(ctx context.Context, name string) error PluginInstall(ctx context.Context, name string, options types.PluginInstallOptions) error PluginPush(ctx context.Context, name string, registryAuth string) error diff --git a/client/plugin_enable.go b/client/plugin_enable.go index 8109814ddb..95517c4b80 100644 --- a/client/plugin_enable.go +++ b/client/plugin_enable.go @@ -1,12 +1,19 @@ package client import ( + "net/url" + "strconv" + + "github.com/docker/docker/api/types" "golang.org/x/net/context" ) // PluginEnable enables a plugin -func (cli *Client) PluginEnable(ctx context.Context, name string) error { - resp, err := cli.post(ctx, "/plugins/"+name+"/enable", nil, nil, nil) +func (cli *Client) PluginEnable(ctx context.Context, name string, options types.PluginEnableOptions) error { + query := url.Values{} + query.Set("timeout", strconv.Itoa(options.Timeout)) + + resp, err := cli.post(ctx, "/plugins/"+name+"/enable", query, nil, nil) ensureReaderClosed(resp) return err } diff --git a/client/plugin_enable_test.go b/client/plugin_enable_test.go index d919914e75..b27681348f 100644 --- a/client/plugin_enable_test.go +++ b/client/plugin_enable_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/docker/docker/api/types" "golang.org/x/net/context" ) @@ -16,7 +17,7 @@ func TestPluginEnableError(t *testing.T) { client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")), } - err := client.PluginEnable(context.Background(), "plugin_name") + err := client.PluginEnable(context.Background(), "plugin_name", types.PluginEnableOptions{}) if err == nil || err.Error() != "Error response from daemon: Server error" { t.Fatalf("expected a Server Error, got %v", err) } @@ -40,7 +41,7 @@ func TestPluginEnable(t *testing.T) { }), } - err := client.PluginEnable(context.Background(), "plugin_name") + err := client.PluginEnable(context.Background(), "plugin_name", types.PluginEnableOptions{}) if err != nil { t.Fatal(err) } diff --git a/client/plugin_install.go b/client/plugin_install.go index 407f1cddf2..f73362ccd3 100644 --- a/client/plugin_install.go +++ b/client/plugin_install.go @@ -62,7 +62,7 @@ func (cli *Client) PluginInstall(ctx context.Context, name string, options types return nil } - return cli.PluginEnable(ctx, name) + return cli.PluginEnable(ctx, name, types.PluginEnableOptions{Timeout: 0}) } func (cli *Client) tryPluginPull(ctx context.Context, query url.Values, registryAuth string) (serverResponse, error) { diff --git a/hack/validate/swagger-gen b/hack/validate/swagger-gen index 5d6fadb6aa..37ced76e8c 100755 --- a/hack/validate/swagger-gen +++ b/hack/validate/swagger-gen @@ -14,12 +14,12 @@ if [ ${#files[@]} -gt 0 ]; then diffs="$(git status --porcelain -- api/types/ 2>/dev/null)" if [ "$diffs" ]; then { - echo 'The result of hack/geneate-swagger-api.sh differs' + echo 'The result of hack/generate-swagger-api.sh differs' echo echo "$diffs" echo echo 'Please update api/swagger.yaml with any api changes, then ' - echo 'run `hack/geneate-swagger-api.sh`.' + echo 'run `hack/generate-swagger-api.sh`.' } >&2 false else diff --git a/pkg/plugins/client.go b/pkg/plugins/client.go index a778677f7c..e8e730eb58 100644 --- a/pkg/plugins/client.go +++ b/pkg/plugins/client.go @@ -19,8 +19,7 @@ const ( defaultTimeOut = 30 ) -// NewClient creates a new plugin client (http). -func NewClient(addr string, tlsConfig *tlsconfig.Options) (*Client, error) { +func newTransport(addr string, tlsConfig *tlsconfig.Options) (transport.Transport, error) { tr := &http.Transport{} if tlsConfig != nil { @@ -45,15 +44,33 @@ func NewClient(addr string, tlsConfig *tlsconfig.Options) (*Client, error) { } scheme := httpScheme(u) - clientTransport := transport.NewHTTPTransport(tr, scheme, socket) - return NewClientWithTransport(clientTransport), nil + return transport.NewHTTPTransport(tr, scheme, socket), nil } -// NewClientWithTransport creates a new plugin client with a given transport. -func NewClientWithTransport(tr transport.Transport) *Client { +// NewClient creates a new plugin client (http). +func NewClient(addr string, tlsConfig *tlsconfig.Options) (*Client, error) { + clientTransport, err := newTransport(addr, tlsConfig) + if err != nil { + return nil, err + } + return newClientWithTransport(clientTransport, 0), nil +} + +// NewClientWithTimeout creates a new plugin client (http). +func NewClientWithTimeout(addr string, tlsConfig *tlsconfig.Options, timeoutInSecs int) (*Client, error) { + clientTransport, err := newTransport(addr, tlsConfig) + if err != nil { + return nil, err + } + return newClientWithTransport(clientTransport, timeoutInSecs), nil +} + +// newClientWithTransport creates a new plugin client with a given transport. +func newClientWithTransport(tr transport.Transport, timeoutInSecs int) *Client { return &Client{ http: &http.Client{ Transport: tr, + Timeout: time.Duration(timeoutInSecs) * time.Second, }, requestFactory: tr, } diff --git a/plugin/backend_linux.go b/plugin/backend_linux.go index 8c8f224561..59ca225a9c 100644 --- a/plugin/backend_linux.go +++ b/plugin/backend_linux.go @@ -36,11 +36,14 @@ func (pm *Manager) Disable(name string) error { } // Enable activates a plugin, which implies that they are ready to be used by containers. -func (pm *Manager) Enable(name string) error { +func (pm *Manager) Enable(name string, config *types.PluginEnableConfig) error { + p, err := pm.pluginStore.GetByName(name) if err != nil { return err } + + p.TimeoutInSecs = config.Timeout if err := pm.enable(p, false); err != nil { return err } diff --git a/plugin/backend_unsupported.go b/plugin/backend_unsupported.go index 2e9ca5b320..e54994fe75 100644 --- a/plugin/backend_unsupported.go +++ b/plugin/backend_unsupported.go @@ -19,7 +19,7 @@ func (pm *Manager) Disable(name string) error { } // Enable activates a plugin, which implies that they are ready to be used by containers. -func (pm *Manager) Enable(name string) error { +func (pm *Manager) Enable(name string, config *types.PluginEnableConfig) error { return errNotSupported } diff --git a/plugin/manager_linux.go b/plugin/manager_linux.go index 26f0c39f2e..6756dba995 100644 --- a/plugin/manager_linux.go +++ b/plugin/manager_linux.go @@ -31,7 +31,7 @@ func (pm *Manager) enable(p *v2.Plugin, force bool) error { return err } - p.PClient, err = plugins.NewClient("unix://"+filepath.Join(p.RuntimeSourcePath, p.GetSocket()), nil) + p.PClient, err = plugins.NewClientWithTimeout("unix://"+filepath.Join(p.RuntimeSourcePath, p.GetSocket()), nil, p.TimeoutInSecs) if err != nil { p.Lock() p.Restart = false diff --git a/plugin/v2/plugin.go b/plugin/v2/plugin.go index b48e56b8ac..56775fce62 100644 --- a/plugin/v2/plugin.go +++ b/plugin/v2/plugin.go @@ -24,6 +24,7 @@ type Plugin struct { Restart bool `json:"-"` ExitChan chan bool `json:"-"` LibRoot string `json:"-"` + TimeoutInSecs int `json:"-"` } const defaultPluginRuntimeDestination = "/run/docker/plugins"