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.

Sign up for GitHub

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

Jump to bottom

Use k8s mount util for bind volume #2861

Merged
merged 1 commit into from
Apr 25, 2024
Merged

Use k8s mount util for bind volume #2861

merged 1 commit into from
Apr 25, 2024

Conversation

nikhilbarge
Copy link
Contributor

@nikhilbarge nikhilbarge commented Apr 22, 2024 •
edited
Loading

What this PR does / why we need it:
Bug fix: PVC mount become ReadOnly on same node when a volume ReadOnly is mounted
Used k8s mount utils instead of gofsutil and used Mount with bind mount parameter.

Testing done:
Running e2e tests
Manually tested the scenario:
As shown in attached log file, I have Created two pods with the same volume one with Readwrite then other one with read only mount permissions.
Verified that i am able to write to volume from pod with RW access even after new pod is created with Readonly

test_read_only_mount.log

tested file pvc RWX and verified if volume mount is successful
test_pvc_RWX.log

tested File pvc RWO and verified if volume mount is successful
test_pvc_file_rwo.log

Here we are using same volume for two Pods. but actually we do not support using same volume for two Pods unless it is
RWM volume.
In terms of k8s RWO is for node and not pod, but we treated it as RWO for pod.

Special notes for your reviewer:

Release note:

Use k8s mount util for bind volume and fix read only mount issue.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 22, 2024
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 22, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @nikhilbarge. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 22, 2024
@svcbot-qecnsdp
Copy link

Started vanilla Block pipeline... Build Number: 2568

@svcbot-qecnsdp
Copy link 

Block vanilla build status: FAILURE 
Stage before exit: make-images

@svcbot-qecnsdp
Copy link

Started vanilla Block pipeline... Build Number: 2569

@svcbot-qecnsdp
Copy link 

Block vanilla build status: FAILURE 
Stage before exit: e2e-tests 
Jenkins E2E Test Results: 
------------------------------

Ran 1 of 848 Specs in 395.789 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 847 Skipped
PASS

Ginkgo ran 1 suite in 7m36.909263404s
Test Suite Passed
--
Ran 14 of 848 Specs in 9495.697 seconds
FAIL! -- 7 Passed | 7 Failed | 0 Pending | 834 Skipped
--- FAIL: TestE2E (9495.78s)
FAIL

Ginkgo ran 1 suite in 2h38m32.129907534s

Test Suite Failed

@svcbot-qecnsdp
Copy link

Started vanilla Block pipeline... Build Number: 2570

@svcbot-qecnsdp
Copy link 

Block vanilla build status: FAILURE 
Stage before exit: testbed-deployment

@svcbot-qecnsdp
Copy link

Started vanilla Block pipeline... Build Number: 2571

@svcbot-qecnsdp
Copy link 

Block vanilla build status: SUCCESS 
Stage before exit: finally 
Jenkins E2E Test Results: 
------------------------------

Ran 0 of 848 Specs in 0.019 seconds
SUCCESS! -- 0 Passed | 0 Failed | 0 Pending | 848 Skipped
PASS

Ginkgo ran 1 suite in 1m7.475228375s
Test Suite Passed

@svcbot-qecnsdp
Copy link

Started Vanilla block pre-checkin pipeline... Build Number: 2745

@svcbot-qecnsdp
Copy link 

Build ID: 2745
Block vanilla build status: SUCCESS 
Stage before exit: e2e-tests 
Jenkins E2E Test Results: 
------------------------------

Ran 0 of 848 Specs in 0.018 seconds
SUCCESS! -- 0 Passed | 0 Failed | 0 Pending | 848 Skipped
PASS

Ginkgo ran 1 suite in 1m10.378598655s
Test Suite Passed

@svcbot-qecnsdp
Copy link

Started Vanilla block pre-checkin pipeline... Build Number: 2746

@svcbot-qecnsdp
Copy link

Started Vanilla block pre-checkin pipeline... Build Number: 2747

@svcbot-qecnsdp
Copy link 

Build ID: 2747
Block vanilla build status: FAILURE 
Stage before exit: e2e-tests 
Jenkins E2E Test Results: 
------------------------------

Ran 1 of 848 Specs in 368.949 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 847 Skipped
PASS

Ginkgo ran 1 suite in 7m18.407774359s
Test Suite Passed
--
Ran 13 of 848 Specs in 1018.237 seconds
FAIL! -- 0 Passed | 13 Failed | 0 Pending | 835 Skipped
--- FAIL: TestE2E (1018.35s)
FAIL

Ginkgo ran 1 suite in 17m17.057650482s

Test Suite Failed

@svcbot-qecnsdp
Copy link

Started vanilla Block pipeline... Build Number: 2572

@svcbot-qecnsdp
Copy link

Started vanilla Block pipeline... Build Number: 2573

@svcbot-qecnsdp
Copy link 

Block vanilla build status: FAILURE 
Stage before exit: e2e-tests 
Jenkins E2E Test Results: 
Ran 1 of 848 Specs in 398.351 seconds
FAIL! -- 0 Passed | 1 Failed | 0 Pending | 847 Skipped
--- FAIL: TestE2E (398.42s)
FAIL

Ginkgo ran 1 suite in 8m3.94279412s

Test Suite Failed

@nikhilbarge nikhilbarge changed the title WIP: use k8s mount util for bind volume Use k8s mount util for bind volume Apr 24, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 24, 2024
@svcbot-qecnsdp
Copy link 

Block vanilla build status: FAILURE 
Stage before exit: e2e-tests 
Jenkins E2E Test Results: 
------------------------------

Ran 1 of 848 Specs in 403.275 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 847 Skipped
PASS

Ginkgo ran 1 suite in 7m45.718938423s
Test Suite Passed
--
------------------------------

Ran 14 of 848 Specs in 7443.332 seconds
SUCCESS! -- 14 Passed | 0 Failed | 0 Pending | 834 Skipped
PASS

Ginkgo ran 1 suite in 2h4m20.427573634s
Test Suite Passed
--

Ran 52 of 848 Specs in 654.439 seconds
FAIL! -- 43 Passed | 9 Failed | 0 Pending | 796 Skipped


Ginkgo ran 1 suite in 11m11.261896661s

Test Suite Failed

@svcbot-qecnsdp
Copy link

Started Vanilla block pre-checkin pipeline... Build Number: 2748

@svcbot-qecnsdp
Copy link 

Build ID: 2748
Block vanilla build status: SUCCESS 
Stage before exit: e2e-tests 
Jenkins E2E Test Results: 
------------------------------

Ran 0 of 848 Specs in 0.021 seconds
SUCCESS! -- 0 Passed | 0 Failed | 0 Pending | 848 Skipped
PASS

Ginkgo ran 1 suite in 1m14.254358329s
Test Suite Passed

@svcbot-qecnsdp
Copy link

Started Vanilla block pre-checkin pipeline... Build Number: 2749

@svcbot-qecnsdp
Copy link 

Build ID: 2749
Block vanilla build status: SUCCESS 
Stage before exit: e2e-tests 
Jenkins E2E Test Results: 
------------------------------

Ran 0 of 848 Specs in 0.018 seconds
SUCCESS! -- 0 Passed | 0 Failed | 0 Pending | 848 Skipped
PASS

Ginkgo ran 1 suite in 1m13.436445689s
Test Suite Passed

@svcbot-qecnsdp
Copy link

Started vanilla Block pipeline... Build Number: 2580

@svcbot-qecnsdp
Copy link 

Block vanilla build status: SUCCESS 
Stage before exit: finally 
Jenkins E2E Test Results: 
------------------------------

Ran 6 of 848 Specs in 1344.153 seconds
SUCCESS! -- 6 Passed | 0 Failed | 0 Pending | 842 Skipped
PASS

Ginkgo ran 1 suite in 23m30.972742822s
Test Suite Passed

@svcbot-qecnsdp
Copy link

Started vanilla file pipeline... Build Number: 1142

@svcbot-qecnsdp
Copy link 

File vanilla build status: FAILURE 
Stage before exit: e2e-tests

@divyenpatel
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 25, 2024
divyenpatel
divyenpatel approved these changes Apr 25, 2024
View reviewed changes
Copy link
Member

@divyenpatel divyenpatel left a comment

Choose a reason for hiding this comment

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

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: divyenpatel, nikhilbarge

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 25, 2024
@divyenpatel
Copy link
Member

@nikhilbarge can you confirm there is no regression for file volume after this change?

@nikhilbarge
Copy link
Contributor Author

@nikhilbarge can you confirm there is no regression for file volume after this change?

tested change with file volume RWX and RWO and verified volume mount is successful. added testing logs

@divyenpatel
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2024
@k8s-ci-robot k8s-ci-robot merged commit 8ff4498 into kubernetes-sigs:master Apr 25, 2024
12 checks passed
@xiaoping8385
Copy link
Contributor

Will this backport to old csi versions? @nikhilbarge ?

@dfajmon dfajmon mentioned this pull request Sep 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@divyenpatel divyenpatel divyenpatel approved these changes

@adikul30 adikul30 Awaiting requested review from adikul30

@SandeepPissay SandeepPissay Awaiting requested review from SandeepPissay

Assignees

@divyenpatel divyenpatel

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nikhilbarge @k8s-ci-robot @svcbot-qecnsdp @divyenpatel @xiaoping8385