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

Add default retry for load config via url #8803

Merged
merged 2 commits into from
Feb 12, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
32 changes: 24 additions & 8 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ var (
`"`, `\"`,
`\`, `\\`,
)
httpLoadConfigRetryInterval = 10 * time.Second
)

// Config specifies the URL/user/password for the database that telegraf
Expand Down Expand Up @@ -921,17 +922,32 @@ func fetchConfig(u *url.URL) ([]byte, error) {
}
req.Header.Add("Accept", "application/toml")
req.Header.Set("User-Agent", internal.ProductToken())
resp, err := http.DefaultClient.Do(req)
if err != nil {
return nil, err
}

if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("failed to retrieve remote config: %s", resp.Status)
retries := 3
ssoroka marked this conversation as resolved.
Show resolved Hide resolved
for i := 1; i <= retries; i++ {
Copy link
Contributor

Choose a reason for hiding this comment

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

if retries is 0, this won't attempt at all. If we later move to user-configurable retries, that won't quite make sense

Copy link
Member Author

Choose a reason for hiding this comment

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

Right changed i to 0

resp, err := http.DefaultClient.Do(req)
if err != nil {
ssoroka marked this conversation as resolved.
Show resolved Hide resolved
if i < retries {
log.Printf("Unable to connect to HTTP config server. Retry %d of %d in %s. %s", i, retries, httpLoadConfigRetryInterval, err)
time.Sleep(httpLoadConfigRetryInterval)
continue
}
return nil, fmt.Errorf("Retry %d of %d failed connecting to HTTP config server %s", i, retries, err)
}

if resp.StatusCode != http.StatusOK {
if i < retries {
log.Printf("Error getting HTTP config. Retry %d of %d in %s. Status=%d", i, retries, httpLoadConfigRetryInterval, resp.StatusCode)
time.Sleep(httpLoadConfigRetryInterval)
continue
}
return nil, fmt.Errorf("Retry %d of %d failed to retrieve remote config: %s", i, retries, resp.Status)
}
defer resp.Body.Close()
return ioutil.ReadAll(resp.Body)
}

defer resp.Body.Close()
return ioutil.ReadAll(resp.Body)
return nil, nil
}

// parseConfig loads a TOML configuration from a provided path and
Expand Down
36 changes: 36 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package config

import (
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -278,3 +280,37 @@ func TestConfig_AzureMonitorNamespacePrefix(t *testing.T) {
assert.Equal(t, "", azureMonitor.NamespacePrefix)
assert.Equal(t, true, ok)
}

func TestConfig_URLRetries3Fails(t *testing.T) {
httpLoadConfigRetryInterval = 0 * time.Second
responseCounter := 0
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
responseCounter++
}))
defer ts.Close()

c := NewConfig()
err := c.LoadConfig(ts.URL)
require.Error(t, err)
require.Equal(t, 3, responseCounter)
}

func TestConfig_URLRetries2FailsThenPasses(t *testing.T) {
httpLoadConfigRetryInterval = 0 * time.Second
responseCounter := 0
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if responseCounter <= 1 {
w.WriteHeader(http.StatusNotFound)
} else {
w.WriteHeader(http.StatusOK)
}
responseCounter++
}))
defer ts.Close()

c := NewConfig()
err := c.LoadConfig(ts.URL)
require.NoError(t, err)
require.Equal(t, 3, responseCounter)
}