Fix shutdown from AWS Creds (#67)

* Use SIGUSR1 to signify clean shutdown
* Fix bug, parse TTL as duration, add test
* Update README

Author: relistan

.travis.yml

@@ -1,7 +1,7 @@
 language: go
 
 go:
-  - 1.13.x
+  - 1.16.x
 
 before_install:
   - curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.21.0

README.md

@@ -261,7 +261,9 @@ vars so that they can be picked up by logging systems. They are as follows:
    request. By itself this will give you the default TTL for the policy
 
  * `vault.AWSRoleTTL` - This will allow you extend the requested time, up to
-   the max allowed by Vault for the policy
+   the max allowed by Vault for the policy. The value is a string, specified
+   in [Go Duration format](https://golang.org/pkg/time/#ParseDuration). E.g.
+   "1m40s" for 1 minute and 40 seconds.
 
 
 Configuring Docker Connectivity

callbacks_test.go

@@ -451,7 +451,7 @@ func Test_ExecutorCallbacks(t *testing.T) {
 
 			Convey("Ups the TTL on creds from Vault when specified", func() {
 				dummyContainerLabels["vault.AWSRole"] = "valid-aws-role"
-				dummyContainerLabels["vault.AWSRoleTTL"] = "100"
+				dummyContainerLabels["vault.AWSRoleTTL"] = "1m40s"
 				taskInfo.Container.Docker.Parameters = labelsToDockerParams(dummyContainerLabels)
 
 				// We'll use logging output to validate that the goroutine ran
@@ -469,6 +469,23 @@ func Test_ExecutorCallbacks(t *testing.T) {
 				So(capture.String(), ShouldNotContainSubstring, "Unable to renew")
 			})
 
+			Convey("Fails the deploy when the TTL is not parseable", func() {
+				dummyContainerLabels["vault.AWSRole"] = "valid-aws-role"
+				dummyContainerLabels["vault.AWSRoleTTL"] = "100"
+				taskInfo.Container.Docker.Parameters = labelsToDockerParams(dummyContainerLabels)
+
+				// We'll use logging output to validate that the goroutine ran
+				var capture bytes.Buffer
+				log.SetLevel(log.DebugLevel)
+				log.SetOutput(&capture)
+
+				exec.LaunchTask(&taskInfo)
+
+				log.SetOutput(ioutil.Discard)
+
+				So(capture.String(), ShouldContainSubstring, "Invalid TTL passed in Docker label vaul.AWSRoleTTL")
+			})
+
 			Convey("Fails to launch a task when the AWS Rols is wrong", func() {
 				dummyContainerLabels["vault.AWSRole"] = "invalid-aws-role"
 				taskInfo.Container.Docker.Parameters = labelsToDockerParams(dummyContainerLabels)

executor.go

@@ -7,7 +7,6 @@ import (
 	"net/http"
 	"os"
 	"regexp"
-	"strconv"
 	"strings"
 	"sync"
 	"syscall"
@@ -266,7 +265,7 @@ func (exec *sidecarExecutor) monitorTask(cntnrId string, taskInfo *mesos.TaskInf
 	}
 
 	// watcherWg is used to let the Sidecar draining exit early if the
-	// container exits
+	// container exits, and when shutting down from the signal handler.
 	exec.watcherWg.Add(1)
 
 	containerName := container.GetContainerName(&taskInfo.TaskID)
@@ -284,15 +283,15 @@ func (exec *sidecarExecutor) monitorTask(cntnrId string, taskInfo *mesos.TaskInf
 
 	if err != nil {
 		log.Errorf("Error! %s", err)
+	}
 
-		if exitCode == StillRunning {
-			// Something went wrong, we better take this thing out!
-			err := container.StopContainer(
-				exec.client, containerName, exec.config.KillTaskTimeout,
-			)
-			if err != nil {
-				log.Errorf("Error stopping container %s! %s", containerName, err)
-			}
+	if exitCode == StillRunning {
+		// Something went wrong, we better take this thing out!
+		err := container.StopContainer(
+			exec.client, containerName, exec.config.KillTaskTimeout,
+		)
+		if err != nil {
+			log.Errorf("Error stopping container %s! %s", containerName, err)
 		}
 	}
 
@@ -466,7 +465,7 @@ func (exec *sidecarExecutor) monitorAWSCredsLease() {
 	}
 
 	log.Info("Attempting to shutdown because of AWS credential lease expiration")
-	ourProcess.Signal(syscall.SIGTERM)
+	ourProcess.Signal(syscall.SIGUSR1)
 }
 
 // AddAndMonitorVaultAWSKeys gets the aws keys for the specified role from Vault, begins monitoring
@@ -491,12 +490,12 @@ func (exec *sidecarExecutor) AddAndMonitorVaultAWSKeys(addEnvVars []string, role
 // will allow longer TTLs than the default, limited to no more than the max allowed by Vault.
 func (exec *sidecarExecutor) SetVaultAWSTTL(ttlStr string) error {
 	log.Infof("Renewing AWS Lease ID '%s'", exec.awsCredsLease.LeaseID)
-	ttl, err := strconv.Atoi(ttlStr)
+	ttl, err := time.ParseDuration(ttlStr)
 	if ttl < 1 || err != nil {
 		return fmt.Errorf("Invalid TTL passed in Docker label vaul.AWSRoleTTL. Could not parse: '%s'", ttlStr)
 	}
 
-	newLease, err := exec.vault.RenewAWSCredsLease(exec.awsCredsLease, ttl)
+	newLease, err := exec.vault.RenewAWSCredsLease(exec.awsCredsLease, int(ttl.Seconds()))
 	if err != nil {
 		return fmt.Errorf("Unable to renew AWS Creds Lease: %s", err)
 	}

main.go

@@ -194,18 +194,34 @@ func handleSignals(scExec *sidecarExecutor) {
 	sigChan := make(chan os.Signal, 1) // Buffered!
 
 	// Grab some signals we want to catch where possible
-	signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM)
+	signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM, syscall.SIGUSR1)
 
 	sig := <-sigChan
 	log.Warnf("Received signal '%s', attempting clean shutdown", sig)
+
+	exitCode := 130
+
 	if scExec.watchLooper != nil {
-		scExec.watchLooper.Done(errors.New("Got " + sig.String() + " signal!"))
+		// Signal to monitorTask() to exit
+		if sig == syscall.SIGUSR1 {
+			// Intentionally invoked clean shutdown
+			scExec.watchLooper.Quit()
+			exitCode = 0
+		} else {
+			scExec.watchLooper.Done(errors.New("Got " + sig.String() + " signal!"))
+		}
+
+		// Wait for monitorTask()'s goroutine to exit
+		scExec.watcherWg.Wait()
 	}
+
+	// Shut down log pump if running
 	if scExec.logsQuitChan != nil {
 		close(scExec.logsQuitChan) // Signal loops to exit
 	}
+
 	time.Sleep(3 * time.Second) // Try to let it quit
-	os.Exit(130)                // Ctrl-C received or equivalent
+	os.Exit(exitCode)                // Ctrl-C received or equivalent
 }
 
 func initConfig() (Config, error) {