Skip to content

add mount validation function in runtimetest #7

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.

Sign up for GitHub

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

Closed
wangkirin wants to merge 1 commit into from
Closed

add mount validation function in runtimetest #7

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions cmd/runtimetest/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,39 @@ func validateSysctls(spec *specs.LinuxSpec, rspec *specs.LinuxRuntimeSpec) error
return nil
}

func validateMount(spec *specs.LinuxSpec, rspec *specs.LinuxRuntimeSpec) error {
fmt.Println("validating mount")
//read the /proc/mount file and covert to map[path]spec.mount
Copy link
Contributor

@wking wking Jan 21, 2016

Choose a reason for hiding this comment

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

I think this approach may be too simplistic.

Instead of writing something that generically validates any bundle's mounts, it's probably easier and effective enough to write a few bundles that make specific mounts and then check for those after launching the bundle (e.g. does the container's /file match the bundle's roofs/file? Is / read-only or not? …).

Copy link
Contributor Author

@wangkirin wangkirin Jan 25, 2016

Choose a reason for hiding this comment

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

@wking thank you for your advice!

  1. about required mounts check, I think it's more appropriate to finish it in bunlde validator , not in runtime test. That means if you do not load these required mounts, your bundle is considered illegal
  2. I agree with your idea about not validating any bundle's mounts, but there is a problem that in current testing framework , we use a single bundle to test all specs items , so we need to cover the restrictions defined in specs, like type in mounts [1]

[1]https://github.com/opencontainers/specs/blob/master/runtime-config.md

Copy link
Contributor

@wking wking Jan 25, 2016

Choose a reason for hiding this comment

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

On Mon, Jan 25, 2016 at 03:18:48AM -0800, Wang Qilin wrote:

  1. about required mounts check, I think it's more appropriate to
    finish it in bunlde validator , not in runtime test. That means if
    you do not load these required mounts, your bundle is considered
    illegal

As I read opencontainers/runtime-spec#164, the runtime must supply those
mounts regardless of the user config (unless we specify an opt-out
mechanism, which we don't have yet). So it's a runtime-author problem
(not a bundle-author problem) if they're missing.

  1. I agree with your idea about not validating any bundle's mounts,
    but there is a problem that in current testing framework , we use a
    single bundle to test all specs items , so we need to cover the
    restrictions defined in specs, like type in mounts [1]

It doesn't seem possible to test different config permutations with a
single bundle. You could probably do it with a single rootfs and a
bunch of per-test configs (which was how I understood this repository
to work). If setting up a bundle to bind-mount a file from the host
into the container is too difficult to do, we should probably adjust
ocitools to make it easier ;).

Copy link
Contributor

@zenlint zenlint Jan 30, 2016

Choose a reason for hiding this comment

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

@wking @wangkirin
To [1],
When we test runtime's compliace, the input is the bundles, output is the runtime can have the behavior we expected or not with the input. So, agree with @wking

To [2],
That is what we need to do in ocitools, actually, we should have a scheduler and a cases manager in ocitools runtimetest/validate command.

A scheduler is whom to schedule the cases(input as bundles here).

A cases manager to manage cases, like TestUnit in PR "Add initial version of runtimetest command: #9". Through adding some elements into cases manger(like TestUnit), scheduler can easily schedule bundles to run on the platform/machine required.

Anyway, let us do effort step by step to reach the destination.

mntori, _ := ioutil.ReadFile("/proc/mounts")
mnt := bytes.Split(mntori, []byte{'\n'})
containermnts := make(map[string]specs.Mount)
for _, row := range mnt {
col := bytes.Split(row, []byte{' '})
if len(col) != 6 {
break
}
ops := strings.Split(string(col[3]), ",")
containermnts[string(col[1])] = specs.Mount{string(col[2]), "", ops}
}
//read config.json and runtime.json and compare the mount
mntpts := spec.Mounts
mnts := rspec.Mounts
for _, mntpt := range mntpts {
mnt := mnts[mntpt.Name]
mntcotainer, exsist := containermnts[mntpt.Path]
if !exsist {
return fmt.Errorf("mountpoint name:%v, path: %v doesn't exsist", mntpt.Name, mntpt.Path)
}
if strings.EqualFold(mnt.Type, "cgroup") && !strings.EqualFold(mntcotainer.Type, "tmpfs") {
return fmt.Errorf("cgroup filesystem error")
}
if !strings.EqualFold(mnt.Type, "cgroup") && !strings.EqualFold(mnt.Type, mntcotainer.Type) {
return fmt.Errorf("mount.Type expected: %v, actual: %v", mnt.Type, mntcotainer.Type)
}
}
return nil
}

func main() {
spec, rspec, err := loadSpecConfig()
if err != nil {
Expand All @@ -212,6 +245,7 @@ func main() {

validations := []validation{
validateProcess,
validateMount,
validateCapabilities,
validateHostname,
validateRlimits,
Expand Down