-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
drivers/exec+java: Add configuration to restore previous PID/IPC namespace behavior #9982
Changes from all commits
b682371
779c90d
1364e33
134eebb
0980482
ede2a63
ce4b59c
6dd5de4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package exec | |
import ( | ||
"bytes" | ||
"context" | ||
"errors" | ||
"fmt" | ||
"io/ioutil" | ||
"os" | ||
|
@@ -16,6 +17,7 @@ import ( | |
"time" | ||
|
||
ctestutils "github.com/hashicorp/nomad/client/testutil" | ||
"github.com/hashicorp/nomad/drivers/shared/executor" | ||
"github.com/hashicorp/nomad/helper/pluginutils/hclutils" | ||
"github.com/hashicorp/nomad/helper/testlog" | ||
"github.com/hashicorp/nomad/helper/testtask" | ||
|
@@ -273,7 +275,7 @@ func TestExecDriver_StartWaitRecover(t *testing.T) { | |
// task dies, the orphans in the PID namespaces are killed by the kernel | ||
func TestExecDriver_NoOrphans(t *testing.T) { | ||
t.Parallel() | ||
require := require.New(t) | ||
r := require.New(t) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we doing this now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I only care that we do not shadow names. Whether we use |
||
ctestutils.ExecCompatible(t) | ||
|
||
ctx, cancel := context.WithCancel(context.Background()) | ||
|
@@ -283,6 +285,17 @@ func TestExecDriver_NoOrphans(t *testing.T) { | |
harness := dtestutil.NewDriverHarness(t, d) | ||
defer harness.Kill() | ||
|
||
config := &Config{ | ||
NoPivotRoot: false, | ||
DefaultModePID: executor.IsolationModePrivate, | ||
DefaultModeIPC: executor.IsolationModePrivate, | ||
} | ||
|
||
var data []byte | ||
r.NoError(basePlug.MsgPackEncode(&data, config)) | ||
baseConfig := &basePlug.Config{PluginConfig: data} | ||
r.NoError(harness.SetConfig(baseConfig)) | ||
|
||
task := &drivers.TaskConfig{ | ||
ID: uuid.Generate(), | ||
Name: "test", | ||
|
@@ -295,21 +308,21 @@ func TestExecDriver_NoOrphans(t *testing.T) { | |
taskConfig["command"] = "/bin/sh" | ||
// print the child PID in the task PID namespace, then sleep for 5 seconds to give us a chance to examine processes | ||
taskConfig["args"] = []string{"-c", fmt.Sprintf(`sleep 3600 & sleep 20`)} | ||
require.NoError(task.EncodeConcreteDriverConfig(&taskConfig)) | ||
r.NoError(task.EncodeConcreteDriverConfig(&taskConfig)) | ||
|
||
handle, _, err := harness.StartTask(task) | ||
require.NoError(err) | ||
r.NoError(err) | ||
defer harness.DestroyTask(task.ID, true) | ||
|
||
waitCh, err := harness.WaitTask(context.Background(), handle.Config.ID) | ||
require.NoError(err) | ||
r.NoError(err) | ||
|
||
require.NoError(harness.WaitUntilStarted(task.ID, 1*time.Second)) | ||
r.NoError(harness.WaitUntilStarted(task.ID, 1*time.Second)) | ||
|
||
var childPids []int | ||
taskState := TaskState{} | ||
testutil.WaitForResult(func() (bool, error) { | ||
require.NoError(handle.GetDriverState(&taskState)) | ||
r.NoError(handle.GetDriverState(&taskState)) | ||
if taskState.Pid == 0 { | ||
return false, fmt.Errorf("task PID is zero") | ||
} | ||
|
@@ -331,14 +344,14 @@ func TestExecDriver_NoOrphans(t *testing.T) { | |
} | ||
return true, nil | ||
}, func(err error) { | ||
require.NoError(err) | ||
r.NoError(err) | ||
}) | ||
|
||
select { | ||
case result := <-waitCh: | ||
require.True(result.Successful(), "command failed: %#v", result) | ||
r.True(result.Successful(), "command failed: %#v", result) | ||
case <-time.After(30 * time.Second): | ||
require.Fail("timeout waiting for task to shutdown") | ||
r.Fail("timeout waiting for task to shutdown") | ||
} | ||
|
||
// isProcessRunning returns an error if process is not running | ||
|
@@ -357,7 +370,7 @@ func TestExecDriver_NoOrphans(t *testing.T) { | |
} | ||
|
||
// task should be dead | ||
require.Error(isProcessRunning(taskState.Pid)) | ||
r.Error(isProcessRunning(taskState.Pid)) | ||
|
||
// all children should eventually be killed by OS | ||
testutil.WaitForResult(func() (bool, error) { | ||
|
@@ -372,7 +385,7 @@ func TestExecDriver_NoOrphans(t *testing.T) { | |
} | ||
return true, nil | ||
}, func(err error) { | ||
require.NoError(err) | ||
r.NoError(err) | ||
}) | ||
} | ||
|
||
|
@@ -711,7 +724,7 @@ config { | |
|
||
func TestExecDriver_NoPivotRoot(t *testing.T) { | ||
t.Parallel() | ||
require := require.New(t) | ||
r := require.New(t) | ||
ctestutils.ExecCompatible(t) | ||
|
||
ctx, cancel := context.WithCancel(context.Background()) | ||
|
@@ -720,11 +733,16 @@ func TestExecDriver_NoPivotRoot(t *testing.T) { | |
d := NewExecDriver(ctx, testlog.HCLogger(t)) | ||
harness := dtestutil.NewDriverHarness(t, d) | ||
|
||
config := &Config{NoPivotRoot: true} | ||
config := &Config{ | ||
NoPivotRoot: true, | ||
DefaultModePID: executor.IsolationModePrivate, | ||
DefaultModeIPC: executor.IsolationModePrivate, | ||
} | ||
|
||
var data []byte | ||
require.NoError(basePlug.MsgPackEncode(&data, config)) | ||
r.NoError(basePlug.MsgPackEncode(&data, config)) | ||
bconfig := &basePlug.Config{PluginConfig: data} | ||
require.NoError(harness.SetConfig(bconfig)) | ||
r.NoError(harness.SetConfig(bconfig)) | ||
|
||
task := &drivers.TaskConfig{ | ||
ID: uuid.Generate(), | ||
|
@@ -738,9 +756,28 @@ func TestExecDriver_NoPivotRoot(t *testing.T) { | |
Command: "/bin/sleep", | ||
Args: []string{"100"}, | ||
} | ||
require.NoError(task.EncodeConcreteDriverConfig(&tc)) | ||
r.NoError(task.EncodeConcreteDriverConfig(&tc)) | ||
|
||
handle, _, err := harness.StartTask(task) | ||
require.NoError(err) | ||
require.NotNil(handle) | ||
r.NoError(err) | ||
r.NotNil(handle) | ||
} | ||
|
||
func TestDriver_Config_validate(t *testing.T) { | ||
for _, tc := range []struct { | ||
pidMode, ipcMode string | ||
exp error | ||
}{ | ||
{pidMode: "host", ipcMode: "host", exp: nil}, | ||
{pidMode: "private", ipcMode: "host", exp: nil}, | ||
{pidMode: "host", ipcMode: "private", exp: nil}, | ||
{pidMode: "private", ipcMode: "private", exp: nil}, | ||
{pidMode: "other", ipcMode: "private", exp: errors.New(`default_pid_mode must be "private" or "host", got "other"`)}, | ||
{pidMode: "private", ipcMode: "other", exp: errors.New(`default_ipc_mode must be "private" or "host", got "other"`)}, | ||
} { | ||
require.Equal(t, tc.exp, (&Config{ | ||
DefaultModePID: tc.pidMode, | ||
DefaultModeIPC: tc.ipcMode, | ||
}).validate()) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about using
DefaultPIDMode
here, to keep parity with the config keydefault_pid_mode
?