Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test using functions instead of aliases for TCL #404

Closed
vsoch opened this issue Jun 9, 2021 · 7 comments
Closed

Test using functions instead of aliases for TCL #404

vsoch opened this issue Jun 9, 2021 · 7 comments

Comments

@vsoch
Copy link
Member

vsoch commented Jun 9, 2021

See discussion in cea-hpc/modules#400 (comment)

@eburgueno
Copy link

Hi @vsoch, I ran into this issue before. In my cluster I started defining aliases in my modulefiles to let users run containers without them knowing (much like you're trying to do here).

For example, an excerpt of a Tcl v3 (before set-function existed) modulefile for stacks:

set exelist "clone_filter denovo_map.pl kmer_filter process_radtags sstacks stacks-integrate-alignments count_fixed_catalog_snps.py gstacks phasedstacks process_shortreadsstacks-dist-extract tsv2bam cstacks integrate_alignments.py populations ref_map.pl stacks-gdb ustacks"

if { [ module-info mode load ] } {
    foreach {exename} $exelist {
        set-alias ${exename} "singularity exec ${prefix}/${appname}-${version}.sif ${exename} \"$\@\""
    }
}

One problem with aliases (as you discovered), is that they don't get exported to sub-shells, so we started doing this instead:

if { [ module-info mode load ] } {
    foreach {exename} $exelist {
        puts stdout "function ${exename} () { singularity exec ${prefix}/${appname}-${version}.sif ${exename} \"$\@\"; };"
        puts stdout "export -f ${exename};"
    }
}

After we upgraded to Tcl Modules v4, we then moved to set-function which get automatically exported in bash shells.

The problem is that set-function only works for sh-like shells. If you use Python, R, Perl, or some other interpreter; and want to load a modulefile the native way for that interpreter, set-function has no effect. I have not checked Lmod, but I suspect they will be the same. In our cluster, our users rely heavily on Python's Modulecmd and/or RLinuxModules. Sooner or later you might run into the same problem.

Our solution was to ditch aliases or functions altogether. Instead, I rely on Singularity's ability to execute image files directly and configure the runscript to exec $SINGULARITY_NAME if it exists. You can then rename the image itself as the command you want to run inside the image, or better yet, just use a symlink. This approach is documented here. This requires you to somehow maintain a manifest of which commands you can run inside each container, but it appears as you are already doing that so, problem solved?

Uh, not exactly. I understand that this will only work with Singularity. Docker and Podman can't use this. Perhaps a common approach would be to create a bunch of executable shims that would then run the appropriate longform command... food for thought.

In any case, swapping to set-function will probably be straightforward and low-hanging fruit. You might also declare support for non sh-like shells as out of scope. In any case, don't let my ramblings put you off from moving forward :)

@vsoch
Copy link
Member Author

vsoch commented Jul 22, 2021

Our solution was to ditch aliases or functions altogether. Instead, I rely on Singularity's ability to execute image files directly and configure the runscript to exec $SINGULARITY_NAME if it exists. You can then rename the image itself as the command you want to run inside the image, or better yet, just use a symlink.

Interesting idea! The problem here is that there can be 10s, even 100 specific commands for a single container, and a command isn't just an entrypoint, it can have options and more complexity too.

In any case, swapping to set-function will probably be straightforward and low-hanging fruit. You might also declare support for non sh-like shells as out of scope. In any case, don't let my ramblings put you off from moving forward :)

haha, thanks for the ramblings! I think my main concern now is just support - e.g., if someone loads shpc in a really old cluster and they don't have Tcl modules 4.0 it's not going to work. I'm also being conservative and waiting for someone to speak up / report an issue / etc. for the current approach not working. You know what they say, if it's not broken don't fix it! 😆

@eburgueno
Copy link

The problem here is that there can be 10s, even 100 specific commands for a single container, and a command isn't just an entrypoint, it can have options and more complexity too.

Yes, correct, which is why the %runscript is generic. It works just like busybox:

if [ -x /usr/local/bin/$SINGULARITY_NAME ]; then
    exec $SINGULARITY_NAME "$@"
fi

$ for myalias in $(cat list_of_commands); do
  ln -s container.sif $myalias
$ ls -l
lrwxrwxrwx. 1 user user       17 Mar  4 10:48 myalias1 -> container.sif
lrwxrwxrwx. 1 user user       17 Mar  4 10:48 myalias2 -> container.sif
-rwxr-xr-x. 1 user user 83996672 Mar  4 10:41 container.sif

$ ./myalias1 --help
help for myalias1

$ ./myalias2 --help
help for myalias2

The most I've done is around 25 symlinks, but I know there are things out there with more.

In any case, and as you say, this won't work for more complex things that are not simply /usr/local/bin/$BLAH (or some other path, you can adjust the runscript), and Docker and Podman can't use it anyway. Creating a bunch of shim scripts instead of symlinks might be better.

if someone loads shpc in a really old cluster and they don't have Tcl modules 4.0 it's not going to work.

You could test for the version of Tcl modules. If it's <4, a trick we used is to add a variable to set-alias, which forced it to be exported as a function. Rough pseudo-code:

if { $Tcl-version <4 } {
  set-alias myalias1 "$__VAR_UNDEFINED singularity exec blah"
} else {
  set-function myalias1 "singularity exec blah"
}

@marcodelapierre
Copy link
Contributor

Hi @vsoch, I ran into this issue before. In my cluster I started defining aliases in my modulefiles to let users run containers without them knowing (much like you're trying to do here).

For example, an excerpt of a Tcl v3 (before set-function existed) modulefile for stacks:

set exelist "clone_filter denovo_map.pl kmer_filter process_radtags sstacks stacks-integrate-alignments count_fixed_catalog_snps.py gstacks phasedstacks process_shortreadsstacks-dist-extract tsv2bam cstacks integrate_alignments.py populations ref_map.pl stacks-gdb ustacks"

if { [ module-info mode load ] } {
    foreach {exename} $exelist {
        set-alias ${exename} "singularity exec ${prefix}/${appname}-${version}.sif ${exename} \"$\@\""
    }
}

One problem with aliases (as you discovered), is that they don't get exported to sub-shells, so we started doing this instead:

if { [ module-info mode load ] } {
    foreach {exename} $exelist {
        puts stdout "function ${exename} () { singularity exec ${prefix}/${appname}-${version}.sif ${exename} \"$\@\"; };"
        puts stdout "export -f ${exename};"
    }
}

After we upgraded to Tcl Modules v4, we then moved to set-function which get automatically exported in bash shells.

The problem is that set-function only works for sh-like shells. If you use Python, R, Perl, or some other interpreter; and want to load a modulefile the native way for that interpreter, set-function has no effect. I have not checked Lmod, but I suspect they will be the same. In our cluster, our users rely heavily on Python's Modulecmd and/or RLinuxModules. Sooner or later you might run into the same problem.

Our solution was to ditch aliases or functions altogether. Instead, I rely on Singularity's ability to execute image files directly and configure the runscript to exec $SINGULARITY_NAME if it exists. You can then rename the image itself as the command you want to run inside the image, or better yet, just use a symlink. This approach is documented here. This requires you to somehow maintain a manifest of which commands you can run inside each container, but it appears as you are already doing that so, problem solved?

Uh, not exactly. I understand that this will only work with Singularity. Docker and Podman can't use this. Perhaps a common approach would be to create a bunch of executable shims that would then run the appropriate longform command... food for thought.

In any case, swapping to set-function will probably be straightforward and low-hanging fruit. You might also declare support for non sh-like shells as out of scope. In any case, don't let my ramblings put you off from moving forward :)

Hi @vsoch ,
I had the chance to progress on this one and do some tests, starting from @eburgueno 's comment above.

See the PR I have just opened: #447

As I wrote in the PR, I see the key advantage of enabling aliases for child shells in a deployment with Environment Modules, regardless of its version.
There's also a disadvantage: the syntax is not recognised by LMOD - LMOD users should use LUA modules though (lmod option in SHPC).

What do you think?

@vsoch
Copy link
Member Author

vsoch commented Oct 26, 2021

Hey @marcodelapierre I responded in the PR - but it sounds like there is minimal harm to supporting them here! Happy to include this update.

@marcodelapierre
Copy link
Contributor

good! :-)
and thanks again to @eburgueno for showing us his snippet.

@vsoch
Copy link
Member Author

vsoch commented Oct 27, 2021

Thank you @eburgueno ! This is now released 0.0.34

@vsoch vsoch closed this as completed Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants