Test conda env activation in container-canary tests

[jakirkham]

Fixes #857

For scripts like profile.sh to run (needed for Conda activation) we need to use a login shell. Hence the addition of -l (L) here.

Also interactive shells tend to behave a bit better from a user experience perspective. So have added -i too.

[jakirkham]

Shellcheck doesn't like this

In context/entrypoint.sh line 1:
#!/usr/bin/env bash -il
^---------------------^ SC2096 (error): On most OS, shebangs can only specify a single parameter.

However in this instance shellcheck is wrong. We make use of this regularly in conda-forge

Will silence this shellcheck error

[jacobtomlinson]

Are you sure? When I try and do this on my Ubuntu 24.04 desktop I get an error

#!/usr/bin/env bash -il
echo "Hello world"

$ chmod +x script.sh
$ ./script.sh
/usr/bin/env: ‘bash -il’: No such file or directory
/usr/bin/env: use -[v]S to pass options in shebang lines

I also see the same when doing this with a container entrypoint.

$ ls
Dockerfile  entrypoint.sh

entrypoint.sh

#!/usr/bin/env bash -il
exec $@

Dockerfile

FROM ubuntu

COPY entrypoint.sh /entrypoint.sh

ENTRYPOINT /entrypoint.sh

CMD "echo 'hello hello'"

$ docker build -t jacobtomlinson/foocont:latest .
[+] Building 0.1s (7/7) FINISHED
$ docker run jacobtomlinson/foocont:latest
/usr/bin/env: 'bash -il': No such file or directory
/usr/bin/env: use -[v]S to pass options in shebang lines

[gforsyth]

Welcome to an underspecified portion of the POSIX spec! There are no answers! (There are some answers)

Everything after the first word (space delimited) of the shebang gets parsed as a single argument and that handling can be kernel-dependent (yayyyy!!!!)

I think the more reliable but slightly less portable solution is to ensure that you are using a sufficiently new coreutils (can't remember the version) and then env supports the -S flag to explicitly allow multiple arguments and then you can do

#!/usr/bin/env -S bash -il

More references and examples here: https://www.gnu.org/software/coreutils/manual/html_node/env-invocation.html#g_t_002dS_002f_002d_002dsplit_002dstring-usage-in-scripts

[jacobtomlinson]

If you add -S does shellcheck still complain?

[jakirkham]

Thanks Gil! 🙏

Looks like coreutils version 8.30 added env -S

It quick run with shellcheck looks promising. Will see what CI says

[jakirkham]

This was done in PR: #860

[jameslamb]

Thanks for starting this, it's an interesting approach to the environment-activation problem. I'm leaving a blocking review because I have some concerns.

[jameslamb]

Just picking a different point here to not complicate the thread above the spelling of the arguments.... I don't think we want an interactive shell.

The rapidsai/base images are intended to also be useful for non-interactive processes, like model inference in this example: https://docs.rapids.ai/deployment/stable/examples/rapids-sagemaker-higgs/notebook/#testing-your-amazon-sagemaker-compatible-rapids-container-locally

Unless we can clearly state how adding -i resolves a specific issue, I think that part should be reverted out of this.

Making the entrypoint run these commands via a login shell is fine, but I think we should add some kind of tests in https://github.com/rapidsai/docker/tree/main/tests/container-canary confirming some observable properties of this so we don't accidentally introduce a regression in the future. For example, if the goal of the login shell is to activate the base conda environment, let's add a container canary test that that environment is activated by default.

[jakirkham]

Thanks James! 🙏

I don't have strong feelings about -i interactive. So have dropped it. Happy to pursue independently

Have added a test below. Would be interested in your feedback on that

[jakirkham]

@jameslamb is this what you had in mind for a test?

Basically CONDA_EXE is only set if the Conda environment is active. If the shell we are running here is run under a login shell (so we went through the entrypoint script), then CONDA_EXE will be set and inherited by this shell

[jameslamb]

Yep perfect! Exactly the type of thing I was looking for.

@jayavenkatesh19 is adding similar tests in #860 too, I'm fine with all of them existing. More validation that the image looks like we expect to it good.

[jameslamb]

Thanks for the changes, I'm good with this as a way to help with the startup environment for interactive uses.

I think we will want both this AND #860 (which fixes the problem generally for non-interactive use too).

[jameslamb]

#860 (now merged) pulled in the login-shell changes, so this now only contains an additional test on activation. I've updated the description accordingly.

[jameslamb]

/merge