Commit Graph

24 Commits

Author SHA1 Message Date
Sebastiaan van Stijn 6560e0b136
cmd/dockerd: initContainerD(): clean-up some logs
Change the log-level for messages about starting the managed containerd instance
to be the same as for the main API. And remove a redundant debug-log.

With this patch:

    dockerd
    INFO[2022-08-11T11:46:32.573299176Z] Starting up
    INFO[2022-08-11T11:46:32.574304409Z] containerd not running, starting managed containerd
    INFO[2022-08-11T11:46:32.575289181Z] started new containerd process                address=/var/run/docker/containerd/containerd.sock module=libcontainerd pid=5370
cmd/dockerd: initContainerD(): clean-up some logs

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-08-11 14:11:08 +02:00
Sebastiaan van Stijn b6b0b0a05f
libcontainerd/supervisor: don't write log-level to config file
the `--log-level` flag overrides whatever is in the containerd configuration file;
f033f6ff85/cmd/containerd/command/main.go (L339-L352)

Given that we set that flag when we start the containerd binary, there is no need
to write it both to the generated config-file and pass it as flag.

This patch also slightly changes the behavior; as both dockerd and containerd use
"info" as default log-level, don't set the log-level if it's the default.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-08-11 14:11:06 +02:00
Sebastiaan van Stijn bff3e85625
libcontainerd/supervisor: store location of config-file
Adding a remote.configFile to store the location instead of re-constructing its
location each time. Also fixing a minor inconsistency in the error formats.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-08-11 14:11:05 +02:00
Sebastiaan van Stijn 7a9791f096
libcontainerd/supervisor: store location of pidFile
Adding a remote.pidFile to store the location instead of re-constructing its
location each time. Also performing a small refactor to use `strconv.Itoa`
instead of `fmt.Sprintf`.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-08-11 14:11:03 +02:00
Sebastiaan van Stijn 89ab14a115
libcontainerd/supervisor: make supervisor adjust OOM score for containerd
Containerd, like dockerd has a OOMScore configuration option to adjust its own
OOM score. In dockerd, this option was added when default installations were not
yet running the daemon as a systemd unit, which made it more complicated to set
the score, and adding a daemon option was convenient.

A binary adjusting its own score has been frowned upon, as it's more logical to
make that the responsibility of the process manager _starting_ the daemon, which
is what we did for dockerd in 21578530d7.

There have been discussions on deprecating the daemon flag for dockerd, and
similar discussions have been happening for containerd.

This patch changes how we set the OOM score for the containerd child process,
and to have dockerd (supervisor) set the OOM score, as it's acting as process
manager in this case (performing a role similar to systemd otherwise).

With this patch, the score is still adjusted as usual, but not written to the
containerd configuration file;

    dockerd --oom-score-adjust=-123
    cat /proc/$(pidof containerd)/oom_score_adj
    -123

As a follow-up, we may consider to adjust the containerd OOM score based on the
daemon's own score instead of on the `cli.OOMScoreAdjust` configuration so that
we will also adjust the score in situations where dockerd's OOM score was set
through other ways (systemd or manually adjusting the cgroup). A TODO was added
for this.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-08-11 14:11:01 +02:00
Sebastiaan van Stijn 1d2a669445
libcontainerd/supervisor: use correct logger
Don't call logrus directly, but use the logger that was set.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-08-11 14:10:59 +02:00
Sebastiaan van Stijn 7b0bd43a27
libcontainerd/supervisor: remove unused remote.rootDir
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-08-10 23:14:39 +02:00
Sebastiaan van Stijn 6b7e19ff42
libcontainerd/supervisor: remove unused RWMutex
This RWMutex was added in 9c4570a958, and used in
the `remote.Client()` method. Commit dd2e19ebd5
split the code for client and daemon, but did not remove the mutex.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-08-10 23:14:35 +02:00
Sebastiaan van Stijn 051e604adc
libcontainerd/supervisor: simplify logic for disabling CRI plugin
The existing implementation used a `nil` value for the CRI plugin's configuration
to indicate that the plugin had to be disabled. Effectively, the `Plugins` value
was only used as an intermediate step, only to be removed later on, and to instead
add the given plugin to `DisabledPlugins` in the containerd configuration.

This patch removes the intermediate step; as a result we also don't need to mask
the containerd `Plugins` field, which was added to allow serializing the toml.

A code comment was added as well to explain why we're (currently) disabling the
CRI plugin by default, which may help future visitors of the code to determin
if that default is still needed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-08-03 10:39:20 +02:00
Sebastiaan van Stijn ba2ff69894
libcontainerd: switch generated containerd.toml to v2 (v1 is deprecated)
Before this patch:

    INFO[2022-07-27T14:30:06.188762628Z] Starting up
    INFO[2022-07-27T14:30:06.190750725Z] libcontainerd: started new containerd process  pid=2028
    ...
    WARN[0000] containerd config version `1` has been deprecated and will be removed in containerd v2.0, please switch to version `2`, see https://github.com/containerd/containerd/blob/main/docs/PLUGINS.md#version-header
    INFO[2022-07-27T14:30:06.220024286Z] starting containerd                           revision=10c12954828e7c7c9b6e0ea9b0c02b01407d3ae1 version=v1.6.6

With this patch:

    INFO[2022-07-27T14:28:04.025543517Z] Starting up
    INFO[2022-07-27T14:28:04.027447105Z] libcontainerd: started new containerd process  pid=1377
    ...
    INFO[2022-07-27T14:28:04.054483270Z] starting containerd                           revision=10c12954828e7c7c9b6e0ea9b0c02b01407d3ae1 version=v1.6.6

And the generated /var/run/docker/containerd/containerd.toml:

```toml
disabled_plugins = ["io.containerd.grpc.v1.cri"]
imports = []
oom_score = 0
plugin_dir = ""
required_plugins = []
root = "/var/lib/docker/containerd/daemon"
state = "/var/run/docker/containerd/daemon"
temp = ""
version = 2

[cgroup]
  path = ""

[debug]
  address = "/var/run/docker/containerd/containerd-debug.sock"
  format = ""
  gid = 0
  level = "debug"
  uid = 0

[grpc]
  address = "/var/run/docker/containerd/containerd.sock"
  gid = 0
  max_recv_message_size = 16777216
  max_send_message_size = 16777216
  tcp_address = ""
  tcp_tls_ca = ""
  tcp_tls_cert = ""
  tcp_tls_key = ""
  uid = 0

[metrics]
  address = ""
  grpc_histogram = false

[plugins]

[proxy_plugins]

[stream_processors]

[timeouts]

[ttrpc]
  address = ""
  gid = 0
  uid = 0
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-07-27 16:33:00 +02:00
Eng Zer Jun c55a4ac779
refactor: move from io/ioutil to io and os package
The io/ioutil package has been deprecated in Go 1.16. This commit
replaces the existing io/ioutil functions with their new definitions in
io and os packages.

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
2021-08-27 14:56:57 +08:00
Sebastiaan van Stijn 2a7c1cc1d6
libcontainerd/supervisor: replace BurntSushi/toml with pelletier/go-toml
Taking the same approach as was taken in containerd

The new library has a slightly different output;

- keys at the same level are sorted alphabetically
- empty sections not omitted (`proxy_plugins`, `stream_processors`, `timeouts`),
  which could possibly be be addressed with an "omitempty" in containerd's struct.
- empty slices are not omitted (`imports`, `required_plugins`)

After sorting the "before" configuration the diff looks like this:

```patch
diff --git a/config-before-sorted.toml b/config-after.toml
index cc771ce7ab..43a727f589 100644
--- a/config-before-sorted.toml
+++ b/config-after.toml
@@ -1,6 +1,8 @@
 disabled_plugins = ["cri"]
+imports = []
 oom_score = 0
 plugin_dir = ""
+required_plugins = []
 root = "/var/lib/docker/containerd/daemon"
 state = "/var/run/docker/containerd/daemon"
 version = 0
@@ -37,6 +39,12 @@ version = 0
     shim = "containerd-shim"
     shim_debug = true

+[proxy_plugins]
+
+[stream_processors]
+
+[timeouts]
+
 [ttrpc]
   address = ""
   gid = 0
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-04-02 17:42:57 +02:00
Brian Goff bef73d8b07 Wait for c8d process exit instead of polling API
In the containerd supervisor, instead of polling the healthcheck API
every 500 milliseconds we can just wait for the process to exit.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2019-10-16 12:23:10 -07:00
Sebastiaan van Stijn e554ab5589
Allow system.MkDirAll() to be used as drop-in for os.MkDirAll()
also renamed the non-windows variant of this file to be
consistent with other files in this package

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-08-08 15:05:49 +02:00
Sebastiaan van Stijn c85fe2d224
Merge pull request #38522 from cpuguy83/fix_timers
Make sure timers are stopped after use.
2019-06-07 13:16:46 +02:00
Simão Reis 3134161be3 Fix nil pointer derefence on failure to connect to containerd
Signed-off-by: Simão Reis <smnrsti@gmail.com>
2019-01-30 12:41:54 -01:00
Brian Goff eaad3ee3cf Make sure timers are stopped after use.
`time.After` keeps a timer running until the specified duration is
completed. It also allocates a new timer on each call. This can wind up
leaving lots of uneccessary timers running in the background that are
not needed and consume resources.

Instead of `time.After`, use `time.NewTimer` so the timer can actually
be stopped.
In some of these cases it's not a big deal since the duraiton is really
short, but in others it is much worse.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2019-01-16 14:32:53 -08:00
Sebastiaan van Stijn dd7799afd4
update containerd client and dependencies to v1.2.0
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2018-11-05 18:46:26 +01:00
Tibor Vass 34eede0296 Remove 'docker-' prefix for containerd and runc binaries
This allows to run the daemon in environments that have upstream containerd installed.

Signed-off-by: Tibor Vass <tibor@docker.com>
2018-09-24 21:49:03 +00:00
Sebastiaan van Stijn 06b9588c2d
Merge pull request #37759 from dmcgowan/fix-libcontainerd-startup-error
Add fail fast path when containerd fails on startup
2018-09-14 15:15:38 +02:00
Derek McGowan ce0b0b72bc
Add fail fast path when containerd fails on startup
Prevents looping of startup errors such as containerd
not being found on the path.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
2018-09-13 17:34:52 -07:00
Derek McGowan c3e3293843
Fix supervisor healthcheck throttling
Fix default case causing the throttling to not be used.
Ensure that nil client condition is handled.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
2018-09-04 11:00:28 -07:00
John Howard 5accd82634 Add containerd.WithTimeout(60*time.Second) to match old calls
Signed-off-by: John Howard <jhoward@microsoft.com>
2018-08-23 12:03:43 -07:00
Derek McGowan dd2e19ebd5
libcontainerd: split client and supervisor
Adds a supervisor package for starting and monitoring containerd.
Separates grpc connection allowing access from daemon.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
2018-08-06 10:23:04 -07:00