oci: use json formatted errors from the runtime #3311
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
approved
|
it is would be safer though to do it only with runc, any idea that doesn't require to hard code the runtime name? |
giuseppe
force-pushed
the
oci-errors
branch
2 times, most recently
from
9b99096 to
0311c32
Compare
request json formatted error messages from the OCI runtime so that we can nicely print them. Signed-off-by: Giuseppe Scrivano <[email protected]>
|
I've addressed the comments, pushed a new version. I still think we should have a way to disable it somehow |
|
@giuseppe Should we put something in the libpod.conf about how to call the OCI |
the problem I had is that the flag will be global while we allow multiple OCI runtimes to be specified, can the TOML conf file be extended to have such a structured data? Otherwise it won't really help when using different OCI runtimes, as you need to change the conf file every time. |
|
I am fine with extending it. |
| if ss.si.Message != "" { | ||
| return errors.Wrapf(ErrInternal, "container create failed: %s", ss.si.Message) | ||
| return errors.Wrapf(ErrOCIRuntime, "%s", ss.si.Message) |
There was a problem hiding this comment.
Personally, I like having the "container created failed:" in the message. Although I'd prefer it to be "container creation failed:".
There was a problem hiding this comment.
LGTM, one comment for consideration.
add a new configuration `runtime_supports_json` to list what OCI runtimes support the --log-format=json option. If the runtime is not listed here, libpod will redirect stdout/stderr from the runtime process. Signed-off-by: Giuseppe Scrivano <[email protected]>
|
added a new patch so that we can configure what runtimes support the json format |
|
/lgtm |
I'm running the BATS tests manually once in a while, and catching several problems each week that make it past the rest of CI. Since the BATS tests run at RPM gating time, we need to catch problems earlier. Try running the tests from Cirrus. Tests will be skipped on Ubuntu due to a too-ancient version of coreutils (8.28; the 'timeout -v' we use requires 8.29). Tests are run *after* integration tests, even though these take three minutes and would be nice to have fail quickly, because running before causes bizarre CI failures. Shrug. UPDATE: also fix run test, broken by containers#3311. Signed-off-by: Ed Santiago <[email protected]>
github-actions
bot
added
the
locked - please file new issue/PR
request json formatted error messages from the OCI runtime so that we
can nicely print them.
Signed-off-by: Giuseppe Scrivano [email protected]