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

(#2223582) test-login: skip consistency checks when logind is not active #404

Merged

Conversation

jacekmigacz
Copy link
Member

@jacekmigacz jacekmigacz commented Jul 18, 2023

There are two ways in swich sd_login_* functions acquire data: some are derived from the cgroup path, but others use the data serialized by logind.

When the tests are executed under Fedora's mock, without systemd-spawn but instead in a traditional chroot, test-login gets confused: the "outside" cgroup path is visible, so sd_pid_get_unit() and sd_pid_get_session() work, but sd_session_is_active() and other functions that need logind data fail.

Such a buildroot setup is fairly bad, but it can be encountered in the wild, so let's just skip the tests in that case.

/* Information printed is from the live system */
sd_pid_get_unit(0, …) → "session-237.scope"
sd_pid_get_user_unit(0, …) → "n/a"
sd_pid_get_slice(0, …) → "user-1000.slice"
sd_pid_get_session(0, …) → "237"
sd_pid_get_owner_uid(0, …) → 1000
sd_pid_get_cgroup(0, …) → "/user.slice/user-1000.slice/session-237.scope" sd_uid_get_display(1000, …) → "(null)"
sd_uid_get_sessions(1000, …) → [0] ""
sd_uid_get_seats(1000, …) → [0] ""
Assertion 'r >= 0' failed at src/libsystemd/sd-login/test-login.c:104, function test_login(). Aborting.

(cherry picked from commit dc400d39b32bf5ad6eefe6ef55f0299cf65787fd)

Resolves: #2223582

@mergify mergify bot added the pr/needs-ci Formerly needs-ci label Jul 18, 2023
@github-actions
Copy link

github-actions bot commented Jul 18, 2023

Tracker - 2223582

The following commits meet all requirements

commit upstream
3268c26 - test-login: skip consistency checks when logind is not active systemd/systemd@ac56446

@systemd-rhel-bot systemd-rhel-bot added pr/needs-review Formerly needs-review tracker/unapproved Formerly needs-acks labels Jul 18, 2023
Copy link
Member

@jamacku jamacku left a comment

Choose a reason for hiding this comment

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

@jacekmigacz please update upstream reference (cherry-pick) to: ac5644635dba54ce5eb0ff394fc0bc772a984849

@systemd-rhel-bot systemd-rhel-bot removed the pr/needs-review Formerly needs-review label Jul 18, 2023
There are two ways in swich sd_login_* functions acquire data:
some are derived from the cgroup path, but others use the data serialized
by logind.

When the tests are executed under Fedora's mock, without systemd-spawn
but instead in a traditional chroot, test-login gets confused:
the "outside" cgroup path is visible, so sd_pid_get_unit() and
sd_pid_get_session() work, but sd_session_is_active() and other functions
that need logind data fail.

Such a buildroot setup is fairly bad, but it can be encountered in the wild, so
let's just skip the tests in that case.

/* Information printed is from the live system */
sd_pid_get_unit(0, …) → "session-237.scope"
sd_pid_get_user_unit(0, …) → "n/a"
sd_pid_get_slice(0, …) → "user-1000.slice"
sd_pid_get_session(0, …) → "237"
sd_pid_get_owner_uid(0, …) → 1000
sd_pid_get_cgroup(0, …) → "/user.slice/user-1000.slice/session-237.scope"
sd_uid_get_display(1000, …) → "(null)"
sd_uid_get_sessions(1000, …) → [0] ""
sd_uid_get_seats(1000, …) → [0] ""
Assertion 'r >= 0' failed at src/libsystemd/sd-login/test-login.c:104, function test_login(). Aborting.

(cherry picked from commit ac56446)

Resolves: #2223582
@mergify mergify bot removed the pr/needs-ci Formerly needs-ci label Jul 18, 2023
Copy link
Member

@jamacku jamacku left a comment

Choose a reason for hiding this comment

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

LGTM

@jamacku jamacku added this to the RHEL-8.9 milestone Jul 18, 2023
@jacekmigacz jacekmigacz merged commit f80320d into redhat-plumbers:main Jul 20, 2023
10 checks passed
@jamacku
Copy link
Member

jamacku commented Jul 21, 2023

@jacekmigacz, the bug wasn't approved, so it shouldn't be merged.

@jamacku jamacku added released and removed tracker/unapproved Formerly needs-acks labels Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants