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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix volume fsType handling for CSI volumes #142

Merged
merged 1 commit into from
Sep 11, 2020

Conversation

ialidzhikov
Copy link
Member

How to categorize this PR?

/area storage
/kind bug
/priority normal
/platform openstack

What this PR does / why we need it:
Currently for v1.19 the guestbook test is failing with:

$ k -n gardener-e2e-v2xt6 get po
NAME             READY   STATUS             RESTARTS   AGE
redis-master-0   0/1     CrashLoopBackOff   5          4m13s

$ k -n gardener-e2e-v2xt6 logs redis-master-0
 06:57:52.24 INFO  ==> ** Starting Redis **
1:C 11 Sep 2020 06:57:52.256 # oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
1:C 11 Sep 2020 06:57:52.256 # Redis version=5.0.7, bits=64, commit=00000000, modified=0, pid=1, just started
1:C 11 Sep 2020 06:57:52.256 # Configuration loaded
1:M 11 Sep 2020 06:57:52.257 # Can't open the append-only file: Permission denied

Investigation shows that the fsType is no longer present in the PV spec for v1.19:

Before the fix for PV with k/k v1.19.

   csi:
      driver: cinder.csi.openstack.org
      volumeAttributes:
        storage.kubernetes.io/csiProvisionerIdentity: 1599816861990-1847-cinder.csi.openstack.org
      volumeHandle: 41dc69a8-c853-4b19-b9bd-fb234d6ad618

After the fix:

   csi:
      driver: cinder.csi.openstack.org
      fsType: ext4
      volumeAttributes:
        storage.kubernetes.io/csiProvisionerIdentity: 1599816861990-1847-cinder.csi.openstack.org
      volumeHandle: 41dc69a8-c853-4b19-b9bd-fb234d6ad618

Basically this afterwards is causing the fsGroup field to be not respected when the volume is mounted to the Pod.

For Pod with configured:

  securityContext:
    fsGroup: 1001

Current:

$ cd /data
$ ls -la
total 24
drwxr-xr-x. 3 root root  4096 Sep 11 06:54 .
drwxr-xr-x  1 root root  4096 Sep 11 08:36 ..
drwx------. 2 root root 16384 Sep 11 06:54 lost+found

Expected:

$ cd /data
$ ls -la
total 28
drwxrwsr-x. 3 root 1001  4096 Sep 11 09:09 .
drwxr-xr-x. 1 root root  4096 Sep 11 09:05 ..
-rw-r--r--. 1 1001 1001     0 Sep 11 09:05 appendonly.aof
drwxrws---. 2 root 1001 16384 Sep 11 09:05 lost+found
-rw-r--r--. 1 1001 1001     0 Sep 11 09:09 test.txt

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
Ref kubernetes-csi/external-provisioner#400

Release note:

An issue causing CSI PV to do not have set `spec.csi.fsType` is now fixed. The csi-provisioner is now started with `--default-fstype=ext4` which is the default fstype to be used when there is no fstype specified in the StorageClass.

Co-authored-by: vpnachev <[email protected]>
Signed-off-by: ialidzhikov <[email protected]>
@ialidzhikov ialidzhikov requested review from a team as code owners September 11, 2020 09:57
@gardener-robot gardener-robot added area/storage Storage related kind/bug Bug platform/openstack OpenStack platform/infrastructure priority/normal labels Sep 11, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 11, 2020
Copy link
Member

@vpnachev vpnachev left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added the reviewed/lgtm Has approval for merging label Sep 11, 2020
@ialidzhikov
Copy link
Member Author

$ go test -timeout=0 -mod=vendor ./test/suites/shoot \
      --v -ginkgo.v -ginkgo.progress -ginkgo.noColor \
      --report-file=/tmp/report.json \
      -kubecfg=$KUBECONFIG \
      -shoot-name=$SHOOT_NAME \
      -project-namespace=$PROJECT_NAME \
      -ginkgo.focus="should deploy guestbook app successfully"

• [SLOW TEST:131.334 seconds]
Shoot application testing
/Users/i331370/go/src/github.com/gardener/gardener/test/integration/shoots/applications/shoot_app.go:56
  GuestBook
  /Users/i331370/go/src/github.com/gardener/gardener/test/integration/shoots/applications/shoot_app.go:67
    [DEFAULT] [RELEASE] [SHOOT] should deploy guestbook app successfully
    /Users/i331370/go/src/github.com/gardener/gardener/test/framework/gingko_utils.go:26
------------------------------
S
Ran 1 of 19 Specs in 131.343 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 18 Skipped
--- PASS: TestGardenerSuite (131.35s)
PASS
ok  	github.com/gardener/gardener/test/suites/shoot	132.147s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/storage Storage related kind/bug Bug needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) platform/openstack OpenStack platform/infrastructure priority/3 Priority (lower number equals higher priority) reviewed/lgtm Has approval for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants