From 7994443c15ec1d8a7773bf37c1379b5e67caeb88 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 19 Jan 2021 13:28:18 +0900 Subject: [PATCH 1/2] integration: port TestRunModePIDHost from CLI test to API test Signed-off-by: Akihiro Suda --- integration-cli/docker_cli_run_test.go | 22 ----------- integration/container/pidmode_linux_test.go | 38 +++++++++++++++++++ .../container/run_cgroupns_linux_test.go | 15 +------- integration/internal/container/ns.go | 21 ++++++++++ 4 files changed, 61 insertions(+), 35 deletions(-) create mode 100644 integration/container/pidmode_linux_test.go create mode 100644 integration/internal/container/ns.go diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 2fdc887188..7e81c2fbfb 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -2426,28 +2426,6 @@ func (s *DockerSuite) TestContainerNetworkMode(c *testing.T) { } } -func (s *DockerSuite) TestRunModePIDHost(c *testing.T) { - // Not applicable on Windows as uses Unix-specific capabilities - testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux, NotUserNamespace) - - hostPid, err := os.Readlink("/proc/1/ns/pid") - if err != nil { - c.Fatal(err) - } - - out, _ := dockerCmd(c, "run", "--pid=host", "busybox", "readlink", "/proc/self/ns/pid") - out = strings.Trim(out, "\n") - if hostPid != out { - c.Fatalf("PID different with --pid=host %s != %s\n", hostPid, out) - } - - out, _ = dockerCmd(c, "run", "busybox", "readlink", "/proc/self/ns/pid") - out = strings.Trim(out, "\n") - if hostPid == out { - c.Fatalf("PID should be different without --pid=host %s == %s\n", hostPid, out) - } -} - func (s *DockerSuite) TestRunModeUTSHost(c *testing.T) { // Not applicable on Windows as uses Unix-specific capabilities testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux) diff --git a/integration/container/pidmode_linux_test.go b/integration/container/pidmode_linux_test.go new file mode 100644 index 0000000000..a83b527a27 --- /dev/null +++ b/integration/container/pidmode_linux_test.go @@ -0,0 +1,38 @@ +package container // import "github.com/docker/docker/integration/container" + +import ( + "context" + "os" + "testing" + "time" + + "github.com/docker/docker/integration/internal/container" + "gotest.tools/v3/assert" + "gotest.tools/v3/poll" + "gotest.tools/v3/skip" +) + +func TestPidHost(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + skip.If(t, testEnv.IsRemoteDaemon()) + skip.If(t, testEnv.IsRootless, "https://github.com/moby/moby/issues/41457") + + hostPid, err := os.Readlink("/proc/1/ns/pid") + assert.NilError(t, err) + + defer setupTest(t)() + client := testEnv.APIClient() + ctx := context.Background() + + cID := container.Run(ctx, t, client, func(c *container.TestContainerConfig) { + c.HostConfig.PidMode = "host" + }) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + cPid := container.GetContainerNS(ctx, t, client, cID, "pid") + assert.Assert(t, hostPid == cPid) + + cID = container.Run(ctx, t, client) + poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) + cPid = container.GetContainerNS(ctx, t, client, cID, "pid") + assert.Assert(t, hostPid != cPid) +} diff --git a/integration/container/run_cgroupns_linux_test.go b/integration/container/run_cgroupns_linux_test.go index ffdeb5513a..3112190032 100644 --- a/integration/container/run_cgroupns_linux_test.go +++ b/integration/container/run_cgroupns_linux_test.go @@ -2,7 +2,6 @@ package container // import "github.com/docker/docker/integration/container" import ( "context" - "strings" "testing" "time" @@ -11,20 +10,10 @@ import ( "github.com/docker/docker/integration/internal/requirement" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" - is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/poll" "gotest.tools/v3/skip" ) -// Gets the value of the cgroup namespace for pid 1 of a container -func containerCgroupNamespace(ctx context.Context, t *testing.T, client *client.Client, cID string) string { - res, err := container.Exec(ctx, client, cID, []string{"readlink", "/proc/1/ns/cgroup"}) - assert.NilError(t, err) - assert.Assert(t, is.Len(res.Stderr(), 0)) - assert.Equal(t, 0, res.ExitCode) - return strings.TrimSpace(res.Stdout()) -} - // Bring up a daemon with the specified default cgroup namespace mode, and then create a container with the container options func testRunWithCgroupNs(t *testing.T, daemonNsMode string, containerOpts ...func(*container.TestContainerConfig)) (string, string) { d := daemon.New(t, daemon.WithDefaultCgroupNamespaceMode(daemonNsMode)) @@ -38,7 +27,7 @@ func testRunWithCgroupNs(t *testing.T, daemonNsMode string, containerOpts ...fun poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) daemonCgroup := d.CgroupNamespace(t) - containerCgroup := containerCgroupNamespace(ctx, t, client, cID) + containerCgroup := container.GetContainerNS(ctx, t, client, cID, "cgroup") return containerCgroup, daemonCgroup } @@ -147,7 +136,7 @@ func TestCgroupNamespacesRunOlderClient(t *testing.T) { poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) daemonCgroup := d.CgroupNamespace(t) - containerCgroup := containerCgroupNamespace(ctx, t, client, cID) + containerCgroup := container.GetContainerNS(ctx, t, client, cID, "cgroup") if testEnv.DaemonInfo.CgroupVersion != "2" { assert.Assert(t, daemonCgroup == containerCgroup) } else { diff --git a/integration/internal/container/ns.go b/integration/internal/container/ns.go new file mode 100644 index 0000000000..bda06dd74c --- /dev/null +++ b/integration/internal/container/ns.go @@ -0,0 +1,21 @@ +package container + +import ( + "context" + "strings" + "testing" + + "github.com/docker/docker/client" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +// GetContainerNS gets the value of the specified namespace of a container +func GetContainerNS(ctx context.Context, t *testing.T, client client.APIClient, cID, nsName string) string { + t.Helper() + res, err := Exec(ctx, client, cID, []string{"readlink", "/proc/self/ns/" + nsName}) + assert.NilError(t, err) + assert.Assert(t, is.Len(res.Stderr(), 0)) + assert.Equal(t, 0, res.ExitCode) + return strings.TrimSpace(res.Stdout()) +} From 227687f2efb848f06e756bc0f6b470b05f7d502a Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 19 Jan 2021 14:12:43 +0900 Subject: [PATCH 2/2] rootless: support --pid=host Fix #41457 related: https://github.com/containers/podman/blob/v3.0.0-rc1/pkg/specgen/generate/oci.go#L248-L257 Signed-off-by: Akihiro Suda --- integration/container/pidmode_linux_test.go | 1 - rootless/specconv/specconv_linux.go | 61 +++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/integration/container/pidmode_linux_test.go b/integration/container/pidmode_linux_test.go index a83b527a27..003192ac05 100644 --- a/integration/container/pidmode_linux_test.go +++ b/integration/container/pidmode_linux_test.go @@ -15,7 +15,6 @@ import ( func TestPidHost(t *testing.T) { skip.If(t, testEnv.DaemonInfo.OSType != "linux") skip.If(t, testEnv.IsRemoteDaemon()) - skip.If(t, testEnv.IsRootless, "https://github.com/moby/moby/issues/41457") hostPid, err := os.Readlink("/proc/1/ns/pid") assert.NilError(t, err) diff --git a/rootless/specconv/specconv_linux.go b/rootless/specconv/specconv_linux.go index 9416076fb7..5b461bb157 100644 --- a/rootless/specconv/specconv_linux.go +++ b/rootless/specconv/specconv_linux.go @@ -2,7 +2,10 @@ package specconv // import "github.com/docker/docker/rootless/specconv" import ( "io/ioutil" + "os" + "path" "strconv" + "strings" specs "github.com/opencontainers/runtime-spec/specs-go" ) @@ -10,6 +13,7 @@ import ( // ToRootless converts spec to be compatible with "rootless" runc. // * Remove non-supported cgroups // * Fix up OOMScoreAdj +// * Fix up /proc if --pid=host // // v2Controllers should be non-nil only if running with v2 and systemd. func ToRootless(spec *specs.Spec, v2Controllers []string) error { @@ -70,5 +74,62 @@ func toRootless(spec *specs.Spec, v2Controllers []string, currentOOMScoreAdj int if spec.Process.OOMScoreAdj != nil && *spec.Process.OOMScoreAdj < currentOOMScoreAdj { *spec.Process.OOMScoreAdj = currentOOMScoreAdj } + + // Fix up /proc if --pid=host + pidHost, err := isPidHost(spec) + if err != nil { + return err + } + if !pidHost { + return nil + } + return bindMountHostProcfs(spec) +} + +func isPidHost(spec *specs.Spec) (bool, error) { + for _, ns := range spec.Linux.Namespaces { + if ns.Type == specs.PIDNamespace { + if ns.Path == "" { + return false, nil + } + pidNS, err := os.Readlink(ns.Path) + if err != nil { + return false, err + } + selfPidNS, err := os.Readlink("/proc/self/ns/pid") + if err != nil { + return false, err + } + return pidNS == selfPidNS, nil + } + } + return true, nil +} + +func bindMountHostProcfs(spec *specs.Spec) error { + // Replace procfs mount with rbind + // https://github.com/containers/podman/blob/v3.0.0-rc1/pkg/specgen/generate/oci.go#L248-L257 + for i, m := range spec.Mounts { + if path.Clean(m.Destination) == "/proc" { + newM := specs.Mount{ + Destination: "/proc", + Type: "bind", + Source: "/proc", + Options: []string{"rbind", "nosuid", "noexec", "nodev"}, + } + spec.Mounts[i] = newM + } + } + + // Remove ReadonlyPaths for /proc/* + newROP := spec.Linux.ReadonlyPaths[:0] + for _, s := range spec.Linux.ReadonlyPaths { + s = path.Clean(s) + if !strings.HasPrefix(s, "/proc/") { + newROP = append(newROP, s) + } + } + spec.Linux.ReadonlyPaths = newROP + return nil }