Fix a bug where log-driver json-file was made no logs #7754
Conversation
When we added the None log driver, it was accidentally added in the middle of a set of Fallthrough stanzas which all should have led to k8s-file, so that JSON file logging accidentally caused no logging to be selected instead of k8s-file. Signed-off-by: Matthew Heon <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon 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
the
approved
|
Why don't we have tests for this? Okay, I'll take the heat for that one, but can we hold off on merging this until I look into it and write one? |
|
LGTM |
|
Never mind. I haven't the slightest idea how to test this. |
|
@edsantiago Yeah, I really have no idea about testing this one. I suppose we could try and stat the log path listed in |
|
LGTM |
|
/lgtm |
openshift-ci-robot
added
the
do-not-merge/hold
Thanks @mheon, that's what I was missing. I have a test now, but your PR is (mostly) green so I'll add my test in a separate PR. One quick nit: in podman-remote, the "not supported" message goes out on the server side only. Is there a way to make it go to the client instead? |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
Tests all (current) values for --log-driver=X, and one test for invalid value. For those drivers that write a local file (json-file, k8s-file), test that the file exists and contains results of the expected form (timestamp, stdout, 'F' for 'F'ull line, and the expected string output. For json-file, confirm that podman issues a "Choosing k8s-file" warning (only on local. On podman-remote, the warning goes only to the server's stderr). Written in response to containers#7754 in which driver=json-file was falling through to 'none' instead of 'k8s-file'. Signed-off-by: Ed Santiago <[email protected]>
github-actions
bot
added
the
locked - please file new issue/PR
When we added the None log driver, it was accidentally added in the middle of a set of Fallthrough stanzas which all should have
led to k8s-file, so that JSON file logging accidentally caused no logging to be selected instead of k8s-file.