From 401f3b65e6434c09218c123f1eac997254bda8d3 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Wed, 30 Aug 2017 16:27:58 -0700 Subject: [PATCH] cmd/runtimetest/main.go: Unify validateMountsExist and validateMountsOrder Also increase the error message detail and continue through the remaining mounts instead of breaking on the first missing/misordered mount. Based on previous discussion in [1,2]. With this commit, a configuration like: "mounts": [ { "destination": "/tmp", "type": "tmpfs", "source": "none" }, { "destination": "/tmp", "type": "tmpfs", "source": "none" }, { "destination": "/dev", "type": "devtmpfs", "source": "devtmpfs" } ] and mountinfo like: $ grep -n '/dev \|/tmp ' /proc/self/mountinfo 2:19 17 0:6 / /dev rw,nosuid,relatime - devtmpfs devtmpfs rw,size=10240k,nr_inodes=2043951,mode=755 25:41 17 0:38 / /tmp rw,relatime - tmpfs none rw will generate errors like: * mounts[1] {/tmp tmpfs none [nosuid strictatime mode=755 size=65536k]} does not exist * mounts[2] {/dev devtmpfs devtmpfs [rw size=10240k nr_inodes=2043951 mode=755]} is system mount 1, while mounts[0] {/tmp tmpfs none [nosuid strictatime mode=755 size=65536k]} is system mount 24 Grep reports 2 and 25 because it's counting from one, and runtimetest reports 1 and 24 because it's counting from zero. Before this commit, the error was just: * Mounts[1] /tmp is not mounted in order [1]: https://github.com/opencontainers/runtime-tools/pull/444#discussion_r134346130 [2]: https://github.com/opencontainers/runtime-tools/pull/444#issuecomment-324122061 Signed-off-by: W. Trevor King --- cmd/runtimetest/main.go | 137 +++++++++++++++++----------------------- 1 file changed, 57 insertions(+), 80 deletions(-) diff --git a/cmd/runtimetest/main.go b/cmd/runtimetest/main.go index 858f216bd..f111973ae 100644 --- a/cmd/runtimetest/main.go +++ b/cmd/runtimetest/main.go @@ -576,113 +576,94 @@ func validateGIDMappings(spec *rspec.Spec) error { return validateIDMappings(spec.Linux.GIDMappings, "/proc/self/gid_map", "linux.gidMappings") } -func mountMatch(configMount rspec.Mount, sysMount rspec.Mount) error { - if filepath.Clean(configMount.Destination) != sysMount.Destination { - return fmt.Errorf("mount destination expected: %v, actual: %v", configMount.Destination, sysMount.Destination) +func mountMatch(configMount rspec.Mount, sysMount *mount.Info) error { + sys := rspec.Mount{ + Destination: sysMount.Mountpoint, + Type: sysMount.Fstype, + Source: sysMount.Source, } - if configMount.Type != sysMount.Type { - return fmt.Errorf("mount %v type expected: %v, actual: %v", configMount.Destination, configMount.Type, sysMount.Type) + if filepath.Clean(configMount.Destination) != sys.Destination { + return fmt.Errorf("mount destination expected: %v, actual: %v", configMount.Destination, sys.Destination) } - if filepath.Clean(configMount.Source) != sysMount.Source { - return fmt.Errorf("mount %v source expected: %v, actual: %v", configMount.Destination, configMount.Source, sysMount.Source) + if configMount.Type != sys.Type { + return fmt.Errorf("mount %v type expected: %v, actual: %v", configMount.Destination, configMount.Type, sys.Type) + } + + if filepath.Clean(configMount.Source) != sys.Source { + return fmt.Errorf("mount %v source expected: %v, actual: %v", configMount.Destination, configMount.Source, sys.Source) } return nil } -func validateMountsExist(spec *rspec.Spec) error { +func validateMounts(spec *rspec.Spec) error { + if runtime.GOOS == "windows" { + logrus.Warnf("mounts validation not yet implemented for OS %q", runtime.GOOS) + return nil + } + mountInfos, err := mount.GetMounts() if err != nil { return err } - mountsMap := make(map[string][]rspec.Mount) - for _, mountInfo := range mountInfos { - m := rspec.Mount{ - Destination: mountInfo.Mountpoint, - Type: mountInfo.Fstype, - Source: mountInfo.Source, - } - mountsMap[mountInfo.Mountpoint] = append(mountsMap[mountInfo.Mountpoint], m) - } - - for _, configMount := range spec.Mounts { + var mountErrs error + var configSys = make(map[int]int) + var consumedSys = make(map[int]bool) + highestMatchedConfig := -1 + var j = 0 + for i, configMount := range spec.Mounts { if configMount.Type == "bind" || configMount.Type == "rbind" { // TODO: add bind or rbind check. continue } found := false - for _, sysMount := range mountsMap[filepath.Clean(configMount.Destination)] { + for k, sysMount := range mountInfos[j:] { if err := mountMatch(configMount, sysMount); err == nil { found = true + j += k + 1 + configSys[i] = j - 1 + consumedSys[j-1] = true + if jMax, ok := configSys[highestMatchedConfig]; !ok || jMax < j-1 { + highestMatchedConfig = i + } break } } if !found { - return fmt.Errorf("Expected mount %v does not exist", configMount) - } - } - - return nil -} - -func validateMountsOrder(spec *rspec.Spec) error { - if runtime.GOOS == "windows" { - logrus.Warnf("mounts order validation not yet implemented for OS %q", runtime.GOOS) - return nil - } - - mountInfos, err := mount.GetMounts() - if err != nil { - return err - } - - type mountOrder struct { - Order int - Root string - Dest string - Source string - } - mountsMap := make(map[string][]mountOrder) - for i, mountInfo := range mountInfos { - m := mountOrder{ - Order: i, - Root: mountInfo.Root, - Dest: mountInfo.Mountpoint, - Source: mountInfo.Source, - } - mountsMap[mountInfo.Mountpoint] = append(mountsMap[mountInfo.Mountpoint], m) - } - current := -1 - for i, configMount := range spec.Mounts { - mounts := mountsMap[configMount.Destination] - if len(mounts) == 0 { - return fmt.Errorf("Mounts[%d] %s is not mounted in order", i, configMount.Destination) - } - for j, mount := range mounts { - source := mount.Source - for _, option := range configMount.Options { - if option == "bind" || option == "rbind" { - source = mount.Root - break + if j > 0 { + for k, sysMount := range mountInfos[:j-1] { + if _, ok := consumedSys[k]; ok { + continue + } + if err := mountMatch(configMount, sysMount); err == nil { + found = true + configSys[i] = k + break + } } } - if source == configMount.Source { - if current > mount.Order { - return fmt.Errorf("Mounts[%d] %s is not mounted in order", i, configMount.Destination) - } - current = mount.Order - // in order to deal with dup mount elements - mountsMap[configMount.Destination] = append(mountsMap[configMount.Destination][:j], mountsMap[configMount.Destination][j+1:]...) - break + if found { + mountErrs = multierror.Append( + mountErrs, + fmt.Errorf( + "mounts[%d] %v is system mount %d, while mounts[%d] %v is system mount %d", + i, + configMount, + configSys[i], + highestMatchedConfig, + spec.Mounts[highestMatchedConfig], + configSys[highestMatchedConfig])) + } else { + mountErrs = multierror.Append(mountErrs, fmt.Errorf("mounts[%d] %v does not exist", i, configMount)) } } } - return nil + return mountErrs } func run(context *cli.Context) error { @@ -711,13 +692,9 @@ func run(context *cli.Context) error { description: "hostname", }, { - test: validateMountsExist, + test: validateMounts, description: "mounts", }, - { - test: validateMountsOrder, - description: "mounts order", - }, } linuxValidations := []validation{