System tests: Update the journalctl function to ignore No entry message #26575

ypu · 2025-07-05T02:57:31Z

For currently version of journalctl, --user option only works when the persistent storage is enabled. So we need to check this option before we use it. Otherwise a set of tests will failed with can not find expected output from journalctl with rootless user.

Does this PR introduce a user-facing change?

NONE

packit-as-a-service · 2025-07-05T03:04:58Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

ypu · 2025-07-07T08:47:28Z

/release-note-none

Luap99

For currently version of journalctl, --user option only works when the persistent storage is enabled.

Do you have some docs for that? Because first of all my system uses Storage=auto which also works. In general config checks like that are a bad idea because systemd might change the behaviours. It is best to do a proper feature check.

Also why does --user-unit work but --user doesn't? I am a bit confused why you can read the system log but not the per user logs.

Luap99 · 2025-07-07T09:08:20Z

test/system/helpers.systemd.bash

@@ -14,12 +14,44 @@ fi

 mkdir -p $UNIT_DIR

+journalctl_support_user() {
+    r_value=0
+    run systemd-analyze cat-config systemd/journald.conf | grep Storage


The pipe does not capture output here at all. $output is set by the run function and that does not output to stdout by default so the pipe cannot work.

Luap99 · 2025-07-07T09:19:53Z

test/system/helpers.systemd.bash

+    if [[ "$output" == "Storage=persistent" ]]; then
+        r_value=1
+    fi


If you actually have a drop in file cat-config prints all file outputs while this only catches one, so I am not sure this is is even correct.

I would suggest you do a proper function test instead by checking if journalctl --user returns logs and laso the result should be cached so we don't always run this command over and over again.

Lastly isn't this just inverse of what you are saying, 0 return should mean success but you return 1 instead which is super confusing for any caller.

Luap99 · 2025-07-07T09:21:11Z

test/system/helpers.systemd.bash

 systemctl() {
    timeout --foreground -v --kill=10 $PODMAN_TIMEOUT systemctl $_DASHUSER "$@"
 }

 journalctl() {
-    timeout --foreground -v --kill=10 $PODMAN_TIMEOUT journalctl $_DASHUSER "$@"
+    if is_rootless and ! journalctl_support_user; then


you need to use && and not and, it would be news to me that and is accepted by bash and from a quick test it doesn't work for me.

Thanks a lot for your reviewing. I am a little mess up with shell and python. And just test it with Fedora and RHEL 9.6 with should go to the if branch and didn’t find the problem in the code. Now the code is updated. Should work as expected.

Luap99 · 2025-07-07T16:18:49Z

I still think checking the config file is the wrong approach, the default uses auto which also works fine. Your code cannot handle drop in files correctly either.

Would it not be much simpler to run something like journalctl --user -b -n1 -q and make sure the output is not empty, i.e. there is at least one log line logged?

Or maybe we should ask a different question. Is there any reason to use --user at all then if --user-unit works better? Could we not just use this all the time instead of complicating the code.

ypu · 2025-07-07T16:23:57Z

Hi @Luap99 Thanks for your reviewing. Actually this is found in our gating tests for 9.6. And I found this from the journactl man page:
`
SOURCE OPTIONS
The following options control where to read journal records from:

   --system, --user
       Show messages from system services and the kernel (with --system). Show messages from service of current user (with --user). If neither is specified, show all messages that the user can see.

       The --user option affects how --unit= arguments are treated. See --unit=.

       Note that --user only works if persistent logging is enabled, via the Storage= setting in journald.conf(5).

       Added in version 205.

`

And the systemd-analyze cat-config command is also pick from the journald.conf ‘s comments.

# Entries in this file show the compile time defaults. Local configuration
# should be created by either modifying this file (or a copy of it placed in
# /etc/ if the original file is shipped in /usr/), or by creating "drop-ins" in
# the /etc/systemd/journald.conf.d/ directory. The latter is generally
# recommended. Defaults can be restored by simply deleting the main
# configuration file and all drop-ins located in /etc/.
#
# Use 'systemd-analyze cat-config systemd/journald.conf' to display the full config.

ypu · 2025-07-07T16:42:16Z

Just double confirmed that the journalctl --user actually works in Fedora even when the journald.conf is not set to any value. This may related with the systemd version in the system. Although this code will not failed, but will try to update the function for the support check.

ypu · 2025-07-08T09:43:56Z

Hi @Luap99
Updated the Pullreq based on your comments. Mainly changed in following items:

Update the checking command by using “joruanlctl --user -b -n 1”
Make the check only run once when we load the file and use a global variable JOURNALCTL_SUPPORT_USER for other functions to use it directly.

Test it with case “quadlet - basic” with both Fedora and RHEL 9.6 and test cases are passed in both host.

Can you help to review this again? Thanks a lot!

Luap99

Just small things but this is what I had in mind, thanks.

test/system/helpers.systemd.bash

For currently version of journalctl, --user option only works when the persistent storage is enabled. So we need to check this option before we use it. Otherwise a set of tests will failed with can not find expected output from journalctl with rootless user. Signed-off-by: Yiqiao Pu <ypu@redhat.com>

Luap99

LGTM

cc @timcoding1988

openshift-ci · 2025-07-08T12:38:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, ypu

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:

~~OWNERS~~ [Luap99]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jankaluza · 2025-07-11T13:07:06Z

LGTM

Tested on my system where I could reproduce the issue without patch and it works correctly with patch.

Luap99

/lgtm

openshift-ci bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Jul 5, 2025

ypu force-pushed the journalctl branch from 8c75909 to ac9c00b Compare July 5, 2025 03:04

ypu force-pushed the journalctl branch 2 times, most recently from 2119a60 to 94fd8a6 Compare July 7, 2025 08:44

openshift-ci bot added release-note-none and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Jul 7, 2025

Luap99 reviewed Jul 7, 2025

View reviewed changes

ypu force-pushed the journalctl branch from 94fd8a6 to 97f6ad0 Compare July 7, 2025 15:28

ypu marked this pull request as draft July 7, 2025 16:42

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2025

ypu force-pushed the journalctl branch from 97f6ad0 to c0ab117 Compare July 8, 2025 09:38

ypu marked this pull request as ready for review July 8, 2025 09:39

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 8, 2025

Luap99 reviewed Jul 8, 2025

View reviewed changes

test/system/helpers.systemd.bash Outdated Show resolved Hide resolved

test/system/helpers.systemd.bash Outdated Show resolved Hide resolved

ypu force-pushed the journalctl branch from c0ab117 to 70143a7 Compare July 8, 2025 10:35

ypu force-pushed the journalctl branch from 70143a7 to c735f8e Compare July 8, 2025 12:29

Luap99 approved these changes Jul 8, 2025

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2025

Luap99 reviewed Jul 11, 2025

View reviewed changes

openshift-ci bot assigned Luap99 Jul 11, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2025

openshift-merge-bot bot merged commit e28445e into containers:main Jul 11, 2025
43 checks passed

System tests: Update the journalctl function to ignore No entry message #26575

System tests: Update the journalctl function to ignore No entry message #26575

Conversation

ypu commented Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Does this PR introduce a user-facing change?

Uh oh!

packit-as-a-service bot commented Jul 5, 2025

Uh oh!

ypu commented Jul 7, 2025

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

Luap99 Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Luap99 Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Luap99 Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

ypu Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Luap99 commented Jul 7, 2025

Uh oh!

ypu commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ypu commented Jul 7, 2025

Uh oh!

ypu commented Jul 8, 2025

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Jul 8, 2025

Uh oh!

jankaluza commented Jul 11, 2025

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ypu commented Jul 5, 2025 •

edited

Loading

ypu commented Jul 7, 2025 •

edited

Loading