From e83ec61f50af55073e292d20f916482d22574a89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Trigo=20Soares?= <joao@jtsoar.es> Date: Thu, 16 May 2019 16:46:24 +0100 Subject: [PATCH 1/5] added --timeout command line parameter and FLUX_TIMEOUT env variable --- .gitignore | 1 + Gopkg.lock | 5 ++--- cmd/fluxctl/await.go | 14 +++++++------- cmd/fluxctl/policy_cmd.go | 2 +- cmd/fluxctl/release_cmd.go | 4 ++-- cmd/fluxctl/root_cmd.go | 15 +++++++++------ cmd/fluxctl/sync_cmd.go | 4 ++-- 7 files changed, 24 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 7e72e279e3..5c46c27462 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,4 @@ docker/fluxy-dumbconf.priv test/profiles test/bin/kubectl test/bin/helm +.idea diff --git a/Gopkg.lock b/Gopkg.lock index b7c2e4bd64..cd42991933 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1108,7 +1108,6 @@ name = "k8s.io/client-go" packages = [ "discovery", - "discovery/cached", "discovery/cached/memory", "discovery/fake", "dynamic", @@ -1263,7 +1262,7 @@ revision = "e17681d19d3ac4837a019ece36c2a0ec31ffe985" [[projects]] - digest = "1:93dacf333c11ff29b3a336c1bbff8f2f1dc689a47a49f9e58a183202eaeae184" + digest = "1:18c5ed628f64365e67293ac1e8b536803e83c50b7b2b5cb319ffdde680d1efd0" name = "k8s.io/helm" packages = [ "pkg/chartutil", @@ -1393,7 +1392,7 @@ "k8s.io/apimachinery/pkg/util/wait", "k8s.io/apimachinery/pkg/watch", "k8s.io/client-go/discovery", - "k8s.io/client-go/discovery/cached", + "k8s.io/client-go/discovery/cached/memory", "k8s.io/client-go/discovery/fake", "k8s.io/client-go/dynamic", "k8s.io/client-go/dynamic/fake", diff --git a/cmd/fluxctl/await.go b/cmd/fluxctl/await.go index 681e34691d..bb1cc4bdd7 100644 --- a/cmd/fluxctl/await.go +++ b/cmd/fluxctl/await.go @@ -16,8 +16,8 @@ var ErrTimeout = errors.New("timeout") // await polls for a job to complete, then for the resulting commit to // be applied -func await(ctx context.Context, stdout, stderr io.Writer, client api.Server, jobID job.ID, apply bool, verbosity int) error { - result, err := awaitJob(ctx, client, jobID) +func await(ctx context.Context, stdout, stderr io.Writer, client api.Server, jobID job.ID, apply bool, verbosity int, timeout int) error { + result, err := awaitJob(ctx, client, jobID, timeout) if err != nil { if err == ErrTimeout { fmt.Fprintln(stderr, ` @@ -41,7 +41,7 @@ is safe to retry operations.`) } if apply && result.Revision != "" { - if err := awaitSync(ctx, client, result.Revision); err != nil { + if err := awaitSync(ctx, client, result.Revision, timeout); err != nil { if err == ErrTimeout { fmt.Fprintln(stderr, ` The operation succeeded, but we timed out waiting for the commit to be @@ -61,9 +61,9 @@ to run a sync interactively.`) } // await polls for a job to have been completed, with exponential backoff. -func awaitJob(ctx context.Context, client api.Server, jobID job.ID) (job.Result, error) { +func awaitJob(ctx context.Context, client api.Server, jobID job.ID, timeout int) (job.Result, error) { var result job.Result - err := backoff(100*time.Millisecond, 2, 50, 1*time.Minute, func() (bool, error) { + err := backoff(100*time.Millisecond, 2, 50, time.Duration(timeout)*time.Second, func() (bool, error) { j, err := client.JobStatus(ctx, jobID) if err != nil { return false, err @@ -86,8 +86,8 @@ func awaitJob(ctx context.Context, client api.Server, jobID job.ID) (job.Result, } // await polls for a commit to have been applied, with exponential backoff. -func awaitSync(ctx context.Context, client api.Server, revision string) error { - return backoff(1*time.Second, 2, 10, 1*time.Minute, func() (bool, error) { +func awaitSync(ctx context.Context, client api.Server, revision string, timeout int) error { + return backoff(1*time.Second, 2, 10, time.Duration(timeout)*time.Second, func() (bool, error) { refs, err := client.SyncStatus(ctx, revision) return err == nil && len(refs) == 0, err }) diff --git a/cmd/fluxctl/policy_cmd.go b/cmd/fluxctl/policy_cmd.go index ba871186b5..048b37fe40 100644 --- a/cmd/fluxctl/policy_cmd.go +++ b/cmd/fluxctl/policy_cmd.go @@ -119,7 +119,7 @@ func (opts *workloadPolicyOpts) RunE(cmd *cobra.Command, args []string) error { if err != nil { return err } - return await(ctx, cmd.OutOrStdout(), cmd.OutOrStderr(), opts.API, jobID, false, opts.verbosity) + return await(ctx, cmd.OutOrStdout(), cmd.OutOrStderr(), opts.API, jobID, false, opts.verbosity, opts.Timeout) } func calculatePolicyChanges(opts *workloadPolicyOpts) (policy.Update, error) { diff --git a/cmd/fluxctl/release_cmd.go b/cmd/fluxctl/release_cmd.go index 4be5907470..0824f6b3b6 100644 --- a/cmd/fluxctl/release_cmd.go +++ b/cmd/fluxctl/release_cmd.go @@ -162,7 +162,7 @@ func (opts *workloadReleaseOpts) RunE(cmd *cobra.Command, args []string) error { return err } - result, err := awaitJob(ctx, opts.API, jobID) + result, err := awaitJob(ctx, opts.API, jobID, opts.Timeout) if err != nil { return err } @@ -188,7 +188,7 @@ func (opts *workloadReleaseOpts) RunE(cmd *cobra.Command, args []string) error { opts.dryRun = false } - err = await(ctx, cmd.OutOrStdout(), cmd.OutOrStderr(), opts.API, jobID, !opts.dryRun, opts.verbosity) + err = await(ctx, cmd.OutOrStdout(), cmd.OutOrStderr(), opts.API, jobID, !opts.dryRun, opts.verbosity, opts.Timeout) if !opts.watch || err != nil { return err } diff --git a/cmd/fluxctl/root_cmd.go b/cmd/fluxctl/root_cmd.go index 50672ba40e..6bca825e9c 100644 --- a/cmd/fluxctl/root_cmd.go +++ b/cmd/fluxctl/root_cmd.go @@ -2,19 +2,17 @@ package main import ( "fmt" - "net/http" - "net/url" - "os" - "strings" - "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/weaveworks/flux/api" transport "github.com/weaveworks/flux/http" "github.com/weaveworks/flux/http/client" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "net/http" + "net/url" + "os" + "strings" ) type rootOpts struct { @@ -23,6 +21,7 @@ type rootOpts struct { Namespace string Labels map[string]string API api.Server + Timeout int } func newRoot() *rootOpts { @@ -56,6 +55,7 @@ const ( envVariableToken = "FLUX_SERVICE_TOKEN" envVariableCloudToken = "WEAVE_CLOUD_TOKEN" defaultURLGivenToken = "https://cloud.weave.works/api/flux" + envVariableTimeout = "FLUX_TIMEOUT" ) func (opts *rootOpts) Command() *cobra.Command { @@ -75,6 +75,8 @@ func (opts *rootOpts) Command() *cobra.Command { fmt.Sprintf("Base URL of the Flux API (defaults to %q if a token is provided); you can also set the environment variable %s", defaultURLGivenToken, envVariableURL)) cmd.PersistentFlags().StringVarP(&opts.Token, "token", "t", "", fmt.Sprintf("Weave Cloud authentication token; you can also set the environment variable %s or %s", envVariableCloudToken, envVariableToken)) + cmd.PersistentFlags().IntVar(&opts.Timeout, "timeout", 60, + fmt.Sprintf("Global command timeout, in seconds; you can also set the environment variable %s", envVariableTimeout)) cmd.AddCommand( newVersionCommand(), @@ -105,6 +107,7 @@ func (opts *rootOpts) PersistentPreRunE(cmd *cobra.Command, _ []string) error { setFromEnvIfNotSet(cmd.Flags(), "k8s-fwd-labels", envVariableLabels) setFromEnvIfNotSet(cmd.Flags(), "token", envVariableToken, envVariableCloudToken) setFromEnvIfNotSet(cmd.Flags(), "url", envVariableURL) + setFromEnvIfNotSet(cmd.Flags(), "timeout", envVariableTimeout) if opts.Token != "" && opts.URL == "" { opts.URL = defaultURLGivenToken diff --git a/cmd/fluxctl/sync_cmd.go b/cmd/fluxctl/sync_cmd.go index f13fccf9b2..9eb1cd9403 100644 --- a/cmd/fluxctl/sync_cmd.go +++ b/cmd/fluxctl/sync_cmd.go @@ -59,7 +59,7 @@ func (opts *syncOpts) RunE(cmd *cobra.Command, args []string) error { if err != nil { return err } - result, err := awaitJob(ctx, opts.API, jobID) + result, err := awaitJob(ctx, opts.API, jobID, opts.Timeout) if isUnverifiedHead(err) { fmt.Fprintf(cmd.OutOrStderr(), "Warning: %s\n", err) } else if err != nil { @@ -70,7 +70,7 @@ func (opts *syncOpts) RunE(cmd *cobra.Command, args []string) error { rev := result.Revision[:7] fmt.Fprintf(cmd.OutOrStderr(), "Revision of %s to apply is %s\n", gitConfig.Remote.Branch, rev) fmt.Fprintf(cmd.OutOrStderr(), "Waiting for %s to be applied ...\n", rev) - err = awaitSync(ctx, opts.API, rev) + err = awaitSync(ctx, opts.API, rev, opts.Timeout) if err != nil { return err } From ecdc5bcfcf05751247925cef168bd39c64430d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Trigo=20Soares?= <joao@jtsoar.es> Date: Thu, 16 May 2019 16:49:32 +0100 Subject: [PATCH 2/5] go fmt --- cmd/fluxctl/args.go | 5 +++-- cmd/fluxctl/args_test.go | 23 +++++++++++------------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd/fluxctl/args.go b/cmd/fluxctl/args.go index 2c864c5237..b1fc7916b4 100644 --- a/cmd/fluxctl/args.go +++ b/cmd/fluxctl/args.go @@ -3,9 +3,9 @@ package main import ( "bytes" "fmt" - "io/ioutil" + "io/ioutil" "os/exec" - "strings" + "strings" "github.com/spf13/cobra" @@ -35,6 +35,7 @@ func getCommitAuthor() string { } var execCommand = exec.Command + func getUserGitConfigValue(arg string) string { var out bytes.Buffer cmd := execCommand("git", "config", "--get", "--null", arg) diff --git a/cmd/fluxctl/args_test.go b/cmd/fluxctl/args_test.go index 1c634f9648..9b6385daf0 100644 --- a/cmd/fluxctl/args_test.go +++ b/cmd/fluxctl/args_test.go @@ -1,10 +1,10 @@ package main import ( - "fmt" - "testing" + "fmt" "os" "os/exec" + "testing" ) func helperCommand(command string, s ...string) (cmd *exec.Cmd) { @@ -21,7 +21,6 @@ func TestHelperProcess(t *testing.T) { } defer os.Exit(0) - args := os.Args for len(args) > 0 { if args[0] == "--" { @@ -35,18 +34,18 @@ func TestHelperProcess(t *testing.T) { } _, args = args[0], args[1:] - for _, a := range args { - if a == "user.name" { - fmt.Fprintf(os.Stdout, "Jane Doe") - } else if a == "user.email" { - fmt.Fprintf(os.Stdout, "jd@j.d") - } - } + for _, a := range args { + if a == "user.name" { + fmt.Fprintf(os.Stdout, "Jane Doe") + } else if a == "user.email" { + fmt.Fprintf(os.Stdout, "jd@j.d") + } + } } func checkAuthor(t *testing.T, input string, expected string) { - execCommand = helperCommand - defer func(){ execCommand = exec.Command }() + execCommand = helperCommand + defer func() { execCommand = exec.Command }() author := getUserGitConfigValue(input) if author != expected { t.Fatalf("author %q does not match expected value %q", author, expected) From 21f7637cb3547af95feb2e70ea0db8e532d354ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Trigo=20Soares?= <joao@jtsoar.es> Date: Thu, 16 May 2019 16:57:29 +0100 Subject: [PATCH 3/5] Revert "go fmt" This reverts commit ecdc5bcfcf05751247925cef168bd39c64430d9b. --- cmd/fluxctl/args.go | 5 ++--- cmd/fluxctl/args_test.go | 23 ++++++++++++----------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd/fluxctl/args.go b/cmd/fluxctl/args.go index b1fc7916b4..2c864c5237 100644 --- a/cmd/fluxctl/args.go +++ b/cmd/fluxctl/args.go @@ -3,9 +3,9 @@ package main import ( "bytes" "fmt" - "io/ioutil" + "io/ioutil" "os/exec" - "strings" + "strings" "github.com/spf13/cobra" @@ -35,7 +35,6 @@ func getCommitAuthor() string { } var execCommand = exec.Command - func getUserGitConfigValue(arg string) string { var out bytes.Buffer cmd := execCommand("git", "config", "--get", "--null", arg) diff --git a/cmd/fluxctl/args_test.go b/cmd/fluxctl/args_test.go index 9b6385daf0..1c634f9648 100644 --- a/cmd/fluxctl/args_test.go +++ b/cmd/fluxctl/args_test.go @@ -1,10 +1,10 @@ package main import ( - "fmt" + "fmt" + "testing" "os" "os/exec" - "testing" ) func helperCommand(command string, s ...string) (cmd *exec.Cmd) { @@ -21,6 +21,7 @@ func TestHelperProcess(t *testing.T) { } defer os.Exit(0) + args := os.Args for len(args) > 0 { if args[0] == "--" { @@ -34,18 +35,18 @@ func TestHelperProcess(t *testing.T) { } _, args = args[0], args[1:] - for _, a := range args { - if a == "user.name" { - fmt.Fprintf(os.Stdout, "Jane Doe") - } else if a == "user.email" { - fmt.Fprintf(os.Stdout, "jd@j.d") - } - } + for _, a := range args { + if a == "user.name" { + fmt.Fprintf(os.Stdout, "Jane Doe") + } else if a == "user.email" { + fmt.Fprintf(os.Stdout, "jd@j.d") + } + } } func checkAuthor(t *testing.T, input string, expected string) { - execCommand = helperCommand - defer func() { execCommand = exec.Command }() + execCommand = helperCommand + defer func(){ execCommand = exec.Command }() author := getUserGitConfigValue(input) if author != expected { t.Fatalf("author %q does not match expected value %q", author, expected) From 3d27c4d2040c915f75fdb3962437b06aca466d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Trigo=20Soares?= <joao@jtsoar.es> Date: Thu, 16 May 2019 16:58:59 +0100 Subject: [PATCH 4/5] revert reordering of imports --- cmd/fluxctl/root_cmd.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cmd/fluxctl/root_cmd.go b/cmd/fluxctl/root_cmd.go index 6bca825e9c..4f664bbad7 100644 --- a/cmd/fluxctl/root_cmd.go +++ b/cmd/fluxctl/root_cmd.go @@ -2,17 +2,20 @@ package main import ( "fmt" + "net/http" + "net/url" + "os" + "strings" + "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" + "github.com/weaveworks/flux/api" transport "github.com/weaveworks/flux/http" "github.com/weaveworks/flux/http/client" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "net/http" - "net/url" - "os" - "strings" + ) type rootOpts struct { From f80dbaf44c84039efaeb436242b8e5965c473c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Trigo=20Soares?= <joao@jtsoar.es> Date: Fri, 17 May 2019 11:40:10 +0100 Subject: [PATCH 5/5] from discussion on PR#2056 --- .gitignore | 1 - cmd/fluxctl/root_cmd.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 5c46c27462..7e72e279e3 100644 --- a/.gitignore +++ b/.gitignore @@ -40,4 +40,3 @@ docker/fluxy-dumbconf.priv test/profiles test/bin/kubectl test/bin/helm -.idea diff --git a/cmd/fluxctl/root_cmd.go b/cmd/fluxctl/root_cmd.go index 4f664bbad7..30aa6533cb 100644 --- a/cmd/fluxctl/root_cmd.go +++ b/cmd/fluxctl/root_cmd.go @@ -15,7 +15,6 @@ import ( transport "github.com/weaveworks/flux/http" "github.com/weaveworks/flux/http/client" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - ) type rootOpts struct { @@ -52,12 +51,12 @@ Workflow: `) const ( + defaultURLGivenToken = "https://cloud.weave.works/api/flux" envVariableURL = "FLUX_URL" envVariableNamespace = "FLUX_FORWARD_NAMESPACE" envVariableLabels = "FLUX_FORWARD_LABELS" envVariableToken = "FLUX_SERVICE_TOKEN" envVariableCloudToken = "WEAVE_CLOUD_TOKEN" - defaultURLGivenToken = "https://cloud.weave.works/api/flux" envVariableTimeout = "FLUX_TIMEOUT" )