Docker Cli - July Branch #2
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
full diff: moby/moby@v27.0.2...v27.0.3 Signed-off-by: Sebastiaan van Stijn <[email protected]>
- Add a case-folding version of Natural sort order This can be used to perform case-insensitive comparisons and sorting. It's been placed in a separate sub-package because it requires the Unicode tables in the standard library, which can add significantly to binary size. full diff: fvbommel/sortorder@v1.0.2...v1.1.0 Signed-off-by: Sebastiaan van Stijn <[email protected]>
This code was updated in 7b9580d, which removed support for using kubernetes as orchestrator, but in doing so made this `sort.Slice` (probably) not do what it was expected to do ':) index 412cc2e5ee86..861ae1be2fb9 100644 @@ -75,8 +54,7 @@ func format(dockerCli command.Cli, opts options.List, orchestrator command.Orche } sort.Slice(stacks, func(i, j int) bool { return sortorder.NaturalLess(stacks[i].Name, stacks[j].Name) || - !sortorder.NaturalLess(stacks[j].Name, stacks[i].Name) && - sortorder.NaturalLess(stacks[j].Namespace, stacks[i].Namespace) + !sortorder.NaturalLess(stacks[j].Name, stacks[i].Name) }) return formatter.StackWrite(stackCtx, stacks) } The extra condition was added in 84241cc to support multiple namespaces. This patch removes it, bringing it back to the state it was before that commit. Signed-off-by: Sebastiaan van Stijn <[email protected]>
vendor: github.com/docker/docker v27.0.3
cli/command/stack: fix faulty sort for sorting stacks
vendor: github.com/fvbommel/sortorder v1.1.0
Signed-off-by: Alano Terblanche <[email protected]>
Signed-off-by: Alano Terblanche <[email protected]>
Move common flag descriptions to the buildx build reference, and make that page the canonical page in docs. Also rewrite some content in image_build to make clear that this page is only for the legacy builder. Signed-off-by: David Karlsson <[email protected]>
- https://github.com/golang/go/issues?q=milestone%3AGo1.21.12+label%3ACherryPickApproved - full diff: golang/go@go1.21.11...go1.21.12 These minor releases include 1 security fixes following the security policy: net/http: denial of service due to improper 100-continue handling The net/http HTTP/1.1 client mishandled the case where a server responds to a request with an "Expect: 100-continue" header with a non-informational (200 or higher) status. This mishandling could leave a client connection in an invalid state, where the next request sent on the connection will fail. An attacker sending a request to a net/http/httputil.ReverseProxy proxy can exploit this mishandling to cause a denial of service by sending "Expect: 100-continue" requests which elicit a non-informational response from the backend. Each such request leaves the proxy with an invalid connection, and causes one subsequent request using that connection to fail. Thanks to Geoff Franks for reporting this issue. This is CVE-2024-24791 and Go issue https://go.dev/issue/67555. View the release notes for more information: https://go.dev/doc/devel/release#go1.21.12 **- Description for the changelog** ```markdown changelog Update Go runtime to 1.21.12 ``` Signed-off-by: Paweł Gronowski <[email protected]>
update to go1.21.12
Signed-off-by: Alano Terblanche <[email protected]>
fix: ctx cancellation on login prompt
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
assorted minor changes in preparation of updating docker/docker dependency
Pass the appropriate API-client where possible instead of all of DockerCLI, and some cleaning up. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Dan Wallis <[email protected]>
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 #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]>
Introduce a (non-exported) ipamOptions that collects all options for creating a network.IPAM, so that this utility is more atomic (potentially even could be moved to a separate package and exported). Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
feat: force lf line endings by default
docs: make buildx build the canonical reference doc
full diff: klauspost/compress@v1.17.4...v1.17.9 Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: golang/sys@v0.19.0...v0.21.0 Signed-off-by: Sebastiaan van Stijn <[email protected]>
Add OomScoreAdj to "docker service create" and "docker compose"
cli/command/container: remove reportError, and put StatusError to use
cli/config/credentials: move warning to fileStore
vendor: github.com/containerd/containerd v1.7.20
use current LTS versions of ubuntu where suitable, remove uses of ubuntu:23.10 (which reache EOL), and and update some other examples to use more current versions. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Looks like this test was failing due to bad syntax on the `while` loop, which caused it to die after 1 second. If the test took a bit longer, the process would be dead before the following assertions run, causing the test to fail/be flaky. Signed-off-by: Laura Brehm <[email protected]>
Fix flaky `TestCloseRunningCommand` test
Follow up to cc68c66 (there were more tests with incorrect syntax). Signed-off-by: Laura Brehm <[email protected]>
…tests tests: fix other flaky `connhelper` tests
docs: refresh image versions in examples
In 3f0d90a 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 <[email protected]>
attach: don't return context cancelled error
`megacheck` has been deprecated/split into `gosimple`, `staticcheck`, and `unused`, which we're already using. https://github.com/golangci/golangci-lint/blob/d2b439faa53bc7d0645049c3e23fc858736b8cc4/pkg/lint/lintersdb/validator_test.go#L227 `vet` is now `govet`. https://github.com/golangci/golangci-lint/blame/d2b439faa53bc7d0645049c3e23fc858736b8cc4/pkg/lint/lintersdb/validator_test.go#L228 For more context, see: golangci/golangci-lint#4562 Signed-off-by: Laura Brehm <[email protected]>
lint: replace deprecated linter names
full diff: golang/sys@v0.21.0...v0.22.0 Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: moby/sys@signal/v0.7.0...signal/v0.7.1 Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: moby/sys@symlink/v0.2.0...symlink/v0.3.0 Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: moby/sys@sequential/v0.5.0...sequential/v0.6.0 Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: moby/moby@aae0440...2b1097f The userns package in libcontainer was integrated into the moby/sys/user module at commit 3778ae603c706494fd1e2c2faf83b406e38d687d. The userns package is used in many places, and currently either depends on runc/libcontainer, or on containerd, both of which have a complex dependency tree. This patch is part of a series of patches to unify the implementations, and to migrate toward that implementation to simplify the dependency tree. [3778ae603c706494fd1e2c2faf83b406e38d687d]: opencontainers/runc@3778ae6 Signed-off-by: Sebastiaan van Stijn <[email protected]>
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 <[email protected]>
…t-code attach: wait for exit code from `ContainerWait`
vendor: docker/docker 2b1097f08088 (removes containerd dependency)
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 991b130 - 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 <[email protected]>
tests/run: fix flaky `RunAttachTermination` test
Signed-off-by: Sebastiaan van Stijn <[email protected]>
…tp v1.21.0 commit 89db01e added these tracing modules as dependency, but did not require the otlptracehttp module. This module was added later through f0a29af as indirect dependency for docker/docker. The otlptracehttp and otlptracegrpc modules have no dependency between each-other, but similar to their otlpmetric cousins, are preferred to be on the same version. This patch aligns their versions. No changes in vendored code; full diff: open-telemetry/opentelemetry-go@exporters/otlp/otlptrace/otlptracehttp/v1.19.0...exporters/otlp/otlptrace/otlptracehttp/v1.21.0 Signed-off-by: Sebastiaan van Stijn <[email protected]>
vendor: go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.21.0
gha: set permissions to read-only by default
eclipse-manish
approved these changes
Aug 4, 2024
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)