Conversation
Signed-off-by: Nemanja Zeljkovic <nocturo@gmail.com>
c52363f to
940a787
Compare
frobware
left a comment
There was a problem hiding this comment.
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.
| 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.
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.