Docker CLI - June [Rebased] #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The test didn't do anything useful... - Despite its name it used newCreateCommand() instead of newConnectCommand() with create flags/options instead of connect. - There was no fake networkCreateFunc(), so the result of the 'connect' wasn't checked. - The fake networkConnectFunc() was never called, so didn't spot the problem. Signed-off-by: Rob Murray <[email protected]>
Support for connecting more than one network using the container run command was added in v25.0 for API > 1.44 - describe that in the docs. Signed-off-by: Rob Murray <[email protected]>
no changes to vendored code full diff: docker/docker-credential-helpers@v0.8.1...v0.8.2 Signed-off-by: Sebastiaan van Stijn <[email protected]>
- 0.14.1 release notes: https://github.com/docker/buildx/releases/tag/v0.14.1 - 0.14.0 release notes: https://github.com/docker/buildx/releases/tag/v0.14.0 - 0.13.1 release notes: https://github.com/docker/buildx/releases/tag/v0.13.1 - 0.13.0 release notes: https://github.com/docker/buildx/releases/tag/v0.13.0 Signed-off-by: Sebastiaan van Stijn <[email protected]>
release notes: https://github.com/docker/compose/releases/tag/v2.27.1 full diff: docker/compose@v2.24.3...v2.27.1 Signed-off-by: Sebastiaan van Stijn <[email protected]>
Update to the current version of Alpine, which is also the default for the golang:alpine image Signed-off-by: Sebastiaan van Stijn <[email protected]>
The output showed the Alpine version that was used for the example, which can get outdated and distracts from the example steps. Use --quiet to reduce the output, and to reduce maintenance (i.e., no need to keep the output updated with current versions). Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Rob Murray <[email protected]>
Signed-off-by: Rob Murray <[email protected]>
Signed-off-by: Rob Murray <[email protected]>
Signed-off-by: Rob Murray <[email protected]>
docs: use --quiet in example to simplify output
The github.com/containerd/containerd/platforms package was moved to a separate module in preparation of the containerd v2.0 release. Switch to the new module, which means we also remove containerd as a direct dependency. Signed-off-by: Sebastiaan van Stijn <[email protected]>
vendor: github.com/docker/docker-credential-helpers v0.8.2
Dockerfile: update buildx to v0.14.1, compose v2.27.1
migrate to new github.com/containerd/platforms package
Dockerfile: update ALPINE_VERSION to 3.20
Signed-off-by: Albin Kerouanton <[email protected]>
docs: deprecate daemon's api-cors-header flag
Document CLI support for per interface sysctls
Signed-off-by: Sebastiaan van Stijn <[email protected]>
The CLI does not currently expose options to add custom metadata to contexts, but contexts support them. - update test-utilities to allow setting custom metadata - update the inspect test to verify that custom metadata is included when inspecting a context. - update the import/export tests to verify that custom metadata is preserved. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
cli/command/context: improve tests and add coverage for custom metadata in contexts
Docker Desktop currently ships with the "cloud integration" wrapper,
which outputs an additional ContextType field in the JSON output.
While this field is non-standard, it made its way into Visual Studio's
Docker integration, which uses this to exclude "aci" and "eci" context
types that are not supported by Visual Studio.
This patch;
- conditionally adds a ContextType field to the JSON output
- but ONLY when using the default "{{json .}}" or "json" formats
(which are the formats used by Visual Studio)
- if the context is a "aci" or "eci" context, that type is
preserved, otherwise the default "moby" type is used.
Signed-off-by: Sebastiaan van Stijn <[email protected]>
context list: temporarily add ContextType to JSON output
- Fix compatibility with go1.22 - fileinfo: internally fix FileBasicInfo memory alignment (fixes compatibility with go1.22) - Switch from syscall to golang.org/x/sys/windows - Remove golang.org/x/mod as dependency - Remove golang.org/x/tools as dependency full diff: microsoft/go-winio@v0.6.1...v0.6.2 Signed-off-by: Sebastiaan van Stijn <[email protected]>
vendor: github.com/Microsoft/go-winio v0.6.2 (for go1.22 compatibility)
full diff: opencontainers/image-spec@v1.1.0-rc5...v1.1.0 Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: microsoft/hcsshim@v0.11.4...v0.11.5 Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Alano Terblanche <[email protected]>
fix: force cli to exit after third sigint/sigterm
build: allow BuildKit to be used on Windows daemons that advertise it
no diff; same commit, but tagged. full diff: moby/moby@018d93d...v27.0.1-rc.1 Signed-off-by: Sebastiaan van Stijn <[email protected]>
vendor: github.com/docker/docker v27.0.1-rc.1
release notes: https://github.com/docker/compose/releases/tag/v2.28.0 full diff: docker/compose@v2.27.1...v2.28.0 Signed-off-by: Sebastiaan van Stijn <[email protected]>
Add a mention of the feature being disabled by default, and the DOCKERD_DEPRECATED_GRAPHDRIVER_PLUGINS env-var. Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: moby/moby@81b2027...ff1e2c0 Signed-off-by: Paweł Gronowski <[email protected]>
docs/deprecated: update status for graphdriver-plugins
vendor: github.com/docker/docker v27.0-dev (ff1e2c0de72a, master)
Dockerfile: update compose to v2.28.0
There's some consumers of the config package that don't need any of the other parts of the code, but because of the pkg/homedir were now forced to also depend on docker/docker. This patch introduces a local copy of the function to prevent this. Signed-off-by: Sebastiaan van Stijn <[email protected]>
cli/config: replace pkg/homedir dependency with local copy
Signed-off-by: Sebastiaan van Stijn <[email protected]>
cli: make initializing the global meter- and tracing providers optional
full diff: golangci/golangci-lint@v1.59.0...v1.59.1 Signed-off-by: Sebastiaan van Stijn <[email protected]>
27.0 is out - update the latest version used for e2e and drop the 25.0 Signed-off-by: Paweł Gronowski <[email protected]>
update golangci-lint to v1.59.1
Signed-off-by: Carston Schilds <[email protected]>
re-introduced support for port numbers in docker registry URL
Signed-off-by: Sebastiaan van Stijn <[email protected]>
cli/config/credentials: ConvertToHostname: handle IP-addresses
gha/e2e: Update latest version to 27.0
no change in vendored files, just changing a tag full diff: moby/moby@ff1e2c0...v27.0.1 Signed-off-by: Paweł Gronowski <[email protected]> (cherry picked from commit bf1a701) Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: moby/moby@v27.0.1...861fde8 Signed-off-by: Paweł Gronowski <[email protected]> (cherry picked from commit 8945848) Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: moby/moby@861fde8...e953d76 Signed-off-by: Paweł Gronowski <[email protected]> (cherry picked from commit c97e809) Signed-off-by: Sebastiaan van Stijn <[email protected]>
no diff, as it's the same commit tagged: moby/moby@e953d76...v27.0.2 Signed-off-by: Sebastiaan van Stijn <[email protected]>
vendor: github.com/docker/docker v27.0.2
TheHamsterBot
pushed a commit
that referenced
this pull request
Aug 4, 2024
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 "#1" 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 docker#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 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)
All good