From 7f6c629719633735a693e175f37c23603927939e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Schmitz=20von=20H=C3=BClst?= Date: Mon, 5 Oct 2026 23:45:22 +0200 Subject: [PATCH 1/4] Add spinners to xpkg build and xpkg push Extend the existing terminal spinner to the two commands that were missing progress feedback during long-running work: - xpkg build now wraps package building and writing to disk with success spinners. - xpkg push now wraps package reading, per-image pushes, and the multi-platform index push with success spinners. xpkg batch already logs per-retry progress and keeps calling pushImages with a nil spinner, so its behavior is unchanged. Fixes #411 Signed-off-by: boxcee-interview --- cmd/crossplane/xpkg/batch.go | 2 +- cmd/crossplane/xpkg/build.go | 28 ++++++++++------ cmd/crossplane/xpkg/push.go | 62 ++++++++++++++++++++++++------------ 3 files changed, 62 insertions(+), 30 deletions(-) diff --git a/cmd/crossplane/xpkg/batch.go b/cmd/crossplane/xpkg/batch.go index 952e8d38..5055a6a0 100644 --- a/cmd/crossplane/xpkg/batch.go +++ b/cmd/crossplane/xpkg/batch.go @@ -281,7 +281,7 @@ func (c *batchCmd) pushWithRetry(logger logging.Logger, imgs []packageImage, s s retryMsg := "" for i := range tries { logger.Info(fmt.Sprintf("Pushing xpkg to %s.%s", t, retryMsg)) - err := pushImages(logger, imgs, t, nil) + err := pushImages(logger, nil, imgs, t, nil) if err == nil { break } diff --git a/cmd/crossplane/xpkg/build.go b/cmd/crossplane/xpkg/build.go index eb48e7d9..a2f83883 100644 --- a/cmd/crossplane/xpkg/build.go +++ b/cmd/crossplane/xpkg/build.go @@ -35,6 +35,8 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/xpkg/parser/examples" "github.com/crossplane/crossplane-runtime/v2/pkg/xpkg/parser/yaml" + "github.com/crossplane/cli/v2/internal/terminal" + _ "embed" ) @@ -164,14 +166,20 @@ func (c *buildCmd) GetOutputFileName(meta runtime.Object, hash v1.Hash) (string, } // Run executes the build command. -func (c *buildCmd) Run(logger logging.Logger) error { +func (c *buildCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter) error { buildOpts, err := c.GetRuntimeBaseImageOpts() if err != nil { return errors.Wrap(err, errGetRuntimeBaseImageOpts) } - img, meta, err := c.builder.Build(context.Background(), buildOpts...) - if err != nil { + var img v1.Image + var meta runtime.Object + + if err := sp.WrapWithSuccessSpinner("Building package", func() error { + var err error + img, meta, err = c.builder.Build(context.Background(), buildOpts...) + return err + }); err != nil { return errors.Wrap(err, errBuildPackage) } @@ -185,14 +193,16 @@ func (c *buildCmd) Run(logger logging.Logger) error { return err } - f, err := c.fs.Create(output) - if err != nil { - return errors.Wrap(err, errCreatePackage) - } + if err := sp.WrapWithSuccessSpinner("Writing package to disk", func() error { + f, err := c.fs.Create(output) + if err != nil { + return errors.Wrap(err, errCreatePackage) + } - defer func() { _ = f.Close() }() + defer func() { _ = f.Close() }() - if err := tarball.Write(nil, img, f); err != nil { + return tarball.Write(nil, img, f) + }); err != nil { return err } diff --git a/cmd/crossplane/xpkg/push.go b/cmd/crossplane/xpkg/push.go index 0c3ea801..d91d2255 100644 --- a/cmd/crossplane/xpkg/push.go +++ b/cmd/crossplane/xpkg/push.go @@ -38,6 +38,8 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/logging" "github.com/crossplane/crossplane-runtime/v2/pkg/xpkg" + "github.com/crossplane/cli/v2/internal/terminal" + _ "embed" ) @@ -85,7 +87,7 @@ func (c *pushCmd) AfterApply() error { } // Run runs the push cmd. -func (c *pushCmd) Run(logger logging.Logger) error { +func (c *pushCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter) error { anns, err := parseAnnotations(c.OCIAnnotation) if err != nil { return errors.Wrap(err, errParseAnnotations) @@ -110,15 +112,21 @@ func (c *pushCmd) Run(logger logging.Logger) error { // load images from all the provided package files images := make([]packageImage, 0, len(c.PackageFiles)) - for _, p := range c.PackageFiles { - cleanPath := filepath.Clean(p) + if err := sp.WrapWithSuccessSpinner("Reading packages", func() error { + for _, p := range c.PackageFiles { + cleanPath := filepath.Clean(p) - img, err := tarball.ImageFromPath(cleanPath, nil) - if err != nil { - return err + img, err := tarball.ImageFromPath(cleanPath, nil) + if err != nil { + return err + } + + images = append(images, packageImage{Image: img, Path: cleanPath}) } - images = append(images, packageImage{Image: img, Path: cleanPath}) + return nil + }); err != nil { + return err } t := http.DefaultTransport.(*http.Transport).Clone() //nolint:forcetypeassert // http.DefaultTransport is always *http.Transport @@ -133,7 +141,7 @@ func (c *pushCmd) Run(logger logging.Logger) error { remote.WithTransport(t), } - return pushImages(logger, images, c.Package, anns, options...) + return pushImages(logger, sp, images, c.Package, anns, options...) } // packageImage describes a package image that will be pushed. @@ -147,7 +155,7 @@ type packageImage struct { } // pushImages pushes package images to the given URL using the provided options. -func pushImages(logger logging.Logger, images []packageImage, url string, annotations map[string]string, options ...remote.Option) error { +func pushImages(logger logging.Logger, sp terminal.SpinnerPrinter, images []packageImage, url string, annotations map[string]string, options ...remote.Option) error { if len(options) == 0 { options = []remote.Option{ remote.WithAuthFromKeychain(authn.DefaultKeychain), @@ -170,13 +178,20 @@ func pushImages(logger logging.Logger, images []packageImage, url string, annota img = annotateImage(img, annotations) - if err := remote.Write(tag, img, options...); err != nil { - return errors.Wrapf(err, errFmtPushPackage, pi.Path) - } + push := func() error { + if err := remote.Write(tag, img, options...); err != nil { + return errors.Wrapf(err, errFmtPushPackage, pi.Path) + } - logger.Debug("Pushed package", "path", pi.Path, "ref", tag.String()) + logger.Debug("Pushed package", "path", pi.Path, "ref", tag.String()) - return nil + return nil + } + + if sp != nil { + return sp.WrapWithSuccessSpinner("Pushing package", push) + } + return push() } // If there's more than one package file we'll write (push) them all by @@ -241,12 +256,19 @@ func pushImages(logger logging.Logger, images []packageImage, url string, annota return err } - idx := annotateIndex(mutate.AppendManifests(empty.Index, adds...), annotations) - if err := remote.WriteIndex(tag, idx, options...); err != nil { - return errors.Wrapf(err, errFmtWriteIndex, len(adds)) - } + pushIndex := func() error { + idx := annotateIndex(mutate.AppendManifests(empty.Index, adds...), annotations) + if err := remote.WriteIndex(tag, idx, options...); err != nil { + return errors.Wrapf(err, errFmtWriteIndex, len(adds)) + } - logger.Debug("Wrote OCI index", "ref", tag.String(), "manifests", len(adds)) + logger.Debug("Wrote OCI index", "ref", tag.String(), "manifests", len(adds)) - return nil + return nil + } + + if sp != nil { + return sp.WrapWithSuccessSpinner("Pushing package index", pushIndex) + } + return pushIndex() } From c1c05329df25c4095f33daf5fae285285e82d556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Schmitz=20von=20H=C3=BClst?= Date: Tue, 6 Oct 2026 01:17:19 +0200 Subject: [PATCH 2/4] xpkg: add Pushing packages spinner for multi-package push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps the concurrent image-upload g.Wait() phase in a success spinner so multi-platform pushes show progress, as requested in review of #412. The nil-spinner path (xpkg batch) is unchanged. Fixes review feedback on #412 (closes #411 remains as-is). Signed-off-by: Moritz Schmitz von Hülst --- cmd/crossplane/xpkg/push.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cmd/crossplane/xpkg/push.go b/cmd/crossplane/xpkg/push.go index d91d2255..7a4fa366 100644 --- a/cmd/crossplane/xpkg/push.go +++ b/cmd/crossplane/xpkg/push.go @@ -252,7 +252,13 @@ func pushImages(logger logging.Logger, sp terminal.SpinnerPrinter, images []pack }) } - if err := g.Wait(); err != nil { + if sp != nil { + if err := sp.WrapWithSuccessSpinner("Pushing packages", func() error { + return g.Wait() + }); err != nil { + return err + } + } else if err := g.Wait(); err != nil { return err } From eccee162e3f04387183040b8c12a9c8a4a4f3b9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Schmitz=20von=20H=C3=BClst?= Date: Sun, 11 Oct 2026 23:51:11 +0200 Subject: [PATCH 3/4] fix: extract per-image push into pushSingle to satisfy gocognit (#411) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Moritz Schmitz von Hülst --- cmd/crossplane/xpkg/push.go | 99 +++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 47 deletions(-) diff --git a/cmd/crossplane/xpkg/push.go b/cmd/crossplane/xpkg/push.go index 7a4fa366..f929dc38 100644 --- a/cmd/crossplane/xpkg/push.go +++ b/cmd/crossplane/xpkg/push.go @@ -154,6 +154,57 @@ type packageImage struct { Path string } +// pushSingle annotates, digests and pushes one package image by digest reference. +func pushSingle(ctx context.Context, adds *[]mutate.IndexAddendum, i int, pi packageImage, tag name.Tag, annotations map[string]string, logger logging.Logger, options ...remote.Option) error { + img, err := xpkg.AnnotateLayers(pi.Image) + if err != nil { + return errors.Wrapf(err, errAnnotateLayers) + } + + img = annotateImage(img, annotations) + + d, err := img.Digest() + if err != nil { + return errors.Wrapf(err, errFmtGetDigest, pi.Path) + } + + n := fmt.Sprintf("%s@%s", tag.Repository.Name(), d.String()) + + ref, err := name.NewDigest(n, name.StrictValidation) + if err != nil { + return errors.Wrapf(err, errFmtNewDigest, n, pi.Path) + } + + mt, err := img.MediaType() + if err != nil { + return errors.Wrapf(err, errFmtGetMediaType, pi.Path) + } + + conf, err := img.ConfigFile() + if err != nil { + return errors.Wrapf(err, errFmtGetConfigFile, pi.Path) + } + + (*adds)[i] = mutate.IndexAddendum{ + Add: img, + Descriptor: v1.Descriptor{ + MediaType: mt, + Platform: &v1.Platform{ + Architecture: conf.Architecture, + OS: conf.OS, + OSVersion: conf.OSVersion, + }, + }, + } + if err := remote.Write(ref, img, append(options, remote.WithContext(ctx))...); err != nil { + return errors.Wrapf(err, errFmtPushPackage, pi.Path) + } + + logger.Debug("Pushed package", "path", pi.Path, "ref", ref.String()) + + return nil +} + // pushImages pushes package images to the given URL using the provided options. func pushImages(logger logging.Logger, sp terminal.SpinnerPrinter, images []packageImage, url string, annotations map[string]string, options ...remote.Option) error { if len(options) == 0 { @@ -202,53 +253,7 @@ func pushImages(logger logging.Logger, sp terminal.SpinnerPrinter, images []pack g, ctx := errgroup.WithContext(context.Background()) for i, pi := range images { g.Go(func() error { - img, err := xpkg.AnnotateLayers(pi.Image) - if err != nil { - return errors.Wrapf(err, errAnnotateLayers) - } - - img = annotateImage(img, annotations) - - d, err := img.Digest() - if err != nil { - return errors.Wrapf(err, errFmtGetDigest, pi.Path) - } - - n := fmt.Sprintf("%s@%s", tag.Repository.Name(), d.String()) - - ref, err := name.NewDigest(n, name.StrictValidation) - if err != nil { - return errors.Wrapf(err, errFmtNewDigest, n, pi.Path) - } - - mt, err := img.MediaType() - if err != nil { - return errors.Wrapf(err, errFmtGetMediaType, pi.Path) - } - - conf, err := img.ConfigFile() - if err != nil { - return errors.Wrapf(err, errFmtGetConfigFile, pi.Path) - } - - adds[i] = mutate.IndexAddendum{ - Add: img, - Descriptor: v1.Descriptor{ - MediaType: mt, - Platform: &v1.Platform{ - Architecture: conf.Architecture, - OS: conf.OS, - OSVersion: conf.OSVersion, - }, - }, - } - if err := remote.Write(ref, img, append(options, remote.WithContext(ctx))...); err != nil { - return errors.Wrapf(err, errFmtPushPackage, pi.Path) - } - - logger.Debug("Pushed package", "path", pi.Path, "ref", ref.String()) - - return nil + return pushSingle(ctx, &adds, i, pi, tag, annotations, logger, options...) }) } From a02de40c88be3acce0cccab6c51b2158535b52be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Schmitz=20von=20H=C3=BClst?= Date: Fri, 16 Oct 2026 21:43:26 +0200 Subject: [PATCH 4/4] xpkg: address review feedback on push/batch spinners (#411) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - push.go: pass g.Wait directly instead of wrapping it in a closure - batch.go: replace the nil SpinnerPrinter with a new NopSpinnerPrinter - terminal: add NopSpinnerPrinter and remove the unused NewSuccessSpinner interface method (confirmed no callers) Signed-off-by: Moritz Schmitz von Hülst --- cmd/crossplane/xpkg/batch.go | 4 +++- cmd/crossplane/xpkg/push.go | 4 +--- internal/terminal/spinner.go | 23 ++++++++++++++++++----- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/cmd/crossplane/xpkg/batch.go b/cmd/crossplane/xpkg/batch.go index 5055a6a0..e5313c8b 100644 --- a/cmd/crossplane/xpkg/batch.go +++ b/cmd/crossplane/xpkg/batch.go @@ -41,6 +41,8 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/xpkg/parser" "github.com/crossplane/crossplane-runtime/v2/pkg/xpkg/parser/examples" "github.com/crossplane/crossplane-runtime/v2/pkg/xpkg/parser/yaml" + + "github.com/crossplane/cli/v2/internal/terminal" ) const ( @@ -281,7 +283,7 @@ func (c *batchCmd) pushWithRetry(logger logging.Logger, imgs []packageImage, s s retryMsg := "" for i := range tries { logger.Info(fmt.Sprintf("Pushing xpkg to %s.%s", t, retryMsg)) - err := pushImages(logger, nil, imgs, t, nil) + err := pushImages(logger, terminal.NewNopSpinnerPrinter(), imgs, t, nil) if err == nil { break } diff --git a/cmd/crossplane/xpkg/push.go b/cmd/crossplane/xpkg/push.go index f929dc38..de5dd710 100644 --- a/cmd/crossplane/xpkg/push.go +++ b/cmd/crossplane/xpkg/push.go @@ -258,9 +258,7 @@ func pushImages(logger logging.Logger, sp terminal.SpinnerPrinter, images []pack } if sp != nil { - if err := sp.WrapWithSuccessSpinner("Pushing packages", func() error { - return g.Wait() - }); err != nil { + if err := sp.WrapWithSuccessSpinner("Pushing packages", g.Wait); err != nil { return err } } else if err := g.Wait(); err != nil { diff --git a/internal/terminal/spinner.go b/internal/terminal/spinner.go index 49ed5ccd..bea33a44 100644 --- a/internal/terminal/spinner.go +++ b/internal/terminal/spinner.go @@ -36,9 +36,6 @@ import ( // SpinnerPrinter prints spinners to the console. type SpinnerPrinter interface { - // NewSuccessSpinner returns a new success spinner. - NewSuccessSpinner(msg string) *SuccessSpinner - // WrapWithSuccessSpinner adds spinners around message and run function. WrapWithSuccessSpinner(msg string, f func() error) error @@ -64,8 +61,24 @@ func NewSpinnerPrinter(out io.Writer, pretty bool) SpinnerPrinter { } } -func (p *defaultSpinnerPrinter) NewSuccessSpinner(msg string) *SuccessSpinner { - return newSuccessSpinner(p.out, msg) +// NopSpinnerPrinter is a SpinnerPrinter that renders nothing; its Wrap +// methods just run the wrapped function. +type NopSpinnerPrinter struct{} + +// NewNopSpinnerPrinter returns a new NopSpinnerPrinter. +func NewNopSpinnerPrinter() SpinnerPrinter { + return &NopSpinnerPrinter{} +} + +// WrapWithSuccessSpinner runs f without rendering any spinner. +func (p *NopSpinnerPrinter) WrapWithSuccessSpinner(_ string, f func() error) error { + return f() +} + +// WrapAsyncWithSuccessSpinners runs fn without rendering any spinner. +func (p *NopSpinnerPrinter) WrapAsyncWithSuccessSpinners(fn func(ch async.EventChannel) error) error { + var ch async.EventChannel + return fn(ch) } func (p *defaultSpinnerPrinter) WrapWithSuccessSpinner(msg string, f func() error) error {