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

ci: fix docker test #2302

Merged
merged 2 commits into from
Dec 6, 2023
Merged

Conversation

rst0git
Copy link
Member

@rst0git rst0git commented Nov 28, 2023

The version of docker-ce on Ubuntu 20.04 has been recently updated to 25.0.0-beta.1. However, with this version docker start --checkpoint fails with the following error:

$ docker start --checkpoint=c1 cr
Error response from daemon: failed to create task for container: content digest fdb1054b00a8c07f08574ce52198c5501d1f552b6a5fb46105c688c70a9acb45: not found: unknown

As a workaround, we install the most recent stable version of docker-ce. In addition, to improve code readability, the recursive function call used for retrying container restoration has been replaced with a loop.

@rst0git rst0git requested a review from avagin November 28, 2023 13:46
@rst0git rst0git marked this pull request as draft November 28, 2023 14:06
@0x7f454c46
Copy link
Member

I remember there was quite a protest from vz guys for bashisms as they don't have bash in the testing setup (?).
But this file already has bashisms: ((current_iteration+=1)), so it shouldn't be any worse than before.

Just for curiosity: is it also fixed by pre-initializing current_iteration=0?

@avagin
Copy link
Member

avagin commented Nov 28, 2023

@0x7f454c46 I remember I was to tired with all these shellshit and replaced /bin/sh with /bin/bash everywhere, so bash is required to run tests.

@rst0git rst0git force-pushed the ci-docker-test-fix branch 3 times, most recently from f65e10c to be9a9fa Compare November 29, 2023 11:58
@rst0git rst0git marked this pull request as ready for review November 29, 2023 13:16
@rst0git rst0git force-pushed the ci-docker-test-fix branch 2 times, most recently from c527e90 to bd16e3e Compare November 29, 2023 13:33
Replace a recursive call with a loop.

Reported-by: Andrei Vagin <[email protected]>
Signed-off-by: Radostin Stoyanov <[email protected]>
Checkpoint/restore with version 25.0.0-beta.1 fails
with the following error:

$ docker start --checkpoint=c1 cr
Error response from daemon: failed to create task for container: content digest fdb1054b00a8c07f08574ce52198c5501d1f552b6a5fb46105c688c70a9acb45: not found: unknown

Release notes:
moby/moby#46816

Signed-off-by: Radostin Stoyanov <[email protected]>
@rst0git rst0git changed the title docker-test: fix condition for max tries ci: fix docker test Nov 29, 2023
@Snorch
Copy link
Member

Snorch commented Nov 30, 2023

I remember there was quite a protest from vz guys for bashisms as they don't have bash in the testing setup (?).

Thanks for the note, I was not aware of it =)

But I know for sure that we don't run scripts/ci/docker-test.sh in vz ci, so adding bashisms here should be ok anyway.

@rst0git rst0git requested a review from avagin December 5, 2023 13:34
@avagin avagin merged commit 0da1ab2 into checkpoint-restore:criu-dev Dec 6, 2023
@rst0git rst0git deleted the ci-docker-test-fix branch December 7, 2023 22:45
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.

4 participants