Add HTTP client timeout.

Signed-off-by: Anusha Ragunathan <anusha@docker.com>
vboulineau · Nov 21, 2016 · 83ca993 · 83ca993
1 parent 0596301
commit 83ca993
Show file tree

Hide file tree

Showing 16 changed files with 96 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -3,27 +3,40 @@ 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"
 	"github.com/spf13/cobra"
 	"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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
+}
+
+// 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) *Client {
+// 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
@@ -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
@@ -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
@@ -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
@@ -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"