Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

etcdctl: use user specified timeout value for entire command execution #3530

Merged
merged 1 commit into from
Sep 28, 2015
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
mitake committed Sep 28, 2015

Verified

This commit was signed with the committer’s verified signature.
germa89 German
commit 8ebc9331111395fedbeedb3a346b302b95b147cc
3 changes: 1 addition & 2 deletions etcdctl/command/auth_commands.go
Original file line number Diff line number Diff line change
@@ -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)
24 changes: 10 additions & 14 deletions etcdctl/command/member_commands.go
Original file line number Diff line number Diff line change
@@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would defer cancel() here. It is reasonable because the ctx is for the whole function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defer cancel() seems nice, I'll fix it.

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 {
20 changes: 8 additions & 12 deletions etcdctl/command/role_commands.go
Original file line number Diff line number Diff line change
@@ -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 {
26 changes: 10 additions & 16 deletions etcdctl/command/user_commands.go
Original file line number Diff line number Diff line change
@@ -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 {
4 changes: 4 additions & 0 deletions etcdctl/command/util.go
Original file line number Diff line number Diff line change
@@ -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"))
}
1 change: 1 addition & 0 deletions etcdctl/main.go
Original file line number Diff line number Diff line change
@@ -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(),