Skip to content

Commit

Permalink
Add --pull-timeout flag to crictl create, run and pull comm…
Browse files Browse the repository at this point in the history
…ands

This allows to set a pull context timeout for the supported commands.
Runtimes may or may not use the timeout from the RPC for image pulls,
but `crictl` should generally support that feature.

Signed-off-by: Sascha Grunert <[email protected]>
  • Loading branch information
saschagrunert committed Jun 11, 2024
1 parent 6c92933 commit 780d044
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 22 deletions.
50 changes: 35 additions & 15 deletions cmd/crictl/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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 {
Expand Down Expand Up @@ -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"),
},
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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"),
}
Expand Down Expand Up @@ -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
}
}
Expand Down
28 changes: 25 additions & 3 deletions cmd/crictl/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"strconv"
"strings"
"syscall"
"time"

"github.com/docker/go-units"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions pkg/framework/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions pkg/validate/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"}))
Expand Down

0 comments on commit 780d044

Please sign in to comment.