From 8ebc9331111395fedbeedb3a346b302b95b147cc Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Thu, 24 Sep 2015 13:52:07 +0900 Subject: [PATCH] etcdctl: use user specified timeout value for entire command execution etcdctl should be capable to use a user specified timeout value for total command execution, not only per request timeout. This commit adds a new option --total-timeout to the command. The value passed via this option is used as a timeout value of entire command execution. Fixes coreos#3517 --- etcdctl/command/auth_commands.go | 3 +-- etcdctl/command/member_commands.go | 24 ++++++++++-------------- etcdctl/command/role_commands.go | 20 ++++++++------------ etcdctl/command/user_commands.go | 26 ++++++++++---------------- etcdctl/command/util.go | 4 ++++ etcdctl/main.go | 1 + 6 files changed, 34 insertions(+), 44 deletions(-) diff --git a/etcdctl/command/auth_commands.go b/etcdctl/command/auth_commands.go index 2ee483fab..b03de7ff7 100644 --- a/etcdctl/command/auth_commands.go +++ b/etcdctl/command/auth_commands.go @@ -20,7 +20,6 @@ import ( "strings" "github.com/coreos/etcd/Godeps/_workspace/src/github.com/codegangsta/cli" - "github.com/coreos/etcd/Godeps/_workspace/src/golang.org/x/net/context" "github.com/coreos/etcd/client" ) @@ -67,7 +66,7 @@ func authEnableDisable(c *cli.Context, enable bool) { os.Exit(1) } s := mustNewAuthAPI(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) var err error if enable { err = s.Enable(ctx) diff --git a/etcdctl/command/member_commands.go b/etcdctl/command/member_commands.go index 2acd614c8..ef6decdc9 100644 --- a/etcdctl/command/member_commands.go +++ b/etcdctl/command/member_commands.go @@ -20,8 +20,6 @@ import ( "strings" "github.com/coreos/etcd/Godeps/_workspace/src/github.com/codegangsta/cli" - "github.com/coreos/etcd/Godeps/_workspace/src/golang.org/x/net/context" - "github.com/coreos/etcd/client" ) func NewMemberCommand() cli.Command { @@ -59,9 +57,9 @@ func actionMemberList(c *cli.Context) { os.Exit(1) } mAPI := mustNewMembersAPI(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) + defer cancel() members, err := mAPI.List(ctx) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -86,9 +84,10 @@ func actionMemberAdd(c *cli.Context) { mAPI := mustNewMembersAPI(c) url := args[1] - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) + defer cancel() + m, err := mAPI.Add(ctx, url) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -98,9 +97,7 @@ func actionMemberAdd(c *cli.Context) { newName := args[0] fmt.Printf("Added member named %s with ID %s to cluster\n", newName, newID) - ctx, cancel = context.WithTimeout(context.Background(), client.DefaultRequestTimeout) members, err := mAPI.List(ctx) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -132,10 +129,11 @@ func actionMemberRemove(c *cli.Context) { removalID := args[0] mAPI := mustNewMembersAPI(c) + + ctx, cancel := contextWithTotalTimeout(c) + defer cancel() // Get the list of members. - listctx, listCancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) - members, err := mAPI.List(listctx) - listCancel() + members, err := mAPI.List(ctx) if err != nil { fmt.Fprintln(os.Stderr, "Error while verifying ID against known members:", err.Error()) os.Exit(1) @@ -158,9 +156,7 @@ func actionMemberRemove(c *cli.Context) { } // Actually attempt to remove the member. - ctx, removeCancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) err = mAPI.Remove(ctx, removalID) - removeCancel() if err != nil { fmt.Fprintf(os.Stderr, "Received an error trying to remove member %s: %s", removalID, err.Error()) os.Exit(1) @@ -180,7 +176,7 @@ func actionMemberUpdate(c *cli.Context) { mid := args[0] urls := args[1] - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) err := mAPI.Update(ctx, mid, strings.Split(urls, ",")) cancel() if err != nil { diff --git a/etcdctl/command/role_commands.go b/etcdctl/command/role_commands.go index 2b3ed2401..c4f6f2bf6 100644 --- a/etcdctl/command/role_commands.go +++ b/etcdctl/command/role_commands.go @@ -21,7 +21,6 @@ import ( "strings" "github.com/coreos/etcd/Godeps/_workspace/src/github.com/codegangsta/cli" - "github.com/coreos/etcd/Godeps/_workspace/src/golang.org/x/net/context" "github.com/coreos/etcd/client" "github.com/coreos/etcd/pkg/pathutil" ) @@ -93,7 +92,7 @@ func actionRoleList(c *cli.Context) { os.Exit(1) } r := mustNewAuthRoleAPI(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) roles, err := r.ListRoles(ctx) cancel() if err != nil { @@ -108,16 +107,15 @@ func actionRoleList(c *cli.Context) { func actionRoleAdd(c *cli.Context) { api, role := mustRoleAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) + defer cancel() currentRole, err := api.GetRole(ctx, role) - cancel() if currentRole != nil { fmt.Fprintf(os.Stderr, "Role %s already exists\n", role) os.Exit(1) } - ctx, cancel = context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + err = api.AddRole(ctx, role) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -128,7 +126,7 @@ func actionRoleAdd(c *cli.Context) { func actionRoleRemove(c *cli.Context) { api, role := mustRoleAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) err := api.RemoveRole(ctx, role) cancel() if err != nil { @@ -182,21 +180,19 @@ func roleGrantRevoke(c *cli.Context, grant bool) { } api, role := mustRoleAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) + defer cancel() currentRole, err := api.GetRole(ctx, role) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) } - ctx, cancel = context.WithTimeout(context.Background(), client.DefaultRequestTimeout) var newRole *client.Role if grant { newRole, err = api.GrantRoleKV(ctx, role, []string{path}, permType) } else { newRole, err = api.RevokeRoleKV(ctx, role, []string{path}, permType) } - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -215,7 +211,7 @@ func roleGrantRevoke(c *cli.Context, grant bool) { func actionRoleGet(c *cli.Context) { api, rolename := mustRoleAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) role, err := api.GetRole(ctx, rolename) cancel() if err != nil { diff --git a/etcdctl/command/user_commands.go b/etcdctl/command/user_commands.go index 5596fe78b..ad3dfea09 100644 --- a/etcdctl/command/user_commands.go +++ b/etcdctl/command/user_commands.go @@ -23,7 +23,6 @@ import ( "github.com/coreos/etcd/Godeps/_workspace/src/github.com/bgentry/speakeasy" "github.com/coreos/etcd/Godeps/_workspace/src/github.com/codegangsta/cli" - "github.com/coreos/etcd/Godeps/_workspace/src/golang.org/x/net/context" "github.com/coreos/etcd/client" ) @@ -89,7 +88,7 @@ func actionUserList(c *cli.Context) { os.Exit(1) } u := mustNewAuthUserAPI(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) users, err := u.ListUsers(ctx) cancel() if err != nil { @@ -104,9 +103,9 @@ func actionUserList(c *cli.Context) { func actionUserAdd(c *cli.Context) { api, user := mustUserAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) + defer cancel() currentUser, err := api.GetUser(ctx, user) - cancel() if currentUser != nil { fmt.Fprintf(os.Stderr, "User %s already exists\n", user) os.Exit(1) @@ -116,9 +115,7 @@ func actionUserAdd(c *cli.Context) { fmt.Fprintln(os.Stderr, "Error reading password:", err) os.Exit(1) } - ctx, cancel = context.WithTimeout(context.Background(), client.DefaultRequestTimeout) err = api.AddUser(ctx, user, pass) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -129,7 +126,7 @@ func actionUserAdd(c *cli.Context) { func actionUserRemove(c *cli.Context) { api, user := mustUserAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) err := api.RemoveUser(ctx, user) cancel() if err != nil { @@ -142,9 +139,9 @@ func actionUserRemove(c *cli.Context) { func actionUserPasswd(c *cli.Context) { api, user := mustUserAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) + defer cancel() currentUser, err := api.GetUser(ctx, user) - cancel() if currentUser == nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -155,9 +152,7 @@ func actionUserPasswd(c *cli.Context) { os.Exit(1) } - ctx, cancel = context.WithTimeout(context.Background(), client.DefaultRequestTimeout) _, err = api.ChangePassword(ctx, user, pass) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -181,23 +176,22 @@ func userGrantRevoke(c *cli.Context, grant bool) { os.Exit(1) } + ctx, cancel := contextWithTotalTimeout(c) + defer cancel() + api, user := mustUserAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) currentUser, err := api.GetUser(ctx, user) - cancel() if currentUser == nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) } - ctx, cancel = context.WithTimeout(context.Background(), client.DefaultRequestTimeout) var newUser *client.User if grant { newUser, err = api.GrantUser(ctx, user, roles) } else { newUser, err = api.RevokeUser(ctx, user, roles) } - cancel() sort.Strings(newUser.Roles) sort.Strings(currentUser.Roles) if reflect.DeepEqual(newUser.Roles, currentUser.Roles) { @@ -217,7 +211,7 @@ func userGrantRevoke(c *cli.Context, grant bool) { func actionUserGet(c *cli.Context) { api, username := mustUserAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) + ctx, cancel := contextWithTotalTimeout(c) user, err := api.GetUser(ctx, username) cancel() if err != nil { diff --git a/etcdctl/command/util.go b/etcdctl/command/util.go index 0e9b51558..33d18fe01 100644 --- a/etcdctl/command/util.go +++ b/etcdctl/command/util.go @@ -263,3 +263,7 @@ func newClient(c *cli.Context) (client.Client, error) { return client.New(cfg) } + +func contextWithTotalTimeout(c *cli.Context) (context.Context, context.CancelFunc) { + return context.WithTimeout(context.Background(), c.GlobalDuration("total-timeout")) +} diff --git a/etcdctl/main.go b/etcdctl/main.go index da30af69c..c2fd73346 100644 --- a/etcdctl/main.go +++ b/etcdctl/main.go @@ -40,6 +40,7 @@ func main() { cli.StringFlag{Name: "ca-file", Value: "", Usage: "verify certificates of HTTPS-enabled servers using this CA bundle"}, cli.StringFlag{Name: "username, u", Value: "", Usage: "provide username[:password] and prompt if password is not supplied."}, cli.DurationFlag{Name: "timeout", Value: time.Second, Usage: "connection timeout per request"}, + cli.DurationFlag{Name: "total-timeout", Value: 5 * time.Second, Usage: "timeout for the command execution (except watch)"}, } app.Commands = []cli.Command{ command.NewBackupCommand(),