Allow file provider to load service config from files in a directory. #1672
Conversation
ldez
added
kind/enhancement
There was a problem hiding this comment.
I optimistically went ahead and left a few comments. However, we still need to formally approve the PR's design and purpose. @emilevauge, WDYT?
| @@ -0,0 +1,30 @@ | |||
| # rules | |||
There was a problem hiding this comment.
Can we trim down the configuration file to the bare minimum we need in order for the test to succeed?
Same for simple2.toml.
provider/file/file.go
Outdated
| return | ||
| case evt := <-watcher.Events: | ||
| event = &evt | ||
| case <-time.After(debouncePeriod): |
There was a problem hiding this comment.
The timer will be reset every time one of the other events comes in (because we will move to the next for-loop iteration and have line 74 create a new fresh time.After timer).
I think what you want to do instead is create a new ticker using time.NewTicker before the loop starts and reference it's C channel in the case statement.
(While doing so, you should also defer-execute Stop() on the created ticker to avoid resource leakage.)
There was a problem hiding this comment.
Having the timer reset each time an event comes in is what I wanted to achieve. This lets things settle - for example if an external process is re-writing the files - before sending the updated configuration via the configuration channel. What I saw during testing was that multiple events were fired, even for a single edit, and Traefik was then trying to reload config more than needed. To be fair, the existing logic which checks if the provided config has changed kicked in and therefore nothing was actually done with it, but I felt it better just to put the delay in at the point where we know multiple events can occur in quick succession.
There was a problem hiding this comment.
I see your point, it's a fair one. On the other hand, that external process could be so busy updating the file that the actual event processing case could be "starving", leaving the configuration out-of-date for possibly more than two seconds.
Admittedly, I don't know how realistic that scenario actually is. My other argument would be the deviation in behavior from the other provider watches.
I can see arguments for both directions, so I'll let @emilevauge chime in on this one.
provider/file/file.go
Outdated
| configuration := new(types.Configuration) | ||
| if _, err := toml.DecodeFile(filename, configuration); err != nil { | ||
| log.Error("Error reading file:", err) | ||
| return nil | ||
| return nil, fmt.Errorf("error reading file: %s", err) |
There was a problem hiding this comment.
Can we say "configuration file"?
provider/file/file.go
Outdated
| } | ||
|
|
||
| for k, v := range c.Backends { | ||
| configuration.Backends[k] = v |
There was a problem hiding this comment.
Should we check here whether a given backend key has already been configured, and if so, log a warning and skip the current backend?
Same question for frontends below.
provider/file/file_test.go
Outdated
| ) | ||
|
|
||
| const ( | ||
| frontends = ` |
There was a problem hiding this comment.
Can we reduce the test definitions?
There was a problem hiding this comment.
I think, you can reduce test definitions like that:
file_test.go
package file
import (
"context"
"fmt"
"io/ioutil"
"os"
"path"
"testing"
"time"
"github.com/containous/traefik/provider"
"github.com/containous/traefik/safe"
"github.com/containous/traefik/types"
"github.com/stretchr/testify/assert"
)
func TestProvideSingleFile(t *testing.T) {
tempDir := createTempDir(t, "testfile")
defer os.RemoveAll(tempDir)
expectedNumFrontends := 2
expectedNumBackends := 2
tempFile := createFile(t,
tempDir, "simple.toml",
createFrontendConfiguration(expectedNumFrontends),
createBackendConfiguration(expectedNumBackends))
configurationChan, signal := createConfigurationRoutine(t, &expectedNumFrontends, &expectedNumBackends)
fileProvider := Provider{
BaseProvider: provider.BaseProvider{
Filename: tempFile.Name(),
Watch: true,
},
}
fileProvider.Provide(configurationChan, safe.NewPool(context.Background()), nil)
// Wait for initial message to be tested
waitForSignal(t, signal, 2*time.Second, "initial config")
// Now test again with single frontend and backend
expectedNumFrontends = 1
expectedNumBackends = 1
tempFile = createFile(t,
tempDir, "simple.toml",
createFrontendConfiguration(expectedNumFrontends),
createBackendConfiguration(expectedNumBackends))
waitForSignal(t, signal, 2*time.Second, "single frontend and backend")
}
func TestProvideDirectory(t *testing.T) {
tempDir := createTempDir(t, "testdir")
defer os.RemoveAll(tempDir)
expectedNumFrontends := 2
expectedNumBackends := 2
tempFile1 := createRandomFile(t, tempDir, createFrontendConfiguration(expectedNumFrontends))
tempFile2 := createRandomFile(t, tempDir, createBackendConfiguration(expectedNumBackends))
configurationChan, signal := createConfigurationRoutine(t, &expectedNumFrontends, &expectedNumBackends)
fileProvider := Provider{
BaseProvider: provider.BaseProvider{
Watch: true,
},
Directory: tempDir,
}
fileProvider.Provide(configurationChan, safe.NewPool(context.Background()), nil)
// Wait for initial config message to be tested
waitForSignal(t, signal, 2*time.Second, "initial config")
// Now remove the backends file
expectedNumFrontends = 2
expectedNumBackends = 0
os.Remove(tempFile2.Name())
waitForSignal(t, signal, 2*time.Second, "remove the backends file")
// Now remove the frontends file
expectedNumFrontends = 0
expectedNumBackends = 0
os.Remove(tempFile1.Name())
waitForSignal(t, signal, 2*time.Second, "remove the frontends file")
}
func createConfigurationRoutine(t *testing.T, expectedNumFrontends *int, expectedNumBackends *int) (chan types.ConfigMessage, chan interface{}) {
configurationChan := make(chan types.ConfigMessage)
signal := make(chan interface{})
go func() {
for {
data := <-configurationChan
assert.Equal(t, "file", data.ProviderName)
assert.Len(t, data.Configuration.Frontends, *expectedNumFrontends)
assert.Len(t, data.Configuration.Backends, *expectedNumBackends)
signal <- nil
}
}()
return configurationChan, signal
}
func waitForSignal(t *testing.T, signal chan interface{}, timeout time.Duration, caseName string) {
timer := time.NewTimer(timeout)
defer timer.Stop()
select {
case <-signal:
case <-timer.C:
t.Fatalf("Timed out waiting for assertions to be tested: %s", caseName)
}
}
// createRandomFile Helper
func createRandomFile(t *testing.T, tempDir string, contents ...string) *os.File {
return createFile(t, tempDir, fmt.Sprintf("temp%d.toml", time.Now().UnixNano()), contents...)
}
// createFile Helper
func createFile(t *testing.T, tempDir string, name string, contents ...string) *os.File {
fileName := path.Join(tempDir, name)
tempFile, err := os.Create(fileName)
if err != nil {
t.Fatal(err)
}
for _, content := range contents {
_, err := tempFile.WriteString(content)
if err != nil {
t.Fatal(err)
}
}
err = tempFile.Close()
if err != nil {
t.Fatal(err)
}
return tempFile
}
// createTempDir Helper
func createTempDir(t *testing.T, dir string) string {
d, err := ioutil.TempDir("", dir)
if err != nil {
t.Fatal(err)
}
return d
}
// createFrontendConfiguration Helper
func createFrontendConfiguration(n int) string {
conf := "[frontends]\n"
for i := 1; i <= n; i++ {
conf += fmt.Sprintf(` [frontends.frontend%[1]d]
backend = "backend%[1]d"
`, i)
}
return conf
}
// createBackendConfiguration Helper
func createBackendConfiguration(n int) string {
conf := "[backends]\n"
for i := 1; i <= n; i++ {
conf += fmt.Sprintf(` [backends.backend%[1]d]
[backends.backend%[1]d.servers.server1]
url = "http://172.17.0.%[1]d:80"
`, i)
}
return conf
}There was a problem hiding this comment.
After reflections, I think you can add 2 tests (when not watch) and simplify the provide section:
file_test.go
package file
import (
"context"
"fmt"
"io/ioutil"
"os"
"path"
"testing"
"time"
"github.com/containous/traefik/safe"
"github.com/containous/traefik/types"
"github.com/stretchr/testify/assert"
)
func TestProvideSingleFileAndWatch(t *testing.T) {
tempDir := createTempDir(t, "testfile")
defer os.RemoveAll(tempDir)
expectedNumFrontends := 2
expectedNumBackends := 2
tempFile := createFile(t,
tempDir, "simple.toml",
createFrontendConfiguration(expectedNumFrontends),
createBackendConfiguration(expectedNumBackends))
configurationChan, signal := createConfigurationRoutine(t, &expectedNumFrontends, &expectedNumBackends)
provide(configurationChan, watch, withFile(tempFile))
// Wait for initial message to be tested
err := waitForSignal(signal, 2*time.Second, "initial config")
assert.NoError(t, err)
// Now test again with single frontend and backend
expectedNumFrontends = 1
expectedNumBackends = 1
tempFile = createFile(t,
tempDir, "simple.toml",
createFrontendConfiguration(expectedNumFrontends),
createBackendConfiguration(expectedNumBackends))
// Must fail because we don't watch the change
err = waitForSignal(signal, 2*time.Second, "single frontend and backend")
assert.NoError(t, err)
}
func TestProvideSingleFileAndNotWatch(t *testing.T) {
tempDir := createTempDir(t, "testfile")
defer os.RemoveAll(tempDir)
expectedNumFrontends := 2
expectedNumBackends := 2
tempFile := createFile(t,
tempDir, "simple.toml",
createFrontendConfiguration(expectedNumFrontends),
createBackendConfiguration(expectedNumBackends))
configurationChan, signal := createConfigurationRoutine(t, &expectedNumFrontends, &expectedNumBackends)
provide(configurationChan, withFile(tempFile))
// Wait for initial message to be tested
err := waitForSignal(signal, 2*time.Second, "initial config")
assert.NoError(t, err)
// Now test again with single frontend and backend
expectedNumFrontends = 1
expectedNumBackends = 1
tempFile = createFile(t,
tempDir, "simple.toml",
createFrontendConfiguration(expectedNumFrontends),
createBackendConfiguration(expectedNumBackends))
// Must fail because we don't watch the changes
err = waitForSignal(signal, 2*time.Second, "single frontend and backend")
assert.Error(t, err)
}
func TestProvideDirectoryAndWatch(t *testing.T) {
tempDir := createTempDir(t, "testdir")
defer os.RemoveAll(tempDir)
expectedNumFrontends := 2
expectedNumBackends := 2
tempFile1 := createRandomFile(t, tempDir, createFrontendConfiguration(expectedNumFrontends))
tempFile2 := createRandomFile(t, tempDir, createBackendConfiguration(expectedNumBackends))
configurationChan, signal := createConfigurationRoutine(t, &expectedNumFrontends, &expectedNumBackends)
provide(configurationChan, watch, withDirectory(tempDir))
// Wait for initial config message to be tested
err := waitForSignal(signal, 2*time.Second, "initial config")
assert.NoError(t, err)
// Now remove the backends file
expectedNumFrontends = 2
expectedNumBackends = 0
os.Remove(tempFile2.Name())
err = waitForSignal(signal, 2*time.Second, "remove the backends file")
assert.NoError(t, err)
// Now remove the frontends file
expectedNumFrontends = 0
expectedNumBackends = 0
os.Remove(tempFile1.Name())
err = waitForSignal(signal, 2*time.Second, "remove the frontends file")
assert.NoError(t, err)
}
func TestProvideDirectoryAndNotWatch(t *testing.T) {
tempDir := createTempDir(t, "testdir")
defer os.RemoveAll(tempDir)
expectedNumFrontends := 2
expectedNumBackends := 2
createRandomFile(t, tempDir, createFrontendConfiguration(expectedNumFrontends))
tempFile2 := createRandomFile(t, tempDir, createBackendConfiguration(expectedNumBackends))
configurationChan, signal := createConfigurationRoutine(t, &expectedNumFrontends, &expectedNumBackends)
provide(configurationChan, withDirectory(tempDir))
// Wait for initial config message to be tested
err := waitForSignal(signal, 2*time.Second, "initial config")
assert.NoError(t, err)
// Now remove the backends file
expectedNumFrontends = 2
expectedNumBackends = 0
os.Remove(tempFile2.Name())
// Must fail because we don't watch the changes
err = waitForSignal(signal, 2*time.Second, "remove the backends file")
assert.Error(t, err)
}
func createConfigurationRoutine(t *testing.T, expectedNumFrontends *int, expectedNumBackends *int) (chan types.ConfigMessage, chan interface{}) {
configurationChan := make(chan types.ConfigMessage)
signal := make(chan interface{})
go func() {
for {
data := <-configurationChan
assert.Equal(t, "file", data.ProviderName)
assert.Len(t, data.Configuration.Frontends, *expectedNumFrontends)
assert.Len(t, data.Configuration.Backends, *expectedNumBackends)
signal <- nil
}
}()
return configurationChan, signal
}
func waitForSignal(signal chan interface{}, timeout time.Duration, caseName string) error {
timer := time.NewTimer(timeout)
defer timer.Stop()
select {
case <-signal:
case <-timer.C:
return fmt.Errorf("Timed out waiting for assertions to be tested: %s", caseName)
}
return nil
}
func provide(configurationChan chan types.ConfigMessage, builders ...func(p *Provider)) {
pvd := &Provider{}
for _, builder := range builders {
builder(pvd)
}
pvd.Provide(configurationChan, safe.NewPool(context.Background()), nil)
}
func watch(pvd *Provider) {
pvd.Watch = true
}
func withDirectory(name string) func(*Provider) {
return func(pvd *Provider) {
pvd.Directory = name
}
}
func withFile(tempFile *os.File) func(*Provider) {
return func(p *Provider) {
p.Filename = tempFile.Name()
}
}
// createRandomFile Helper
func createRandomFile(t *testing.T, tempDir string, contents ...string) *os.File {
return createFile(t, tempDir, fmt.Sprintf("temp%d.toml", time.Now().UnixNano()), contents...)
}
// createFile Helper
func createFile(t *testing.T, tempDir string, name string, contents ...string) *os.File {
fileName := path.Join(tempDir, name)
tempFile, err := os.Create(fileName)
if err != nil {
t.Fatal(err)
}
for _, content := range contents {
_, err := tempFile.WriteString(content)
if err != nil {
t.Fatal(err)
}
}
err = tempFile.Close()
if err != nil {
t.Fatal(err)
}
return tempFile
}
// createTempDir Helper
func createTempDir(t *testing.T, dir string) string {
d, err := ioutil.TempDir("", dir)
if err != nil {
t.Fatal(err)
}
return d
}
// createFrontendConfiguration Helper
func createFrontendConfiguration(n int) string {
conf := "[frontends]\n"
for i := 1; i <= n; i++ {
conf += fmt.Sprintf(` [frontends.frontend%[1]d]
backend = "backend%[1]d"
`, i)
}
return conf
}
// createBackendConfiguration Helper
func createBackendConfiguration(n int) string {
conf := "[backends]\n"
for i := 1; i <= n; i++ {
conf += fmt.Sprintf(` [backends.backend%[1]d]
[backends.backend%[1]d.servers.server1]
url = "http://172.17.0.%[1]d:80"
`, i)
}
return conf
}
provider/file/file_test.go
Outdated
| provider.Provide(c, safe.NewPool(context.Background()), nil) | ||
|
|
||
| // Wait for initial message to be tested | ||
| wg.Wait() |
There was a problem hiding this comment.
We should define an upper bound after which we stop waiting for the asserting goroutine to complete. Otherwise, our test could lock up if we make a mistake or otherwise fail to send an event down the ConfigMessage channel.
The usual way to achieve this is to define a time.Timer of, say, 500 ms or so, and have the test t.Fail if it fires. If you also replace the wait group by a done channel that's being closed by the asserting goroutine as soon as it finishes, you can have a simple select statements over the timeout and done channels.
Similar for other tests below.
provider/file/file_test.go
Outdated
| func createFile(t *testing.T, file string) *os.File { | ||
| f, err := os.Create(file) | ||
| if err != nil { | ||
| t.Error(err) |
There was a problem hiding this comment.
Should likely be a t.Fatal.
provider/file/file_test.go
Outdated
| func createTempDir(t *testing.T, dir string) string { | ||
| d, err := ioutil.TempDir("", dir) | ||
| if err != nil { | ||
| t.Error(err) |
There was a problem hiding this comment.
Should likely be a t.Fatal.
provider/file/file_test.go
Outdated
| tempFile.WriteString(frontends + backends) | ||
| tempFile.Close() | ||
|
|
||
| provider := new(Provider) |
There was a problem hiding this comment.
How about using the &Provider{...} notation and inline the field settings from lines 107-108 directly?
provider/file/file.go
Outdated
| if p.Directory != "" { | ||
| watchDir = p.Directory | ||
| configuration, err = loadFileConfigFromDirectory(p.Directory) | ||
| } else { |
There was a problem hiding this comment.
If we have both a single filename and a directory defined, the directory wins. I wonder if that's desirable or if we should at least log a warning, if not let Traefik fail to bootstrap during a configuration sanity check. WDYT?
And whatever we decide, we should document how Traefik deals with this situation.
There was a problem hiding this comment.
Ideally, yes, we'd notify if both a single file and directory are specified. However, file is always specified, as it's then name of the main Traefik config file, even if the filename isn't set in the [file] config section. I didn't see a straightforward way to address this.
There was a problem hiding this comment.
True, that's a tricky. I'd just amend the documentation then by noting that the directory setting takes precedence.
|
Thanks, as always, for the comments @timoreimann. I've made the changes requested and look forward to @emilevauge 's views too. |
provider/file/file_test.go
Outdated
| for { | ||
| data := <-c | ||
| assert.Equal(t, "file", data.ProviderName) | ||
| assert.Equal(t, numFrontends, len(data.Configuration.Frontends)) |
There was a problem hiding this comment.
assert.Len(t, data.Configuration.Frontends, numFrontends)
assert.Len(t, data.Configuration.Backends, numBackends)
provider/file/file.go
Outdated
| var fileList []os.FileInfo | ||
| var err error | ||
|
|
||
| if fileList, err = ioutil.ReadDir(directory); err != nil { |
There was a problem hiding this comment.
you can simplify:
fileList, err := ioutil.ReadDir(directory)
if err != nil {
// ...
provider/file/file.go
Outdated
| } | ||
| return configuration | ||
|
|
||
| configuration := &types.Configuration{Frontends: make(map[string]*types.Frontend), |
There was a problem hiding this comment.
you can change the format to:
configuration := &types.Configuration{
Frontends: make(map[string]*types.Frontend),
Backends: make(map[string]*types.Backend),
}
provider/file/file.go
Outdated
| } | ||
|
|
||
| var c *types.Configuration | ||
| if c, err = loadFileConfig(path.Join(directory, file.Name())); err != nil { |
There was a problem hiding this comment.
this can be simplify:
c, err := loadFileConfig(path.Join(directory, file.Name()))
if err != nil {
// ...
provider/file/file.go
Outdated
| } else { | ||
| configuration.Frontends[k] = v | ||
| } | ||
|
|
provider/file/file.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| for k, v := range c.Backends { |
There was a problem hiding this comment.
maybe replace k, v by backendName, backend
provider/file/file.go
Outdated
| } | ||
| } | ||
|
|
||
| for k, v := range c.Frontends { |
There was a problem hiding this comment.
maybe replace k, v by frontendName, frontend
provider/file/file_test.go
Outdated
| data := <-c | ||
| assert.Equal(t, "file", data.ProviderName) | ||
| assert.Equal(t, numFrontends, len(data.Configuration.Frontends)) | ||
| assert.Equal(t, numBackends, len(data.Configuration.Backends)) |
There was a problem hiding this comment.
assert.Len(t, data.Configuration.Frontends, numFrontends)
assert.Len(t, data.Configuration.Backends, numBackends)
provider/file/file.go
Outdated
| var fileList []os.FileInfo | ||
| var err error | ||
|
|
||
| fileList, err = ioutil.ReadDir(directory) |
There was a problem hiding this comment.
you could remove:
var fileList []os.FileInfo
var err errorand change:
fileList, err = ioutil.ReadDir(directory)
// by
fileList, err := ioutil.ReadDir(directory)
provider/file/file.go
Outdated
| } else { | ||
| watchDir = filepath.Dir(p.Filename) | ||
| configuration, err = loadFileConfig(p.Filename) | ||
| } |
There was a problem hiding this comment.
I think you can replace this duplicate block (here and in the watcherCallback) by this method:
func (p *Provider) loadConfig() (*types.Configuration, error) {
if p.Directory != "" {
return loadFileConfigFromDirectory(p.Directory)
} else {
return loadFileConfig(p.Filename)
}
}
ldez
added
contributor/waiting-for-corrections
and removed
contributor/waiting-for-corrections
labels
provider/file/file.go
Outdated
| file, err := os.Open(p.Filename) | ||
| if err != nil { | ||
| log.Error("Error opening file", err) | ||
| if err := p.addWatcher(pool, watchDir, configurationChan, p.watcherCallback); err != nil { |
There was a problem hiding this comment.
you need to use p.Watch condition:
if p.Watch {
var watchDir string
if p.Directory != "" {
watchDir = p.Directory
} else {
watchDir = filepath.Dir(p.Filename)
}
if err := p.addWatcher(pool, watchDir, configurationChan, p.watcherCallback); err != nil {
return err
}
}
provider/file/file_test.go
Outdated
| ) | ||
|
|
||
| const ( | ||
| frontends = ` |
There was a problem hiding this comment.
I think, you can reduce test definitions like that:
file_test.go
package file
import (
"context"
"fmt"
"io/ioutil"
"os"
"path"
"testing"
"time"
"github.com/containous/traefik/provider"
"github.com/containous/traefik/safe"
"github.com/containous/traefik/types"
"github.com/stretchr/testify/assert"
)
func TestProvideSingleFile(t *testing.T) {
tempDir := createTempDir(t, "testfile")
defer os.RemoveAll(tempDir)
expectedNumFrontends := 2
expectedNumBackends := 2
tempFile := createFile(t,
tempDir, "simple.toml",
createFrontendConfiguration(expectedNumFrontends),
createBackendConfiguration(expectedNumBackends))
configurationChan, signal := createConfigurationRoutine(t, &expectedNumFrontends, &expectedNumBackends)
fileProvider := Provider{
BaseProvider: provider.BaseProvider{
Filename: tempFile.Name(),
Watch: true,
},
}
fileProvider.Provide(configurationChan, safe.NewPool(context.Background()), nil)
// Wait for initial message to be tested
waitForSignal(t, signal, 2*time.Second, "initial config")
// Now test again with single frontend and backend
expectedNumFrontends = 1
expectedNumBackends = 1
tempFile = createFile(t,
tempDir, "simple.toml",
createFrontendConfiguration(expectedNumFrontends),
createBackendConfiguration(expectedNumBackends))
waitForSignal(t, signal, 2*time.Second, "single frontend and backend")
}
func TestProvideDirectory(t *testing.T) {
tempDir := createTempDir(t, "testdir")
defer os.RemoveAll(tempDir)
expectedNumFrontends := 2
expectedNumBackends := 2
tempFile1 := createRandomFile(t, tempDir, createFrontendConfiguration(expectedNumFrontends))
tempFile2 := createRandomFile(t, tempDir, createBackendConfiguration(expectedNumBackends))
configurationChan, signal := createConfigurationRoutine(t, &expectedNumFrontends, &expectedNumBackends)
fileProvider := Provider{
BaseProvider: provider.BaseProvider{
Watch: true,
},
Directory: tempDir,
}
fileProvider.Provide(configurationChan, safe.NewPool(context.Background()), nil)
// Wait for initial config message to be tested
waitForSignal(t, signal, 2*time.Second, "initial config")
// Now remove the backends file
expectedNumFrontends = 2
expectedNumBackends = 0
os.Remove(tempFile2.Name())
waitForSignal(t, signal, 2*time.Second, "remove the backends file")
// Now remove the frontends file
expectedNumFrontends = 0
expectedNumBackends = 0
os.Remove(tempFile1.Name())
waitForSignal(t, signal, 2*time.Second, "remove the frontends file")
}
func createConfigurationRoutine(t *testing.T, expectedNumFrontends *int, expectedNumBackends *int) (chan types.ConfigMessage, chan interface{}) {
configurationChan := make(chan types.ConfigMessage)
signal := make(chan interface{})
go func() {
for {
data := <-configurationChan
assert.Equal(t, "file", data.ProviderName)
assert.Len(t, data.Configuration.Frontends, *expectedNumFrontends)
assert.Len(t, data.Configuration.Backends, *expectedNumBackends)
signal <- nil
}
}()
return configurationChan, signal
}
func waitForSignal(t *testing.T, signal chan interface{}, timeout time.Duration, caseName string) {
timer := time.NewTimer(timeout)
defer timer.Stop()
select {
case <-signal:
case <-timer.C:
t.Fatalf("Timed out waiting for assertions to be tested: %s", caseName)
}
}
// createRandomFile Helper
func createRandomFile(t *testing.T, tempDir string, contents ...string) *os.File {
return createFile(t, tempDir, fmt.Sprintf("temp%d.toml", time.Now().UnixNano()), contents...)
}
// createFile Helper
func createFile(t *testing.T, tempDir string, name string, contents ...string) *os.File {
fileName := path.Join(tempDir, name)
tempFile, err := os.Create(fileName)
if err != nil {
t.Fatal(err)
}
for _, content := range contents {
_, err := tempFile.WriteString(content)
if err != nil {
t.Fatal(err)
}
}
err = tempFile.Close()
if err != nil {
t.Fatal(err)
}
return tempFile
}
// createTempDir Helper
func createTempDir(t *testing.T, dir string) string {
d, err := ioutil.TempDir("", dir)
if err != nil {
t.Fatal(err)
}
return d
}
// createFrontendConfiguration Helper
func createFrontendConfiguration(n int) string {
conf := "[frontends]\n"
for i := 1; i <= n; i++ {
conf += fmt.Sprintf(` [frontends.frontend%[1]d]
backend = "backend%[1]d"
`, i)
}
return conf
}
// createBackendConfiguration Helper
func createBackendConfiguration(n int) string {
conf := "[backends]\n"
for i := 1; i <= n; i++ {
conf += fmt.Sprintf(` [backends.backend%[1]d]
[backends.backend%[1]d.servers.server1]
url = "http://172.17.0.%[1]d:80"
`, i)
}
return conf
}
provider/file/file.go
Outdated
| if p.Directory != "" { | ||
| watchDir = p.Directory | ||
| } else { | ||
| watchDir = filepath.Dir(p.Filename) |
There was a problem hiding this comment.
By adding watcher to the directory even if you just want to wach a file, you are going to watch all the directory events.
For example, if it exists 10 toml files into your directory, you are going to reload the configuration when all of these files will be modified.
That's why I suggest you to just add a watcher for the configuration file : watchDir = p.Filename and, maybe rename watchDir into watchFile (dir is a file ;) ) or watchItem...
There was a problem hiding this comment.
If you just want to watch a file, then you'd just specify the Filename and not use Directory at all. So that Watch applies only to the active setting Filename or Directory - they are mutually exclusive.
There was a problem hiding this comment.
@rjshep Of course, but when user specifies FileName, the watcher is added on its parent directory (filepath.Dir(p.Filename)). It was the mechanism before your PR.
However, before the modification the callback checked if the events received by the watcher were sent by the configuration file as described below :
case event := <-watcher.Events:
if strings.Contains(event.Name, file.Name())It would have been better to add directly the watcher on the file but, thanks to this code, the configuration was reloaded only when the Filename was modified.
But now, the configuration will be reloaded if one of all the toml file contained in the same directory than the Filename.
There will have no problem because only the Filename configuration will be reloaded but, it should be better to take advantage of you PR to improve the Filename watching by replacing watchDir = filepath.Dir(p.Filename) by watchDir = p.Filename.
Then, the configuration will only be reloaded when the Filename will change.
There was a problem hiding this comment.
Sorry @nmengin, completely missed your original point. Yes that is a sensible change to make.
| [entryPoints.http] | ||
| address = ":8000" | ||
|
|
||
| logLevel = "DEBUG" |
There was a problem hiding this comment.
The logLevel should to be defined before the entrypoints otherwise the logger will not work correctly.
|
@rjshep could you squash and rebase (for semaphoreCI)? |
Currently the file provider reads service config either from the global config, or from a single file specified in global config.
In some deployments it would be nice to load from multiple files - perhaps because each service is defined in a single file and is maintained separately from the others. This PR extends the file provider to allow this.
Also added a unit test for the file provider and fixed bug in integration test which didn't check err response of GetRequest.