-
Notifications
You must be signed in to change notification settings - Fork 24
Fix overlaying bpf mount #477
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
Conversation
Signed-off-by: Nemanja Zeljkovic <nocturo@gmail.com>
c52363f to
940a787
Compare
frobware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but would prefer long form options to findmnt.
| - | | ||
| #!/bin/sh | ||
| if ! /bin/mount | /bin/grep -q 'bpffs on /sys/fs/bpf'; then | ||
| if ! /usr/bin/findmnt -n -t bpf /sys/fs/bpf >/dev/null 2>&1; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if ! /usr/bin/findmnt -n -t bpf /sys/fs/bpf >/dev/null 2>&1; then | |
| if ! /usr/bin/findmnt --noheadings --types bpf /sys/fs/bpf >/dev/null 2>&1; then |
I'd prefer long-form flags instead of short flags for self-documentation. I had to go lookup -n to see what that did.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a problem, pushed new commit with long-form flags.
Signed-off-by: Nemanja Zeljkovic <nocturo@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #477 +/- ##
===========================
===========================
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@nocturo - Just a quick heads-up and courtesy call: I’ve raised #490. I wanted to mention this since you previously raised and fixed the bpffs mount issue. The goal of #490 is to create a Go implementation that performs the equivalent functions of findmnt/mount. This means we can eliminate the need for an init container that requires a Fedora image, as the functionality will now be integrated into the agent. From your perspective, nothing should change... |
The check in the initContainer for the deployed
daemonSetis only looking for exact text from the mount command, which leads to false positives andbpfman-operatormounting an overlay bpf filesystem causing problems with other applications relying on it. (I had issues with Cilium but it will break anything that is using it)Below is an excerpt of what I mean, first we check the initial state using current method:
then after
bpfman-operatorhas been deployed:This PR addresses the problem by using
findmntfor finding the right mount type for/sys/fs/bpfmountpoint. After changing mydaemonSetto this, I no longer have issues runningbpfman-operatorand Cilium.