mrunalp / ocitools

Various tools for OCI
Other
17 stars 7 forks source link

Support RKT in validating #12

Open zenlint opened 8 years ago

zenlint commented 8 years ago

If we want to support rkt in validation tool, I find out that some of the container end test programms should be different from the runc. For example, for vallidate specs.Process, we use command below,

cmdlineBytes, err := ioutil.ReadFile("/proc/1/cmdline")
    if err != nil {
        return err
    }

    args := strings.Split(string(bytes.Trim(cmdlineBytes, "\x00")), " ")
    if len(args) != len(spec.Process.Args) {
        return fmt.Errorf("Process arguments expected: %v, actual: %v")
    }
    for i, a := range args {
        if a != spec.Process.Args[i] {
            return fmt.Errorf("Process arguments expected: %v, actual: %v", a, spec.Process.Args[i])
        }
    }

However, the /proc/1/cmdline in rkt contains systemd args, so the code logic in validating specs.Process.Args should be different with runc.

List my suggestion as below, First, split the validation programme into seperate samples, such as,

func main() {
    app := cli.NewApp()
    app.Name = "oci-runtimeValidate"
    app.Version = "0.0.1"
    app.Usage = "Utilities for OCI runtime validation"
    app.EnableBashCompletion = true

    app.Commands = []cli.Command{
        {
            Name:    "validateOverall",
            Aliases: []string{"va"},
            Usage:   "Validate overall specs",
            Action: func(c *cli.Context) {
                validations := []validation{
                    validateHooks,
                    validateSelinux,
                    validateSeccomp,
                                        ...
                }

                for _, v := range validations {
                    if err := v(spec, rspec); err != nil {
                        logrus.Fatalf("Validation failed: %q", err)
                    }
                }
            },
        },
        {
            Name:    "validateSeccomp",
            Aliases: []string{"vse"},
            Usage:   "Validate Seccomp with specs",
            Action: func(c *cli.Context) {
                if err := validateSeccomp(spec, rspec); err != nil {
                    logrus.Fatalf("Validation failed: %q", err)
                }
            },
        },
        {
            Name:    "validateHooks",
            Aliases: []string{"vho"},
            Usage:   "Validate Hooks with specs",
            Action: func(c *cli.Context) {
                if err := validateHooks(spec, rspec); err != nil {
                    logrus.Fatalf("Validation failed: %q", err)
                }
            },
        },
        {
            Name:    "validateSelinux",
            Aliases: []string{"vsel"},
            Usage:   "Validate selinuxlabel with specs",
            Action: func(c *cli.Context) {
                if err := validateSelinux(spec, rspec); err != nil {
                    logrus.Fatalf("Validation failed: %q", err)
                }
            },
        },
...

With this way, we will have benifits as below,

  1. To do less job in container end with each case, lead to the same effort.
  2. Cases which have different realization in container end have less impect to others.
  3. Because we use the standard default template of specs in validate, so we have no need to validate all of the items every time.

What is your opinion? Expect to your response.

mrunalp commented 8 years ago

The same tool should work for different implementations. To get around the rkt pid 1 issue, we could change the code to look at the cmdline of all the processes in the container. If there is a match, we can break else error out.

zenlint commented 8 years ago

@mrunalp To "To get around the rkt pid 1 issue, we could change the code to look at the cmdline of all the processes in the container", so agree with you.

And to “The same tool should work for different implementations”, I also quite agreed, but I think there is no impact if we make runtimetest to support different flags for each item of specs(also consist of "runtimetest --overall" as a default validate option to validate all of the config items), like,

NAME:
   oci-runtimeValidate - Utilities for OCI runtime validation

USAGE:
   ./runtimetest [global options] command [command options] [arguments...]

VERSION:
   0.0.1

COMMANDS:
   validateOverall, va      Validate overall specs
   validateSeccomp, vse     Validate Seccomp with specs
   validateHooks, vho       Validate Hooks with specs
   validateSelinux, vsel    Validate selinuxlabel with specs
   validateMount, vmo       Validate Mounts information with specs
   validateNamespace, vna   Validate Namespace  with specs
   validateDevices, vde     Validate Devices  with specs
   validateIDmappings, vid  Validate IDmappings  with specs
   validatePlatform, vpl    Validate Platform  with specs
   validateRoot, vro        Validate root  with specs
   validateProcess, vpr     Validate process with specs
   validateCapabilities, vca    Validate capabilities with specs
   validateHostname, vho    Validate hostname with specs
   validateRlimits, vrl     Validate rlimits with specs
   validateSysctls, vsy     Validate sysctls with specs
   help, h          Shows a list of commands or help for one command

What about your opinion? Thanks.

wking commented 8 years ago

On Sat, Dec 05, 2015 at 10:48:15PM -0800, LinZhinan(Zen Lin) wrote:

validateOverall, va Validate overall specs validateSeccomp, vse Validate Seccomp with specs validateHooks, vho Validate Hooks with specs

Instead of breaking this out into commands, I'd rather see it broken out into options (like generate uses), so:

validate --hooks …

to make it easier to run a single command that performs several tests.

Python's unittest has a built-in command-line interface that makes it easy to run subsets of tests 1. Is there a Go test-suite library like that?

mrunalp commented 8 years ago

I am not seeing the benefit of breaking this down. What do we gain? compliant runtimes are expected to pass all the checks.

wking commented 8 years ago

On Sun, Dec 06, 2015 at 11:34:28AM -0800, Mrunal Patel wrote:

I am not seeing the benefit of breaking this down. What do we gain? compliant runtimes are expected to pass all the checks.

I expect like POSIX there will be systems that are pretty close, but don't bother to check off all the corner cases in the spec (e.g. maybe they leave out seccomp or some such 1). Or maybe they're trying to be fully compliant, and just want a faster test cycle as they work on filling in support for a particular feature. Neither sounds like a huge win for me, but I don't see a benefit to not supplying that flexibility. With a framework like Python's unittest, the implementation and maintenance cost for providing the flexibility is negligable. I don't have a handle on what the implementation and maintenance cost is in Go, maybe it's more than the flexibility is worth.

zenlint commented 8 years ago

@wking @mrunalp ocitool is going to provide functionality of generating and validating, but some other projects(e.g. like OCT) want to do full compliant testing using high coverage test cases, it is no necessary to check other specs items(e.g. If want to check spec.Process.Arg, just validate itself, without validating other items) in each call of runtimetest. Calling of runtimetest is a frequent operation,runtimetest is a public container-end tool anyway, it is better to do less job than to do more job in it.

@wking Options like generate are used to accept parameters, do u mean something like, "runtimtest --testObj=mount" but not "runtimetest vmo" ?

wking commented 8 years ago

On Sun, Dec 06, 2015 at 09:59:45PM -0800, LinZhinan(Zen Lin) wrote:

ocitool is going to provide functionality of generating and validating, but some other projects(e.g. like OCT) want to do full compliant testing using high coverage test cases…

I'm not clear on how responsibilities are allocated between ocitools and OCT, but the charter talks about a “certification program” 1. The runtime-validation tooling for that will have to be exhaustive, and I don't see a need for further testing on top of that exhaustive tool (but I'm not sure whether ocitools and/or OCT are intending to be that tool).

Options like generate are used to accept parameters, do u mean something like, "runtimtest --testObj=mount" but not "runtimetest vmo" ?

I meant ‘runtimetest [-m|--mount] [-h|--hooks] …’, so you can stack in a single call:

$ runtimetest -mh…

But I don't have strong feelings about the syntax.

liangchenye commented 8 years ago

@wking Agree, since the scope of OCI is quite limited, 'tool -- test -- certification' could be merged into one project. Comparing to testing, there will be more documentation work in certification.

I'd like to add this topic to the weekly meeting. Maybe we could have a project like oci-certify in 'opencontainers' one day and maintain the tool/testing/certification together in that repo.

Hi @mrunalp what do you think about this?