integration: add userns and checkpoint integration test#5122
integration: add userns and checkpoint integration test#5122everzakov wants to merge 1 commit intoopencontainers:mainfrom
Conversation
ebaa9ff to
a8d17e0
Compare
rata
left a comment
There was a problem hiding this comment.
LGTM, but of course only after CI is passing :)
Github is doing something odd, it is showing as passed some runners that failed, like this one: https://github.com/opencontainers/runc/actions/runs/22224593666/job/64327155532?pr=5122#step:13:366. It passed, is green, but if you check the integration, this test failed.
It seems like a bug on github side, I don't think we have anything to do here.
tests/integration/userns.bats
Outdated
| run ! ip link del dummy0 | ||
| } | ||
|
|
||
| @test "checkpoint userns container with non default host id" { |
There was a problem hiding this comment.
what do you mean with non default id?
There was a problem hiding this comment.
I mean user namespace with non-zero host id
There was a problem hiding this comment.
All tests in this file do that already and none uses that terminology. Maybe just:
| @test "checkpoint userns container with non default host id" { | |
| @test "checkpoint userns container" { |
Or do you think this is confusing for some reason?
There was a problem hiding this comment.
For me, it is ok. I just wanted to highlight that this test differs from TestUsernsCheckpoint unit test :)
@rata this is because since #5012 we ignore criu-dev failures, and this is criu-dev job. I wish there would be a way to mark it red but not required, but can't think of a simple way of doing it. |
|
Set to draft until criu issue is fixed. |
|
@kolyshkin ohh, okay. And I guess the arm builds don't have that, as those are failing? Or maybe I'm just too tired and not seeing the obvious :-D |
Hello! checkpoint-restore/criu#2903 is merged. Can you restart the CI pipeline to try integration tests with criu-dev? |
|
I guess we need to guard this with something like criu > v4.2 if we want to merge it (as older versions will fail). Or, maybe, this test case belongs to criu not to runc? |
Well, this test is failed again with criu-dev branch. I think i can make the same test in criu. |
|
@everzakov it's not just criu-dev that is broken if you look at our CI run. If you can write the test in a way that passes on CI and doesn't seem flaky, I'm fine with keeping it here. But it definitely need to be in criu first, as that is the reason it's broken and I don't want our CI to depend on features that criu doesn't test and break often. |
a8d17e0 to
2bbdc61
Compare
guard is added Some checks with criu versions: vboxuser@vboxuser:~/runc$ criu --version
Version: 4.2
GitID: v4.2-89-g09134c8ca
ok 11 checkpoint userns container # skip requires CRIU version 4.3
vboxuser@vboxuser:~/runc$ criu --version
Version: 4.3
GitID: v4.2-89-g09134c8ca
ok 11 checkpoint userns container
vboxuser@vboxuser:~/runc$ criu --version
Version: 4.10
GitID: v4.2-89-g09134c8ca
ok 11 checkpoint userns container
vboxuser@vboxuser:~/runc$ criu --version
Version: 5.0
GitID: v4.2-89-g09134c8ca
ok 11 checkpoint userns container
vboxuser@vboxuser:~/runc$ criu --version
Version: 3.19
GitID: v4.2-89-g09134c8ca
ok 11 checkpoint userns container # skip requires CRIU version 4.3
|
I will check criu tests later to be sure that this test won't be flaky. However, sometimes criu is stuck in dump / restore phase. It will be cool if someone checks that it is not only my env installation problem. Also i have added the for loop like in checkpoint integration tests. |
|
centos-stream-10 is failed but not in my integration test https://cirrus-ci.com/task/4963093102460928?logs=integration_systemd#L284 From very quick look there is the difference in installed deps: |
@rata should I open the new issue? |
|
@everzakov it's not a runc issue. I think you probably need to rebase on latest main, that includes this PR: #5123. |
Signed-off-by: Efim Verzakov <efimverzakov@gmail.com>
2bbdc61 to
26ac320
Compare
This PR adds the integration test to checkpoint/restore container with user namespace (host id is not default).
Now it fails because in prepare_cgns function criu sends the wrong uid.
Fixes #5121