-
Notifications
You must be signed in to change notification settings - Fork 39.3k
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
Fix chown on distributed flex volumes (like gluster) #68680
Conversation
pkg/volume/flexvolume/driver-call.go
Outdated
@@ -222,12 +222,14 @@ type DriverStatus struct { | |||
type DriverCapabilities struct { | |||
Attach bool `json:"attach"` | |||
SELinuxRelabel bool `json:"selinuxRelabel"` | |||
SupportsFSGroup bool `json:"supportsFSGroup"` |
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.
Lets call this just FSGroup, similar to SELinuxRelabel & Attach.
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.
@chakri-nelluri fixed
/ok-to-test |
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.
One minor comment.
/retest |
Can you please add a release note to the PR? |
Yes. Could you run again tests. I fixed comment |
pkg/volume/flexvolume/driver-call.go
Outdated
@@ -222,12 +222,14 @@ type DriverStatus struct { | |||
type DriverCapabilities struct { | |||
Attach bool `json:"attach"` | |||
SELinuxRelabel bool `json:"selinuxRelabel"` | |||
FSGroup bool `json:"fSGroup"` |
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.
*Nit - Please make it "fsGroup"
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.
@chakri-nelluri ok thanks, fixed
@gnufied release notes added |
/kind bug |
/lgtm |
putting this one hold. So as @chakri-nelluri can have a final look before it merges. /hold |
/lgtm |
/hold cancel |
LGTM, but holding this for 1.13 |
FYI I rebased code due to conflicts after some other PRs have been merged |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: benoitf, chakri-nelluri, gnufied The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
With this patch it's possible to disable fsGroup kubernetes/kubernetes#68680 it is also merged on OKD openshift/origin#21070 3.11.x Fix gluster#23
What this PR does / why we need it:
FlexVolume mount operation is performing by default the change of volume ownership when fsGroup is provided.
The problem is that when performing that on distributed volumes like a gluster volume, when mounting the volume into a pod, there is big
chown
operation running on all files on this volume.For example, as we may always get the same volume for the same user, k8s is trying to perform a lot of
chown
operations . If there are few files on the volume it’s immediate but with like 5K files, etc it takes a huge timeWith the provided patch, the driver capability
fsGroup:false
would skip that.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #68137
Release note:
/sig-storage