Merge pull request #40804 from runcom/prepull-cri

Automatic merge from submit-queue

test/e2e_node: prepull images with CRI

Part of https://github.com/kubernetes/kubernetes/issues/40739

- This PR builds on top of #40525 (and contains one commit from #40525)
- The second commit contains a tiny change in the `Makefile`.
- Third commit is a patch to be able to prepull images using the CRI (as opposed to run `docker` to pull images which doesn't make sense if you're using CRI most of the times)

Marked WIP till #40525 makes its way into master

@Random-Liu @lucab @yujuhong @mrunalp @rhatdan

Makefile

@@ -233,11 +233,15 @@ define TEST_E2E_NODE_HELP_INFO
 #  PARALLELISM: The number of gingko nodes to run.  Defaults to 8.
 #  RUNTIME: Container runtime to use (eg. docker, rkt, remote).
 #    Defaults to "docker".
+#  CONTAINER_RUNTIME_ENDPOINT: remote container endpoint to connect to.
+#   Used when RUNTIME is set to "remote".
+#  IMAGE_SERVICE_ENDPOINT: remote image endpoint to connect to, to prepull images.
+#   Used when RUNTIME is set to "remote".
 #
 # Example:
 #   make test-e2e-node FOCUS=Kubelet SKIP=container
 #   make test-e2e-node REMOTE=true DELETE_INSTANCES=true
-#   make test-e2e-node TEST_ARGS="--cgroups-per-qos=true"
+#   make test-e2e-node TEST_ARGS='--kubelet-flags="--cgroups-per-qos=true"'
 # Build and run tests.
 endef
 .PHONY: test-e2e-node

hack/make-rules/test-e2e-node.sh

@@ -29,6 +29,8 @@ parallelism=${PARALLELISM:-8}
 artifacts=${ARTIFACTS:-"/tmp/_artifacts/`date +%y%m%dT%H%M%S`"}
 remote=${REMOTE:-"false"}
 runtime=${RUNTIME:-"docker"}
+container_runtime_endpoint=${CONTAINER_RUNTIME_ENDPOINT:-""}
+image_service_endpoint=${IMAGE_SERVICE_ENDPOINT:-""}
 run_until_failure=${RUN_UNTIL_FAILURE:-"false"}
 test_args=${TEST_ARGS:-""}
 
@@ -149,12 +151,21 @@ else
   test_args='--kubelet-flags="--container-runtime='$runtime'" '$test_args
   if [[ $runtime == "remote" ]] ; then
       test_args='--kubelet-flags="--experimental-cri=true" '$test_args
+      if [[ ! -z $container_runtime_endpoint ]] ; then
+	      test_args='--kubelet-flags="--container-runtime-endpoint='$container_runtime_endpoint'" '$test_args
+      fi
+      if [[ ! -z $image_service_endpoint ]] ; then
+	      test_args='--kubelet-flags="--image-service-endpoint='$image_service_endpoint'" '$test_args
+      fi
   fi
 
   # Test using the host the script was run on
   # Provided for backwards compatibility
   go run test/e2e_node/runner/local/run_local.go --ginkgo-flags="$ginkgoflags" \
-    --test-flags="--container-runtime=${runtime} --alsologtostderr --v 4 --report-dir=${artifacts} --node-name $(hostname) \
+    --test-flags="--container-runtime=${runtime} \
+    --container-runtime-endpoint=${container_runtime_endpoint} \
+    --image-service-endpoint=${image_service_endpoint} \
+    --alsologtostderr --v 4 --report-dir=${artifacts} --node-name $(hostname) \
     $test_args" --build-dependencies=true 2>&1 | tee -i "${artifacts}/build-log.txt"
   exit $?
 fi

pkg/kubelet/remote/remote_image.go

@@ -35,16 +35,16 @@ type RemoteImageService struct {
 }
 
 // NewRemoteImageService creates a new internalapi.ImageManagerService.
-func NewRemoteImageService(addr string, connectionTimout time.Duration) (internalapi.ImageManagerService, error) {
+func NewRemoteImageService(addr string, connectionTimeout time.Duration) (internalapi.ImageManagerService, error) {
 	glog.V(3).Infof("Connecting to image service %s", addr)
-	conn, err := grpc.Dial(addr, grpc.WithInsecure(), grpc.WithTimeout(connectionTimout), grpc.WithDialer(dial))
+	conn, err := grpc.Dial(addr, grpc.WithInsecure(), grpc.WithTimeout(connectionTimeout), grpc.WithDialer(dial))
 	if err != nil {
 		glog.Errorf("Connect remote image service %s failed: %v", addr, err)
 		return nil, err
 	}
 
 	return &RemoteImageService{
-		timeout:     connectionTimout,
+		timeout:     connectionTimeout,
 		imageClient: runtimeapi.NewImageServiceClient(conn),
 	}, nil
 }

test/e2e/framework/test_context.go

@@ -55,6 +55,8 @@ type TestContextType struct {
 	GCEUpgradeScript         string
 	PrometheusPushGateway    string
 	ContainerRuntime         string
+	ContainerRuntimeEndpoint string
+	ImageServiceEndpoint     string
 	MasterOSDistro           string
 	NodeOSDistro             string
 	VerifyServiceAccount     bool
@@ -165,6 +167,8 @@ func RegisterCommonFlags() {
 	flag.StringVar(&TestContext.FeatureGates, "feature-gates", "", "A set of key=value pairs that describe feature gates for alpha/experimental features.")
 	flag.StringVar(&TestContext.Viper, "viper-config", "e2e", "The name of the viper config i.e. 'e2e' will read values from 'e2e.json' locally.  All e2e parameters are meant to be configurable by viper.")
 	flag.StringVar(&TestContext.ContainerRuntime, "container-runtime", "docker", "The container runtime of cluster VM instances (docker/rkt/remote).")
+	flag.StringVar(&TestContext.ContainerRuntimeEndpoint, "container-runtime-endpoint", "", "The container runtime endpoint of cluster VM instances.")
+	flag.StringVar(&TestContext.ImageServiceEndpoint, "image-service-endpoint", "", "The image service endpoint of cluster VM instances.")
 }
 
 // Register flags specific to the cluster e2e test suite.

test/e2e_node/BUILD

@@ -29,7 +29,10 @@ go_library(
         "//pkg/apis/componentconfig/v1alpha1:go_default_library",
         "//pkg/client/clientset_generated/clientset:go_default_library",
         "//pkg/client/clientset_generated/clientset/typed/core/v1:go_default_library",
+        "//pkg/kubelet/api:go_default_library",
+        "//pkg/kubelet/api/v1alpha1/runtime:go_default_library",
         "//pkg/kubelet/api/v1alpha1/stats:go_default_library",
+        "//pkg/kubelet/remote:go_default_library",
         "//pkg/util/procfs:go_default_library",
         "//test/e2e/common:go_default_library",
         "//test/e2e/framework:go_default_library",

test/e2e_node/image_list.go

@@ -17,6 +17,8 @@ limitations under the License.
 package e2e_node
 
 import (
+	"errors"
+	"fmt"
 	"os/exec"
 	"os/user"
 	"time"
@@ -24,6 +26,9 @@ import (
 	"github.com/golang/glog"
 
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/kubernetes/pkg/kubelet/api"
+	"k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
+	"k8s.io/kubernetes/pkg/kubelet/remote"
 	commontest "k8s.io/kubernetes/test/e2e/common"
 	"k8s.io/kubernetes/test/e2e/framework"
 )
@@ -33,6 +38,8 @@ const (
 	maxImagePullRetries = 5
 	// Sleep duration between image pull retry attempts.
 	imagePullRetryDelay = time.Second
+	// connection timeout for gRPC image service connection
+	imageServiceConnectionTimeout = 15 * time.Minute
 )
 
 // NodeImageWhiteList is a list of images used in node e2e test. These images will be prepulled
@@ -54,14 +61,79 @@ func init() {
 	framework.ImageWhiteList = NodeImageWhiteList.Union(commontest.CommonImageWhiteList)
 }
 
+// puller represents a generic image puller
+type puller interface {
+	// Pull pulls an image by name
+	Pull(image string) ([]byte, error)
+	// Name returns the name of the specific puller implementation
+	Name() string
+}
+
+type dockerPuller struct {
+}
+
+func (dp *dockerPuller) Name() string {
+	return "docker"
+}
+
+func (dp *dockerPuller) Pull(image string) ([]byte, error) {
+	// TODO(random-liu): Use docker client to get rid of docker binary dependency.
+	return exec.Command("docker", "pull", image).CombinedOutput()
+}
+
+type remotePuller struct {
+	imageService api.ImageManagerService
+}
+
+func (rp *remotePuller) Name() string {
+	return "CRI"
+}
+
+func (rp *remotePuller) Pull(image string) ([]byte, error) {
+	// TODO(runcom): should we check if the image is already pulled with ImageStatus?
+	_, err := rp.imageService.PullImage(&runtime.ImageSpec{Image: image}, nil)
+	return nil, err
+}
+
+func getPuller() (puller, error) {
+	runtime := framework.TestContext.ContainerRuntime
+	switch runtime {
+	case "docker":
+		return &dockerPuller{}, nil
+	case "remote":
+		endpoint := framework.TestContext.ContainerRuntimeEndpoint
+		if framework.TestContext.ImageServiceEndpoint != "" {
+			//ImageServiceEndpoint is the same as ContainerRuntimeEndpoint if not
+			//explicitly specified
+			//https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/kubelet.go#L517
+			endpoint = framework.TestContext.ImageServiceEndpoint
+		}
+		if endpoint == "" {
+			return nil, errors.New("can't prepull images, no remote endpoint provided")
+		}
+		is, err := remote.NewRemoteImageService(endpoint, imageServiceConnectionTimeout)
+		if err != nil {
+			return nil, err
+		}
+		return &remotePuller{
+			imageService: is,
+		}, nil
+	}
+	return nil, fmt.Errorf("can't prepull images, unknown container runtime %q", runtime)
+}
+
 // Pre-fetch all images tests depend on so that we don't fail in an actual test.
 func PrePullAllImages() error {
+	puller, err := getPuller()
+	if err != nil {
+		return err
+	}
 	usr, err := user.Current()
 	if err != nil {
 		return err
 	}
 	images := framework.ImageWhiteList.List()
-	glog.V(4).Infof("Pre-pulling images %+v", images)
+	glog.V(4).Infof("Pre-pulling images with %s %+v", puller.Name(), images)
 	for _, image := range images {
 		var (
 			err    error
@@ -71,15 +143,14 @@ func PrePullAllImages() error {
 			if i > 0 {
 				time.Sleep(imagePullRetryDelay)
 			}
-			// TODO(random-liu): Use docker client to get rid of docker binary dependency.
+			if output, err = puller.Pull(image); err == nil {
-			if output, err = exec.Command("docker", "pull", image).CombinedOutput(); err == nil {
 				break
 			}
 			glog.Warningf("Failed to pull %s as user %q, retrying in %s (%d of %d): %v",
 				image, usr.Username, imagePullRetryDelay.String(), i+1, maxImagePullRetries, err)
 		}
 		if err != nil {
-			glog.Warningf("Could not pre-pull image %s %v output:  %s", image, err, output)
+			glog.Warningf("Could not pre-pull image %s %v output: %s", image, err, output)
 			return err
 		}
 	}