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

Switch to unified cgroup mode for rc edition #18

Merged
merged 1 commit into from
Aug 25, 2023
Merged

Conversation

jandubois
Copy link
Member

@jandubois jandubois commented Aug 24, 2023

Buildkit v0.12.0+ seems to no longer work with the hybrid layout.

Real explanation remains elusive, as buildkitd 0.12.1 seems to work fine on Alpine in WSL2, which still uses the hybrid layout. But for Lima the unified layout avoid the issue.

@jandubois jandubois requested a review from mook-as August 24, 2023 22:55
Copy link

@mook-as mook-as left a comment

Choose a reason for hiding this comment

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

(Haven't tried it yet, will do so shortly.)

Please modify the commit message to describe why we're doing this. As it is (combined with the lack of a PR description) there's no hint, which would really hurt code archaeology.

@@ -32,6 +32,10 @@ makefile root:root 0644 "$tmp"/etc/hostname <<EOF
$HOSTNAME
EOF

if [ "$LIMA_CGROUP_MODE" != "hybrid" ]; then
sed -E "s/#(rc_cgroup_mode).*/\1=\"$LIMA_CGROUP_MODE\"/" /etc/rc.conf >"$tmp"/etc/rc.conf
Copy link

Choose a reason for hiding this comment

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

Wouldn't there technically be an issue if the builder and the build target uses different versions? (I don't think that's actually possible, though, so we should be fine… but I didn't spot any other copy-from-host things in this file at a quick glance.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I wondered about this too, but I don't know where also to get a matching copy of rc.conf. Lmk if you have any idea!

Most lines in the default rc.conf are just commented out defaults anyways, so not sure if this matters a lot:

# grep -E -v '^(#|$)' /etc/rc.conf
rc_tty_number=12
respawn_delay=2
respawn_max=5
respawn_period=1800

Buildkit v0.12.0+ seems to no longer work with the hybrid layout.

Real explanation remains elusive, as buildkitd 0.12.1 seems to work
fine on Alpine in WSL2, which still uses the hybrid layout. But for
Lima the unified layout avoid the issue.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
@jandubois jandubois merged commit c8a8a25 into main Aug 25, 2023
2 checks passed
@jandubois jandubois deleted the cgroup_mode branch August 25, 2023 22:06
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.

2 participants