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 configuration to restore previous PID/IPC namespace behavior #9982

Merged
merged 8 commits into from
Feb 8, 2021

Conversation

shoenig
Copy link
Member

@shoenig shoenig commented Feb 5, 2021

This PR adds default_pid_mode and default_ipc_mode options to the exec and java
task drivers. By default these will default to "private" mode, enabling PID and
IPC isolation for tasks. Setting them to "host" mode disables isolation. Doing
so is not recommended, but may be necessary to support legacy job configurations.

Closes #9969

…space behavior.

This PR adds default_pid_mode and default_ipc_mode options to the exec and java
task drivers. By default these will default to "private" mode, enabling PID and
IPC isolation for tasks. Setting them to "host" mode disables isolation. Doing
so is not recommended, but may be necessary to support legacy job configurations.

Closes #9969
@shoenig
Copy link
Member Author

shoenig commented Feb 5, 2021

spot check exec

exec.nomad

job "exec" {
  datacenters = ["dc1"]

  type = "batch"

  group "catter" {
    task "cat" {
      driver = "exec"
      config {
	command = "/usr/bin/cat"
	args = ["/proc/self/stat"]
      }
    }
  }
}

private mode

nomad.hcl

plugin "exec" {
  config {
    default_pid_mode = "private"
    default_ipc_mode = "private"
  }
}

/proc/self/stat indicates PID 1, i.e. in a namespace

[x52 nsiso-driver (master)] $ nomad job run exec.nomad
==> Monitoring evaluation "bc3a4e30"
    Evaluation triggered by job "exec"
    Allocation "7963d179" created: node "02912ff7", group "catter"
==> Monitoring evaluation "bc3a4e30"
    Evaluation status changed: "pending" -> "complete"
==> Evaluation "bc3a4e30" finished with status "complete"
[x52 nsiso-driver (master)] $ nomad alloc logs 796
1 (cat) R 0 1 1 0 -1 4194560 1701 0 0 0 0 1 0 0 20 0 1 0 16268147 8425472 146 18446744073709551615 94617221357568 94617221375025 140735364814288 0 0 0 0 0 0 0 0 0 17 3 0 0 0 0 0 94617221393040 94617221394624 94617247051776 140735364822617 140735364822646 140735364822646 140735364825067 0

host mode

nomad.hcl

plugin "exec" {
  config {
    default_pid_mode = "host"
    default_ipc_mode = "host"
  }
}

/proc/self/stat indicates PID 214264, i.e. sharing host PID namespace

[x52 nsiso-driver (master)] $ nomad job run exec.nomad
==> Monitoring evaluation "75d89109"
    Evaluation triggered by job "exec"
    Allocation "07d5cc15" created: node "063de9b8", group "catter"
==> Monitoring evaluation "75d89109"
    Evaluation status changed: "pending" -> "complete"
==> Evaluation "75d89109" finished with status "complete"
[x52 nsiso-driver (master)] $ nomad alloc logs 07d
214264 (cat) R 214252 214264 214264 0 -1 4194560 1670 0 0 0 1 0 0 0 20 0 1 0 16277382 8425472 130 18446744073709551615 93974594875392 93974594892849 140729171405200 0 0 0 0 0 0 0 0 0 17 3 0 0 0 0 0 93974594910864 93974594912448 93974621917184 140729171408473 140729171408502 140729171408502 140729171410923 0

@shoenig
Copy link
Member Author

shoenig commented Feb 5, 2021

spot check java

PID.java

import java.io.File;
import java.io.FileInputStream;

public class PID {
    public static void main(String[] args) throws Exception {
	int pid = Integer.parseInt(new File("/proc/self").getCanonicalFile().getName());
	System.out.println(pid);
    }
}

java.nomad

job "java" {
  datacenters = ["dc1"]

  type = "batch"

  group "printer" {
    task "print" {
      driver = "java"
      config {
	class = "PID"
	class_path = "local/"
      }

      artifact {
	source = "http://localhost:8000/PID.class"
      }
    }
  }
}

compile & serve class file

[x52 nsiso-driver (master)] $ javac PID.java
[x52 nsiso-driver (master)] $ python3 -m http.server 8000
Serving HTTP on 0.0.0.0 port 8000 (http://0.0.0.0:8000/) ...

private mode

nomad.hcl

plugin "java" {                                                                                                                                                               
  config {                                                                                                                                                                    
    default_pid_mode = "private"                                                                                                                                              
    default_ipc_mode = "private"                                                                                                                                              
  }                                                                                                                                                                           
} 

PID 1 indicates isolated namespace

[x52 nsiso-driver (master)] $ nomad job run java.nomad
==> Monitoring evaluation "3e0fa2b3"
    Evaluation triggered by job "java"
    Allocation "d9cc2e91" created: node "25facb9e", group "printer"
==> Monitoring evaluation "3e0fa2b3"
    Evaluation status changed: "pending" -> "complete"
==> Evaluation "3e0fa2b3" finished with status "complete"
[x52 nsiso-driver (master)] $ nomad alloc logs d9c
1

host mode

nomad.hcl

plugin "java" {
  config {
    default_pid_mode = "host"
    default_ipc_mode = "host"
  }
}

PID of 215856 indicates using host PID namespace

[x52 nsiso-driver (master)] $ nomad job run java.nomad
==> Monitoring evaluation "dd73c70a"
    Evaluation triggered by job "java"
    Allocation "722372e3" created: node "aa4179ca", group "printer"
==> Monitoring evaluation "dd73c70a"
    Evaluation status changed: "pending" -> "complete"
==> Evaluation "dd73c70a" finished with status "complete"
[x52 nsiso-driver (master)] $ nomad alloc logs 722
215856

@shoenig shoenig marked this pull request as ready for review February 5, 2021 22:26

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

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 key default_pid_mode?

{Type: lconfigs.NEWPID},
{Type: lconfigs.NEWIPC},
}
// setup default namespaces as configured
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// setup default namespaces as configured
// set up default namespaces as configured

@@ -34,6 +34,12 @@ const (
// ExecutorVersionPre0_9 is the version of executor use prior to the release
// of 0.9.x
ExecutorVersionPre0_9 = "1.1.0"

// IsoModePrivate represents the private isolation mode for a namespace
IsoModePrivate = "private"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel like "Iso" is a common short form of Isolation/Isolate in the codebase; would it be possible to expand this to IsolationModePrivate?

NetworkIsolation *drivers.NetworkIsolationSpec

// DefaultModePID is the default PID isolation mode (private or host).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the "(private or host)" part of this comment should be removed; if there was a future mode other than private or host, this comment would become out of date, but nobody would know.

Also, comments aren't meant to have full stops at the end.

Copy link
Contributor

@cgbaker cgbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks, @shoenig

@@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we doing this now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only care that we do not shadow names. Whether we use require.Blah(t, ... or rename require as a variable or something else, I don't have much of an opinion.

website/content/docs/drivers/exec.mdx Outdated Show resolved Hide resolved
website/content/docs/drivers/exec.mdx Outdated Show resolved Hide resolved
website/content/docs/drivers/java.mdx Outdated Show resolved Hide resolved
website/content/docs/drivers/java.mdx Outdated Show resolved Hide resolved
website/content/docs/drivers/java.mdx Outdated Show resolved Hide resolved
website/content/docs/drivers/exec.mdx Outdated Show resolved Hide resolved
Co-authored-by: Chris Baker <1675087+cgbaker@users.noreply.github.com>
shoenig and others added 5 commits February 8, 2021 10:52
Co-authored-by: Chris Baker <1675087+cgbaker@users.noreply.github.com>
Co-authored-by: Chris Baker <1675087+cgbaker@users.noreply.github.com>
Co-authored-by: Chris Baker <1675087+cgbaker@users.noreply.github.com>
Co-authored-by: Chris Baker <1675087+cgbaker@users.noreply.github.com>
Co-authored-by: Chris Baker <1675087+cgbaker@users.noreply.github.com>
@krishicks
Copy link
Contributor

p.s. I really like the spot checks. So nice to see that in a PR!

@shoenig shoenig merged commit 6c376fc into master Feb 8, 2021
@shoenig shoenig deleted the f-nsiso-driver branch February 8, 2021 17:19
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuration for task namespace isolation in driver plugin
3 participants