Spoof json-file logging support #3375
Conversation
For docker scripting compatibility, allow for json-file logging when creating args for conmon. That way, when json-file is supported, that case can be easily removed. Signed-off-by: Peter Hunt <[email protected]>
haircommander
force-pushed
the
json-file-hotfix
branch
from
90a4141 to
11fbd20
Compare
| @@ -246,7 +246,9 @@ func (r *OCIRuntime) createOCIContainer(ctr *Container, cgroupParent string, res | |||
| } | |||
|
|
|||
| logDriver := KubernetesLogging | |||
| if ctr.LogDriver() != "" { | |||
| if ctr.LogDriver() == JSONLogging { | |||
| logrus.Errorf("json-file logging specified but not supported. Choosing k8s-file logging instead") | |||
There was a problem hiding this comment.
This should be a Warnf not an Errorf.
There was a problem hiding this comment.
I really want to keep the Errrof here - we need to make it very obvious that they should switch to k8s-file
There was a problem hiding this comment.
I would be fine either way, I'll let you two decide. I will weigh in that the only reason a user cares about the difference is if they want to read the log as a json. If we leave it as a warning, and document it, we can point users to the fact they can't read it as json, and thus not have a loud error.
Signed-off-by: Peter Hunt <[email protected]>
|
I also added a documentation fix to fix #3363 (comment) |
|
Ok I will go along with @mheon |
|
PTAL @mheon I could use approval :) |
|
Oh, oops |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, 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
github-actions
bot
added
the
locked - please file new issue/PR
For docker scripting compatibility, allow for json-file logging when creating args for conmon. That way, when json-file is supported, that case can be easily removed.
Fixes: #3363