From 0835aa82c8ad0985569abe6d1b27f5bd2b72a48e Mon Sep 17 00:00:00 2001 From: Yury Yantsevich Date: Thu, 18 Jun 2015 20:49:15 +0200 Subject: [PATCH 1/5] adds validation warning to config. exit with error when supplied no files to watch --- config_manager.go | 12 ++++++++++++ remote_syslog.go | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/config_manager.go b/config_manager.go index f87d1fb..0181c2a 100644 --- a/config_manager.go +++ b/config_manager.go @@ -35,6 +35,16 @@ type ConfigFile struct { ExcludePatterns *RegexCollection `yaml:"exclude_patterns"` } +func (cf ConfigFile) printValidationWarnings() { + if len(cf.Files) == 0 { + log.Warningf("Configuration file contains no files to watch") + } + // Warn, that port is not set, only if host is specified + if cf.Destination.Host != "" && cf.Destination.Port == 0 { + log.Warningf("Destination port is not specified in configuration file") + } +} + type ConfigManager struct { Config ConfigFile FlagFiles []string @@ -199,6 +209,8 @@ func (cm *ConfigManager) loadConfigFile() error { if err != nil { return fmt.Errorf("Could not parse the config file: %s", err) } + cm.Config.printValidationWarnings() + return nil } diff --git a/remote_syslog.go b/remote_syslog.go index 9bd5d89..eea5ea9 100644 --- a/remote_syslog.go +++ b/remote_syslog.go @@ -53,6 +53,11 @@ func tailOne(file string, excludePatterns []*regexp.Regexp, logger *syslog.Logge // Tails files speficied in the globs and re-evaluates the globs // at the specified interval func tailFiles(globs []string, excludedFiles []*regexp.Regexp, excludePatterns []*regexp.Regexp, interval RefreshInterval, logger *syslog.Logger, severity syslog.Priority, facility syslog.Priority, poll bool) { + if len(globs) == 0 { + log.Criticalf("Supplied no files to watch") + os.Exit(1) + } + wr := NewWorkerRegistry() log.Debugf("Evaluating globs every %s", interval.Duration) logMissingFiles := true From 39160874aafdc328bf39c53373a311d93d759e94 Mon Sep 17 00:00:00 2001 From: Yury Yantsevich Date: Fri, 19 Jun 2015 23:04:20 +0200 Subject: [PATCH 2/5] update VividCortex/godaemon dependency --- Godeps/Godeps.json | 2 +- .../github.com/VividCortex/godaemon/README.md | 12 + .../github.com/VividCortex/godaemon/daemon.go | 286 ++++++++++++++---- .../VividCortex/godaemon/daemon_linux.go | 3 +- .../src/github.com/VividCortex/godaemon/os.go | 38 +++ 5 files changed, 283 insertions(+), 58 deletions(-) create mode 100644 Godeps/_workspace/src/github.com/VividCortex/godaemon/os.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index e544579..8ca7a2d 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -8,7 +8,7 @@ }, { "ImportPath": "github.com/VividCortex/godaemon", - "Rev": "2fdf3f9fa715a998e834f09e07a8070d9046bcfd" + "Rev": "560bd39278f31c0ec39d52beadad9558dcb08ba2" }, { "ImportPath": "github.com/howbazaar/loggo", diff --git a/Godeps/_workspace/src/github.com/VividCortex/godaemon/README.md b/Godeps/_workspace/src/github.com/VividCortex/godaemon/README.md index 268fc56..4456ea4 100644 --- a/Godeps/_workspace/src/github.com/VividCortex/godaemon/README.md +++ b/Godeps/_workspace/src/github.com/VividCortex/godaemon/README.md @@ -43,6 +43,18 @@ two valid readers (`io.Reader`) that you can read from the program itself. That's particularly useful for functions that write error or diagnosis messages right to the error output, which are normally lost in a daemon. +Use the `Files` attribute if you need to inherit open files into the daemon. +This is primarily intended for avoiding race conditions when holding locks on +those files (flocks). Releasing and re-acquiring locks between successive fork +calls opens up the chance for another program to steal the lock. However, by +declaring your file descriptors in the `Files` attribute, `MakeDaemon()` will +guarantee that locks are not released throughout the whole process. Your daemon +will inherit the file still holding the same locks, with no other process having +intervened in between. See the +[package documentation](http://godoc.org/github.com/VividCortex/godaemon) for +more details and sample code. (Note that you shouldn't use this feature to +inherit TTY descriptors; otherwise what you get is technically not a daemon.) + ## Contribute diff --git a/Godeps/_workspace/src/github.com/VividCortex/godaemon/daemon.go b/Godeps/_workspace/src/github.com/VividCortex/godaemon/daemon.go index 1d05d8a..ce52002 100644 --- a/Godeps/_workspace/src/github.com/VividCortex/godaemon/daemon.go +++ b/Godeps/_workspace/src/github.com/VividCortex/godaemon/daemon.go @@ -5,6 +5,7 @@ package godaemon // Please see the LICENSE file for applicable license terms. import ( + "bytes" "crypto/sha1" "encoding/hex" "fmt" @@ -16,12 +17,17 @@ import ( "time" ) -// The name of the env var that indicates what stage of daemonization we're at. -const stageVar = "__DAEMON_STAGE" +// Environment variables to support this process +const ( + stageVar = "__DAEMON_STAGE" + fdVarPrefix = "__DAEMON_FD_" +) // DaemonAttr describes the options that apply to daemonization type DaemonAttr struct { - CaptureOutput bool // whether to capture stdout/stderr + ProgramName string // child's os.Args[0]; copied from parent if empty + CaptureOutput bool // whether to capture stdout/stderr + Files []**os.File // files to keep open in the daemon } /* @@ -43,6 +49,56 @@ that this function takes no action whatsoever on any of the streams.) NOTE: If you use them, make sure NOT to take one of these readers and write the data back again to standard output/error, or you'll end up with a loop. +Also, note that data will be flushed on a line-by-line basis; i.e., partial +lines will be buffered until an end-of-line is seen. + +By using the Files member of DaemonAttr you can inherit open files that will +still be open once the program is running as a daemon. This may be convenient in +general, but it's primarily intended to avoid race conditions while forking, in +case a lock (flock) was held on that file. Repeatedly releasing and re-locking +while forking is subject to race conditions, cause a different process could +lock the file in between. But locks held on files declared at DaemonAttr.Files +are guaranteed NOT to be released during the whole process, and still be held by +the daemon. To use this feature you should open the file(s), lock if required +and then call MakeDaemon using pointers to that *os.File objects; i.e., you'd be +passing **os.File objects to MakeDaemon(). However, opening the files (and +locking if required) should only be attempted at the parent. (Recall that +MakeDaemon() will run the code coming "before" it three times; see the +explanation above.) You can filter that by calling Stage() and looking for a +godaemon.StageParent result. The last call to MakeDaemon() at the daemon itself +will actually *load* the *os.File objects for you; that's why you need to +provide a pointer to them. So here's how you'd use it: + + var ( + f *os.File + err error + ) + + if godaemon.Stage() == godaemon.StageParent { + f, err = os.OpenFile(name, opts, perm) + if err != nil { + os.Exit(1) + } + err = syscall.Flock(int(f.Fd()), syscall.LOCK_EX) + if err != nil { + os.Exit(1) + } + } + + _, _, err = godaemon.MakeDaemon(&godaemon.DaemonAttr{ + Files: []**os.File{&f}, + }) + + // Only the daemon will reach this point, where f will be a valid descriptor + // pointing to your file "name", still holding the lock (which will have + // never been released during successive forks). You can operate on f as you + // normally would, like: + f.Close() + +NOTE: Do not abuse this feature. Even though you could, it's obviously not a +good idea to use this mechanism to keep a terminal device open, for instance. +Otherwise, what you get is not strictly a daemon. + Daemonizing is a 3-stage process. In stage 0, the program increments the magical environment variable and starts a copy of itself that's a session @@ -58,74 +114,99 @@ and reestablishes the original value for the environment variable. func MakeDaemon(attrs *DaemonAttr) (io.Reader, io.Reader, error) { stage, advanceStage, resetEnv := getStage() - // getExecutablePath() is OS-specific. - procName, err := GetExecutablePath() - - if err != nil { - err = fmt.Errorf("can't determine full path to executable: %v", err) + // This is a handy wrapper to do the proper thing in case of fatal + // conditions. For the first stage you may want to recover, so it will + // return the error. Otherwise it will exit the process, cause you'll be + // half-way with some descriptors already changed. There's no chance to + // write to stdout or stderr in the later case; they'll be already closed. + fatal := func(err error) (io.Reader, io.Reader, error) { + if stage > 0 { + os.Exit(1) + } + resetEnv() return nil, nil, err } - // If getExecutablePath() returns "" but no error, determinating the - // executable path is not implemented on the host OS, so daemonization - // is not supported. - if len(procName) == 0 { - err = fmt.Errorf("can't determine full path to executable") - return nil, nil, err - } + fileCount := 3 + len(attrs.Files) + files := make([]*os.File, fileCount, fileCount+2) - if stage == 0 || stage == 1 { + if stage == 0 { // Descriptors 0, 1 and 2 are fixed in the "os" package. If we close // them, the process may choose to open something else there, with bad // consequences if some write to os.Stdout or os.Stderr follows (even // from Go's library itself, through the default log package). We thus // reserve these descriptors to avoid that. nullDev, err := os.OpenFile("/dev/null", 0, 0) + if err != nil { + return fatal(err) + } + files[0], files[1], files[2] = nullDev, nullDev, nullDev + + fd := 3 + for _, fPtr := range attrs.Files { + files[fd] = *fPtr + saveFileName(fd, (*fPtr).Name()) + fd++ + } + } else { + files[0], files[1], files[2] = os.Stdin, os.Stdout, os.Stderr + + fd := 3 + for _, fPtr := range attrs.Files { + *fPtr = os.NewFile(uintptr(fd), getFileName(fd)) + syscall.CloseOnExec(fd) + files[fd] = *fPtr + fd++ + } + } + if stage < 2 { + // getExecutablePath() is OS-specific. + procName, err := GetExecutablePath() if err != nil { - fmt.Fprintf(os.Stderr, "unable to open /dev/null: ", err, "\n") - os.Exit(1) + return fatal(fmt.Errorf("can't determine full path to executable: %s", err)) } - files := make([]*os.File, 3, 5) - files[0] = nullDev // stdin - // FDs should not be changed; we're using literals (as opposed to - // constants) on purpose to discourage such a practice. + // If getExecutablePath() returns "" but no error, determinating the + // executable path is not implemented on the host OS, so daemonization + // is not supported. + if len(procName) == 0 { + return fatal(fmt.Errorf("can't determine full path to executable")) + } if stage == 1 && attrs.CaptureOutput { - files = files[0:5] + files = files[:fileCount+2] - // stdout: write at fd:1, read at fd:3 - if files[3], files[1], err = os.Pipe(); err != nil { - fmt.Fprintf(os.Stderr, "unable to create stdout pipe: ", err, "\n") - os.Exit(1) + // stdout: write at fd:1, read at fd:fileCount + if files[fileCount], files[1], err = os.Pipe(); err != nil { + return fatal(err) } - - // stderr: write at fd:2, read at fd:4 - if files[4], files[2], err = os.Pipe(); err != nil { - fmt.Fprintf(os.Stderr, "unable to create stderr pipe: ", err, "\n") - os.Exit(1) + // stderr: write at fd:2, read at fd:fileCount+1 + if files[fileCount+1], files[2], err = os.Pipe(); err != nil { + return fatal(err) } - } else { - files[1], files[2] = nullDev, nullDev } - advanceStage() + if err := advanceStage(); err != nil { + return fatal(err) + } dir, _ := os.Getwd() - attrs := os.ProcAttr{Dir: dir, Env: os.Environ(), Files: files} + osAttrs := os.ProcAttr{Dir: dir, Env: os.Environ(), Files: files} if stage == 0 { sysattrs := syscall.SysProcAttr{Setsid: true} - attrs.Sys = &sysattrs + osAttrs.Sys = &sysattrs } - proc, err := os.StartProcess(procName, os.Args, &attrs) - + progName := attrs.ProgramName + if len(progName) == 0 { + progName = os.Args[0] + } + args := append([]string{progName}, os.Args[1:]...) + proc, err := os.StartProcess(procName, args, &osAttrs) if err != nil { - fmt.Fprintf(os.Stderr, "can't create process %s\n", procName) - os.Exit(1) + return fatal(fmt.Errorf("can't create process %s: %s", procName, err)) } - proc.Release() os.Exit(0) } @@ -134,14 +215,66 @@ func MakeDaemon(attrs *DaemonAttr) (io.Reader, io.Reader, error) { syscall.Umask(0) resetEnv() + for fd := 3; fd < fileCount; fd++ { + resetFileName(fd) + } + currStage = DaemonStage(stage) + var stdout, stderr *os.File if attrs.CaptureOutput { - stdout = os.NewFile(uintptr(3), "stdout") - stderr = os.NewFile(uintptr(4), "stderr") + stdout = os.NewFile(uintptr(fileCount), "stdout") + stderr = os.NewFile(uintptr(fileCount+1), "stderr") } return stdout, stderr, nil } +func saveFileName(fd int, name string) { + // We encode in hex to avoid issues with filename encoding, and to be able + // to separate it from the original variable value (if set) that we want to + // keep. Otherwise, all non-zero characters are valid in the name, and we + // can't insert a zero in the var as a separator. + fdVar := fdVarPrefix + fmt.Sprint(fd) + value := fmt.Sprintf("%s:%s", + hex.EncodeToString([]byte(name)), os.Getenv(fdVar)) + + if err := os.Setenv(fdVar, value); err != nil { + fmt.Fprintf(os.Stderr, "can't set %s: %s\n", fdVar, err) + os.Exit(1) + } +} + +func getFileName(fd int) string { + fdVar := fdVarPrefix + fmt.Sprint(fd) + value := os.Getenv(fdVar) + sep := bytes.IndexByte([]byte(value), ':') + + if sep < 0 { + fmt.Fprintf(os.Stderr, "bad fd var %s\n", fdVar) + os.Exit(1) + } + name, err := hex.DecodeString(value[:sep]) + if err != nil { + fmt.Fprintf(os.Stderr, "error decoding %s\n", fdVar) + os.Exit(1) + } + return string(name) +} + +func resetFileName(fd int) { + fdVar := fdVarPrefix + fmt.Sprint(fd) + value := os.Getenv(fdVar) + sep := bytes.IndexByte([]byte(value), ':') + + if sep < 0 { + fmt.Fprintf(os.Stderr, "bad fd var %s\n", fdVar) + os.Exit(1) + } + if err := os.Setenv(fdVar, value[sep+1:]); err != nil { + fmt.Fprintf(os.Stderr, "can't reset %s\n", fdVar) + os.Exit(1) + } +} + // Daemonize is equivalent to MakeDaemon(&DaemonAttr{}). It is kept only for // backwards API compatibility, but it's usage is otherwise discouraged. Use // MakeDaemon() instead. The child parameter, previously used to tell whether @@ -151,12 +284,60 @@ func Daemonize(child ...bool) { MakeDaemon(&DaemonAttr{}) } +// DaemonStage tells in what stage in the process we are. See Stage(). +type DaemonStage int + +// Stages in the daemonizing process. +const ( + StageParent = DaemonStage(iota) // Original process + StageChild // MakeDaemon() called once: first child + StageDaemon // MakeDaemon() run twice: final daemon + + stageUnknown = DaemonStage(-1) +) + +// currStage keeps the current stage. This is used only as a cache for Stage(), +// in order to extend a valid result after MakeDaemon() has returned, where the +// environment variable would have already been reset. (Also, this is faster +// than repetitive calls to getStage().) Note that this approach is valid cause +// the stage doesn't change throughout any single process execution. It does +// only for the next process after the MakeDaemon() call. +var currStage = stageUnknown + +// Stage returns the "stage of daemonizing", i.e., it allows you to know whether +// you're currently working in the parent, first child, or the final daemon. +// This is useless after the call to MakeDaemon(), cause that call will only +// return for the daemon stage. However, you can still use Stage() to tell +// whether you've daemonized or not, in case you have a running path that may +// exclude the call to MakeDaemon(). +func Stage() DaemonStage { + if currStage == stageUnknown { + s, _, _ := getStage() + currStage = DaemonStage(s) + } + return currStage +} + +// String returns a humanly readable daemonization stage. +func (s DaemonStage) String() string { + switch s { + case StageParent: + return "parent" + case StageChild: + return "first child" + case StageDaemon: + return "daemon" + default: + return "unknown" + } +} + // Returns the current stage in the "daemonization process", that's kept in // an environment variable. The variable is instrumented with a digital // signature, to avoid misbehavior if it was present in the user's // environment. The original value is restored after the last stage, so that // there's no final effect on the environment the application receives. -func getStage() (stage int, advanceStage func(), resetEnv func()) { +func getStage() (stage int, advanceStage func() error, resetEnv func() error) { var origValue string stage = 0 @@ -184,24 +365,19 @@ func getStage() (stage int, advanceStage func(), resetEnv func()) { origValue = daemonStage } - advanceStage = func() { + advanceStage = func() error { base := fmt.Sprintf("%d/%09d/", stage+1, time.Now().Nanosecond()) hash := sha1.New() hash.Write([]byte(base)) - tag := base + hex.EncodeToString(hash.Sum([]byte{})) if err := os.Setenv(stageVar, tag+":"+origValue); err != nil { - fmt.Fprintf(os.Stderr, "can't set %s (stage %d)\n", stageVar, stage) - os.Exit(1) + return fmt.Errorf("can't set %s: %s", stageVar, err) } + return nil } - - resetEnv = func() { - if err := os.Setenv(stageVar, origValue); err != nil { - fmt.Fprintf(os.Stderr, "can't reset %s\n", stageVar) - os.Exit(1) - } + resetEnv = func() error { + return os.Setenv(stageVar, origValue) } return stage, advanceStage, resetEnv diff --git a/Godeps/_workspace/src/github.com/VividCortex/godaemon/daemon_linux.go b/Godeps/_workspace/src/github.com/VividCortex/godaemon/daemon_linux.go index 6f1c3a7..728f099 100644 --- a/Godeps/_workspace/src/github.com/VividCortex/godaemon/daemon_linux.go +++ b/Godeps/_workspace/src/github.com/VividCortex/godaemon/daemon_linux.go @@ -5,7 +5,6 @@ package godaemon import ( "fmt" - "os" "path/filepath" ) @@ -13,7 +12,7 @@ import ( // executable. It is used internally by the godaemon package, and exported // publicly because it's useful outside of the package too. func GetExecutablePath() (string, error) { - exePath, err := os.Readlink("/proc/self/exe") + exePath, err := Readlink("/proc/self/exe") if err != nil { err = fmt.Errorf("can't read /proc/self/exe: %v", err) diff --git a/Godeps/_workspace/src/github.com/VividCortex/godaemon/os.go b/Godeps/_workspace/src/github.com/VividCortex/godaemon/os.go new file mode 100644 index 0000000..92deb37 --- /dev/null +++ b/Godeps/_workspace/src/github.com/VividCortex/godaemon/os.go @@ -0,0 +1,38 @@ +package godaemon + +import ( + "bytes" + "os" + "syscall" +) + +// Readlink returns the file pointed to by the given soft link, or an error of +// type PathError otherwise. This mimics the os.Readlink() function, but works +// around a bug we've seen in CentOS 5.10 (kernel 2.6.27.10 on x86_64) where the +// underlying OS function readlink() returns a wrong number of bytes for the +// result (see man readlink). Here we don't rely blindly on that value; if +// there's a zero byte among that number of bytes, then we keep only up to that +// point. +// +// NOTE: We chose not to use os.Readlink() and then search on its result to +// avoid an extra overhead of converting back to []byte. The function to search +// for a byte over the string itself (strings.IndexByte()) is only available +// starting with Go 1.2. Also, we're not searching at every iteration to save +// some CPU time, even though that could mean extra iterations for systems +// affected with this bug. But it's wiser to optimize for the general case +// (i.e., those not affected). +func Readlink(name string) (string, error) { + for len := 128; ; len *= 2 { + b := make([]byte, len) + n, e := syscall.Readlink(name, b) + if e != nil { + return "", &os.PathError{"readlink", name, e} + } + if n < len { + if z := bytes.IndexByte(b[:n], 0); z >= 0 { + n = z + } + return string(b[:n]), nil + } + } +} From f79b47aaa0c98da272b75b18098ef208ade4bad9 Mon Sep 17 00:00:00 2001 From: Yury Yantsevich Date: Fri, 19 Jun 2015 23:05:50 +0200 Subject: [PATCH 3/5] print validation warnings before daemonization; add more warnings to configuration manager --- config_manager.go | 25 ++++++++++++++++++++----- remote_syslog.go | 6 +++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/config_manager.go b/config_manager.go index 0181c2a..2621790 100644 --- a/config_manager.go +++ b/config_manager.go @@ -21,6 +21,8 @@ const ( DefaultConfigFile = "/etc/log_files.yml" ) +var defaultConfigNotExists = fmt.Errorf("Default configuration file '%s' does not exist", DefaultConfigFile) + type ConfigFile struct { Files []string Destination struct { @@ -37,11 +39,11 @@ type ConfigFile struct { func (cf ConfigFile) printValidationWarnings() { if len(cf.Files) == 0 { - log.Warningf("Configuration file contains no files to watch") + log.Warningf("Validation: configuration file contains no files to watch") } // Warn, that port is not set, only if host is specified if cf.Destination.Host != "" && cf.Destination.Port == 0 { - log.Warningf("Destination port is not specified in configuration file") + log.Warningf("Validation: destination port is not specified in configuration file") } } @@ -189,7 +191,12 @@ func (cm *ConfigManager) readConfig() error { log.Infof("Reading configuration file %s", cm.Flags.ConfigFile) err := cm.loadConfigFile() if err != nil { - log.Errorf("%s", err) + if err == defaultConfigNotExists { + log.Warningf(err.Error()) + err = nil + } else { + log.Errorf(err.Error()) + } return err } return nil @@ -199,7 +206,7 @@ func (cm *ConfigManager) loadConfigFile() error { file, err := ioutil.ReadFile(cm.Flags.ConfigFile) // don't error if the default config file isn't found if os.IsNotExist(err) && cm.Flags.ConfigFile == DefaultConfigFile { - return nil + return defaultConfigNotExists } if err != nil { return fmt.Errorf("Could not read the config file: %s", err) @@ -209,7 +216,6 @@ func (cm *ConfigManager) loadConfigFile() error { if err != nil { return fmt.Errorf("Could not parse the config file: %s", err) } - cm.Config.printValidationWarnings() return nil } @@ -365,3 +371,12 @@ func (cm *ConfigManager) ExcludeFiles() []*regexp.Regexp { func (cm *ConfigManager) ExcludePatterns() []*regexp.Regexp { return *cm.Config.ExcludePatterns } + +func (cm *ConfigManager) printValidationWarnings() { + cm.Config.printValidationWarnings() + for _, fileToWatch := range cm.Files() { + if _, err := os.Stat(fileToWatch); os.IsNotExist(err) { + log.Warningf("Validation: file does not exist: %s", fileToWatch) + } + } +} diff --git a/remote_syslog.go b/remote_syslog.go index eea5ea9..919f1d9 100644 --- a/remote_syslog.go +++ b/remote_syslog.go @@ -10,6 +10,7 @@ import ( "time" "github.com/ActiveState/tail" + "github.com/VividCortex/godaemon" "github.com/howbazaar/loggo" "github.com/papertrail/remote_syslog2/syslog" "github.com/papertrail/remote_syslog2/utils" @@ -21,7 +22,7 @@ var log = loggo.GetLogger("") func tailOne(file string, excludePatterns []*regexp.Regexp, logger *syslog.Logger, wr *WorkerRegistry, severity syslog.Priority, facility syslog.Priority, poll bool) { defer wr.Remove(file) wr.Add(file) - tailConfig := tail.Config{ReOpen: true, Follow: true, MustExist: true, Poll: poll, Location: &tail.SeekInfo{0, os.SEEK_END}} + tailConfig := tail.Config{ReOpen: true, Follow: true, MustExist: true, Poll: poll, Location: &tail.SeekInfo{0, os.SEEK_END}} t, err := tail.TailFile(file, tailConfig) @@ -108,6 +109,9 @@ func matchExps(value string, expressions []*regexp.Regexp) bool { func main() { cm := NewConfigManager() + if !cm.Daemonize() || godaemon.Stage() == godaemon.StageParent { + cm.printValidationWarnings() + } if cm.Daemonize() { utils.Daemonize(cm.DebugLogFile(), cm.PidFile()) From 86f18aa7cf816222501da23416e19440cdffd32c Mon Sep 17 00:00:00 2001 From: Yury Yantsevich Date: Wed, 24 Jun 2015 02:35:04 +0200 Subject: [PATCH 4/5] change var name --- config_manager.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config_manager.go b/config_manager.go index 2621790..590ff1a 100644 --- a/config_manager.go +++ b/config_manager.go @@ -21,7 +21,7 @@ const ( DefaultConfigFile = "/etc/log_files.yml" ) -var defaultConfigNotExists = fmt.Errorf("Default configuration file '%s' does not exist", DefaultConfigFile) +var defaultConfigDoesNotExist = fmt.Errorf("Default configuration file '%s' does not exist", DefaultConfigFile) type ConfigFile struct { Files []string @@ -191,7 +191,7 @@ func (cm *ConfigManager) readConfig() error { log.Infof("Reading configuration file %s", cm.Flags.ConfigFile) err := cm.loadConfigFile() if err != nil { - if err == defaultConfigNotExists { + if err == defaultConfigDoesNotExist { log.Warningf(err.Error()) err = nil } else { @@ -206,7 +206,7 @@ func (cm *ConfigManager) loadConfigFile() error { file, err := ioutil.ReadFile(cm.Flags.ConfigFile) // don't error if the default config file isn't found if os.IsNotExist(err) && cm.Flags.ConfigFile == DefaultConfigFile { - return defaultConfigNotExists + return defaultConfigDoesNotExist } if err != nil { return fmt.Errorf("Could not read the config file: %s", err) From 7c67e297d42270fda2e3aa8bcd290e510288a8a8 Mon Sep 17 00:00:00 2001 From: Yury Yantsevich Date: Thu, 2 Jul 2015 13:04:54 +0200 Subject: [PATCH 5/5] constantise magic number (default port) --- config_manager.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config_manager.go b/config_manager.go index 590ff1a..332d959 100644 --- a/config_manager.go +++ b/config_manager.go @@ -19,6 +19,7 @@ import ( const ( MinimumRefreshInterval = (time.Duration(10) * time.Second) DefaultConfigFile = "/etc/log_files.yml" + DefaultPort = 514 ) var defaultConfigDoesNotExist = fmt.Errorf("Default configuration file '%s' does not exist", DefaultConfigFile) @@ -38,6 +39,7 @@ type ConfigFile struct { } func (cf ConfigFile) printValidationWarnings() { + // Files section could be malformed if len(cf.Files) == 0 { log.Warningf("Validation: configuration file contains no files to watch") } @@ -262,7 +264,7 @@ func (cm ConfigManager) DestPort() int { case cm.Config.Destination.Port != 0: return cm.Config.Destination.Port default: - return 514 + return DefaultPort } }