diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go index ea4b58d28..f2b04f4d5 100644 --- a/drivers/exec/driver_test.go +++ b/drivers/exec/driver_test.go @@ -25,6 +25,7 @@ import ( "github.com/hashicorp/nomad/plugins/shared/hclspec" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" ) func TestMain(m *testing.M) { @@ -121,7 +122,7 @@ func TestExecDriver_StartWaitStop(t *testing.T) { taskConfig := map[string]interface{}{ "command": "/bin/sleep", - "args": []string{"5"}, + "args": []string{"600"}, } encodeDriverHelper(require, task, taskConfig) @@ -134,38 +135,95 @@ func TestExecDriver_StartWaitStop(t *testing.T) { ch, err := harness.WaitTask(context.Background(), handle.Config.ID) require.NoError(err) - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - result := <-ch - require.Equal(2, result.Signal) - }() - require.NoError(harness.WaitUntilStarted(task.ID, 1*time.Second)) - wg.Add(1) go func() { - defer wg.Done() - err := harness.StopTask(task.ID, 2*time.Second, "SIGINT") - require.NoError(err) - }() - - waitCh := make(chan struct{}) - go func() { - defer close(waitCh) - wg.Wait() + harness.StopTask(task.ID, 2*time.Second, "SIGINT") }() select { - case <-waitCh: - status, err := harness.InspectTask(task.ID) - require.NoError(err) - require.Equal(drivers.TaskStateExited, status.State) - case <-time.After(1 * time.Second): + case result := <-ch: + require.Equal(int(unix.SIGINT), result.Signal) + case <-time.After(10 * time.Second): require.Fail("timeout waiting for task to shutdown") } + // Ensure that the task is marked as dead, but account + // for WaitTask() closing channel before internal state is updated + testutil.WaitForResult(func() (bool, error) { + status, err := harness.InspectTask(task.ID) + if err != nil { + return false, fmt.Errorf("inspecting task failed: %v", err) + } + if status.State != drivers.TaskStateExited { + return false, fmt.Errorf("task hasn't exited yet; status: %v", status.State) + } + + return true, nil + }, func(err error) { + require.NoError(err) + }) + + require.NoError(harness.DestroyTask(task.ID, true)) +} + +func TestExecDriver_StartWaitStopKill(t *testing.T) { + t.Parallel() + require := require.New(t) + ctestutils.ExecCompatible(t) + + d := NewExecDriver(testlog.HCLogger(t)) + harness := drivers.NewDriverHarness(t, d) + task := &drivers.TaskConfig{ + ID: uuid.Generate(), + Name: "test", + } + + taskConfig := map[string]interface{}{ + "command": "/bin/bash", + "args": []string{"-c", "echo hi; sleep 600"}, + } + encodeDriverHelper(require, task, taskConfig) + + cleanup := harness.MkAllocDir(task, false) + defer cleanup() + + handle, _, err := harness.StartTask(task) + require.NoError(err) + defer harness.DestroyTask(task.ID, true) + + ch, err := harness.WaitTask(context.Background(), handle.Config.ID) + require.NoError(err) + + require.NoError(harness.WaitUntilStarted(task.ID, 1*time.Second)) + + go func() { + harness.StopTask(task.ID, 2*time.Second, "SIGINT") + }() + + select { + case result := <-ch: + require.False(result.Successful()) + case <-time.After(10 * time.Second): + require.Fail("timeout waiting for task to shutdown") + } + + // Ensure that the task is marked as dead, but account + // for WaitTask() closing channel before internal state is updated + testutil.WaitForResult(func() (bool, error) { + status, err := harness.InspectTask(task.ID) + if err != nil { + return false, fmt.Errorf("inspecting task failed: %v", err) + } + if status.State != drivers.TaskStateExited { + return false, fmt.Errorf("task hasn't exited yet; status: %v", status.State) + } + + return true, nil + }, func(err error) { + require.NoError(err) + }) + require.NoError(harness.DestroyTask(task.ID, true)) } diff --git a/drivers/java/driver_test.go b/drivers/java/driver_test.go index 62c0aec0b..34ba1fb1e 100644 --- a/drivers/java/driver_test.go +++ b/drivers/java/driver_test.go @@ -1,11 +1,11 @@ package java import ( + "fmt" "io" "io/ioutil" "os" "path/filepath" - "sync" "testing" dtestutil "github.com/hashicorp/nomad/plugins/drivers/testutils" @@ -107,7 +107,7 @@ func TestJavaDriver_Jar_Stop_Wait(t *testing.T) { task := basicTask(t, "demo-app", map[string]interface{}{ "jar_path": "demoapp.jar", - "args": []string{"20"}, + "args": []string{"600"}, "jvm_options": []string{"-Xmx64m", "-Xms32m"}, }) @@ -122,40 +122,36 @@ func TestJavaDriver_Jar_Stop_Wait(t *testing.T) { ch, err := harness.WaitTask(context.Background(), handle.Config.ID) require.NoError(err) - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - result := <-ch - require.Equal(2, result.Signal) - }() - require.NoError(harness.WaitUntilStarted(task.ID, 1*time.Second)) - wg.Add(1) go func() { - defer wg.Done() - time.Sleep(10 * time.Millisecond) - err := harness.StopTask(task.ID, 2*time.Second, "SIGINT") - require.NoError(err) - }() - - waitCh := make(chan struct{}) - go func() { - defer close(waitCh) - wg.Wait() + harness.StopTask(task.ID, 2*time.Second, "SIGINT") }() select { - case <-waitCh: - status, err := harness.InspectTask(task.ID) - require.NoError(err) - require.Equal(drivers.TaskStateExited, status.State) - case <-time.After(5 * time.Second): + case result := <-ch: + require.False(result.Successful()) + case <-time.After(10 * time.Second): require.Fail("timeout waiting for task to shutdown") } + // Ensure that the task is marked as dead, but account + // for WaitTask() closing channel before internal state is updated + testutil.WaitForResult(func() (bool, error) { + status, err := harness.InspectTask(task.ID) + if err != nil { + return false, fmt.Errorf("inspecting task failed: %v", err) + } + if status.State != drivers.TaskStateExited { + return false, fmt.Errorf("task hasn't exited yet; status: %v", status.State) + } + + return true, nil + }, func(err error) { + require.NoError(err) + }) + require.NoError(harness.DestroyTask(task.ID, true)) } diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index 989cea353..c82f276ef 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -247,7 +247,7 @@ func (l *LibcontainerExecutor) wait() { ps = exitErr.ProcessState } else { l.logger.Error("failed to call wait on user process", "error", err) - l.exitState = &ProcessState{Pid: 0, ExitCode: 0, Time: time.Now()} + l.exitState = &ProcessState{Pid: 0, ExitCode: 1, Time: time.Now()} return } } @@ -310,6 +310,8 @@ func (l *LibcontainerExecutor) Shutdown(signal string, grace time.Duration) erro return fmt.Errorf("error unknown signal given for shutdown: %s", signal) } + // Signal initial container processes only during graceful + // shutdown; hence `false` arg. err = l.container.Signal(sig, false) if err != nil { return err @@ -319,10 +321,12 @@ func (l *LibcontainerExecutor) Shutdown(signal string, grace time.Duration) erro case <-l.userProcExited: return nil case <-time.After(grace): - return l.container.Signal(os.Kill, false) + // Force kill all container processes after grace period, + // hence `true` argument. + return l.container.Signal(os.Kill, true) } } else { - return l.container.Signal(os.Kill, false) + return l.container.Signal(os.Kill, true) } }