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

Several updates to config and environment settings #403

Merged
merged 6 commits into from
Jun 11, 2021

Conversation

vsoch
Copy link
Member

@vsoch vsoch commented Jun 8, 2021

This PR will address several new issues!

Signed-off-by: vsoch [email protected]

@vsoch vsoch force-pushed the updates/environment-settings branch from 4124487 to 4e7f409 Compare June 8, 2021 23:30
@marcodelapierre
Copy link
Contributor

Registry is now a list to support multiple registry locations

this works great!

@marcodelapierre
Copy link
Contributor

Config supports add/remove to append/delete from list to support

this one shows a weird behaviour:

when I add an entry, it seems like it reshuffles the order of items in the list, which should not happen, right?

Oh, I think I found at least some extra conditions for this error.

So, if the list is formatted without spaces before the dashes it works fine, by adding the add entry to the end of the list.

If the list has spaces before dashes, it reshuffles the items in the list.

Also Note: when adding to the list, maybe it should go as first item by default, rather than last? (if I am adding it, it probably has priority over the default one)

@marcodelapierre
Copy link
Contributor

Allow environment variables in settings

this works great!

@marcodelapierre
Copy link
Contributor

User settings file creation and use with shpc config inituser

works beautifully!

just a comment here, on editing the settings file.

once the inituser is created, should we provide a way to edit the central one if needed?
e.g. a --central (??) flag for sub-commands edit, add, remove, set and get?

This would be useful in deployments, even if users can't edit the central settings, at least they can inspect them..?
(oh, so probably it's only needed for edit and get)

Thoughts?

@vsoch
Copy link
Member Author

vsoch commented Jun 9, 2021

@marcodelapierre I think the list sorting is just because pyaml does that automatically - I'd need to create a special sorter to disable it.

@marcodelapierre
Copy link
Contributor

because the order has a meaning of priority, then I'd suggest having the default settings.yaml with no spaces before dashes, and then putting a warning about this (in the yaml and in the docos)

@marcodelapierre
Copy link
Contributor

can this help? yaml/pyyaml#143

@vsoch
Copy link
Member Author

vsoch commented Jun 9, 2021

@marcodelapierre I was just looking at that! The one constraint is:

This can be useful for python >= 3.7 to preserve key order.

Is that reasonable to set? I can do a try/except and then print a warning if it isn't supported. And I'm not sure this is for lists too.

@marcodelapierre
Copy link
Contributor

yeah saw it says dicts ... worth a quick go?

if it works, yes try/except is desirable, I know of old clusters that still have 3.6.x ...

@vsoch
Copy link
Member Author

vsoch commented Jun 9, 2021

once the inituser is created, should we provide a way to edit the central one if needed? e.g. a --central (??) flag for sub-commands edit, add, remove, set and get?

I can add this, but I'm not sure how useful it is if the user-config over-rides it.

@vsoch vsoch force-pushed the updates/environment-settings branch 3 times, most recently from 1b6ed87 to 38a28b0 Compare June 9, 2021 15:13
@vsoch
Copy link
Member Author

vsoch commented Jun 9, 2021

okay @marcodelapierre please see updates! 38a28b0.

  • --central is added to edit/interact with the default config
  • podman/docker now pull digests (and tag them after so they can be interacted with further)
  • we try/except to use sort_keys on saving the yaml.
  • I fixed the typos you found in module files
  • new entries are added to the front of the list

@vsoch vsoch force-pushed the updates/environment-settings branch from 38a28b0 to cffe038 Compare June 9, 2021 15:39
@vsoch
Copy link
Member Author

vsoch commented Jun 9, 2021

@marcodelapierre the docker/podman rmi is updated so the tag and image is actually deleted :)

@vsoch vsoch force-pushed the updates/environment-settings branch from cffe038 to d129dad Compare June 9, 2021 23:08
@marcodelapierre
Copy link
Contributor

I can add this [--central], but I'm not sure how useful it is if the user-config over-rides it.

Let me share with you my evil deployment setup.

There's one SHPC installation system wide, managed by the sys. admin.

Sys. admin. customises the "central" settings using user-specific env variables (on the lines of $USER ..) for at least containers and modules, so that each user is able to install container modules in their own specific directory out-of-the-box, without the need to customise the settings (unless they really want/need to).

But then sys. admin. still has to be able to install container modules in a system wide location...and can achieve that thanks to their SHPC user settings.

To summarise, in this deployment then, the "user" settings are used somewhat counter-intuitively for system-wide installations, and the "central" ones for user-specific installations. This flipping allows SHPC to work out-of-the-box for users, without them having to customise settings in most cases.

Sys. admins. might still need to edit both the "central" settings and their own "user" ones, hence the --central flag turns out handy for them :-)

@marcodelapierre
Copy link
Contributor

Now proceeding with tests of the new commits above

@marcodelapierre
Copy link
Contributor

this one's fine: --central is added to edit/interact with the default config

@marcodelapierre
Copy link
Contributor

with the ordering of the registry list, I am still having issues (python 3.8.2):

$ shpc config get registry
registry                       ['/astro/pawsey0001/mdelapierre/PSS/shpc-tests/shpc-pr-vsoch/shpc_fixes_10jun_garrawarla/registry']
$ shpc config add registry:/zio1
Added registry to /zio1
$ shpc config get registry
registry                       ['/zio1', '/astro/pawsey0001/mdelapierre/PSS/shpc-tests/shpc-pr-vsoch/shpc_fixes_10jun_garrawarla/registry']
$ shpc config add registry:/lella2
Added registry to /lella2
$ shpc config get registry
registry                       ['/astro/pawsey0001/mdelapierre/PSS/shpc-tests/shpc-pr-vsoch/shpc_fixes_10jun_garrawarla/registry', '/zio1', '/lella2']

@vsoch
Copy link
Member Author

vsoch commented Jun 10, 2021

What should we try? It looks like sort_keys didn’t work. I’ll google around for a custom save function.

@marcodelapierre
Copy link
Contributor

Also, note how CLI edits to registry "eat up" some of the next lines (the comments ahead of module_base, and a blank line):

BEFORE

# Registry Recipes (order of paths here is honored for search path))
registry:
 - $root_dir/registry

# Lmod or Environment Modules settings 
# The install directory for modules. Defaults to the install directory/modules
module_base: $root_dir/modules

AFTER

# Registry Recipes (order of paths here is honored for search path))
registry:
- $root_dir/registry
- /zio1
- /lella2
module_base: $root_dir/modules

@vsoch
Copy link
Member Author

vsoch commented Jun 10, 2021

That is strange - it must be a bug with the library ruamel. I chose it because it explicitly allowed me to save those comments!

@marcodelapierre
Copy link
Contributor

What should we try? It looks like sort_keys didn’t work. I’ll google around for a custom save function.

Not really a solution, but just a patch:

As it was before, ie without sort_keys, it was working as far as there was no empty space before the dashes .. so we could put a warning in the yml and docos..

I'll get back with a proper solution if I find one!

@vsoch vsoch force-pushed the updates/environment-settings branch from c538784 to eb43118 Compare June 10, 2021 04:19
@vsoch vsoch mentioned this pull request Jun 11, 2021
@marcodelapierre
Copy link
Contributor

Re: podman uninstall

I am still getting a different behaviour compared to yours, in that I always have a spare entry in my podman images, just for the digest image tag, even after the installation step (my output vs yours).

This is with the latest commit 67517dc, changing only container_base and container_tech.

However, there might be an important detail, I am using a very old version of Podman, 1.6.2. I was lazy when I setup the VM and got whatever version was in the apt repo. Maybe that explains things..
If it works with your setup (what version of Podman?) maybe you can ignore my report for this one!

ubuntu@u18:~/shpc_fixes_11jun$ podman images
REPOSITORY   TAG   IMAGE ID   CREATED   SIZE
ubuntu@u18:~/shpc_fixes_11jun$ shpc install biocontainers/samtools:v1.9-4-deb_cv1
Trying to pull docker.io/biocontainers/samtools@sha256:da61624fda230e94867c9429ca1112e1e77c24e500b52dfc84eaf2f5820b4a2a...
Getting image source signatures
Copying blob 94d6a239eb0e done
Copying blob 732b6ad56c57 done
Copying blob e8e87313e9cb done
Copying blob f464fdba1cc8 done
Copying blob 478cd0aa93c0 done
Copying blob 1badce2e48be done
Copying blob a955c5be8430 done
Copying config f210eb625b done
Writing manifest to image destination
Storing signatures
f210eb625ba612c85b7e721490130b2afead736d604032b7597e47758c3c6c4d
Module biocontainers/samtools:v1.9-4-deb_cv1 was created.
ubuntu@u18:~/shpc_fixes_11jun$ podman images
REPOSITORY                         TAG                                                                       IMAGE ID       CREATED         SIZE
docker.io/biocontainers/samtools   sha256:da61624fda230e94867c9429ca1112e1e77c24e500b52dfc84eaf2f5820b4a2a   f210eb625ba6   19 months ago   686 MB
docker.io/biocontainers/samtools   v1.9-4-deb_cv1                                                            f210eb625ba6   19 months ago   686 MB
ubuntu@u18:~/shpc_fixes_11jun$ shpc uninstall biocontainers/samtools:v1.9-4-deb_cv1
$container_base/biocontainers/samtools/v1.9-4-deb_cv1 does not exist.
Are you sure you want to uninstall $module_base/biocontainers/samtools/v1.9-4-deb_cv1, and all content below it? y
$module_base/biocontainers/samtools/v1.9-4-deb_cv1 and all subdirectories been removed.
ubuntu@u18:~/shpc_fixes_11jun$ podman images
REPOSITORY                         TAG                                                                       IMAGE ID       CREATED         SIZE
docker.io/biocontainers/samtools   sha256:da61624fda230e94867c9429ca1112e1e77c24e500b52dfc84eaf2f5820b4a2a   f210eb625ba6   19 months ago   686 MB
ubuntu@u18:~/shpc_fixes_11jun$ podman version
Version:            1.6.2
RemoteAPI Version:  1
Go Version:         go1.10.4
OS/Arch:            linux/amd64

@vsoch
Copy link
Member Author

vsoch commented Jun 11, 2021

oh interesting! I'm using a much newer one:

podman version 3.2.0-rc3

Maybe the change is with this PR here: containers/podman#7155 ?

I think that might be it because in testing I noticed that the container is only untagged unless I use --force. Even if that's not the right PR, I would say that merging here will still put us in a better state than not - I don't think the rmi functionality was working for you on the current branch? I'd want to get more user feedback about if this works / doesn't work for them (it works for me).

Just curious - when you install and do podman rmi biocontainers/samtools:v1.9-4-deb_cv1 (and then check) and then podman rmi --force biocontainers/samtools:v1.9-4-deb_cv1 (and check again) do you get a different outcome?

@marcodelapierre
Copy link
Contributor

marcodelapierre commented Jun 11, 2021

Can I ask you to test shpc test biocontainers/samtools with your version?

My run succeeds, but it has leftovers upon completion, with both the digest and the tag images:

ubuntu@u18:~/shpc_fixes_11jun$ shpc test biocontainers/samtools
Trying to pull docker.io/biocontainers/samtools@sha256:da61624fda230e94867c9429ca1112e1e77c24e500b52dfc84eaf2f5820b4a2a...
Getting image source signatures
Copying blob 94d6a239eb0e done
Copying blob 732b6ad56c57 done
Copying blob 478cd0aa93c0 done
Copying blob e8e87313e9cb done
Copying blob 1badce2e48be done
Copying blob f464fdba1cc8 done
Copying blob a955c5be8430 done
Copying config f210eb625b done
Writing manifest to image destination
Storing signatures
f210eb625ba612c85b7e721490130b2afead736d604032b7597e47758c3c6c4d
Module biocontainers/samtools:v1.9-4-deb_cv1 was created.
Testing load of biocontainers/samtools:v1.9-4-deb_cv1
ubuntu@u18:~/shpc_fixes_11jun$ podman images
REPOSITORY                         TAG                                                                       IMAGE ID       CREATED         SIZE
docker.io/biocontainers/samtools   sha256:da61624fda230e94867c9429ca1112e1e77c24e500b52dfc84eaf2f5820b4a2a   f210eb625ba6   19 months ago   686 MB
docker.io/biocontainers/samtools   v1.9-4-deb_cv1                                                            f210eb625ba6   19 months ago   686 MB

based on my output of shpc uninstall, I would have expected the tag image to be gone.

@vsoch
Copy link
Member Author

vsoch commented Jun 11, 2021

@marcodelapierre that's a bug I think - I need to add an uninstall to the module. Before it was simple enough to just delete the entire testing root, but now with podman we need to run uninstall to delete containers too!

As another check I can add a podman images to the test file.

@vsoch vsoch force-pushed the updates/environment-settings branch from 0576c04 to e8f37df Compare June 11, 2021 02:24
@marcodelapierre
Copy link
Contributor

Thanks for checking with your latest version, sure I totally agree we can ignore this behaviour from such an old version :-)

Just curious - when you install and do podman rmi biocontainers/samtools:v1.9-4-deb_cv1 (and then check) and then podman rmi --force biocontainers/samtools:v1.9-4-deb_cv1 (and check again) do you get a different outcome?

Getting the same behaviour - the digest is always left there:

ubuntu@u18:~/shpc_fixes_11jun$ podman images
REPOSITORY                         TAG                                                                       IMAGE ID       CREATED         SIZE
docker.io/biocontainers/samtools   sha256:da61624fda230e94867c9429ca1112e1e77c24e500b52dfc84eaf2f5820b4a2a   f210eb625ba6   19 months ago   686 MB
docker.io/biocontainers/samtools   v1.9-4-deb_cv1                                                            f210eb625ba6   19 months ago   686 MB
ubuntu@u18:~/shpc_fixes_11jun$ podman rmi --force biocontainers/samtools:v1.9-4-deb_cv1
Untagged: docker.io/biocontainers/samtools:v1.9-4-deb_cv1
ubuntu@u18:~/shpc_fixes_11jun$ podman images
REPOSITORY                         TAG                                                                       IMAGE ID       CREATED         SIZE
docker.io/biocontainers/samtools   sha256:da61624fda230e94867c9429ca1112e1e77c24e500b52dfc84eaf2f5820b4a2a   f210eb625ba6   19 months ago   686 MB

@marcodelapierre
Copy link
Contributor

Very final comment for this PR -- only if easy to implement: #406

@vsoch vsoch force-pushed the updates/environment-settings branch 8 times, most recently from e8b4ca8 to 256dbd1 Compare June 11, 2021 02:55
@vsoch
Copy link
Member Author

vsoch commented Jun 11, 2021

Phew finally! I was struggling with GitHub action syntax. @marcodelapierre since you've confirmed the issue with podman is on your host (and it works on mine):

$ podman pull python:latest
✔ docker.io/library/python:latest
Trying to pull docker.io/library/python:latest...
Getting image source signatures
Copying blob d960726af2be done  
Copying blob 8962bc0fad55 done  
Copying blob e8d62473a22d done  
Copying blob 532f6f723709 done  
Copying blob 65d943ee54c1 done  
Copying blob 1334e0fe2851 done  
Copying blob aec2e3a89371 done  
Copying blob 062ada600c9e done  
Copying blob 1ec7c3bcb4b2 done  
Copying config 5b3b4504ff done  
Writing manifest to image destination
Storing signatures
5b3b4504ff1f7b859dbc5d7fb86f4afc644be62f99b8ced636fbca64c8a6c2de

# python is there!
$ podman images
REPOSITORY                TAG     IMAGE ID      CREATED      SIZE
docker.io/library/python  latest  5b3b4504ff1f  2 weeks ago  909 MB
docker.io/vanessa/salad   <none>  daf4e5a88131  2 years ago  8 MB

$ podman rmi python:latest
Untagged: docker.io/library/python:latest
Deleted: 5b3b4504ff1f7b859dbc5d7fb86f4afc644be62f99b8ced636fbca64c8a6c2de

# python is no longer there!
$ podman images
REPOSITORY               TAG     IMAGE ID      CREATED      SIZE
docker.io/vanessa/salad  <none>  daf4e5a88131  2 years ago  8 MB

I think we can disregard that issue. Are we ready for merge and release?

@marcodelapierre
Copy link
Contributor

Thanks @vsoch , I am going ahead with one final test then

@marcodelapierre
Copy link
Contributor

marcodelapierre commented Jun 11, 2021

So, when testing with biocontainers/samtools (one version) it worked as expected (digests still there - ignore),

however when testing with a modified python with two versions it also left one tag on the system.

$ podman images
REPOSITORY                 TAG                                                                       IMAGE ID       CREATED       SIZE
docker.io/library/python   sha256:1e554b804d653d5d49271bd15c6d53665be5f3097a9fc14deb2c7405e10fc406   609da079b03a   2 weeks ago   120 MB
docker.io/library/python   sha256:20bcb1872e105bc06d9e78ef90844dd21f5f38356eca5807c6e0c2ff0bcf4279   5b3b4504ff1f   2 weeks ago   909 MB
docker.io/library/python   3.9.5                                                                     5b3b4504ff1f   2 weeks ago   909 MB

@vsoch
Copy link
Member Author

vsoch commented Jun 11, 2021

Can you give me full commands to reproduce?

@marcodelapierre
Copy link
Contributor

edited registry/python/container.yaml:

docker: python
url: https://hub.docker.com/_/python
maintainer: '@vsoch'
description: An interpreted, high-level and general-purpose programming language.
latest:
  3.9.5-slim: sha256:1e554b804d653d5d49271bd15c6d53665be5f3097a9fc14deb2c7405e10fc406
tags:
  3.9.5: sha256:20bcb1872e105bc06d9e78ef90844dd21f5f38356eca5807c6e0c2ff0bcf4279
  3.9.5-slim: sha256:1e554b804d653d5d49271bd15c6d53665be5f3097a9fc14deb2c7405e10fc406
filter:
- 3.9.*
aliases:
  python: /usr/local/bin/python

and then

shpc test python

@vsoch vsoch force-pushed the updates/environment-settings branch from 256dbd1 to c66051b Compare June 11, 2021 03:50
@vsoch
Copy link
Member Author

vsoch commented Jun 11, 2021

I can't do a test on my local machine, but test does run uninstall, so I can mimic that. When I do that, I can't reproduce, either referencing the repository name python or just one tag.

$ shpc install --container-tech podman python:3.9.5-alpine
Trying to pull docker.io/library/python@sha256:02311d686cd35b0f838854d6035c679acde2767a4fd09904e65355fbd9780f8a...
Getting image source signatures
Copying blob 540db60ca938 done  
Copying blob 0faf4e7f2207 done  
Copying blob f9ef3a05a91e done  
Copying blob 629719f9106a done  
Copying blob d037ddac5dde done  
Copying config 2d64a2341b done  
Writing manifest to image destination
Storing signatures
2d64a2341b7c19d37cc1262280756014b071eb187e4605c6cc9f749311a3d0b8
Module python:3.9.5-alpine was created.

$ shpc install --container-tech podman python:3.9.2-alpine
Trying to pull docker.io/library/python@sha256:f046c06388c0721961fe5c9b6184d2f8aeb7eb01b39601babab06cfd975dae01...
Getting image source signatures
Copying blob 1d917aee477c done  
Copying blob 83ca130858d1 done  
Copying blob ca3cd42a7c95 done  
Copying blob 7bc50e7d1ae3 done  
Copying blob 3d2c0e52073b done  
Copying config 1238239159 done  
Writing manifest to image destination
Storing signatures
po123823915946cc85077b22e1672d38d449c988cbb4d40335da69e962d0e83204
dmanModule python:3.9.2-alpine was created.

$ podman images
REPOSITORY                TAG           IMAGE ID      CREATED       SIZE
docker.io/library/python  3.9.5-alpine  2d64a2341b7c  2 weeks ago   48 MB
docker.io/library/python  3.9.2-alpine  123823915946  2 months ago  47.6 MB
docker.io/vanessa/salad   <none>        daf4e5a88131  2 years ago   8 MB

$ shpc uninstall --container-tech podman python
Are you sure you want to uninstall $container_base/python, and all content below it? y
$container_base/python and all subdirectories been removed.
Are you sure you want to uninstall $module_base/python, and all content below it? y
$module_base/python and all subdirectories been removed.

$ podman images
REPOSITORY               TAG     IMAGE ID      CREATED      SIZE
docker.io/vanessa/salad  <none>  daf4e5a88131  2 years ago  8 MB

$ shpc install --container-tech podman python:3.9.2-alpine
Trying to pull docker.io/library/python@sha256:f046c06388c0721961fe5c9b6184d2f8aeb7eb01b39601babab06cfd975dae01...
Getting image source signatures
Copying blob ca3cd42a7c95 done  
Copying blob 3d2c0e52073b done  
Copying blob 1d917aee477c done  
Copying blob 83ca130858d1 done  
Copying blob 7bc50e7d1ae3 done  
Copying config 1238239159 done  
Writing manifest to image destination
Storing signatures
123823915946cc85077b22e1672d38d449c988cbb4d40335da69e962d0e83204
Module python:3.9.2-alpine was created.

$ shpc install --container-tech podman python:3.9.5-alpine
Trying to pull docker.io/library/python@sha256:02311d686cd35b0f838854d6035c679acde2767a4fd09904e65355fbd9780f8a...
Getting image source signatures
Copying blob d037ddac5dde done  
Copying blob 0faf4e7f2207 done  
Copying blob 540db60ca938 done  
Copying blob f9ef3a05a91e done  
Copying blob 629719f9106a done  
Copying config 2d64a2341b done  
Writing manifest to image destination
Storing signatures
2d64a2341b7c19d37cc1262280756014b071eb187e4605c6cc9f749311a3d0b8
Module python:3.9.5-alpine was created.

$ shpc uninstall --container-tech podman python:3.9.5-alpine
$container_base/python/3.9.5-alpine does not exist.
Are you sure you want to uninstall $module_base/python/3.9.5-alpine, and all content below it? y
$module_base/python/3.9.5-alpine and all subdirectories been removed.
(env) (base) vanessa@vanessa-ThinkPad-T490s:~/Desktop/Code/singularity-hpc$ podman images
REPOSITORY                TAG           IMAGE ID      CREATED       SIZE
docker.io/library/python  3.9.2-alpine  123823915946  2 months ago  47.6 MB
docker.io/vanessa/salad   <none>        daf4e5a88131  2 years ago   8 MB

It could be that for test we need to just refer to the general namespace and not the tag, I'll try updating to do that (although I can't test here).

@vsoch
Copy link
Member Author

vsoch commented Jun 11, 2021

@marcodelapierre I updated the test. Sorry if it doesn't work ;/

@marcodelapierre
Copy link
Contributor

YESSSSS! it works!

well, to me this can be merged!

@vsoch
Copy link
Member Author

vsoch commented Jun 11, 2021

Music to my ears! 🎶 Hooray!! :D

@vsoch vsoch merged commit 7387d95 into main Jun 11, 2021
@vsoch vsoch deleted the updates/environment-settings branch June 11, 2021 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants