Merge pull request #12 from kkupreeva/http-client-timeout

Configure HTTP client timeouts

README.md

@@ -67,6 +67,7 @@ wait-for:
     target: http://the-service:8080/health?reload=true
     interval: 5s
     timeout: 60s
+    http-client-timeout: 3s
   another-service:
     type: http
     target: https://another-one

cmd/wait-for/main.go

@@ -14,10 +14,12 @@ func main() {
 	log.SetFlags(log.LstdFlags | log.Lmicroseconds)
 
 	timeoutParam := "5s"
+	httpTimeoutParam := "1s"
 	configFile := ""
 	var quiet bool
 
 	flag.StringVar(&timeoutParam, "timeout", timeoutParam, "time to wait for services to become available")
+	flag.StringVar(&httpTimeoutParam, "http_timeout", httpTimeoutParam, "timeout for requests made by a http client")
 	flag.StringVar(&configFile, "config", "", "configuration file to use")
 	flag.BoolVar(&quiet, "quiet", false, "reduce output to the minimum")
 	flag.Parse()
@@ -32,7 +34,7 @@ func main() {
 		logger = waitfor.NullLogger
 	}
 
-	config, err := waitfor.OpenConfig(configFile, timeoutParam, fs)
+	config, err := waitfor.OpenConfig(configFile, timeoutParam, httpTimeoutParam, fs)
 	if err != nil {
 		_, _ = fmt.Fprintf(os.Stderr, "%v", err)
 		os.Exit(1)

config.go

@@ -9,9 +9,12 @@ import (
 	yaml "gopkg.in/yaml.v3"
 )
 
-// DefautTimeout is the amount of time to wait for target before failing
+// DefaultTimeout is the amount of time to wait for target before failing
 const DefaultTimeout = time.Second * 5
 
+// DefaultHTTPClientTimeout a default value for a time limit for requests made by http client
+const DefaultHTTPClientTimeout = time.Second
+
 // TargetConfig is the configuration of a single target
 type TargetConfig struct {
 	// Type is the kind of target being described
@@ -20,19 +23,23 @@ type TargetConfig struct {
 	Target string
 	// Timeout is the timeout to use for this specific target if it is different to DefaultTimeout
 	Timeout time.Duration
+	// HTTPClientTimeout is the timeout for requests made by a http client
+	HTTPClientTimeout time.Duration `yaml:"http-client-timeout"`
 }
 
 // Config represents all of the config that can be defined in a config file
 type Config struct {
-	DefaultTimeout time.Duration `yaml:"default-timeout"`
-	Targets        map[string]TargetConfig
+	DefaultTimeout           time.Duration `yaml:"default-timeout"`
+	Targets                  map[string]TargetConfig
+	DefaultHTTPClientTimeout time.Duration `yaml:"default-http-client-timeout"`
 }
 
 // NewConfig creates an empty Config
 func NewConfig() *Config {
 	return &Config{
-		DefaultTimeout: DefaultTimeout,
-		Targets:        map[string]TargetConfig{},
+		DefaultTimeout:           DefaultTimeout,
+		Targets:                  map[string]TargetConfig{},
+		DefaultHTTPClientTimeout: DefaultHTTPClientTimeout,
 	}
 }
 
@@ -46,12 +53,18 @@ func NewConfigFromFile(r io.Reader) (*Config, error) {
 	if config.DefaultTimeout == 0 {
 		config.DefaultTimeout = DefaultTimeout
 	}
+	if config.DefaultHTTPClientTimeout == 0 {
+		config.DefaultHTTPClientTimeout = DefaultHTTPClientTimeout
+	}
 	for t := range config.Targets {
+		target := config.Targets[t]
 		if config.Targets[t].Timeout == 0 {
-			target := config.Targets[t]
 			target.Timeout = config.DefaultTimeout
-			config.Targets[t] = target
 		}
+		if config.Targets[t].HTTPClientTimeout == 0 {
+			target.HTTPClientTimeout = config.DefaultHTTPClientTimeout
+		}
+		config.Targets[t] = target
 	}
 	return &config, nil
 }
@@ -75,9 +88,10 @@ func (c *Config) AddFromString(t string) error {
 
 	if strings.HasPrefix(t, "http:") || strings.HasPrefix(t, "https:") {
 		c.Targets[t] = TargetConfig{
-			Target:  t,
-			Type:    "http",
-			Timeout: c.DefaultTimeout,
+			Target:            t,
+			Type:              "http",
+			Timeout:           c.DefaultTimeout,
+			HTTPClientTimeout: c.DefaultHTTPClientTimeout,
 		}
 		return nil
 	}

config_test.go

@@ -17,6 +17,7 @@ func TestConfig_fromYAML(t *testing.T) {
 	assert.Equal(t, "http://localhost/health", config.Targets["http-connection"].Target)
 	assert.Equal(t, time.Second*10, config.Targets["http-connection"].Timeout)
 	assert.Equal(t, "http", config.Targets["http-connection"].Type)
+	assert.Equal(t, time.Second*5, config.Targets["http-connection"].HTTPClientTimeout)
 	assert.Equal(t, "localhost:80", config.Targets["tcp-connection"].Target)
 	assert.Equal(t, "tcp", config.Targets["tcp-connection"].Type)
 	assert.Equal(t, time.Second*5, config.Targets["tcp-connection"].Timeout)
@@ -33,6 +34,18 @@ func TestConfig_incorrectTimeDurationFails(t *testing.T) {
 	assert.Nil(t, config)
 }
 
+func TestConfig_incorrectHTTPTimeoutDurationFails(t *testing.T) {
+	config, err := NewConfigFromFile(strings.NewReader(`targets:
+  timeout-connection:
+    type: http
+    target: http://localhost/health
+    timeout: 10s
+    http-client-timeout: not parsable`))
+
+	assert.Error(t, err)
+	assert.Nil(t, config)
+}
+
 func TestConfig_settingDefaultTimeoutWorks(t *testing.T) {
 	config, err := NewConfigFromFile(strings.NewReader(`
 default-timeout: 18s
@@ -45,6 +58,19 @@ targets:
 	assert.Equal(t, time.Second*18, config.Targets["http-connection"].Timeout)
 }
 
+func TestConfig_settingDefaultHTTPTimeoutWorks(t *testing.T) {
+	config, err := NewConfigFromFile(strings.NewReader(`
+default-http-client-timeout: 18s
+targets:
+  http-connection:
+    type: http
+    target: http://localhost/health
+    timeout: 10s`))
+
+	assert.NoError(t, err)
+	assert.Equal(t, time.Second*18, config.Targets["http-connection"].HTTPClientTimeout)
+}
+
 func TestConfig_GotTarget(t *testing.T) {
 	config, _ := NewConfigFromFile(strings.NewReader(defaultConfigYaml()))
 
@@ -66,14 +92,17 @@ func TestConfig_AddFromString(t *testing.T) {
 	assert.Equal(t, "http://some-host/endpoint", config.Targets["http://some-host/endpoint"].Target)
 	assert.Equal(t, "http", config.Targets["http://some-host/endpoint"].Type)
 	assert.Equal(t, time.Second*5, config.Targets["http://some-host/endpoint"].Timeout)
+	assert.Equal(t, time.Second, config.Targets["http://some-host/endpoint"].HTTPClientTimeout)
 
 	assert.Equal(t, "https://some-host/endpoint", config.Targets["https://some-host/endpoint"].Target)
 	assert.Equal(t, "http", config.Targets["https://some-host/endpoint"].Type)
 	assert.Equal(t, time.Second*5, config.Targets["https://some-host/endpoint"].Timeout)
+	assert.Equal(t, time.Second, config.Targets["http://some-host/endpoint"].HTTPClientTimeout)
 
 	assert.Equal(t, "http://another-host/endpoint", config.Targets["http://another-host/endpoint"].Target)
 	assert.Equal(t, "http", config.Targets["http://another-host/endpoint"].Type)
 	assert.Equal(t, time.Second*5, config.Targets["http://some-host/endpoint"].Timeout)
+	assert.Equal(t, time.Second, config.Targets["http://some-host/endpoint"].HTTPClientTimeout)
 
 	assert.Equal(t, "listener-tcp:9090", config.Targets["tcp:listener-tcp:9090"].Target)
 	assert.Equal(t, "tcp", config.Targets["tcp:listener-tcp:9090"].Type)
@@ -100,6 +129,7 @@ func defaultConfigYaml() string {
     type: http
     target: http://localhost/health
     timeout: 10s
+    http-client-timeout: 5s
   tcp-connection:
     type: tcp
     target: localhost:80

waitfor.go

@@ -13,7 +13,7 @@ import (
 
 // WaiterFunc is used to implement waiting for a specific type of target.
 // The name is used in the error and target is the actual destination being tested.
-type WaiterFunc func(name string, target string) error
+type WaiterFunc func(name string, target *TargetConfig) error
 type Logger func(string, ...interface{})
 
 // NullLogger can be used in place of a real logging function
@@ -46,7 +46,7 @@ func WaitOn(config *Config, logger Logger, targets []string, waiters map[string]
 	return nil
 }
 
-func OpenConfig(configFile, defaultTimeout string, fs afero.Fs) (*Config, error) {
+func OpenConfig(configFile, defaultTimeout, defaultHTTPTimeout string, fs afero.Fs) (*Config, error) {
 	var config *Config
 	if configFile == "" {
 		config = NewConfig()
@@ -67,6 +67,12 @@ func OpenConfig(configFile, defaultTimeout string, fs afero.Fs) (*Config, error)
 	}
 	config.DefaultTimeout = timeout
 
+	httpTimeout, err := time.ParseDuration(defaultHTTPTimeout)
+	if err != nil {
+		return nil, fmt.Errorf("unable to parse http timeout: %v", err)
+	}
+	config.DefaultHTTPClientTimeout = httpTimeout
+
 	return config, nil
 }
 
@@ -101,11 +107,11 @@ func waitOnTargets(logger Logger, targets map[string]TargetConfig, waiters map[s
 func waitOnSingleTarget(name string, logger Logger, target TargetConfig, waiter WaiterFunc) error {
 	end := time.Now().Add(target.Timeout)
 
-	err := waiter(name, target.Target)
+	err := waiter(name, &target)
 	for err != nil && end.After(time.Now()) {
 		logger("error while waiting for %s: %v", name, err)
 		time.Sleep(time.Second)
-		err = waiter(name, target.Target)
+		err = waiter(name, &target)
 	}
 
 	if err != nil {
@@ -117,8 +123,8 @@ func waitOnSingleTarget(name string, logger Logger, target TargetConfig, waiter
 	return nil
 }
 
-func TCPWaiter(name string, target string) error {
-	conn, err := net.Dial("tcp", target)
+func TCPWaiter(name string, target *TargetConfig) error {
+	conn, err := net.Dial("tcp", target.Target)
 	if err != nil {
 		return fmt.Errorf("could not connect to %s: %v", name, err)
 	}
@@ -127,11 +133,11 @@ func TCPWaiter(name string, target string) error {
 	return nil
 }
 
-func HTTPWaiter(name string, target string) error {
+func HTTPWaiter(name string, target *TargetConfig) error {
 	client := &http.Client{
-		Timeout: time.Second * 1,
+		Timeout: target.HTTPClientTimeout,
 	}
-	req, _ := http.NewRequest("GET", target, nil)
+	req, _ := http.NewRequest("GET", target.Target, nil)
 	resp, err := client.Do(req)
 	if err != nil {
 		return fmt.Errorf("could not connect to %s: %v", name, err)

waitfor_test.go

@@ -25,7 +25,7 @@ func Test_isSuccess(t *testing.T) {
 func TestOpenConfig_errorOnFileOpenFailure(t *testing.T) {
 	mockFS := afero.NewMemMapFs()
 
-	config, err := OpenConfig("./wait-for.yaml", "", afero.NewReadOnlyFs(mockFS))
+	config, err := OpenConfig("./wait-for.yaml", "", "", afero.NewReadOnlyFs(mockFS))
 	assert.Error(t, err)
 	assert.Nil(t, config)
 }
@@ -34,7 +34,7 @@ func TestOpenConfig_errorOnFileParsingFailure(t *testing.T) {
 	mockFS := afero.NewMemMapFs()
 	_ = afero.WriteFile(mockFS, "./wait-for.yaml", []byte("this isn't yaml!"), 0444)
 
-	config, err := OpenConfig("./wait-for.yaml", "", afero.NewReadOnlyFs(mockFS))
+	config, err := OpenConfig("./wait-for.yaml", "", "", afero.NewReadOnlyFs(mockFS))
 	assert.Error(t, err)
 	assert.Nil(t, config)
 }
@@ -43,7 +43,16 @@ func TestOpenConfig_errorOnParsingDefaultTimeout(t *testing.T) {
 	mockFS := afero.NewMemMapFs()
 	_ = afero.WriteFile(mockFS, "./wait-for.yaml", []byte(defaultConfigYaml()), 0444)
 
-	config, err := OpenConfig("./wait-for.yaml", "invalid duration", afero.NewReadOnlyFs(mockFS))
+	config, err := OpenConfig("./wait-for.yaml", "invalid duration", "1s", afero.NewReadOnlyFs(mockFS))
+	assert.Error(t, err)
+	assert.Nil(t, config)
+}
+
+func TestOpenConfig_errorOnParsingDefaultHTTPTimeout(t *testing.T) {
+	mockFS := afero.NewMemMapFs()
+	_ = afero.WriteFile(mockFS, "./wait-for.yaml", []byte(defaultConfigYaml()), 0444)
+
+	config, err := OpenConfig("./wait-for.yaml", "10s", "invalid duration", afero.NewReadOnlyFs(mockFS))
 	assert.Error(t, err)
 	assert.Nil(t, config)
 }
@@ -52,12 +61,22 @@ func TestOpenConfig_defaultTimeoutCanBeSet(t *testing.T) {
 	mockFS := afero.NewMemMapFs()
 	_ = afero.WriteFile(mockFS, "./wait-for.yaml", []byte(defaultConfigYaml()), 0444)
 
-	config, err := OpenConfig("./wait-for.yaml", "19s", afero.NewReadOnlyFs(mockFS))
+	config, err := OpenConfig("./wait-for.yaml", "19s", "1s", afero.NewReadOnlyFs(mockFS))
 	assert.NoError(t, err)
 	assert.NotNil(t, config)
 	assert.Equal(t, time.Second*19, config.DefaultTimeout)
 }
 
+func TestOpenConfig_defaultHTTPTimeoutCanBeSet(t *testing.T) {
+	mockFS := afero.NewMemMapFs()
+	_ = afero.WriteFile(mockFS, "./wait-for.yaml", []byte(defaultConfigYaml()), 0444)
+
+	config, err := OpenConfig("./wait-for.yaml", "19s", "20s", afero.NewReadOnlyFs(mockFS))
+	assert.NoError(t, err)
+	assert.NotNil(t, config)
+	assert.Equal(t, time.Second*20, config.DefaultHTTPClientTimeout)
+}
+
 func TestWaitOn_errorsInvalidTarget(t *testing.T) {
 	err := WaitOn(NewConfig(), NullLogger, []string{"localhost"}, map[string]WaiterFunc{})
 	assert.Error(t, err)
@@ -76,7 +95,7 @@ func TestWaitOnSingleTarget_succeedsImmediately(t *testing.T) {
 		"name",
 		doLog,
 		TargetConfig{Timeout: time.Second * 2},
-		func(name string, target string) error { return nil },
+		func(name string, target *TargetConfig) error { return nil },
 	)
 
 	assert.NoError(t, err)
@@ -94,7 +113,7 @@ func TestWaitOnSingleTarget_succeedsAfterWaiting(t *testing.T) {
 		"name",
 		doLog,
 		TargetConfig{Timeout: time.Second * 2},
-		func(name string, target string) error {
+		func(name string, target *TargetConfig) error {
 			if waitUntil.After(time.Now()) {
 				return fmt.Errorf("there was an error")
 			}
@@ -115,7 +134,7 @@ func TestWaitOnSingleTarget_failsIfTimerExpires(t *testing.T) {
 		"name",
 		doLog,
 		TargetConfig{Timeout: time.Second * 2},
-		func(name string, target string) error {
+		func(name string, target *TargetConfig) error {
 			return fmt.Errorf("")
 		},
 	)
@@ -128,7 +147,7 @@ func TestWaitOnTargets_failsForUnknownType(t *testing.T) {
 	err := waitOnTargets(
 		NullLogger,
 		map[string]TargetConfig{"unkown": {Type: "unknown type"}},
-		map[string]WaiterFunc{"type": func(string, string) error { return errors.New("") }},
+		map[string]WaiterFunc{"type": func(string, *TargetConfig) error { return errors.New("") }},
 	)
 
 	require.Error(t, err)
@@ -142,8 +161,8 @@ func TestWaitOnTargets_selectsCorrectWaiter(t *testing.T) {
 			"type 1": {Type: "t1"},
 		},
 		map[string]WaiterFunc{
-			"t1": func(string, string) error { return nil },
-			"t2": func(string, string) error { return errors.New("an error") },
+			"t1": func(string, *TargetConfig) error { return nil },
+			"t2": func(string, *TargetConfig) error { return errors.New("an error") },
 		},
 	)
 
@@ -158,8 +177,8 @@ func TestWaitOnTargets_failsWhenWaiterFails(t *testing.T) {
 			"type 2": {Type: "t2"},
 		},
 		map[string]WaiterFunc{
-			"t1": func(string, string) error { return nil },
-			"t2": func(string, string) error { return errors.New("an error") },
+			"t1": func(string, *TargetConfig) error { return nil },
+			"t2": func(string, *TargetConfig) error { return errors.New("an error") },
 		},
 	)