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 $RUNC_USE_SYSTEMD to run tests using systemd cgroup driver #2014

Merged
merged 5 commits into from
Mar 15, 2019

Conversation

filbranden
Copy link
Contributor

@filbranden filbranden commented Mar 14, 2019

This allows us to test runc using libcontainer's systemd driver, by passing a RUNC_USE_SYSTEMD=1 argument to localintegration tests.

Tested:

$ sudo make localintegration RUNC_USE_SYSTEMD=1

All tests passed to me (with exception of one writing to blkio.weight which for some reason is missing on my Linux kernel 5.0, will look into that...)

And confirmed that systemd was in use by looking at creation and removal of libcontainer_<pid>_systemd_test_default.slice test slices. Also introduced a breakage in systemd cgroup driver and confirmed that the tests failed as expected.

@filbranden
Copy link
Contributor Author

This doesn't seem to be enough... Many tests fail with:

expected cgroupsPath to be of format "slice:prefix:name" for systemd cgroups

I'll maybe rethink this to use a boolean flag such as RUNC_USE_SYSTEMD or RUNC_SYSTEMD_DRIVER etc. and then check this from the tests themselves as well.

More information should help troubleshoot an issue when this error occurs.

Signed-off-by: Filipe Brandenburger <filbranden@google.com>
This allows us to test runc using libcontainer's systemd driver, by
passing an extra `--systemd-cgroup` argument to the calls to runc.

Tested:
  $ sudo make localintegration TESTPATH='/exec.bats' RUNC_USE_SYSTEMD=1

And confirmed that systemd was in use by looking at creation and removal
of libcontainer_<pid>_systemd_test_default.slice test slices. Also
introduced a breakage in systemd cgroup driver and confirmed that the
tests failed as expected.

Signed-off-by: Filipe Brandenburger <filbranden@google.com>
When $RUNC_USE_SYSTEMD is set, then use a systemd syntax for the
cgroupsPath. Also fix $CGROUPS_PATH to look under the actual path to the
slice/scope created by systemd.

Tested:
  $ sudo make localintegration TESTPATH='/cgroups.bats' RUNC_USE_SYSTEMD=1

That test will fail without this commit.

Signed-off-by: Filipe Brandenburger <filbranden@google.com>
These tests sometimes hang, so let's skip them for now.

Tested:
  $ sudo make localintegration TESTPATH='/checkpoint.bats' RUNC_USE_SYSTEMD=1

The 5 tests in this test suite will be skipped.

Signed-off-by: Filipe Brandenburger <filbranden@google.com>
@filbranden filbranden changed the title Add $EXTRA_RUNC_ARGS to pass runc additional arguments in tests Add $RUNC_USE_SYSTEMD to run tests using systemd cgroup driver Mar 14, 2019
@filbranden
Copy link
Contributor Author

Ok, this has now been updated and passes tests.

  • Switched to a more explicit option RUNC_USE_SYSTEMD=1 that should be passed to make.
  • Tests that create cgroups look at the two separate cases.
  • For now, skipping checkpoint/restore tests when $RUNC_USE_SYSTEMD is set. (I've had some of those tests hang, so I thought it was easier to just skip them for now.)
  • Added some extra debugging to a message parsing the systemd cgroup path format.

/cc @mrunalp Please take a look!

Cheers,
Filipe

@mrunalp
Copy link
Contributor

mrunalp commented Mar 14, 2019

LGTM

Approved with PullApprove

@mrunalp
Copy link
Contributor

mrunalp commented Mar 14, 2019

/hold @filbranden is going to add travis tests for this as well.

The additional test shows as a separate job. It sets environment
RUNC_USE_SYSTEMD=1 so it will be clear in Travis-CI that this job is
testing the systemd cgroup driver.

Signed-off-by: Filipe Brandenburger <filbranden@google.com>
@filbranden
Copy link
Contributor Author

@mrunalp Done, added a commit to add a Travis-CI job to run the systemd cgroup driver integration tests. PTAL.

@mrunalp
Copy link
Contributor

mrunalp commented Mar 15, 2019

LGTM

Approved with PullApprove

1 similar comment
@dqminh
Copy link
Contributor

dqminh commented Mar 15, 2019

LGTM

Approved with PullApprove

@dqminh dqminh merged commit 7341c22 into opencontainers:master Mar 15, 2019
@filbranden filbranden deleted the testing1 branch March 15, 2019 15:27
@filbranden
Copy link
Contributor Author

Thanks @mrunalp and @dqminh for the quick code reviews! 👍

thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Mar 28, 2019
full diff: opencontainers/runc@2b18fe1...v1.0.0-rc7

changes included:

- opencontainers/runc#2012 Need to setup labeling of kernel keyrings
- opencontainers/runc#2014 Add $RUNC_USE_SYSTEMD to run tests using systemd cgroup driver
- opencontainers/runc#2015 Use getenv not secure_getenv
  - fixes opencontainers/runc#2013 build fails with musl libc
- opencontainers/runc#2023 Fixes regression causing zombie runc:[1:CHILD] processes

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Mar 28, 2019
full diff: opencontainers/runc@2b18fe1...v1.0.0-rc7

changes included:

- opencontainers/runc#2012 Need to setup labeling of kernel keyrings
- opencontainers/runc#2014 Add $RUNC_USE_SYSTEMD to run tests using systemd cgroup driver
- opencontainers/runc#2015 Use getenv not secure_getenv
  - fixes opencontainers/runc#2013 build fails with musl libc
- opencontainers/runc#2023 Fixes regression causing zombie runc:[1:CHILD] processes

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit bc6ac08)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Mar 28, 2019
full diff: opencontainers/runc@2b18fe1...v1.0.0-rc7

changes included:

- opencontainers/runc#2012 Need to setup labeling of kernel keyrings
- opencontainers/runc#2014 Add $RUNC_USE_SYSTEMD to run tests using systemd cgroup driver
- opencontainers/runc#2015 Use getenv not secure_getenv
  - fixes opencontainers/runc#2013 build fails with musl libc
- opencontainers/runc#2023 Fixes regression causing zombie runc:[1:CHILD] processes

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit bc6ac08)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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