Skip to content

Commit

Permalink
drivers/exec+java: Add task configuration to restore previous PID/IPC…
Browse files Browse the repository at this point in the history
… isolation behavior

This PR adds pid_mode and ipc_mode options to the exec and java task
driver config options. By default these will defer to the default_pid_mode
and default_ipc_mode agent plugin options created in #9969. Setting
these values to "host" mode disables isolation for the task. Doing so
is not recommended, but may be necessary to support legacy job configurations.

Closes #9970
  • Loading branch information
shoenig committed Feb 8, 2021
1 parent 6c376fc commit 362bcfd
Show file tree
Hide file tree
Showing 16 changed files with 330 additions and 37 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +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)]
* drivers/exec+java: Added client plugin and task configuration options to re-enable previous PID/IPC namespace behavior [[GH-9982](https://github.com/hashicorp/nomad/pull/9982)] [[tbd](tbd)]

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
45 changes: 39 additions & 6 deletions drivers/exec/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,10 @@ var (
// taskConfigSpec is the hcl specification for the driver config section of
// a task within a job. It is returned in the TaskConfigSchema RPC
taskConfigSpec = hclspec.NewObject(map[string]*hclspec.Spec{
"command": hclspec.NewAttr("command", "string", true),
"args": hclspec.NewAttr("args", "list(string)", false),
"command": hclspec.NewAttr("command", "string", true),
"args": hclspec.NewAttr("args", "list(string)", false),
"pid_mode": hclspec.NewAttr("pid_mode", "string", false),
"ipc_mode": hclspec.NewAttr("ipc_mode", "string", false),
})

// capabilities is returned by the Capabilities RPC and indicates what
Expand Down Expand Up @@ -158,8 +160,35 @@ func (c *Config) validate() error {

// TaskConfig is the driver configuration of a task within a job
type TaskConfig struct {
Command string `codec:"command"`
Args []string `codec:"args"`
// Command is the thing to exec.
Command string `codec:"command"`

// Args are passed along to Command.
Args []string `codec:"args"`

// ModePID indicates whether PID namespace isolation is enabled for the task.
// Must be "private" or "host" if set.
ModePID string `codec:"pid_mode"`

// ModeIPC indicates whether IPC namespace isolation is enabled for the task.
// Must be "private" or "host" if set.
ModeIPC string `codec:"ipc_mode"`
}

func (tc *TaskConfig) validate() error {
switch tc.ModePID {
case "", executor.IsolationModePrivate, executor.IsolationModeHost:
default:
return fmt.Errorf("pid_mode must be %q or %q, got %q", executor.IsolationModePrivate, executor.IsolationModeHost, tc.ModePID)
}

switch tc.ModeIPC {
case "", executor.IsolationModePrivate, executor.IsolationModeHost:
default:
return fmt.Errorf("ipc_mode must be %q or %q, got %q", executor.IsolationModePrivate, executor.IsolationModeHost, tc.ModeIPC)
}

return nil
}

// TaskState is the state which is encoded in the handle returned in
Expand Down Expand Up @@ -374,6 +403,10 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
return nil, nil, fmt.Errorf("failed to decode driver config: %v", err)
}

if err := driverConfig.validate(); err != nil {
return nil, nil, fmt.Errorf("failed driver config validation: %v", err)
}

d.logger.Info("starting task", "driver_cfg", hclog.Fmt("%+v", driverConfig))
handle := drivers.NewTaskHandle(taskHandleVersion)
handle.Config = cfg
Expand Down Expand Up @@ -419,8 +452,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,
ModePID: executor.IsolationMode(d.config.DefaultModePID, driverConfig.ModePID),
ModeIPC: executor.IsolationMode(d.config.DefaultModeIPC, driverConfig.ModeIPC),
}

ps, err := exec.Launch(execCmd)
Expand Down
22 changes: 22 additions & 0 deletions drivers/exec/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -781,3 +781,25 @@ func TestDriver_Config_validate(t *testing.T) {
}).validate())
}
}

func TestDriver_TaskConfig_validate(t *testing.T) {
for _, tc := range []struct {
pidMode, ipcMode string
exp error
}{
{pidMode: "host", ipcMode: "host", exp: nil},
{pidMode: "host", ipcMode: "private", exp: nil},
{pidMode: "host", ipcMode: "", exp: nil},
{pidMode: "host", ipcMode: "other", exp: errors.New(`ipc_mode must be "private" or "host", got "other"`)},

{pidMode: "host", ipcMode: "host", exp: nil},
{pidMode: "private", ipcMode: "host", exp: nil},
{pidMode: "", ipcMode: "host", exp: nil},
{pidMode: "other", ipcMode: "host", exp: errors.New(`pid_mode must be "private" or "host", got "other"`)},
} {
require.Equal(t, tc.exp, (&TaskConfig{
ModePID: tc.pidMode,
ModeIPC: tc.ipcMode,
}).validate())
}
}
29 changes: 27 additions & 2 deletions drivers/java/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ var (
"jar_path": hclspec.NewAttr("jar_path", "string", false),
"jvm_options": hclspec.NewAttr("jvm_options", "list(string)", false),
"args": hclspec.NewAttr("args", "list(string)", false),
"pid_mode": hclspec.NewAttr("pid_mode", "string", false),
"ipc_mode": hclspec.NewAttr("ipc_mode", "string", false),
})

// capabilities is returned by the Capabilities RPC and indicates what
Expand Down Expand Up @@ -144,6 +146,25 @@ type TaskConfig struct {
JarPath string `codec:"jar_path"`
JvmOpts []string `codec:"jvm_options"`
Args []string `codec:"args"` // extra arguments to java executable
ModePID string `codec:"pid_mode"`
ModeIPC string `codec:"ipc_mode"`
}

func (tc *TaskConfig) validate() error {
switch tc.ModePID {
case "", executor.IsolationModePrivate, executor.IsolationModeHost:
default:
return fmt.Errorf("pid_mode must be %q or %q, got %q", executor.IsolationModePrivate, executor.IsolationModeHost, tc.ModePID)

}

switch tc.ModeIPC {
case "", executor.IsolationModePrivate, executor.IsolationModeHost:
default:
return fmt.Errorf("ipc_mode must be %q or %q, got %q", executor.IsolationModePrivate, executor.IsolationModeHost, tc.ModeIPC)
}

return nil
}

// TaskState is the state which is encoded in the handle returned in
Expand Down Expand Up @@ -369,6 +390,10 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
return nil, nil, fmt.Errorf("failed to decode driver config: %v", err)
}

if err := driverConfig.validate(); err != nil {
return nil, nil, fmt.Errorf("failed driver config validation: %v", err)
}

if driverConfig.Class == "" && driverConfig.JarPath == "" {
return nil, nil, fmt.Errorf("jar_path or class must be specified")
}
Expand Down Expand Up @@ -425,8 +450,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,
ModePID: executor.IsolationMode(d.config.DefaultModePID, driverConfig.ModePID),
ModeIPC: executor.IsolationMode(d.config.DefaultModeIPC, driverConfig.ModeIPC),
}

ps, err := exec.Launch(execCmd)
Expand Down
4 changes: 2 additions & 2 deletions drivers/shared/executor/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ func (c *grpcExecutorClient) Launch(cmd *ExecCommand) (*ProcessState, error) {
Mounts: drivers.MountsToProto(cmd.Mounts),
Devices: drivers.DevicesToProto(cmd.Devices),
NetworkIsolation: drivers.NetworkIsolationSpecToProto(cmd.NetworkIsolation),
DefaultPidMode: cmd.DefaultModePID,
DefaultIpcMode: cmd.DefaultModeIPC,
DefaultPidMode: cmd.ModePID,
DefaultIpcMode: cmd.ModeIPC,
}
resp, err := c.client.Launch(ctx, req)
if err != nil {
Expand Down
8 changes: 4 additions & 4 deletions drivers/shared/executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ type ExecCommand struct {
// NetworkIsolation is the network isolation configuration.
NetworkIsolation *drivers.NetworkIsolationSpec

// DefaultModePID is the default PID isolation mode
DefaultModePID string
// ModePID is the PID isolation mode (private or host).
ModePID string

// DefaultModeIPC is the default IPC isolation mode
DefaultModeIPC string
// ModeIPC is the IPC isolation mode (private or host).
ModeIPC string
}

// SetWriters sets the writer for the process stdout and stderr. This should
Expand Down
2 changes: 1 addition & 1 deletion drivers/shared/executor/executor_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ func configureIsolation(cfg *lconfigs.Config, command *ExecCommand) error {
cfg.NoPivotRoot = command.NoPivotRoot

// set up default namespaces as configured
cfg.Namespaces = configureNamespaces(command.DefaultModePID, command.DefaultModeIPC)
cfg.Namespaces = configureNamespaces(command.ModePID, command.ModeIPC)

if command.NetworkIsolation != nil {
cfg.Namespaces = append(cfg.Namespaces, lconfigs.Namespace{
Expand Down
8 changes: 4 additions & 4 deletions drivers/shared/executor/executor_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ func TestExecutor_Isolation_PID_and_IPC_hostMode(t *testing.T) {
defer allocDir.Destroy()

execCmd.ResourceLimits = true
execCmd.DefaultModePID = "host" // disable PID namespace
execCmd.DefaultModeIPC = "host" // disable IPC namespace
execCmd.ModePID = "host" // disable PID namespace
execCmd.ModeIPC = "host" // disable IPC namespace

executor := NewExecutorWithIsolation(testlog.HCLogger(t))
defer executor.Shutdown("SIGKILL", 0)
Expand Down Expand Up @@ -170,8 +170,8 @@ func TestExecutor_IsolationAndConstraints(t *testing.T) {
defer allocDir.Destroy()

execCmd.ResourceLimits = true
execCmd.DefaultModePID = "private"
execCmd.DefaultModeIPC = "private"
execCmd.ModePID = "private"
execCmd.ModeIPC = "private"

executor := NewExecutorWithIsolation(testlog.HCLogger(t))
defer executor.Shutdown("SIGKILL", 0)
Expand Down
4 changes: 2 additions & 2 deletions drivers/shared/executor/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ func (s *grpcExecutorServer) Launch(ctx context.Context, req *proto.LaunchReques
Mounts: drivers.MountsFromProto(req.Mounts),
Devices: drivers.DevicesFromProto(req.Devices),
NetworkIsolation: drivers.NetworkIsolationSpecFromProto(req.NetworkIsolation),
DefaultModePID: req.DefaultPidMode,
DefaultModeIPC: req.DefaultIpcMode,
ModePID: req.DefaultPidMode,
ModeIPC: req.DefaultIpcMode,
})

if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions drivers/shared/executor/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,13 @@ func processStateFromProto(pb *proto.ProcessState) (*ProcessState, error) {
Time: timestamp,
}, nil
}

// IsolationMode returns the namespace isolation mode as determined from agent
// plugin configuration and task driver configuration. The task configuration
// takes precedence, if it is configured.
func IsolationMode(plugin, task string) string {
if task != "" {
return task
}
return plugin
}
28 changes: 28 additions & 0 deletions drivers/shared/executor/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package executor

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestUtils_IsolationMode(t *testing.T) {
private := IsolationModePrivate
host := IsolationModeHost
blank := ""

for _, tc := range []struct {
plugin, task, exp string
}{
{plugin: private, task: private, exp: private},
{plugin: private, task: host, exp: host},
{plugin: private, task: blank, exp: private}, // default to private

{plugin: host, task: private, exp: private},
{plugin: host, task: host, exp: host},
{plugin: host, task: blank, exp: host}, // default to host
} {
result := IsolationMode(tc.plugin, tc.task)
require.Equal(t, tc.exp, result)
}
}
40 changes: 40 additions & 0 deletions e2e/isolation/input/exec_host.nomad
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
job "exec" {
datacenters = ["dc1"]
type = "batch"

constraint {
attribute = "${attr.kernel.name}"
value = "linux"
}

group "exec" {
task "exec" {
driver = "exec"

config {
command = "bash"
args = [
"-c", "local/pid.sh"
]
pid_mode = "host"
ipc_mode = "host"
}

template {
data = <<EOF
#!/usr/bin/env bash
echo my pid is $BASHPID
EOF

destination = "local/pid.sh"
perms = "777"
change_mode = "noop"
}

resources {
cpu = 100
memory = 64
}
}
}
}
41 changes: 41 additions & 0 deletions e2e/isolation/input/java_host.nomad
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
job "java_pid" {
datacenters = ["dc1"]
type = "batch"

group "java" {

task "build" {
lifecycle {
hook = "prestart"
sidecar = false
}

driver = "exec"
config {
command = "javac"
args = ["-d", "${NOMAD_ALLOC_DIR}", "local/Pid.java"]
}

template {
destination = "local/Pid.java"
data = <<EOH
public class Pid {
public static void main(String... s) throws Exception {
System.out.println("my pid is " + ProcessHandle.current().pid());
}
}
EOH
}
}

task "pid" {
driver = "java"
config {
class_path = "${NOMAD_ALLOC_DIR}"
class = "Pid"
pid_mode = "host"
ipc_mode = "host"
}
}
}
}
Loading

0 comments on commit 362bcfd

Please sign in to comment.