Improve error handling in runc #628
Merged
Conversation
This file contains 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
The error handling on the runc cli is currenly pretty messy because messages to the user are split between regular stderr format and logrus message format. This changes all the error reporting to the cli to only output on stderr and exit(1) for consumers of the api. By default logrus logs to /dev/null so that it is not seen by the user. If the user wants extra and/or structured loggging/errors from runc they can use the `--log` flag to provide a path to the file where they want this information. This allows a consistent behavior on the cli but extra power and information when debugging with logs. This also includes a change to enable the same logging information inside the container's init by adding an init cli command that can share the existing flags for all other runc commands. Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
| @@ -63,12 +62,12 @@ is a directory with a specification file and a root filesystem.`, | |||
| } | |||
|
|
|||
| if os.Geteuid() != 0 { | |||
| logrus.Fatal("runc should be run as root") | |||
| fatalf("runc should be run as root") | |||
There was a problem hiding this comment.
nit: fatal is enough here
There was a problem hiding this comment.
not really because fatal takes an error not string
There was a problem hiding this comment.
i could change fatal to take an interface
There was a problem hiding this comment.
I don't think it needs to get that complicated, should I paid more attention when I reviewed the code :)
|
LGTM |
1 similar comment
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The error handling on the runc cli is currenly pretty messy because
messages to the user are split between regular stderr format and logrus
message format. This changes all the error reporting to the cli to only
output on stderr and exit(1) for consumers of the api.
By default logrus logs to /dev/null so that it is not seen by the user.
If the user wants extra and/or structured loggging/errors from runc they
can use the
--logflag to provide a path to the file where they wantthis information. This allows a consistent behavior on the cli but
extra power and information when debugging with logs.
This also includes a change to enable the same logging information
inside the container's init by adding an init cli command that can share
the existing flags for all other runc commands.
Signed-off-by: Michael Crosby crosbymichael@gmail.com