From e86b95f03e2fbad3ba744a4f9be8ed5980f5f308 Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Tue, 15 Sep 2015 17:14:45 +0900 Subject: [PATCH] etcdctl: use user specified timeout value for entire RPC 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 | 28 +++++---- etcdctl/command/member_commands.go | 55 ++++++++--------- etcdctl/command/role_commands.go | 72 +++++++++++----------- etcdctl/command/user_commands.go | 98 ++++++++++++++---------------- etcdctl/command/util.go | 6 ++ etcdctl/main.go | 1 + 6 files changed, 129 insertions(+), 131 deletions(-) diff --git a/etcdctl/command/auth_commands.go b/etcdctl/command/auth_commands.go index 2ee483fab7e3..b989b8b55116 100644 --- a/etcdctl/command/auth_commands.go +++ b/etcdctl/command/auth_commands.go @@ -30,25 +30,29 @@ func NewAuthCommands() cli.Command { Usage: "overall auth controls", Subcommands: []cli.Command{ { - Name: "enable", - Usage: "enable auth access controls", - Action: actionAuthEnable, + Name: "enable", + Usage: "enable auth access controls", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionAuthEnable) + }, }, { - Name: "disable", - Usage: "disable auth access controls", - Action: actionAuthDisable, + Name: "disable", + Usage: "disable auth access controls", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionAuthDisable) + }, }, }, } } -func actionAuthEnable(c *cli.Context) { - authEnableDisable(c, true) +func actionAuthEnable(c *cli.Context, ctx context.Context) { + authEnableDisable(c, true, ctx) } -func actionAuthDisable(c *cli.Context) { - authEnableDisable(c, false) +func actionAuthDisable(c *cli.Context, ctx context.Context) { + authEnableDisable(c, false, ctx) } func mustNewAuthAPI(c *cli.Context) client.AuthAPI { @@ -61,20 +65,18 @@ func mustNewAuthAPI(c *cli.Context) client.AuthAPI { return client.NewAuthAPI(hc) } -func authEnableDisable(c *cli.Context, enable bool) { +func authEnableDisable(c *cli.Context, enable bool, ctx context.Context) { if len(c.Args()) != 0 { fmt.Fprintln(os.Stderr, "No arguments accepted") os.Exit(1) } s := mustNewAuthAPI(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) var err error if enable { err = s.Enable(ctx) } else { err = s.Disable(ctx) } - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) diff --git a/etcdctl/command/member_commands.go b/etcdctl/command/member_commands.go index 2acd614c8d27..9ba7f8c4e44c 100644 --- a/etcdctl/command/member_commands.go +++ b/etcdctl/command/member_commands.go @@ -21,7 +21,6 @@ import ( "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 { @@ -30,38 +29,44 @@ func NewMemberCommand() cli.Command { Usage: "member add, remove and list subcommands", Subcommands: []cli.Command{ { - Name: "list", - Usage: "enumerate existing cluster members", - Action: actionMemberList, + Name: "list", + Usage: "enumerate existing cluster members", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionMemberList) + }, }, { - Name: "add", - Usage: "add a new member to the etcd cluster", - Action: actionMemberAdd, + Name: "add", + Usage: "add a new member to the etcd cluster", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionMemberAdd) + }, }, { - Name: "remove", - Usage: "remove an existing member from the etcd cluster", - Action: actionMemberRemove, + Name: "remove", + Usage: "remove an existing member from the etcd cluster", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionMemberRemove) + }, }, { - Name: "update", - Usage: "update an existing member in the etcd cluster", - Action: actionMemberUpdate, + Name: "update", + Usage: "update an existing member in the etcd cluster", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionMemberUpdate) + }, }, }, } } -func actionMemberList(c *cli.Context) { +func actionMemberList(c *cli.Context, ctx context.Context) { if len(c.Args()) != 0 { fmt.Fprintln(os.Stderr, "No arguments accepted") os.Exit(1) } mAPI := mustNewMembersAPI(c) - 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) @@ -76,7 +81,7 @@ func actionMemberList(c *cli.Context) { } } -func actionMemberAdd(c *cli.Context) { +func actionMemberAdd(c *cli.Context, ctx context.Context) { args := c.Args() if len(args) != 2 { fmt.Fprintln(os.Stderr, "Provide a name and a single member peerURL") @@ -86,9 +91,7 @@ func actionMemberAdd(c *cli.Context) { mAPI := mustNewMembersAPI(c) url := args[1] - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) m, err := mAPI.Add(ctx, url) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -98,9 +101,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) @@ -123,7 +124,7 @@ func actionMemberAdd(c *cli.Context) { fmt.Printf("ETCD_INITIAL_CLUSTER_STATE=\"existing\"\n") } -func actionMemberRemove(c *cli.Context) { +func actionMemberRemove(c *cli.Context, ctx context.Context) { args := c.Args() if len(args) != 1 { fmt.Fprintln(os.Stderr, "Provide a single member ID") @@ -133,9 +134,7 @@ func actionMemberRemove(c *cli.Context) { mAPI := mustNewMembersAPI(c) // 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 +157,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) @@ -169,7 +166,7 @@ func actionMemberRemove(c *cli.Context) { fmt.Printf("Removed member %s from cluster\n", removalID) } -func actionMemberUpdate(c *cli.Context) { +func actionMemberUpdate(c *cli.Context, ctx context.Context) { args := c.Args() if len(args) != 2 { fmt.Fprintln(os.Stderr, "Provide an ID and a list of comma separated peerURL (0xabcd http://example.com,http://example1.com)") @@ -180,9 +177,7 @@ func actionMemberUpdate(c *cli.Context) { mid := args[0] urls := args[1] - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) err := mAPI.Update(ctx, mid, strings.Split(urls, ",")) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) diff --git a/etcdctl/command/role_commands.go b/etcdctl/command/role_commands.go index 2b3ed2401117..32cf10179f96 100644 --- a/etcdctl/command/role_commands.go +++ b/etcdctl/command/role_commands.go @@ -32,24 +32,32 @@ func NewRoleCommands() cli.Command { Usage: "role add, grant and revoke subcommands", Subcommands: []cli.Command{ { - Name: "add", - Usage: "add a new role for the etcd cluster", - Action: actionRoleAdd, + Name: "add", + Usage: "add a new role for the etcd cluster", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionRoleAdd) + }, }, { - Name: "get", - Usage: "get details for a role", - Action: actionRoleGet, + Name: "get", + Usage: "get details for a role", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionRoleGet) + }, }, { - Name: "list", - Usage: "list all roles", - Action: actionRoleList, + Name: "list", + Usage: "list all roles", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionRoleList) + }, }, { - Name: "remove", - Usage: "remove a role from the etcd cluster", - Action: actionRoleRemove, + Name: "remove", + Usage: "remove a role from the etcd cluster", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionRoleRemove) + }, }, { Name: "grant", @@ -60,7 +68,9 @@ func NewRoleCommands() cli.Command { cli.BoolFlag{Name: "write", Usage: "Grant write-only access"}, cli.BoolFlag{Name: "readwrite", Usage: "Grant read-write access"}, }, - Action: actionRoleGrant, + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionRoleGrant) + }, }, { Name: "revoke", @@ -71,7 +81,9 @@ func NewRoleCommands() cli.Command { cli.BoolFlag{Name: "write", Usage: "Revoke write access"}, cli.BoolFlag{Name: "readwrite", Usage: "Revoke read-write access"}, }, - Action: actionRoleRevoke, + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionRoleRevoke) + }, }, }, } @@ -87,15 +99,13 @@ func mustNewAuthRoleAPI(c *cli.Context) client.AuthRoleAPI { return client.NewAuthRoleAPI(hc) } -func actionRoleList(c *cli.Context) { +func actionRoleList(c *cli.Context, ctx context.Context) { if len(c.Args()) != 0 { fmt.Fprintln(os.Stderr, "No arguments accepted") os.Exit(1) } r := mustNewAuthRoleAPI(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) roles, err := r.ListRoles(ctx) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -106,18 +116,14 @@ func actionRoleList(c *cli.Context) { } } -func actionRoleAdd(c *cli.Context) { +func actionRoleAdd(c *cli.Context, ctx context.Context) { api, role := mustRoleAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) 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) @@ -126,11 +132,9 @@ func actionRoleAdd(c *cli.Context) { fmt.Printf("Role %s created\n", role) } -func actionRoleRemove(c *cli.Context) { +func actionRoleRemove(c *cli.Context, ctx context.Context) { api, role := mustRoleAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) err := api.RemoveRole(ctx, role) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -139,15 +143,15 @@ func actionRoleRemove(c *cli.Context) { fmt.Printf("Role %s removed\n", role) } -func actionRoleGrant(c *cli.Context) { - roleGrantRevoke(c, true) +func actionRoleGrant(c *cli.Context, ctx context.Context) { + roleGrantRevoke(c, true, ctx) } -func actionRoleRevoke(c *cli.Context) { - roleGrantRevoke(c, false) +func actionRoleRevoke(c *cli.Context, ctx context.Context) { + roleGrantRevoke(c, false, ctx) } -func roleGrantRevoke(c *cli.Context, grant bool) { +func roleGrantRevoke(c *cli.Context, grant bool, ctx context.Context) { path := c.String("path") if path == "" { fmt.Fprintln(os.Stderr, "No path specified; please use `-path`") @@ -182,21 +186,17 @@ func roleGrantRevoke(c *cli.Context, grant bool) { } api, role := mustRoleAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) 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) @@ -212,12 +212,10 @@ func roleGrantRevoke(c *cli.Context, grant bool) { fmt.Printf("Role %s updated\n", role) } -func actionRoleGet(c *cli.Context) { +func actionRoleGet(c *cli.Context, ctx context.Context) { api, rolename := mustRoleAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) role, err := api.GetRole(ctx, rolename) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) diff --git a/etcdctl/command/user_commands.go b/etcdctl/command/user_commands.go index 5596fe78b6b7..a402bca0a818 100644 --- a/etcdctl/command/user_commands.go +++ b/etcdctl/command/user_commands.go @@ -33,41 +33,55 @@ func NewUserCommands() cli.Command { Usage: "user add, grant and revoke subcommands", Subcommands: []cli.Command{ { - Name: "add", - Usage: "add a new user for the etcd cluster", - Action: actionUserAdd, + Name: "add", + Usage: "add a new user for the etcd cluster", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionUserAdd) + }, }, { - Name: "get", - Usage: "get details for a user", - Action: actionUserGet, + Name: "get", + Usage: "get details for a user", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionUserGet) + }, }, { - Name: "list", - Usage: "list all current users", - Action: actionUserList, + Name: "list", + Usage: "list all current users", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionUserList) + }, }, { - Name: "remove", - Usage: "remove a user for the etcd cluster", - Action: actionUserRemove, + Name: "remove", + Usage: "remove a user for the etcd cluster", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionUserRemove) + }, }, { - Name: "grant", - Usage: "grant roles to an etcd user", - Flags: []cli.Flag{cli.StringSliceFlag{Name: "roles", Value: new(cli.StringSlice), Usage: "List of roles to grant or revoke"}}, - Action: actionUserGrant, + Name: "grant", + Usage: "grant roles to an etcd user", + Flags: []cli.Flag{cli.StringSliceFlag{Name: "roles", Value: new(cli.StringSlice), Usage: "List of roles to grant or revoke"}}, + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionUserGrant) + }, }, { - Name: "revoke", - Usage: "revoke roles for an etcd user", - Flags: []cli.Flag{cli.StringSliceFlag{Name: "roles", Value: new(cli.StringSlice), Usage: "List of roles to grant or revoke"}}, - Action: actionUserRevoke, + Name: "revoke", + Usage: "revoke roles for an etcd user", + Flags: []cli.Flag{cli.StringSliceFlag{Name: "roles", Value: new(cli.StringSlice), Usage: "List of roles to grant or revoke"}}, + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionUserRevoke) + }, }, { - Name: "passwd", - Usage: "change password for a user", - Action: actionUserPasswd, + Name: "passwd", + Usage: "change password for a user", + Action: func(c *cli.Context) { + actionWithTotalTimeout(c, actionUserPasswd) + }, }, }, } @@ -83,15 +97,13 @@ func mustNewAuthUserAPI(c *cli.Context) client.AuthUserAPI { return client.NewAuthUserAPI(hc) } -func actionUserList(c *cli.Context) { +func actionUserList(c *cli.Context, ctx context.Context) { if len(c.Args()) != 0 { fmt.Fprintln(os.Stderr, "No arguments accepted") os.Exit(1) } u := mustNewAuthUserAPI(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) users, err := u.ListUsers(ctx) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -102,11 +114,9 @@ func actionUserList(c *cli.Context) { } } -func actionUserAdd(c *cli.Context) { +func actionUserAdd(c *cli.Context, ctx context.Context) { api, user := mustUserAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) 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 +126,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) @@ -127,11 +135,9 @@ func actionUserAdd(c *cli.Context) { fmt.Printf("User %s created\n", user) } -func actionUserRemove(c *cli.Context) { +func actionUserRemove(c *cli.Context, ctx context.Context) { api, user := mustUserAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) err := api.RemoveUser(ctx, user) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) @@ -140,11 +146,9 @@ func actionUserRemove(c *cli.Context) { fmt.Printf("User %s removed\n", user) } -func actionUserPasswd(c *cli.Context) { +func actionUserPasswd(c *cli.Context, ctx context.Context) { 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) @@ -155,9 +159,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) @@ -166,15 +168,15 @@ func actionUserPasswd(c *cli.Context) { fmt.Printf("Password updated\n") } -func actionUserGrant(c *cli.Context) { - userGrantRevoke(c, true) +func actionUserGrant(c *cli.Context, ctx context.Context) { + userGrantRevoke(c, true, ctx) } -func actionUserRevoke(c *cli.Context) { - userGrantRevoke(c, false) +func actionUserRevoke(c *cli.Context, ctx context.Context) { + userGrantRevoke(c, false, ctx) } -func userGrantRevoke(c *cli.Context, grant bool) { +func userGrantRevoke(c *cli.Context, grant bool, ctx context.Context) { roles := c.StringSlice("roles") if len(roles) == 0 { fmt.Fprintln(os.Stderr, "No roles specified; please use `-roles`") @@ -182,22 +184,18 @@ func userGrantRevoke(c *cli.Context, grant bool) { } 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) { @@ -215,11 +213,9 @@ func userGrantRevoke(c *cli.Context, grant bool) { fmt.Printf("User %s updated\n", user) } -func actionUserGet(c *cli.Context) { +func actionUserGet(c *cli.Context, ctx context.Context) { api, username := mustUserAPIAndName(c) - ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout) user, err := api.GetUser(ctx, username) - cancel() if err != nil { fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) diff --git a/etcdctl/command/util.go b/etcdctl/command/util.go index 0e9b51558386..cd9d9ee2caa8 100644 --- a/etcdctl/command/util.go +++ b/etcdctl/command/util.go @@ -263,3 +263,9 @@ func newClient(c *cli.Context) (client.Client, error) { return client.New(cfg) } + +func actionWithTotalTimeout(c *cli.Context, action func(c *cli.Context, ctx context.Context)) { + ctx, cancel := context.WithTimeout(context.Background(), c.GlobalDuration("total-timeout")) + action(c, ctx) + cancel() +} diff --git a/etcdctl/main.go b/etcdctl/main.go index da30af69c21b..c2fd7334623e 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(),