Skip to content
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.

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

add annotation and prop tests #586

Merged
merged 2 commits into from
Mar 12, 2018
Merged

Conversation

liangchenye
Copy link
Member

test cases for AnnotationsKeyReversedDomain, AnnotationsKeyReservedNS,
AnnotationsKeyIgnoreUnknown, ExtensibilityIgnoreUnknownProp and
ValidValues

Signed-off-by: Liang Chenye [email protected]

@liangchenye
Copy link
Member Author

  • add a random key to test 'AnnotationsKeyIgnoreUnknown'
  • add an 'Unknown' proper to config.json to test 'ExtensibilityIgnoreUnknownProp'
  • using invalid version to test 'ValidValues'
    but seems runc does not popup errors.

@liangchenye
Copy link
Member Author

rebased, PTAL @q384566678

PreCreate: func(r *util.Runtime) error {
r.SetID(containerID)
saveConfig(configFile, c.eSpec)
return nil
Copy link

Choose a reason for hiding this comment

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

I think it's better to return saveConfig(configFile, c.eSpec)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll update it.

@zhouhao3
Copy link

zhouhao3 commented Mar 6, 2018

It was in a panic when I was testing. Here is the test. I'm looking for the reason.

➜  runtime-tools git:(4826ab8) ✗ ./oci-runtime-tool validate
key  SHOULD be named using a reverse domain notation
Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#annotations
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x575f02]

goroutine 1 [running]:
github.com/opencontainers/runtime-tools/vendor/github.com/hashicorp/go-multierror.(*Error).Error(0x0, 0x9eaea0, 0x0)
	/home/zhouhao/workspace/Go/src/github.com/opencontainers/runtime-tools/vendor/github.com/hashicorp/go-multierror/multierror.go:15 +0x22
github.com/opencontainers/runtime-tools/vendor/github.com/urfave/cli.HandleExitCoder(0x9eaea0, 0x0)
	/home/zhouhao/workspace/Go/src/github.com/opencontainers/runtime-tools/vendor/github.com/urfave/cli/errors.go:102 +0xff
github.com/opencontainers/runtime-tools/vendor/github.com/urfave/cli.Command.Run(0x801238, 0x8, 0x0, 0x0, 0x0, 0x0, 0x0, 0x807013, 0x16, 0x0, ...)
	/home/zhouhao/workspace/Go/src/github.com/opencontainers/runtime-tools/vendor/github.com/urfave/cli/command.go:196 +0xadc
github.com/opencontainers/runtime-tools/vendor/github.com/urfave/cli.(*App).Run(0xc420086b60, 0xc42000c080, 0x2, 0x2, 0x0, 0x0)
	/home/zhouhao/workspace/Go/src/github.com/opencontainers/runtime-tools/vendor/github.com/urfave/cli/app.go:250 +0x758
main.main()
	/home/zhouhao/workspace/Go/src/github.com/opencontainers/runtime-tools/cmd/oci-runtime-tool/main.go:50 +0x404

Add test cases for test cases for AnnotationsKeyReversedDomain,
AnnotationsKeyReservedNS, AnnotationsKeyIgnoreUnknown, ValidValues
and ExtensibilityIgnoreUnknownProp.

Signed-off-by: Liang Chenye <[email protected]>
@liangchenye
Copy link
Member Author

@q384566678 it works in my test, can you post your 'config.json' file?

@zhouhao3
Copy link

zhouhao3 commented Mar 6, 2018

{
        "ociVersion": "1.0.0",
        "process": {
                "user": {
                        "uid": 0,
                        "gid": 0
                },
                "args": [
                        "sh"
                ],
                "env": [
                        "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
                        "TERM=xterm"
                ],
                "cwd": "/"
        },
        "root": {
                "path": "rootfs"
        },
        "linux": {
                "resources": {
                        "devices": [
                                {
                                        "allow": false,
                                        "access": "rwm"
                                }
                        ]
                }
        },
        "annotations": {
                    "test": "22"
        }
}

@liangchenye
Copy link
Member Author

@q384566678 thanks, I can reproduce it. Debugging now.

@liangchenye liangchenye changed the title add annotation and prop tests WIP: add annotation and prop tests Mar 6, 2018
@liangchenye
Copy link
Member Author

Very strange, if I add the following useless lines, it works. I'll continue to debug it tomorrow.

			if levelErrors.Error == nil {
				return nil
			}
 			return levelErrors.Error

@liangchenye liangchenye changed the title WIP: add annotation and prop tests add annotation and prop tests Mar 9, 2018
@liangchenye
Copy link
Member Author

The original code return *error instread of error. The new commit solves that.
It is more accurate to return levelErrors.Error.ErrorOrNil(). It works in my test.
PTAL @q384566678


type extendedSpec struct {
rspecs.Spec
Unknown string `json:"unknown,omitempty"`

Choose a reason for hiding this comment

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

I don't quite understand what that does.

Copy link
Member Author

Choose a reason for hiding this comment

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

I use this to test ’runtimes that are reading or processing this configuration file MUST NOT generate an error if they encounter an unknown property‘. 'Unknown' here (in my commit) means the 'unknown property', so when a runtime reads this, it does not know what it means and it should not generete an error.

@zhouhao3
Copy link

zhouhao3 commented Mar 12, 2018

LGTM

Approved with PullApprove

@zhouhao3 zhouhao3 merged commit 3a83c2c into opencontainers:master Mar 12, 2018
@alban
Copy link
Contributor

alban commented Jul 19, 2018

using invalid version to test 'ValidValues' but seems runc does not popup errors.

I filed an issue about this: opencontainers/runc#1846

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants