89luca89 / distrobox

Use any linux distribution inside your terminal. Enable both backward and forward compatibility with software and freedom to use whatever distribution you’re more comfortable with. Mirror available at: https://gitlab.com/89luca89/distrobox
https://distrobox.it/
GNU General Public License v3.0
10.07k stars 415 forks source link

[Error] Distrobox is not location independent. #315

Open wamserma opened 2 years ago

wamserma commented 2 years ago

Describe the bug distrobox-enter fails if location of distrobox-scripts changed on disk.

To Reproduce This happened to me with Nix on Ubuntu when updating and the old distrobox version got GC'd from the nix store:

  1. Assume a nix-enabled system
  2. Get older distrobox: nix-store -r /nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15
  3. Create a box /nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15/bin/distrobox-create --name testbox --image ubuntu:20.04
  4. Enter box /nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15/bin/distrobox-enter testbox and leave again - works.
  5. Stop box /nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15/bin/distrobox-stop testbox
  6. Remove old distrobox: nix-store --delete /nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15
  7. Get newer distrobox: nix-store -r /nix/store/7nsxw7jggd33hrcbclgm5v13xds90r8w-distrobox-1.3.0
  8. Try to enter box /nix/store/7nsxw7jggd33hrcbclgm5v13xds90r8w-distrobox-1.3.0/bin/distrobox-enter testbox and observe reference to old version in failure message.
  9. Get older distrobox again: nix-store -r /nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15
  10. Try to enter box /nix/store/7nsxw7jggd33hrcbclgm5v13xds90r8w-distrobox-1.3.0/bin/distrobox-enter testbox and observe things work.

Expected behavior Distrobox should map its own location to a static path inside the container. (Distrobox already has great Nix support!)

Logs Podman logs do not provide any useful data.

Desktop (please complete the following information):

89luca89 commented 2 years ago

Right now, distrobox does indeed mount some part of itself inside the container in order to work:

It resolves the path in this way:

distrobox_entrypoint_path="$(cd "$(dirname "${0}")" && pwd)/distrobox-init"
distrobox_export_path="$(cd "$(dirname "${0}")" && pwd)/distrobox-export"
distrobox_hostexec_path="$(cd "$(dirname "${0}")" && pwd)/distrobox-host-exec"

It does this for installations which are not in PATH (eg I want to launch it from a dedicated directory)

This should indeed work if you use the nix symlinks instead of the hardpath. I assume when you install something with nix it will symlink (or hardlink? can't remember) to a more canonical path like ~/.nix-profile/bin and /nix/var/nix/profiles/default/bin or /run/current-system/sw/bin which will always point to latest version you install. This way the path does not change between versions

wamserma commented 2 years ago

It is symlinking, but ${0} refers to the running script and the symlinks have been resolved at this point, so this will be a path in the nix store.

Instead it should be sth. like

distrobox_entrypoint_path="/.host-distrobox/distrobox-init"
distrobox_export_path="/.host-distrobox/distrobox-export"
distrobox_hostexec_path="/.host-distrobox/distrobox-host-exec"

with distrobox-enter passing "$(dirname "${0}" as /.host-distroboxinto the container, which evaluates the script locations upon entering/starting the container vs. upon creating it.

89luca89 commented 2 years ago

@wamserma mmh I might need to investigate further, I'll set up a NixOS vm for this

Problem is that being a mount at creation time, we cannot evaluate later in the init script and we need it earlier than container creation itself, as the init itself is mounted like this

wamserma commented 2 years ago

Ok, this sounds more complicated than I expected it to be. Maybe it's better to solve this with a wrapper on the Nix side.

89luca89 commented 2 years ago

Yea the podman/docker mounts are "hard wired" so changing them needs a container recreation

I'll see if I can come up with something that does not resolve the symlinks, but I don't know if it's easy using ${0} is handy for using distrobox in non-in-path installations, and generally to not rely on PATH too much

89luca89 commented 2 years ago

I've solved this with my latest commit. This way, every time we start a stopped container, we do a cp of the utilities, This should cover this use case, as it will continue to work for running containers, and will fix stopped ones.

No need to recreate containers, just stop and start again

wamserma commented 2 years ago

Tested now that 1.4.0 was released. Unfortunately the fix does not work.

nix-store -r /nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15
/nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15/bin/distrobox-create --name testbox --image ubuntu:20.04
/nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15/bin/distrobox-enter testbox # and leave again - works.
/nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15/bin/distrobox-stop testbox
nix-store --delete /nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15
nix-store -r /nix/store/hkgf0b94858bswa854frkl5v50x4mir8-distrobox-1.4.0
/nix/store/hkgf0b94858bswa854frkl5v50x4mir8-distrobox-1.4.0/bin/distrobox-enter testbox # boom
/nix/store/hkgf0b94858bswa854frkl5v50x4mir8-distrobox-1.4.0/bin/distrobox-stop testbox
/nix/store/hkgf0b94858bswa854frkl5v50x4mir8-distrobox-1.4.0/bin/distrobox-rm testbox
89luca89 commented 2 years ago

I'm going to revert the commit that fixed this, as it's encountering an upstream docker bug anyway #399

profetik-777 commented 1 year ago

@89luca89 is this a legacy issue that no longer impacts new installs for nix/ubuntu systems? if so, should we just punt this and close-wont fix?

Or see if OP can do some more research regarding this: https://github.com/89luca89/distrobox/issues/315#issuecomment-1159487345

wamserma commented 1 year ago

This is still open. Tested with current stable nixpkgs and podman on Ubuntu 22.04.2 LTS.

OLD_DBOX=/nix/store/hcn0gjbb8xpl717pdp2bamzjcmspz3rp-distrobox-1.2.15
NEW_DBOX=/nix/store/n7znc92ix8dl8kmy07p4bfbmidmhd4f8-distrobox-1.4.2.1

nix-store -r "$OLD_DBOX"
"$OLD_DBOX/bin/distrobox-create" --name testbox --image ubuntu:20.04
"$OLD_DBOX/bin/distrobox-enter" testbox # and leave again - works.

"$OLD_DBOX/bin/distrobox-stop" testbox
nix-store --delete "$OLD_DBOX"
nix-store -r "$NEW_DBOX"
"$NEW_DBOX/bin/distrobox-enter" testbox # boom
"$NEW_DBOX/bin/distrobox-stop" testbox
"$NEW_DBOX/bin/distrobox-rm" testbox

Distrobox still doesn't update the paths in the image and expects an in-place upgrade of its scripts.

89luca89 commented 1 year ago

Distrobox still doesn't update the paths in the image and expects an in-place upgrade of its scripts.

This is needed to inject the init and other utils (host-exec, export)

wamserma commented 1 year ago

After investigating a bit more I think the root cause are the bind mounts used when the container is created. Unfortunately, there seems to be no official way to modify bind mounts of a container before starting it. Copying in the files as proposed in https://github.com/89luca89/distrobox/commit/52a34fbd52e1f0f035657b167ebe997d41db0993 might help with future containers, but does not help with existing ones, as starting the containers fails when the paths originally given in the --volume argument no longer exist.

A possible fix: If the container is not running, add a step before starting the container (e.g. at https://github.com/89luca89/distrobox/blob/a1c5095f7b41f8b92434ef5b744fbca68811e237/distrobox-enter#L453) that runs ${container_manager} inspect to extract the paths of the bind mounts. If those are not matching the paths of the current installation location, create a snapshot (commit) of the container, rename the container, create a new container from the snapshot with updated --volume args and the correct name, remove the snapshot.

89luca89 commented 1 year ago

Seems like an awful lot of work for each update (podman save/restore is not really a light thing)

I'll try to think for a faster/cheaper way of doing it, previously the docker/podman cp approach was creating problems because of docker+btrfs bugs so I needed to revert it

wamserma commented 1 year ago

Seems like an awful lot of work for each update (podman save/restore is not really a light thing)

But this is probably the only way to upgrade existing containers, as the bind mounts are recorded in the container metadata and the container will fail to start if the paths on the host are no longer present.

I'll try to think for a faster/cheaper way of doing it, previously the docker/podman cp approach was creating problems because of docker+btrfs bugs so I needed to revert it

This will improve the situation for all containers created after this change.