cri-integration tests: Pull images once#5313
Conversation
|
Hi @claudiubelu. 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 Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
Build succeeded.
|
claudiubelu
force-pushed
the
pull-images-once
branch
from
3bef81f to
7fa3cb4
Compare
|
Build succeeded.
|
claudiubelu
force-pushed
the
pull-images-once
branch
from
7fa3cb4 to
a0f4db3
Compare
|
Build succeeded.
|
claudiubelu
force-pushed
the
pull-images-once
branch
from
a0f4db3 to
2a1ab0c
Compare
|
Build succeeded.
|
claudiubelu
force-pushed
the
pull-images-once
branch
from
2a1ab0c to
9c7ae9e
Compare
|
Build succeeded.
|
claudiubelu
force-pushed
the
pull-images-once
branch
from
9c7ae9e to
680e553
Compare
claudiubelu
changed the title
|
Build succeeded.
|
claudiubelu
force-pushed
the
pull-images-once
branch
from
680e553 to
31caa22
Compare
|
Build succeeded.
|
|
I think we will merge #5403 before this change. |
claudiubelu
force-pushed
the
pull-images-once
branch
3 times, most recently
from
c9f71dc to
a104d48
Compare
|
Build succeeded.
|
Most of the tests are pulling and deleting the same test images, which can be quite inefficient, especially on Windows nodes, where the images are larger than the Linux ones (a nanoserver Container image is ~250MB in size). We can instead pull them only once, and reuse them. This will reduce the test run time on Windows considerably. Additionally, some of the test images are currently hosted on dockerhub (busybox image), which has introduced image ratelimiting in November 2020, which means that test runners could potentially hit that limit faster with the current implementation. This will reduce that risk. Some tests are specifically deleting images, so we always have to ensure that they are pulled. Signed-off-by: Claudiu Belu <[email protected]>
claudiubelu
force-pushed
the
pull-images-once
branch
from
a104d48 to
273c2bb
Compare
|
Build succeeded.
|
| defer func() { | ||
| assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) | ||
| }() |
There was a problem hiding this comment.
We no longer remove images after this change. I'm personally fine but want to highlight the behavior change just in case.
There was a problem hiding this comment.
yeah, I think that was the purpose of the PR--to simply reuse images throughout the test run rather than keep removing, re-pulling on various tests. On Windows these images can be much larger, so has more impact.
|
The change LGTM. Since we doesn't add |
|
Thanks @claudiubelu ! |
5 participants
Most of the tests are pulling and deleting the same test images, which can be quite inefficient, especially on Windows nodes, where the images are larger than the Linux ones (a nanoserver Container image is ~250MB in size). We can instead pull them
only once, and reuse them. This will reduce the test run time on Windows considerably.
Additionally, some of the test images are currently hosted on dockerhub (
busyboximage), which has introduced image ratelimiting in November 2020, which means that test runners could potentially hit that limit faster with the current implementation. This will reduce that risk.Some tests are specifically deleting images, so we always have to ensure that they are pulled.