kubelet: fix disk space check on btrfs (issue 47046) #47051
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @flavio. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: flavio Assign the PR to them by writing Associated issue: 38337 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
size/M
| major := major(buf.Dev) | ||
| minor := minor(buf.Dev) | ||
| for device, partition := range self.partitions { | ||
| if partition.major == major && partition.minor == minor { | ||
| return &DeviceInfo{device, major, minor}, nil | ||
| } | ||
| } | ||
|
|
||
| mount, ok := self.mounts[dir] |
There was a problem hiding this comment.
nit, I think ok should be found in this case.
|
|
||
| mount, ok := self.mounts[dir] | ||
| if ok && mount.Fstype == "btrfs" && mount.Major == 0 && strings.HasPrefix(mount.Source, "/dev/") { | ||
| major, minor, ok := getBtrfsMajorMinorIds(mount) |
There was a problem hiding this comment.
I feel like this should be major, minor, err
|
@k8s-bot ok to test This generally looks ok to me, but I'm not a storage expert. calling @kubernetes/sig-storage-pr-reviews |
k8s-ci-robot
added
sig/storage
This commit fixes the warning messages reported by kubelet when checking for the disk space on a btrfs `/` which has `/var/lib/kubelet` inside of a btrfs sub-volume. This fix follows the same principle adopted to fix issue kubernetes#38337 with commit dc8b6cc. This commit fixes issue 47046. Signed-off-by: Flavio Castelli <fcastelli@suse.com>
flavio
force-pushed
the
kubelet-btrfs-snapshots-issue-47046
branch
from
72605b8
to
0206d68
Compare
|
@brendandburns: thanks for the feedback, I updated the code accordingly. |
|
@flavio: The following tests failed, say
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. |
|
@flavio wouldn't the fix first go to https://github.com/google/cadvisor/blob/master/fs/fs.go before vendoring in? |
I don't know what is the approach preferred by kubernetes. I just followed what was done with the previous fix about btrfs and kubelet. I've already submitted the patch against cadvisor, but the patch had to be reworked a little because it didn't apply cleanly against master (they made some unrelated changes that are not available inside of the vendored version kubernetes has). I'm open to revisit the approach I've taken. |
|
Oh, yes, sorry for not noticing that. This should be fixed in upstream and then vendored into k8s... |
|
I'm going to close this in favor of vendoring in the change. Please see: https://github.com/kubernetes/community/blob/master/contributors/devel/godep.md#using-godep For instructions for updating a dependency. Make sure that you go to the "Updating a dependency" section not the "adding a dependency" section. And let me apologize in advance for the pain that is |
What this PR does / why we need it:
This PR fixes the warning messages reported by kubelet when checking for the disk space on a btrfs
/which has/var/lib/kubeletinside of a btrfs sub-volume.This fix follows the same principle adopted to fix issue #38337 with commit dc8b6cc.
Which issue this PR fixes: fixes #47046
Special notes for your reviewer:
I'm going to submit the same fix also to cadvisor.