-
Notifications
You must be signed in to change notification settings - Fork 315
[IntegTest][develop] Fix Access Denied issue when mutiple AD integ tests run in parallel in the same region #7067
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
base: develop
Are you sure you want to change the base?
Conversation
…n tests Add file-based registry with file locking to coordinate directory stack sharing across parallel pytest workers. This prevents premature deletion of shared AD directory stacks when multiple OS tests run concurrently in the same region. - Add FileLock-based cross-process synchronization - Implement atomic registry file operations in /var/tmp/.pcluster_tests - Replace in-memory reference counting with persistent file registry - Ensure directory stacks are only deleted when all tests complete Fixes issue where first completing test would delete shared directory stack, causing subsequent tests to fail when accessing secrets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about using the existing SharedFixture mechanism which implkemnents already the locking?
| class SharedFixture: | |
| """ | |
| Define the methods to implement fixtures that can be shared across multiple pytest-dist processes. |
| ).is_true() | ||
|
|
||
|
|
||
| class DirectoryStackSharedFixture(SharedFixture): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[BLOCKING] I think the easiest way to use SharedFixture is by using the decorated @xdist_session_fixture
See example:
| @xdist_session_fixture(autouse=True) |
Why can't we reuse the same approach we are currently using for shared VPC stacks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out, you are right. Done. New test ongoing. This time I did exact the same thing as the vpc_stack. However, the behavior now is different.
Previously, before the latest change, when we need a stack, we create one.
Now, we pre-create stacks in all regions and all AD types, although we only need a few of them. And that's what vpc_stack do.
I am figuring if there's a way to create when we need the stack.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7067 +/- ##
===========================================
- Coverage 90.18% 90.17% -0.01%
===========================================
Files 182 183 +1
Lines 16467 16515 +48
===========================================
+ Hits 14851 14893 +42
- Misses 1616 1622 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ad of pre-create stacks in all regions for all directory types.
| logging.warning("Error releasing shared fixture: %s", e) | ||
|
|
||
|
|
||
| @xdist_session_fixture(autouse=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do you prevent the creation of AD stack in every region if this is a session fixture that is autoused?
Description of changes
Implement cross-worker directory stack sharing for AD integration tests
Add file-based registry with file locking to coordinate directory stack sharing across parallel pytest workers. This prevents premature deletion of shared AD directory stacks when multiple OS tests run concurrently in the same region.
Fixes issue where first completing test would delete shared directory stack, causing subsequent tests to fail when accessing secrets.
Tests
Checklist
developadd the branch name as prefix in the PR title (e.g.[release-3.6]).Please review the guidelines for contributing and Pull Request Instructions.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.