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

Add/sif variable for singularity #553

Merged
merged 21 commits into from
Jun 23, 2022
Merged

Conversation

vsoch
Copy link
Member

@vsoch vsoch commented Jun 4, 2022

This will close #549 if it works here - I think this was the last detail todo!

@vsoch
Copy link
Member Author

vsoch commented Jun 4, 2022

@marcodelapierre I'm not sure what I'm doing wrong, but I can't get it work. :/ So I think I'll need your help with this after all! Sorry I couldn't help.

@marcodelapierre
Copy link
Contributor

No worries @vsoch, I will have a look as soon as I can, hopefully in the next few days.

@marcodelapierre
Copy link
Contributor

Hi @vsoch , sorry it has taken me aaaaaaages to get back to this one.

I was having a look at those errors above, let me add some first thoughts.

  • Errors in the cases of podman/docker: I implemented the feature only for Singularity . Do you prefer to remove the podman/docker tests, or me to add it for Docker/Podman, too? (hopefully I can do it in non-Jurassic timespans)
  • Singularity errors 3x: I have noticed the happens with the tcsh shell, wondering whether/how this can play a role

@marcodelapierre
Copy link
Contributor

Ok, got one!

The Lmod error (empty container alias for Tcsh )is because I forgot to add the second definition, which indeed is required in the case of Csh/Tcsh.

Have committed the fix on my branch:
add/sif_variable_for_singularity...marcodelapierre:add/sif_variable_for_singularity

@marcodelapierre
Copy link
Contributor

I struggle to understand the other error, with Illegal variable name, for TCL + Tcsh.

I can't reproduce it here in a simple example taken from a python shpc container.

Next trials:

  • try and comment out echoing the variable in the test, just to see whether other errors pop-up, that maybe underlying (eg very curious to see whether the alias works).
  • try with echoing another variable from the module, eg SINGULARITY_SHELL?.

@marcodelapierre
Copy link
Contributor

All right, got also the other error, this is something that haunts me actually:

-          test -f $(python-container)
+          test -f `python-container`

The syntax $( ) is legal only for Bash, not Csh.
I have edited the test, to always the inverted ticks (Bash and Csh) ... to avoid any temptation.

@marcodelapierre
Copy link
Contributor

Have a look in my branch, it has those fixes:

https://github.com/marcodelapierre/singularity-hpc/tree/add/sif_variable_for_singularity

Oh! I am also suggesting this other edit in your test:

-          grep --quiet 'sif' container_output
+          grep --quiet '\.sif$' container_output

to be slightly more pedantic and look for the sif extension specifically.

@marcodelapierre
Copy link
Contributor

The only outstanding point:
shall we add a dedicated Docker/Podman alias + function, or ditch the corresponding tests.

If so, how shall we call the variable PODMAN_CONTAINER for both case, as for PODMAN_OPTS?
Also, is _CONTAINER / -container fine, or shall we use _IMAGE / -image in this case?
(you can tell it's getting late evening here, starting to be overly obsessive)

@vsoch
Copy link
Member Author

vsoch commented Jun 22, 2022

okay - just added your commits here!

shall we add a dedicated Docker/Podman alias + function, or ditch the corresponding tests.

Not until someone asks for it, imho.

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
Copy link
Member Author

vsoch commented Jun 23, 2022

@marcodelapierre are you able to see or download the output for the failed test? I think there is a bug with my GitHub (or browser)? because I'm getting this -> https://twitter.com/vsoch/status/1539722882984464384

I added the PODMAN_CONTAINER and the test should work, so I'm wanting to see what's going wrong (for docker/podman) so i can fix it... but I can't see it! lol! 😆 😭 <- laughing and crying moment.

@marcodelapierre
Copy link
Contributor

oh wow! I suspect .. you broke Github @vsoch!! 😆

interestingly, I can see the error message, e.g. tcl 4.7.1 docker ; I am using Safari on a Mac. Or maybe they solved it meanwhile...?

@vsoch
Copy link
Member Author

vsoch commented Jun 23, 2022

oh no, that means it's me! I've tried Firefox and Brave, so I could try Chrome as my last resort. I think it might be related to something with my ip-address because my linkedin site has been down (again spinning) for about a week, and that's true for any computer or browser in my apartment. When I connect to work VPN (on a work computer) it magically works. Of course it's a catch 22 because I'm not allowed to work on personal projects on a work computer, so I couldn't.

This might be really annoying, but could you show me one of the errors so I can try fixing it?

@marcodelapierre
Copy link
Contributor

I think the test fails because it Singularity specific, i.e. it looks for a .sif extension:

grep --quiet '\.sif$' container_output

Not sure what's the best way to test for the Docker image specification.. the prefix can be docker.io/, quay.io/, nvcr.io/, and so on ... so maybe grepping for \.io/ .
However, this might fail as I gather it is just a convention, and private registries might have a different naming?
So perhaps for docker/podman, we can only test that the output of the -container command is not empty...?

@vsoch
Copy link
Member Author

vsoch commented Jun 23, 2022

Ah of course! Thank you @marcodelapierre ! Yes I can think about how to fix this.

Update: chrome did not work, nor did disabling ad blockers (which wouldn't make sense since I've had them for years but worth a shot).

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
Copy link
Member Author

vsoch commented Jun 23, 2022

okay I found an old mac (and turned it on) and I can see the logs (thank goodness!)

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
Copy link
Member Author

vsoch commented Jun 23, 2022

oh man I am epically failing! @marcodelapierre do you see anything that I'm missing?

@vsoch
Copy link
Member Author

vsoch commented Jun 23, 2022

oh hmm docker is working but not podman...

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
Copy link
Member Author

vsoch commented Jun 23, 2022

I think it's maybe as you suggested, that we cannnot get the URI perfectly given a hash or registry is added.

@vsoch vsoch merged commit c335396 into main Jun 23, 2022
@vsoch vsoch deleted the add/sif_variable_for_singularity branch June 23, 2022 04:28
@vsoch
Copy link
Member Author

vsoch commented Jun 23, 2022

woohoo! Well, that was an adventure! 😆

@marcodelapierre
Copy link
Contributor

Thanks Vanessa!

@vsoch
Copy link
Member Author

vsoch commented Jun 23, 2022

Also @marcodelapierre I've recently bit the bullet and got my own cloud accounts (for development) so if you ever get any 'wouldn't it be cool if.." ideas working with GCP or AWS... you know who to ping!!

@marcodelapierre
Copy link
Contributor

Thanks for mentioning Vanessa! Mostly on-premises right now, but will keep it in mind :-)

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

Successfully merging this pull request may close these issues.

None yet

2 participants