Support disabling default setup of shim logger.#3239
Merged
estesp merged 1 commit intocontainerd:masterfrom Apr 25, 2019
Merged
Support disabling default setup of shim logger.#3239estesp merged 1 commit intocontainerd:masterfrom
estesp merged 1 commit intocontainerd:masterfrom
Conversation
Before this change, the v2 runtime shim setup code was hardcoded to always configure logrus to write logs to the "log" FIFO present in the current working directory. This only happens in the "default" action codepath (i.e. not shim start or shim delete). This is problematic for shims that execute outside the current working directory of a bundle. For example, it often doesn't make sense for shims that manage multiple containers to execute in a single bundle directory. Additionally, shim processes that require being pre-created, i.e. spun up before tasks they will handle are actually created, won't have a log FIFO to write to until a task is created. This change leaves the default behavior as is but introduces a Binary Config field that will optionally disable automatic configuration of logrus to use the "log" FIFO. This allows shims to configure their own logger if necessary while still re-using the rest of the shim helper code in containerd. Signed-off-by: Erik Sipsma <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #3239 +/- ##
==========================================
- Coverage 44.63% 44.63% -0.01%
==========================================
Files 113 113
Lines 12161 12162 +1
==========================================
Hits 5428 5428
- Misses 5898 5899 +1
Partials 835 835
Continue to review full report at Codecov.
|
samuelkarp
approved these changes
Apr 25, 2019
Member
|
LGTM |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Before this change, the v2 runtime shim setup code was hardcoded to always
configure logrus to write logs to the "log" FIFO present in the current working
directory. This only happens in the "default" action codepath
(i.e. not shim start or shim delete).
This is problematic for shims that execute outside the current working
directory of a bundle. For example, it often doesn't make sense for shims that
manage multiple containers to execute in a single bundle directory. Additionally,
shim processes that require being pre-created, i.e. spun up before tasks they
will handle are actually created, won't have a log FIFO to write to until a task
is created.
This change leaves the default behavior as is but introduces a Binary Config
field that will optionally disable automatic configuration of logrus to use the
"log" FIFO. This allows shims to configure their own logger if necessary while
still re-using the rest of the shim helper code in containerd.
Signed-off-by: Erik Sipsma [email protected]