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

(RHEL-65343) backport https://github.com/systemd/systemd/issues/15654#issuecomment-623396457 to RHEL 8.6 #443

Open
wants to merge 10 commits into
base: rhel-8.6.0
Choose a base branch
from
Open

(RHEL-65343) backport https://github.com/systemd/systemd/issues/15654#issuecomment-623396457 to RHEL 8.6 #443

wants to merge 10 commits into from
+138 −67

Conversation

tcornell-bus
Copy link

@tcornell-bus tcornell-bus commented Nov 15, 2024 •
edited by github-actions bot
Loading

I have cherry-picked seven existing commits from the rhel-8.10 branch:
a0b5239
d8fabe7
cd85a65
538bd9b
e019afe
afcfb65
2f55aea

Replicating this pull request

Resolves: RHEL-65343

@github-actions github-actions bot added rhel-8.6.0 pr/needs-ci Formerly needs-ci pr/needs-review Formerly needs-review labels Nov 15, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 15, 2024 •
edited
Loading

Commit validation

Tracker - RHEL-65343

The following commits meet all requirements

commit upstream
2148cc7 - journald: Increase stdout buffer size sooner, when almost full systemd/systemd@034e971
3f140e5 - journald: rework end of line marker handling to use a field table systemd/systemd@549b737
0a2207e - journald: use the fact that client_context_release() returns NULL systemd/systemd@020b4a0
cb1b457 - journald: rework pid change handling systemd/systemd@45ba1ea
43e39e3 - test: Add a test case for #15654 systemd/systemd@c11d8fd
60b08bc - test: Stricter test case for #15654 (Add more checks) systemd/systemd@d38b3b7
6748107 - _man: document the new LINE_BREAK= type systemd/systemd@a3d9aee
e432282 - Fix build with µhttpd 0.9.71 systemd/systemd@d17eabb

The following commits need an inspection

commit note
0bac767 - ci: point C8S containers to the Vault Missing upstream reference ‼️
d64fc36 - test: skip the symlink part of test_touch_file() in GH Actions Missing upstream reference ‼️

Tracker validation

Success

🟢 Tracker RHEL-65343 has set desired product: rhel-8.6.0.z
🟢 Tracker RHEL-65343 has set desired component: systemd
🟢 Tracker RHEL-65343 has been approved
🟢 Tracker RHEL-65343 has set severity

Pull Request validation

Failed

🔴 Review - Missing review from a member (1 required)

Success

🟡 CI - Waived

Auto Merge

Failed

🔴 Pull Request has unsupported target branch rhel-8.6.0, expected branches are: 'main,master'

Success

🟢 Pull Request is not marked as draft and it's not blocked by dont-merge label
🟢 Pull Request meet requirements, title has correct form
🟢 Pull Request meet requirements, mergeable is true
🟠 Pull Request doesn't meet requirements, mergeable_state is blocked

@mrc0mmand
Copy link
Member

mrc0mmand commented Nov 18, 2024 •
edited
Loading

Hey, could you also cherrypick following commits from the main branch to make CIs happy?

There might be more, but let's see if this is enough to make CIs happy for now.

@github-actions github-actions bot added the tracker/missing Formerly needs-bz label Nov 18, 2024
jamacku
jamacku reviewed Nov 18, 2024
View reviewed changes
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.

@tcornell-bus Please drop reference to old JIRA and Bugzilla in c1eab42 and
b4cc106 and reference the RHEL-65343

Please also add rhel-only: ci to the c1eab42 commit message.

@github-actions github-actions bot removed the tracker/missing Formerly needs-bz label Nov 18, 2024
@mrc0mmand
Copy link
Member

For the CIs a4e0b7a should get cherry-picked as well, to make (at least) GH actions happy.

Also, please use git cherry-pick -x when cherry-picking commits, it adds a reference to the original commit in a standardized format, so it then parse-able by our automation.

@tcornell-bus
Copy link
Author

Will do @mrc0mmand! Should I change "[tcornell: This has been cherry picked from downstream systemd-rhel8
commit hash]" to the standard "(cherry picked from commit hash)" for all the earlier commits as well?

@mrc0mmand
Copy link
Member

Will do @mrc0mmand! Should I change "[tcornell: This has been cherry picked from downstream systemd-rhel8 commit hash]" to the standard "(cherry picked from commit hash)" for all the earlier commits as well?

Yes, please, it'll make them consistent with the rest of the backports.

@tcornell-bus
journald: Increase stdout buffer size sooner, when almost full
2148cc7 
If the previous received buffer length is almost equal to the allocated
buffer size, before this change the next read can only receive a couple
of bytes (in the worst case only 1 byte), which is not efficient.

(cherry picked from commit 034e971)
(cherry picked from commit a0b5239)

Related: RHEL-65343
@tcornell-bus
journald: rework end of line marker handling to use a field table
3f140e5 
(cherry picked from commit 549b737)
(cherry picked from commit d8fabe7)

Related: RHEL-65343
@tcornell-bus
journald: use the fact that client_context_release() returns NULL
0a2207e 
(cherry picked from commit 020b4a0)
(cherry picked from commit cd85a65)

Related: RHEL-65343
@tcornell-bus
journald: rework pid change handling
cb1b457 
Let's introduce an explicit line ending marker for line endings due to
pid change.

Let's also make sure we don't get confused with buffer management.

(cherry picked from commit 45ba1ea)
(cherry picked from commit 538bd9b)

Resolves: RHEL-65343
@tcornell-bus
test: Add a test case for #15654
43e39e3 
(cherry picked from commit c11d8fd)
(cherry picked from commit e019afe)

Related: RHEL-65343
@tcornell-bus
test: Stricter test case for #15654 (Add more checks)
60b08bc 
Check:
 - There is only 3 messages logged with type stdout
 - Check all messages logged does not have new line: LINE_BREAK=eof
 - Check that the 3 messages are logged from a different PID
 - Check the 3 MESSAGE= content

(cherry picked from commit d38b3b7)
(cherry picked from commit afcfb65)

Related: RHEL-65343
@tcornell-bus
man: document the new _LINE_BREAK= type
6748107 
(cherry picked from commit a3d9aee)
(cherry picked from commit 2f55aea)

Related: RHEL-65343
@tcornell-bus
ci: point C8S containers to the Vault
0bac767 
Temporarily point repos in C8S containers to the Vault (since C8S is
EOL), until we figure out a _proper_ solution.

rhel-only: ci
(cherry picked from commit 3aae107)

Related: RHEL-65343
@tcornell-bus
Fix build with µhttpd 0.9.71
e432282 
The return type of callbacks was changed from int to an enum.

(cherry picked from commit d17eabb)
(cherry picked from commit ca86de2)

Related: RHEL-65343
@tcornell-bus
test: skip the symlink part of test_touch_file() in GH Actions
d64fc36 
Our (RHEL 8) touch_file() is not clever enough and does chmod() on a
symlink, which fails with EOPNOTSUPP on newer kernels. This is not an
issue on the RHEL 8 kernel, where doing chmod() on a symlink works
(albeit only on tmpfs) but in GH Actions we run in a container, and with
the underlying kernel doing chmod() on a symlink fails even on tmpfs:

RHEL 8:
~# mount -t tmpfs tmpfs /tmp
~# (cd /tmp; ln -s symlink dangling; ln -s /etc/os-release symlink)
~# (cd /var/tmp; ln -s symlink dangling; ln -s /etc/os-release symlink)
~# gcc -o main main.c -D_GNU_SOURCE
~# ./main /tmp/dangling
chmod(/proc/self/fd/3)=0 (0)
~# ./main /tmp/symlink
chmod(/proc/self/fd/3)=0 (0)
~# ./main /var/tmp/dangling
chmod(/proc/self/fd/3)=-1 (95)
~# ./main /var/tmp/symlink
chmod(/proc/self/fd/3)=-1 (95)

Newer kernel:
~# uname -r
6.7.4-200.fc39.x86_64
~# ./main /tmp/dangling
chmod(/proc/self/fd/3)=-1 (95)
~# ./main /tmp/symlink
chmod(/proc/self/fd/3)=-1 (95)
~# ./main /var/tmp/dangling
chmod(/proc/self/fd/3)=-1 (95)
~# ./main /var/tmp/symlink
chmod(/proc/self/fd/3)=-1 (95)

Backporting the necessary patches would be way too risky so late in the
RHEL 8 cycle, so let's just skip the offending test when running in GH
Actions. To do that we have to jump through a couple of hoops, since
RHEL 8 systemd can't detect docker. Oh well.

rhel-only: ci
(cherry picked from commit a4e0b7a)

Related: RHEL-65343
@mrc0mmand
Copy link
Member

I don't think the CentOS CI fail is related to this PR (due to C8S EOL we run the tests on C9S, so we're probably missing some patch(es)). I'll check this manually on RHEL 8 and waive the CI afterwards if everything works fine.

@mrc0mmand
Copy link
Member

Yup, the tests pass locally on RHEL 8, so this is an unrelated issue.

@github-actions github-actions bot added pr/needs-manual-merge and removed pr/needs-ci Formerly needs-ci labels Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamacku jamacku jamacku left review comments

@msekletar msekletar Awaiting requested review from msekletar

@dtardon dtardon Awaiting requested review from dtardon

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
ci-waived pr/needs-manual-merge pr/needs-review Formerly needs-review rhel-8.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tcornell-bus @mrc0mmand @jamacku