This moves all the terminal writing to a goroutine that updates the
terminal periodically.
In our MITM copier we just use an atomic to add to the total number of
bytes read/written, the goroutine reads the total and updates the
terminal as needed.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit eb392ff4ce)
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Only show progress updates after a time threshold has elapsed in order
to reduce the number of writes to the terminal.
This improves readability of the progress.
Also moves cursor show/hide into the progress printer to reduce chances
if messing up the user's terminal in case of cancellation.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit 90b7bc36d4)
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This just makes it easier to reason about what is happening.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit efd011b793)
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
- Instead of rewriting the entire line every time only clear and write
the parts that changed.
- Hide the cursor while writing progress
Both these things make the progress updates significantly easier to
read.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit ccae6e9299)
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This fixes a case where a non-tty will have control characters + the log
line for every single read operation.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit f27927d934)
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
The error returned from "os/exec".Command when attempting to execute a
directory has been changed from syscall.EACCESS to syscall.EISDIR on
Go 1.20. 2b8f214094
Consequently, any runc runtime built against Go 1.20 will return an
error containing 'is a directory' and not 'permission denied'. Update
the string matching so the CLI exits with status code 126 on 'is a
directory' errors (EISDIR) in addition to 'permission denied' (EACCESS).
Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit 9b5ceb52b0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Synchronize append on the `removed` slice with mutex because
containerRemoveFunc is called in parallel for each removed container by
`container rm` cli command.
Also reduced the shared access area by separating the scopes of test
cases.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This comment was added in 7929888214
when this code was still in the Moby repository. That comment doesn't appear
to apply to the CLI's usage of this struct though, as nothing in the CLI
sets this field (or uses it), so this should be safe to remove.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Make sure that the container has multiple port-mappings to illustrate
that only the given port is matched.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- use strings.Cut
- don't use nat.NewPort as we don't accept port ranges
- use an early return if there's no results
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This internalizes constructing the Client(), which allows us to provide
fallbacks when trying to determin the current API version.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/container/opts.go:928:2: assigned to src, but reassigned without using the value (wastedassign)
src := ""
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Having the intermediate variable made it difficult to see if it was
possibly mutated and/or something special done with it, so just use
the cli's accessors to get its Err().
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Older versions of Go do not format these comments, so we can already
reformat them ahead of time to prevent gofmt linting failing once
we update to Go 1.19 or up.
Result of:
gofmt -s -w $(find . -type f -name '*.go' | grep -v "/vendor/")
With some manual adjusting.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Cobra allows for aliases to be defined for a command, but only allows these
to be defined at the same level (for example, `docker image ls` as alias for
`docker image list`). Our CLI has some commands that are available both as a
top-level shorthand as well as `docker <object> <verb>` subcommands. For example,
`docker ps` is a shorthand for `docker container ps` / `docker container ls`.
This patch introduces a custom "aliases" annotation that can be used to print
all available aliases for a command. While this requires these aliases to be
defined manually, in practice the list of aliases rarely changes, so maintenance
should be minimal.
As a convention, we could consider the first command in this list to be the
canonical command, so that we can use this information to add redirects in
our documentation in future.
Before this patch:
docker images --help
Usage: docker images [OPTIONS] [REPOSITORY[:TAG]]
List images
Options:
-a, --all Show all images (default hides intermediate images)
...
With this patch:
docker images --help
Usage: docker images [OPTIONS] [REPOSITORY[:TAG]]
List images
Aliases:
docker image ls, docker image list, docker images
Options:
-a, --all Show all images (default hides intermediate images)
...
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this change, specifying the `--pull` flag without a value, could
result in the flag after it, or the positional argument to be used as
value.
This patch makes sure that the value is an expected value;
docker create --pull --rm hello-world
docker: invalid pull option: '--rm': must be one of "always", "missing" or "never".
docker run --pull --rm hello-world
docker: invalid pull option: '--rm': must be one of "always", "missing" or "never".
docker run --pull hello-world
docker: invalid pull option: 'hello-world': must be one of "always", "missing" or "never".
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- do an early check if a custom format is specified either through the
command-line, or through the cli's configuration, before adjusting
the options (to add "size" if needed).
- also removes a redundant `options.Size = opts.size` line, as this value is
already copied at the start of buildContainerListOptions()
- Update NewContainerFormat to use "table" format as a default if no format
was given.
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Phong Tran <tran.pho@northeastern.edu>
This makes the containers have an expected console size not only for
`run` but also for `create`. Also remove the comment, as this is no
longer ignored on Linux daemon since e994efcf64c133de799f16f5cd6feb1fc41fade4
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Wording and documentation still need to be updated, but will do
so in a follow-up.
Also removing the default "10 seconds" from the timeout flags, as
this default is not actually used, and may not match the actual
default (which is defined on the daemon side).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Formatting stats runs in a loop to refresh the stats for each container. This
patch makes some small performance improvments by reducing the use of Sprintf
in favor of concatenating strings, and using strconv directly where possible.
Benchmark can be run with:
GO111MODULE=off go test -test.v -test.bench '^BenchmarkStatsFormat' -test.run '^$' ./cli/command/container/
Before/after:
BenchmarkStatsFormatOld-8 2655 428064 ns/op 62432 B/op 5600 allocs/op
BenchmarkStatsFormat-8 3338 335822 ns/op 52832 B/op 4700 allocs/op
Average of 5 runs;
benchstat old.txt new.txt
name old time/op new time/op delta
StatsFormat-8 432µs ± 1% 344µs ± 5% -20.42% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
StatsFormat-8 62.4kB ± 0% 52.8kB ± 0% -15.38% (p=0.000 n=5+4)
name old allocs/op new allocs/op delta
StatsFormat-8 5.60k ± 0% 4.70k ± 0% -16.07% (p=0.008 n=5+5)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
cli/command/container/hijack.go:188:1⚠️ nolint directive did not match any issue (nolint)
cli/command/image/trust.go:346:1⚠️ nolint directive did not match any issue (nolint)
cli/command/manifest/push.go:211:1⚠️ nolint directive did not match any issue (nolint)
cli/command/trust/signer_remove.go:79:1⚠️ nolint directive did not match any issue (nolint)
internal/pkg/containerized/snapshot.go:95:1⚠️ nolint directive did not match any issue (nolint)
internal/pkg/containerized/snapshot.go:138:1⚠️ nolint directive did not match any issue (nolint)
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Prevent completion on "create" subcommands to prevent them
from completing with local filenames
- Add completion for "docker image save"
- Add completion for "docker image tag"
- Disable completion for "docker login"
- Exclude "paused" containers for "docker container attach" and
"docker container exec"
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test is very flaky, the retry loop runs for 550ms and some more, 750ms is
cleary not enough for everything to set and for the cli to return the tty resize
error. A sleep of 1.5 seconds in this test should be enough for the retry loop to
finish.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
I some cases, for example if there is a heavy load, the initialization of the TTY size
would fail. This change makes the cli retry 10 times instead of 5 and we wait
incrementally from 10ms to 100ms
Relates to #3554
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Brian Goff
Laura Brehm
Cory Snider
Akihiro Suda
CrazyMax
Paweł Gronowski
Sebastiaan van Stijn
Sebastiaan van Stijn
Phong Tran
Nicolas De Loof
teivah
Djordje Lukic