Merge pull request containerd#9815 from kiashok/updateCRIServicePull

Pass runtimehandler from CRI to pull image request

Author: mikebrow

integration/cri-api/pkg/apis/services.go

@@ -140,7 +140,7 @@ type ImageManagerService interface {
 	// ImageStatus returns the status of the image.
 	ImageStatus(image *runtimeapi.ImageSpec, opts ...grpc.CallOption) (*runtimeapi.Image, error)
 	// PullImage pulls an image with the authentication config.
-	PullImage(image *runtimeapi.ImageSpec, auth *runtimeapi.AuthConfig, podSandboxConfig *runtimeapi.PodSandboxConfig, opts ...grpc.CallOption) (string, error)
+	PullImage(image *runtimeapi.ImageSpec, auth *runtimeapi.AuthConfig, podSandboxConfig *runtimeapi.PodSandboxConfig, runtimeHandler string, opts ...grpc.CallOption) (string, error)
 	// RemoveImage removes the image.
 	RemoveImage(image *runtimeapi.ImageSpec, opts ...grpc.CallOption) error
 	// ImageFsInfo returns information of the filesystem that is used to store images.

integration/image_pull_timeout_test.go

@@ -92,7 +92,7 @@ func testCRIImagePullTimeoutBySlowCommitWriter(t *testing.T) {
 
 	ctx := namespaces.WithNamespace(logtest.WithT(context.Background(), t), k8sNamespace)
 
-	_, err = criService.PullImage(ctx, pullProgressTestImageName, nil, nil)
+	_, err = criService.PullImage(ctx, pullProgressTestImageName, nil, nil, "")
 	assert.NoError(t, err)
 }
 
@@ -211,7 +211,7 @@ func testCRIImagePullTimeoutByHoldingContentOpenWriter(t *testing.T) {
 	go func() {
 		defer close(errCh)
 
-		_, err := criService.PullImage(ctx, pullProgressTestImageName, nil, nil)
+		_, err := criService.PullImage(ctx, pullProgressTestImageName, nil, nil, "")
 		errCh <- err
 	}()
 
@@ -294,7 +294,7 @@ func testCRIImagePullTimeoutByNoDataTransferred(t *testing.T) {
 		dctx, _, err := cli.WithLease(ctx)
 		assert.NoError(t, err)
 
-		_, err = criService.PullImage(dctx, fmt.Sprintf("%s/%s", mirrorURL.Host, "containerd/volume-ownership:2.1"), nil, nil)
+		_, err = criService.PullImage(dctx, fmt.Sprintf("%s/%s", mirrorURL.Host, "containerd/volume-ownership:2.1"), nil, nil, "")
 
 		assert.Equal(t, context.Canceled, errors.Unwrap(err), "[%v] expected canceled error, but got (%v)", idx, err)
 		assert.True(t, mirrorSrv.limiter.clearHitCircuitBreaker(), "[%v] expected to hit circuit breaker", idx)

integration/main_test.go

@@ -735,7 +735,7 @@ func EnsureImageExists(t *testing.T, imageName string) string {
 	}
 
 	t.Logf("Pull test image %q", imageName)
-	imgID, err := imageService.PullImage(&runtime.ImageSpec{Image: imageName}, nil, nil)
+	imgID, err := imageService.PullImage(&runtime.ImageSpec{Image: imageName}, nil, nil, "")
 	require.NoError(t, err)
 
 	return imgID

integration/release_upgrade_linux_test.go

@@ -116,7 +116,7 @@ func shouldRecoverAllThePodsAfterUpgrade(t *testing.T, criRuntimeService cri.Run
 	var busyboxImage = images.Get(images.BusyBox)
 
 	t.Logf("Pulling image %q", busyboxImage)
-	_, err := criImageService.PullImage(&criruntime.ImageSpec{Image: busyboxImage}, nil, nil)
+	_, err := criImageService.PullImage(&criruntime.ImageSpec{Image: busyboxImage}, nil, nil, "")
 	require.NoError(t, err)
 
 	t.Log("Create first sandbox")
@@ -195,7 +195,7 @@ func execToExistingContainer(t *testing.T, criRuntimeService cri.RuntimeService,
 	var busyboxImage = images.Get(images.BusyBox)
 
 	t.Logf("Pulling image %q", busyboxImage)
-	_, err := criImageService.PullImage(&criruntime.ImageSpec{Image: busyboxImage}, nil, nil)
+	_, err := criImageService.PullImage(&criruntime.ImageSpec{Image: busyboxImage}, nil, nil, "")
 	require.NoError(t, err)
 	t.Log("Create sandbox")
 	sbConfig := PodSandboxConfig("sandbox", "running")
@@ -263,7 +263,7 @@ func shouldManipulateContainersInPodAfterUpgrade(t *testing.T, criRuntimeService
 	var busyboxImage = images.Get(images.BusyBox)
 
 	t.Logf("Pulling image %q", busyboxImage)
-	_, err := criImageService.PullImage(&criruntime.ImageSpec{Image: busyboxImage}, nil, nil)
+	_, err := criImageService.PullImage(&criruntime.ImageSpec{Image: busyboxImage}, nil, nil, "")
 	require.NoError(t, err)
 
 	t.Log("Create a sandbox")
@@ -353,7 +353,7 @@ func shouldRecoverExistingImages(t *testing.T, criRuntimeService cri.RuntimeServ
 	expectedRefs := []string{}
 	for _, img := range images {
 		t.Logf("Pulling image %q", img)
-		imgRef, err := criImageService.PullImage(&criruntime.ImageSpec{Image: img}, nil, nil)
+		imgRef, err := criImageService.PullImage(&criruntime.ImageSpec{Image: img}, nil, nil, "")
 		require.NoError(t, err)
 		expectedRefs = append(expectedRefs, imgRef)
 	}

integration/remote/remote_image.go

@@ -122,7 +122,7 @@ func (r *ImageService) ImageStatus(image *runtimeapi.ImageSpec, opts ...grpc.Cal
 }
 
 // PullImage pulls an image with authentication config.
-func (r *ImageService) PullImage(image *runtimeapi.ImageSpec, auth *runtimeapi.AuthConfig, podSandboxConfig *runtimeapi.PodSandboxConfig, opts ...grpc.CallOption) (string, error) {
+func (r *ImageService) PullImage(image *runtimeapi.ImageSpec, auth *runtimeapi.AuthConfig, podSandboxConfig *runtimeapi.PodSandboxConfig, runtimeHandler string, opts ...grpc.CallOption) (string, error) {
 	ctx, cancel := getContextWithCancel()
 	defer cancel()
 

internal/cri/server/container_status_test.go

@@ -289,7 +289,7 @@ func (s *fakeImageService) LocalResolve(refOrID string) (imagestore.Image, error
 
 func (s *fakeImageService) ImageFSPaths() map[string]string { return make(map[string]string) }
 
-func (s *fakeImageService) PullImage(context.Context, string, func(string) (string, string, error), *runtime.PodSandboxConfig) (string, error) {
+func (s *fakeImageService) PullImage(context.Context, string, func(string) (string, string, error), *runtime.PodSandboxConfig, string) (string, error) {
 	return "", errors.New("not implemented")
 }
 

internal/cri/server/images/image_pull.go

@@ -108,15 +108,14 @@ func (c *GRPCCRIImageService) PullImage(ctx context.Context, r *runtime.PullImag
 		return ParseAuth(hostauth, host)
 	}
 
-	ref, err := c.CRIImageService.PullImage(ctx, imageRef, credentials, r.SandboxConfig)
+	ref, err := c.CRIImageService.PullImage(ctx, imageRef, credentials, r.SandboxConfig, r.GetImage().GetRuntimeHandler())
 	if err != nil {
 		return nil, err
 	}
 	return &runtime.PullImageResponse{ImageRef: ref}, nil
 }
 
-func (c *CRIImageService) PullImage(ctx context.Context, name string, credentials func(string) (string, string, error), sandboxConfig *runtime.PodSandboxConfig) (_ string, err error) {
-
+func (c *CRIImageService) PullImage(ctx context.Context, name string, credentials func(string) (string, string, error), sandboxConfig *runtime.PodSandboxConfig, runtimeHandler string) (_ string, err error) {
 	span := tracing.SpanFromContext(ctx)
 	defer func() {
 		// TODO: add domain label for imagePulls metrics, and we may need to provide a mechanism
@@ -770,6 +769,8 @@ func (c *CRIImageService) snapshotterFromPodSandboxConfig(ctx context.Context, i
 		return snapshotter, nil
 	}
 
+	// TODO(kiashok): honor the new CRI runtime handler field added to v0.29.0
+	// for image pull per runtime class support.
 	runtimeHandler, ok := s.Annotations[annotations.RuntimeHandler]
 	if !ok {
 		return snapshotter, nil

internal/cri/server/podsandbox/controller.go

@@ -107,7 +107,7 @@ type RuntimeService interface {
 type ImageService interface {
 	LocalResolve(refOrID string) (imagestore.Image, error)
 	GetImage(id string) (imagestore.Image, error)
-	PullImage(ctx context.Context, name string, creds func(string) (string, string, error), sc *runtime.PodSandboxConfig) (string, error)
+	PullImage(ctx context.Context, name string, creds func(string) (string, string, error), sc *runtime.PodSandboxConfig, runtimeHandler string) (string, error)
 	RuntimeSnapshotter(ctx context.Context, ociRuntime criconfig.Runtime) string
 	PinnedImage(string) string
 }

internal/cri/server/podsandbox/sandbox_run.go

@@ -80,7 +80,7 @@ func (c *Controller) Start(ctx context.Context, id string) (cin sandbox.Controll
 		sandboxImage = criconfig.DefaultSandboxImage
 	}
 	// Ensure sandbox container image snapshot.
-	image, err := c.ensureImageExists(ctx, sandboxImage, config)
+	image, err := c.ensureImageExists(ctx, sandboxImage, config, metadata.RuntimeHandler)
 	if err != nil {
 		return cin, fmt.Errorf("failed to get sandbox image %q: %w", sandboxImage, err)
 	}
@@ -293,7 +293,7 @@ func (c *Controller) Create(_ctx context.Context, info sandbox.Sandbox, opts ...
 	return c.store.Save(podSandbox)
 }
 
-func (c *Controller) ensureImageExists(ctx context.Context, ref string, config *runtime.PodSandboxConfig) (*imagestore.Image, error) {
+func (c *Controller) ensureImageExists(ctx context.Context, ref string, config *runtime.PodSandboxConfig, runtimeHandler string) (*imagestore.Image, error) {
 	image, err := c.imageService.LocalResolve(ref)
 	if err != nil && !errdefs.IsNotFound(err) {
 		return nil, fmt.Errorf("failed to get image %q: %w", ref, err)
@@ -303,7 +303,7 @@ func (c *Controller) ensureImageExists(ctx context.Context, ref string, config *
 	}
 	// Pull image to ensure the image exists
 	// TODO: Cleaner interface
-	imageID, err := c.imageService.PullImage(ctx, ref, nil, config)
+	imageID, err := c.imageService.PullImage(ctx, ref, nil, config, runtimeHandler)
 	if err != nil {
 		return nil, fmt.Errorf("failed to pull image %q: %w", ref, err)
 	}

internal/cri/server/service.go

@@ -78,7 +78,7 @@ type RuntimeService interface {
 type ImageService interface {
 	RuntimeSnapshotter(ctx context.Context, ociRuntime criconfig.Runtime) string
 
-	PullImage(ctx context.Context, name string, credentials func(string) (string, string, error), sandboxConfig *runtime.PodSandboxConfig) (string, error)
+	PullImage(ctx context.Context, name string, credentials func(string) (string, string, error), sandboxConfig *runtime.PodSandboxConfig, runtimeHandler string) (string, error)
 	UpdateImage(ctx context.Context, r string) error
 
 	CheckImages(ctx context.Context) error