Make beats properly exit

Parts of libbeat were exiting the application directly by using os.Exit(). This makes it impossible always clean up the application before cleanup. In addition, a manager which is part of a beat gets also exited in case the beat exits, which is not inteded.

* os.Exit() was removed from all places
* Additional tests were added to test exit behaviour
* Test for version check was added
* Add test for -configtest flag
* Make error log messages starting lowercase

CHANGELOG.asciidoc

@@ -55,6 +55,7 @@ https://github.com/elastic/beats/compare/1.0.0...master[Check the HEAD diff]
 - Update builds to Golang version 1.5.2
 - Make logstash output compression level configurable. {pull}630[630]
 - Add ability to override configuration settings using environment variables {issue}114[114]
+- Libbeat now always exits through a single exit method for proper cleanup and control {pull}736[736]
 
 *Packetbeat*
 - Add support for capturing DNS over TCP network traffic. {pull}486[486] {pull}554[554]

libbeat/beat/beat.go

@@ -104,7 +104,14 @@ func Run(name string, version string, bt Beater) {
 
 	// Runs beat inside a go process
 	go func() {
-		b.Start()
+		err := b.Start()
+
+		if err != nil {
+			// TODO: detect if logging was already fully setup or not
+			fmt.Printf("Start error: %v\n", err)
+			logp.Critical("Start error: %v", err)
+			os.Exit(1)
+		}
 
 		// If start finishes, exit has to be called. This requires start to be blocking
 		// which is currently the default.
@@ -120,18 +127,31 @@ func Run(name string, version string, bt Beater) {
 	}
 }
 
+// Start starts the Beat by parsing and interpreting the command line flags,
+// loading and parsing the configuration file, and running the Beat. This
+// method blocks until the Beat exits. If an error occurs while initializing
+// or running the Beat it will be returned.
 func (b *Beat) Start() error {
 	// Additional command line args are used to overwrite config options
-	b.CommandLineSetup()
+	err, exit := b.CommandLineSetup()
+	if err != nil {
+		return err
+	}
+
+	if exit {
+		return nil
+	}
 
 	// Loads base config
-	b.LoadConfig()
+	err = b.LoadConfig()
+	if err != nil {
+		return err
+	}
 
 	// Configures beat
-	err := b.BT.Config(b)
+	err = b.BT.Config(b)
 	if err != nil {
-		logp.Critical("Config error: %v", err)
-		os.Exit(1)
+		return err
 	}
 
 	// Run beat. This calls first beater.Setup,
@@ -141,43 +161,43 @@ func (b *Beat) Start() error {
 
 // Reads and parses the default command line params
 // To set additional cmd line args use the beat.CmdLine type before calling the function
-func (beat *Beat) CommandLineSetup() {
+// The second return param is to detect if system should exit. True if should exit
+// Exit can also be without error
+func (beat *Beat) CommandLineSetup() (error, bool) {
 
 	// The -c flag is treated separately because it needs the Beat name
 	err := cfgfile.ChangeDefaultCfgfileFlag(beat.Name)
 	if err != nil {
-		fmt.Printf("Failed to fix the -c flag: %v\n", err)
-		os.Exit(1)
+		return fmt.Errorf("failed to fix the -c flag: %v\n", err), true
 	}
 
 	flag.Parse()
 
 	if *printVersion {
 		fmt.Printf("%s version %s (%s)\n", beat.Name, beat.Version, runtime.GOARCH)
-		os.Exit(0)
+		return nil, true
 	}
 
 	// if beater implements CLIFlags for additional CLI handling, call it now
 	if flagsHandler, ok := beat.BT.(FlagsHandler); ok {
 		flagsHandler.HandleFlags(beat)
 	}
+
+	return nil, false
 }
 
 // LoadConfig inits the config file and reads the default config information
 // into Beat.Config. It exists the processes in case of errors.
-func (b *Beat) LoadConfig() {
+func (b *Beat) LoadConfig() error {
 
 	err := cfgfile.Read(&b.Config, "")
 	if err != nil {
-		// logging not yet initialized, so using fmt.Printf
-		fmt.Printf("Loading config file error: %v\n", err)
-		os.Exit(1)
+		return fmt.Errorf("loading config file error: %v\n", err)
 	}
 
 	err = logp.Init(b.Name, &b.Config.Logging)
 	if err != nil {
-		fmt.Printf("Error initializing logging: %v\n", err)
-		os.Exit(1)
+		return fmt.Errorf("error initializing logging: %v\n", err)
 	}
 
 	// Disable stderr logging if requested by cmdline flag
@@ -187,14 +207,14 @@ func (b *Beat) LoadConfig() {
 
 	pub, err := publisher.New(b.Name, b.Config.Output, b.Config.Shipper)
 	if err != nil {
-		fmt.Printf("Error Initialising publisher: %v\n", err)
-		logp.Critical(err.Error())
-		os.Exit(1)
+		return fmt.Errorf("error Initialising publisher: %v\n", err)
 	}
 
 	b.Events = pub.Client()
 
 	logp.Info("Init Beat: %s; Version: %s", b.Name, b.Version)
+
+	return nil
 }
 
 // Run calls the beater Setup and Run methods. In case of errors
@@ -204,14 +224,14 @@ func (b *Beat) Run() error {
 	// Setup beater object
 	err := b.BT.Setup(b)
 	if err != nil {
-		logp.Critical("Setup returned an error: %v", err)
-		os.Exit(1)
+		return fmt.Errorf("setup returned an error: %v", err)
 	}
 
 	// Up to here was the initialization, now about running
 	if cfgfile.IsTestConfig() {
-		// all good, exit with 0
-		os.Exit(0)
+		logp.Info("Testing configuration file")
+		// all good, exit
+		return nil
 	}
 	service.BeforeRun()
 

libbeat/mock/mockbeat.go

@@ -1,21 +1,18 @@
 package mock
 
 import (
-	"fmt"
-
 	"github.com/elastic/beats/libbeat/beat"
 )
 
 ///*** Mock Beat Setup ***///
 
-var Version = "0.0.1"
+var Version = "9.9.9"
 var Name = "mockbeat"
 
 type Mockbeat struct {
 }
 
 func (mb *Mockbeat) Config(b *beat.Beat) error {
-	fmt.Print("hello world")
 	return nil
 }
 

libbeat/tests/files/invalid.yml

@@ -0,0 +1,3 @@
+test:
+  test were
+  : invalid yml

libbeat/tests/system/test_base.py

@@ -1,6 +1,8 @@
 from mockbeat import TestCase
 
 import os
+import shutil
+import subprocess
 
 
 # Additional tests to be added:
@@ -9,22 +11,76 @@
 class Test(TestCase):
     def test_base(self):
         """
-        Checks if all lines are read from the log file.
+        Basic test with exiting Mockbeat normally
         """
 
         self.render_config_template(
             path=os.path.abspath(self.working_dir) + "/log/*"
         )
-        os.mkdir(self.working_dir + "/log/")
 
-        testfile = self.working_dir + "/log/test.log"
-        file = open(testfile, 'w')
+        exit_code = self.run_mockbeat()
+        assert exit_code == 0
 
-        iterations = 80
-        for n in range(0, iterations):
-            file.write("hello world" + str(n))
-            file.write("\n")
 
-        file.close()
+    def test_no_config(self):
+        """
+        Tests starting without a config
+        """
+        exit_code = self.run_mockbeat()
+
+        assert exit_code == 1
+        assert True == self.log_contains("loading config file error")
+        assert True == self.log_contains("Failed to read")
+
+
+    def test_invalid_config(self):
+        """
+        Checks stop on invalid config
+        """
+        shutil.copy("../files/invalid.yml", os.path.join(self.working_dir, "invalid.yml"))
+
+        exit_code = self.run_mockbeat(config="invalid.yml")
+
+        assert exit_code == 1
+        assert True == self.log_contains("loading config file error")
+        assert True == self.log_contains("YAML config parsing failed")
+
+
+    def test_config_test(self):
+        """
+        Checks if -configtest works as expected
+        """
+        shutil.copy("../../etc/libbeat.yml", os.path.join(self.working_dir, "libbeat.yml"))
+
+        exit_code = self.run_mockbeat(config="libbeat.yml", extra_args=["-configtest"])
+
+        assert exit_code == 0
+        assert True == self.log_contains("Testing configuration file")
+
+    def test_version(self):
+        """
+        Checks if version param works
+        """
+        args = ["../../libbeat.test"]
+
+        args.extend(["-version",
+                     "-e",
+                     "-systemTest",
+                     "-v",
+                     "-d", "*",
+                     "-test.coverprofile",
+                     os.path.join(self.working_dir, "coverage.cov")
+                     ])
+
+        assert False == self.log_contains("loading config file error")
+
+        with open(os.path.join(self.working_dir, "mockbeat.log"), "wb") as outputfile:
+            proc = subprocess.Popen(args,
+                                    stdout=outputfile,
+                                    stderr=subprocess.STDOUT)
+            exit_code = proc.wait()
+            assert exit_code == 0
 
-        mockbeat = self.run_mockbeat()
+        assert True == self.log_contains("mockbeat")
+        assert True == self.log_contains("version")
+        assert True == self.log_contains("9.9.9")

winlogbeat/.gitignore

@@ -21,7 +21,7 @@ _testmain.go
 *.out
 *.log
 *.bak
-*.winlogbeat.yaml
+*.winlogbeat.yml
 etc/*.dev.yml
 
 cover/