Add/remote registry #576

vsoch · 2022-08-03T01:56:24Z

This is a huge PR to replace #567 that will make the default registry remote, and add the concept of a registry backend, either remote (GitHub) or a local filesystem. I've done careful work to ensure all tests try both cases, but likely this will need a lot of manual testing to be double sure I didn't break anything!

this still needs tests written - I think I will need to make an external remote registry with some arbitrary change to test, OR just test the interaction with the remote provider and handle the updates via local test files Signed-off-by: vsoch <vsoch@users.noreply.github.com>

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

I am wondering if the upgrade function should be done between two registries, where the registry being upgraded is required to be a filesystem? Signed-off-by: vsoch <vsoch@users.noreply.github.com>

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

vsoch · 2022-08-03T02:12:25Z

@muffato @marcodelapierre @surak I pulled it off!

muffato · 2022-08-07T09:26:02Z

Hi @vsoch . That looks very impressive 👏🏼 ! I've started looking at the code, but I haven't run any tests yet. I'll try to get to it this week if that's OK ?

vsoch · 2022-08-07T10:26:40Z

Definitely OK by me!

muffato

Looks very good to me. sync-registry is a great addition, and I think it can simplify our workflow tremendously. But most importantly, the repository feels so clean with the container.yaml out of the way 😌 !

I've tested a few things (installing and adding a container) and left some comments.

docs/getting_started/user-guide.rst

shpc/main/registry/github.py

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

vsoch · 2022-08-18T23:07:22Z

@muffato I've done various tweaks (and ran isort/black) a55c243.

For next steps:

I want to add GitLab but we will need a template first. I'm going to make a new issue for this that I will tackle after the initial work here. update, added and done
I want to add functionality for the registry file - can you give me the example you were wanting to use (or do we need another remote to be made first?) (done, I made the GitLab registry and could use that).
Can you give me the sync commands that borked your registry? I am going to see if I can add more logic to only consider container modules (and a test for it).

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

vsoch · 2022-08-19T03:39:21Z

okay - and got started on creating a GitLab equivalent! https://gitlab.com/singularityhub/shpc-registry supposedly pages take 30 minutes to deploy... so not sure if I did it right yet! I also realized default branches there are "master" so I added a branch arg to the docgen command here.

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

vsoch · 2022-08-19T04:36:38Z

okay Gitlab registry (and remote here) is added, and I un-broke the CI! 😆

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

vsoch · 2022-08-20T01:35:20Z

@muffato I've finished 2/3 things we discussed above - creating a GitLab remote (and support for it) and configuring sync from file. The last bullet I'll need some pointers from you for how to reproduce the bugs you saw (and to be sure we test for them!)

vsoch · 2022-08-20T12:44:12Z

Okay - I opted for https://docs.python.org/3/library/shutil.html#shutil.copy2 because that also preserves metadata (in addition to data and permissions).

muffato

Hi. FYI I'm looking at the code that does the copying, to make it exclude .git. I found where the bug is, and I'll submit a PR onto this PR later today

muffato · 2022-08-20T22:26:27Z

I'm not sure where I'm going with my changes, so best is that I describe what I've noticed.

sync_from_remote identifies what needs to be copied by first calling iter_modules (it then calls update_container_module when required).
iter_modules calls shpc.utils.recursive_find and then discards the files that start with a dot, therefore allowing the directories that start with a dot.

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

…larity-hpc into add/remote-registry

vsoch · 2022-08-20T22:57:30Z

@muffato give that a shot! I'm going for a run, then probably a break/dinner, I promise I'll be back later tonight after that! 🥑

muffato · 2022-08-21T12:06:12Z

8739676 made the trick, fantastic ! 💯

I can't find how to do it anymore, but #576 (comment) and #576 (comment) can be marked as resolved 🚀

I think the only things I want to test today are:

GitLab remote
sync yaml file

vsoch · 2022-08-21T18:08:47Z

Sounds good!

muffato

Hi

The yaml file works great (with that little addition) ! And I've also tested the check for unknown arguments on the command-line: a life saver !

About GitLab:

shpc show breaks if the repo is under groups because shpc does org, repo = self.source.split("/")[3:], while gitlab allows groups in between the org and repo name, e.g. https://gitlab.internal.sanger.ac.uk/tol-it/software/shpc-installation.git
Once moved to match org, repo, then it fails because I don't have anything under https://$org.gitlab.io/$repo/library.json, and it's due to two things:
- I haven't set up .gitlab-ci.yml (I guess that's what's needed to generate library.json ?)
- Even if had it, our "pages" are at https://$org.pages.internal.sanger.ac.uk/$repo_name (I don't know how it looks like with groups !)
At least shpc sync-registry works, even with our internal one

Questions:

How important is this library.json API ? In case it can't be found, what about taking a temporary local checkout and building a cache from there ?
Then, what about ssh support ? If a substitute for library.json can be found, I guess we don't require https any more, and ssh would be just fine (since all git access would be done via git clone)
How / where to define the tag/branch and the subdir ? I can see the code supports that, but only the tag option in sync-registry
Do you really want to support on-prem installations of gitlab ? and do they all follow this pattern ?

shpc/client/sync.py

vsoch · 2022-08-21T23:25:51Z

@muffato try using my dummy as an example: https://gitlab.com/singularityhub/shpc-registry. That should have the workflow to generate and deploy to pages - I'm kind of a GitLab noob but after that I was able to click pages and make sure my artifact in "public" worked. For the other stuffs I'll take a closer look and make some tweaks tonight, and in the meantime if you want to share the update to match "org" and "repo" I can be sure to have it handy later! We should probably have a fallback for show that checks for the library and then issues an error if it doesn't exist, and tells the user to visit the repository if they want to browse. More specifically:

How important is this library.json API ? In case it can't be found, what about taking a temporary local checkout and building a cache from there ?

I think this is important for quick query - e.g., shpc show. I wouldn't want it to do a clone to tmp every time someone ran that! But as I mentioned above, I think we can add a fallback that tells the user to visit the repository if there is no library.json. The alternative (if it's available without token auth) is to use some GItHub/GitLab API to list the top level contents of the repo (in the same way that we do after we clone).

Then, what about ssh support ? If a substitute for library.json can be found, I guess we don't require https any more, and ssh would be just fine (since all git access would be done via git clone)

We could definitely look into this! My thinking is we should work on this after this PR (which is already very big). If you want to open an issue with some details about how this works / what you want to work we can have it handy for when that time comes.

How / where to define the tag/branch and the subdir ? I can see the code supports that, but only the tag option in sync-registry

Currently it's just via the command line, so you can provide it with or without a file. I think we might honor the branch but I don't recall if we pass forward a subdirectory preference, the reason being we assume a remote registry is going to honor a convention of always having recipes at the root (do you see a use case where this isn't the case?).

Do you really want to support on-prem installations of gitlab ? and do they all follow this pattern ?

I don't know - what do you think? My senses are we should support official Gitlab, and if on-prem isn't too hard, we should strive for that too.

muffato · 2022-08-24T14:13:44Z

Status update:

I could get something to run our GitLab CI, but it's not working properly: it can't find any of the local recipes
I've found the URL format of GitLab Pages when there are groups or sub-groups, and can make a PR for that
Re. on prem, GitLab Pages can actually be deployed anywhere, it's left to the administrators to decide. shpc can't find this out by itself without a dedicated config option

the reason being we assume a remote registry is going to honor a convention of always having recipes at the root (do you see a use case where this isn't the case?).

The other shpc asset is the general template scripts (i.e. ones that can be shared between recipes and therefore are not in the same directory as container.yaml). Having both the registry and the templates in the same repository can be useful to track things under the same git history

muffato · 2022-08-24T14:24:38Z

I got the CI to work ! It was doing a pip install of the main branch. Changed it to add/remote-registry

muffato

I think that's all I wanted to test. I'll create issues with additional features that can be addressed later

muffato · 2022-08-24T21:43:11Z

shpc/main/registry/remote.py

+        if not self.source.startswith("http"):
+            self.source = "https://%s" % self.source
+
+        # Check for API
+        org, repo = self.source.split("/")[3:]
+        gh_pages = "https://%s.%s.io/%s/library.json" % (org, self.provider_name, repo)
+        response = requests.get(gh_pages)
+        if response.status_code != 200:
+            sys.exit(
+                "Remote %s is not deploying a Registry API. Open a GitHub issue to ask for help."
+                % self.source
+            )


Suggested change

if not self.source.startswith("http"):

self.source = "https://%s" % self.source

# Check for API

org, repo = self.source.split("/")[3:]

gh_pages = "https://%s.%s.io/%s/library.json" % (org, self.provider_name, repo)

response = requests.get(gh_pages)

if response.status_code != 200:

sys.exit(

"Remote %s is not deploying a Registry API. Open a GitHub issue to ask for help."

% self.source

)

url = self.source

if not url.startswith("http"):

url = "https://%s" % url

if url.endswith(".git"):

url = source[:-4]

# Check for API

url_parts = url.split("/")[3:]

gh_pages = "https://%s.%s.io/%s/library.json" % (url_parts[0], self.provider_name, "/".join(url_parts[1:]))

response = requests.get(gh_pages)

if response.status_code != 200:

sys.exit(

"Remote %s is not deploying a Registry API (%s). Open a GitHub issue to ask for help."

% (self.source, gh_pages)

)

Can you walk me through this change? Why wouldn't we want to change the source (for good) with a proper https url?

About this particular change, I actually don't understand why the source would not start with https:// at this stage, since the constructor checks that and raises an error.
That's why I had suggested to remove it in https://github.com/singularityhub/singularity-hpc/pull/579/files#diff-183e41e47a9fe06125f88dd47d33eb5bc399a6458ad3bc0e8d5c98e422179239L147-L148

I did that because a person may just want to specify it as github.com/<org>/<name> . I don't see that the constructor checks and throws an error, it seems to just set it:

singularity-hpc/shpc/main/registry/remote.py

Line 86 in 132d04a

self._url = self.source

.

The check happens in the super-class

singularity-hpc/shpc/main/registry/provider.py

Lines 35 to 40 in f948e4e

def __init__(self, source, *args, **kwargs):

if not (source.startswith("https://") or os.path.exists(source)):

raise ValueError(

"Registry source must exist on the filesystem or be given as https://."

)

self.source = source

But not to get distracted with this check, which could be argued is redundant (but certainly isn't causing any major issues?) - can you explain the changes in the above? What problem is it fixing? I think we are close and (things look generally ok) and could merge/release soon.

The patch is to allow support for "groups" in gitlab, cf your message (#576 (comment)):

and in the meantime if you want to share the update to match "org" and "repo"

with groups, the url is https://gitlab.com/group/sub-group/repo.git

Ah gotcha, thank you for clarifying! Let me see if I can organize the functions a bit - brb!

okay check out 1f29489. !

muffato · 2022-08-24T21:57:15Z

FYI I will not create an issue to support cloning over ssh, as the only advantage I think it'd bring is being able to clone private repositories. But shpc anyway requires a public library.json, which defeats the purpose of keeping the registry private.

vsoch · 2022-08-24T21:59:05Z

@muffato I think we could have support for that still, it just would be a different kind of clone (and show/list would do a directory listing instead of library.json).

muffato · 2022-08-24T22:01:08Z

🤔 . You also said

I think this is important for quick query - e.g., shpc show. I wouldn't want it to do a clone to tmp every time someone ran that!

I'm confused 😅 !

vsoch · 2022-08-24T22:02:54Z

There are two different cases here:

Anything with a public GitHub (or GitLab) url should work with show. If pages is available, we expect a library.json.
Anything that is private we cannot enforce that expectation (ssh).

It could be the case that a public GitHub/GitLab registry is allowed to not have a library.json, but I'd rather (to start) force people to start with my template to use a (better) practice of having the library easily seen. The reason is because along with the API, it provides a user interface for recipes: https://singularityhub.github.io/shpc-registry/.

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

vsoch · 2022-08-26T00:00:35Z

@muffato let me know if you have more questions for the review! Apologies my responses are a bit scattered.

muffato · 2022-08-26T13:20:39Z

I get from your previous message that you don't see the registry providing a pseudo API and library.json as a hard requirement anymore. So, I can create a ticket about adding support for private repositories (access could be done over https and ssh), and we'll deal with the implications of not having library.json there ?
Apart from that, I think I had tested everything I wanted to test, and I should be able to approve this PR. I'll double-check later today if that's OK ?

vsoch · 2022-08-26T17:27:31Z

I get from your previous message that you don't see the registry providing a pseudo API and library.json as a hard requirement anymore. So, I can create a ticket about adding support for private repositories (access could be done over https and ssh), and we'll deal with the implications of not having library.json there ?

Yes absolutely, I do think we should support this - encourage folks to provide the extra metadata but it shouldn't be a hard requirement. Open away!

Apart from that, I think I had tested everything I wanted to test, and I should be able to approve this PR. I'll double-check later today if that's OK ?
Yes totally! I'm in a workday I won't be able to do another pass and/or release until the evening. Thank you @muffato !

muffato

Can you add to the developer guide instructions for making your own registry ? (especially setting up the CI)

muffato · 2022-08-26T22:31:12Z

docs/getting_started/user-guide.rst

+
+Use the default 
+
+TODO WRITE THESE OUT SHOW HOW TO UPDATE 


Is it still TODO ?

lol, I think I just totally forget - yes let me add a few lines!

See da4c91c !

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

muffato · 2022-08-27T01:39:04Z

giphy.gif.mp4

vsoch added 11 commits July 19, 2022 17:16

simplify dryrun logic

90252d7

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

formatting

208e98f

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

spelling

37b0880

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

basic test of upgrade

0d08a8b

I am wondering if the upgrade function should be done between two registries, where the registry being upgraded is required to be a filesystem? Signed-off-by: vsoch <vsoch@users.noreply.github.com>

linting

619f4fd

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

refactoring registry logic to be within registry module, updating tests

398b19a

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

adding upgrade test for remote registry

ca7dcdc

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

rename to sync registry and base GitHub provider off Filesystem

79cb628

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

preparing for refactor to add default remote registry

4cf25d9

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

delete registry

e4e96f0

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

muffato reviewed Aug 18, 2022

View reviewed changes

black and various tweaks to logic

a55c243

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

add support for docgen to a specific branch

92f5502

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

vsoch added 3 commits August 18, 2022 22:05

testing support for gitlab

0bfeaa9

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

add github/gitlab name check to classes

3a48a11

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

wrong base class

f28cba7

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

vsoch added 2 commits August 19, 2022 19:24

adding support to sync from file

b2c69b0

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

adding support for registries sync file

86330ed

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

Use shutil.copy2 to preserve file metadata

b4a134d

muffato reviewed Aug 20, 2022

View reviewed changes

vsoch added 2 commits August 20, 2022 16:49

ensure we do not add non-container-module directories

8739676

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

Merge branch 'add/remote-registry' of github.com:singularityhub/singu…

f948e4e

…larity-hpc into add/remote-registry

muffato reviewed Aug 21, 2022

View reviewed changes

shpc/client/sync.py Show resolved Hide resolved

muffato approved these changes Aug 24, 2022

View reviewed changes

muffato mentioned this pull request Aug 24, 2022

Support for on-prem GitLab #580

Open

adding separate properties for each of the source url and web url

1f29489

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

muffato mentioned this pull request Aug 26, 2022

Substitute for the library.json API (for private and ssh-based remote registries) #581

Open

muffato reviewed Aug 26, 2022

View reviewed changes

add more user/developer docs for using registry

da4c91c

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

muffato approved these changes Aug 27, 2022

View reviewed changes

vsoch merged commit ce1452d into main Aug 27, 2022

vsoch deleted the add/remote-registry branch August 27, 2022 01:34

	def __init__(self, source, args, *kwargs):
	if not (source.startswith("https://") or os.path.exists(source)):
	raise ValueError(
	"Registry source must exist on the filesystem or be given as https://."
	)
	self.source = source

Uh oh!

Add/remote registry #576

Add/remote registry #576

Uh oh!

Conversation

vsoch commented Aug 3, 2022

Uh oh!

vsoch commented Aug 3, 2022

Uh oh!

muffato commented Aug 7, 2022

Uh oh!

vsoch commented Aug 7, 2022

Uh oh!

muffato left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vsoch commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vsoch commented Aug 19, 2022

Uh oh!

vsoch commented Aug 19, 2022

Uh oh!

vsoch commented Aug 20, 2022

Uh oh!

vsoch commented Aug 20, 2022

Uh oh!

muffato left a comment

Choose a reason for hiding this comment

Uh oh!

muffato commented Aug 20, 2022

Uh oh!

vsoch commented Aug 20, 2022

Uh oh!

muffato commented Aug 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vsoch commented Aug 21, 2022

Uh oh!

muffato left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vsoch commented Aug 21, 2022

Uh oh!

muffato commented Aug 24, 2022

Uh oh!

muffato commented Aug 24, 2022

Uh oh!

muffato left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

muffato Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsoch Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

vsoch commented Aug 18, 2022 •

edited

Loading

muffato commented Aug 21, 2022 •

edited

Loading

muffato Aug 24, 2022 •

edited

Loading

vsoch Aug 24, 2022 •

edited

Loading

vsoch commented Aug 24, 2022 •

edited

Loading

muffato Aug 26, 2022 •

edited

Loading