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

test: Drop logind user ID cache #19757

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

martinpitt
Copy link
Member

TestAccounts creates and recycles UIDs a lot, with different user names.
This can lead to failures: testFirst creating user testA, then
cleaning it up, then testSecond creates testB with the same UID, and all
of a sudden logind thinks that the running session of testB belongs to
testA (literally the user name, not just the UID).

logind caches this in /run/systemd/users/, and I haven't found a way to
wean it off that cache. So delete all uid cache files in between tests.
Move from restart to stop, as otherwise the new logind will rewrite
the files; and we don't have to explicitly start it as it gets D-Bus
activated.

Move the restarting until after stopping the remaining user@ services,
as otherwise there are still legitimate processes around from the
to-be-removed user.


See my debug odysee notes. This definitively fixes the failure that I can reproduce 100% locally, but this is the kind of change that very likely rips something apart somewhere else. Let's see what the bots think!

Commit a2dcd62 switched from `split('\n')` to `splitlines()` which
treats an empty input as expected. Drop the now unnecessary `if`.
TestAccounts creates and recycles UIDs a lot, with different user names.
This can lead to failures: testFirst creating user testA, then
cleaning it up, then testSecond creates testB with the same UID, and all
of a sudden logind thinks that the running session of testB belongs to
testA (literally the user name, not just the UID).

logind caches this in /run/systemd/users/, and I haven't found a way to
wean it off that cache. So delete all uid cache files in between tests.
Move from `restart` to `stop`, as otherwise the new logind will rewrite
the files; and we don't have to explicitly start it as it gets D-Bus
activated.

Move the restarting until after stopping the remaining user@ services,
as otherwise there are still legitimate processes around from the
to-be-removed user.
@martinpitt
Copy link
Member Author

Most failures are just a simple stupidity (fixed), but this failure now tells me it's not enough to restart logind once.. Re-running for cleaner results.

@martinpitt
Copy link
Member Author

Much better! There are only two flakes now, and they are well known and unrelated. Retrying.

@martinpitt martinpitt marked this pull request as ready for review December 19, 2023 03:31
@jelly jelly merged commit 1b1ab98 into cockpit-project:main Dec 19, 2023
90 of 91 checks passed
@martinpitt martinpitt deleted the logind-users-cache branch December 19, 2023 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flake unstable test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants