@jpetazzo Ok, added the quotes, seems to work correctly (fingers crossed :)).

BTW, I was reading one of your older blogposts (http://jpetazzo.github.io/2014/03/23/lxc-attach-nsinit-nsenter-docker-0-9/); any reason you decided to use nsenter, not nsinit?

@JessThrysoee Looks good. But if I understood your comment here https://github.com/jpetazzo/nsenter/issues/5#issuecomment-50095177 correctly, exec -cl should already clear the environment variables, so wouldn't env --ignore-environment be redundant?

@JessThrysoee was thinking along the line of; "$NSENTER" $OPTS sh -c "exec -cl [COMMAND]", so also use it to run sh ("$NSENTER" $OPTS sh -c "exec sh")

ototh, clearing the environment twice wouldn't hurt. LOL

Probably, the $HOME var should be somewhere added to OPTS

Yes I also didn't think clearing the environment twice mattered. The -c could probably be omitted and exec -l would be sufficient (I haven't testet that).

But, sh -c "exec -cl [COMMAND]" will not work, because the -cl opts are a bash ting.

$HOME should not be added to OPTS, at least not for the exec -cl case, the -l handles $HOME for us.

In fact the only reason to go with exec is so that argv0 will become -bash (note the leading dash) which will start bash as a login shell with $HOME set -- that's what this article tells us http://lists.gnu.org/archive/html/bug-bash/2014-01/msg00063.html

Because the bourne shell version of exec doesn't accept options, there is nothing gained by sh -c "exec sh", it will be the same as just calling sh directly (except for an extra unnecessary execve).

@JessThrysoee makes sense (you're a lot better than this than I am!).

Just did a quick search and apparently, /root should be present after all (I was assuming this could differ per-distro). If not, it's probably not a big issue.

The sh version will then be;

"$NSENTER" $OPTS HOME=/root sh 

Is it okay to use your suggestions in my PR? Or would you prefer to open your own PR? Feels like plagiarism 😺

@jpetazzo only thing I run into when using sh instead of bash is I get a "sh: 3: Cannot set tty process group (No such process)" if I exit the shell. Any thoughts? Could this be a Docker thing?

@thaJeztah No need for another PR :-)

The "Cannot set tty..." seems to be a /bin/dash warning (the /bin/sh on most linux systems).

The exec -l trick is still not quite enough. Even though PATH and HOME is set, they are not exported. So I have come up with a better solution which also makes the script simpler:

#!/bin/sh

NSENTER="$(dirname "$0")/nsenter"

if [ -z "$1" ]; then
    echo "usage: docker-enter <container id/name> <command to run default:bash>"
else
    PID=$(docker inspect --format {{.State.Pid}} "$1")
    if [ -z "$PID" ]; then
        exit 1
    fi
    shift

    OPTS="--target $PID --mount --uts --ipc --net --pid -- env --ignore-environment --"

    if [ -z "$1" ]; then
        "$NSENTER" $OPTS su - root
    else
        "$NSENTER" $OPTS "$@"
    fi
fi

su will use the default shell and make sure the environment is set up properly.

@JessThrysoee clever! Love it!

Maybe we should add some documentation inside the script to explain why su is used as it may confuse people.

I'll update the PR later and will think of documentation later

Updated the PR. Haven squashed yet, in case more improvements come 😄

Looks good, but I have just one more tweak :-)

I just read the su manpage and it says "clears all environment variables except for TERM". We really want to preserve TERM so here is yet another attempt:

#!/bin/sh

NSENTER=$(dirname "$0")/nsenter

if [ -z "$1" ]; then
    echo "usage: docker-enter <container id/name> <command to run default:bash>"
else
    PID=$(docker inspect --format "{{.State.Pid}}" "$1")
    if [ -z "$PID" ]; then
        exit 1
    fi
    shift

    OPTS="--target $PID --mount --uts --ipc --net --pid --"

    if [ -z "$1" ]; then
        # No command given.
        # Use su to clear all host environment variables except for TERM, 
        # initialize the environment variables HOME, SHELL, USER, LOGNAME, PATH,
        # and start a login shell.
        "$NSENTER" $OPTS su - root

    else
        # Use env to clear all host environment variables.
        "$NSENTER" $OPTS env --ignore-environment -- "$@"
    fi
fi

I also updated the comments with the information from the su manpage.

@JessThrysoee I'll update the PR when I'm back at my computer.

This has been very educational!

Only thing I'm not sure about is "what" to mention as default command here <command to run default:bash>; that description is inaccurate, but a full description will be difficult to provide, so maybe we should just leave that as a "known issue" 😉

How about?

echo "Usage: docker-enter CONTAINER [COMMAND [ARG]...]"
echo ""
echo "Enters the Docker CONTAINER and executes the specified COMMAND."
echo "If COMMAND is not specified, runs an interactive shell in CONTAINER.

The docker manpages just use CONTAINER for a container id or name.

@JessThrysoee guess something like that would work, was too focused on keeping it a single line (as it was now) but, that's nonsense. Guess it's getting late :)

Will update the PR in the morning, and clean up the commits if all things are ready.

Saw that quite a lot of work is in progress for implementing RunIn(), which will bring native nsinit/nsenter support to Docker

ping @jpetazzo

I think the new docker-enter has come quite a long way (the heavy lifting has been done by @JessThrysoee)

Do you want them rebased?

Thanks! You're the best, all of you :-)

I'm happy to merge this. And sorry for the delay; I was at OSCON the whole week, and couldn't check GitHub notifications as much as I wanted!

@jpetazzo no problem at all, due to your delay we were able to come up with a better version!

....just in time before docker RunIn() is implemented LOL