lxd
lxd copied to clipboard
Better error handling in `lxc exec`
When a user attempts to execute a non-existing command or a file which isn't executable or anything else which causes an execve()
error, the behavior on the lxc exec
side is to not print anything on stdout or stderr and to just exit with the appropriate exit code.
stgraber@dakara:~$ lxc exec c1 non-existent
stgraber@dakara:~$ echo $?
127
stgraber@dakara:~$ lxc exec c1 /etc/passwd
stgraber@dakara:~$ echo $?
255
stgraber@dakara:~$ lxc exec v1 non-existent
stgraber@dakara:~$ echo $?
127
stgraber@dakara:~$ lxc exec v1 /etc/passwd
Error: fork/exec /etc/passwd: permission denied
stgraber@dakara:~$ echo $?
1
stgraber@dakara:~$
So first of all, we have a problem with the second v1
case here. We should make sure that lxd-agent
behaves the same as liblxc
and so returns no user-visible error and exits with 255.
Once we get the behavior properly lined up between containers and VMs, we should actually improve the user experience.
I believe we still want to stay away from putting anything on stdout or stderr through websocket as those should be reserved for actual output from the command. But I think it'd be fine to fail the operation and return an operation error instead.
This would then let lxc exec
show that error to the user while being able to differentiate (at the API level) such an exec error from the executed command printing the same error and exitting with the same exit code.
Sounds good!
@stgraber can i assign this issue to myself and give it a try?
Sure!
@root-ali how are you getting on with this? Thanks
We are in iran struggling with the fight with the regime and internet in our country under massive block from government and i cant test any of my code. I'm sorry about it but i searching for a way to test and create a pull request. @tomponline
Ok thanks. Dont worry about this!
@root-ali I can take this over if that is OK with you?
I think we need to return 126 for both containers and VM when a file is found but is not executable.
https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html
If a command is not found, the child process created to execute it returns a status of 127. If a command is found but is not executable, the return status is 126.
That looks like an lxc bug as lxc-attach
has the same behaviour.
@stgraber out of interest why does LXD have main_forkexec rather than using go-lxc's RunCommand*()
functions?
Is calling the liblxc's attach function not secure?
The concern tends to be around a lot of the attach logic requiring running functions like setns which can affect entire threads or processes and could seriously confuse the Go runtime.
That's why we use forkXYZ for any liblxc interaction which would result in changes to namespaces.