Do not modify default logging parameters - #494
Conversation
|
/retest |
|
@mangelajo this makes sense to me. Would you recommend we also remove those from MicroShift itself for the time being? |
|
/test verify |
|
Unit testing needs some looking: https://github.com/redhat-et/microshift/runs/4598372160?check_suite_focus=true#step:6:8 @oglok ^ can you have a look? , I'm looking into the cni/flannel rabbit-hole |
|
other than that it LGTM! :) |
f98e971 to
99eca8d
Compare
|
/lgtm If it doesn't do what we expect we can always fix it |
|
@mangelajo: you cannot LGTM your own PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
Bot
added
the
approved
|
/retest |
|
/test e2e-openshift-conformance-sig-api-machinery |
|
/lgtm |
|
/retest |
|
/retest |
klog is a singleton library, and since we use that library from all our services in a single process setting different log files won't work, and will syphon all logs into the last log file we add. Keep the simple strategy of letting all output to stderr for now. Related-Issue: openshift#493 Signed-off-by: Miguel Angel Ajo <majopela@redhat.com>
- vloglevel - vmodule - logtostderr - alsologtostderr Signed-off-by: Ricardo Noriega <rnoriega@redhat.com>
Signed-off-by: Ricardo Noriega <rnoriega@redhat.com>
Signed-off-by: Ricardo Noriega <rnoriega@redhat.com>
|
New changes are detected. LGTM label has been removed. |
| data = append(data, ` | ||
| auditConfig: | ||
| auditFilePath: "` + cfg.LogDir + `/openshift-apiserver/audit.log" | ||
| auditFilePath: `+cfg.AuditLogDir+`openshift-apiserver-audit.log |
There was a problem hiding this comment.
| auditFilePath: `+cfg.AuditLogDir+`openshift-apiserver-audit.log | |
| auditFilePath: "`+cfg.AuditLogDir+`openshift-apiserver-audit.log" |
should we keep the commas?
| // Keeping the args in case something must be added in the future | ||
| args := []string{""} |
There was a problem hiding this comment.
| // Keeping the args in case something must be added in the future | |
| args := []string{""} | |
| // Keeping the args in case something must be added in the future | |
| args := []string{} |
There was a problem hiding this comment.
I wonder if this makes kubeproxy fail to start, empty arg in the arg list?
Locally we would not see it, because kubeproxy is internal, and already has configured all iptables, etc.
| logVModule: microshift=4 | ||
| logAlsotostderr: true |
There was a problem hiding this comment.
shall we remove those two?
| logVModule: microshift=4 | |
| logAlsotostderr: true |
Signed-off-by: Ricardo Noriega <rnoriega@redhat.com>
|
/retest |
5 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
xinx!!! ❇️ |
|
/retest |
1 similar comment
|
/retest |
|
/lgtm |
|
@mangelajo: you cannot LGTM your own PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fzdarsky, mangelajo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
klog is a singleton library, and since we use that library from all
our services in a single process setting different log files won't
work, and will syphon all logs into the last log file we add.
Keep the simple strategy of letting all output to stderr for now.
Related-Issue: #493
Signed-off-by: Miguel Angel Ajo majopela@redhat.com