Skip to content

Commit 780d044

Browse files
committed
Add --pull-timeout flag to crictl create, run and pull commands
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]>
1 parent 6c92933 commit 780d044

File tree

5 files changed

+88
-22
lines changed

5 files changed

+88
-22
lines changed

Diff for: cmd/crictl/container.go

+35-15
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ type pullOptions struct {
8585
// Username to use for accessing the registry
8686
// password will be requested on the command line
8787
username string
88+
89+
// timeout is the maximum time used for the image pull
90+
timeout time.Duration
8891
}
8992

9093
var createPullFlags = []cli.Flag{
@@ -111,6 +114,17 @@ var createPullFlags = []cli.Flag{
111114
Value: "",
112115
Usage: "Use `USERNAME` for accessing the registry. The password will be requested on the command line",
113116
},
117+
&cli.DurationFlag{
118+
Name: "cancel-timeout",
119+
Aliases: []string{"T"},
120+
Usage: "Seconds to wait for a container create request to complete before cancelling the request",
121+
},
122+
&cli.DurationFlag{
123+
Name: "pull-timeout",
124+
Aliases: []string{"pt"},
125+
Usage: "Maximum time to be used for pulling the image, disabled if set to 0s",
126+
EnvVars: []string{"CRICTL_PULL_TIMEOUT"},
127+
},
114128
}
115129

116130
var runPullFlags = []cli.Flag{
@@ -137,17 +151,29 @@ var runPullFlags = []cli.Flag{
137151
Value: "",
138152
Usage: "Use `USERNAME` for accessing the registry. password will be requested",
139153
},
154+
&cli.StringFlag{
155+
Name: "runtime",
156+
Aliases: []string{"r"},
157+
Usage: "Runtime handler to use. Available options are defined by the container runtime.",
158+
},
159+
&cli.DurationFlag{
160+
Name: "timeout",
161+
Aliases: []string{"t"},
162+
Usage: "Seconds to wait for a container create request before cancelling the request",
163+
},
164+
&cli.DurationFlag{
165+
Name: "pull-timeout",
166+
Aliases: []string{"pt"},
167+
Usage: "Maximum time to be used for pulling the image, disabled if set to 0s",
168+
EnvVars: []string{"CRICTL_PULL_TIMEOUT"},
169+
},
140170
}
141171

142172
var createContainerCommand = &cli.Command{
143173
Name: "create",
144174
Usage: "Create a new container",
145175
ArgsUsage: "POD container-config.[json|yaml] pod-config.[json|yaml]",
146-
Flags: append(createPullFlags, &cli.DurationFlag{
147-
Name: "cancel-timeout",
148-
Aliases: []string{"T"},
149-
Usage: "Seconds to wait for a container create request to complete before cancelling the request",
150-
}),
176+
Flags: createPullFlags,
151177

152178
Action: func(c *cli.Context) (err error) {
153179
if c.Args().Len() != 3 {
@@ -177,6 +203,7 @@ var createContainerCommand = &cli.Command{
177203
creds: c.String("creds"),
178204
auth: c.String("auth"),
179205
username: c.String("username"),
206+
timeout: c.Duration("pull-timeout"),
180207
},
181208
timeout: c.Duration("cancel-timeout"),
182209
},
@@ -581,15 +608,7 @@ var runContainerCommand = &cli.Command{
581608
Name: "run",
582609
Usage: "Run a new container inside a sandbox",
583610
ArgsUsage: "container-config.[json|yaml] pod-config.[json|yaml]",
584-
Flags: append(runPullFlags, &cli.StringFlag{
585-
Name: "runtime",
586-
Aliases: []string{"r"},
587-
Usage: "Runtime handler to use. Available options are defined by the container runtime.",
588-
}, &cli.DurationFlag{
589-
Name: "timeout",
590-
Aliases: []string{"t"},
591-
Usage: "Seconds to wait for a container create request before cancelling the request",
592-
}),
611+
Flags: runPullFlags,
593612

594613
Action: func(c *cli.Context) (err error) {
595614
if c.Args().Len() != 2 {
@@ -617,6 +636,7 @@ var runContainerCommand = &cli.Command{
617636
creds: c.String("creds"),
618637
auth: c.String("auth"),
619638
username: c.String("username"),
639+
timeout: c.Duration("pull-timeout"),
620640
},
621641
timeout: c.Duration("timeout"),
622642
}
@@ -747,7 +767,7 @@ func CreateContainer(
747767

748768
// Try to pull the image before container creation
749769
ann := config.GetImage().GetAnnotations()
750-
if _, err := PullImageWithSandbox(iClient, image, auth, podConfig, ann); err != nil {
770+
if _, err := PullImageWithSandbox(iClient, image, auth, podConfig, ann, opts.pullOptions.timeout); err != nil {
751771
return "", err
752772
}
753773
}

Diff for: cmd/crictl/image.go

+25-3
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"strconv"
2727
"strings"
2828
"syscall"
29+
"time"
2930

3031
"github.com/docker/go-units"
3132
"github.com/sirupsen/logrus"
@@ -83,6 +84,12 @@ var pullImageCommand = &cli.Command{
8384
Aliases: []string{"a"},
8485
Usage: "Annotation to be set on the pulled image",
8586
},
87+
&cli.DurationFlag{
88+
Name: "pull-timeout",
89+
Aliases: []string{"pt"},
90+
Usage: "Maximum time to be used for pulling the image, disabled if set to 0s",
91+
EnvVars: []string{"CRICTL_PULL_TIMEOUT"},
92+
},
8693
},
8794
ArgsUsage: "NAME[:TAG|@DIGEST]",
8895
Action: func(c *cli.Context) error {
@@ -119,7 +126,8 @@ var pullImageCommand = &cli.Command{
119126
return err
120127
}
121128
}
122-
r, err := PullImageWithSandbox(imageClient, imageName, auth, sandbox, ann)
129+
timeout := c.Duration("timeout")
130+
r, err := PullImageWithSandbox(imageClient, imageName, auth, sandbox, ann, timeout)
123131
if err != nil {
124132
return fmt.Errorf("pulling image: %w", err)
125133
}
@@ -633,7 +641,7 @@ func normalizeRepoDigest(repoDigests []string) (string, string) {
633641

634642
// PullImageWithSandbox sends a PullImageRequest to the server, and parses
635643
// the returned PullImageResponse.
636-
func PullImageWithSandbox(client internalapi.ImageManagerService, image string, auth *pb.AuthConfig, sandbox *pb.PodSandboxConfig, ann map[string]string) (*pb.PullImageResponse, error) {
644+
func PullImageWithSandbox(client internalapi.ImageManagerService, image string, auth *pb.AuthConfig, sandbox *pb.PodSandboxConfig, ann map[string]string, timeout time.Duration) (*pb.PullImageResponse, error) {
637645
request := &pb.PullImageRequest{
638646
Image: &pb.ImageSpec{
639647
Image: image,
@@ -647,7 +655,21 @@ func PullImageWithSandbox(client internalapi.ImageManagerService, image string,
647655
request.SandboxConfig = sandbox
648656
}
649657
logrus.Debugf("PullImageRequest: %v", request)
650-
res, err := client.PullImage(context.TODO(), request.Image, request.Auth, request.SandboxConfig)
658+
659+
if timeout < 0 {
660+
return nil, errors.New("timeout should be bigger than 0")
661+
}
662+
663+
ctx, cancel := context.WithCancel(context.Background())
664+
defer cancel()
665+
666+
if timeout > 0 {
667+
logrus.Debugf("Using context with timeout of %s", timeout)
668+
ctx, cancel = context.WithTimeout(ctx, timeout)
669+
defer cancel()
670+
}
671+
672+
res, err := client.PullImage(ctx, request.Image, request.Auth, request.SandboxConfig)
651673
if err != nil {
652674
return nil, err
653675
}

Diff for: go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ require (
2626
golang.org/x/sys v0.21.0
2727
golang.org/x/term v0.21.0
2828
golang.org/x/text v0.16.0
29+
google.golang.org/grpc v1.64.0
2930
gopkg.in/yaml.v3 v3.0.1
3031
k8s.io/api v0.30.1
3132
k8s.io/apimachinery v0.30.1
@@ -85,7 +86,6 @@ require (
8586
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
8687
google.golang.org/genproto/googleapis/api v0.0.0-20240520151616-dc85e6b867a5 // indirect
8788
google.golang.org/genproto/googleapis/rpc v0.0.0-20240515191416-fc5f0ca64291 // indirect
88-
google.golang.org/grpc v1.64.0 // indirect
8989
google.golang.org/protobuf v1.34.1 // indirect
9090
gopkg.in/inf.v0 v0.9.1 // indirect
9191
gopkg.in/yaml.v2 v2.4.0 // indirect

Diff for: pkg/framework/util.go

+9-3
Original file line numberDiff line numberDiff line change
@@ -328,9 +328,8 @@ func ListImage(c internalapi.ImageManagerService, filter *runtimeapi.ImageFilter
328328
return images
329329
}
330330

331-
// PullPublicImage pulls the public image named imageName.
332-
func PullPublicImage(c internalapi.ImageManagerService, imageName string, podConfig *runtimeapi.PodSandboxConfig) string {
333-
331+
// PrepareImageName builds a pullable image name for the provided one.
332+
func PrepareImageName(imageName string) string {
334333
ref, err := reference.ParseNamed(imageName)
335334
if err == nil {
336335
// Modify the image if it's a fully qualified image name
@@ -352,6 +351,13 @@ func PullPublicImage(c internalapi.ImageManagerService, imageName string, podCon
352351
Failf("Unable to parse imageName: %v", err)
353352
}
354353

354+
return imageName
355+
}
356+
357+
// PullPublicImage pulls the public image named imageName.
358+
func PullPublicImage(c internalapi.ImageManagerService, imageName string, podConfig *runtimeapi.PodSandboxConfig) string {
359+
imageName = PrepareImageName(imageName)
360+
355361
By("Pull image : " + imageName)
356362
imageSpec := &runtimeapi.ImageSpec{
357363
Image: imageName,

Diff for: pkg/validate/image.go

+18
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ import (
2121
"fmt"
2222
"runtime"
2323
"sort"
24+
"time"
2425

26+
"google.golang.org/grpc/codes"
27+
"google.golang.org/grpc/status"
2528
internalapi "k8s.io/cri-api/pkg/apis"
2629
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
2730
"sigs.k8s.io/cri-tools/pkg/framework"
@@ -45,6 +48,21 @@ var _ = framework.KubeDescribe("Image Manager", func() {
4548
})
4649
})
4750

51+
It("public image should timeout if requested [Conformance]", func() {
52+
imageName := framework.PrepareImageName(testImageWithTag)
53+
54+
ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond)
55+
defer cancel()
56+
res, err := c.PullImage(ctx, &runtimeapi.ImageSpec{Image: imageName}, nil, nil)
57+
58+
Expect(res).To(BeEmpty())
59+
Expect(err).To(HaveOccurred())
60+
61+
statusErr, ok := status.FromError(err)
62+
Expect(ok).To(BeTrue())
63+
Expect(statusErr.Code()).To(Equal(codes.DeadlineExceeded))
64+
})
65+
4866
It("public image without tag should be pulled and removed [Conformance]", func() {
4967
testPullPublicImage(c, testImageWithoutTag, testImagePodSandbox, func(s *runtimeapi.Image) {
5068
Expect(s.RepoTags).To(Equal([]string{testImageWithoutTag + ":latest"}))

0 commit comments

Comments
 (0)