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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop reading from /proc/sys/kernel/osrelease at trailing newline #9374

Merged
merged 1 commit into from
Jun 27, 2020

Conversation

sjuxax
Copy link
Contributor

@sjuxax sjuxax commented Jun 18, 2020

Summary

ebpf.plugin and several parts of the kernel-collector sibling project either read /proc/sys/kernel/osrelease directly or use it as a fallback. This file ends with a newline, which is read into the kernel_string variable. ebpf.plugin then fails to find the eBPF libraries because it's looking for a filename that ends with a newline.

Since it never makes sense for osrelease to include a newline, update ebpf.plugin to stop reading by the newline. This fixes the eBPF plugin on my system (which is running a custom kernel) and allows it to load the kernel-collector components.

Component Name

ebpf.plugin

Test Plan

I haven't added any tests, but it'd be a good thing to add a test for. I assume a test for this doesn't already exist or it wouldn't have been a problem. 馃槉

Additional Information

Similar commit for the kernel-collectors is on my local and may get around to making a PR for it some day.

@squash-labs
Copy link

squash-labs bot commented Jun 18, 2020

Manage this branch in Squash

Test this branch here: https://sjuxaxosrelease-newline-n4ikd.squash.io

@CLAassistant
Copy link

CLAassistant commented Jun 18, 2020

CLA assistant check
All committers have signed the CLA.

@prologic
Copy link
Contributor

@netdata/agent 鈽濓笍

Copy link
Contributor

@thiagoftsm thiagoftsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the command hexdump -C 聽/proc/sys/kernel/osrelease on Slackware Current and CentOS 8 and I observed that on both distributions it is safety to stop read when a 0a character is found.
I also tested your PR on the following environments:

  • Slackware current (kernel 5.7.2)
  • Slackware current (kernel 5.4.47)
  • Debian 10.4 (kernel 4.19.118-2 )
  • CentOS 8 (Kernel 4.18.0-193.6.3.el8_2.x86_64 )
  • Slackware current (kernel 4.16.18)
  • Slackware current (kernel 4.14.184)
  • CentOS 7.8 ( kernel 3.10.0-1127)

and everything worked as expected.
Thank you very much @sjuxax !

@sjuxax
Copy link
Contributor Author

sjuxax commented Jun 25, 2020

np, thanks for the reviews everyone. Was a little bummed to see this didn't get merged into 1.23, especially since eBPF is one of the release's major headlines. Is there something else I need to do before the PR can be merged? No big deal if this was patched through something else of course, would just be good to notate here for the record. Thanks!

@underhood
Copy link
Contributor

underhood commented Jun 25, 2020

@sjuxax you seem to have 2 approving reviews already so probable reason why it didn't get merged is only the current code freeze (which is active few days pre/post release)

@thiagoftsm
Copy link
Contributor

Hi @sjuxax ,

I am sorry, I forgot to give you an explanation about the code freeze, but as soon it finishes we will merge your PR.

best regards!

@thiagoftsm thiagoftsm merged commit 1ba7c28 into netdata:master Jun 27, 2020
prologic pushed a commit that referenced this pull request Jun 29, 2020
* [ci skip] create nightly packages and update changelog

* [Package amd64 DEB][Build latest] Package build process trigger

* [Package i386 DEB][Build latest] Package build process trigger

* [Package amd64 RPM][Build latest] Package build process trigger

* Stop reading from /proc/sys/kernel/osrelease at trailing newline. (#9374)

Remove new line that was creating wrong log information.

* fixed typo

Co-authored-by: netdatabot <bot@netdata.cloud>
Co-authored-by: Jeff Cook <jeff@jeffcook.io>
@joelhans joelhans changed the title Stop reading from /proc/sys/kernel/osrelease at trailing newline. Stop reading from /proc/sys/kernel/osrelease at trailing newline Jun 30, 2020
Ferroin pushed a commit to Ferroin/netdata that referenced this pull request Jul 1, 2020
* [ci skip] create nightly packages and update changelog

* [Package amd64 DEB][Build latest] Package build process trigger

* [Package i386 DEB][Build latest] Package build process trigger

* [Package amd64 RPM][Build latest] Package build process trigger

* Stop reading from /proc/sys/kernel/osrelease at trailing newline. (netdata#9374)

Remove new line that was creating wrong log information.

* fixed typo

Co-authored-by: netdatabot <bot@netdata.cloud>
Co-authored-by: Jeff Cook <jeff@jeffcook.io>
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.

None yet

5 participants