Explicitly set the gRPC connection params to take the timeout into
account to workaround the containerd v2 client not passing down the
stack.
containerd v2 replaced usages of deprecated gRPC functions but didn't
pass the timeout to the actual dial connection options.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The "local" client does not use containerd, but implements the same interface,
many args are not used though, so remove these to make it more clear what's
actually in use.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The "local" client does not use containerd, but implements the same interface,
many args are not used though, so remove these to make it more clear what's
actually in use.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also remove an intermediate var, and remove a "WithError" in favor of
adding the error field to the "WithFields".
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Noticed this log in CI on Windows, which wasn't clear if it was an error
in Windows or in Docker;
2025-02-25T03:21:35.9273942Z [Error] Handler for POST /v1.48/containers/1713bc845f9bde79aa0017c16613fbfc8810b3272b31dbb2535d3fb1a3550f9c/pause returned error: cannot pause container 1713bc845f9bde79aa0017c16613fbfc8810b3272b31dbb2535d3fb1a3550f9c: Unimplemented: not implemented
Looks like it's a feature that's not implemented when using process-isolation,
so updating the error-message to make it more identifiable as an error
produced by us.
I kept the type to be a "not implemented", which will be converted to a
501 HTTP status (so still logged as error); alternatively, we could make
this a "invalid parameter".
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The default message size is set unconditionally in containerd's client,
so unlike Dial-options, there's no risk of implicitly dropping these
options.
TThis patch removes the options, as they were the same as the default
already set in containerd itself.
https://github.com/containerd/containerd/blob/v1.7.22/client.go#L133-L138
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
pkg/idtools/idtools_unix_test.go:188:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The config.logLevel field, when set, is used to set the `--log-level`
flag when starting the managed `containerd` binary. This flag is the
equivalent to setting the `Config.Debug.Level` field, as can be seen
in the [`md/containerd/command.setLogLevel()`][1] function.
As we're already producing a generated containerd configuration file,
and this file already includes `Debug` options, we might as well include
the option in that file, instead of using the `--log-level` flag.
For entertainment of whoever reads this commit-message, it's worth noting
that previously we were writing this option to the config-file, and
yours truly removed that part in b6b0b0a05f,
but to my defence, we were _also_ setting the `--log-level` flag at the
time :)
[1]: https://github.com/containerd/containerd/blob/v1.7.20/cmd/containerd/command/main.go#L348-L357
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit a0009345f5 updated the default
MaxRecvMsgSize and MaxSendMsgSize for Linux, but did not modify the
defaults for Windows. Those options should not be platform-specific,
which means that the only difference between the Linux and Windows
config are the addresses for GRPC and Debug (Windows defaulting
to a named pipe, whereas Linux sockets within exec-root).
This patch
- implements functions to return the default addresses for each platform
- moves the defaults into `supervisor.Start()`
- removes the now redundant `remote.setDefaults()` method
It's worth noting that prior to this path, `remove.setDefaults()` would
be applied _after_ any (custom) `DaemonOpt` was applied. However, none of
the existing `DaemonOpt` options currently mutates these options. `remote`
is also a non-exported type, so no external implementations can currently
be created. It is therefore safe to set these defaults before options are
applied.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
In docker versions before v24.0, the dockerd daemon allowed adjusting
its own oom-score; in that case we would forward the configuration to
containerd (when running as child process) for it to do the same.
This feature was deprecated in 5a922dc162,
and removed in fb96b94ed0, but some fields
were left behind in the supervisor package.
This patch removes the oomScore field and adjustOOMScore method from
the remove type.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Update to containerd 1.7.18, which now migrated to the errdefs module. The
existing errdefs package is now an alias for the module, and should no longer
be used directly.
This patch:
- updates the containerd dependency: https://github.com/containerd/containerd/compare/v1.7.17...v1.7.18
- replaces uses of the old package in favor of the new module
- adds a linter check to prevent accidental re-introduction of the old package
- adds a linter check to prevent using the "log" package, which was also
migrated to a separate module.
There are still some uses of the old package in (indirect) dependencies,
which should go away over time.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
For current implementation of Checkpoint Restore (C/R) in docker, it
will write the checkpoint to content store. However, when restoring
libcontainerd uses .Digest().Encoded(), which will remove the info
of alg, leading to error.
Signed-off-by: huang-jl <1046678590@qq.com>
The monitorDaemon() goroutine calls startContainerd() then blocks on
<-daemonWaitCh to wait for it to exit. The startContainerd() function
would (re)initialize the daemonWaitCh so a restarted containerd could be
waited on. This implementation was race-free because startContainerd()
would synchronously initialize the daemonWaitCh before returning. When
the call to start the managed containerd process was moved into the
waiter goroutine, the code to initialize the daemonWaitCh struct field
was also moved into the goroutine. This introduced a race condition.
Move the daemonWaitCh initialization to guarantee that it happens before
the startContainerd() call returns.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Split task creation and start into two separate method calls in the
libcontainerd API. Clients now have the opportunity to inspect the
freshly-created task and customize its runtime environment before
starting execution of the user-specified binary.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The workaround is no longer required. The bug has been fixed in stable
versions of all supported containerd branches.
This reverts commit fb7ec1555c.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The github.com/containerd/containerd/log package was moved to a separate
module, which will also be used by upcoming (patch) releases of containerd.
This patch moves our own uses of the package to use the new module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The DeepEqual ignore required in the daemon tests is a bit ugly, but it
works given the new protoc output.
We also have to ignore lints related to schema1 deprecations; these do
not apply as we must continue to support this schema version.
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
This type was introduced in
0a79e67e4f
Make use of it throughout our log-format handling code, and convert back
to a string before we pass it to the containerd client.
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
- use local variables and remove some intermediate variables
- handle the events inside the switch itself; this makes all the
switch branches use the same logic, instead of "some" using
a `continue`, and others falling through to have the event handled
outside of the switch.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Synchronize the code to do the same thing as Exec.
reap doesn't need to be called before the start event was sent.
There's already a defer block which cleans up the process in case where
an error occurs.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Error check in defer block used wrong error variable which is always nil
if the flow reaches the defer. This caused the `newProcess.Kill` to be
never called if the subsequent attemp to attach to the stdio failed.
Although this only happens in Exec (as Start does overwrite the error),
this also adjusts the Start to also use the returned error to avoid this
kind of mistake in future changes.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
