Currently the cp will tar from the same directory it will untar into
simultaneously. There is a race between reading the file and truncating
the file for write, however, the race will not show up with a large
enough buffer on the tar side if buffered before the copy begins.
Also removes the unnecessary deferred removal, the removal is handled by
cleanup and respects the no cleanup env.
Signed-off-by: Derek McGowan <derek@mcg.dev>
(cherry picked from commit 8c0cb30515)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Just a minor cleanup; use archive.Tar as we're not using other
options here.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a8f83d5d99)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This patch fixes the context cancellation
behaviour for the `runContainer` function,
specifically the `createContainer` function
introduced in this commit 991b1303da.
It delays stripping the `cancel` from the context
passed into the `runContainer` function so that
the `createContainer` function can be cancelled
gracefully by a SIGTERM/SIGINT.
This is especially true when the requested image
does not exist and `docker run` needs to `pull`
the image before creating the container.
Although this patch does gracefully cancel
the `runContainer` function it does not address
the root cause. Some functions in the call path
are not context aware, such as `pullImage`.
Future work would still be necessary to ensure
a consistent behaviour in the CLI.
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
(cherry picked from commit 30a73ff19c)
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
Instead of clearing the whole screen and then writing the new stats,
we now write the new stats on top of the old text, and then clear
the remaining text.
This is a more efficient way to update the stats, as it avoids the
flickering that happens when the screen is cleared and rewritten.
Signed-off-by: Giedrius Jonikas <giedriusj1@gmail.com>
(cherry picked from commit cb2f95ceee)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
make shell
make -C ./internal/gocompat/
GO111MODULE=on go test -v
# github.com/docker/cli/cli/command/container
../../cli/command/container/completion.go:37:28: implicit function instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/command/container/completion.go:82:25: implicit function instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
../../cli/command/container/completion.go:92:27: implicit function instantiation requires go1.18 or later (-lang was set to go1.16; check go.mod)
FAIL gocompat [build failed]
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit d1d5353269)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
commit 4a7b04d412 configured golangci-lint
to use go1.23 semantics, which enabled the copyloopvar linter.
go1.22 now creates a copy of variables when assigned in a loop; make sure we
don't have files that may downgrade semantics to go1.21 in case that also means
disabling that feature; https://go.dev/ref/spec#Go_1.22
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 7c80e4f938)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This change reduces the flickering of the terminal when
running `docker stats` by buffering the formatted stats
text and printing it in one write.
Should also consume less CPU as we now only have to issue
a single syscall to write the stats text to the terminal.
Signed-off-by: Giedrius Jonikas <giedriusj1@gmail.com>
(cherry picked from commit 0b16070ae6)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Prevent some tests from failing when running from a pre-compiled
testbinary, and discard output to make the output less noisy.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 3b38dc67be)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
With this patch, completion is provided for `--platform` flags:
docker run --platform<TAB>
linux linux/amd64 linux/arm/v5 linux/arm/v7 linux/arm64/v8 linux/riscv64 wasip1 windows
linux/386 linux/arm linux/arm/v6 linux/arm64 linux/ppc64le linux/s390x wasip1/wasm windows/amd64
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 8c7f713db6)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We used a hard-coded list of capabilities that we copied from containerd,
but the new "capability" package allows use to have a maintained list
of capabilities.
There's likely still some improvements to be made;
First of all, the capability package could provide a function to get the list
of strings.
On the completion-side, we need to consider what format is most convenient;
currently we use the canonical name (uppercase and "CAP_" prefix), however,
tab-completion is case-sensitive by default, so requires the user to type
uppercase letters to filter the list of options.
Bash completion provides a `completion-ignore-case on` option to make completion
case-insensitive (https://askubuntu.com/a/87066), but it looks to be a global
option; the current cobra.CompletionOptions also don't provide this as an option
to be used in the generated completion-script.
Fish completion has `smartcase` (by default?) which matches any case if
all of the input is lowercase.
Zsh does not have a dedicated option, but allows setting matching-rules
(see https://superuser.com/a/1092328).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 462e08219d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Move the code for parsing key-value files, such as used for
env-files and label-files to a separate package. This allows
other projects (such as compose) to use the same parsing
logic, but provide custom lookup functions for their situation
(which is slightly different).
The new package provides utilities for parsing key-value files
for either a file or an io.Reader. Most tests for EnvFile were
now testing functionality that's already tested in the new package,
so were (re)moved.
Co-authored-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9ecfe4f5a7)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
This test was just incorrect (and testing incorrect
behavior): it was checking that `docker run` exited with a `context
canceled` error after signalling the CLI/cancelling the command's
context, but this was incorrect (and was fixed in
991b1303da - which was when this test
started failing).
However, since this test assertion was happening inside of a goroutine,
it would sometimes pass if this assertion didn't get to run before the
test suite terminated. It was flaky because sometimes this assertion
inside the goroutine did get to execute, but after the test finished
execution, which is a big no-no.
As an aside, assertions inside goroutines are generally bad, and `govet`
even has a linter for this (but it only catches `t.Fatal` and `t.FailNow`
calls and not `assert.Xx`.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
(cherry picked from commit eac83574c1)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Such as with `docker run`, if a user CTRL-Cs while attached to a
container, we should forward the signal and wait for the exit from
`ContainerWait`, instead of just returning.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
(cherry picked from commit 7b46bfc5ac)
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
In 3f0d90a2a9 we introduced a global
signal handler and made sure all the contexts passed into command
execution get (appropriately) cancelled when we get a SIGINT.
Due to that change, and how we use this context during `docker attach`,
we started to return the context cancelation error when a user signals
the running `docker attach`.
Since this is the intended behavior, we shouldn't return an error, so
this commit adds checks to ignore this specific error in this case.
Also adds a regression test.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
(cherry picked from commit 66aa0f672c)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This makes a quick pass through our tests;
Discard output/err
----------------------------------------------
Many tests were testing for error-conditions, but didn't discard output.
This produced a lot of noise when running the tests, and made it hard
to discover if there were actual failures, or if the output was expected.
For example:
=== RUN TestConfigCreateErrors
Error: "create" requires exactly 2 arguments.
See 'create --help'.
Usage: create [OPTIONS] CONFIG file|- [flags]
Create a config from a file or STDIN
Error: "create" requires exactly 2 arguments.
See 'create --help'.
Usage: create [OPTIONS] CONFIG file|- [flags]
Create a config from a file or STDIN
Error: error creating config
--- PASS: TestConfigCreateErrors (0.00s)
And after discarding output:
=== RUN TestConfigCreateErrors
--- PASS: TestConfigCreateErrors (0.00s)
Use sub-tests where possible
----------------------------------------------
Some tests were already set-up to use test-tables, and even had a usable
name (or in some cases "error" to check for). Change them to actual sub-
tests. Same test as above, but now with sub-tests and output discarded:
=== RUN TestConfigCreateErrors
=== RUN TestConfigCreateErrors/requires_exactly_2_arguments
=== RUN TestConfigCreateErrors/requires_exactly_2_arguments#01
=== RUN TestConfigCreateErrors/error_creating_config
--- PASS: TestConfigCreateErrors (0.00s)
--- PASS: TestConfigCreateErrors/requires_exactly_2_arguments (0.00s)
--- PASS: TestConfigCreateErrors/requires_exactly_2_arguments#01 (0.00s)
--- PASS: TestConfigCreateErrors/error_creating_config (0.00s)
PASS
It's not perfect in all cases (in the above, there's duplicate "expected"
errors, but Go conveniently adds "#01" for the duplicate). There's probably
also various tests I missed that could still use the same changes applied;
we can improve these in follow-ups.
Set cmd.Args to prevent test-failures
----------------------------------------------
When running tests from my IDE, it compiles the tests before running,
then executes the compiled binary to run the tests. Cobra doesn't like
that, because in that situation `os.Args` is taken as argument for the
command that's executed. The command that's tested now sees the test-
flags as arguments (`-test.v -test.run ..`), which causes various tests
to fail ("Command XYZ does not accept arguments").
# compile the tests:
go test -c -o foo.test
# execute the test:
./foo.test -test.v -test.run TestFoo
=== RUN TestFoo
Error: "foo" accepts no arguments.
The Cobra maintainers ran into the same situation, and for their own
use have added a special case to ignore `os.Args` in these cases;
https://github.com/spf13/cobra/blob/v1.8.1/command.go#L1078-L1083
args := c.args
// Workaround FAIL with "go test -v" or "cobra.test -test.v", see #155
if c.args == nil && filepath.Base(os.Args[0]) != "cobra.test" {
args = os.Args[1:]
}
Unfortunately, that exception is too specific (only checks for `cobra.test`),
so doesn't automatically fix the issue for other test-binaries. They did
provide a `cmd.SetArgs()` utility for this purpose
https://github.com/spf13/cobra/blob/v1.8.1/command.go#L276-L280
// SetArgs sets arguments for the command. It is set to os.Args[1:] by default, if desired, can be overridden
// particularly useful when testing.
func (c *Command) SetArgs(a []string) {
c.args = a
}
And the fix is to explicitly set the command's args to an empty slice to
prevent Cobra from falling back to using `os.Args[1:]` as arguments.
cmd := newSomeThingCommand()
cmd.SetArgs([]string{})
Some tests already take this issue into account, and I updated some tests
for this, but there's likely many other ones that can use the same treatment.
Perhaps the Cobra maintainers would accept a contribution to make their
condition less specific and to look for binaries ending with a `.test`
suffix (which is what compiled binaries usually are named as).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit ab230240ad)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
With this patch:
docker run --volumes-from amazing_nobel
amazing_cannon boring_wozniak determined_banzai
elegant_solomon reverent_booth amazing_nobel
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit d6f78cdbb1)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
With this patch:
docker run --restart <TAB>
always no on-failure unless-stopped
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 7fe7223c2c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
"docker run" and "docker create" are mostly identical, so we can copy
the same completion functions,
We could possibly create a utility for this (similar to `addFlags()` which
configures both commands with the flags they share). I considered combining
his with `addFlags()`, but that utility is also used in various tests, in
which we don't need this feature, so keeping that for a future exercise.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 162d9748b9)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It's an alias for cobra.FixedCompletions but takes a variadic list
of strings, so that it's not needed to construct an array for this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 5e7bcbeac6)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
EnvVarNames offers completion for environment-variable names. This
completion can be used for "--env" and "--build-arg" flags, which
allow obtaining the value of the given environment-variable if present
in the local environment, so we only should complete the names of the
environment variables, and not their value. This also prevents the
completion script from printing values of environment variables
containing sensitive values.
For example;
export MY_VAR=hello
docker run --rm --env MY_VAR alpine printenv MY_VAR
hello
Before this patch:
docker run --env GO
GO111MODULE=auto GOLANG_VERSION=1.21.12 GOPATH=/go GOTOOLCHAIN=local
With this patch:
docker run --env GO<tab>
GO111MODULE GOLANG_VERSION GOPATH GOTOOLCHAIN
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit e3427f341b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is just a convenience function to allow defining completion to
use the default (complete with filenames and directories).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9207ff1046)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This wraps the cli stderr stream the same way as stdin and stdout, which
extends the stream with TTY-related methods.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
