local-storage: Fix permission #7217

BoleynSu · 2023-04-05T13:44:39Z

/var/lib/rancher/k3s/storage/ should be 700
/var/lib/rancher/k3s/storage/* should be 777

brandond · 2023-04-05T15:49:47Z

tests/integration/localstorage/testdata/localstorage_pod.yaml

    imagePullPolicy: IfNotPresent
+    commands:
+    - sleep


Modify this to:

Run as a non-root user

Successfully touch a file under the /data volume before sleeping

Good catch! Changed the uid/gid of the pod.

We also need to touch or perhaps echo some content into the file to confirm that it can be created.

brandond · 2023-04-05T15:50:17Z

tests/integration/localstorage/localstorage_int_test.go

@@ -72,6 +72,10 @@ var _ = Describe("local storage", func() {
 			fileStat, err = os.Stat(k3sStorage + "/" + volumeName)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(fmt.Sprintf("%04o", fileStat.Mode().Perm())).To(Equal("0777"))
+
+			touchResult, err := testutil.K3sCmd("kubectl --namespace=default exec -it volume-test -- touch /data/file")


Lets have the pod itself do this

I prefer to exec here to avoid any possible race condition.

No, I want to see the pod itself successfully creating the file, not your exec command. If you want to use an exec to test for the file's existence or having the correct content, that's fine. But We need to see that the file can be successfully created by a non-root user within the pod.

exec commands excute the touch in the pod with a non-root user.

I will add another test for what you want though. However, that test may be flaky in the future and I do not want to spend time fixing it atm.

I'm not sure why it would be flakey, are you concerned that the test kubectl exec command might run before the pod has entered the sleep? If so, just wait for a second or two after the pod becomes ready before running the test.

I do want to see the pod command itself successfully writing a file to the PVC, not just the test command.

Consider that the pod is ready and running then it is context switch to execute other codes before it write to the file. Later, we check for existence of the file before it gets context switched back. The eventally can work around this if the delay it less than 10s but the flakiness is still possible and will happen in the future.

/var/lib/rancher/k3s/storage/ should be 700 /var/lib/rancher/k3s/storage/* should be 777 Fixes k3s-io#2348 Signed-off-by: Boleyn Su <[email protected]>

brandond · 2023-04-17T23:30:04Z

@dereknola this looks good to me. The AWX example workload from #3704 seems to work with this change:

root@k3s-server-1:/# kubectl get pod -n awx
NAME                                               READY   STATUS    RESTARTS   AGE
awx-operator-controller-manager-77c67f9445-znc85   2/2     Running   0          18m
awx-demo-postgres-13-0                             1/1     Running   0          14m
awx-demo-task-7d48b5497-dphxz                      4/4     Running   0          14m
awx-demo-web-54f46445f-rqf25                       3/3     Running   0          13m


root@k3s-server-1:/# kubectl get pvc -n awx
NAME                                 STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   AGE

postgres-13-awx-demo-postgres-13-0   Bound    pvc-bdeca81e-985b-434a-9c2e-bc235be7d04c   8Gi        RWO            local-path     14m

root@k3s-server-1:/# kubectl get pod -n awx
NAME                                               READY   STATUS    RESTARTS   AGE
awx-operator-controller-manager-77c67f9445-znc85   2/2     Running   0          18m
awx-demo-postgres-13-0                             1/1     Running   0          14m
awx-demo-task-7d48b5497-dphxz                      4/4     Running   0          14m
awx-demo-web-54f46445f-rqf25                       3/3     Running   0          13m

root@k3s-server-1:/# ls -la /var/lib/rancher/k3s/storage/
total 12
drwx------ 3 root root 4096 Apr 17 23:14 .
drwxr-xr-x 6 root root 4096 Apr 17 23:14 ..
drwxrwxrwx 3 root root 4096 Apr 17 23:14 pvc-bdeca81e-985b-434a-9c2e-bc235be7d04c_awx_postgres-13-awx-demo-postgres-13-0

root@k3s-server-1:/# ls -la /var/lib/rancher/k3s/storage/pvc-bdeca81e-985b-434a-9c2e-bc235be7d04c_awx_postgres-13-awx-demo-postgres-13-0
total 12
drwxrwxrwx 3 root root 4096 Apr 17 23:14 .
drwx------ 3 root root 4096 Apr 17 23:14 ..
drwxrwxrwx 3 root root 4096 Apr 17 23:14 data

root@k3s-server-1:/# ls -la /var/lib/rancher/k3s/storage/pvc-bdeca81e-985b-434a-9c2e-bc235be7d04c_awx_postgres-13-awx-demo-postgres-13-0/data
total 12
drwxrwxrwx  3 root root 4096 Apr 17 23:14 .
drwxrwxrwx  3 root root 4096 Apr 17 23:14 ..
drwx------ 19  999 root 4096 Apr 17 23:14 pgdata

root@k3s-server-1:/# kubectl exec -it -n awx awx-demo-postgres-13-0 -- /bin/sh -c 'grep data /proc/mounts; ls -la /var/lib/postgresql/data'
/dev/root /var/lib/postgresql/data ext4 rw,relatime,discard,errors=remount-ro 0 0
total 12
drwxrwxrwx  3 root     root     4096 Apr 17 23:14 .
drwxr-xr-x  1 postgres postgres 4096 Apr 12 05:57 ..
drwx------ 19 postgres root     4096 Apr 17 23:14 pgdata

dereknola · 2023-04-18T16:44:39Z

I will approve this once the local storage integration test passes in CI

tests/integration/localstorage/testdata/localstorage_pod.yaml

BoleynSu · 2023-05-02T23:51:07Z

Friendly ping

brandond · 2023-05-03T04:04:36Z

It's still failing for some reason, haven't had a chance to see why

Signed-off-by: Derek Nola <[email protected]>

codecov · 2023-05-03T20:45:23Z

Codecov Report

Patch coverage has no change and project coverage change: +9.75 🎉

Comparison is base (257fa2c) 9.82% compared to head (2cc8beb) 19.57%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7217      +/-   ##
==========================================
+ Coverage    9.82%   19.57%   +9.75%     
==========================================
  Files         147       81      -66     
  Lines       10845     5461    -5384     
==========================================
+ Hits         1065     1069       +4     
+ Misses       9558     4170    -5388     
  Partials      222      222

Flag	Coverage Δ
unittests	`19.57% <ø> (+9.75%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 74 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

brandond · 2023-05-03T21:07:33Z

tests/integration/localstorage/localstorage_int_test.go

@@ -26,7 +26,7 @@ var _ = BeforeSuite(func() {
 	}
 })

-var _ = Describe("local storage", func() {
+var _ = Describe("local storage", Ordered, func() {


oh yeah, that'd help.

brandond · 2023-05-03T21:08:22Z

Lets be sure to squash this on merge.

* local-storage: Fix permission /var/lib/rancher/k3s/storage/ should be 700 /var/lib/rancher/k3s/storage/* should be 777 Fixes k3s-io#2348 Signed-off-by: Boleyn Su <[email protected]> * Fix pod command field type * Fix to int test Signed-off-by: Derek Nola <[email protected]> --------- Signed-off-by: Boleyn Su <[email protected]> Signed-off-by: Derek Nola <[email protected]> Co-authored-by: Brad Davidson <[email protected]> Co-authored-by: Derek Nola <[email protected]>

* local-storage: Fix permission /var/lib/rancher/k3s/storage/ should be 700 /var/lib/rancher/k3s/storage/* should be 777 Fixes #2348 * Fix pod command field type * Fix to int test --------- Signed-off-by: Boleyn Su <[email protected]> Signed-off-by: Derek Nola <[email protected]> Co-authored-by: Boleyn Su <[email protected]> Co-authored-by: Brad Davidson <[email protected]>

* local-storage: Fix permission /var/lib/rancher/k3s/storage/ should be 700 /var/lib/rancher/k3s/storage/* should be 777 Fixes #2348 Signed-off-by: Boleyn Su <[email protected]> Signed-off-by: Derek Nola <[email protected]> Co-authored-by: Boleyn Su <[email protected]> Co-authored-by: Brad Davidson <[email protected]>

BoleynSu requested a review from a team as a code owner April 5, 2023 13:44

BoleynSu force-pushed the master branch 3 times, most recently from fdaaf73 to 55b6af7 Compare April 5, 2023 13:51

BoleynSu mentioned this pull request Apr 5, 2023

local-storage: Fix permission #7196

Closed

BoleynSu force-pushed the master branch from 55b6af7 to e4edf4b Compare April 5, 2023 14:14

brandond requested changes Apr 5, 2023

View reviewed changes

BoleynSu force-pushed the master branch 2 times, most recently from b8c0f71 to 0d41313 Compare April 6, 2023 14:52

BoleynSu requested a review from brandond April 6, 2023 14:53

local-storage: Fix permission

1df8e7f

/var/lib/rancher/k3s/storage/ should be 700 /var/lib/rancher/k3s/storage/* should be 777 Fixes k3s-io#2348 Signed-off-by: Boleyn Su <[email protected]>

BoleynSu force-pushed the master branch from 0d41313 to 1df8e7f Compare April 16, 2023 10:10

jessebot mentioned this pull request Apr 16, 2023

nextcloud-nginx container crashlooping after securityContext update; /var/www/html/config always owned by root nextcloud/helm#335

Open

brandond approved these changes Apr 17, 2023

View reviewed changes

brandond requested changes Apr 18, 2023

View reviewed changes

tests/integration/localstorage/testdata/localstorage_pod.yaml Outdated Show resolved Hide resolved

Fix pod command field type

d459121

Fix to int test

2cc8beb

Signed-off-by: Derek Nola <[email protected]>

dereknola approved these changes May 3, 2023

View reviewed changes

brandond reviewed May 3, 2023

View reviewed changes

brandond approved these changes May 3, 2023

View reviewed changes

dereknola merged commit a736b4b into k3s-io:master May 4, 2023

This was referenced May 9, 2023

[Release-1.24] local-storage: Fix permission #7472

Merged

[Release-1.25] local-storage: Fix permission #7473

Merged

[Release-1.26] local-storage: Fix permission #7474

Merged

cguertin14 mentioned this pull request May 31, 2023

Minor: k3s update from v1.27.1+k3s1 to v1.27.2+k3s1 cguertin14/k3s-ansible-ha#21

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

local-storage: Fix permission #7217

local-storage: Fix permission #7217

BoleynSu commented Apr 5, 2023

brandond Apr 5, 2023

BoleynSu Apr 6, 2023

brandond Apr 6, 2023

brandond Apr 5, 2023

BoleynSu Apr 6, 2023

brandond Apr 6, 2023

BoleynSu Apr 6, 2023

BoleynSu Apr 6, 2023

brandond Apr 7, 2023

BoleynSu Apr 16, 2023 •

edited

Loading

brandond commented Apr 17, 2023 •

edited

Loading

dereknola commented Apr 18, 2023

BoleynSu commented May 2, 2023

brandond commented May 3, 2023

codecov bot commented May 3, 2023 •

edited

Loading

brandond May 3, 2023

brandond commented May 3, 2023

local-storage: Fix permission #7217

local-storage: Fix permission #7217

Conversation

BoleynSu commented Apr 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BoleynSu Apr 16, 2023 • edited Loading

Choose a reason for hiding this comment

brandond commented Apr 17, 2023 • edited Loading

dereknola commented Apr 18, 2023

BoleynSu commented May 2, 2023

brandond commented May 3, 2023

codecov bot commented May 3, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

brandond commented May 3, 2023

BoleynSu Apr 16, 2023 •

edited

Loading

brandond commented Apr 17, 2023 •

edited

Loading

codecov bot commented May 3, 2023 •

edited

Loading