validation: test cgroups with different input values #637

dongsupark · 2018-05-16T13:30:52Z

This PR tries to cover more test cases by running cgroup tests with different combinations of input values. Also it tries to print out detailed output results in the TAP format.

To do that, first of all, we change prototype of RuntimeOutsideValidate() as well as AfterFunc(), updating all corresponding call sites. And then we update cgroups tests. Changed cgroups tests are:

Memory
CPU
Network
Blkio
HugeTLB

Especially I needed to update CPU cgroups tests a lot, by following the same semantics of memory cgroups. For that, I had to add helpers in validation/util/linux_resources_cpus.go.

/cc @alban

dongsupark · 2018-05-16T15:46:55Z

I changed prototype of RuntimeOutsideValidate() as well as AfterFunc(), updating all corresponding call sites. This is needed for making use of t (*tap.T) inside individual test functions, as well as keeping context of multiple tests.
Now sequential numbers of tests are kept as before.

Fix several nil dereference errors in validation tests for cpu & blkio cgroups, such as: ``` panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x6eee8b] goroutine 1 [running]: github.com/opencontainers/runtime-tools/validation/util.ValidateLinuxResourcesBlockIO(0xc420264070, 0xc420248a50, 0x5, 0xc42027e540) /home/vagrant/go/src/github.com/opencontainers/runtime-tools/validation/util/linux_resources_blkio.go:37 +0x2bb github.com/opencontainers/runtime-tools/validation/util.RuntimeOutsideValidate(0xc4200102b0, 0x7c1c00, 0x0, 0x0) /home/vagrant/go/src/github.com/opencontainers/runtime-tools/validation/util/test.go:279 +0x397 main.testEmptyBlkio(0x0, 0x0) /home/vagrant/go/src/github.com/opencontainers/runtime-tools/validation/linux_cgroups_blkio.go:68 +0x19e main.main() /home/vagrant/go/src/github.com/opencontainers/runtime-tools/validation/linux_cgroups_blkio.go:88 +0x8c ``` Note that this PR fixes only a part of potential nil dereferences in validation tests. It only touches cpu and blkio, which were discovered during testing the PR opencontainers#637. Signed-off-by: Dongsu Park <[email protected]>

liangchenye · 2018-05-21T05:00:20Z

generate/config.go


-func (g *Generator) initConfigLinuxResourcesCPU() {
+// InitConfigLinuxResourcesCPU initializes CPU of Linux resources
+func (g *Generator) InitConfigLinuxResourcesCPU() {


this change LGTM

liangchenye · 2018-05-21T05:00:45Z

validation/linux_cgroups_blkio.go

+	t.Header(0)
+	defer t.AutoPlan()
+
+	// It's assumed that a device /dev/sda (8:0) exists on the test system.


why do we need this assumption?

@liangchenye
I'm not sure why, as I'm not the original author of the blkio test. The test has always relied on static major/minor numbers, since the beginning:
f1e02ff

What I'm doing with this PR is only to improve the existing test. Though during running the blkio test on my local system, I realized that the test failed because of a missing device /dev/sda (8:0). The local machine had only /dev/nvme (243:0), not /dev/sda. That's why I added the comment above.

Ideally we could improve this part so that it detects the root device at runtime, instead of hard-coding like that. Though I think that it should be done in a separate PR. If you want me to change so, I can make another PR.

liangchenye · 2018-05-21T05:04:16Z

validation/linux_cgroups_blkio.go

+	return nil
+}
+
+func testEmptyBlkio() error {


I think it could be part of testBlkioCgroups. We can add more args to the cases struct. Take https://github.com/opencontainers/runtime-tools/blob/master/validation/state.go#L29 for example.

@liangchenye That sounds like a good idea. Will try to do that.

liangchenye · 2018-05-21T05:09:57Z

This PR tries to cover more test cases by running cgroup tests with different combinations of input values. Also it tries to print out detailed output results in the TAP format.

The combinations idea looks cool. I just wonder if the testing values were representative since we are not doing performance/pressure test.

dongsupark · 2018-05-22T12:20:57Z

@liangchenye Sure, in some cases testing values could not be optimal. But I don't have a better idea.

dongsupark · 2018-05-22T13:35:51Z

Rebased on top of master, and updated CPU & blkio tests.

Added more nil checks in CPU tests
Squashed empty blkio tests into testBlkioCgroups.

dongsupark · 2018-06-04T15:03:13Z

Updated the PR, to replace util.Fatal with t.Fail, as done in #645.

zhouhao3 · 2018-06-06T07:36:16Z

validation/util/linux_resources_devices.go

 		}
 	}

-	t.AutoPlan()


According to your idea, there is t.AutoPlan() in this file that has not been deleted yet.

@q384566678 Good catch.
I've removed t.AutoPlan() from linux_resources_devices.go as well as linux_resources_blkio.go.

zhouhao3 · 2018-06-06T07:44:13Z

Looks good, WDYT @liangchenye .

To be able to print out TAP results for individual tests, and at the same time, to keep sequential numbers of tests, we need to allow `RuntimeOutsideValidate()` to take a new parameter `t` (*tap.T). Doing that we are able to keep testing helpers in `validation/util`. Signed-off-by: Dongsu Park <[email protected]>

Test memory cgroups with different values for memory limit as well as swappiness. Signed-off-by: Dongsu Park <[email protected]>

Test CPU cgroups with various combinations of input values, as well as an empty cgroup to check for the default values. For that, create `validation/util/linux_resources_cpus.go` that holds test helpers for CPU cgroups. Determine number of CPUs correctly at runtime. If possible, test also realtime runtime as well as realtime periods. Print out result outputs in the TAP format. Signed-off-by: Dongsu Park <[email protected]>

Test if network cgroups tests work correctly, with different test cases, e.g. class ID, prio, interface name, whether net/user namespace is enabled or not. Signed-off-by: Dongsu Park <[email protected]>

Test if blkio cgroups tests work corectly, with different rate values. Also test it with empty weight values in the input json file. Signed-off-by: Dongsu Park <[email protected]>

Test if hugetlb cgroups tests work correctly, with different page size values. Also test if the test fails with an error when creating a hugetlb cgroup with a wrong page size like 3GB. Make it also print out correct TAP output. Signed-off-by: Dongsu Park <[email protected]>

zhouhao3 · 2018-06-12T07:14:31Z

LGTM

dongsupark force-pushed the dongsu/test-cgroups branch from 999e411 to b70abaf Compare May 16, 2018 15:40

dongsupark changed the title ~~[WIP] validation: test cgroups with different input values~~ validation: test cgroups with different input values May 16, 2018

dongsupark mentioned this pull request May 17, 2018

validation: fix nil deferences in cpu & blkio cgroups tests #638

Merged

liangchenye reviewed May 21, 2018

View reviewed changes

dongsupark force-pushed the dongsu/test-cgroups branch 2 times, most recently from e25eb21 to 83203d0 Compare May 22, 2018 13:33

dongsupark mentioned this pull request May 23, 2018

validation/linux_cgroups_*hugetlb: Use smaller limits #619

Open

dongsupark force-pushed the dongsu/test-cgroups branch from 83203d0 to 469cd06 Compare June 4, 2018 15:01

dongsupark force-pushed the dongsu/test-cgroups branch from 469cd06 to 246c198 Compare June 4, 2018 15:19

zhouhao3 reviewed Jun 6, 2018

View reviewed changes

dongsupark force-pushed the dongsu/test-cgroups branch from 246c198 to 16177dc Compare June 6, 2018 10:12

Dongsu Park added 5 commits June 6, 2018 12:14

validation: test linux_cgroups_memory with different values

871f0eb

Test memory cgroups with different values for memory limit as well as swappiness. Signed-off-by: Dongsu Park <[email protected]>

validation: add more test cases for linux_cgroups_network

cf9decf

Test if network cgroups tests work correctly, with different test cases, e.g. class ID, prio, interface name, whether net/user namespace is enabled or not. Signed-off-by: Dongsu Park <[email protected]>

validation: add different test cases for blkio cgroup tests

ef113d1

Test if blkio cgroups tests work corectly, with different rate values. Also test it with empty weight values in the input json file. Signed-off-by: Dongsu Park <[email protected]>

dongsupark force-pushed the dongsu/test-cgroups branch from 16177dc to 0a7749a Compare June 6, 2018 10:19

zhouhao3 merged commit f256851 into opencontainers:master Jun 12, 2018

dongsupark deleted the dongsu/test-cgroups branch June 12, 2018 07:59

validation: test cgroups with different input values #637

validation: test cgroups with different input values #637

Uh oh!

Conversation

dongsupark commented May 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongsupark commented May 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liangchenye May 21, 2018

Choose a reason for hiding this comment

Uh oh!

liangchenye May 21, 2018

Choose a reason for hiding this comment

Uh oh!

dongsupark May 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liangchenye May 21, 2018

Choose a reason for hiding this comment

Uh oh!

dongsupark May 22, 2018

Choose a reason for hiding this comment

Uh oh!

liangchenye commented May 21, 2018

Uh oh!

dongsupark commented May 22, 2018

Uh oh!

dongsupark commented May 22, 2018

Uh oh!

dongsupark commented Jun 4, 2018

Uh oh!

zhouhao3 Jun 6, 2018

Choose a reason for hiding this comment

Uh oh!

dongsupark Jun 6, 2018

Choose a reason for hiding this comment

Uh oh!

zhouhao3 commented Jun 6, 2018

Uh oh!

zhouhao3 commented Jun 12, 2018 • edited by caniszczyk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dongsupark commented May 16, 2018 •

edited

Loading

dongsupark commented May 16, 2018 •

edited

Loading

dongsupark May 22, 2018 •

edited

Loading

zhouhao3 commented Jun 12, 2018 •

edited by caniszczyk

Loading