Commit 330a003533
introduced "synchronous" service update and rollback, using progress bars to show
current status for each task.
As part of that change, progress bars were "reversed" when doing a rollback, to
indicate that status was rolled back to a previous state.
Reversing direction is somewhat confusing, as progress bars now return to their
"initial" state to indicate it was "completed"; for an "automatic" rollback, this
may be somewhat clear (progress bars "move to the right", then "roll back" if the
update failed), but when doing a manual rollback, it feels counter-intuitive
(rolling back is the _expected_ outcome).
This patch removes the code to reverse the direction of progress-bars, and makes
progress-bars always move from left ("start") to right ("finished").
Before this patch
----------------------------------------
1. create a service with automatic rollback on failure
$ docker service create --update-failure-action=rollback --name foo --tty --replicas=5 nginx:alpine
9xi1w3mv5sqtyexsuh78qg0cb
overall progress: 5 out of 5 tasks
1/5: running [==================================================>]
2/5: running [==================================================>]
3/5: running [==================================================>]
4/5: running [==================================================>]
5/5: running [==================================================>]
verify: Waiting 2 seconds to verify that tasks are stable...
2. update the service, making it fail after 3 seconds
$ docker service update --entrypoint="/bin/sh -c 'sleep 3; exit 1'" foo
overall progress: rolling back update: 2 out of 5 tasks
1/5: running [==================================================>]
2/5: running [==================================================>]
3/5: starting [============================================> ]
4/5: running [==================================================>]
5/5: running [==================================================>]
3. Once the service starts failing, automatic rollback is started; progress-bars now move in the reverse direction;
overall progress: rolling back update: 3 out of 5 tasks
1/5: ready [===========> ]
2/5: ready [===========> ]
3/5: running [> ]
4/5: running [> ]
5/5: running [> ]
4. When the rollback is completed, the progressbars are at the "start" to indicate they completed;
overall progress: rolling back update: 5 out of 5 tasks
1/5: running [> ]
2/5: running [> ]
3/5: running [> ]
4/5: running [> ]
5/5: running [> ]
rollback: update rolled back due to failure or early termination of task bndiu8a998agr8s6sjlg9tnrw
verify: Service converged
After this patch
----------------------------------------
Progress bars always go from left to right; also in a rollback situation;
After updating to the "faulty" entrypoint, task are deployed:
$ docker service update --entrypoint="/bin/sh -c 'sleep 3; exit 1'" foo
foo
overall progress: 1 out of 5 tasks
1/5:
2/5: running [==================================================>]
3/5: ready [======================================> ]
4/5:
5/5:
Once tasks start failing, rollback is started, and presented the same as a regular
update; progress bars go from left to right;
overall progress: rolling back update: 3 out of 5 tasks
1/5: ready [======================================> ]
2/5: starting [============================================> ]
3/5: running [==================================================>]
4/5: running [==================================================>]
5/5: running [==================================================>]
rollback: update rolled back due to failure or early termination of task c11dxd7ud3d5pq8g45qkb4rjx
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This extends #2929 to Darwin as well as Linux.
Running the example in https://github.com/golang/go/issues/37942
I see lots of:
```
dave@m1 sigurg % uname -ms
Darwin arm64
dave@m1 sigurg % go run main.go
received urgent I/O condition: 2021-05-21 16:03:03.482211 +0100 BST m=+0.014553751
received urgent I/O condition: 2021-05-21 16:03:03.507171 +0100 BST m=+0.039514459
```
Signed-off-by: David Scott <dave@recoil.org>
The kernel memory limit is deprecated in Docker 20.10.0,
and its support was removed in runc v1.0.0-rc94.
So, this warning can be safely removed.
Relevant: b8ca7de823
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
- use var/const blocks when declaring a list of variables
- use const where possible
TestCheckKubernetesConfigurationRaiseAnErrorOnInvalidValue:
- use keys when assigning values
- make sure test is dereferenced in the loop
- use subtests
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Some tests were using domain names that were intended to be "fake", but are
actually registered domain names (such as mycorp.com).
Even though we were not actually making connections to these domains, it's
better to use domains that are designated for testing/examples in RFC2606:
https://tools.ietf.org/html/rfc2606
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
New solution is not hardcoded to amd64 but integrates
with the cross toolchain and support creating arm binaries.
Go has been updated so that ASLR works
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Relates to the deprecation, added in 3c0a167ed5
The docker CLI up until v1.7.0 used the `~/.dockercfg` file to store credentials
after authenticating to a registry (`docker login`). Docker v1.7.0 replaced this
file with a new CLI configuration file, located in `~/.docker/config.json`. When
implementing the new configuration file, the old file (and file-format) was kept
as a fall-back, to assist existing users with migrating to the new file.
Given that the old file format encourages insecure storage of credentials
(credentials are stored unencrypted), and that no version of the CLI since
Docker v1.7.0 has created this file, the file is marked deprecated, and support
for this file will be removed in a future release.
This patch adds a deprecation warning, which is printed if the CLI falls back
to using the deprecated ~/.dockercfg file.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We refactorted `ForwardAllSignals` so it blocks but did not update the
call in `start` to call it in a goroutine.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Commit fff164c22e modified ForwardAllSignals to
take `SIGURG` signals into account, which can be generated by the Go runtime
on Go 1.14 and up as an interrupt to support pre-emptable system calls on Linux.
With the updated code, the signal (`s`) would sometimes be `nil`, causing spurious
(but otherwise harmless) warnings to be printed;
Unsupported signal: <nil>. Discarding.
To debug this issue, I patched v20.10.4 to handle `nil`, and added a debug line
to print the signal in all cases;
```patch
diff --git a/cli/command/container/signals.go b/cli/command/container/signals.go
index 06e4d9eb6..0cb53ef06 100644
--- a/cli/command/container/signals.go
+++ b/cli/command/container/signals.go
@@ -22,8 +22,9 @@ func ForwardAllSignals(ctx context.Context, cli command.Cli, cid string, sigc <-
case <-ctx.Done():
return
}
+ fmt.Fprintf(cli.Err(), "Signal: %v\n", s)
if s == signal.SIGCHLD || s == signal.SIGPIPE {
```
When running a cross-compiled macOS binary with Go 1.13 (`make -f docker.Makefile binary-osx`):
# regular "docker run" (note that the `<nil>` signal only happens "sometimes"):
./build/docker run --rm alpine/git clone https://github.com/docker/getting-started.git
Cloning into 'getting-started'...
Signal: <nil>
# when cancelling with CTRL-C:
./build/docker run --rm alpine/git clone https://github.com/docker/getting-started.git
^CSignal: interrupt
Cloning into 'getting-started'...
error: could not lock config file /git/getting-started/.git/config: No such file or directory
fatal: could not set 'core.repositoryformatversion' to '0'
Signal: <nil>
Signal: <nil>
When running a macOS binary built with Go 1.15 (`DISABLE_WARN_OUTSIDE_CONTAINER=1 make binary`):
# regular "docker run" (note that the `<nil>` signal only happens "sometimes"):
# this is the same as on Go 1.13
./build/docker run --rm alpine/git clone https://github.com/docker/getting-started.git
Cloning into 'getting-started'...
Signal: <nil>
# when cancelling with CTRL-C:
./build/docker run --rm alpine/git clone https://github.com/docker/getting-started.git
Cloning into 'getting-started'...
^CSignal: interrupt
Signal: urgent I/O condition
Signal: urgent I/O condition
fatal: --stdin requires a git repository
fatal: index-pack failed
Signal: <nil>
Signal: <nil>
This patch checks if the channel is closed, and removes the warning (to prevent warnings if new
signals are added that are not in our known list of signals)
We should also consider updating `notfiyAllSignals()`, which currently forwards
_all_ signals (`signal.Notify(sigc)` without passing a list of signals), and
instead pass it "all signals _minus_ the signals we don't want forwarded":
35f023a7c2/cli/command/container/signals.go (L55)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
On Windows, the os/exec.{Command,CommandContext,LookPath} functions
resolve command names that have neither path separators nor file extension
(e.g., "git") by first looking in the current working directory before
looking in the PATH environment variable.
Go maintainers intended to match cmd.exe's historical behavior.
However, this is pretty much never the intended behavior and as an abundance of precaution
this patch prevents that when executing commands.
Example of commands that docker.exe may execute: `git`, `docker-buildx` (or other cli plugin), `docker-credential-wincred`, `docker`.
Note that this was prompted by the [Go 1.15.7 security fixes](https://blog.golang.org/path-security), but unlike in `go.exe`,
the windows path lookups in docker are not in a code path allowing remote code execution, thus there is no security impact on docker.
Signed-off-by: Tibor Vass <tibor@docker.com>
Prior to this change, progressbars would sometimes be hidden, and the function
would return early. In addition, the direction of the progressbars would sometimes
be "incrementing" (similar to "docker service update"), and sometimes be "decrementing"
(to indicate a "rollback" is being performed).
This fix makes sure that we always proceed with the "verifying" step, and now
prints a message _after_ the verifying stage was completed;
$ docker service rollback foo
foo
overall progress: rolling back update: 5 out of 5 tasks
1/5: running [> ]
2/5: starting [===========> ]
3/5: starting [===========> ]
4/5: running [> ]
5/5: running [> ]
verify: Service converged
rollback: rollback completed
$ docker service rollback foo
foo
overall progress: rolling back update: 1 out of 1 tasks
1/1: running [> ]
verify: Service converged
rollback: rollback completed
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Before this change:
--------------------------------------------
$ docker service create --replicas=1 --name foo -p 8080:80 nginx:alpine
t33qvykv8y0zbz266rxynsbo3
overall progress: 1 out of 1 tasks
1/1: running [==================================================>]
verify: Service converged
$ echo $?
0
$ docker service update --replicas=5 foo
foo
overall progress: 5 out of 5 tasks
1/5: running [==================================================>]
2/5: running [==================================================>]
3/5: running [==================================================>]
4/5: running [==================================================>]
5/5: running [==================================================>]
verify: Service converged
$ echo $?
0
$ docker service rollback foo
foo
rollback: manually requested rollback
overall progress: rolling back update: 1 out of 1 tasks
1/1: running [> ]
verify: Service converged
$ echo $?
0
$ docker service rollback foo
foo
service rolled back: rollback completed
$ echo $?
1
After this change:
--------------------------------------------
$ docker service create --replicas=1 --name foo -p 8080:80 nginx:alpine
t33qvykv8y0zbz266rxynsbo3
overall progress: 1 out of 1 tasks
1/1: running [==================================================>]
verify: Service converged
$ echo $?
0
$ docker service update --replicas=5 foo
foo
overall progress: 5 out of 5 tasks
1/5: running [==================================================>]
2/5: running [==================================================>]
3/5: running [==================================================>]
4/5: running [==================================================>]
5/5: running [==================================================>]
verify: Waiting 1 seconds to verify that tasks are stable...
$ echo $?
0
$ docker service rollback foo
foo
rollback: manually requested rollback
overall progress: rolling back update: 1 out of 1 tasks
1/1: running [> ]
verify: Service converged
$ echo $?
0
$ docker service rollback foo
foo
service rolled back: rollback completed
$ echo $?
0
$ docker service ps foo
ID NAME IMAGE NODE DESIRED STATE CURRENT STATE ERROR PORTS
4dt4ms4c5qfb foo.1 nginx:alpine docker-desktop Running Running 2 minutes ago
Remaining issues with reconciliation
--------------------------------------------
Note that both before, and after this change, the command sometimes terminates
early, and does not wait for the service to reconcile; this is most apparent
when rolling back is scaling up (so more tasks are deployed);
$ docker service rollback foo
foo
service rolled back: rollback completed
$ docker service rollback foo
foo
rollback: manually requested rollback
overall progress: rolling back update: 1 out of 5 tasks
1/5: pending [=================================> ]
2/5: running [> ]
3/5: pending [=================================> ]
4/5: pending [=================================> ]
5/5: pending [=================================> ]
service rolled back: rollback completed
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
commit c2626a8270 replaced the use of
github.com/docker/docker/pkg/homedir with Golang's os.UserHomeDir().
This change was partially reverted in 7a279af43d
to account for situations where `$HOME` is not set.
In situations where no configuration file is present in `~/.config/`, the CLI
falls back to looking for the (deprecated) `~/.dockercfg` configuration file,
which was still using `os.UserHomeDir()`, which produces an error/warning if
`$HOME` is not set.
This patch introduces a helper function and a global variable to get the user's
home-directory. The global variable is used to prevent repeatedly looking up
the user's information (which, depending on the setup can be a costly operation).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit f32731f902 fixed a potential panic
when an error was returned while trying to get existing credentials.
However, other code paths currently use the result of `GetDefaultAuthConfig()`
even in an error condition; this resulted in a panic, because a `nil` was
returned.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: 75b288015a...c1f2f97bff
relevant changes:
- pkcs12: document that we use the wrong PEM type
- pkcs12: drop PKCS#12 attributes with unknown OIDs
- ocsp: Improve documentation for ParseResponse and ParseResponseForCert
other changes (not in vendor);
- ssh: improve error message for KeyboardInteractiveChallenge
- ssh: remove slow unnecessary diffie-hellman-group-exchange primality check
- ssh/terminal: replace with a golang.org/x/term wrapper
- Deprecates ssh/terminal in favor of golang.org/x/term
- ssh/terminal: add support for zos
- ssh/terminal: bump x/term dependency to fix js/nacl
- nacl/auth: use Size instead of KeySize for Sum output
- sha3: remove go:nocheckptr annotation
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Will display when user types `docker help` or `docker --help`, but not for `docker run --help`.
Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
This hack was added in an attempt to continue supporting the experimental
(non-buildkit) `--platform` option, by dynamically updating the API version
required if buildkit isn't enabled.
This hack didn't work, however, because at the moment the override is
added, the command is not yet attached to the "root" (`docker`) command,
and because of that, the command itself is the `root` command;
`cmd.Root()` returned the `build` command.
As a result, validation steps defined as `PersistentPreRunE` on the root
command were not executed, causing invalid flags/options to not producing
an error.
Attempts to use an alternative approach (for example, cobra supports both
a `PersistentPreRun` and `PersistentPreRunE`) did not work either, because
`PersistentPreRunE` takes precedence over `PersistentPreRun`, and only one
will be executed.
Now that `--platform` should be supported for other cases than just for
experimental (LCOW), let's remove the 'experimental' check, and just assume
it's supported for API v1.32 and up.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn
Sebastiaan van Stijn
Silvin Lubecki
David Scott
Akihiro Suda
Maximillian Fan Xavier
Tonis Tiigi
Brian Goff
Chris Crone
Tibor Vass
Jim Lin
Alexey Igrychev
Tibor Vass
Djordje Lukic
Guillaume Tardif