From 0ec652e6e74a0ffa440b0394ff11ffd884648e44 Mon Sep 17 00:00:00 2001 From: Mrudul Harwani Date: Fri, 30 Jun 2023 03:33:06 -0700 Subject: [PATCH 1/6] refactor: allow custom handlers for processing push/pull status updates Signed-off-by: Mrudul Harwani --- cmd/nerdctl/container_create.go | 1 - cmd/nerdctl/image_pull.go | 1 - pkg/api/types/image_types.go | 11 ++++-- pkg/cmd/compose/compose.go | 19 +++++++--- pkg/cmd/container/create.go | 13 ++++++- pkg/cmd/image/pull.go | 29 +++++++++++---- pkg/cmd/image/push.go | 10 +++++- pkg/imgutil/imgutil.go | 63 +++++++++++++++++++-------------- pkg/imgutil/jobs/jobs.go | 40 ++++++++++++++++----- pkg/imgutil/pull/pull.go | 9 +++-- pkg/imgutil/push/push.go | 17 ++------- pkg/ipfs/image.go | 15 ++++---- 12 files changed, 150 insertions(+), 78 deletions(-) diff --git a/cmd/nerdctl/container_create.go b/cmd/nerdctl/container_create.go index 788429086fd..7c084a437e1 100644 --- a/cmd/nerdctl/container_create.go +++ b/cmd/nerdctl/container_create.go @@ -400,7 +400,6 @@ func processContainerCreateOptions(cmd *cobra.Command) (opt types.ContainerCreat VerifyOptions: imageVerifyOpt, IPFSAddress: opt.IPFSAddress, Stdout: opt.Stdout, - Stderr: opt.Stderr, } // #endregion diff --git a/cmd/nerdctl/image_pull.go b/cmd/nerdctl/image_pull.go index 71d00899d23..df8bbdcadfb 100644 --- a/cmd/nerdctl/image_pull.go +++ b/cmd/nerdctl/image_pull.go @@ -102,7 +102,6 @@ func processPullCommandFlags(cmd *cobra.Command) (types.ImagePullOptions, error) Quiet: quiet, IPFSAddress: ipfsAddressStr, Stdout: cmd.OutOrStdout(), - Stderr: cmd.OutOrStderr(), }, nil } diff --git a/pkg/api/types/image_types.go b/pkg/api/types/image_types.go index 51fd3bc4bff..7d6ea9e3703 100644 --- a/pkg/api/types/image_types.go +++ b/pkg/api/types/image_types.go @@ -16,7 +16,11 @@ package types -import "io" +import ( + "io" + + "github.com/containerd/nerdctl/pkg/imgutil/jobs" +) // ImageListOptions specifies options for `nerdctl image list`. type ImageListOptions struct { @@ -165,6 +169,8 @@ type ImagePushOptions struct { IpfsAddress string // Suppress verbose output Quiet bool + // If non-nil, the Push job will send upload statuses to the handler instead of Stdout + ProgressHandler jobs.StatusHandler // AllowNondistributableArtifacts allow pushing non-distributable artifacts AllowNondistributableArtifacts bool } @@ -172,7 +178,6 @@ type ImagePushOptions struct { // ImagePullOptions specifies options for `nerdctl (image) pull`. type ImagePullOptions struct { Stdout io.Writer - Stderr io.Writer GOptions GlobalCommandOptions VerifyOptions ImageVerifyOptions // Unpack the image for the current single platform (auto/true/false) @@ -183,6 +188,8 @@ type ImagePullOptions struct { AllPlatforms bool // Suppress verbose output Quiet bool + // If non-nil, the Pull job will send download statuses to the handler instead of Stdout + ProgressHandler jobs.StatusHandler // multiaddr of IPFS API (default uses $IPFS_PATH env variable if defined or local directory ~/.ipfs) IPFSAddress string } diff --git a/pkg/cmd/compose/compose.go b/pkg/cmd/compose/compose.go index 1b5cd8d37c0..cdbaa378fe3 100644 --- a/pkg/cmd/compose/compose.go +++ b/pkg/cmd/compose/compose.go @@ -31,6 +31,7 @@ import ( "github.com/containerd/nerdctl/pkg/composer" "github.com/containerd/nerdctl/pkg/composer/serviceparser" "github.com/containerd/nerdctl/pkg/imgutil" + "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/containerd/nerdctl/pkg/ipfs" "github.com/containerd/nerdctl/pkg/netutil" "github.com/containerd/nerdctl/pkg/referenceutil" @@ -110,6 +111,18 @@ func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, op ocispecPlatforms = []ocispec.Platform{parsed} // no append } + pullCfg := imgutil.PullConfig{ + Ref: imageName, + Platforms: ocispecPlatforms, + Snapshotter: globalOptions.Snapshotter, + Insecure: globalOptions.InsecureRegistry, + HostsDir: globalOptions.HostsDir, + Mode: pullMode, + Unpack: nil, + Quiet: quiet, + ProgressHandler: jobs.DefaultStatusHandler(stdout), + } + // IPFS reference if scheme, ref, err := referenceutil.ParseIPFSRefWithScheme(imageName); err == nil { var ipfsPath string @@ -124,8 +137,7 @@ func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, op } ipfsPath = dir } - _, err = ipfs.EnsureImage(ctx, client, stdout, stderr, globalOptions.Snapshotter, scheme, ref, - pullMode, ocispecPlatforms, nil, quiet, ipfsPath) + _, err = ipfs.EnsureImage(ctx, client, scheme, ref, ipfsPath, pullCfg) return err } @@ -135,8 +147,7 @@ func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, op return err } - _, err = imgutil.EnsureImage(ctx, client, stdout, stderr, globalOptions.Snapshotter, ref, - pullMode, globalOptions.InsecureRegistry, globalOptions.HostsDir, ocispecPlatforms, nil, quiet) + _, err = imgutil.EnsureImage(ctx, client, ref, pullCfg) return err } diff --git a/pkg/cmd/container/create.go b/pkg/cmd/container/create.go index 2038f291f7a..83fafe189fe 100644 --- a/pkg/cmd/container/create.go +++ b/pkg/cmd/container/create.go @@ -42,6 +42,7 @@ import ( "github.com/containerd/nerdctl/pkg/flagutil" "github.com/containerd/nerdctl/pkg/idgen" "github.com/containerd/nerdctl/pkg/imgutil" + "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/containerd/nerdctl/pkg/inspecttypes/dockercompat" "github.com/containerd/nerdctl/pkg/labels" "github.com/containerd/nerdctl/pkg/logging" @@ -115,7 +116,17 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa } rawRef := args[0] - ensuredImage, err = image.EnsureImage(ctx, client, rawRef, ocispecPlatforms, options.Pull, nil, false, options.ImagePullOpt) + pullCfg := imgutil.PullConfig{ + Ref: rawRef, + Platforms: ocispecPlatforms, + Snapshotter: options.GOptions.Snapshotter, + Insecure: options.GOptions.InsecureRegistry, + HostsDir: options.GOptions.HostsDir, + Mode: options.Pull, + Quiet: false, + ProgressHandler: jobs.DefaultStatusHandler(options.ImagePullOpt.Stdout), + } + ensuredImage, err = image.EnsureImage(ctx, client, rawRef, pullCfg, options.ImagePullOpt) if err != nil { return nil, nil, err } diff --git a/pkg/cmd/image/pull.go b/pkg/cmd/image/pull.go index 50ac66bb97f..01583ab3855 100644 --- a/pkg/cmd/image/pull.go +++ b/pkg/cmd/image/pull.go @@ -25,12 +25,12 @@ import ( "github.com/containerd/containerd" "github.com/containerd/nerdctl/pkg/api/types" "github.com/containerd/nerdctl/pkg/imgutil" + "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/containerd/nerdctl/pkg/ipfs" "github.com/containerd/nerdctl/pkg/platformutil" "github.com/containerd/nerdctl/pkg/referenceutil" "github.com/containerd/nerdctl/pkg/signutil" "github.com/containerd/nerdctl/pkg/strutil" - v1 "github.com/opencontainers/image-spec/specs-go/v1" ) // Pull pulls an image specified by `rawRef`. @@ -45,7 +45,24 @@ func Pull(ctx context.Context, client *containerd.Client, rawRef string, options return err } - _, err = EnsureImage(ctx, client, rawRef, ocispecPlatforms, "always", unpack, options.Quiet, options) + var progressHandler jobs.StatusHandler + if options.ProgressHandler != nil { + progressHandler = options.ProgressHandler + } else { + progressHandler = jobs.DefaultStatusHandler(options.Stdout) + } + pullCfg := imgutil.PullConfig{ + Ref: rawRef, + Platforms: ocispecPlatforms, + Snapshotter: options.GOptions.Snapshotter, + Insecure: options.GOptions.InsecureRegistry, + HostsDir: options.GOptions.HostsDir, + Mode: "always", + Unpack: unpack, + Quiet: options.Quiet, + ProgressHandler: progressHandler, + } + _, err = EnsureImage(ctx, client, rawRef, pullCfg, options) if err != nil { return err } @@ -54,7 +71,7 @@ func Pull(ctx context.Context, client *containerd.Client, rawRef string, options } // EnsureImage pulls an image either from ipfs or from registry. -func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string, ocispecPlatforms []v1.Platform, pull string, unpack *bool, quiet bool, options types.ImagePullOptions) (*imgutil.EnsuredImage, error) { +func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string, pullCfg imgutil.PullConfig, options types.ImagePullOptions) (*imgutil.EnsuredImage, error) { var ensured *imgutil.EnsuredImage if scheme, ref, err := referenceutil.ParseIPFSRefWithScheme(rawRef); err == nil { @@ -75,8 +92,7 @@ func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string, ipfsPath = dir } - ensured, err = ipfs.EnsureImage(ctx, client, options.Stdout, options.Stderr, options.GOptions.Snapshotter, scheme, ref, - pull, ocispecPlatforms, unpack, quiet, ipfsPath) + ensured, err = ipfs.EnsureImage(ctx, client, scheme, ref, ipfsPath, pullCfg) if err != nil { return nil, err } @@ -88,8 +104,7 @@ func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string, return nil, err } - ensured, err = imgutil.EnsureImage(ctx, client, options.Stdout, options.Stderr, options.GOptions.Snapshotter, ref, - pull, options.GOptions.InsecureRegistry, options.GOptions.HostsDir, ocispecPlatforms, unpack, quiet) + ensured, err = imgutil.EnsureImage(ctx, client, ref, pullCfg) if err != nil { return nil, err } diff --git a/pkg/cmd/image/push.go b/pkg/cmd/image/push.go index c9029bf17d8..4a807cc60ec 100644 --- a/pkg/cmd/image/push.go +++ b/pkg/cmd/image/push.go @@ -33,6 +33,7 @@ import ( "github.com/containerd/nerdctl/pkg/api/types" "github.com/containerd/nerdctl/pkg/errutil" "github.com/containerd/nerdctl/pkg/imgutil/dockerconfigresolver" + "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/containerd/nerdctl/pkg/imgutil/push" "github.com/containerd/nerdctl/pkg/ipfs" "github.com/containerd/nerdctl/pkg/platformutil" @@ -117,8 +118,15 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options logrus.Infof("pushing as an eStargz image (%s, %s)", esgzImg.Target.MediaType, esgzImg.Target.Digest) } + var progressHandler jobs.StatusHandler + if options.ProgressHandler != nil { + progressHandler = options.ProgressHandler + } else { + progressHandler = jobs.DefaultStatusHandler(options.Stdout) + } + pushFunc := func(r remotes.Resolver) error { - return push.Push(ctx, client, r, options.Stdout, pushRef, ref, platMC, options.AllowNondistributableArtifacts, options.Quiet) + return push.Push(ctx, client, r, progressHandler, pushRef, ref, platMC, options.AllowNondistributableArtifacts, options.Quiet) } var dOpts []dockerconfigresolver.Opt diff --git a/pkg/imgutil/imgutil.go b/pkg/imgutil/imgutil.go index 14e04c40705..cf174eaee0c 100644 --- a/pkg/imgutil/imgutil.go +++ b/pkg/imgutil/imgutil.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "fmt" - "io" "reflect" "github.com/containerd/containerd" @@ -35,6 +34,7 @@ import ( "github.com/containerd/nerdctl/pkg/errutil" "github.com/containerd/nerdctl/pkg/idutil/imagewalker" "github.com/containerd/nerdctl/pkg/imgutil/dockerconfigresolver" + "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/containerd/nerdctl/pkg/imgutil/pull" "github.com/docker/docker/errdefs" "github.com/opencontainers/image-spec/identity" @@ -51,6 +51,19 @@ type EnsuredImage struct { Remote bool // true for stargz or overlaybd } +// PullConfig contains configurations for pulling an image +type PullConfig struct { + Ref string + Platforms []ocispec.Platform + Snapshotter string + Insecure bool + HostsDir []string + Mode PullMode + Unpack *bool + Quiet bool + ProgressHandler jobs.StatusHandler +} + // PullMode is either one of "always", "missing", "never" type PullMode = string @@ -101,26 +114,24 @@ func GetExistingImage(ctx context.Context, client *containerd.Client, snapshotte // EnsureImage ensures the image. // // # When insecure is set, skips verifying certs, and also falls back to HTTP when the registry does not speak HTTPS -// -// FIXME: this func has too many args -func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr io.Writer, snapshotter, rawRef string, mode PullMode, insecure bool, hostsDirs []string, ocispecPlatforms []ocispec.Platform, unpack *bool, quiet bool) (*EnsuredImage, error) { - switch mode { +func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string, cfg PullConfig) (*EnsuredImage, error) { + switch cfg.Mode { case "always", "missing", "never": // NOP default: - return nil, fmt.Errorf("unexpected pull mode: %q", mode) + return nil, fmt.Errorf("unexpected pull mode: %q", cfg.Mode) } // if not `always` pull and given one platform and image found locally, return existing image directly. - if mode != "always" && len(ocispecPlatforms) == 1 { - if res, err := GetExistingImage(ctx, client, snapshotter, rawRef, ocispecPlatforms[0]); err == nil { + if cfg.Mode != "always" && len(cfg.Platforms) == 1 { + if res, err := GetExistingImage(ctx, client, cfg.Snapshotter, rawRef, cfg.Platforms[0]); err == nil { return res, nil } else if !errdefs.IsNotFound(err) { return nil, err } } - if mode == "never" { + if cfg.Mode == "never" { return nil, fmt.Errorf("image not available: %q", rawRef) } @@ -132,30 +143,30 @@ func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr refDomain := refdocker.Domain(named) var dOpts []dockerconfigresolver.Opt - if insecure { + if cfg.Insecure { logrus.Warnf("skipping verifying HTTPS certs for %q", refDomain) dOpts = append(dOpts, dockerconfigresolver.WithSkipVerifyCerts(true)) } - dOpts = append(dOpts, dockerconfigresolver.WithHostsDirs(hostsDirs)) + dOpts = append(dOpts, dockerconfigresolver.WithHostsDirs(cfg.HostsDir)) resolver, err := dockerconfigresolver.New(ctx, refDomain, dOpts...) if err != nil { return nil, err } - img, err := PullImage(ctx, client, stdout, stderr, snapshotter, resolver, ref, ocispecPlatforms, unpack, quiet) + img, err := PullImage(ctx, client, ref, resolver, cfg) if err != nil { // In some circumstance (e.g. people just use 80 port to support pure http), the error will contain message like "dial tcp : connection refused". if !errutil.IsErrHTTPResponseToHTTPSClient(err) && !errutil.IsErrConnectionRefused(err) { return nil, err } - if insecure { + if cfg.Insecure { logrus.WithError(err).Warnf("server %q does not seem to support HTTPS, falling back to plain HTTP", refDomain) dOpts = append(dOpts, dockerconfigresolver.WithPlainHTTP(true)) resolver, err = dockerconfigresolver.New(ctx, refDomain, dOpts...) if err != nil { return nil, err } - return PullImage(ctx, client, stdout, stderr, snapshotter, resolver, ref, ocispecPlatforms, unpack, quiet) + return PullImage(ctx, client, ref, resolver, cfg) } logrus.WithError(err).Errorf("server %q does not seem to support HTTPS", refDomain) logrus.Info("Hint: you may want to try --insecure-registry to allow plain HTTP (if you are in a trusted network)") @@ -194,7 +205,7 @@ func ResolveDigest(ctx context.Context, rawRef string, insecure bool, hostsDirs } // PullImage pulls an image using the specified resolver. -func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io.Writer, snapshotter string, resolver remotes.Resolver, ref string, ocispecPlatforms []ocispec.Platform, unpack *bool, quiet bool) (*EnsuredImage, error) { +func PullImage(ctx context.Context, client *containerd.Client, ref string, resolver remotes.Resolver, cfg PullConfig) (*EnsuredImage, error) { ctx, done, err := client.WithLease(ctx) if err != nil { return nil, err @@ -205,24 +216,24 @@ func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io config := &pull.Config{ Resolver: resolver, RemoteOpts: []containerd.RemoteOpt{}, - Platforms: ocispecPlatforms, // empty for all-platforms + Platforms: cfg.Platforms, // empty for all-platforms } - if !quiet { - config.ProgressOutput = stderr + if !cfg.Quiet { + config.ProgressHandler = cfg.ProgressHandler } // unpack(B) if given 1 platform unless specified by `unpack` - unpackB := len(ocispecPlatforms) == 1 - if unpack != nil { - unpackB = *unpack - if unpackB && len(ocispecPlatforms) != 1 { + unpackB := len(cfg.Platforms) == 1 + if cfg.Unpack != nil { + unpackB = *cfg.Unpack + if unpackB && len(cfg.Platforms) != 1 { return nil, fmt.Errorf("unpacking requires a single platform to be specified (e.g., --platform=amd64)") } } - snOpt := getSnapshotterOpts(snapshotter) + snOpt := getSnapshotterOpts(cfg.Snapshotter) if unpackB { - logrus.Debugf("The image will be unpacked for platform %q, snapshotter %q.", ocispecPlatforms[0], snapshotter) + logrus.Debugf("The image will be unpacked for platform %q, snapshotter %q.", cfg.Platforms[0], cfg.Snapshotter) imgcryptPayload := imgcrypt.Payload{} imgcryptUnpackOpt := encryption.WithUnpackConfigApplyOpts(encryption.WithDecryptedUnpack(&imgcryptPayload)) config.RemoteOpts = append(config.RemoteOpts, @@ -232,7 +243,7 @@ func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io // different remote snapshotters will update pull.Config separately snOpt.apply(config, ref) } else { - logrus.Debugf("The image will not be unpacked. Platforms=%v.", ocispecPlatforms) + logrus.Debugf("The image will not be unpacked. Platforms=%v.", cfg.Platforms) } containerdImage, err = pull.Pull(ctx, client, ref, config) @@ -247,7 +258,7 @@ func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io Ref: ref, Image: containerdImage, ImageConfig: *imgConfig, - Snapshotter: snapshotter, + Snapshotter: cfg.Snapshotter, Remote: snOpt.isRemote(), } return res, nil diff --git a/pkg/imgutil/jobs/jobs.go b/pkg/imgutil/jobs/jobs.go index da5da32c4ef..60dace0d0cc 100644 --- a/pkg/imgutil/jobs/jobs.go +++ b/pkg/imgutil/jobs/jobs.go @@ -37,10 +37,13 @@ import ( // by checking status in the content store. // // From https://github.com/containerd/containerd/blob/v1.7.0-rc.2/cmd/ctr/commands/content/fetch.go#L219-L336 -func ShowProgress(ctx context.Context, ongoing *Jobs, cs content.Store, out io.Writer) { +func ShowProgress(ctx context.Context, ongoing *Jobs, cs content.Store, handler StatusHandler) { + if handler == nil { + return + } + var ( ticker = time.NewTicker(100 * time.Millisecond) - fw = progress.NewWriter(out) start = time.Now() statuses = map[string]StatusInfo{} done bool @@ -51,10 +54,6 @@ outer: for { select { case <-ticker.C: - fw.Flush() - - tw := tabwriter.NewWriter(fw, 1, 8, 1, ' ', 0) - resolved := StatusResolved if !ongoing.IsResolved() { resolved = StatusResolving @@ -141,11 +140,9 @@ outer: ordered = append(ordered, statuses[key]) } - Display(tw, ordered, start) - tw.Flush() + handler(ordered, start, done) if done { - fw.Flush() return } case <-ctx.Done(): @@ -236,6 +233,31 @@ type StatusInfo struct { UpdatedAt time.Time } +// StatusHandler defines a func signature for handling StatusInfo objects per tick +type StatusHandler func(statuses []StatusInfo, start time.Time, done bool) + +// DefaultStatusHandler returns the default StatusHandler to display progress on the provided writer +// Returns nil if the writer is nil +func DefaultStatusHandler(w io.Writer) StatusHandler { + if w == nil { + return nil + } + + fw := progress.NewWriter(w) + return func(statuses []StatusInfo, start time.Time, done bool) { + fw.Flush() + + tw := tabwriter.NewWriter(fw, 1, 8, 1, ' ', 0) + + Display(tw, statuses, start) + tw.Flush() + + if done { + fw.Flush() + } + } +} + // Display pretty prints out the download or upload progress. // From https://github.com/containerd/containerd/blob/v1.7.0-rc.2/cmd/ctr/commands/content/fetch.go#L412-L452 func Display(w io.Writer, statuses []StatusInfo, start time.Time) { diff --git a/pkg/imgutil/pull/pull.go b/pkg/imgutil/pull/pull.go index 11a257684c8..3feae55db52 100644 --- a/pkg/imgutil/pull/pull.go +++ b/pkg/imgutil/pull/pull.go @@ -19,7 +19,6 @@ package pull import ( "context" - "io" "github.com/containerd/containerd" "github.com/containerd/containerd/images" @@ -34,8 +33,8 @@ import ( type Config struct { // Resolver Resolver remotes.Resolver - // ProgressOutput to display progress - ProgressOutput io.Writer + // ProgressHandler to handler progress statuses + ProgressHandler jobs.StatusHandler // RemoteOpts, e.g. containerd.WithPullUnpack. // // Regardless to RemoteOpts, the following opts are always set: @@ -54,9 +53,9 @@ func Pull(ctx context.Context, client *containerd.Client, ref string, config *Co progress := make(chan struct{}) go func() { - if config.ProgressOutput != nil { + if config.ProgressHandler != nil { // no progress bar, because it hides some debug logs - jobs.ShowProgress(pctx, ongoing, client.ContentStore(), config.ProgressOutput) + jobs.ShowProgress(pctx, ongoing, client.ContentStore(), config.ProgressHandler) } close(progress) }() diff --git a/pkg/imgutil/push/push.go b/pkg/imgutil/push/push.go index d16b4b613f7..58ea270afc5 100644 --- a/pkg/imgutil/push/push.go +++ b/pkg/imgutil/push/push.go @@ -20,15 +20,12 @@ package push import ( "context" "fmt" - "io" "sync" - "text/tabwriter" "time" "github.com/containerd/containerd" "github.com/containerd/containerd/images" "github.com/containerd/containerd/log" - "github.com/containerd/containerd/pkg/progress" "github.com/containerd/containerd/platforms" "github.com/containerd/containerd/remotes" "github.com/containerd/containerd/remotes/docker" @@ -40,7 +37,7 @@ import ( ) // Push pushes an image to a remote registry. -func Push(ctx context.Context, client *containerd.Client, resolver remotes.Resolver, stdout io.Writer, +func Push(ctx context.Context, client *containerd.Client, resolver remotes.Resolver, progressHandler jobs.StatusHandler, localRef, remoteRef string, platform platforms.MatchComparer, allowNonDist, quiet bool) error { img, err := client.ImageService().Get(ctx, localRef) if err != nil { @@ -78,11 +75,10 @@ func Push(ctx context.Context, client *containerd.Client, resolver remotes.Resol ) }) - if !quiet { + if !quiet && progressHandler != nil { eg.Go(func() error { var ( ticker = time.NewTicker(100 * time.Millisecond) - fw = progress.NewWriter(stdout) start = time.Now() done bool ) @@ -92,15 +88,8 @@ func Push(ctx context.Context, client *containerd.Client, resolver remotes.Resol for { select { case <-ticker.C: - fw.Flush() - - tw := tabwriter.NewWriter(fw, 1, 8, 1, ' ', 0) - - jobs.Display(tw, ongoing.status(), start) - tw.Flush() - + progressHandler(ongoing.status(), start, done) if done { - fw.Flush() return nil } case <-doneCh: diff --git a/pkg/ipfs/image.go b/pkg/ipfs/image.go index 931c673cac0..e0be19aa8c4 100644 --- a/pkg/ipfs/image.go +++ b/pkg/ipfs/image.go @@ -40,12 +40,12 @@ import ( const ipfsPathEnv = "IPFS_PATH" // EnsureImage pull the specified image from IPFS. -func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr io.Writer, snapshotter string, scheme string, ref string, mode imgutil.PullMode, ocispecPlatforms []ocispec.Platform, unpack *bool, quiet bool, ipfsPath string) (*imgutil.EnsuredImage, error) { - switch mode { +func EnsureImage(ctx context.Context, client *containerd.Client, scheme string, ref string, ipfsPath string, cfg imgutil.PullConfig) (*imgutil.EnsuredImage, error) { + switch cfg.Mode { case "always", "missing", "never": // NOP default: - return nil, fmt.Errorf("unexpected pull mode: %q", mode) + return nil, fmt.Errorf("unexpected pull mode: %q", cfg.Mode) } switch scheme { case "ipfs", "ipns": @@ -55,15 +55,15 @@ func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr } // if not `always` pull and given one platform and image found locally, return existing image directly. - if mode != "always" && len(ocispecPlatforms) == 1 { - if res, err := imgutil.GetExistingImage(ctx, client, snapshotter, ref, ocispecPlatforms[0]); err == nil { + if cfg.Mode != "always" && len(cfg.Platforms) == 1 { + if res, err := imgutil.GetExistingImage(ctx, client, cfg.Snapshotter, ref, cfg.Platforms[0]); err == nil { return res, nil } else if !errdefs.IsNotFound(err) { return nil, err } } - if mode == "never" { + if cfg.Mode == "never" { return nil, fmt.Errorf("image %q is not available", ref) } r, err := ipfs.NewResolver(ipfs.ResolverOptions{ @@ -73,7 +73,8 @@ func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr if err != nil { return nil, err } - return imgutil.PullImage(ctx, client, stdout, stderr, snapshotter, r, ref, ocispecPlatforms, unpack, quiet) + + return imgutil.PullImage(ctx, client, ref, r, cfg) } // Push pushes the specified image to IPFS. From 2db8ffc9f04e59f527efbb223a5f45834fdf618e Mon Sep 17 00:00:00 2001 From: Mrudul Harwani Date: Fri, 30 Jun 2023 04:26:25 -0700 Subject: [PATCH 2/6] redirect image pull progress to stderr instead of stdout Signed-off-by: Mrudul Harwani --- cmd/nerdctl/image_pull.go | 2 +- pkg/cmd/compose/compose.go | 2 +- pkg/cmd/container/create.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/nerdctl/image_pull.go b/cmd/nerdctl/image_pull.go index df8bbdcadfb..6825cc5f17a 100644 --- a/cmd/nerdctl/image_pull.go +++ b/cmd/nerdctl/image_pull.go @@ -101,7 +101,7 @@ func processPullCommandFlags(cmd *cobra.Command) (types.ImagePullOptions, error) Unpack: unpackStr, Quiet: quiet, IPFSAddress: ipfsAddressStr, - Stdout: cmd.OutOrStdout(), + Stdout: cmd.OutOrStderr(), }, nil } diff --git a/pkg/cmd/compose/compose.go b/pkg/cmd/compose/compose.go index cdbaa378fe3..07ec7122a27 100644 --- a/pkg/cmd/compose/compose.go +++ b/pkg/cmd/compose/compose.go @@ -120,7 +120,7 @@ func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, op Mode: pullMode, Unpack: nil, Quiet: quiet, - ProgressHandler: jobs.DefaultStatusHandler(stdout), + ProgressHandler: jobs.DefaultStatusHandler(stderr), } // IPFS reference diff --git a/pkg/cmd/container/create.go b/pkg/cmd/container/create.go index 83fafe189fe..11a42d397fd 100644 --- a/pkg/cmd/container/create.go +++ b/pkg/cmd/container/create.go @@ -124,7 +124,7 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa HostsDir: options.GOptions.HostsDir, Mode: options.Pull, Quiet: false, - ProgressHandler: jobs.DefaultStatusHandler(options.ImagePullOpt.Stdout), + ProgressHandler: jobs.DefaultStatusHandler(options.Stderr), } ensuredImage, err = image.EnsureImage(ctx, client, rawRef, pullCfg, options.ImagePullOpt) if err != nil { From ae192be4930efcac654f2fca47bb77c1e54bc808 Mon Sep 17 00:00:00 2001 From: Mrudul Harwani Date: Mon, 10 Jul 2023 04:53:02 -0700 Subject: [PATCH 3/6] refactor EnsureImage in pkg/cmd/pull.go to keep the original arguments Signed-off-by: Mrudul Harwani --- pkg/cmd/compose/compose.go | 2 +- pkg/cmd/container/create.go | 13 +------------ pkg/cmd/image/pull.go | 30 ++++++++++++++++-------------- pkg/cmd/image/push.go | 2 +- pkg/imgutil/jobs/jobs.go | 7 ++++--- 5 files changed, 23 insertions(+), 31 deletions(-) diff --git a/pkg/cmd/compose/compose.go b/pkg/cmd/compose/compose.go index 07ec7122a27..f31982c3314 100644 --- a/pkg/cmd/compose/compose.go +++ b/pkg/cmd/compose/compose.go @@ -120,7 +120,7 @@ func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, op Mode: pullMode, Unpack: nil, Quiet: quiet, - ProgressHandler: jobs.DefaultStatusHandler(stderr), + ProgressHandler: jobs.PrintProgress(stderr), } // IPFS reference diff --git a/pkg/cmd/container/create.go b/pkg/cmd/container/create.go index 11a42d397fd..2038f291f7a 100644 --- a/pkg/cmd/container/create.go +++ b/pkg/cmd/container/create.go @@ -42,7 +42,6 @@ import ( "github.com/containerd/nerdctl/pkg/flagutil" "github.com/containerd/nerdctl/pkg/idgen" "github.com/containerd/nerdctl/pkg/imgutil" - "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/containerd/nerdctl/pkg/inspecttypes/dockercompat" "github.com/containerd/nerdctl/pkg/labels" "github.com/containerd/nerdctl/pkg/logging" @@ -116,17 +115,7 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa } rawRef := args[0] - pullCfg := imgutil.PullConfig{ - Ref: rawRef, - Platforms: ocispecPlatforms, - Snapshotter: options.GOptions.Snapshotter, - Insecure: options.GOptions.InsecureRegistry, - HostsDir: options.GOptions.HostsDir, - Mode: options.Pull, - Quiet: false, - ProgressHandler: jobs.DefaultStatusHandler(options.Stderr), - } - ensuredImage, err = image.EnsureImage(ctx, client, rawRef, pullCfg, options.ImagePullOpt) + ensuredImage, err = image.EnsureImage(ctx, client, rawRef, ocispecPlatforms, options.Pull, nil, false, options.ImagePullOpt) if err != nil { return nil, nil, err } diff --git a/pkg/cmd/image/pull.go b/pkg/cmd/image/pull.go index 01583ab3855..8123a73daf8 100644 --- a/pkg/cmd/image/pull.go +++ b/pkg/cmd/image/pull.go @@ -31,6 +31,7 @@ import ( "github.com/containerd/nerdctl/pkg/referenceutil" "github.com/containerd/nerdctl/pkg/signutil" "github.com/containerd/nerdctl/pkg/strutil" + v1 "github.com/opencontainers/image-spec/specs-go/v1" ) // Pull pulls an image specified by `rawRef`. @@ -45,11 +46,23 @@ func Pull(ctx context.Context, client *containerd.Client, rawRef string, options return err } + EnsureImage(ctx, client, rawRef, ocispecPlatforms, "always", unpack, options.Quiet, options) + if err != nil { + return err + } + + return nil +} + +// EnsureImage pulls an image either from ipfs or from registry. +func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string, ocispecPlatforms []v1.Platform, pull string, unpack *bool, quiet bool, options types.ImagePullOptions) (*imgutil.EnsuredImage, error) { + var ensured *imgutil.EnsuredImage + var progressHandler jobs.StatusHandler if options.ProgressHandler != nil { progressHandler = options.ProgressHandler } else { - progressHandler = jobs.DefaultStatusHandler(options.Stdout) + progressHandler = jobs.PrintProgress(options.Stdout) } pullCfg := imgutil.PullConfig{ Ref: rawRef, @@ -57,22 +70,11 @@ func Pull(ctx context.Context, client *containerd.Client, rawRef string, options Snapshotter: options.GOptions.Snapshotter, Insecure: options.GOptions.InsecureRegistry, HostsDir: options.GOptions.HostsDir, - Mode: "always", + Mode: pull, Unpack: unpack, - Quiet: options.Quiet, + Quiet: quiet, ProgressHandler: progressHandler, } - _, err = EnsureImage(ctx, client, rawRef, pullCfg, options) - if err != nil { - return err - } - - return nil -} - -// EnsureImage pulls an image either from ipfs or from registry. -func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string, pullCfg imgutil.PullConfig, options types.ImagePullOptions) (*imgutil.EnsuredImage, error) { - var ensured *imgutil.EnsuredImage if scheme, ref, err := referenceutil.ParseIPFSRefWithScheme(rawRef); err == nil { if options.VerifyOptions.Provider != "none" { diff --git a/pkg/cmd/image/push.go b/pkg/cmd/image/push.go index 4a807cc60ec..dba4b5ab207 100644 --- a/pkg/cmd/image/push.go +++ b/pkg/cmd/image/push.go @@ -122,7 +122,7 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options if options.ProgressHandler != nil { progressHandler = options.ProgressHandler } else { - progressHandler = jobs.DefaultStatusHandler(options.Stdout) + progressHandler = jobs.PrintProgress(options.Stdout) } pushFunc := func(r remotes.Resolver) error { diff --git a/pkg/imgutil/jobs/jobs.go b/pkg/imgutil/jobs/jobs.go index 60dace0d0cc..ba24da5c086 100644 --- a/pkg/imgutil/jobs/jobs.go +++ b/pkg/imgutil/jobs/jobs.go @@ -236,9 +236,10 @@ type StatusInfo struct { // StatusHandler defines a func signature for handling StatusInfo objects per tick type StatusHandler func(statuses []StatusInfo, start time.Time, done bool) -// DefaultStatusHandler returns the default StatusHandler to display progress on the provided writer -// Returns nil if the writer is nil -func DefaultStatusHandler(w io.Writer) StatusHandler { +// PrintProgress returns the default StatusHandler to display progress on the provided writer. +// +// Returns nil if the writer is nil. +func PrintProgress(w io.Writer) StatusHandler { if w == nil { return nil } From a0caa421f8dac743b30c28c7f3e24f956339cc6c Mon Sep 17 00:00:00 2001 From: Mrudul Harwani Date: Mon, 10 Jul 2023 05:07:51 -0700 Subject: [PATCH 4/6] fix a minor issue and a typo Signed-off-by: Mrudul Harwani --- pkg/cmd/image/pull.go | 2 +- pkg/imgutil/pull/pull.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/image/pull.go b/pkg/cmd/image/pull.go index 8123a73daf8..c678e8fc8eb 100644 --- a/pkg/cmd/image/pull.go +++ b/pkg/cmd/image/pull.go @@ -46,7 +46,7 @@ func Pull(ctx context.Context, client *containerd.Client, rawRef string, options return err } - EnsureImage(ctx, client, rawRef, ocispecPlatforms, "always", unpack, options.Quiet, options) + _, err = EnsureImage(ctx, client, rawRef, ocispecPlatforms, "always", unpack, options.Quiet, options) if err != nil { return err } diff --git a/pkg/imgutil/pull/pull.go b/pkg/imgutil/pull/pull.go index 3feae55db52..6b85890b006 100644 --- a/pkg/imgutil/pull/pull.go +++ b/pkg/imgutil/pull/pull.go @@ -33,7 +33,7 @@ import ( type Config struct { // Resolver Resolver remotes.Resolver - // ProgressHandler to handler progress statuses + // ProgressHandler to handle progress statuses ProgressHandler jobs.StatusHandler // RemoteOpts, e.g. containerd.WithPullUnpack. // From 2e36861463b2343b11e0194751564db3f9970624 Mon Sep 17 00:00:00 2001 From: Mrudul Harwani Date: Mon, 10 Jul 2023 10:15:48 -0700 Subject: [PATCH 5/6] redirect pull status output to stderr in container create Signed-off-by: Mrudul Harwani --- cmd/nerdctl/container_create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/nerdctl/container_create.go b/cmd/nerdctl/container_create.go index 7c084a437e1..0c768a2aa98 100644 --- a/cmd/nerdctl/container_create.go +++ b/cmd/nerdctl/container_create.go @@ -399,7 +399,7 @@ func processContainerCreateOptions(cmd *cobra.Command) (opt types.ContainerCreat GOptions: opt.GOptions, VerifyOptions: imageVerifyOpt, IPFSAddress: opt.IPFSAddress, - Stdout: opt.Stdout, + Stdout: opt.Stderr, } // #endregion From 747fe3d3b81414ce1e26d3e7a8cb34e0457d8fa2 Mon Sep 17 00:00:00 2001 From: Mrudul Harwani Date: Wed, 19 Jul 2023 10:05:55 -0700 Subject: [PATCH 6/6] remove stdout from pull/push options in favor of progress handler Signed-off-by: Mrudul Harwani --- cmd/nerdctl/container_create.go | 9 ++--- cmd/nerdctl/image_pull.go | 25 +++++++++----- cmd/nerdctl/image_push.go | 11 +++++-- pkg/api/types/image_types.go | 6 ++-- pkg/cmd/image/pull.go | 21 ++++-------- pkg/cmd/image/push.go | 58 ++++++++++++++------------------- pkg/imgutil/pull/pull.go | 2 +- 7 files changed, 65 insertions(+), 67 deletions(-) diff --git a/cmd/nerdctl/container_create.go b/cmd/nerdctl/container_create.go index 0c768a2aa98..f3831440ac6 100644 --- a/cmd/nerdctl/container_create.go +++ b/cmd/nerdctl/container_create.go @@ -24,6 +24,7 @@ import ( "github.com/containerd/nerdctl/pkg/clientutil" "github.com/containerd/nerdctl/pkg/cmd/container" "github.com/containerd/nerdctl/pkg/containerutil" + "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/spf13/cobra" ) @@ -396,10 +397,10 @@ func processContainerCreateOptions(cmd *cobra.Command) (opt types.ContainerCreat return } opt.ImagePullOpt = types.ImagePullOptions{ - GOptions: opt.GOptions, - VerifyOptions: imageVerifyOpt, - IPFSAddress: opt.IPFSAddress, - Stdout: opt.Stderr, + GOptions: opt.GOptions, + VerifyOptions: imageVerifyOpt, + IPFSAddress: opt.IPFSAddress, + ProgressHandler: jobs.PrintProgress(opt.Stderr), } // #endregion diff --git a/cmd/nerdctl/image_pull.go b/cmd/nerdctl/image_pull.go index 6825cc5f17a..e5ee588f964 100644 --- a/cmd/nerdctl/image_pull.go +++ b/cmd/nerdctl/image_pull.go @@ -17,9 +17,12 @@ package main import ( + "fmt" + "github.com/containerd/nerdctl/pkg/api/types" "github.com/containerd/nerdctl/pkg/clientutil" "github.com/containerd/nerdctl/pkg/cmd/image" + "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/spf13/cobra" ) @@ -94,14 +97,14 @@ func processPullCommandFlags(cmd *cobra.Command) (types.ImagePullOptions, error) return types.ImagePullOptions{}, err } return types.ImagePullOptions{ - GOptions: globalOptions, - VerifyOptions: verifyOptions, - AllPlatforms: allPlatforms, - Platform: platform, - Unpack: unpackStr, - Quiet: quiet, - IPFSAddress: ipfsAddressStr, - Stdout: cmd.OutOrStderr(), + GOptions: globalOptions, + VerifyOptions: verifyOptions, + AllPlatforms: allPlatforms, + Platform: platform, + Unpack: unpackStr, + Quiet: quiet, + IPFSAddress: ipfsAddressStr, + ProgressHandler: jobs.PrintProgress(cmd.OutOrStderr()), }, nil } @@ -117,5 +120,9 @@ func pullAction(cmd *cobra.Command, args []string) error { } defer cancel() - return image.Pull(ctx, client, args[0], options) + ref, err := image.Pull(ctx, client, args[0], options) + if options.Quiet { + fmt.Fprintln(cmd.OutOrStderr(), ref) + } + return err } diff --git a/cmd/nerdctl/image_push.go b/cmd/nerdctl/image_push.go index b217d281a97..357590ea431 100644 --- a/cmd/nerdctl/image_push.go +++ b/cmd/nerdctl/image_push.go @@ -17,9 +17,12 @@ package main import ( + "fmt" + "github.com/containerd/nerdctl/pkg/api/types" "github.com/containerd/nerdctl/pkg/clientutil" "github.com/containerd/nerdctl/pkg/cmd/image" + "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/spf13/cobra" ) @@ -111,7 +114,7 @@ func processImagePushOptions(cmd *cobra.Command) (types.ImagePushOptions, error) IpfsAddress: ipfsAddress, Quiet: quiet, AllowNondistributableArtifacts: allowNonDist, - Stdout: cmd.OutOrStdout(), + ProgressHandler: jobs.PrintProgress(cmd.OutOrStdout()), }, nil } @@ -128,7 +131,11 @@ func pushAction(cmd *cobra.Command, args []string) error { } defer cancel() - return image.Push(ctx, client, rawRef, options) + ref, err := image.Push(ctx, client, rawRef, options) + if options.Quiet { + fmt.Fprintln(cmd.OutOrStdout(), ref) + } + return err } func pushShellComplete(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { diff --git a/pkg/api/types/image_types.go b/pkg/api/types/image_types.go index 7d6ea9e3703..401e5a20d6b 100644 --- a/pkg/api/types/image_types.go +++ b/pkg/api/types/image_types.go @@ -153,7 +153,6 @@ type ImageInspectOptions struct { // ImagePushOptions specifies options for `nerdctl (image) push`. type ImagePushOptions struct { - Stdout io.Writer GOptions GlobalCommandOptions SignOptions ImageSignOptions // Platforms convert content for a specific platform @@ -169,7 +168,7 @@ type ImagePushOptions struct { IpfsAddress string // Suppress verbose output Quiet bool - // If non-nil, the Push job will send upload statuses to the handler instead of Stdout + // If non-nil, the Push job will pass upload statuses to the handler in small intervals ProgressHandler jobs.StatusHandler // AllowNondistributableArtifacts allow pushing non-distributable artifacts AllowNondistributableArtifacts bool @@ -177,7 +176,6 @@ type ImagePushOptions struct { // ImagePullOptions specifies options for `nerdctl (image) pull`. type ImagePullOptions struct { - Stdout io.Writer GOptions GlobalCommandOptions VerifyOptions ImageVerifyOptions // Unpack the image for the current single platform (auto/true/false) @@ -188,7 +186,7 @@ type ImagePullOptions struct { AllPlatforms bool // Suppress verbose output Quiet bool - // If non-nil, the Pull job will send download statuses to the handler instead of Stdout + // If non-nil, the Pull job will pass download statuses to the handler in small intervals ProgressHandler jobs.StatusHandler // multiaddr of IPFS API (default uses $IPFS_PATH env variable if defined or local directory ~/.ipfs) IPFSAddress string diff --git a/pkg/cmd/image/pull.go b/pkg/cmd/image/pull.go index c678e8fc8eb..e19618c5cb0 100644 --- a/pkg/cmd/image/pull.go +++ b/pkg/cmd/image/pull.go @@ -25,7 +25,6 @@ import ( "github.com/containerd/containerd" "github.com/containerd/nerdctl/pkg/api/types" "github.com/containerd/nerdctl/pkg/imgutil" - "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/containerd/nerdctl/pkg/ipfs" "github.com/containerd/nerdctl/pkg/platformutil" "github.com/containerd/nerdctl/pkg/referenceutil" @@ -35,35 +34,29 @@ import ( ) // Pull pulls an image specified by `rawRef`. -func Pull(ctx context.Context, client *containerd.Client, rawRef string, options types.ImagePullOptions) error { +func Pull(ctx context.Context, client *containerd.Client, rawRef string, options types.ImagePullOptions) (imageRef string, err error) { ocispecPlatforms, err := platformutil.NewOCISpecPlatformSlice(options.AllPlatforms, options.Platform) if err != nil { - return err + return "", err } unpack, err := strutil.ParseBoolOrAuto(options.Unpack) if err != nil { - return err + return "", err } - _, err = EnsureImage(ctx, client, rawRef, ocispecPlatforms, "always", unpack, options.Quiet, options) + img, err := EnsureImage(ctx, client, rawRef, ocispecPlatforms, "always", unpack, options.Quiet, options) if err != nil { - return err + return "", err } - return nil + return img.Ref, nil } // EnsureImage pulls an image either from ipfs or from registry. func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string, ocispecPlatforms []v1.Platform, pull string, unpack *bool, quiet bool, options types.ImagePullOptions) (*imgutil.EnsuredImage, error) { var ensured *imgutil.EnsuredImage - var progressHandler jobs.StatusHandler - if options.ProgressHandler != nil { - progressHandler = options.ProgressHandler - } else { - progressHandler = jobs.PrintProgress(options.Stdout) - } pullCfg := imgutil.PullConfig{ Ref: rawRef, Platforms: ocispecPlatforms, @@ -73,7 +66,7 @@ func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string, Mode: pull, Unpack: unpack, Quiet: quiet, - ProgressHandler: progressHandler, + ProgressHandler: options.ProgressHandler, } if scheme, ref, err := referenceutil.ParseIPFSRefWithScheme(rawRef); err == nil { diff --git a/pkg/cmd/image/push.go b/pkg/cmd/image/push.go index dba4b5ab207..c23e02b12fd 100644 --- a/pkg/cmd/image/push.go +++ b/pkg/cmd/image/push.go @@ -33,7 +33,6 @@ import ( "github.com/containerd/nerdctl/pkg/api/types" "github.com/containerd/nerdctl/pkg/errutil" "github.com/containerd/nerdctl/pkg/imgutil/dockerconfigresolver" - "github.com/containerd/nerdctl/pkg/imgutil/jobs" "github.com/containerd/nerdctl/pkg/imgutil/push" "github.com/containerd/nerdctl/pkg/ipfs" "github.com/containerd/nerdctl/pkg/platformutil" @@ -48,10 +47,10 @@ import ( ) // Push pushes an image specified by `rawRef`. -func Push(ctx context.Context, client *containerd.Client, rawRef string, options types.ImagePushOptions) error { +func Push(ctx context.Context, client *containerd.Client, rawRef string, options types.ImagePushOptions) (imageRef string, err error) { if scheme, ref, err := referenceutil.ParseIPFSRefWithScheme(rawRef); err == nil { if scheme != "ipfs" { - return fmt.Errorf("ipfs scheme is only supported but got %q", scheme) + return "", fmt.Errorf("ipfs scheme is only supported but got %q", scheme) } logrus.Infof("pushing image %q to IPFS", ref) @@ -59,11 +58,11 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options if options.IpfsAddress != "" { dir, err := os.MkdirTemp("", "apidirtmp") if err != nil { - return err + return "", err } defer os.RemoveAll(dir) if err := os.WriteFile(filepath.Join(dir, "api"), []byte(options.IpfsAddress), 0600); err != nil { - return err + return "", err } ipfsPath = dir } @@ -75,22 +74,21 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options c, err := ipfs.Push(ctx, client, ref, layerConvert, options.AllPlatforms, options.Platforms, options.IpfsEnsureImage, ipfsPath) if err != nil { logrus.WithError(err).Warnf("ipfs push failed") - return err + return "", err } - fmt.Fprintln(options.Stdout, c) - return nil + return c, nil } named, err := refdocker.ParseDockerRef(rawRef) if err != nil { - return err + return "", err } ref := named.String() refDomain := refdocker.Domain(named) platMC, err := platformutil.NewMatchComparer(options.AllPlatforms, options.Platforms) if err != nil { - return err + return "", err } pushRef := ref if !options.AllPlatforms { @@ -100,9 +98,9 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options platImg, err := converter.Convert(ctx, client, pushRef, ref, converter.WithPlatform(platMC)) if err != nil { if len(options.Platforms) == 0 { - return fmt.Errorf("failed to create a tmp single-platform image %q: %w", pushRef, err) + return "", fmt.Errorf("failed to create a tmp single-platform image %q: %w", pushRef, err) } - return fmt.Errorf("failed to create a tmp reduced-platform image %q (platform=%v): %w", pushRef, options.Platforms, err) + return "", fmt.Errorf("failed to create a tmp reduced-platform image %q (platform=%v): %w", pushRef, options.Platforms, err) } defer client.ImageService().Delete(ctx, platImg.Name, images.SynchronousDelete()) logrus.Infof("pushing as a reduced-platform image (%s, %s)", platImg.Target.MediaType, platImg.Target.Digest) @@ -112,21 +110,14 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options pushRef = ref + "-tmp-esgz" esgzImg, err := converter.Convert(ctx, client, pushRef, ref, converter.WithPlatform(platMC), converter.WithLayerConvertFunc(eStargzConvertFunc())) if err != nil { - return fmt.Errorf("failed to convert to eStargz: %v", err) + return "", fmt.Errorf("failed to convert to eStargz: %v", err) } defer client.ImageService().Delete(ctx, esgzImg.Name, images.SynchronousDelete()) logrus.Infof("pushing as an eStargz image (%s, %s)", esgzImg.Target.MediaType, esgzImg.Target.Digest) } - var progressHandler jobs.StatusHandler - if options.ProgressHandler != nil { - progressHandler = options.ProgressHandler - } else { - progressHandler = jobs.PrintProgress(options.Stdout) - } - pushFunc := func(r remotes.Resolver) error { - return push.Push(ctx, client, r, progressHandler, pushRef, ref, platMC, options.AllowNondistributableArtifacts, options.Quiet) + return push.Push(ctx, client, r, options.ProgressHandler, pushRef, ref, platMC, options.AllowNondistributableArtifacts, options.Quiet) } var dOpts []dockerconfigresolver.Opt @@ -137,45 +128,46 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options dOpts = append(dOpts, dockerconfigresolver.WithHostsDirs(options.GOptions.HostsDir)) resolver, err := dockerconfigresolver.New(ctx, refDomain, dOpts...) if err != nil { - return err + return "", err } if err = pushFunc(resolver); err != nil { // In some circumstance (e.g. people just use 80 port to support pure http), the error will contain message like "dial tcp : connection refused" if !errutil.IsErrHTTPResponseToHTTPSClient(err) && !errutil.IsErrConnectionRefused(err) { - return err + return "", err } if options.GOptions.InsecureRegistry { logrus.WithError(err).Warnf("server %q does not seem to support HTTPS, falling back to plain HTTP", refDomain) dOpts = append(dOpts, dockerconfigresolver.WithPlainHTTP(true)) resolver, err = dockerconfigresolver.New(ctx, refDomain, dOpts...) if err != nil { - return err + return "", err } - return pushFunc(resolver) + err = pushFunc(resolver) + if err != nil { + return "", err + } + return ref, err } logrus.WithError(err).Errorf("server %q does not seem to support HTTPS", refDomain) logrus.Info("Hint: you may want to try --insecure-registry to allow plain HTTP (if you are in a trusted network)") - return err + return "", err } img, err := client.ImageService().Get(ctx, pushRef) if err != nil { - return err + return "", err } refSpec, err := reference.Parse(pushRef) if err != nil { - return err + return "", err } signRef := fmt.Sprintf("%s@%s", refSpec.String(), img.Target.Digest.String()) if err = signutil.Sign(signRef, options.GOptions.Experimental, options.SignOptions); err != nil { - return err - } - if options.Quiet { - fmt.Fprintln(options.Stdout, ref) + return "", err } - return nil + return ref, nil } func eStargzConvertFunc() converter.ConvertFunc { diff --git a/pkg/imgutil/pull/pull.go b/pkg/imgutil/pull/pull.go index 6b85890b006..5cee4bcbfdc 100644 --- a/pkg/imgutil/pull/pull.go +++ b/pkg/imgutil/pull/pull.go @@ -92,10 +92,10 @@ func Pull(ctx context.Context, client *containerd.Client, ref string, config *Co img = containerd.NewImageWithPlatform(client, imagesImg, platformMC) } stopProgress() + <-progress if err != nil { return nil, err } - <-progress return img, nil }