Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report pulled image size #752

Closed
HarryMichal opened this issue Apr 8, 2021 · 7 comments
Closed

Report pulled image size #752

HarryMichal opened this issue Apr 8, 2021 · 7 comments
Assignees
Labels
1. Feature request A request for a new feature 2. Host Realm The issue is related to what happens on the host machine where Toolbox is executed

Comments

@HarryMichal
Copy link
Member

Is your feature request related to a problem? Please describe.

When Toolbox offers to pull an image, it estimates the size of the pulled image will be 500MB. This number is hard-coded and in most cases does not represent the reality. Toolbox images are usually less then 400MB big and that is after decompression. Compressed images are even smaller.

Describe the solution you'd like

Have a mechanism for reporting the size of the pulled image.

Additional context

This will, probably, require some work in https://github.com/containers/skopeo. Relevant upstream issue.

@HarryMichal HarryMichal added 1. Feature request A request for a new feature 2. Host Realm The issue is related to what happens on the host machine where Toolbox is executed labels Apr 8, 2021
juhp added a commit to juhp/toolbox that referenced this issue Sep 13, 2021
The download in particular and diskspace are less.
juhp added a commit to juhp/toolbox that referenced this issue Nov 16, 2021
fedora-toolbox:35 skopeo image Size is currently ~340MB
juhp added a commit to juhp/toolbox that referenced this issue Nov 16, 2021
fedora-toolbox:35 skopeo image Size is currently ~340MB
@debarshiray
Copy link
Member

We should be careful not to add a hard dependency on skopeo. Toolbx is part of size-constrained operating systems like Fedora CoreOS, where an extra mandatory dependency on skopeo might be problematic. We should attempt to use it, but if it's absent, we should have a fallback of some sort.

@debarshiray
Copy link
Member

debarshiray commented Nov 30, 2022

That commit from @juhp that is showing up against this issue can be ignored. It was addressed earlier in #875 (comment)

@nievesmontero
Copy link
Collaborator

In order to add this feature, I'm going through the create.go file. The variable %s is used all along this file and it gets the following value registry.fedoraproject.org/fedora-toolbox:36. Where does it take that value? In the create.go file or in some other file? I think I need that information to implement the feature.

@debarshiray
Copy link
Member

In order to add this feature, I'm going through the create.go file. The variable %s is used all along this file and it gets the following value registry.fedoraproject.org/fedora-toolbox:36. Where does it take that value?

That %s is a format specifier (or format verb as Go calls it) that's used in printf format strings. Can you show me the exact line(s) that you are looking at?

@nievesmontero
Copy link
Collaborator

nievesmontero commented Dec 12, 2022

The main lines I've been looking at are from the 700-705 of the create.go file. The variable imageFull is the one from which I think we should get the image size.

@debarshiray
Copy link
Member

You mean these lines in src/cmd/create.go:

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

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

Yes, imageFull has the fully qualified image name. eg., registry.fedoraproject.org/fedora-toolbox:37.

nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Jan 9, 2023
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Jan 12, 2023
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Jan 16, 2023
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Jan 18, 2023
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Jan 19, 2023
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Jan 19, 2023
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Jan 19, 2023
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Jan 26, 2023
Fix containers#752

The following package and file have been created src/pkg/skopeo/skopeo.go
Signed-off-by: Nieves Montero <[email protected]>
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Feb 9, 2023
Fix containers#752

The following package and file have been created src/pkg/skopeo/skopeo.go
Signed-off-by: Nieves Montero <[email protected]>
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Feb 20, 2023
Fix containers#752

The following package and file have been created src/pkg/skopeo/skopeo.go
Signed-off-by: Nieves Montero <[email protected]>
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Feb 20, 2023
Fix containers#752

The following package and file have been created src/pkg/skopeo/skopeo.go
Signed-off-by: Nieves Montero <[email protected]>
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Feb 20, 2023
Fix containers#752

The following package and file have been created src/pkg/skopeo/skopeo.go
Signed-off-by: Nieves Montero <[email protected]>
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Feb 20, 2023
Fix containers#752

The following package and file have been created src/pkg/skopeo/skopeo.go
Signed-off-by: Nieves Montero <[email protected]>
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Feb 20, 2023
Fix containers#752

The following package and file have been created src/pkg/skopeo/skopeo.go
Signed-off-by: Nieves Montero <[email protected]>
nievesmontero added a commit to nievesmontero/toolbox that referenced this issue Feb 20, 2023
Fix containers#752

The following package and file have been created src/pkg/skopeo/skopeo.go
Signed-off-by: Nieves Montero <[email protected]>
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 12, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.

To do this, at some point, the terminal device will be put into
non-canonical mode input and the echoing of input characters will be
disabled to retain full control of the cursor position.  Unfortunately,
this will require access to the full termios(3) struct that isn't given
by golang.org/x/term, and, hence, the code needs to be written using the
underlying termios(3) API.

This future code will have enough overlap with the IsTerminal API from
golang.org/x/term that it doesn't make sense to use a separate module
(ie., golang.org/x/term) for it.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 13, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  Initially, the prompt will be
shown without the image size.  Once the size has been fetched, the older
prompt will be cancelled and a new one will be shown that includes the
size.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 13, 2023
This is meant to avoid duplicating the code that shows the prompt, and
reads and parses the user's input.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 13, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.

Initially, the prompt will be shown without the image size.  Once the
size has been fetched, the older prompt will be cancelled and a new one
will be shown that includes the size.  While the prompt is getting
updated, the terminal device will be put into non-canonical mode input
and the echoing of input characters will be disabled to retain full
control of the cursor position.  Once the new prompt is in place, the
previous state of the terminal will be restored.  However, anything that
was typed in the interim will be discarded to avoid surprising the user
with invisible input.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 13, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.

To do this, at some point, the terminal device will be put into
non-canonical mode input and the echoing of input characters will be
disabled to retain full control of the cursor position.  Unfortunately,
this will require access to the full termios(3) struct that isn't given
by golang.org/x/term, and, hence, the code needs to be written using the
underlying termios(3) API.

This future code will have enough overlap with the IsTerminal API from
golang.org/x/term that it doesn't make sense to use a separate module
(ie., golang.org/x/term) for it.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 13, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  To do this, at some point, the
terminal device will be put into non-canonical mode input and the
echoing of input characters will be disabled to retain full control of
the cursor position.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 13, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.

To do this, at some point, the terminal device will be put into
non-canonical mode input and the echoing of input characters will be
disabled to retain full control of the cursor position.  Unfortunately,
this will require access to the full termios(3) struct that isn't given
by golang.org/x/term, and, hence, the code needs to be written using the
underlying termios(3) API.

This future code will have enough overlap with the IsTerminal API from
golang.org/x/term that it doesn't make sense to use a separate module
(ie., golang.org/x/term) for it.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 13, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  To do this, at some point, the
terminal device will be put into non-canonical mode input and the
echoing of input characters will be disabled to retain full control of
the cursor position.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 13, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  Initially, the prompt will be
shown without the image size.  If the user responds before the size is
fetched, then the pending 'skopeo inspect' will be cancelled.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 14, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  Initially, the prompt will be
shown without the image size.  If the user responds before the size is
fetched, then the pending 'skopeo inspect' will be cancelled.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 14, 2023
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 image download 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]:

Updating the image download prompt with the results of 'skopeo inspect'
is vulnerable to races.  At the same time as the terminal's cursor is
moved to the beginning of the current line to overwrite the earlier
prompt with the new one, the user can keep typing, which will keep
moving the cursor forward.  This competition over the cursor can lead to
awkward outcomes. For example, the prompt can overwrite the characters
typed in by the user, leaving characters in the terminal's input buffer
waiting for the user to hit ENTER, even though they are not visible on
the screen.  Another example is that hitting BACKSPACE can end up
deleting parts of the prompt, instead of stopping at the edge.

This is solved by putting the terminal device into non-canonical mode
input and disabling the echoing of input characters, while the prompt is
being updated.  This prevents input from moving the terminal's cursor
forward, and from accumulating in the terminal's input buffer even if
it might not be visible.  Any input during this interim period is
discarded and replaced by '...', and a fresh new prompt is shown in the
following line.

In practice, this race shouldn't be too common.  It can only happen if
the user is typing right when the prompt is being updated, which is
unlikely because it's only supposed to be a short 'yes' or 'no' input.

The use of the context.Cause and context.WithCancelCause functions [1]
requires Go >= 1.20.  Bumping the Go version in src/go.mod then requires
a 'go mod tidy'.  Otherwise, it leads to:
  $ meson compile -C builddir --verbose
  ...
  /home/rishi/devel/containers/git/toolbox/src/go-build-wrapper
    /home/rishi/devel/containers/git/toolbox/src
    /home/rishi/devel/containers/git/toolbox/builddir src/toolbox
    0.0.99.4 cc /lib64/ld-linux-x86-64.so.2 false
  go: updates to go.mod needed; to update it:
          go mod tidy
  ninja: build stopped: subcommand failed.

[1] https://pkg.go.dev/context

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 14, 2023
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 image download 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]:

Updating the image download prompt with the results of 'skopeo inspect'
is vulnerable to races.  At the same time as the terminal's cursor is
moved to the beginning of the current line to overwrite the earlier
prompt with the new one, the user can keep typing, which will keep
moving the cursor forward.  This competition over the cursor can lead to
awkward outcomes. For example, the prompt can overwrite the characters
typed in by the user, leaving characters in the terminal's input buffer
waiting for the user to hit ENTER, even though they are not visible on
the screen.  Another example is that hitting BACKSPACE can end up
deleting parts of the prompt, instead of stopping at the edge.

This is solved by putting the terminal device into non-canonical mode
input and disabling the echoing of input characters, while the prompt is
being updated.  This prevents input from moving the terminal's cursor
forward, and from accumulating in the terminal's input buffer even if
it might not be visible.  Any input during this interim period is
discarded and replaced by '...', and a fresh new prompt is shown in the
following line.

In practice, this race shouldn't be too common.  It can only happen if
the user is typing right when the prompt is being updated, which is
unlikely because it's only supposed to be a short 'yes' or 'no' input.

The use of the context.Cause and context.WithCancelCause functions [1]
requires Go >= 1.20.  Bumping the Go version in src/go.mod then requires
a 'go mod tidy'.  Otherwise, it leads to:
  $ meson compile -C builddir --verbose
  ...
  /home/rishi/devel/containers/git/toolbox/src/go-build-wrapper
    /home/rishi/devel/containers/git/toolbox/src
    /home/rishi/devel/containers/git/toolbox/builddir src/toolbox
    0.0.99.4 cc /lib64/ld-linux-x86-64.so.2 false
  go: updates to go.mod needed; to update it:
          go mod tidy
  ninja: build stopped: subcommand failed.

[1] https://pkg.go.dev/context

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 15, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  Initially, the prompt will be
shown without the image size.  Once the size has been fetched, the older
prompt will be cancelled and a new one will be shown that includes the
size.

Even though this code is only expected to be used to read from the
standard input stream, when it's connected to a terminal device, the use
of poll(2) here was tested with FIFOs or named pipes and regular files
as well, in case they might be necessary in future.

An eventfd(2) file descriptor expects a 8-byte or 64-bit integer value
to be given to write(2) to increase its counter by that amount [1].  In
C, it could be phrased as:
  uint64_t one = 1;
  write (eventfd, &one, sizeof(one));

However, Go's wrapper for write(2) expects a sequence of bytes (ie.,
[]byte), and not an arbitrary memory address.  Therefore the 'binary'
package [3] is used to encode the integer into a byte sequence as a
varint.

Even though a varint-encoded 64-bit integer takes a maximum of 10
bytes, as defined by binary.MaxVarintLen64, 1 byte is enough to encode
the number 1 as an unsigned 64-bit integer [3].  That's enough to fit
into a byte sequence of length 8 to satisfy what an eventfd(2) file
descriptor expects.  Ultimately, it doesn't matter exactly what value
the receiving end assigns to the number given to write(2), as long as
it's not zero.

[1] https://man7.org/linux/man-pages/man2/eventfd.2.html

[2] https://pkg.go.dev/golang.org/x/sys/unix#Write

[3] https://protobuf.dev/programming-guides/encoding/

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 15, 2023
This is meant to avoid duplicating the code that shows the prompt, and
reads and parses the user's input.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 15, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.

Initially, the prompt will be shown without the image size.  Once the
size has been fetched, the older prompt will be cancelled and a new one
will be shown that includes the size.  While the prompt is getting
updated, the terminal device will be put into non-canonical mode input
and the echoing of input characters will be disabled to retain full
control of the cursor position.  Once the new prompt is in place, the
previous state of the terminal will be restored.  However, anything that
was typed in the interim will be discarded to avoid surprising the user
with invisible input.

Even though this code is only expected to be used to read from the
standard input stream, when it's connected to a terminal device, the use
of poll(2) here was tested with FIFOs or named pipes and regular files
as well, in case they might be necessary in future.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 15, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.

To do this, at some point, the terminal device will be put into
non-canonical mode input and the echoing of input characters will be
disabled to retain full control of the cursor position.  Unfortunately,
this will require access to the full termios(3) struct that isn't given
by golang.org/x/term, and, hence, the code needs to be written using the
underlying termios(3) API.

This future code will have enough overlap with the IsTerminal API from
golang.org/x/term that it doesn't make sense to use a separate module
(ie., golang.org/x/term) for it.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 15, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  To do this, at some point, the
terminal device will be put into non-canonical mode input and the
echoing of input characters will be disabled to retain full control of
the cursor position.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 15, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  Initially, the prompt will be
shown without the image size.  If the user responds before the size is
fetched, then the pending 'skopeo inspect' will be cancelled.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 15, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  Initially, the prompt will be
shown without the image size.  If the user responds before the size is
fetched, then the pending 'skopeo inspect' will be cancelled.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 15, 2023
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 image download prompt can be
updated to include it.

So, initially:
  $ toolbox create
  Image required to create toolbox container.
  Download registry.fedoraproject.org/fedora-toolbox:39 ( ... MB)? [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]:

The placeholder for the missing image size (ie., ' ... MB') was chosen
to have seven characters, so that it matches the most common sizes.  The
human-readable representation of the image size is capped at four valid
numbers [1].  Unless it's a perfect round number like 1KB or 1.2MB, it
will likely use all four numbers and the decimal point, which is five
characters.  Then two more for the unit, because it's very unlikely that
there will be an image that's less than 1KB in size and will be shown in
bytes with a B.  That makes it seven characters in total.

Updating the image download prompt with the results of 'skopeo inspect'
is vulnerable to races.  At the same time as the terminal's cursor is
being moved to the beginning of the current line to overwrite the
earlier prompt with the new one, the user can keep typing and keep
moving the cursor forward.  This competition over the cursor can lead to
awkward outcomes.

For example, the prompt can overwrite the characters typed in by the
user, leaving characters in the terminal's input buffer waiting for the
user to hit ENTER, even though they are not visible on the screen.
Another example is that hitting BACKSPACE can end up deleting parts of
the prompt, instead of stopping at the edge.

This is solved by putting the terminal device into non-canonical mode
input and disabling the echoing of input characters, while the prompt is
being updated.  This prevents input from moving the terminal's cursor
forward, and from accumulating in the terminal's input buffer even if
it might not be visible.  Any input during this interim period is
discarded and replaced by '...', and a fresh new prompt is shown in the
following line.

In practice, this race shouldn't be too common.  It can only happen if
the user is typing right when the prompt is being updated, which is
unlikely because it's only supposed to be a short 'yes' or 'no' input.

The use of the context.Cause and context.WithCancelCause functions [2]
requires Go >= 1.20.  Bumping the Go version in src/go.mod then requires
a 'go mod tidy'.  Otherwise, it leads to:
  $ meson compile -C builddir --verbose
  ...
  /home/rishi/devel/containers/git/toolbox/src/go-build-wrapper
    /home/rishi/devel/containers/git/toolbox/src
    /home/rishi/devel/containers/git/toolbox/builddir src/toolbox
    0.0.99.4 cc /lib64/ld-linux-x86-64.so.2 false
  go: updates to go.mod needed; to update it:
          go mod tidy
  ninja: build stopped: subcommand failed.

[1] https://pkg.go.dev/github.com/docker/go-units#HumanSize

[2] https://pkg.go.dev/context

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 16, 2023
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 image download prompt can be
updated to include it.

So, initially:
  $ toolbox create
  Image required to create toolbox container.
  Download registry.fedoraproject.org/fedora-toolbox:39 ( ... MB)? [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]:

The placeholder for the missing image size (ie., ' ... MB') was chosen
to have seven characters, so that it matches the most common sizes.  The
human-readable representation of the image size is capped at four valid
numbers [1].  Unless it's a perfect round number like 1KB or 1.2MB, it
will likely use all four numbers and the decimal point, which is five
characters.  Then two more for the unit, because it's very unlikely that
there will be an image that's less than 1KB in size and will be shown in
bytes with a B.  That makes it seven characters in total.

Updating the image download prompt with the results of 'skopeo inspect'
is vulnerable to races.  At the same time as the terminal's cursor is
being moved to the beginning of the current line to overwrite the
earlier prompt with the new one, the user can keep typing and keep
moving the cursor forward.  This competition over the cursor can lead to
awkward outcomes.

For example, the prompt can overwrite the characters typed in by the
user, leaving characters in the terminal's input buffer waiting for the
user to hit ENTER, even though they are not visible on the screen.
Another example is that hitting BACKSPACE can end up deleting parts of
the prompt, instead of stopping at the edge.

This is solved by putting the terminal device into non-canonical mode
input and disabling the echoing of input characters, while the prompt is
being updated.  This prevents input from moving the terminal's cursor
forward, and from accumulating in the terminal's input buffer even if
it might not be visible.  Any input during this interim period is
discarded and replaced by '...', and a fresh new prompt is shown in the
following line.

In practice, this race shouldn't be too common.  It can only happen if
the user is typing right when the prompt is being updated, which is
unlikely because it's only supposed to be a short 'yes' or 'no' input.

The use of the context.Cause and context.WithCancelCause functions [2]
requires Go >= 1.20.  Bumping the Go version in src/go.mod then requires
a 'go mod tidy'.  Otherwise, it leads to:
  $ meson compile -C builddir --verbose
  ...
  /home/rishi/devel/containers/git/toolbox/src/go-build-wrapper
    /home/rishi/devel/containers/git/toolbox/src
    /home/rishi/devel/containers/git/toolbox/builddir src/toolbox
    0.0.99.4 cc /lib64/ld-linux-x86-64.so.2 false
  go: updates to go.mod needed; to update it:
          go mod tidy
  ninja: build stopped: subcommand failed.

[1] https://pkg.go.dev/github.com/docker/go-units#HumanSize

[2] https://pkg.go.dev/context

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 16, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  Initially, the prompt will be
shown without the image size.  Once the size has been fetched, the older
prompt will be cancelled and a new one will be shown that includes the
size.

Even though this code is only expected to be used to read from the
standard input stream when it's connected to a terminal device, the use
of poll(2) here was tested with FIFOs or named pipes and regular files
as well, in case they might be necessary in future.

An eventfd(2) file descriptor expects a 8-byte or 64-bit integer value
to be given to write(2) to increase its counter by that amount [1].  In
C, it could be phrased as:
  uint64_t one = 1;
  write (eventfd, &one, sizeof (one));

However, Go's wrapper for write(2) expects a sequence of bytes (ie.,
[]byte), and not an arbitrary memory address [2].  Therefore, the
'encoding/binary' package [3] is used to encode the integer into a byte
sequence as a varint.

Even though a varint-encoded 64-bit integer takes a maximum of 10
bytes, as defined by binary.MaxVarintLen64, 1 byte is enough to encode
the number 1 as an unsigned 64-bit integer [4].  That's enough to fit
into a byte sequence of length 8 to satisfy what an eventfd(2) file
descriptor expects.  Ultimately, it doesn't matter exactly what value
the receiving end assigns to the number given to write(2), as long as
it's not zero.

[1] https://man7.org/linux/man-pages/man2/eventfd.2.html

[2] https://pkg.go.dev/golang.org/x/sys/unix#Write

[3] https://pkg.go.dev/encoding/binary

[4] https://protobuf.dev/programming-guides/encoding/

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 16, 2023
This is meant to avoid duplicating the code that shows the prompt, and
reads and parses the user's input.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 16, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.

Initially, the prompt will be shown without the image size.  Once the
size has been fetched, the older prompt will be cancelled and a new one
will be shown that includes the size.  While the prompt is getting
updated, the terminal device will be put into non-canonical mode input
and the echoing of input characters will be disabled to retain full
control of the cursor position.  Once the new prompt is in place, the
previous state of the terminal will be restored.  However, anything that
was typed in the interim will be discarded to avoid surprising the user
with invisible input.

Even though this code is only expected to be used to read from the
standard input stream when it's connected to a terminal device, the use
of poll(2) here was tested with FIFOs or named pipes and regular files
as well, in case they might be necessary in future.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 16, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.

To do this, at some point, the terminal device will be put into
non-canonical mode input and the echoing of input characters will be
disabled to retain full control of the cursor position.  Unfortunately,
this will require access to the full termios(3) struct that isn't given
by golang.org/x/term, and, hence, the code needs to be written using the
underlying termios(3) API.

This future code will have enough overlap with the IsTerminal API from
golang.org/x/term that it doesn't make sense to use a separate module
(ie., golang.org/x/term) for it.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 16, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  To do this, at some point, the
terminal device will be put into non-canonical mode input and the
echoing of input characters will be disabled to retain full control of
the cursor position.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 16, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  Initially, the prompt will be
shown without the image size.  If the user responds before the size is
fetched, then the pending 'skopeo inspect' will be cancelled.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 16, 2023
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.  Initially, the prompt will be
shown without the image size.  If the user responds before the size is
fetched, then the pending 'skopeo inspect' will be cancelled.

containers#752
containers#1263
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 16, 2023
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 image download prompt can be
updated to include it.

So, initially:
  $ toolbox create
  Image required to create toolbox container.
  Download registry.fedoraproject.org/fedora-toolbox:39 ( ... MB)? [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]:

The placeholder for the missing image size (ie., ' ... MB') was chosen
to have seven characters, so that it matches the most common sizes.  The
human-readable representation of the image size is capped at four valid
numbers [1].  Unless it's a perfect round number like 1KB or 1.2MB, it
will likely use all four numbers and the decimal point, which is five
characters.  Then two more for the unit, because it's very unlikely that
there will be an image that's less than 1KB in size and will be shown in
bytes with a B.  That makes it seven characters in total.

Updating the image download prompt with the results of 'skopeo inspect'
is vulnerable to races.  At the same time as the terminal's cursor is
being moved to the beginning of the current line to overwrite the
earlier prompt with the new one, the user can keep typing and keep
moving the cursor forward.  This competition over the cursor can lead to
awkward outcomes.

For example, the prompt can overwrite the characters typed in by the
user, leaving characters in the terminal's input buffer waiting for the
user to hit ENTER, even though they are not visible on the screen.
Another example is that hitting BACKSPACE can end up deleting parts of
the prompt, instead of stopping at the edge.

This is solved by putting the terminal device into non-canonical mode
input and disabling the echoing of input characters, while the prompt is
being updated.  This prevents input from moving the terminal's cursor
forward, and from accumulating in the terminal's input buffer even if
it might not be visible.  Any input during this interim period is
discarded and replaced by '...', and a fresh new prompt is shown in the
following line.

In practice, this race shouldn't be too common.  It can only happen if
the user is typing right when the prompt is being updated, which is
unlikely because it's only supposed to be a short 'yes' or 'no' input.

The use of the context.Cause and context.WithCancelCause functions [2]
requires Go >= 1.20.  Bumping the Go version in src/go.mod then requires
a 'go mod tidy'.  Otherwise, it leads to:
  $ meson compile -C builddir --verbose
  ...
  /home/rishi/devel/containers/git/toolbox/src/go-build-wrapper
    /home/rishi/devel/containers/git/toolbox/src
    /home/rishi/devel/containers/git/toolbox/builddir src/toolbox
    0.0.99.4 cc /lib64/ld-linux-x86-64.so.2 false
  go: updates to go.mod needed; to update it:
          go mod tidy
  ninja: build stopped: subcommand failed.

[1] https://pkg.go.dev/github.com/docker/go-units#HumanSize

[2] https://pkg.go.dev/context

containers#752
containers#1263
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. Feature request A request for a new feature 2. Host Realm The issue is related to what happens on the host machine where Toolbox is executed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants