From 9d34c3e206808f4c58f9b61d88b94385791fb6e4 Mon Sep 17 00:00:00 2001 From: Ruben Jenster Date: Fri, 30 Apr 2021 20:39:35 +0200 Subject: [PATCH 1/7] cli: Fix container loading in list subcommand. Signed-off-by: Ruben Jenster --- cmd/lxcri/cli.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/lxcri/cli.go b/cmd/lxcri/cli.go index 7756e890..91e1a524 100644 --- a/cmd/lxcri/cli.go +++ b/cmd/lxcri/cli.go @@ -68,8 +68,8 @@ func (app *app) configureLogger() error { return nil } -func (app *app) loadContainer() (*lxcri.Container, error) { - c, err := clxc.Load(app.cfg.ContainerID) +func (app *app) loadContainer(containerID string) (*lxcri.Container, error) { + c, err := clxc.Load(containerID) if err != nil { return c, err } @@ -404,7 +404,7 @@ func doStart(ctxcli *cli.Context) error { } func doStartInternal(ctx context.Context) error { - c, err := clxc.loadContainer() + c, err := clxc.loadContainer(clxc.cfg.ContainerID) if err != nil { return err } @@ -424,7 +424,7 @@ var stateCmd = cli.Command{ } func doState(unused *cli.Context) error { - c, err := clxc.loadContainer() + c, err := clxc.loadContainer(clxc.cfg.ContainerID) if err != nil { return err } @@ -468,7 +468,7 @@ func doKill(ctxcli *cli.Context) error { return fmt.Errorf("invalid signal param %q", sig) } - c, err := clxc.loadContainer() + c, err := clxc.loadContainer(clxc.cfg.ContainerID) if err != nil { return err } @@ -628,7 +628,7 @@ func doExec(ctxcli *cli.Context) error { return err } - c, err := clxc.loadContainer() + c, err := clxc.loadContainer(clxc.cfg.ContainerID) if err != nil { return err } @@ -757,7 +757,7 @@ func doList(ctxcli *cli.Context) (err error) { } func inspectContainer(id string, t *template.Template) error { - c, err := clxc.loadContainer() + c, err := clxc.loadContainer(id) if err != nil { return err } From a61a888a5cf29fc1faa84d56da4f850df2db26bb Mon Sep 17 00:00:00 2001 From: Ruben Jenster Date: Fri, 30 Apr 2021 21:57:25 +0200 Subject: [PATCH 2/7] Fix test directory leak. Signed-off-by: Ruben Jenster --- runtime_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/runtime_test.go b/runtime_test.go index 39aae8f7..fe9838ae 100644 --- a/runtime_test.go +++ b/runtime_test.go @@ -68,10 +68,6 @@ func newConfig(t *testing.T, cmd string, args ...string) *ContainerConfig { require.NoError(t, err) t.Logf("container rootfs: %s", rootfs) - // copy test binary to rootfs - //err = exec.Command("cp", cmd, rootfs).Run() - //require.NoError(t, err) - level, err := log.ParseLevel(logLevel) require.NoError(t, err) @@ -195,7 +191,8 @@ func TestNonEmptyCgroup(t *testing.T) { //time.Sleep(60*time.Second) cfg2 := newConfig(t, "lxcri-test") - defer os.RemoveAll(cfg.Spec.Root.Path) + defer os.RemoveAll(cfg2.Spec.Root.Path) + cfg2.Spec.Linux.CgroupsPath = cfg.Spec.Linux.CgroupsPath if os.Getuid() != 0 { From 32898d7f87e64f7d5e75fcced81451a2da026222 Mon Sep 17 00:00:00 2001 From: Ruben Jenster Date: Fri, 30 Apr 2021 23:30:16 +0200 Subject: [PATCH 3/7] Fix /dev tmpfs mount. Signed-off-by: Ruben Jenster --- create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/create.go b/create.go index 1f1df705..671004a2 100644 --- a/create.go +++ b/create.go @@ -162,7 +162,7 @@ func configureContainer(rt *Runtime, c *Container) error { } newMounts = append(newMounts, m) } - c.Spec.Mounts = append(c.Spec.Mounts, + newMounts = append(newMounts, specs.Mount{ Destination: "/dev", Source: "tmpfs", Type: "tmpfs", Options: []string{"rw", "nosuid", "noexec", "relatime"}, From 479599a70a0b1903863ae8ba39c8823b1455c550 Mon Sep 17 00:00:00 2001 From: Ruben Jenster Date: Fri, 30 Apr 2021 23:31:24 +0200 Subject: [PATCH 4/7] Fix runtime test logging in unprivileged mode. Signed-off-by: Ruben Jenster --- runtime_test.go | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/runtime_test.go b/runtime_test.go index fe9838ae..ecfbc190 100644 --- a/runtime_test.go +++ b/runtime_test.go @@ -79,26 +79,15 @@ func newConfig(t *testing.T, cmd string, args ...string) *ContainerConfig { id := filepath.Base(rootfs) cfg := ContainerConfig{ ContainerID: id, Spec: spec, - Log: log.ConsoleLogger(true, level), + Log: log.ConsoleLogger(true, level), + LogFile: "/dev/stderr", + LogLevel: logLevel, } cfg.Spec.Linux.CgroupsPath = id + ".slice" // use /proc/self/cgroup" cfg.Spec.Mounts = append(cfg.Spec.Mounts, specki.BindMount(cmdAbs, cmdDest), ) - - // FIXME /dev/stderr has perms 600 - // If container process user is not equal to the - // runtime process user then setting lxc log file will fail - // because of missing permissions. - if runAsRuntimeUser(cfg.Spec) { - cfg.LogFile = "/dev/stderr" - } else { - cfg.LogFile = filepath.Join("/tmp", "log") - } - t.Logf("liblxc log output is written to %s", cfg.LogFile) - cfg.LogLevel = logLevel - return &cfg } From 69d4711f4cbaa2ff785d35213318ab74d3d11175 Mon Sep 17 00:00:00 2001 From: Ruben Jenster Date: Fri, 30 Apr 2021 23:32:41 +0200 Subject: [PATCH 5/7] runtime tests: Fail on directory removal failures. Do not ignore errors when removing the container runtime directory or the runtime root directory. Signed-off-by: Ruben Jenster --- runtime_test.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/runtime_test.go b/runtime_test.go index ecfbc190..ddbca157 100644 --- a/runtime_test.go +++ b/runtime_test.go @@ -38,6 +38,11 @@ func mkdirTemp() (string, error) { return os.MkdirTemp(tmpRoot, "lxcri-test") } +func removeAll(t *testing.T, filename string) { + err := os.RemoveAll(filename) + require.NoError(t, err) +} + func newRuntime(t *testing.T) *Runtime { runtimeRoot, err := mkdirTemp() require.NoError(t, err) @@ -94,10 +99,10 @@ func newConfig(t *testing.T, cmd string, args ...string) *ContainerConfig { func TestEmptyNamespaces(t *testing.T) { t.Parallel() rt := newRuntime(t) - defer os.RemoveAll(rt.Root) + defer removeAll(t, rt.Root) cfg := newConfig(t, "lxcri-test") - defer os.RemoveAll(cfg.Spec.Root.Path) + defer removeAll(t, cfg.Spec.Root.Path) // Clearing all namespaces should not work, // since the mount namespace must never be shared with the host. @@ -118,10 +123,10 @@ func TestSharedPIDNamespace(t *testing.T) { t.Skipf("PID namespace sharing is only permitted as root.") } rt := newRuntime(t) - defer os.RemoveAll(rt.Root) + defer removeAll(t, rt.Root) cfg := newConfig(t, "lxcri-test") - defer os.RemoveAll(cfg.Spec.Root.Path) + defer removeAll(t, cfg.Spec.Root.Path) ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) defer cancel() @@ -155,10 +160,10 @@ func TestSharedPIDNamespace(t *testing.T) { func TestNonEmptyCgroup(t *testing.T) { t.Parallel() rt := newRuntime(t) - defer os.RemoveAll(rt.Root) + defer removeAll(t, rt.Root) cfg := newConfig(t, "lxcri-test") - defer os.RemoveAll(cfg.Spec.Root.Path) + defer removeAll(t, cfg.Spec.Root.Path) if os.Getuid() != 0 { cfg.Spec.Linux.UIDMappings = []specs.LinuxIDMapping{ @@ -180,7 +185,7 @@ func TestNonEmptyCgroup(t *testing.T) { //time.Sleep(60*time.Second) cfg2 := newConfig(t, "lxcri-test") - defer os.RemoveAll(cfg2.Spec.Root.Path) + defer removeAll(t, cfg2.Spec.Root.Path) cfg2.Spec.Linux.CgroupsPath = cfg.Spec.Linux.CgroupsPath @@ -218,10 +223,10 @@ func TestRuntimePrivileged(t *testing.T) { } rt := newRuntime(t) - defer os.RemoveAll(rt.Root) + defer removeAll(t, rt.Root) cfg := newConfig(t, "lxcri-test") - defer os.RemoveAll(cfg.Spec.Root.Path) + defer removeAll(t, cfg.Spec.Root.Path) testRuntime(t, rt, cfg) } @@ -242,10 +247,10 @@ func TestRuntimeUnprivileged(t *testing.T) { } rt := newRuntime(t) - defer os.RemoveAll(rt.Root) + defer removeAll(t, rt.Root) cfg := newConfig(t, "lxcri-test") - defer os.RemoveAll(cfg.Spec.Root.Path) + defer removeAll(t, cfg.Spec.Root.Path) // The container UID must have full access to the rootfs. // MkdirTemp sets directory permissions to 0700. @@ -271,10 +276,10 @@ func TestRuntimeUnprivileged(t *testing.T) { func TestRuntimeUnprivileged2(t *testing.T) { t.Parallel() rt := newRuntime(t) - defer os.RemoveAll(rt.Root) + defer removeAll(t, rt.Root) cfg := newConfig(t, "lxcri-test") - defer os.RemoveAll(cfg.Spec.Root.Path) + defer removeAll(t, cfg.Spec.Root.Path) if os.Getuid() != 0 { cfg.Spec.Linux.UIDMappings = []specs.LinuxIDMapping{ From 374d37a5141d13fb8a971d423f07bd7942a180c3 Mon Sep 17 00:00:00 2001 From: Ruben Jenster Date: Sun, 2 May 2021 11:22:15 +0200 Subject: [PATCH 6/7] Update cli flag documentation. Signed-off-by: Ruben Jenster --- cmd/lxcri/cli.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/lxcri/cli.go b/cmd/lxcri/cli.go index 91e1a524..4f0c4d47 100644 --- a/cmd/lxcri/cli.go +++ b/cmd/lxcri/cli.go @@ -157,14 +157,14 @@ func main() { }, &cli.StringFlag{ Name: "root", - Usage: "container runtime root where (logs, init and hook scripts). tmpfs is recommended.", + Usage: "root directory for storage of container runtime state (tmpfs is recommended)", // exec permissions are not required because init is bind mounted into the root Value: "/run/lxcri", Destination: &clxc.Root, }, &cli.BoolFlag{ Name: "systemd-cgroup", - Usage: "enable support for systemd encoded cgroup path", + Usage: "cgroup path in container spec is systemd encoded and must be expanded", Destination: &clxc.SystemdCgroup, }, &cli.StringFlag{ @@ -176,7 +176,7 @@ func main() { }, &cli.StringFlag{ Name: "libexec", - Usage: "directory to load runtime executables from", + Usage: "path to directory that contains the runtime executables", EnvVars: []string{"LXCRI_LIBEXEC"}, Value: libexecDir, Destination: &clxc.LibexecDir, From a2b8f0cab35dc520457b2e8881dae8cc5587ba6c Mon Sep 17 00:00:00 2001 From: Ruben Jenster Date: Thu, 6 May 2021 08:54:52 +0200 Subject: [PATCH 7/7] cli: Use ExecOptions for detached exec. Enable ExecOptions for detached exec. Improve logging for executed command. Signed-off-by: Ruben Jenster --- cmd/lxcri/cli.go | 61 ++++++++++++++++++++++++++---------------------- container.go | 4 ---- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/cmd/lxcri/cli.go b/cmd/lxcri/cli.go index 4f0c4d47..1470908f 100644 --- a/cmd/lxcri/cli.go +++ b/cmd/lxcri/cli.go @@ -634,8 +634,40 @@ func doExec(ctxcli *cli.Context) error { } defer clxc.releaseContainer(c) + opts := lxcri.ExecOptions{} + + if ctxcli.Bool("cgroup") { + opts.Namespaces = append(opts.Namespaces, specs.CgroupNamespace) + } + if ctxcli.Bool("ipc") { + opts.Namespaces = append(opts.Namespaces, specs.IPCNamespace) + } + if ctxcli.Bool("mnt") { + opts.Namespaces = append(opts.Namespaces, specs.MountNamespace) + } + if ctxcli.Bool("net") { + opts.Namespaces = append(opts.Namespaces, specs.NetworkNamespace) + } + if ctxcli.Bool("pid") { + opts.Namespaces = append(opts.Namespaces, specs.PIDNamespace) + } + //if ctxcli.Bool("time") { + // opts.Namespaces = append(opts.Namespaces, specs.TimeNamespace) + //} + if ctxcli.Bool("user") { + opts.Namespaces = append(opts.Namespaces, specs.UserNamespace) + } + if ctxcli.Bool("uts") { + opts.Namespaces = append(opts.Namespaces, specs.UTSNamespace) + } + + c.Log.Info().Str("cmd", procSpec.Args[0]). + Uint32("uid", procSpec.User.UID).Uint32("gid", procSpec.User.GID). + Uints32("groups", procSpec.User.AdditionalGids). + Str("namespaces", fmt.Sprintf("%s", opts.Namespaces)).Msg("execute cmd") + if detach { - pid, err := c.ExecDetached(procSpec, nil) + pid, err := c.ExecDetached(procSpec, &opts) if err != nil { return err } @@ -643,33 +675,6 @@ func doExec(ctxcli *cli.Context) error { return createPidFile(pidFile, pid) } } else { - opts := lxcri.ExecOptions{} - - if ctxcli.Bool("cgroup") { - opts.Namespaces = append(opts.Namespaces, specs.CgroupNamespace) - } - if ctxcli.Bool("ipc") { - opts.Namespaces = append(opts.Namespaces, specs.IPCNamespace) - } - if ctxcli.Bool("mnt") { - opts.Namespaces = append(opts.Namespaces, specs.MountNamespace) - } - if ctxcli.Bool("net") { - opts.Namespaces = append(opts.Namespaces, specs.NetworkNamespace) - } - if ctxcli.Bool("pid") { - opts.Namespaces = append(opts.Namespaces, specs.PIDNamespace) - } - //if ctxcli.Bool("time") { - // opts.Namespaces = append(opts.Namespaces, specs.TimeNamespace) - //} - if ctxcli.Bool("user") { - opts.Namespaces = append(opts.Namespaces, specs.UserNamespace) - } - if ctxcli.Bool("uts") { - opts.Namespaces = append(opts.Namespaces, specs.UTSNamespace) - } - status, err := c.Exec(procSpec, &opts) if err != nil { return err diff --git a/container.go b/container.go index 627cd3de..b4ee8c66 100644 --- a/container.go +++ b/container.go @@ -389,10 +389,6 @@ func (c *Container) ExecDetached(proc *specs.Process, execOpts *ExecOptions) (pi return 0, errorf("failed to create attach options: %w", err) } - c.Log.Info().Strs("args", proc.Args). - Int("uid", opts.UID).Int("gid", opts.GID). - Ints("groups", opts.Groups).Msg("execute cmd") - pid, err = c.LinuxContainer.RunCommandNoWait(proc.Args, opts) if err != nil { return pid, errorf("failed to run exec cmd detached: %w", err)