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

Improve session cleanup tests stability. #121961

Merged
merged 5 commits into from
Dec 27, 2021
Merged

Improve session cleanup tests stability. #121961

merged 5 commits into from
Dec 27, 2021

Conversation

azasypkin
Copy link
Member

@azasypkin azasypkin commented Dec 23, 2021 •
edited
Loading

Summary

In the flaky test we take the following steps:

  1. Log in via '/internal/security/login'
  2. Verify that session cookie is valid via '/internal/security/me'
  3. Wait until session expires (idle timeout is 5s) and the cleanup job removes it
  4. Verify that session is no longer valid via '/internal/security/me'

The test fails on the step 2 likely because sometimes Kibana is too slow on CI and it takes more than 5s between steps 1 and 2.

In this PR I'm just doubling timeouts in the configuration for the affected tests.

Flaky test runner:

Fixes: #121482

@azasypkin azasypkin removed the skip-ci label Dec 24, 2021
@azasypkin azasypkin force-pushed the issue-121482-fix-session-cleanup-test branch from 1b3754e to fbcddd6 Compare December 24, 2021 08:07
@azasypkin azasypkin added Feature:Security/Authentication Platform Security - Authentication Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! test v7.17.0 v8.0.0 v8.1.0 auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes labels Dec 24, 2021
@azasypkin azasypkin marked this pull request as ready for review December 24, 2021 09:58
@azasypkin azasypkin requested a review from a team as a code owner December 24, 2021 09:58
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

jportner
jportner approved these changes Dec 27, 2021
View reviewed changes
Copy link
Contributor

@jportner jportner left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this!

One question -- I noticed the values in session_lifespan.config.ts are also 5s/10s. It appears those tests aren't flaky (yet) but do you think it might be worth it to change those too?

@azasypkin
Copy link
Member Author

One question -- I noticed the values in session_lifespan.config.ts are also 5s/10s. It appears those tests aren't flaky (yet) but do you think it might be worth it to change those too?

Yeah, it's a good point! It'd not hurt to bump these timeouts there as well, I'll update PR (and run a flaky-test-runner job before merging).

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@azasypkin
Copy link
Member Author

https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/38 (x30 runs, CI group 6 and 18, ✅ ) - merging.

@azasypkin azasypkin merged commit 660023b into elastic:main Dec 27, 2021
@azasypkin azasypkin deleted the issue-121482-fix-session-cleanup-test branch December 27, 2021 19:21
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
❌ 8.0 Commit could not be cherrypicked due to conflicts
❌ 7.17 Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 121961

This was referenced Dec 28, 2021
Merged
Merged
azasypkin added a commit that referenced this pull request Dec 28, 2021
@azasypkin
Improve session cleanup tests stability. (#121961) (#122062)
d4e31e2 
# Conflicts:
#	x-pack/test/security_api_integration/tests/session_idle/cleanup.ts
#	x-pack/test/security_api_integration/tests/session_lifespan/cleanup.ts
azasypkin added a commit that referenced this pull request Dec 28, 2021
@azasypkin
Improve session cleanup tests stability. (#121961) (#122063)
9ea57db 
# Conflicts:
#	x-pack/test/security_api_integration/tests/session_idle/cleanup.ts
#	x-pack/test/security_api_integration/tests/session_lifespan/cleanup.ts
@azasypkin azasypkin mentioned this pull request Jan 24, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jportner jportner jportner approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Security/Authentication Platform Security - Authentication release_note:skip Skip the PR/issue when compiling release notes Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! test v7.17.0 v8.0.0 v8.1.0
Projects
None yet
Milestone
No milestone
5 participants
@azasypkin @elasticmachine @kibana-ci @kibanamachine @jportner