agent: return a non-zero exit code on error (#9670)

* agent: return a non-zero exit code on error

* agent/template: always return on template server error, add case for error_on_missing_key

* agent: fix tests by updating Run params to use an errCh

* agent/template: add permission denied test case, clean up test var

* agent: use unbuffered errCh, emit fatal errors directly to the UI output

* agent: use oklog's run.Group to schedule subsystem runners (#9761)

* agent: use oklog's run.Group to schedule subsystem runners

* agent: clean up unused DoneCh, clean up agent's main Run func

* agent/template: use ts.stopped.CAS to atomically swap value

* fix tests

* fix tests

* agent/template: add timeout on TestRunServer

* agent: output error via logs and return a generic error on non-zero exit

* fix TestAgent_ExitAfterAuth

* agent/template: do not restart ct runner on new incoming token if exit_after_auth is set to true

* agent: drain ah.OutputCh after sink exits to avoid blocking on the channel

* use context.WithTimeout, expand comments around ordering of defer cancel()

command/agent/cert_end_to_end_test.go

@@ -135,11 +135,7 @@ func testCertEndToEnd(t *testing.T, withCertRoleName, ahWrapping bool) {
 		logger.Trace("wrote dh param file", "path", dhpath)
 	}
 
-	ctx, cancelFunc := context.WithCancel(context.Background())
-	timer := time.AfterFunc(30*time.Second, func() {
-		cancelFunc()
-	})
-	defer timer.Stop()
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 
 	aaConfig := map[string]interface{}{}
 
@@ -165,9 +161,18 @@ func testCertEndToEnd(t *testing.T, withCertRoleName, ahWrapping bool) {
 		ahConfig.WrapTTL = 10 * time.Second
 	}
 	ah := auth.NewAuthHandler(ahConfig)
-	go ah.Run(ctx, am)
+	errCh := make(chan error)
+	go func() {
+		errCh <- ah.Run(ctx, am)
+	}()
 	defer func() {
-		<-ah.DoneCh
+		select {
+		case <-ctx.Done():
+		case err := <-errCh:
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
 	}()
 
 	config := &sink.SinkConfig{
@@ -193,13 +198,25 @@ func testCertEndToEnd(t *testing.T, withCertRoleName, ahWrapping bool) {
 		Logger: logger.Named("sink.server"),
 		Client: client,
 	})
-	go ss.Run(ctx, ah.OutputCh, []*sink.SinkConfig{config})
+	go func() {
+		errCh <- ss.Run(ctx, ah.OutputCh, []*sink.SinkConfig{config})
+	}()
 	defer func() {
-		<-ss.DoneCh
+		select {
+		case <-ctx.Done():
+		case err := <-errCh:
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
 	}()
 
-	// This has to be after the other defers so it happens first
-	defer cancelFunc()
+	// This has to be after the other defers so it happens first. It allows
+	// successful test runs to immediately cancel all of the runner goroutines
+	// and unblock any of the blocking defer calls by the runner's DoneCh that
+	// comes before this and avoid successful tests from taking the entire
+	// timeout duration.
+	defer cancel()
 
 	cloned, err := client.Clone()
 	if err != nil {
@@ -454,11 +471,7 @@ func TestCertEndToEnd_CertsInConfig(t *testing.T) {
 	// Auth handler (auto-auth) setup
 	// /////////////
 
-	ctx, cancelFunc := context.WithCancel(context.Background())
-	timer := time.AfterFunc(30*time.Second, func() {
-		cancelFunc()
-	})
-	defer timer.Stop()
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 
 	am, err := agentcert.NewCertAuthMethod(&auth.AuthConfig{
 		Logger:    logger.Named("auth.cert"),
@@ -480,9 +493,18 @@ func TestCertEndToEnd_CertsInConfig(t *testing.T) {
 	}
 
 	ah := auth.NewAuthHandler(ahConfig)
-	go ah.Run(ctx, am)
+	errCh := make(chan error)
+	go func() {
+		errCh <- ah.Run(ctx, am)
+	}()
 	defer func() {
-		<-ah.DoneCh
+		select {
+		case <-ctx.Done():
+		case err := <-errCh:
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
 	}()
 
 	// /////////////
@@ -515,13 +537,25 @@ func TestCertEndToEnd_CertsInConfig(t *testing.T) {
 		Logger: logger.Named("sink.server"),
 		Client: client,
 	})
-	go ss.Run(ctx, ah.OutputCh, []*sink.SinkConfig{config})
+	go func() {
+		errCh <- ss.Run(ctx, ah.OutputCh, []*sink.SinkConfig{config})
+	}()
 	defer func() {
-		<-ss.DoneCh
+		select {
+		case <-ctx.Done():
+		case err := <-errCh:
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
 	}()
 
-	// This has to be after the other defers so it happens first
-	defer cancelFunc()
+	// This has to be after the other defers so it happens first. It allows
+	// successful test runs to immediately cancel all of the runner goroutines
+	// and unblock any of the blocking defer calls by the runner's DoneCh that
+	// comes before this and avoid successful tests from taking the entire
+	// timeout duration.
+	defer cancel()
 
 	// Read the token from the sink
 	timeout := time.Now().Add(5 * time.Second)