From 5353448a92fff98efbf6c853e8855f18b0ad12c4 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Wed, 5 Apr 2017 16:09:43 +0300 Subject: [PATCH 1/4] Adds log-path config --- cmd/server.go | 1 + cmd/server_test.go | 20 ++++++++++++++++++++ config.go | 2 ++ server/server.go | 16 +++++++++++++++- 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/cmd/server.go b/cmd/server.go index 5d5035ba7..20de85a3f 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -79,6 +79,7 @@ on the configured port.`, flags.StringSliceVarP(&Server.Config.Cluster.Nodes, "cluster.hosts", "", []string{}, "Comma separated list of hosts in cluster.") flags.DurationVarP((*time.Duration)(&Server.Config.Cluster.PollingInterval), "cluster.poll-interval", "", time.Minute, "Polling interval for cluster.") // TODO what actually is this? flags.StringVarP(&Server.Config.Plugins.Path, "plugins.path", "", "", "Path to plugin directory.") + flags.StringVar(&Server.Config.LogPath, "log-path", "", "Log path") flags.DurationVarP((*time.Duration)(&Server.Config.AntiEntropy.Interval), "anti-entropy.interval", "", time.Minute*10, "Interval at which to run anti-entropy routine.") flags.StringVarP(&Server.CPUProfile, "profile.cpu", "", "", "Where to store CPU profile.") flags.DurationVarP(&Server.CPUTime, "profile.cpu-time", "", 30*time.Second, "CPU profile duration.") diff --git a/cmd/server_test.go b/cmd/server_test.go index d677d67a8..2e5aa4ad1 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -23,6 +23,8 @@ func TestServerConfig(t *testing.T) { failErr(t, err, "making data dir") profFile, err := ioutil.TempFile("", "") failErr(t, err, "making temp file") + logFile, err := ioutil.TempFile("", "") + failErr(t, err, "making log file") tests := []commandTest{ // TEST 0 { @@ -99,6 +101,24 @@ data-dir = "` + actualDataDir + `" return v.Error() }, }, + // TEST 3 - test log path can be read from command line and it's priority is highest + { + args: []string{"server", "--log-path", logFile.Name()}, + env: map[string]string{"PILOSA_LOG_PATH": "/tmp/mylog_env"}, + cfgFileContent: ` +log-path = "/tmp/mylog_cfg" +`, + validation: func() error { + v := validator{} + v.Check(cmd.Server.Config.LogPath, logFile.Name()) + if err := v.Error(); err != nil { + return v.Error() + } + // check that the log file was created + _, err = logFile.Stat() + return err + }, + }, } // run server tests diff --git a/config.go b/config.go index e664c0771..8250c7f91 100644 --- a/config.go +++ b/config.go @@ -26,6 +26,8 @@ type Config struct { AntiEntropy struct { Interval Duration `toml:"interval"` } `toml:"anti-entropy"` + + LogPath string `toml:"log-path"` } // NewConfig returns an instance of Config with default options. diff --git a/server/server.go b/server/server.go index cd9aa3cb0..5d713e60b 100644 --- a/server/server.go +++ b/server/server.go @@ -68,7 +68,15 @@ func (m *Command) Run(args ...string) (err error) { } // Setup logging output. - m.Server.LogOutput = m.Stderr + if m.Config.LogPath == "" { + m.Server.LogOutput = m.Stderr + } else { + logFile, err := os.OpenFile(m.Config.LogPath, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0600) + if err != nil { + return err + } + m.Server.LogOutput = logFile + } // Configure index. fmt.Fprintf(m.Stderr, "Using data from: %s\n", m.Config.DataDir) @@ -109,6 +117,12 @@ func normalizeHost(host string) (string, error) { // Close shuts down the server. func (m *Command) Close() error { err := m.Server.Close() + logOutput := m.Server.LogOutput + if logOutput != m.Stderr { + if file, ok := logOutput.(*os.File); ok { + file.Close() + } + } close(m.Done) return err } From 63aadd9b5ea9206d44d49a7027b0237b62da17d5 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Wed, 5 Apr 2017 18:26:55 +0300 Subject: [PATCH 2/4] Simplified log path test --- cmd/server_test.go | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/cmd/server_test.go b/cmd/server_test.go index 2e5aa4ad1..186b5a51c 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -75,7 +75,7 @@ data-dir = "` + actualDataDir + `" }, // TEST 2 { - args: []string{"server"}, + args: []string{"server", "--log-path", logFile.Name()}, env: map[string]string{"PILOSA_PROFILE.CPU_TIME": "1m"}, cfgFileContent: ` bind = "localhost:0" @@ -98,25 +98,8 @@ data-dir = "` + actualDataDir + `" v.Check(cmd.Server.Config.AntiEntropy.Interval, pilosa.Duration(time.Minute*11)) v.Check(cmd.Server.CPUProfile, profFile.Name()) v.Check(cmd.Server.CPUTime, time.Minute) - return v.Error() - }, - }, - // TEST 3 - test log path can be read from command line and it's priority is highest - { - args: []string{"server", "--log-path", logFile.Name()}, - env: map[string]string{"PILOSA_LOG_PATH": "/tmp/mylog_env"}, - cfgFileContent: ` -log-path = "/tmp/mylog_cfg" -`, - validation: func() error { - v := validator{} v.Check(cmd.Server.Config.LogPath, logFile.Name()) - if err := v.Error(); err != nil { - return v.Error() - } - // check that the log file was created - _, err = logFile.Stat() - return err + return v.Error() }, }, } From 2a87c6783abc8f179e3776c758c04151ae067ab8 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Wed, 5 Apr 2017 19:10:41 +0300 Subject: [PATCH 3/4] Check log file size > 0 --- cmd/server_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/cmd/server_test.go b/cmd/server_test.go index 186b5a51c..927ce8b66 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -1,6 +1,7 @@ package cmd_test import ( + "errors" "io/ioutil" "strings" "testing" @@ -99,7 +100,15 @@ data-dir = "` + actualDataDir + `" v.Check(cmd.Server.CPUProfile, profFile.Name()) v.Check(cmd.Server.CPUTime, time.Minute) v.Check(cmd.Server.Config.LogPath, logFile.Name()) - return v.Error() + if v.Error() != nil { + return v.Error() + } + // confirm log file was written + info, err := logFile.Stat() + if err != nil || info.Size() == 0 { + return errors.New("Log file was not written!") + } + return nil }, }, } From 6c7b1b513af3bcde7d68c25f196d1b9d93e992d2 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Wed, 5 Apr 2017 21:35:30 +0300 Subject: [PATCH 4/4] return log err too --- server/server.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/server/server.go b/server/server.go index 5d713e60b..469b5bc3b 100644 --- a/server/server.go +++ b/server/server.go @@ -116,13 +116,17 @@ func normalizeHost(host string) (string, error) { // Close shuts down the server. func (m *Command) Close() error { - err := m.Server.Close() + var logErr error + serveErr := m.Server.Close() logOutput := m.Server.LogOutput - if logOutput != m.Stderr { - if file, ok := logOutput.(*os.File); ok { - file.Close() - } + if closer, ok := logOutput.(io.Closer); ok { + logErr = closer.Close() } close(m.Done) - return err + if serveErr != nil && logErr != nil { + return fmt.Errorf("closing server: '%v', closing logs: '%v'", serveErr, logErr) + } else if logErr != nil { + return logErr + } + return serveErr }