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

Prep patches for /opt and /usr/local support #4763

Merged
merged 4 commits into from
Jan 10, 2024

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Jan 9, 2024

Split out of #4728

@jlebon jlebon force-pushed the pr/opt-usrlocal-state-overlays-prep branch from 5048485 to ce848a7 Compare January 9, 2024 16:19
jlebon added 3 commits January 9, 2024 12:00
In a server-side compose, we call `rootfs_prepare_links()` twice:
once as part of unified core assembly, and once as part of final
postprocessing. In case of the former, all we really want are the `/var`
compat symlinks before running scriptlets.

Otherwise, let's reduce to a single call the place where we determine
the fate of `/usr/local` server-side. And certainly client-side, we
shouldn't touch it at all (unless it's part of e.g. some experimental
knob that purposely does more invasive things).

A follow up to this is to split out `/usr/local` handling entirely
into a separate function call, and only call that function in the
server-side compose path (and rename `rootfs_prepare_links()` to e.g.
`rootfs_prepare_compat_var_symlinks()`).
`systemd-tmpfiles` follows symlinks, so we can write `/usr/local/` here
instead of `/var/usrlocal/`. This doesn't matter right now, but will in
a future patch where `/usr/local` will no longer be a symlink.
@jlebon
Copy link
Member Author

jlebon commented Jan 9, 2024

Hmm, Prow is failing with

Adding rpm-ostree-0-integration.conf
error: Finalizing rootfs: fstatat(rpm-ostree-0-integration-opt-usrlocal.conf): No such file or directory
failed to execute cmd-build: exit status 1

which is really weird. I see it in the logs getting added to the cosa image:

2024-01-09T16:32:55.533771622Z [2/2] STEP 6/12: RUN rsync -rlv /cosa/component-install/ /
2024-01-09T16:32:56.109748513Z sending incremental file list
2024-01-09T16:32:56.114130699Z etc/rpm-ostreed.conf
2024-01-09T16:32:56.114237826Z usr/bin/rpm-ostree
...
2024-01-09T16:32:56.763218749Z usr/lib64/rpm-ostree/rpm-ostree-0-integration-opt-usrlocal.conf
2024-01-09T16:32:56.763279890Z usr/lib64/rpm-ostree/rpm-ostree-0-integration.conf

Seems to be working fine in CoreOS CI at least.

@jlebon jlebon force-pushed the pr/opt-usrlocal-state-overlays-prep branch from ce848a7 to 4a82e66 Compare January 9, 2024 17:24
@jlebon
Copy link
Member Author

jlebon commented Jan 9, 2024

Oh right, I think I understand this. Prow is unprivileged so uses supermin for the treecompose. But the new file isn't in the rpmdb so supermin doesn't know to pull it in. CoreOS CI also runs unprivileged, but we build a new rpm-ostree RPM there.

Until a new rpm-ostree is out with the new
`rpm-ostree-0-integration-opt-usrlocal.conf` file and part of cosa.

This duplicates the CoreOS CI test anyway.

Note we do still put rpm-ostree in the target system we compose and run
kola tests on.

See coreos#4763 (comment)
@jlebon jlebon force-pushed the pr/opt-usrlocal-state-overlays-prep branch from 4a82e66 to 6ccf858 Compare January 9, 2024 19:57
@HuijingHei
Copy link
Member

Overall, LGTM.
The failed CI is not related the PR, should be fixed by #4764

Copy link
Member

@HuijingHei HuijingHei left a comment

Choose a reason for hiding this comment

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

LGTM

@jlebon
Copy link
Member Author

jlebon commented Jan 10, 2024

Thanks, I restarted CI. It should pass now that #4764 is in.

@cgwalters cgwalters enabled auto-merge (rebase) January 10, 2024 16:01
@cgwalters cgwalters merged commit 1451675 into coreos:main Jan 10, 2024
17 checks passed
jlebon added a commit to jlebon/rpm-ostree that referenced this pull request Feb 6, 2024
Let's match CoreOS CI and also build RPMs here to not fall into issues
relating to supermin relying on the rpmdb:

coreos#4763 (comment)

This implicitly reverts 1451675.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants