Skip to content

Commit

Permalink
Explicitly set timeout in each CLI command (#443) (#465)
Browse files Browse the repository at this point in the history
* Explicitly set timeout in each CLI command

Right now, the context deadline is initially set at root level when the command is created.
This causes problems for interactive commands as the deadline may be exceeded while the user inputs the data.
To prevent this, we remove the global timeout and specifically set it in
each command right before the RPC calls.

* update docs

Co-authored-by: Benjamin Schimke <benjamin.schimke@canonical.com>
  • Loading branch information
neoaggelos and bschimke95 authored Jun 3, 2024
1 parent 072b6aa commit 17a6023
Show file tree
Hide file tree
Showing 21 changed files with 179 additions and 61 deletions.
1 change: 1 addition & 0 deletions docs/src/_parts/commands/k8s_bootstrap.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ k8s bootstrap [flags]
--interactive interactively configure the most important cluster options
--name string node name, defaults to hostname
--output-format string set the output format to one of plain, json or yaml (default "plain")
--timeout duration the max time to wait for the command to execute (default 1m30s)
```

### SEE ALSO
Expand Down
1 change: 1 addition & 0 deletions docs/src/_parts/commands/k8s_disable.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ k8s disable <feature> ... [flags]
```
-h, --help help for disable
--output-format string set the output format to one of plain, json or yaml (default "plain")
--timeout duration the max time to wait for the command to execute (default 1m30s)
```

### SEE ALSO
Expand Down
1 change: 1 addition & 0 deletions docs/src/_parts/commands/k8s_enable.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ k8s enable <feature> ... [flags]
```
-h, --help help for enable
--output-format string set the output format to one of plain, json or yaml (default "plain")
--timeout duration the max time to wait for the command to execute (default 1m30s)
```

### SEE ALSO
Expand Down
5 changes: 3 additions & 2 deletions docs/src/_parts/commands/k8s_get-join-token.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ k8s get-join-token <node-name> [flags]
### Options

```
-h, --help help for get-join-token
--worker generate a join token for a worker node
-h, --help help for get-join-token
--timeout duration the max time to wait for the command to execute (default 1m30s)
--worker generate a join token for a worker node
```

### SEE ALSO
Expand Down
1 change: 1 addition & 0 deletions docs/src/_parts/commands/k8s_get.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ k8s get <feature.key> [flags]
```
-h, --help help for get
--output-format string set the output format to one of plain, json or yaml (default "plain")
--timeout duration the max time to wait for the command to execute (default 1m30s)
```

### SEE ALSO
Expand Down
1 change: 1 addition & 0 deletions docs/src/_parts/commands/k8s_join-cluster.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ k8s join-cluster <join-token> [flags]
-h, --help help for join-cluster
--name string node name, defaults to hostname
--output-format string set the output format to one of plain, json or yaml (default "plain")
--timeout duration the max time to wait for the command to execute (default 1m30s)
```

### SEE ALSO
Expand Down
1 change: 1 addition & 0 deletions docs/src/_parts/commands/k8s_remove-node.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ k8s remove-node <node-name> [flags]
--force forcibly remove the cluster member
-h, --help help for remove-node
--output-format string set the output format to one of plain, json or yaml (default "plain")
--timeout duration the max time to wait for the command to execute (default 1m30s)
```

### SEE ALSO
Expand Down
1 change: 1 addition & 0 deletions docs/src/_parts/commands/k8s_set.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ k8s set <feature.key=value> ... [flags]
```
-h, --help help for set
--output-format string set the output format to one of plain, json or yaml (default "plain")
--timeout duration the max time to wait for the command to execute (default 1m30s)
```

### SEE ALSO
Expand Down
1 change: 1 addition & 0 deletions docs/src/_parts/commands/k8s_status.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ k8s status [flags]
```
-h, --help help for status
--output-format string set the output format to one of plain, json or yaml (default "plain")
--timeout duration the max time to wait for the command to execute (default 1m30s)
--wait-ready wait until at least one cluster node is ready
```

Expand Down
22 changes: 2 additions & 20 deletions src/k8s/cmd/k8s/k8s.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package k8s

import (
"context"
"time"

cmdutil "github.com/canonical/k8s/cmd/util"
Expand All @@ -14,6 +13,8 @@ var (
outputFormatter cmdutil.Formatter
)

const minTimeout = 3 * time.Second

func addCommands(root *cobra.Command, group *cobra.Group, commands ...*cobra.Command) {
if group != nil {
root.AddGroup(group)
Expand All @@ -31,28 +32,11 @@ func NewRootCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
logDebug bool
logVerbose bool
stateDir string
timeout time.Duration
}
)
cmd := &cobra.Command{
Use: "k8s",
Short: "Canonical Kubernetes CLI",
PersistentPreRun: func(cmd *cobra.Command, args []string) {
// initialize context
ctx := cmd.Context()

// configure command context timeout
const minTimeout = 3 * time.Second
if opts.timeout < minTimeout {
cmd.PrintErrf("Timeout %v is less than minimum of %v. Using the minimum %v instead.\n", opts.timeout, minTimeout, minTimeout)
opts.timeout = minTimeout
}

ctx, cancel := context.WithTimeout(ctx, opts.timeout)
cobra.OnFinalize(cancel)

cmd.SetContext(ctx)
},
}

// set input/output streams
Expand All @@ -63,14 +47,12 @@ func NewRootCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
cmd.PersistentFlags().StringVar(&opts.stateDir, "state-dir", "", "directory with the dqlite datastore")
cmd.PersistentFlags().BoolVarP(&opts.logDebug, "debug", "d", false, "show all debug messages")
cmd.PersistentFlags().BoolVarP(&opts.logVerbose, "verbose", "v", true, "show all information messages")
cmd.PersistentFlags().DurationVar(&opts.timeout, "timeout", 90*time.Second, "the max time to wait for the command to execute")

// By default, the state dir is set to a fixed directory in the snap.
// This shouldn't be overwritten by the user.
cmd.PersistentFlags().MarkHidden("state-dir")
cmd.PersistentFlags().MarkHidden("debug")
cmd.PersistentFlags().MarkHidden("verbose")
cmd.PersistentFlags().MarkHidden("timeout")

// General
addCommands(
Expand Down
14 changes: 13 additions & 1 deletion src/k8s/cmd/k8s/k8s_bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package k8s
import (
"bufio"
"bytes"
"context"
"fmt"
"io"
"os"
"slices"
"strings"
"time"
"unicode"

apiv1 "github.com/canonical/k8s/api/v1"
Expand Down Expand Up @@ -38,6 +40,7 @@ func newBootstrapCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
name string
address string
outputFormat string
timeout time.Duration
}
cmd := &cobra.Command{
Use: "bootstrap",
Expand All @@ -51,6 +54,11 @@ func newBootstrapCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
return
}

if opts.timeout < minTimeout {
cmd.PrintErrf("Timeout %v is less than minimum of %v. Using the minimum %v instead.\n", opts.timeout, minTimeout, minTimeout)
opts.timeout = minTimeout
}

// Use hostname as default node name
if opts.name == "" {
hostname, err := os.Hostname()
Expand Down Expand Up @@ -122,7 +130,10 @@ func newBootstrapCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
Config: bootstrapConfig,
}

node, err := client.Bootstrap(cmd.Context(), request)
ctx, cancel := context.WithTimeout(cmd.Context(), opts.timeout)
cobra.OnFinalize(cancel)

node, err := client.Bootstrap(ctx, request)
if err != nil {
cmd.PrintErrf("Error: Failed to bootstrap the cluster.\n\nThe error was: %v\n", err)
env.Exit(1)
Expand All @@ -138,6 +149,7 @@ func newBootstrapCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
cmd.Flags().StringVar(&opts.name, "name", "", "node name, defaults to hostname")
cmd.Flags().StringVar(&opts.address, "address", "", "microcluster address, defaults to the node IP address")
cmd.Flags().StringVar(&opts.outputFormat, "output-format", "plain", "set the output format to one of plain, json or yaml")
cmd.Flags().DurationVar(&opts.timeout, "timeout", 90*time.Second, "the max time to wait for the command to execute")

return cmd
}
Expand Down
17 changes: 15 additions & 2 deletions src/k8s/cmd/k8s/k8s_config.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,30 @@
package k8s

import (
"context"
"time"

apiv1 "github.com/canonical/k8s/api/v1"
cmdutil "github.com/canonical/k8s/cmd/util"
"github.com/spf13/cobra"
)

func newKubeConfigCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
var opts struct {
server string
server string
timeout time.Duration
}
cmd := &cobra.Command{
Use: "config",
Hidden: true,
Short: "Generate an admin kubeconfig that can be used to access the Kubernetes cluster",
PreRun: chainPreRunHooks(hookRequireRoot(env)),
Run: func(cmd *cobra.Command, args []string) {
if opts.timeout < minTimeout {
cmd.PrintErrf("Timeout %v is less than minimum of %v. Using the minimum %v instead.\n", opts.timeout, minTimeout, minTimeout)
opts.timeout = minTimeout
}

client, err := env.Client(cmd.Context())
if err != nil {
cmd.PrintErrf("Error: Failed to create a k8sd client. Make sure that the k8sd service is running.\n\nThe error was: %v\n", err)
Expand All @@ -29,7 +38,10 @@ func newKubeConfigCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
return
}

config, err := client.KubeConfig(cmd.Context(), apiv1.GetKubeConfigRequest{Server: opts.server})
ctx, cancel := context.WithTimeout(cmd.Context(), opts.timeout)
cobra.OnFinalize(cancel)

config, err := client.KubeConfig(ctx, apiv1.GetKubeConfigRequest{Server: opts.server})
if err != nil {
cmd.PrintErrf("Error: Failed to generate an admin kubeconfig for %q.\n\nThe error was: %v\n", opts.server, err)
env.Exit(1)
Expand All @@ -40,5 +52,6 @@ func newKubeConfigCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
},
}
cmd.Flags().StringVar(&opts.server, "server", "", "custom cluster server address")
cmd.Flags().DurationVar(&opts.timeout, "timeout", 90*time.Second, "the max time to wait for the command to execute")
return cmd
}
16 changes: 14 additions & 2 deletions src/k8s/cmd/k8s/k8s_disable.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package k8s

import (
"context"
"fmt"
"github.com/canonical/k8s/pkg/utils"
"strings"
"time"

"github.com/canonical/k8s/pkg/utils"

api "github.com/canonical/k8s/api/v1"
cmdutil "github.com/canonical/k8s/cmd/util"
Expand All @@ -21,6 +24,7 @@ func (d DisableResult) String() string {
func newDisableCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
var opts struct {
outputFormat string
timeout time.Duration
}
cmd := &cobra.Command{
Use: "disable <feature> ...",
Expand All @@ -31,6 +35,11 @@ func newDisableCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
Run: func(cmd *cobra.Command, args []string) {
config := api.UserFacingClusterConfig{}

if opts.timeout < minTimeout {
cmd.PrintErrf("Timeout %v is less than minimum of %v. Using the minimum %v instead.\n", opts.timeout, minTimeout, minTimeout)
opts.timeout = minTimeout
}

for _, feature := range args {
switch feature {
case "network":
Expand Down Expand Up @@ -79,7 +88,9 @@ func newDisableCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
}

cmd.PrintErrf("Disabling %s from the cluster. This may take a few seconds, please wait.\n", strings.Join(args, ", "))
if err := client.UpdateClusterConfig(cmd.Context(), request); err != nil {
ctx, cancel := context.WithTimeout(cmd.Context(), opts.timeout)
cobra.OnFinalize(cancel)
if err := client.UpdateClusterConfig(ctx, request); err != nil {
cmd.PrintErrf("Error: Failed to disable %s from the cluster.\n\nThe error was: %v\n", strings.Join(args, ", "), err)
env.Exit(1)
return
Expand All @@ -90,6 +101,7 @@ func newDisableCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
}

cmd.Flags().StringVar(&opts.outputFormat, "output-format", "plain", "set the output format to one of plain, json or yaml")
cmd.Flags().DurationVar(&opts.timeout, "timeout", 90*time.Second, "the max time to wait for the command to execute")

return cmd
}
16 changes: 14 additions & 2 deletions src/k8s/cmd/k8s/k8s_enable.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package k8s

import (
"context"
"fmt"
"github.com/canonical/k8s/pkg/utils"
"strings"
"time"

"github.com/canonical/k8s/pkg/utils"

api "github.com/canonical/k8s/api/v1"
cmdutil "github.com/canonical/k8s/cmd/util"
Expand All @@ -21,6 +24,7 @@ func (e EnableResult) String() string {
func newEnableCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
var opts struct {
outputFormat string
timeout time.Duration
}
cmd := &cobra.Command{
Use: "enable <feature> ...",
Expand All @@ -31,6 +35,11 @@ func newEnableCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
Run: func(cmd *cobra.Command, args []string) {
config := api.UserFacingClusterConfig{}

if opts.timeout < minTimeout {
cmd.PrintErrf("Timeout %v is less than minimum of %v. Using the minimum %v instead.\n", opts.timeout, minTimeout, minTimeout)
opts.timeout = minTimeout
}

for _, feature := range args {
switch feature {
case "network":
Expand Down Expand Up @@ -79,7 +88,9 @@ func newEnableCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
}

cmd.PrintErrf("Enabling %s on the cluster. This may take a few seconds, please wait.\n", strings.Join(args, ", "))
if err := client.UpdateClusterConfig(cmd.Context(), request); err != nil {
ctx, cancel := context.WithTimeout(cmd.Context(), opts.timeout)
cobra.OnFinalize(cancel)
if err := client.UpdateClusterConfig(ctx, request); err != nil {
cmd.PrintErrf("Error: Failed to enable %s on the cluster.\n\nThe error was: %v\n", strings.Join(args, ", "), err)
env.Exit(1)
return
Expand All @@ -90,6 +101,7 @@ func newEnableCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
}

cmd.Flags().StringVar(&opts.outputFormat, "output-format", "plain", "set the output format to one of plain, json or yaml")
cmd.Flags().DurationVar(&opts.timeout, "timeout", 90*time.Second, "the max time to wait for the command to execute")

return cmd
}
9 changes: 9 additions & 0 deletions src/k8s/cmd/k8s/k8s_generate_auth_token.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package k8s

import (
"time"

apiv1 "github.com/canonical/k8s/api/v1"
cmdutil "github.com/canonical/k8s/cmd/util"
"github.com/spf13/cobra"
Expand All @@ -10,13 +12,19 @@ func newGenerateAuthTokenCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
var opts struct {
username string
groups []string
timeout time.Duration
}

cmd := &cobra.Command{
Use: "generate-auth-token --username <user> [--groups <group1>,<group2>]",
Hidden: true,
PreRun: chainPreRunHooks(hookRequireRoot(env)),
Run: func(cmd *cobra.Command, args []string) {
if opts.timeout < minTimeout {
cmd.PrintErrf("Timeout %v is less than minimum of %v. Using the minimum %v instead.\n", opts.timeout, minTimeout, minTimeout)
opts.timeout = minTimeout
}

client, err := env.Client(cmd.Context())
if err != nil {
cmd.PrintErrf("Error: Failed to create a k8sd client. Make sure that the k8sd service is running.\n\nThe error was: %v\n", err)
Expand All @@ -35,5 +43,6 @@ func newGenerateAuthTokenCmd(env cmdutil.ExecutionEnvironment) *cobra.Command {
}
cmd.Flags().StringVar(&opts.username, "username", "", "Username")
cmd.Flags().StringSliceVar(&opts.groups, "groups", nil, "Groups")
cmd.Flags().DurationVar(&opts.timeout, "timeout", 90*time.Second, "the max time to wait for the command to execute")
return cmd
}
Loading

0 comments on commit 17a6023

Please sign in to comment.