Re-enable CRIU tests by not using overlayfs snapshotter #4708

kzys · 2020-11-09T21:49:55Z

While the issue hasn't been fixed in the kernel yet, we can workaround
the issue by not using overlayfs snapshotter.

Signed-off-by: Kazuyoshi Kato katokazu@amazon.com

k8s-ci-robot · 2020-11-09T21:50:04Z

Hi @kzys. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

kzys · 2020-11-09T22:24:47Z

Hmm, let me check the kernel version on the GitHub Actions' Ubuntu.

kzys · 2020-11-09T22:36:36Z

Ubuntu 14.04 uses Linux fv-az54-676 5.4.0-1031-azure #32~18.04.1-Ubuntu SMP Tue Oct 6 10:03:22 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux.

estesp · 2020-11-09T22:42:25Z

I don't think this will work based on ongoing commentary in checkpoint-restore/criu#860 (see this comment). The upstream bug is confusing as it has been marked fixed at least once or twice, but is clearly still an issue.

The recommendation in checkpoint-restore/criu#1239 (comment) might be worth considering--should we run just the checkpoint/restore tests to make sure our code is still working by running a quick test with a different graphdriver?

kzys · 2020-11-10T19:37:48Z

Thanks @estesp. How about running the tests with multiple different snapshotters? criu may need devmappper, but I'm bit concerned to move containerd's tests from overlay to let's say devmapper entirely because of criu.

estesp · 2020-11-10T19:50:36Z

Yes, I think we can build a small action that only runs the criu tests with a native/devicemapper snapshotter and let the rest of the testsuite continue using overlay.

kzys · 2020-11-12T01:22:48Z

container_checkpoint_test.go:142: type with url : not found: unknown is coming from

containerd/runtime/v2/runc/container.go

Line 53 in f1a469a

v, err := typeurl.UnmarshalAny(r.Options)

.

kzys · 2020-11-12T14:20:51Z

runtime/v2/runc/container.go

@@ -49,7 +49,7 @@ func NewContainer(ctx context.Context, platform stdio.Platform, r *task.CreateTa
 	}

 	var opts options.Options
-	if r.Options != nil {
+	if r.Options != nil && r.Options.GetTypeUrl() != "" {


I'm unsure this is the right place to fix this issue. Shouldn't it be handled by typeurl.UnmarshalAny()?

@crosbymichael can you provide any insight here?

Ya, i think this is fine. It should be handled by the caller because of the nil check. I'll have to think a bit on a better way to fix this in the lib but it shouldn't be common.

kzys · 2020-11-16T17:51:52Z

@estesp Could you take a look? Seems like CRIU has been broken.

theopenlab-ci · 2021-03-12T01:01:48Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 6m 18s (non-voting)

estesp · 2021-03-12T15:20:06Z

Sorry @kzys this needs a rebase due to the PR which moved *_test.go from the root being merged.

Also, thinking about the time cost of having yet another pass through make integration just to pick up the checkpoint tests:
What about renaming the last test in the checkpoint test file here to start with TestCheckpoint and then you could pass EXTRA_TESTFLAGS="-run TestCheckpoint" to your make integration and only have the handful of checkpoint tests run rather than adding 2-3 minutes to all of the runc+Linux integration combinations in CI?

kzys · 2021-03-12T18:04:47Z

@estesp Sounds good to me. Let me update the PR.

theopenlab-ci · 2021-03-12T18:17:12Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 5m 51s (non-voting)

kzys · 2021-03-12T18:58:03Z

Rebased! Not so sure why "CGroupsV2 and SELinux Integration" is failing here.

mikebrow · 2021-03-12T19:22:39Z

Rebased! Not so sure why "CGroupsV2 and SELinux Integration" is failing here.

429 Too Many Requests. ... we're trying to work on this.. it's the new docker.io pull rate limits

kzys · 2021-03-16T23:31:55Z

@estesp Can you take a look?

.github/workflows/ci.yml

- process.Init#io could be nil - Make sure CreateTaskRequest#Options is not empty before unmarshaling Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

While the issue hasn't been fixed in the kernel yet, we can workaround the issue by not using overlayfs snapshotter. The newly added step runs all tests that match /TestCheckpoint/. So, TestCRWithImagePath has been renamed to match the regexp. Fixes containerd#3930. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

theopenlab-ci · 2021-03-16T23:56:04Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 5m 44s (non-voting)

estesp

LGTM; would love @crosbymichael to confirm the options typeurl change

kzys · 2021-03-17T21:31:28Z

/ok-to-test

k8s-ci-robot added the needs-ok-to-test label Nov 9, 2020

kzys force-pushed the enable-criu branch 6 times, most recently from 6bace7f to 4213735 Compare November 11, 2020 23:00

kzys changed the title ~~Re-enable CRIU tests~~ Re-enable CRIU tests by not using overlayfs snapshotter Nov 11, 2020

kzys force-pushed the enable-criu branch 2 times, most recently from a8a3388 to fd8bc31 Compare November 12, 2020 00:31

kzys force-pushed the enable-criu branch from e2fae95 to 432ec8a Compare November 12, 2020 06:06

kzys commented Nov 12, 2020

View reviewed changes

containerd deleted a comment from theopenlab-ci bot Nov 13, 2020

kzys force-pushed the enable-criu branch from 432ec8a to 340b112 Compare March 12, 2021 00:54

kzys force-pushed the enable-criu branch 2 times, most recently from 706a99c to 36afcb7 Compare March 12, 2021 18:09

estesp reviewed Mar 16, 2021

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

kzys added 2 commits March 16, 2021 16:46

Fix CRIU

b520428

- process.Init#io could be nil - Make sure CreateTaskRequest#Options is not empty before unmarshaling Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

kzys force-pushed the enable-criu branch from 36afcb7 to 8731888 Compare March 16, 2021 23:48

estesp approved these changes Mar 17, 2021

View reviewed changes

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Mar 17, 2021

crosbymichael approved these changes Mar 19, 2021

View reviewed changes

crosbymichael merged commit e0c94bb into containerd:master Mar 19, 2021

estesp mentioned this pull request Jul 28, 2021

Reenable CRIU tests when kernel issue is resolved #3930

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-enable CRIU tests by not using overlayfs snapshotter #4708

Re-enable CRIU tests by not using overlayfs snapshotter #4708

kzys commented Nov 9, 2020 •

edited

k8s-ci-robot commented Nov 9, 2020

kzys commented Nov 9, 2020

kzys commented Nov 9, 2020

estesp commented Nov 9, 2020

kzys commented Nov 10, 2020 •

edited

estesp commented Nov 10, 2020

kzys commented Nov 12, 2020

kzys Nov 12, 2020

estesp Mar 16, 2021

crosbymichael Mar 19, 2021

kzys commented Nov 16, 2020

theopenlab-ci bot commented Mar 12, 2021

estesp commented Mar 12, 2021

kzys commented Mar 12, 2021

theopenlab-ci bot commented Mar 12, 2021

kzys commented Mar 12, 2021

mikebrow commented Mar 12, 2021

kzys commented Mar 16, 2021

theopenlab-ci bot commented Mar 16, 2021

estesp left a comment

kzys commented Mar 17, 2021

Re-enable CRIU tests by not using overlayfs snapshotter #4708

Re-enable CRIU tests by not using overlayfs snapshotter #4708

Conversation

kzys commented Nov 9, 2020 • edited

k8s-ci-robot commented Nov 9, 2020

kzys commented Nov 9, 2020

kzys commented Nov 9, 2020

estesp commented Nov 9, 2020

kzys commented Nov 10, 2020 • edited

estesp commented Nov 10, 2020

kzys commented Nov 12, 2020

kzys Nov 12, 2020

Choose a reason for hiding this comment

estesp Mar 16, 2021

Choose a reason for hiding this comment

crosbymichael Mar 19, 2021

Choose a reason for hiding this comment

kzys commented Nov 16, 2020

theopenlab-ci bot commented Mar 12, 2021

estesp commented Mar 12, 2021

kzys commented Mar 12, 2021

theopenlab-ci bot commented Mar 12, 2021

kzys commented Mar 12, 2021

mikebrow commented Mar 12, 2021

kzys commented Mar 16, 2021

theopenlab-ci bot commented Mar 16, 2021

estesp left a comment

Choose a reason for hiding this comment

kzys commented Mar 17, 2021

kzys commented Nov 9, 2020 •

edited

kzys commented Nov 10, 2020 •

edited