Skip to content

Commit

Permalink
cmd/create: Don't block user interaction while fetching the image size
Browse files Browse the repository at this point in the history
It takes 'skopeo inspect' a few seconds to fetch the image size from the
remote registry, and while that happens the user can't interact with the
image download prompt:
  $ toolbox create
  Image required to create toolbox container.
  <wait for a few seconds>
  Download registry.fedoraproject.org/fedora-toolbox:39 (359.8MB)? [y/N]:

This feels awkward because it's not clear to the user what's going on
during those few seconds.  Moreover, while knowing the image size can be
convenient at times, for example when disk space and network bandwidth
are limited, it's not always important.

It will be better if 'skopeo inspect' ran in the background, while
waiting for the user to respond to the image download prompt, and once
the image size has been fetched, the prompt can be updated to include
it.

So, initially:
  $ toolbox create
  Image required to create toolbox container.
  Download registry.fedoraproject.org/fedora-toolbox:39 (...)? [y/N]:

... and then once the size is available:
  $ toolbox create
  Image required to create toolbox container.
  Download registry.fedoraproject.org/fedora-toolbox:39 (359.8MB)? [y/N]:

If skopeo(1) is missing or too old, then the prompt can continue without
the size, as it did before:
  $ toolbox create
  Image required to create toolbox container.
  Download registry.fedoraproject.org/fedora-toolbox:39 [y/N]:

#752
#1263
  • Loading branch information
debarshiray committed Dec 7, 2023
1 parent 8401092 commit aa48a0c
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 16 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/ubuntu-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:
fish \
gcc \
go-md2man \
golang \
golang-1.20 \
meson \
ninja-build \
openssl \
Expand All @@ -54,6 +54,9 @@ jobs:
systemd \
udisks2
- name: Set up PATH for Go 1.20
run: echo "export PATH=/usr/lib/go-1.20/bin:\$PATH" >> $GITHUB_ENV

- name: Checkout Bats
uses: actions/checkout@v3
with:
Expand Down
165 changes: 151 additions & 14 deletions src/cmd/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,8 +576,7 @@ func getFullyQualifiedImageFromRepoTags(image string) (string, error) {
return imageFull, nil
}

func getImageSizeFromRegistry(imageFull string) (string, error) {
ctx := context.Background()
func getImageSizeFromRegistry(ctx context.Context, imageFull string) (string, error) {
image, err := skopeo.Inspect(ctx, imageFull)
if err != nil {
return "", err
Expand All @@ -601,6 +600,23 @@ func getImageSizeFromRegistry(imageFull string) (string, error) {
return imageSizeHuman, nil
}

func getImageSizeFromRegistryAsync(ctx context.Context, imageFull string) (<-chan string, <-chan error) {
retValCh := make(chan string)
errCh := make(chan error)

go func() {
imageSize, err := getImageSizeFromRegistry(ctx, imageFull)
if err != nil {
errCh <- err
return
}

retValCh <- imageSize
}()

return retValCh, errCh
}

func getServiceSocket(serviceName string, unitName string) (string, error) {
logrus.Debugf("Resolving path to the %s socket", serviceName)

Expand Down Expand Up @@ -713,18 +729,7 @@ func pullImage(image, release, authFile string) (bool, error) {
}

if promptForDownload {
fmt.Println("Image required to create toolbox container.")

var prompt string

if imageSize, err := getImageSizeFromRegistry(imageFull); err != nil {
logrus.Debugf("Getting image size failed: %s", err)
prompt = fmt.Sprintf("Download %s? [y/N]:", imageFull)
} else {
prompt = fmt.Sprintf("Download %s (%s)? [y/N]:", imageFull, imageSize)
}

shouldPullImage = askForConfirmation(prompt)
shouldPullImage = showPromptForDownload(imageFull)
}

if !shouldPullImage {
Expand Down Expand Up @@ -756,6 +761,138 @@ func pullImage(image, release, authFile string) (bool, error) {
return true, nil
}

func showPromptForDownload(imageFull string) bool {
fmt.Println("Image required to create toolbox container.")

prompt := fmt.Sprintf("Download %s ( ... MB)? [y/N]:", imageFull)

parentCtx := context.Background()
askCtx, askCancel := context.WithCancelCause(parentCtx)
defer askCancel(errors.New("clean-up"))

askCh, askErrCh := askForConfirmationAsync(askCtx, prompt, false)

imageSizeCtx, imageSizeCancel := context.WithCancelCause(parentCtx)
defer imageSizeCancel(errors.New("clean-up"))

imageSizeCh, imageSizeErrCh := getImageSizeFromRegistryAsync(imageSizeCtx, imageFull)

var imageSize string
var shouldPullImage bool

select {
case val := <-askCh:
shouldPullImage = val
cause := fmt.Errorf("%w: received confirmation without image size", context.Canceled)
imageSizeCancel(cause)
case err := <-askErrCh:
shouldPullImage = false
cause := fmt.Errorf("failed to ask for confirmation without image size: %w", err)
imageSizeCancel(cause)
case val := <-imageSizeCh:
imageSize = val
cause := fmt.Errorf("%w: received image size", context.Canceled)
askCancel(cause)
case err := <-imageSizeErrCh:
cause := fmt.Errorf("failed to get image size: %w", err)
askCancel(cause)
}

if imageSizeCtx.Err() != nil && askCtx.Err() == nil {
cause := context.Cause(imageSizeCtx)
logrus.Debugf("Show prompt for download: image size canceled: %s", cause)
return shouldPullImage
}

var done bool

if imageSizeCtx.Err() == nil && askCtx.Err() != nil {
select {
case val := <-askCh:
logrus.Debugf("Show prompt for download: received pending confirmation without image size")
shouldPullImage = val
done = true
case err := <-askErrCh:
logrus.Debugf("Show prompt for download: failed to ask for confirmation without image size: %s",
err)
}
} else {
panic("code should not be reached")
}

cause := context.Cause(askCtx)
logrus.Debugf("Show prompt for download: ask canceled: %s", cause)

if done {
return shouldPullImage
}

var restoreCursor bool

if errors.Is(cause, context.Canceled) {
imageSizeLen := len(imageSize)
var padding1 int
var padding2 int
var padding3 int
var padding4 int

if imageSizeLen < 7 {
padding4 = 7 - imageSizeLen
}

if padding4 > 1 {
padding3 = padding4 - 1
padding4 = 1
}

if padding3 > 1 {
padding2 = padding3 - 1
padding3 = 1
}

if padding2 > 1 {
padding1 = padding2 - 1
padding2 = 1
}

prompt = fmt.Sprintf("Download %s (%*s%s%*s)? %*s[y/N]:%*s",
imageFull,
padding1, "",
imageSize,
padding2, "",
padding3, "",
padding4, "")

// Save the cursor position.
fmt.Printf("\033[s")

restoreCursor = true
} else {
prompt = fmt.Sprintf("Download %s? [y/N]:", imageFull)

// Delete entire line regardless of cursor position.
fmt.Printf("\033[2K")
}

fmt.Printf("\r")

askAgainCtx, askAgainCancel := context.WithCancelCause(parentCtx)
defer askAgainCancel(errors.New("clean-up"))

askAgainCh, askAgainErrCh := askForConfirmationAsync(askAgainCtx, prompt, restoreCursor)

select {
case val := <-askAgainCh:
logrus.Debug("Show prompt for download: received confirmation with image size")
shouldPullImage = val
case err := <-askAgainErrCh:
logrus.Debugf("Show prompt for download: failed to ask for confirmation with image size: %s", err)
shouldPullImage = false
}

return shouldPullImage
}

// systemdNeedsEscape checks whether a byte in a potential dbus ObjectPath needs to be escaped
func systemdNeedsEscape(i int, b byte) bool {
// Escape everything that is not a-z-A-Z-0-9
Expand Down
2 changes: 1 addition & 1 deletion src/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/containers/toolbox

go 1.14
go 1.20

require (
github.com/HarryMichal/go-version v1.0.1
Expand Down

0 comments on commit aa48a0c

Please sign in to comment.