Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use sdjournal instead of journalctl for oom parsing #1707

Closed
wants to merge 1 commit into from

Conversation

sjenning
Copy link
Contributor

Addresses kube issue kubernetes/kubernetes#34965

Alternative to #1544

Use github.com/coreos/go-systemd/sdjournal instead of exec'ing journalctl -f -k and leaking the process.

NOTE: Using sdjournal requires the systemd headers be installed on the compiling system. For EL distros that is yum install systemd-devel.

@derekwaynecarr @eparis @euank @dchen1107 @vishh @adohe

@sjenning
Copy link
Contributor Author

cc @jsafrane

@derekwaynecarr
Copy link
Collaborator

our queue of cadvisor updates is growing, need to track down e2e problems.

@dashpole
Copy link
Collaborator

dashpole commented Aug 1, 2017

/test pull-cadvisor-e2e

@dashpole
Copy link
Collaborator

dashpole commented Aug 1, 2017

@sjenning please rebase. The infra issue should be fixed

@k8s-ci-robot
Copy link
Collaborator

@sjenning: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-cadvisor-e2e b06faf0 link /test pull-cadvisor-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@sjenning
Copy link
Contributor Author

sjenning commented Aug 1, 2017

@dashpole so it hit the new dependency on systemd-devel I mentioned in the PR description.

I0801 19:30:09.573] >> building cadvisor
W0801 19:30:11.299] # github.com/google/cadvisor/vendor/github.com/coreos/go-systemd/sdjournal
W0801 19:30:11.300] vendor/github.com/coreos/go-systemd/sdjournal/journal.go:27:33: fatal error: systemd/sd-journal.h: No such file or directory
W0801 19:30:11.300]  // #include <systemd/sd-journal.h>
W0801 19:30:11.300]                                  ^

Can you add the distro appropriate package to the builder?

@euank
Copy link
Collaborator

euank commented Aug 1, 2017

If we're planning to switch to directly parsing kmsg instead, I don't see why this change is necessary.

@sjenning
Copy link
Contributor Author

sjenning commented Aug 1, 2017

@euank at the time I submitted this, that PR hadn't moved since mid-March. I did note that this was an alternative to #1544

Only one need be accepted, but one does need to be accepted.

@sjenning
Copy link
Contributor Author

sjenning commented Aug 1, 2017

It looks like both of these approaches (sdjounal and reading /dev/kmsg) are used in NPD.
https://github.com/kubernetes/node-problem-detector/blob/master/pkg/systemlogmonitor/logwatchers/kmsg/log_watcher.go
https://github.com/kubernetes/node-problem-detector/blob/master/pkg/systemlogmonitor/logwatchers/journald/log_watcher.go

@dchen1107 just mentioned this as a way during a sig-node meeting, so I wrote it up.

@sjenning
Copy link
Contributor Author

sjenning commented Sep 6, 2017

closed in favor of #1729

@sjenning sjenning closed this Sep 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants