Skip to content

Commit

Permalink
Merge pull request #9982 from hashicorp/f-nsiso-driver
Browse files Browse the repository at this point in the history
drivers/exec+java: Add configuration to restore previous PID/IPC namespace behavior
  • Loading branch information
shoenig committed Feb 8, 2021
2 parents 0078854 + 6dd5de4 commit 6c376fc
Show file tree
Hide file tree
Showing 15 changed files with 402 additions and 107 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ FEATURES:
IMPROVEMENTS:
* cli: Improved `scaling policy` commands with -verbose, auto-completion, and prefix-matching [[GH-9964](https://github.com/hashicorp/nomad/issues/9964)]
* consul/connect: Made handling of sidecar task container image URLs consistent with the `docker` task driver. [[GH-9580](https://github.com/hashicorp/nomad/issues/9580)]
* drivers/exec+java: Added client plugin configuration to re-enable previous PID/IPC namespace behavior [[GH-9982](https://github.com/hashicorp/nomad/pull/9982)]

BUG FIXES:
* consul: Fixed a bug where failing tasks with group services would only cause the allocation to restart once instead of respecting the `restart` field. [[GH-9869](https://github.com/hashicorp/nomad/issues/9869)]
Expand Down
8 changes: 4 additions & 4 deletions command/job_init.bindata_assetfs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 39 additions & 1 deletion drivers/exec/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ var (
hclspec.NewAttr("no_pivot_root", "bool", false),
hclspec.NewLiteral("false"),
),
"default_pid_mode": hclspec.NewDefault(
hclspec.NewAttr("default_pid_mode", "string", false),
hclspec.NewLiteral(`"private"`),
),
"default_ipc_mode": hclspec.NewDefault(
hclspec.NewAttr("default_ipc_mode", "string", false),
hclspec.NewLiteral(`"private"`),
),
})

// taskConfigSpec is the hcl specification for the driver config section of
Expand Down Expand Up @@ -122,6 +130,30 @@ type Config struct {
// NoPivotRoot disables the use of pivot_root, useful when the root partition
// is on ramdisk
NoPivotRoot bool `codec:"no_pivot_root"`

// DefaultModePID is the default PID isolation set for all tasks using
// exec-based task drivers.
DefaultModePID string `codec:"default_pid_mode"`

// DefaultModeIPC is the default IPC isolation set for all tasks using
// exec-based task drivers.
DefaultModeIPC string `codec:"default_ipc_mode"`
}

func (c *Config) validate() error {
switch c.DefaultModePID {
case executor.IsolationModePrivate, executor.IsolationModeHost:
default:
return fmt.Errorf("default_pid_mode must be %q or %q, got %q", executor.IsolationModePrivate, executor.IsolationModeHost, c.DefaultModePID)
}

switch c.DefaultModeIPC {
case executor.IsolationModePrivate, executor.IsolationModeHost:
default:
return fmt.Errorf("default_ipc_mode must be %q or %q, got %q", executor.IsolationModePrivate, executor.IsolationModeHost, c.DefaultModeIPC)
}

return nil
}

// TaskConfig is the driver configuration of a task within a job
Expand Down Expand Up @@ -182,14 +214,18 @@ func (d *Driver) ConfigSchema() (*hclspec.Spec, error) {
}

func (d *Driver) SetConfig(cfg *base.Config) error {
// unpack, validate, and set agent plugin config
var config Config
if len(cfg.PluginConfig) != 0 {
if err := base.MsgPackDecode(cfg.PluginConfig, &config); err != nil {
return err
}
}

if err := config.validate(); err != nil {
return err
}
d.config = config

if cfg != nil && cfg.AgentConfig != nil {
d.nomadConfig = cfg.AgentConfig.Driver
}
Expand Down Expand Up @@ -383,6 +419,8 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
Mounts: cfg.Mounts,
Devices: cfg.Devices,
NetworkIsolation: cfg.NetworkIsolation,
DefaultModePID: d.config.DefaultModePID,
DefaultModeIPC: d.config.DefaultModeIPC,
}

ps, err := exec.Launch(execCmd)
Expand Down
73 changes: 55 additions & 18 deletions drivers/exec/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package exec
import (
"bytes"
"context"
"errors"
"fmt"
"io/ioutil"
"os"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
ctestutils.ExecCompatible(t)

ctx, cancel := context.WithCancel(context.Background())
Expand All @@ -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",
Expand All @@ -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")
}
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -372,7 +385,7 @@ func TestExecDriver_NoOrphans(t *testing.T) {
}
return true, nil
}, func(err error) {
require.NoError(err)
r.NoError(err)
})
}

Expand Down Expand Up @@ -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())
Expand All @@ -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(),
Expand All @@ -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())
}
}
55 changes: 54 additions & 1 deletion drivers/java/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,16 @@ var (
}

// configSpec is the hcl specification returned by the ConfigSchema RPC
configSpec = hclspec.NewObject(map[string]*hclspec.Spec{})
configSpec = hclspec.NewObject(map[string]*hclspec.Spec{
"default_pid_mode": hclspec.NewDefault(
hclspec.NewAttr("default_pid_mode", "string", false),
hclspec.NewLiteral(`"private"`),
),
"default_ipc_mode": hclspec.NewDefault(
hclspec.NewAttr("default_ipc_mode", "string", false),
hclspec.NewLiteral(`"private"`),
),
})

// taskConfigSpec is the hcl specification for the driver config section of
// a taskConfig within a job. It is returned in the TaskConfigSchema RPC
Expand Down Expand Up @@ -101,6 +110,33 @@ func init() {
}
}

// Config is the driver configuration set by the SetConfig RPC call
type Config struct {
// DefaultModePID is the default PID isolation set for all tasks using
// exec-based task drivers.
DefaultModePID string `codec:"default_pid_mode"`

// DefaultModeIPC is the default IPC isolation set for all tasks using
// exec-based task drivers.
DefaultModeIPC string `codec:"default_ipc_mode"`
}

func (c *Config) validate() error {
switch c.DefaultModePID {
case executor.IsolationModePrivate, executor.IsolationModeHost:
default:
return fmt.Errorf("default_pid_mode must be %q or %q, got %q", executor.IsolationModePrivate, executor.IsolationModeHost, c.DefaultModePID)
}

switch c.DefaultModeIPC {
case executor.IsolationModePrivate, executor.IsolationModeHost:
default:
return fmt.Errorf("default_ipc_mode must be %q or %q, got %q", executor.IsolationModePrivate, executor.IsolationModeHost, c.DefaultModeIPC)
}

return nil
}

// TaskConfig is the driver configuration of a taskConfig within a job
type TaskConfig struct {
Class string `codec:"class"`
Expand All @@ -126,6 +162,9 @@ type Driver struct {
// event can be broadcast to all callers
eventer *eventer.Eventer

// config is the driver configuration set by the SetConfig RPC
config Config

// tasks is the in memory datastore mapping taskIDs to taskHandle
tasks *taskStore

Expand Down Expand Up @@ -159,6 +198,18 @@ func (d *Driver) ConfigSchema() (*hclspec.Spec, error) {
}

func (d *Driver) SetConfig(cfg *base.Config) error {
// unpack, validate, and set agent plugin config
var config Config
if len(cfg.PluginConfig) != 0 {
if err := base.MsgPackDecode(cfg.PluginConfig, &config); err != nil {
return err
}
}
if err := config.validate(); err != nil {
return err
}
d.config = config

if cfg != nil && cfg.AgentConfig != nil {
d.nomadConfig = cfg.AgentConfig.Driver
}
Expand Down Expand Up @@ -374,6 +425,8 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
Mounts: cfg.Mounts,
Devices: cfg.Devices,
NetworkIsolation: cfg.NetworkIsolation,
DefaultModePID: d.config.DefaultModePID,
DefaultModeIPC: d.config.DefaultModeIPC,
}

ps, err := exec.Launch(execCmd)
Expand Down
Loading

0 comments on commit 6c376fc

Please sign in to comment.