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

Start the Keep Alive handler for VSphere client used in e2e tests #1518

Conversation

srm09
Copy link
Contributor

@srm09 srm09 commented Apr 19, 2022

What this PR does / why we need it:
Some of the e2e test runs on testgrid keep failing due to failures in the test listed in storage_policy_test.go file. The reason for the failure is that the govmomi Client object we use to assert some conditions times out due to inactivity. As part of an earlier PR, a Keep-Alive handler was added to the client which did not seem to solve the issue. The reason was that the groutine for the handler needs an explicit call to client.Login() method to get activated.

This patch slightly modifies the creation of the govmomi Client object and adds an explicit call to login after adding the keep alive handler and also logs out the session once the test is finished.

Which issue(s) this PR fixes:
n/a

Release note:

NONE

@srm09 srm09 added kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. area/testing labels Apr 19, 2022
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 19, 2022
@srm09
Copy link
Contributor Author

srm09 commented Apr 19, 2022

/retest

Signed-off-by: Sagar Muchhal <muchhals@vmware.com>
@srm09 srm09 force-pushed the fix-e2e/setup-and-terminate-session-with-handler branch from aca12bd to 0f2b832 Compare April 20, 2022 01:39
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 20, 2022
@srm09
Copy link
Contributor Author

srm09 commented Apr 22, 2022

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: srm09

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 22, 2022
Copy link
Member

@yastij yastij left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold

feel free to remove the hold when you want

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 25, 2022
@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, 2022
@srm09
Copy link
Contributor Author

srm09 commented Apr 25, 2022

/unhold
Thanks for taking a look. Tested this one locally on multiple runs, and it worked.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 25, 2022
@k8s-ci-robot k8s-ci-robot merged commit 9a19cdd into kubernetes-sigs:main Apr 25, 2022
@srm09 srm09 deleted the fix-e2e/setup-and-terminate-session-with-handler branch June 15, 2022 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/testing cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants