From 06a655a063f1266aa6626e1ef6ea27429b7601c9 Mon Sep 17 00:00:00 2001 From: reubenmiller Date: Mon, 1 Jan 2024 15:40:48 +0100 Subject: [PATCH 1/2] feat: support automatic retries for specific http responses http responses are retried if a StatusCode in the 5xx range is received. retries can be set via global flag or config --- go.mod | 2 ++ go.sum | 8 +++++++ pkg/cmd/factory/c8yclient.go | 28 +++++++++++++++++++++++- pkg/cmd/root/root.go | 3 +++ pkg/config/cliConfiguration.go | 39 ++++++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 9913d0da0..09285ded8 100644 --- a/go.mod +++ b/go.mod @@ -47,6 +47,7 @@ require github.com/hashicorp/go-version v1.6.0 require ( github.com/cli/browser v1.3.0 + github.com/hashicorp/go-retryablehttp v0.7.5 github.com/reubenmiller/gojsonq/v2 v2.0.0-20221119213524-0fd921ac20a3 ) @@ -66,6 +67,7 @@ require ( github.com/golang-jwt/jwt/v4 v4.5.0 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/gorilla/css v1.0.1 // indirect + github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/hcl v1.0.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect diff --git a/go.sum b/go.sum index bdf59a5cf..129f8b496 100644 --- a/go.sum +++ b/go.sum @@ -71,6 +71,13 @@ github.com/gorilla/css v1.0.1 h1:ntNaBIghp6JmvWnxbZKANoLyuXTPZ4cAMlo6RyhlbO8= github.com/gorilla/css v1.0.1/go.mod h1:BvnYkspnSzMmwRK+b8/xgNPLiIuNZr6vbZBTPQ2A3b0= github.com/gorilla/websocket v1.5.1 h1:gmztn0JnHVt9JZquRuzLw3g4wouNVzKL15iLr/zn/QY= github.com/gorilla/websocket v1.5.1/go.mod h1:x3kM2JMyaluk02fnUJpQuwD2dCS5NDG2ZHL0uE0tcaY= +github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= +github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= +github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ= +github.com/hashicorp/go-hclog v1.5.0 h1:bI2ocEMgcVlz55Oj1xZNBsVi900c7II+fWDyV9o+13c= +github.com/hashicorp/go-hclog v1.5.0/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M= +github.com/hashicorp/go-retryablehttp v0.7.5 h1:bJj+Pj19UZMIweq/iie+1u5YCdGrnxCT9yvm0e+Nd5M= +github.com/hashicorp/go-retryablehttp v0.7.5/go.mod h1:Jy/gPYAdjqffZ/yFGCFV2doI5wjtH1ewM9u8iYVjtX8= github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek= github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= @@ -185,6 +192,7 @@ github.com/spf13/viper v1.18.2/go.mod h1:EKmWIqdnk5lOcmR72yw6hS+8OPYcwD0jteitLMV github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= diff --git a/pkg/cmd/factory/c8yclient.go b/pkg/cmd/factory/c8yclient.go index dbbd60e8b..387582b75 100644 --- a/pkg/cmd/factory/c8yclient.go +++ b/pkg/cmd/factory/c8yclient.go @@ -10,6 +10,7 @@ import ( "time" "github.com/gorilla/websocket" + "github.com/hashicorp/go-retryablehttp" "github.com/reubenmiller/go-c8y-cli/v2/pkg/c8ysession" "github.com/reubenmiller/go-c8y-cli/v2/pkg/cmdutil" "github.com/reubenmiller/go-c8y-cli/v2/pkg/config" @@ -46,6 +47,16 @@ func WithCompression(enable bool) c8y.ClientOption { } } +// RetryLogger to customize the log messages produced by the http retry client +type RetryLogger struct { + l *logger.Logger +} + +func (l RetryLogger) Printf(format string, args ...interface{}) { + format = strings.TrimPrefix(format, "[DEBUG] ") + l.l.Infof(format, args...) +} + func CreateCumulocityClient(f *cmdutil.Factory, sessionFile, username, password string, disableEncryptionCheck bool) func() (*c8y.Client, error) { return func() (*c8y.Client, error) { cfg, err := f.Config() @@ -68,12 +79,27 @@ func CreateCumulocityClient(f *cmdutil.Factory, sessionFile, username, password log.Debug("Creating c8y client") configureProxySettings(cfg, log) - httpClient := c8y.NewHTTPClient( + internalHttpClient := c8y.NewHTTPClient( WithProxyDisabled(cfg.IgnoreProxy()), c8y.WithInsecureSkipVerify(cfg.SkipSSLVerify()), WithCompression(cfg.ShouldUseCompression()), ) + // Use retry client + retryClient := retryablehttp.NewClient() + retryClient.RetryMax = cfg.HTTPRetryMax() + retryClient.RetryWaitMin = cfg.HTTPRetryWaitMin() + retryClient.RetryWaitMax = cfg.HTTPRetryWaitMax() + retryClient.Logger = RetryLogger{l: log} + retryClient.ErrorHandler = func(resp *http.Response, err error, numTries int) (*http.Response, error) { + // Pass error back so that the activity log is processed + log.Warnf("Giving up after %d attempt/s. err=%s", numTries, err) + defer resp.Body.Close() + return resp, err + } + retryClient.HTTPClient = internalHttpClient + httpClient := retryClient.StandardClient() + cacheBodyPaths := cfg.CacheBodyKeys() if len(cacheBodyPaths) > 0 { log.Infof("Caching of body only includes paths: %s", strings.Join(cacheBodyPaths, ", ")) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index b5ddaa248..05cd0fb4d 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -270,6 +270,9 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *CmdRoot { // ssl settings cmd.PersistentFlags().BoolP("insecure", "k", false, "Allow insecure server connections when using SSL") + // retries + cmd.PersistentFlags().Int("retries", 3, "Max number of attempts when a failed http call is encountered") + completion.WithOptions( cmd, completion.WithValidateSet("dryFormat", "json", "dump", "curl", "markdown"), diff --git a/pkg/config/cliConfiguration.go b/pkg/config/cliConfiguration.go index 2b4a3302d..315759cbc 100644 --- a/pkg/config/cliConfiguration.go +++ b/pkg/config/cliConfiguration.go @@ -112,6 +112,15 @@ const ( // SettingsUseCompression use compression for HTTP client SettingsUseCompression = "settings.http.compression" + // SettingsHTTPMaxRetries maximum number of retries by the HTTP client + SettingsHTTPMaxRetries = "settings.defaults.retries" + + // SettingsHTTPRetryWaitMin minimum duration to wait before retrying a failed HTTP request + SettingsHTTPRetryWaitMin = "settings.http.retryWaitMin" + + // SettingsHTTPRetryWaitMax maximum duration to wait before retrying a failed HTTP request + SettingsHTTPRetryWaitMax = "settings.http.retryWaitMax" + // SettingsDryRunFormat dry run output format. Controls how the dry run information is displayed SettingsDryRunFormat = "settings.defaults.dryFormat" @@ -447,6 +456,9 @@ func (c *Config) bindSettings() { // HTTP settings WithBindEnv(SettingsUseCompression, true), + WithBindEnv(SettingsHTTPMaxRetries, 3), + WithBindEnv(SettingsHTTPRetryWaitMax, "50s"), + WithBindEnv(SettingsHTTPRetryWaitMin, "5s"), // Dry run options WithBindEnv(SettingsDryRunPattern, ""), @@ -1103,6 +1115,33 @@ func (c *Config) ShouldUseCompression() bool { return c.viper.GetBool(SettingsUseCompression) } +// HTTPRetryMax get the maximum number of retries on failed http requests +func (c *Config) HTTPRetryMax() int { + return c.viper.GetInt(SettingsHTTPMaxRetries) +} + +// HTTPRetryWaitMax get the maximum wait time between failed http requests +func (c *Config) HTTPRetryWaitMax() time.Duration { + value := c.viper.GetString(SettingsHTTPRetryWaitMax) + duration, err := flags.GetDuration(value, true, time.Second) + if err != nil { + c.Logger.Warnf("Invalid duration. value=%s, err=%s", duration, err) + return 0 + } + return duration +} + +// HTTPRetryWaitMin get the minimum wait time between failed http requests +func (c *Config) HTTPRetryWaitMin() time.Duration { + value := c.viper.GetString(SettingsHTTPRetryWaitMin) + duration, err := flags.GetDuration(value, true, time.Second) + if err != nil { + c.Logger.Warnf("Invalid duration. value=%s, err=%s", duration, err) + return 0 + } + return duration +} + // ShouldUseDryRun returns true of dry run should be applied to the command based on the type of method func (c *Config) ShouldUseDryRun(commandLine string) bool { if c.DryRun() { From e188b226eb793d5545608c5d3b2fa28a2d2adabf Mon Sep 17 00:00:00 2001 From: reubenmiller Date: Mon, 1 Jan 2024 16:32:47 +0100 Subject: [PATCH 2/2] add response nil check --- pkg/cmd/factory/c8yclient.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/factory/c8yclient.go b/pkg/cmd/factory/c8yclient.go index 387582b75..87683cdc6 100644 --- a/pkg/cmd/factory/c8yclient.go +++ b/pkg/cmd/factory/c8yclient.go @@ -94,7 +94,9 @@ func CreateCumulocityClient(f *cmdutil.Factory, sessionFile, username, password retryClient.ErrorHandler = func(resp *http.Response, err error, numTries int) (*http.Response, error) { // Pass error back so that the activity log is processed log.Warnf("Giving up after %d attempt/s. err=%s", numTries, err) - defer resp.Body.Close() + if resp != nil && resp.Body != nil { + defer resp.Body.Close() + } return resp, err } retryClient.HTTPClient = internalHttpClient