Close connections on worker on shutdown, add force shutdown (#1354)

This adds functionality to ensure that connections are closed on a
worker when the server is shut down or reloaded.

Connections are closed, and then the shutdown process waits for the next
report to the controller to go through, waiting the grace period that
was added in #1330. The rest of shutdown is then proceeded with.
Listener shutdown has been moved to the start of the process to ensure
that new connections can't be made while we are shutting down.

Additionally, functionality has been added to force shut down servers
after a second interrupt has been received after the first one. This
takes the form of code in both the server and dev mode code paths that
just performs an os.Exit if another shutdown is received during the
normal shutdown process.

Additionally, the commit also includes some refactoring to tests so that
we could use some of the new E2E testing code to test session
connections during shutdown and reload.

Additionally, this fixes a panic in worker startup when there are no
controllers defined. Looks like the shutdown call was attempting to call
into the controller, not the worker.

Finally, this also removes the extra shutdown within the outer worker
startup.

Shutdown is actually handled within the inner StartWorker(), attempting
shutdown on errors was actually leading to potential use of an
uninitialized worker, which is leading to panics in testing.

internal/cmd/commands/dev/dev.go

@@ -5,8 +5,11 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"os"
+	"os/signal"
 	"runtime"
 	"strings"
+	"syscall"
 
 	"github.com/hashicorp/boundary/internal/auth/oidc"
 	"github.com/hashicorp/boundary/internal/auth/password"
@@ -626,7 +629,23 @@ func (c *Command) Run(args []string) int {
 	for !shutdownTriggered {
 		select {
 		case <-c.ShutdownCh:
-			c.UI.Output("==> Boundary dev environment shutdown triggered")
+			c.UI.Output("==> Boundary dev environment shutdown triggered, interrupt again to force")
+
+			// Add a force-shutdown goroutine to consume another interrupt
+			abortForceShutdownCh := make(chan struct{})
+			defer close(abortForceShutdownCh)
+			go func() {
+				shutdownCh := make(chan os.Signal, 4)
+				signal.Notify(shutdownCh, os.Interrupt, syscall.SIGTERM)
+				select {
+				case <-shutdownCh:
+					c.UI.Error("Second interrupt received, forcing shutdown")
+					os.Exit(base.CommandUserError)
+
+				case <-abortForceShutdownCh:
+					// No-op, we just use this to shut down the goroutine
+				}
+			}()
 
 			if !c.flagControllerOnly {
 				if err := c.worker.Shutdown(false); err != nil {

internal/cmd/commands/server/listener_reload_test.go

@@ -92,7 +92,12 @@ func TestServer_ReloadListener(t *testing.T) {
 
 	controllerKey, workerAuthKey, recoveryKey := config.DevKeyGeneration()
 
-	cmd := testServerCommand(t, controllerKey)
+	cmd := testServerCommand(t, testServerCommandOpts{
+		CreateDevDatabase: true,
+		ControllerKey:     controllerKey,
+		UseDevAuthMethod:  true,
+		UseDevTarget:      true,
+	})
 
 	defer func() {
 		if cmd.DevDatabaseCleanupFunc != nil {

internal/cmd/commands/server/server.go

@@ -5,8 +5,11 @@ import (
 	stderrors "errors"
 	"fmt"
 	"net"
+	"os"
+	"os/signal"
 	"runtime"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/hashicorp/boundary/globals"
@@ -459,9 +462,6 @@ func (c *Command) Run(args []string) int {
 	if c.Config.Worker != nil {
 		if err := c.StartWorker(); err != nil {
 			c.UI.Error(err.Error())
-			if err := c.controller.Shutdown(false); err != nil {
-				c.UI.Error(fmt.Errorf("Error with controller shutdown: %w", err).Error())
-			}
 			return base.CommandCliError
 		}
 	}
@@ -592,14 +592,31 @@ func (c *Command) WaitForInterrupt() int {
 	for !shutdownTriggered {
 		select {
 		case <-c.ShutdownCh:
-			c.UI.Output("==> Boundary server shutdown triggered")
+			c.UI.Output("==> Boundary server shutdown triggered, interrupt again to force")
+
+			// Add a force-shutdown goroutine to consume another interrupt
+			abortForceShutdownCh := make(chan struct{})
+			defer close(abortForceShutdownCh)
+			go func() {
+				shutdownCh := make(chan os.Signal, 4)
+				signal.Notify(shutdownCh, os.Interrupt, syscall.SIGTERM)
+				select {
+				case <-shutdownCh:
+					c.UI.Error("Second interrupt received, forcing shutdown")
+					os.Exit(base.CommandUserError)
+				case <-abortForceShutdownCh:
+					// No-op, we just use this to shut down the goroutine
+				}
+			}()
 
+			// Do worker shutdown
 			if c.Config.Worker != nil {
 				if err := c.worker.Shutdown(false); err != nil {
 					c.UI.Error(fmt.Errorf("Error shutting down worker: %w", err).Error())
 				}
 			}
 
+			// Do controller shutdown
 			if c.Config.Controller != nil {
 				if err := c.controller.Shutdown(c.Config.Worker != nil); err != nil {
 					c.UI.Error(fmt.Errorf("Error shutting down controller: %w", err).Error())

internal/cmd/commands/server/server_test.go

@@ -6,10 +6,16 @@ import (
 
 	"github.com/hashicorp/boundary/internal/cmd/base"
 	"github.com/hashicorp/boundary/internal/cmd/config"
+	"github.com/hashicorp/boundary/internal/servers/controller"
 	"github.com/mitchellh/cli"
 	"github.com/stretchr/testify/require"
 )
 
+// We try to pull these from TestController, but targets et al are
+// computed off of a suffix instead of having constants. Just define
+// for now here, this can be tweaked later if need be.
+const defaultTestTargetId = "ttcp_1234567890"
+
 const rootKmsConfig = `
 kms "aead" {
 	purpose = "root"
@@ -18,7 +24,21 @@ kms "aead" {
 	key_id = "global_root"
 }`
 
-func testServerCommand(t *testing.T, controllerKey string) *Command {
+type testServerCommandOpts struct {
+	// Whether or not to create the dev database
+	CreateDevDatabase bool
+
+	// The controller key used in dev database creation
+	ControllerKey string
+
+	// Use the well-known dev mode auth method id (1234567890)
+	UseDevAuthMethod bool
+
+	// Use the well-known dev mode target method id (1234567890)
+	UseDevTarget bool
+}
+
+func testServerCommand(t *testing.T, opts testServerCommandOpts) *Command {
 	require := require.New(t)
 	t.Helper()
 	cmd := &Command{
@@ -31,17 +51,29 @@ func testServerCommand(t *testing.T, controllerKey string) *Command {
 	require.NoError(cmd.SetupLogging("trace", "", "", ""))
 	require.NoError(cmd.SetupEventing(cmd.Logger, cmd.StderrLock, "test-server-command"))
 
-	kmsHcl := fmt.Sprintf(rootKmsConfig, controllerKey)
-	parsedKmsConfig, err := config.Parse(kmsHcl)
-	require.NoError(err)
-	require.NoError(cmd.SetupKMSes(cmd.UI, parsedKmsConfig))
+	if opts.CreateDevDatabase {
+		kmsHcl := fmt.Sprintf(rootKmsConfig, opts.ControllerKey)
+		parsedKmsConfig, err := config.Parse(kmsHcl)
+		require.NoError(err)
+		require.NoError(cmd.SetupKMSes(cmd.UI, parsedKmsConfig))
 
-	err = cmd.CreateDevDatabase(cmd.Context, base.WithContainerImage("postgres"), base.WithSkipOidcAuthMethodCreation())
-	if err != nil {
-		if cmd.DevDatabaseCleanupFunc != nil {
-			require.NoError(cmd.DevDatabaseCleanupFunc())
+		if opts.UseDevAuthMethod {
+			cmd.Server.DevPasswordAuthMethodId = controller.DefaultTestPasswordAuthMethodId
+			cmd.Server.DevLoginName = controller.DefaultTestLoginName
+			cmd.Server.DevPassword = controller.DefaultTestPassword
+		}
+
+		if opts.UseDevTarget {
+			cmd.Server.DevTargetId = defaultTestTargetId
+		}
+
+		err = cmd.CreateDevDatabase(cmd.Context, base.WithContainerImage("postgres"), base.WithSkipOidcAuthMethodCreation())
+		if err != nil {
+			if cmd.DevDatabaseCleanupFunc != nil {
+				require.NoError(cmd.DevDatabaseCleanupFunc())
+			}
+			require.NoError(err)
 		}
-		require.NoError(err)
 	}
 
 	return cmd