Allow containers to have their own default shells different from the host

[alvarlagerlof]

Describe the bug
Shell does not change from bash to zsh with neither chsh or usermod -s

Steps how to reproduce the behaviour

toolbox create --distro fedora --release 33
toolbox enter --release 33
echo $SHELL
sudo dnf install zsh
sudo usermod -s /bin/zsh alvar (or chsh, using `util-linux-user`)
exit
toolbox enter --release 33
echo $SHELL

Expected behaviour
$SHELL changes from bash to zsh

Actual behaviour
$SHELL stays at bash

Output of toolbox --version (v0.0.90+)
toolbox version 0.0.99.1

Toolbox package info (rpm -q toolbox)
toolbox-0.0.99.1-1.fc34.x86_64

Output of podman version

Version:      3.1.0
API Version:  3.1.0
Go Version:   go1.16
Built:        Tue Mar 30 15:29:36 2021
OS/Arch:      linux/amd64

Podman package info (rpm -q podman)
podman-3.1.0-1.fc34.x86_64

Info about your OS
Fedora Silverblue 34

[junjieyuan]

toolbox enter read host's SHELL environment variable as container's default shell.

Just a workaround:

SHELL=/bin/zsh toolbox enter

[alvarlagerlof]

toolbox enter read host's SHELL environment variable as container's default shell.

Just a workaround:

SHELL=/bin/zsh toolbox enter

Thank you, this works well for now and fits with the alias I already made. I would still like it to just work as expected in the future though.

[debarshiray]

I wonder if we need to add a toolbox shell command that:

• Is meant to be run only inside the container, like init-container.
• Looks at the default shell set in the container's /etc/passwd and tries to run it. If that's absent, falls back to whatever is specified by the host's SHELL environment variable, and finally to /bin/bash.

This might actually speed up our current fallback code path for shells used by toolbox enter. Instead of attempting multple podman exec calls, each of which can be quite slow, we would be doing everything inside a single podman exec invocation.

[alvarlagerlof]

I like that idea. I'm assuimg that shell command would be used to change the shell then?

[debarshiray]

Ultimately we need something to look at the user's default shell inside the container, and then run it, and it needs to happen as part of the toolbox enter sequence.

I suppose there are several ways to do this. One option is to invoke:

podman exec ... toolbox shell

... and have the toolbox shell command figure out the shell, and then exec it.

[alvarlagerlof]

I don't know the internals well enough yet to reason about that, but I do think that being able to run just one command to change it is important. If overriding the exising ones to change shell is possible I think that would be the best user experience.

[debarshiray]

If overriding the exising ones to change shell is possible
I think that would be the best user experience.

No, we wouldn't be able to update existing containers in this case, because of the immutability of OCI containers. They would have to be re-created.

[alvarlagerlof]

If overriding the exising ones to change shell is possible
I think that would be the best user experience.

No, we wouldn't be able to update existing containers in this case, because of the immutability of OCI containers. They would have to be re-created.

Ah right. Guess an external command is needed then. Maybe for future containers though?

[yann-soubeyrand]

I’m having a look at how to tackle this issue. I started coding the shell command as proposed and replaced the command used during toolbox enter here with toolbox sh. However, each time we enter the toolbox container, it seems that the user’s configured shell is reset to its configured shell on host at toolbox creation time:

toolbox/src/cmd/initContainer.go

Lines 239 to 257 in e80cba4

	if _, err := user.Lookup(initContainerFlags.user); err != nil {
	if err := configureUsers(initContainerFlags.uid,
	initContainerFlags.user,
	initContainerFlags.home,
	initContainerFlags.shell,
	initContainerFlags.homeLink,
	false); err != nil {
	return err
	}
	} else {
	if err := configureUsers(initContainerFlags.uid,
	initContainerFlags.user,
	initContainerFlags.home,
	initContainerFlags.shell,
	initContainerFlags.homeLink,
	true); err != nil {
	return err
	}
	}

What’s the purpose of reseting the shell here? I tried to remove this and it seems to work as expected though. Could you review my work in progress please?

[VarLad]

@alvarlagerlof I tried SHELL=fish toolbox enter and similarly for nushell. For both cases, the local directories aren't on PATH by default for some reason. Any way to keep the local directories on path, other than hardcoding them in shell's config files?

@yann-soubeyrand Does your PR have the same behavior? Are local directories like .local/bin still on PATH? Also, really waiting for that PR to get merged, any idea whats left in it? :D

[junjieyuan]

@alvarlagerlof I tried SHELL=fish toolbox enter and similarly for fish. For both cases, the local directories aren't on PATH by default for some reason. Any way to keep the local directories on path, other than hardcoding them in shell's config files?

@yann-soubeyrand Does your PR have the same behavior? Are local directories like .local/bin still on PATH? Also, really waiting for that PR to get merged, any idea whats left in it? :D

@VarLad You can using fish_add_path /path/to/add to add path to fish's PATH environment variable.

[VarLad]

@junjieyuan while I'm aware of that, I've a wider issue here #1076
I believe that .local/bin or local paths in general should be considered by application run with toolbox run

[debarshiray]

I’m having a look at how to tackle this issue. I started coding the shell command as proposed and replaced the command used during toolbox enter here with toolbox sh. However, each time we enter the toolbox container, it seems that the user’s configured shell is reset to its configured shell on host at toolbox creation time:

toolbox/src/cmd/initContainer.go

Lines 239 to 257 in e80cba4

	if _, err := user.Lookup(initContainerFlags.user); err != nil {
	if err := configureUsers(initContainerFlags.uid,
	initContainerFlags.user,
	initContainerFlags.home,
	initContainerFlags.shell,
	initContainerFlags.homeLink,
	false); err != nil {
	return err
	}
	} else {
	if err := configureUsers(initContainerFlags.uid,
	initContainerFlags.user,
	initContainerFlags.home,
	initContainerFlags.shell,
	initContainerFlags.homeLink,
	true); err != nil {
	return err
	}
	}

What’s the purpose of reseting the shell here? I tried to remove this and it seems to work as expected though. Could you review my work in progress please?

Maybe this commit offers some background?

When a Toolbx container is created and run for the first time, we want its initial state to match the host as much as possible. Let's call this is the factory state. As part of this factory state, Toolbx sets up the user's default shell inside the container to match that on the host.

We need to figure out a way to persistently record (across reboots) that the container was already set up as per this factory state, and that everything from now on is a local change made by the user. This way, toolbox init-container can avoid overwriting the user's local changes.

Removing this persistent record (whatever it is) should reset the container back to its factory state. This can be very helpful to debug problems and/or recover broken containers by going back to a known-good state.

Going by that terminology, one can say that Toolbx containers are somewhat stateless at the moment because toolbox init-container keeps enforcing the factory state.

[debarshiray]

If overriding the exising ones to change shell is possible
I think that would be the best user experience.

No, we wouldn't be able to update existing containers in this case, because of the immutability of OCI containers. They would have to be re-created.

Ah right. Guess an external command is needed then. Maybe for future containers though?

What I suggested would be purely an internal implementation change inside Toolbx.

As a user, you would use chsh(1) or usermod -s inside the container to change your shell. toolbox enter would start using that shell, as long as it's installed inside the container.

The new command that I suggested is not something that users would actually have to use. It will be an internal detail of the toolbox enter command.

[debarshiray]

I wonder if we need to add a toolbox shell command that:

Here is another interesting use-case for a toolbox shell command: #988

This might actually speed up our current fallback code path
for shells used by toolbox enter. Instead of attempting multple
podman exec calls, each of which can be quite slow, we would
be doing everything inside a single podman exec invocation.

These performance considerations are also valid for #988 because the alternative there is also to use an extra podman exec.