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

Automate tests for feature Prevent CSI session leaks #2533

Closed
wants to merge 1 commit into from
Closed

Automate tests for feature Prevent CSI session leaks #2533

wants to merge 1 commit into from

Conversation

nikhilbarge
Copy link
Contributor

@nikhilbarge nikhilbarge commented Sep 1, 2023 •
edited
Loading

What this PR does / why we need it:
PR will add e2e tests for feature prevent CSI vCenter session leaks, to verify sessions are created with unique user-agent (feature-wise) and session are cleaned up on container stops

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Testing done:
executed e2e tests

Special notes for your reviewer:

Release note:

adding tests for feature "prevent CSI vCenter session leaks"

@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 Sep 1, 2023
@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 Sep 1, 2023
@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/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 1, 2023
@nikhilbarge nikhilbarge force-pushed the add-tests-prevent-session-leaks branch from 159730e to 31f50f9 Compare September 1, 2023 15:26
@svcbot-qecnsdp
Copy link

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

@nikhilbarge nikhilbarge changed the title WIP:Automate tests for prevent csi session leaks Automate tests for prevent csi session leaks Sep 2, 2023
@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 Sep 2, 2023
@svcbot-qecnsdp
Copy link

Started Vanilla file pre-checkin pipeline... Build Number: 702

@svcbot-qecnsdp
Copy link

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

@svcbot-qecnsdp
Copy link 

Build ID: 702
File vanilla build status: FAILURE 
Stage before exit: e2e-tests 
Jenkins E2E Test Results: 
FAIL! -- 37 Passed | 1 Failed | 0 Pending | 761 Skipped
--- FAIL: TestE2E (5791.17s)
FAIL

Ginkgo ran 1 suite in 1h38m41.978453773s

Test Suite Failed
make: Leaving directory `/home/worker/workspace/csi-file-vanilla-precheckin/702/vsphere-csi-driver`

@svcbot-qecnsdp
Copy link 

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

Ran 1 of 799 Specs in 309.222 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 798 Skipped
PASS

Ginkgo ran 1 suite in 18m24.566743134s
Test Suite Passed
--
------------------------------

Ran 12 of 799 Specs in 4481.624 seconds
SUCCESS! -- 12 Passed | 0 Failed | 0 Pending | 787 Skipped
PASS

Ginkgo ran 1 suite in 1h15m55.868860902s
Test Suite Passed
--
Ran 51 of 799 Specs in 5434.137 seconds
FAIL! -- 50 Passed | 1 Failed | 0 Pending | 748 Skipped
--- FAIL: TestE2E (5434.26s)
FAIL

Ginkgo ran 1 suite in 1h31m48.971490606s

Test Suite Failed

@svcbot-qecnsdp
Copy link

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

@svcbot-qecnsdp
Copy link

Started Vanilla file pre-checkin pipeline... Build Number: 703

@svcbot-qecnsdp
Copy link 

Build ID: 2364
Block vanilla build status: FAILURE 
Stage before exit: e2e-tests 
Jenkins E2E Test Results: 
Ran 3 of 799 Specs in 134.443 seconds
FAIL! -- 1 Passed | 2 Failed | 0 Pending | 796 Skipped
--- FAIL: TestE2E (134.53s)
FAIL

Ginkgo ran 1 suite in 12m24.020226025s

Test Suite Failed

@svcbot-qecnsdp
Copy link

Started vanilla file pipeline... Build Number: 958

@nikhilbarge nikhilbarge force-pushed the add-tests-prevent-session-leaks branch from 35a1640 to 56cacb9 Compare September 2, 2023 18:42
@svcbot-qecnsdp
Copy link

Started vanilla Block pipeline... Build Number: 2147

@svcbot-qecnsdp
Copy link 

File vanilla build status: FAILURE 
Stage before exit: e2e-tests 
Jenkins E2E Test Results: 
Ran 42 of 799 Specs in 7678.306 seconds
FAIL! -- 41 Passed | 1 Failed | 0 Pending | 757 Skipped
--- FAIL: TestE2E (7678.39s)
FAIL

Ginkgo ran 1 suite in 2h9m31.559331703s

Test Suite Failed

@svcbot-qecnsdp
Copy link 

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

Ran 1 of 799 Specs in 390.342 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 798 Skipped
PASS

Ginkgo ran 1 suite in 7m55.803620305s
Test Suite Passed
--
------------------------------

Ran 13 of 799 Specs in 5539.609 seconds
SUCCESS! -- 13 Passed | 0 Failed | 0 Pending | 786 Skipped
PASS

Ginkgo ran 1 suite in 1h32m39.939497894s
Test Suite Passed
--

Ran 53 of 799 Specs in 1202.399 seconds
FAIL! -- 51 Passed | 2 Failed | 0 Pending | 746 Skipped


Ginkgo ran 1 suite in 20m22.659969885s

Test Suite Failed

@svcbot-qecnsdp
Copy link

Started Vanilla file pre-checkin pipeline... Build Number: 705

@nikhilbarge nikhilbarge force-pushed the add-tests-prevent-session-leaks branch from 56cacb9 to 62efd3d Compare September 3, 2023 07:11
@svcbot-qecnsdp
Copy link 

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

Ran 2 of 799 Specs in 935.116 seconds
SUCCESS! -- 2 Passed | 0 Failed | 0 Pending | 797 Skipped
PASS

Ginkgo ran 1 suite in 17m34.228366504s
Test Suite Passed
make: Leaving directory `/home/worker/workspace/csi-file-vanilla-precheckin/705/vsphere-csi-driver`

@svcbot-qecnsdp
Copy link

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

@svcbot-qecnsdp
Copy link 

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

Ran 3 of 799 Specs in 924.587 seconds
SUCCESS! -- 3 Passed | 0 Failed | 0 Pending | 796 Skipped
PASS

Ginkgo ran 1 suite in 24m32.702016397s
Test Suite Passed

@nikhilbarge nikhilbarge changed the title Automate tests for prevent csi session leaks Automate tests for feature Prevent CSI session leaks Sep 4, 2023
@nikhilbarge nikhilbarge force-pushed the add-tests-prevent-session-leaks branch 4 times, most recently from b686152 to 3a3286b Compare September 6, 2023 06:25
@svcbot-qecnsdp
Copy link

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

@svcbot-qecnsdp
Copy link 

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

Ran 3 of 799 Specs in 1094.066 seconds
SUCCESS! -- 3 Passed | 0 Failed | 0 Pending | 796 Skipped
PASS

Ginkgo ran 1 suite in 28m34.897250984s
Test Suite Passed

@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 Sep 12, 2023
@divyenpatel
Copy link
Member

/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 Sep 12, 2023
@nikhilbarge nikhilbarge force-pushed the add-tests-prevent-session-leaks branch 2 times, most recently from c02053e to 7183f02 Compare September 13, 2023 10:43
sashrith
sashrith suggested changes Sep 13, 2023
View reviewed changes
tests/e2e/prevent_csi_session_leaks.go
admissionapi "k8s.io/pod-security-admission/api"
)

var _ = ginkgo.Describe("[csi-block-vanilla][csi-file-vanilla][csi-supervisor]"+
Copy link
Contributor

Choose a reason for hiding this comment

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

can you attach the test logs from all applicable flavours (as gist)

tests/e2e/prevent_csi_session_leaks.go Outdated Show resolved Hide resolved
tests/e2e/prevent_csi_session_leaks.go Outdated Show resolved Hide resolved
tests/e2e/prevent_csi_session_leaks.go Show resolved Hide resolved
tests/e2e/prevent_csi_session_leaks.go Outdated Show resolved Hide resolved
tests/e2e/prevent_csi_session_leaks.go Outdated Show resolved Hide resolved
tests/e2e/prevent_csi_session_leaks.go Show resolved Hide resolved
tests/e2e/prevent_csi_session_leaks.go
admissionapi "k8s.io/pod-security-admission/api"
)

var _ = ginkgo.Describe("[csi-block-vanilla][csi-file-vanilla][csi-supervisor]"+
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I don't see any tests here for restapi sessions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is common functionality for all session cleanup no specific logic required for for rest-api.

Copy link
Contributor

@sashrith sashrith Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

we should verify if restapi sessions are getting created with correct user agent values

tests/e2e/prevent_csi_session_leaks.go
})
gomega.Expect(waitErr).NotTo(gomega.HaveOccurred())

// check sessions are cleaned up
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we check all the older csi sessions are gone since we are checking for session leaks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are checking if all sessions are cleaned up created from current installed driver, sessions created from driver with older versions will not be cleaned up with this feature.

tests/e2e/prevent_csi_session_leaks.go Show resolved Hide resolved
@nikhilbarge nikhilbarge closed this Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@divyenpatel divyenpatel divyenpatel left review comments

@sashrith sashrith sashrith requested changes

@kavyashree-r kavyashree-r Awaiting requested review from kavyashree-r

Assignees
No one assigned
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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nikhilbarge @k8s-ci-robot @svcbot-qecnsdp @divyenpatel @sashrith