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

[Cherry-picked to v0.20.2] alpine: avoid wiping out writable host mounts under /home, etc. #2234

Merged
merged 1 commit into from
Mar 11, 2024
+81 −9
Merged

[Cherry-picked to v0.20.2] alpine: avoid wiping out writable host mounts under /home, etc. #2234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ jobs:
- opensuse.yaml
- experimental/net-user-v2.yaml
- docker.yaml
- ../hack/test-templates/alpine-9p-writable.yaml
steps:
- uses: actions/checkout@v4
with:
Expand Down
2 changes: 1 addition & 1 deletion hack/test-port-forwarding.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
foreach my $id (0..@test-1) {
my $test = $test[$id];
my $nc = "nc -l $test->{guest_ip} $test->{guest_port}";
if ($instance eq "alpine") {
if ($instance =~ /^alpine/) {
$nc = "nc -l -s $test->{guest_ip} -p $test->{guest_port}";
}

Expand Down
23 changes: 22 additions & 1 deletion hack/test-templates.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ declare -A CHECKS=(
["vmnet"]=""
["disk"]=""
["user-v2"]=""
["mount-path-with-spaces"]=""
)

case "$NAME" in
"alpine")
"alpine"*)
WARNING "Alpine does not support systemd"
CHECKS["systemd"]=
CHECKS["container-engine"]=
[ "$NAME" = "alpine-9p-writable" ] && CHECKS["mount-path-with-spaces"]="1"
;;
"k3s")
ERROR "File \"$FILE\" is not testable with this script"
Expand All @@ -59,6 +61,7 @@ case "$NAME" in
CHECKS["disk"]=1
CHECKS["snapshot-online"]="1"
CHECKS["snapshot-offline"]="1"
CHECKS["mount-path-with-spaces"]="1"
;;
"net-user-v2")
CHECKS["port-forwards"]=""
Expand Down Expand Up @@ -113,6 +116,11 @@ set -x
"${LIMACTL_CREATE[@]}" ${LIMACTL_CREATE_ARGS} "$FILE"
set +x

if [[ -n ${CHECKS["mount-path-with-spaces"]} ]]; then
mkdir -p "/tmp/lima test dir with spaces"
echo "test file content" >"/tmp/lima test dir with spaces/test file"
fi

INFO "Starting \"$NAME\""
set -x
if ! limactl start "$NAME"; then
Expand All @@ -126,6 +134,15 @@ limactl shell "$NAME" uname -a
limactl shell "$NAME" cat /etc/os-release
set +x

INFO "Testing that host home is not wiped out"
[ -e "$HOME/.lima" ]

if [[ -n ${CHECKS["mount-path-with-spaces"]} ]]; then
INFO 'Testing that "/tmp/lima test dir with spaces" is not wiped out'
[ "$(cat "/tmp/lima test dir with spaces/test file")" = "test file content" ]
[ "$(limactl shell "$NAME" cat "/tmp/lima test dir with spaces/test file")" = "test file content" ]
fi

INFO "Testing proxy settings are imported"
got=$(limactl shell "$NAME" env | grep FTP_PROXY)
# Expected: FTP_PROXY is set in addition to ftp_proxy, localhost is replaced
Expand Down Expand Up @@ -409,3 +426,7 @@ sleep 3

INFO "Deleting \"$NAME\""
limactl delete "$NAME"

if [[ -n ${CHECKS["mount-path-with-spaces"]} ]]; then
rm -rf "/tmp/lima test dir with spaces"
fi
22 changes: 22 additions & 0 deletions hack/test-templates/alpine-9p-writable.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Background: https://github.com/lima-vm/lima/pull/2234
# Should be tested on a Linux host
images:
- location: "https://github.com/lima-vm/alpine-lima/releases/download/v0.2.37/alpine-lima-std-3.19.0-x86_64.iso"
arch: "x86_64"
digest: "sha512:568852df405e6b9858e678171a9894c058f483df0b0570c22cf33fc75f349ba6cc5bb3d50188180d8c31faaf53400fe884ca3e5f949961b03b2bf53e65de88d7"
- location: "https://github.com/lima-vm/alpine-lima/releases/download/v0.2.37/alpine-lima-std-3.19.0-aarch64.iso"
arch: "aarch64"
digest: "sha512:3a4bd5ad0201f503e9bb9f3b812aa0df292e2e099148c0323d23244046ad199a2946ef9e0619fec28726bfdcc528233f43c3b4b036c9e06e92ac730d579f0ca3"

mountType: "9p"
mounts:
- location: "~"
writable: true
- location: "/tmp/lima test dir with spaces"
writable: true
- location: "/tmp/lima"
writable: true

containerd:
system: false
user: false
3 changes: 3 additions & 0 deletions hack/test-templates/test-misc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ images:

mounts:
- location: "~"
writable: true
- location: "/tmp/lima test dir with spaces"
writable: true
- location: "/tmp/lima"
writable: true

Expand Down
39 changes: 32 additions & 7 deletions pkg/cidata/cidata.TEMPLATE.d/boot/04-persistent-data-volume.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,36 @@ test -f /etc/alpine-release || exit 0
# Data directories that should be persisted across reboots
DATADIRS="/etc /home /root /tmp /usr/local /var/lib"

# Prepare mnt.sh (used for restoring mounts later)
echo "#!/bin/sh" >/mnt.sh
echo "set -eux" >>/mnt.sh
for DIR in ${DATADIRS}; do
while IFS= read -r LINE; do
[ -z "$LINE" ] && continue
MNTDEV="$(echo "${LINE}" | awk '{print $1}')"
# unmangle " \t\n\\#"
# https://github.com/torvalds/linux/blob/v6.6/fs/proc_namespace.c#L89
MNTPNT="$(echo "${LINE}" | awk '{print $2}' | sed -e 's/\\040/ /g; s/\\011/\t/g; s/\\012/\n/g; s/\\134/\\/g; s/\\043/#/g')"
# Ignore if MNTPNT is neither DIR nor a parent directory of DIR.
# It is not a parent if MNTPNT doesn't start with DIR, or the first
# character after DIR isn't a slash.
WITHOUT_DIR="${MNTPNT#"$DIR"}"
# shellcheck disable=SC2166
[ "$MNTPNT" != "$DIR" ] && [ "$MNTPNT" == "$WITHOUT_DIR" -o "${WITHOUT_DIR::1}" != "/" ] && continue
MNTTYPE="$(echo "${LINE}" | awk '{print $3}')"
[ "${MNTTYPE}" = "ext4" ] && continue
[ "${MNTTYPE}" = "tmpfs" ] && continue
MNTOPTS="$(echo "${LINE}" | awk '{print $4}')"
MNTPNT=${MNTPNT//\\/\\\\}
MNTPNT=${MNTPNT//\"/\\\"}
echo "mount -t \"${MNTTYPE}\" -o \"${MNTOPTS}\" \"${MNTDEV}\" \"${MNTPNT}\"" >>/mnt.sh
Copy link
Member

@jandubois jandubois Mar 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is fine to merge as-is. But if you want to support double-quotes and backslashes in mountpoints, then you could to

Suggested change
echo "mount -t \"${MNTTYPE}\" -o \"${MNTOPTS}\" \"${MNTDEV}\" \"${MNTPNT}\"" >>/mnt.sh
MNTPNT=${MNTPNT//\\/\\\\}
MNTPNT=${MNTPNT//\"/\\\"}
echo "mount -t \"${MNTTYPE}\" -o \"${MNTOPTS}\" \"${MNTDEV}\" \"${MNTPNT}\"" >>/mnt.sh

I've tested it with

$ MNTPNT=$'foo\\bar"baz\nfoo\\bar"baz'
$ echo "$MNTPNT"
foo\bar"baz
foo\bar"baz
$ MNTPNT=${MNTPNT//\\/\\\\}
$ MNTPNT=${MNTPNT//\"/\\\"}
$ echo "$MNTPNT"
foo\\bar\"baz
foo\\bar\"baz

If you do that, then you should probably also update the test case to use backslashes and quotes in the test directory name.

Just realized that lima-init.sh in alpine-lima doesn't support backslashes and quotes in mountpoint either right now: https://github.com/rancher-sandbox/alpine-lima/blob/5a55b61512a6e7b4affb8a9779d179f559b0ac18/lima-init.sh#L65-L66

So the tests would fail unless we fix this in alpine-lima as-well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, applied

# Before mv, unmount filesystems (virtiofs, 9p, etc.) below "${DIR}", otherwise host mounts will be wiped out
# https://github.com/rancher-sandbox/rancher-desktop/issues/6582
umount "${MNTPNT}" || exit 1
done </proc/mounts
done
chmod +x /mnt.sh

# When running from RAM try to move persistent data to data-volume
# FIXME: the test for tmpfs mounts is probably Alpine-specific
if [ "$(awk '$2 == "/" {print $3}' /proc/mounts)" == "tmpfs" ]; then
Expand Down Expand Up @@ -61,11 +91,6 @@ if [ "$(awk '$2 == "/" {print $3}' /proc/mounts)" == "tmpfs" ]; then
PART=$(lsblk --list /dev/"${DISK}" --noheadings --output name,type | awk '$2 == "part" {print $1}')
mkfs.ext4 -L data-volume /dev/"${PART}"
mount -t ext4 /dev/disk/by-label/data-volume /mnt/data
# Unmount all mount points under /tmp so we can move it to the data volume:
# "mount1 on /tmp/lima type 9p (rw,dirsync,relatime,mmap,access=client,trans=virtio)"
for MP in $(mount | awk '$3 ~ /^\/tmp\// {print $3}'); do
umount "${MP}"
done
# setup apk package cache
mkdir -p /mnt/data/apk/cache
mkdir -p /etc/apk
Expand All @@ -88,8 +113,8 @@ if [ "$(awk '$2 == "/" {print $3}' /proc/mounts)" == "tmpfs" ]; then
mount --bind /mnt/data"${DIR}" "${DIR}"
fi
done
# Make sure to re-mount any mount points under /tmp
mount -a
# Remount submounts on top of the new ${DIR}
/mnt.sh
# Reinstall packages from /mnt/data/apk/cache into the RAM disk
apk fix --no-network
fi
Loading