Skip to content

libcontainerd/supervisor: set log-level through the config-file#48355

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:libcontainer_consolidate_defaults_step2
Aug 23, 2024
Merged

libcontainerd/supervisor: set log-level through the config-file#48355
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:libcontainer_consolidate_defaults_step2

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

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() 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 b6b0b0a, but to my defence, we were also setting the --log-level flag at the time :)

@thaJeztah thaJeztah added area/runtime Runtime area/daemon Core Engine kind/refactor PR's that refactor, or clean-up code labels Aug 20, 2024
@thaJeztah thaJeztah added this to the 28.0.0 milestone Aug 20, 2024
@thaJeztah thaJeztah self-assigned this Aug 20, 2024
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 b6b0b0a,
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 <[email protected]>
@thaJeztah thaJeztah force-pushed the libcontainer_consolidate_defaults_step2 branch from f41c468 to abcb9e9 Compare August 20, 2024 13:59
@thaJeztah thaJeztah marked this pull request as ready for review August 20, 2024 15:09
Copy link
Copy Markdown
Contributor

@coolljt0725 coolljt0725 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah thaJeztah merged commit 202d9cc into moby:master Aug 23, 2024
@thaJeztah thaJeztah deleted the libcontainer_consolidate_defaults_step2 branch August 23, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/daemon Core Engine area/runtime Runtime kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants