From 998fd2ece0480e581e013124d0969a1af6305110 Mon Sep 17 00:00:00 2001 From: umohnani8 Date: Tue, 3 Apr 2018 13:37:25 -0400 Subject: Functionality changes to the following flags --group-add --blkio-weight-device --device-read-bps --device-write-bps --device-read-iops --device-write-iops --group-add now supports group names as well as the gid associated with them. All the --device flags work now with moderate changes to the code to support both bps and iops. Added tests for all the flags. Signed-off-by: umohnani8 Closes: #590 Approved by: mheon --- cmd/podman/create.go | 10 ++---- cmd/podman/run.go | 3 +- cmd/podman/run_test.go | 2 +- cmd/podman/spec.go | 78 +++++++++++++++++++++++++++----------------- libpod/container.go | 2 ++ libpod/container_internal.go | 24 ++++++++++++++ libpod/options.go | 11 +++++++ pkg/chrootuser/user.go | 5 +++ pkg/chrootuser/user_linux.go | 32 ++++++++++++++++++ test/e2e/run_test.go | 42 ++++++++++++++++++++++++ 10 files changed, 170 insertions(+), 39 deletions(-) diff --git a/cmd/podman/create.go b/cmd/podman/create.go index a25f31717..8bf61a2ca 100644 --- a/cmd/podman/create.go +++ b/cmd/podman/create.go @@ -86,7 +86,7 @@ type createConfig struct { Entrypoint []string //entrypoint Env map[string]string //env ExposedPorts map[nat.Port]struct{} - GroupAdd []uint32 // group-add + GroupAdd []string // group-add HostAdd []string //add-host Hostname string //hostname Image string @@ -208,6 +208,7 @@ func createCmd(c *cli.Context) error { options = append(options, libpod.WithUser(createConfig.User)) options = append(options, libpod.WithShmDir(createConfig.ShmDir)) options = append(options, libpod.WithShmSize(createConfig.Resources.ShmSize)) + options = append(options, libpod.WithGroups(createConfig.GroupAdd)) ctr, err := runtime.NewContainer(runtimeSpec, options...) if err != nil { return err @@ -406,11 +407,6 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime, imageName string, return nil, errors.Wrapf(err, "invalid value for sysctl") } - groupAdd, err := stringSlicetoUint32Slice(c.StringSlice("group-add")) - if err != nil { - return nil, errors.Wrapf(err, "invalid value for groups provided") - } - if c.String("memory") != "" { memoryLimit, err = units.RAMInBytes(c.String("memory")) if err != nil { @@ -625,7 +621,7 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime, imageName string, Entrypoint: entrypoint, Env: env, //ExposedPorts: ports, - GroupAdd: groupAdd, + GroupAdd: c.StringSlice("group-add"), Hostname: c.String("hostname"), HostAdd: c.StringSlice("add-host"), Image: imageName, diff --git a/cmd/podman/run.go b/cmd/podman/run.go index e9eaf83d2..4966316c5 100644 --- a/cmd/podman/run.go +++ b/cmd/podman/run.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "os" "strings" "github.com/pkg/errors" @@ -10,7 +11,6 @@ import ( "github.com/projectatomic/libpod/libpod/image" "github.com/sirupsen/logrus" "github.com/urfave/cli" - "os" ) var runDescription = "Runs a command in a new container from the given image" @@ -94,6 +94,7 @@ func runCmd(c *cli.Context) error { options = append(options, libpod.WithUser(createConfig.User)) options = append(options, libpod.WithShmDir(createConfig.ShmDir)) options = append(options, libpod.WithShmSize(createConfig.Resources.ShmSize)) + options = append(options, libpod.WithGroups(createConfig.GroupAdd)) // Default used if not overridden on command line diff --git a/cmd/podman/run_test.go b/cmd/podman/run_test.go index 344fdcce5..3baee4615 100644 --- a/cmd/podman/run_test.go +++ b/cmd/podman/run_test.go @@ -101,7 +101,7 @@ func TestPIDsLimit(t *testing.T) { // TestBLKIOWeightDevice verifies the inputed blkio weigh device is correctly defined in the spec func TestBLKIOWeightDevice(t *testing.T) { a := createCLI() - args := []string{"--blkio-weight-device", "/dev/sda:100"} + args := []string{"--blkio-weight-device", "/dev/zero:100"} a.Run(append(cmd, args...)) runtimeSpec, err := getRuntimeSpec(CLI) if err != nil { diff --git a/cmd/podman/spec.go b/cmd/podman/spec.go index 5e98d5b50..14fe3a38a 100644 --- a/cmd/podman/spec.go +++ b/cmd/podman/spec.go @@ -181,9 +181,7 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { g.SetProcessCwd(config.WorkDir) g.SetProcessArgs(config.Command) g.SetProcessTerminal(config.Tty) - for _, gid := range config.GroupAdd { - g.AddProcessAdditionalGid(gid) - } + for key, val := range config.GetAnnotations() { g.AddAnnotation(key, val) } @@ -369,7 +367,7 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { } // BLOCK IO - blkio, err := config.CreateBlockIO() + blkio, err := config.createBlockIO() if err != nil { return nil, errors.Wrapf(err, "error creating block io") } @@ -403,7 +401,12 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { return configSpec, nil } -func (c *createConfig) CreateBlockIO() (*spec.LinuxBlockIO, error) { +const ( + bps = iota + iops +) + +func (c *createConfig) createBlockIO() (*spec.LinuxBlockIO, error) { bio := &spec.LinuxBlockIO{} bio.Weight = &c.Resources.BlkioWeight if len(c.Resources.BlkioWeightDevice) > 0 { @@ -413,7 +416,10 @@ func (c *createConfig) CreateBlockIO() (*spec.LinuxBlockIO, error) { if err != nil { return bio, errors.Wrapf(err, "invalid values for blkio-weight-device") } - wdStat := getStatFromPath(wd.path) + wdStat, err := getStatFromPath(wd.path) + if err != nil { + return bio, errors.Wrapf(err, "error getting stat from path %q", wd.path) + } lwd := spec.LinuxWeightDevice{ Weight: &wd.weight, } @@ -424,28 +430,28 @@ func (c *createConfig) CreateBlockIO() (*spec.LinuxBlockIO, error) { bio.WeightDevice = lwds } if len(c.Resources.DeviceReadBps) > 0 { - readBps, err := makeThrottleArray(c.Resources.DeviceReadBps) + readBps, err := makeThrottleArray(c.Resources.DeviceReadBps, bps) if err != nil { return bio, err } bio.ThrottleReadBpsDevice = readBps } if len(c.Resources.DeviceWriteBps) > 0 { - writeBpds, err := makeThrottleArray(c.Resources.DeviceWriteBps) + writeBpds, err := makeThrottleArray(c.Resources.DeviceWriteBps, bps) if err != nil { return bio, err } bio.ThrottleWriteBpsDevice = writeBpds } if len(c.Resources.DeviceReadIOps) > 0 { - readIOps, err := makeThrottleArray(c.Resources.DeviceReadIOps) + readIOps, err := makeThrottleArray(c.Resources.DeviceReadIOps, iops) if err != nil { return bio, err } bio.ThrottleReadIOPSDevice = readIOps } if len(c.Resources.DeviceWriteIOps) > 0 { - writeIOps, err := makeThrottleArray(c.Resources.DeviceWriteIOps) + writeIOps, err := makeThrottleArray(c.Resources.DeviceWriteIOps, iops) if err != nil { return bio, err } @@ -454,6 +460,35 @@ func (c *createConfig) CreateBlockIO() (*spec.LinuxBlockIO, error) { return bio, nil } +func makeThrottleArray(throttleInput []string, rateType int) ([]spec.LinuxThrottleDevice, error) { + var ( + ltds []spec.LinuxThrottleDevice + t *throttleDevice + err error + ) + for _, i := range throttleInput { + if rateType == bps { + t, err = validateBpsDevice(i) + } else { + t, err = validateIOpsDevice(i) + } + if err != nil { + return []spec.LinuxThrottleDevice{}, err + } + ltdStat, err := getStatFromPath(t.path) + if err != nil { + return ltds, errors.Wrapf(err, "error getting stat from path %q", t.path) + } + ltd := spec.LinuxThrottleDevice{ + Rate: t.rate, + } + ltd.Major = int64(unix.Major(ltdStat.Rdev)) + ltd.Minor = int64(unix.Minor(ltdStat.Rdev)) + ltds = append(ltds, ltd) + } + return ltds, nil +} + // GetAnnotations returns the all the annotations for the container func (c *createConfig) GetAnnotations() map[string]string { a := getDefaultAnnotations() @@ -668,27 +703,10 @@ func (c *createConfig) GetContainerCreateOptions() ([]libpod.CtrCreateOption, er return options, nil } -func getStatFromPath(path string) unix.Stat_t { +func getStatFromPath(path string) (unix.Stat_t, error) { s := unix.Stat_t{} - _ = unix.Stat(path, &s) - return s -} - -func makeThrottleArray(throttleInput []string) ([]spec.LinuxThrottleDevice, error) { - var ltds []spec.LinuxThrottleDevice - for _, i := range throttleInput { - t, err := validateBpsDevice(i) - if err != nil { - return []spec.LinuxThrottleDevice{}, err - } - ltd := spec.LinuxThrottleDevice{} - ltd.Rate = t.rate - ltdStat := getStatFromPath(t.path) - ltd.Major = int64(unix.Major(ltdStat.Rdev)) - ltd.Minor = int64(unix.Major(ltdStat.Rdev)) - ltds = append(ltds, ltd) - } - return ltds, nil + err := unix.Stat(path, &s) + return s, err } // CreatePortBindings iterates ports mappings and exposed ports into a format CNI understands diff --git a/libpod/container.go b/libpod/container.go index cbfa09538..f778933c2 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -201,6 +201,8 @@ type ContainerConfig struct { // User and group to use in the container // Can be specified by name or UID/GID User string `json:"user,omitempty"` + // Additional groups to add + Groups []string `json:"groups, omitempty"` // Namespace Config // IDs of container to share namespaces with diff --git a/libpod/container_internal.go b/libpod/container_internal.go index f3247b1c0..c9454db8a 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "regexp" + "strconv" "strings" "syscall" "time" @@ -956,6 +957,29 @@ func (c *Container) generateSpec() (*spec.Spec, error) { g.SetProcessGID(gid) } + // Add addition groups if c.config.GroupAdd is not empty + if len(c.config.Groups) > 0 { + if !c.state.Mounted { + return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be mounted in order to add additional groups", c.ID()) + } + for _, group := range c.config.Groups { + _, gid, err := chrootuser.GetUser(c.state.Mountpoint, strconv.Itoa(int(g.Spec().Process.User.UID))+":"+group) + if err != nil { + return nil, err + } + g.AddProcessAdditionalGid(uint32(gid)) + } + } + + // Look up and add groups the user belongs to + groups, err := chrootuser.GetAdditionalGroupsForUser(c.state.Mountpoint, uint64(g.Spec().Process.User.UID)) + if err != nil { + return nil, err + } + for _, gid := range groups { + g.AddProcessAdditionalGid(gid) + } + // Add shared namespaces from other containers if c.config.IPCNsCtr != "" { if err := c.addNamespaceContainer(&g, IPCNS, c.config.IPCNsCtr, spec.IPCNamespace); err != nil { diff --git a/libpod/options.go b/libpod/options.go index f9d6cb211..2c51b5834 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -823,3 +823,14 @@ func WithConmonPidFile(path string) CtrCreateOption { return nil } } + +// WithGroups sets additional groups for the container, which are defined by the user +func WithGroups(groups []string) CtrCreateOption { + return func(ctr *Container) error { + if ctr.valid { + return ErrCtrFinalized + } + ctr.config.Groups = groups + return nil + } +} diff --git a/pkg/chrootuser/user.go b/pkg/chrootuser/user.go index 9be386ef9..22ba5ff8a 100644 --- a/pkg/chrootuser/user.go +++ b/pkg/chrootuser/user.go @@ -69,3 +69,8 @@ func GetUser(rootdir, userspec string) (uint32, uint32, error) { } return 0, 0, err } + +// GetAdditionalGroupsForUser returns a list of gids that userid is associated with +func GetAdditionalGroupsForUser(rootdir string, userid uint64) ([]uint32, error) { + return lookupAdditionalGroupsForUIDInContainer(rootdir, userid) +} diff --git a/pkg/chrootuser/user_linux.go b/pkg/chrootuser/user_linux.go index 2baf9ea33..64ff7cef6 100644 --- a/pkg/chrootuser/user_linux.go +++ b/pkg/chrootuser/user_linux.go @@ -88,6 +88,7 @@ type lookupPasswdEntry struct { type lookupGroupEntry struct { name string gid uint64 + user string } func readWholeLine(rc *bufio.Reader) ([]byte, error) { @@ -153,6 +154,7 @@ func parseNextGroup(rc *bufio.Reader) *lookupGroupEntry { return &lookupGroupEntry{ name: fields[0], gid: gid, + user: fields[3], } } @@ -208,6 +210,36 @@ func lookupGroupForUIDInContainer(rootdir string, userid uint64) (username strin return "", 0, user.UnknownUserError(fmt.Sprintf("error looking up user with UID %d", userid)) } +func lookupAdditionalGroupsForUIDInContainer(rootdir string, userid uint64) (gid []uint32, err error) { + // Get the username associated with userid + username, _, err := lookupGroupForUIDInContainer(rootdir, userid) + if err != nil { + return nil, err + } + + cmd, f, err := openChrootedFile(rootdir, "/etc/group") + if err != nil { + return nil, err + } + defer func() { + _ = cmd.Wait() + }() + rc := bufio.NewReader(f) + defer f.Close() + + lookupGroup.Lock() + defer lookupGroup.Unlock() + + grp := parseNextGroup(rc) + for grp != nil { + if strings.Contains(grp.user, username) { + gid = append(gid, uint32(grp.gid)) + } + grp = parseNextGroup(rc) + } + return gid, nil +} + func lookupGroupInContainer(rootdir, groupname string) (gid uint64, err error) { cmd, f, err := openChrootedFile(rootdir, "/etc/group") if err != nil { diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 21c577d9a..a9b2a474f 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -185,6 +185,34 @@ var _ = Describe("Podman run", func() { Expect(session.OutputToString()).To(ContainSubstring("15")) }) + It("podman run device-read-bps test", func() { + session := podmanTest.Podman([]string{"run", "--rm", "--device-read-bps=/dev/zero:1mb", ALPINE, "cat", "/sys/fs/cgroup/blkio/blkio.throttle.read_bps_device"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(ContainSubstring("1048576")) + }) + + It("podman run device-write-bps test", func() { + session := podmanTest.Podman([]string{"run", "--rm", "--device-write-bps=/dev/zero:1mb", ALPINE, "cat", "/sys/fs/cgroup/blkio/blkio.throttle.write_bps_device"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(ContainSubstring("1048576")) + }) + + It("podman run device-read-iops test", func() { + session := podmanTest.Podman([]string{"run", "--rm", "--device-read-iops=/dev/zero:100", ALPINE, "cat", "/sys/fs/cgroup/blkio/blkio.throttle.read_iops_device"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(ContainSubstring("100")) + }) + + It("podman run device-write-iops test", func() { + session := podmanTest.Podman([]string{"run", "--rm", "--device-write-iops=/dev/zero:100", ALPINE, "cat", "/sys/fs/cgroup/blkio/blkio.throttle.write_iops_device"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(ContainSubstring("100")) + }) + It("podman run notify_socket", func() { sock := "/run/sock" os.Setenv("NOTIFY_SOCKET", sock) @@ -258,4 +286,18 @@ var _ = Describe("Podman run", func() { Expect(err).To(BeNil()) }) + It("podman run without group-add", func() { + session := podmanTest.Podman([]string{"run", "--rm", ALPINE, "id"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(Equal("uid=0(root) gid=0(root) groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),20(dialout),26(tape),27(video)")) + }) + + It("podman run with group-add", func() { + session := podmanTest.Podman([]string{"run", "--rm", "--group-add=audio", "--group-add=nogroup", "--group-add=777", ALPINE, "id"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(Equal("uid=0(root) gid=0(root) groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),18(audio),20(dialout),26(tape),27(video),777,65533(nogroup)")) + }) + }) -- cgit v1.2.3-54-g00ecf