Skip to content
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 task configuration to restore previous PID/IPC isolation behavior #9990

Merged
merged 1 commit into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)] [[GH-9990](https://github.com/hashicorp/nomad/pull/9990)]

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