Merge pull request #1448 from saschagrunert/pull-timeout

Add `--pull-timeout` flag to `crictl` `create`, `run` and `pull` commands

cmd/crictl/container.go

@@ -85,6 +85,9 @@ type pullOptions struct {
 	// Username to use for accessing the registry
 	// password will be requested on the command line
 	username string
+
+	// timeout is the maximum time used for the image pull
+	timeout time.Duration
 }
 
 var createPullFlags = []cli.Flag{
@@ -111,6 +114,17 @@ var createPullFlags = []cli.Flag{
 		Value: "",
 		Usage: "Use `USERNAME` for accessing the registry. The password will be requested on the command line",
 	},
+	&cli.DurationFlag{
+		Name:    "cancel-timeout",
+		Aliases: []string{"T"},
+		Usage:   "Seconds to wait for a container create request to complete before cancelling the request",
+	},
+	&cli.DurationFlag{
+		Name:    "pull-timeout",
+		Aliases: []string{"pt"},
+		Usage:   "Maximum time to be used for pulling the image, disabled if set to 0s",
+		EnvVars: []string{"CRICTL_PULL_TIMEOUT"},
+	},
 }
 
 var runPullFlags = []cli.Flag{
@@ -137,17 +151,29 @@ var runPullFlags = []cli.Flag{
 		Value: "",
 		Usage: "Use `USERNAME` for accessing the registry. password will be requested",
 	},
+	&cli.StringFlag{
+		Name:    "runtime",
+		Aliases: []string{"r"},
+		Usage:   "Runtime handler to use. Available options are defined by the container runtime.",
+	},
+	&cli.DurationFlag{
+		Name:    "timeout",
+		Aliases: []string{"t"},
+		Usage:   "Seconds to wait for a container create request before cancelling the request",
+	},
+	&cli.DurationFlag{
+		Name:    "pull-timeout",
+		Aliases: []string{"pt"},
+		Usage:   "Maximum time to be used for pulling the image, disabled if set to 0s",
+		EnvVars: []string{"CRICTL_PULL_TIMEOUT"},
+	},
 }
 
 var createContainerCommand = &cli.Command{
 	Name:      "create",
 	Usage:     "Create a new container",
 	ArgsUsage: "POD container-config.[json|yaml] pod-config.[json|yaml]",
-	Flags: append(createPullFlags, &cli.DurationFlag{
-		Name:    "cancel-timeout",
-		Aliases: []string{"T"},
-		Usage:   "Seconds to wait for a container create request to complete before cancelling the request",
-	}),
+	Flags:     createPullFlags,
 
 	Action: func(c *cli.Context) (err error) {
 		if c.Args().Len() != 3 {
@@ -177,6 +203,7 @@ var createContainerCommand = &cli.Command{
 					creds:    c.String("creds"),
 					auth:     c.String("auth"),
 					username: c.String("username"),
+					timeout:  c.Duration("pull-timeout"),
 				},
 				timeout: c.Duration("cancel-timeout"),
 			},
@@ -581,15 +608,7 @@ var runContainerCommand = &cli.Command{
 	Name:      "run",
 	Usage:     "Run a new container inside a sandbox",
 	ArgsUsage: "container-config.[json|yaml] pod-config.[json|yaml]",
-	Flags: append(runPullFlags, &cli.StringFlag{
-		Name:    "runtime",
-		Aliases: []string{"r"},
-		Usage:   "Runtime handler to use. Available options are defined by the container runtime.",
-	}, &cli.DurationFlag{
-		Name:    "timeout",
-		Aliases: []string{"t"},
-		Usage:   "Seconds to wait for a container create request before cancelling the request",
-	}),
+	Flags:     runPullFlags,
 
 	Action: func(c *cli.Context) (err error) {
 		if c.Args().Len() != 2 {
@@ -617,6 +636,7 @@ var runContainerCommand = &cli.Command{
 				creds:    c.String("creds"),
 				auth:     c.String("auth"),
 				username: c.String("username"),
+				timeout:  c.Duration("pull-timeout"),
 			},
 			timeout: c.Duration("timeout"),
 		}
@@ -747,7 +767,7 @@ func CreateContainer(
 
 		// Try to pull the image before container creation
 		ann := config.GetImage().GetAnnotations()
-		if _, err := PullImageWithSandbox(iClient, image, auth, podConfig, ann); err != nil {
+		if _, err := PullImageWithSandbox(iClient, image, auth, podConfig, ann, opts.pullOptions.timeout); err != nil {
 			return "", err
 		}
 	}

cmd/crictl/image.go

@@ -26,6 +26,7 @@ import (
 	"strconv"
 	"strings"
 	"syscall"
+	"time"
 
 	"github.com/docker/go-units"
 	"github.com/sirupsen/logrus"
@@ -83,6 +84,12 @@ var pullImageCommand = &cli.Command{
 			Aliases: []string{"a"},
 			Usage:   "Annotation to be set on the pulled image",
 		},
+		&cli.DurationFlag{
+			Name:    "pull-timeout",
+			Aliases: []string{"pt"},
+			Usage:   "Maximum time to be used for pulling the image, disabled if set to 0s",
+			EnvVars: []string{"CRICTL_PULL_TIMEOUT"},
+		},
 	},
 	ArgsUsage: "NAME[:TAG|@DIGEST]",
 	Action: func(c *cli.Context) error {
@@ -119,7 +126,8 @@ var pullImageCommand = &cli.Command{
 				return err
 			}
 		}
-		r, err := PullImageWithSandbox(imageClient, imageName, auth, sandbox, ann)
+		timeout := c.Duration("timeout")
+		r, err := PullImageWithSandbox(imageClient, imageName, auth, sandbox, ann, timeout)
 		if err != nil {
 			return fmt.Errorf("pulling image: %w", err)
 		}
@@ -633,7 +641,7 @@ func normalizeRepoDigest(repoDigests []string) (string, string) {
 
 // PullImageWithSandbox sends a PullImageRequest to the server, and parses
 // the returned PullImageResponse.
-func PullImageWithSandbox(client internalapi.ImageManagerService, image string, auth *pb.AuthConfig, sandbox *pb.PodSandboxConfig, ann map[string]string) (*pb.PullImageResponse, error) {
+func PullImageWithSandbox(client internalapi.ImageManagerService, image string, auth *pb.AuthConfig, sandbox *pb.PodSandboxConfig, ann map[string]string, timeout time.Duration) (*pb.PullImageResponse, error) {
 	request := &pb.PullImageRequest{
 		Image: &pb.ImageSpec{
 			Image:       image,
@@ -647,7 +655,21 @@ func PullImageWithSandbox(client internalapi.ImageManagerService, image string,
 		request.SandboxConfig = sandbox
 	}
 	logrus.Debugf("PullImageRequest: %v", request)
-	res, err := client.PullImage(context.TODO(), request.Image, request.Auth, request.SandboxConfig)
+
+	if timeout < 0 {
+		return nil, errors.New("timeout should be bigger than 0")
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	if timeout > 0 {
+		logrus.Debugf("Using context with timeout of %s", timeout)
+		ctx, cancel = context.WithTimeout(ctx, timeout)
+		defer cancel()
+	}
+
+	res, err := client.PullImage(ctx, request.Image, request.Auth, request.SandboxConfig)
 	if err != nil {
 		return nil, err
 	}

go.mod

@@ -26,6 +26,7 @@ require (
 	golang.org/x/sys v0.21.0
 	golang.org/x/term v0.21.0
 	golang.org/x/text v0.16.0
+	google.golang.org/grpc v1.64.0
 	gopkg.in/yaml.v3 v3.0.1
 	k8s.io/api v0.30.1
 	k8s.io/apimachinery v0.30.1
@@ -85,7 +86,6 @@ require (
 	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240520151616-dc85e6b867a5 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240515191416-fc5f0ca64291 // indirect
-	google.golang.org/grpc v1.64.0 // indirect
 	google.golang.org/protobuf v1.34.1 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect

pkg/framework/util.go

@@ -328,9 +328,8 @@ func ListImage(c internalapi.ImageManagerService, filter *runtimeapi.ImageFilter
 	return images
 }
 
-// PullPublicImage pulls the public image named imageName.
-func PullPublicImage(c internalapi.ImageManagerService, imageName string, podConfig *runtimeapi.PodSandboxConfig) string {
-
+// PrepareImageName builds a pullable image name for the provided one.
+func PrepareImageName(imageName string) string {
 	ref, err := reference.ParseNamed(imageName)
 	if err == nil {
 		// Modify the image if it's a fully qualified image name
@@ -352,6 +351,13 @@ func PullPublicImage(c internalapi.ImageManagerService, imageName string, podCon
 		Failf("Unable to parse imageName: %v", err)
 	}
 
+	return imageName
+}
+
+// PullPublicImage pulls the public image named imageName.
+func PullPublicImage(c internalapi.ImageManagerService, imageName string, podConfig *runtimeapi.PodSandboxConfig) string {
+	imageName = PrepareImageName(imageName)
+
 	By("Pull image : " + imageName)
 	imageSpec := &runtimeapi.ImageSpec{
 		Image: imageName,

pkg/validate/image.go

@@ -21,7 +21,10 @@ import (
 	"fmt"
 	"runtime"
 	"sort"
+	"time"
 
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	internalapi "k8s.io/cri-api/pkg/apis"
 	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
 	"sigs.k8s.io/cri-tools/pkg/framework"
@@ -45,6 +48,21 @@ var _ = framework.KubeDescribe("Image Manager", func() {
 		})
 	})
 
+	It("public image should timeout if requested [Conformance]", func() {
+		imageName := framework.PrepareImageName(testImageWithTag)
+
+		ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond)
+		defer cancel()
+		res, err := c.PullImage(ctx, &runtimeapi.ImageSpec{Image: imageName}, nil, nil)
+
+		Expect(res).To(BeEmpty())
+		Expect(err).To(HaveOccurred())
+
+		statusErr, ok := status.FromError(err)
+		Expect(ok).To(BeTrue())
+		Expect(statusErr.Code()).To(Equal(codes.DeadlineExceeded))
+	})
+
 	It("public image without tag should be pulled and removed [Conformance]", func() {
 		testPullPublicImage(c, testImageWithoutTag, testImagePodSandbox, func(s *runtimeapi.Image) {
 			Expect(s.RepoTags).To(Equal([]string{testImageWithoutTag + ":latest"}))