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 support for OCI registries through ORAS #541

Merged
merged 52 commits into from
Dec 2, 2024
Merged

Add support for OCI registries through ORAS #541

merged 52 commits into from
Dec 2, 2024

Conversation

isc-dchui
Copy link
Contributor

@isc-dchui isc-dchui commented Aug 1, 2024

Introduce support for OCI registries using ORAS.

Mostly uses the ORAS Python client library via embedded Python, but augmented with ObjectScript and REST.

A docker container containing a simple OCI registry (ZOT) has been added to the docker-compose. To test out the containerized zot registry, in IPM configure an ORAS registry with http://oras:5000 as the URL.

Screenshot 2024-08-01 161527 50%

It can then be published to and installed from like any other remote registry. Visit http://localhost:5001 to see the registry's UI.

Fixes #581

Copy link
Collaborator

@isc-kiyer isc-kiyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isc-dchui done an initial pass (not very thoroughly). Left some comments/questions

@@ -0,0 +1 @@
oras == 0.1.30
Copy link
Collaborator

@isc-kiyer isc-kiyer Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are using embedded python in IPM, does this further limit the earliest IRIS version that IPM can be installed on?
I believe requirements.txt is currently always enforced. We should maybe make this optional so that IPM can still be installed on older versions of IRIS which don't have embedded python with just ORAS support disabled unless embedded python exists.
cc @isc-tleavitt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! Just had a discussion with @isc-tleavitt and it seems one possible approach is allow IRIS versions without embedded python to tolerate a failure to compile requirements.txt and handle run time issues by preventing the creation of ORAS repository definitions. This would also mean avoiding [Language = python ] methods so that the classes compile.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isc-shuliu @isc-tleavitt Not a blocker for this PR but what happens behind a firewall with installing python dependencies? Should IPM be packaged with a version of oras as a backup in case the internet is not available to install the latest version?
cc @isc-eneil @isc-jili @isc-jlechtne

src/cls/IPM/Repo/Oras/Definition.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/Definition.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/Definition.cls Show resolved Hide resolved
src/cls/IPM/Repo/Oras/Definition.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/PackageService.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/PublishService.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/PublishService.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/PublishService.cls Outdated Show resolved Hide resolved
src/cls/IPM/Main.cls Outdated Show resolved Hide resolved
@isc-kiyer isc-kiyer requested a review from isc-eneil August 2, 2024 15:54
@isc-dchui
Copy link
Contributor Author

@isc-kiyer, @isc-shuliu fixed the majority of the requested changes. The only issue is the backwards compatibility with embedded Python. Unfortunately it looks like removing all [ Language = python ] methods has not proven straightforward. For example, take this snippet of Python code

# library function that returns a dictionary
manifest = client.get_manifest(target)
for key in manifest['annotations']:
	if key == "com.intersystems.ipm.module.v1+xml":
		moduleXML = manifest['annotations'][key]
		# remove whitespace
		return "".join(moduleXML.split())

Unless there's something I'm missing (which could be very possible), iterating through the Python dictionary in Objectscript is incredibly painful.
@isc-tleavitt Thoughts or suggestions?

@isc-kiyer
Copy link
Collaborator

@isc-dchui Hmm I checked and embedded python was introduced in 21.2 and the first EM release to include it was 22.1. If we want to say we don't support older versions of IRIS for IPM v1, we could include this embedded python (I am fine with that but would want @isc-tleavitt to confirm). That being said, we need to make sure we don't hit any bugs embedded python may have had in those older versions so would need to test this so am a bit conflicted.

@isc-tleavitt
Copy link
Contributor

In weekly meeting today, https://openexchange.intersystems.com/package/PyHelper came up as an option - specifically ArrayFrompyDict looks useful.

@isc-tleavitt
Copy link
Contributor

Discussed with @isc-shuliu today. Plan is:

  • I've polled on DC to see if anyone really wants earlier version compatibility: https://community.intersystems.com/post/anyone-using-ipm-earlier-20221
  • @isc-shuliu is going to spend some time kicking the tires on this, then we'll merge to v1-next and possibly take removal [ Language = python ] methods as a follow-up action. This will get ORAS out of a feature branch and into a branch that will also be getting related fixes that HS/TC will need.

Copy link
Contributor

@isc-tleavitt isc-tleavitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me like this is in good shape!

Copy link
Collaborator

@isc-kiyer isc-kiyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isc-shuliu final round of comments!

src/cls/IPM/Repo/Http/Definition.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Http/Definition.cls Show resolved Hide resolved
src/cls/IPM/Repo/Oras/Definition.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/Definition.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/Definition.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/PackageService.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/PublishService.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Oras/PublishService.cls Show resolved Hide resolved
Copy link
Collaborator

@isc-eneil isc-eneil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isc-shuliu I left a few small comments/questions!

src/cls/IPM/Main.cls Show resolved Hide resolved
src/cls/IPM/Repo/Http/PackageService.cls Outdated Show resolved Hide resolved
src/cls/IPM/Repo/Http/PackageService.cls Outdated Show resolved Hide resolved
@isc-shuliu isc-shuliu requested a review from isc-kiyer November 27, 2024 19:31
@isc-tleavitt isc-tleavitt merged commit db078b1 into v1-next Dec 2, 2024
13 checks passed
@isc-tleavitt isc-tleavitt deleted the oras branch December 2, 2024 14:42
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.

6 participants