From 273c2bb168d1733ebcbe7418ca1232237f3dc603 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Mon, 19 Apr 2021 20:51:42 +0000 Subject: [PATCH] tests: Prepull images used in tests 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 --- integration/addition_gids_test.go | 8 ++------ integration/common.go | 20 +++++++++++++++++++ integration/container_log_test.go | 16 ++++----------- integration/container_stop_test.go | 16 ++++----------- .../container_without_image_ref_test.go | 8 ++------ integration/containerd_image_test.go | 9 ++------- integration/imagefs_info_test.go | 8 ++------ integration/pod_dualstack_test.go | 8 ++------ integration/pod_hostname_test.go | 8 ++------ integration/restart_test.go | 6 +----- integration/truncindex_test.go | 7 ++----- integration/volume_copy_up_test.go | 9 ++------- 12 files changed, 45 insertions(+), 78 deletions(-) diff --git a/integration/addition_gids_test.go b/integration/addition_gids_test.go index 7674fe0ed..76eebbb7d 100644 --- a/integration/addition_gids_test.go +++ b/integration/addition_gids_test.go @@ -49,12 +49,8 @@ func TestAdditionalGids(t *testing.T) { testImage = GetImage(BusyBox) containerName = "test-container" ) - t.Logf("Pull test image %q", testImage) - img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) - }() + + EnsureImageExists(t, testImage) t.Log("Create a container to print id") cnConfig := ContainerConfig( diff --git a/integration/common.go b/integration/common.go index 24cb7de82..ac657eb91 100644 --- a/integration/common.go +++ b/integration/common.go @@ -21,10 +21,13 @@ package integration import ( "fmt" "io/ioutil" + "testing" "github.com/pelletier/go-toml" "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" cri "k8s.io/cri-api/pkg/apis" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" ) // ImageList holds public image references @@ -99,3 +102,20 @@ func initImageMap(imageList ImageList) map[int]string { func GetImage(image int) string { return imageMap[image] } + +// EnsureImageExists pulls the given image, ensures that no error was encountered +// while pulling it. +func EnsureImageExists(t *testing.T, imageName string) string { + img, err := imageService.ImageStatus(&runtime.ImageSpec{Image: imageName}) + require.NoError(t, err) + if img != nil { + t.Logf("Image %q already exists, not pulling.", imageName) + return img.Id + } + + t.Logf("Pull test image %q", imageName) + imgID, err := imageService.PullImage(&runtime.ImageSpec{Image: imageName}, nil, nil) + require.NoError(t, err) + + return imgID +} diff --git a/integration/container_log_test.go b/integration/container_log_test.go index 56aaa443e..6c226c13b 100644 --- a/integration/container_log_test.go +++ b/integration/container_log_test.go @@ -52,12 +52,8 @@ func TestContainerLogWithoutTailingNewLine(t *testing.T) { testImage = GetImage(BusyBox) containerName = "test-container" ) - t.Logf("Pull test image %q", testImage) - img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) - }() + + EnsureImageExists(t, testImage) t.Log("Create a container with log path") cnConfig := ContainerConfig( @@ -112,12 +108,8 @@ func TestLongContainerLog(t *testing.T) { testImage = GetImage(BusyBox) containerName = "test-container" ) - t.Logf("Pull test image %q", testImage) - img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) - }() + + EnsureImageExists(t, testImage) t.Log("Create a container with log path") config, err := CRIConfig() diff --git a/integration/container_stop_test.go b/integration/container_stop_test.go index 6c5c8cec6..fd5d6eb12 100644 --- a/integration/container_stop_test.go +++ b/integration/container_stop_test.go @@ -46,12 +46,8 @@ func TestSharedPidMultiProcessContainerStop(t *testing.T) { testImage = GetImage(BusyBox) containerName = "test-container" ) - t.Logf("Pull test image %q", testImage) - img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) - }() + + EnsureImageExists(t, testImage) t.Log("Create a multi-process container") cnConfig := ContainerConfig( @@ -90,12 +86,8 @@ func TestContainerStopCancellation(t *testing.T) { testImage = GetImage(BusyBox) containerName = "test-container" ) - t.Logf("Pull test image %q", testImage) - img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) - }() + + EnsureImageExists(t, testImage) t.Log("Create a container which traps sigterm") cnConfig := ContainerConfig( diff --git a/integration/container_without_image_ref_test.go b/integration/container_without_image_ref_test.go index e284bf680..4be07b147 100644 --- a/integration/container_without_image_ref_test.go +++ b/integration/container_without_image_ref_test.go @@ -41,12 +41,8 @@ func TestContainerLifecycleWithoutImageRef(t *testing.T) { testImage = GetImage(BusyBox) containerName = "test-container" ) - t.Log("Pull test image") - img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) - }() + + img := EnsureImageExists(t, testImage) t.Log("Create test container") cnConfig := ContainerConfig( diff --git a/integration/containerd_image_test.go b/integration/containerd_image_test.go index 9d81b38ef..9907a1d36 100644 --- a/integration/containerd_image_test.go +++ b/integration/containerd_image_test.go @@ -185,13 +185,8 @@ func TestContainerdImageInOtherNamespaces(t *testing.T) { } require.NoError(t, Consistently(checkImage, 100*time.Millisecond, time.Second)) - sbConfig := PodSandboxConfig("sandbox", "test") - t.Logf("pull the image into cri plugin") - id, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: id})) - }() + PodSandboxConfig("sandbox", "test") + EnsureImageExists(t, testImage) t.Logf("cri plugin should see the image now") img, err := imageService.ImageStatus(&runtime.ImageSpec{Image: testImage}) diff --git a/integration/imagefs_info_test.go b/integration/imagefs_info_test.go index f64653d78..dc9210c10 100644 --- a/integration/imagefs_info_test.go +++ b/integration/imagefs_info_test.go @@ -33,12 +33,8 @@ func TestImageFSInfo(t *testing.T) { config := PodSandboxConfig("running-pod", "imagefs") t.Logf("Pull an image to make sure image fs is not empty") - img, err := imageService.PullImage(&runtime.ImageSpec{Image: GetImage(BusyBox)}, nil, config) - require.NoError(t, err) - defer func() { - err := imageService.RemoveImage(&runtime.ImageSpec{Image: img}) - assert.NoError(t, err) - }() + EnsureImageExists(t, GetImage(BusyBox)) + t.Logf("Create a sandbox to make sure there is an active snapshot") sb, err := runtimeService.RunPodSandbox(config, *runtimeHandler) require.NoError(t, err) diff --git a/integration/pod_dualstack_test.go b/integration/pod_dualstack_test.go index 95c22b5ca..339460ab8 100644 --- a/integration/pod_dualstack_test.go +++ b/integration/pod_dualstack_test.go @@ -50,12 +50,8 @@ func TestPodDualStack(t *testing.T) { testImage = GetImage(BusyBox) containerName = "test-container" ) - t.Logf("Pull test image %q", testImage) - img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) - }() + + EnsureImageExists(t, testImage) t.Log("Create a container to print env") cnConfig := ContainerConfig( diff --git a/integration/pod_hostname_test.go b/integration/pod_hostname_test.go index 38a02c4b7..dcebee2b5 100644 --- a/integration/pod_hostname_test.go +++ b/integration/pod_hostname_test.go @@ -86,12 +86,8 @@ func TestPodHostname(t *testing.T) { testImage = GetImage(BusyBox) containerName = "test-container" ) - t.Logf("Pull test image %q", testImage) - img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) - }() + + EnsureImageExists(t, testImage) t.Log("Create a container to print env") cnConfig := ContainerConfig( diff --git a/integration/restart_test.go b/integration/restart_test.go index 2ba89fcfe..38dc8693e 100644 --- a/integration/restart_test.go +++ b/integration/restart_test.go @@ -135,11 +135,7 @@ func TestContainerdRestart(t *testing.T) { t.Logf("Pull test images") for _, image := range []string{GetImage(BusyBox), GetImage(Alpine)} { - img, err := imageService.PullImage(&runtime.ImageSpec{Image: image}, nil, nil) - require.NoError(t, err) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) - }() + EnsureImageExists(t, image) } imagesBeforeRestart, err := imageService.ListImages(nil) assert.NoError(t, err) diff --git a/integration/truncindex_test.go b/integration/truncindex_test.go index 67cddeb1e..be9ebc9d6 100644 --- a/integration/truncindex_test.go +++ b/integration/truncindex_test.go @@ -35,12 +35,9 @@ func TestTruncIndex(t *testing.T) { t.Logf("Pull an image") var appImage = GetImage(BusyBox) - imgID, err := imageService.PullImage(&runtimeapi.ImageSpec{Image: appImage}, nil, sbConfig) - require.NoError(t, err) + + imgID := EnsureImageExists(t, appImage) imgTruncID := genTruncIndex(imgID) - defer func() { - assert.NoError(t, imageService.RemoveImage(&runtimeapi.ImageSpec{Image: imgTruncID})) - }() t.Logf("Get image status by truncindex, truncID: %s", imgTruncID) res, err := imageService.ImageStatus(&runtimeapi.ImageSpec{Image: imgTruncID}) diff --git a/integration/volume_copy_up_test.go b/integration/volume_copy_up_test.go index d6840a516..c03212772 100644 --- a/integration/volume_copy_up_test.go +++ b/integration/volume_copy_up_test.go @@ -26,7 +26,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" ) func TestVolumeCopyUp(t *testing.T) { @@ -44,9 +43,7 @@ func TestVolumeCopyUp(t *testing.T) { assert.NoError(t, runtimeService.RemovePodSandbox(sb)) }() - t.Logf("Pull test image") - _, err = imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) + EnsureImageExists(t, testImage) t.Logf("Create a container with volume-copy-up test image") cnConfig := ContainerConfig( @@ -106,9 +103,7 @@ func TestVolumeOwnership(t *testing.T) { assert.NoError(t, runtimeService.RemovePodSandbox(sb)) }() - t.Logf("Pull test image") - _, err = imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil, sbConfig) - require.NoError(t, err) + EnsureImageExists(t, testImage) t.Logf("Create a container with volume-ownership test image") cnConfig := ContainerConfig(