Add save_to_oci_registry ptyhon client method #800

Crazyglue · 2025-02-13T19:05:07Z

Description

This PR aims to add a helper method to the python client to append a set of files to a given oci-compatible container image. In the future this method will be used as a part of the PR found here: #761 which aims to provide yet another helper method that will both upload and register a model to the MR.

With this PR and the one mentioned above, the use case for the client (after training) is to simply call 1 method to both push the newly trained model to a storage location AND register that newly trained model in the MR. Removing a number of manual steps that would have to be done separately as a part of a standard DS flow.

How Has This Been Tested?

Added an E2E test and validated that they run. The E2E test uses a local registry to upload test-images to.

The local environment will need to at a minimum install skopeo on the system (for MacOS and Brew, this means running brew install skopeo).

Merge criteria:

All the commits have been signed-off (To pass the DCO check)

The commits have meaningful messages; the author will squash them after approval or in case of manual merges will ask to merge with squash.
Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has manually tested the changes and verified that the changes work.
Code changes follow the kubeflow contribution guidelines.

If you have UI changes

The developer has added tests or explained why testing cannot be added.
Included any necessary screenshots or gifs if it was a UI change.
Verify that UI/UX changes conform the UX guidelines for Kubeflow.

Signed-off-by: Eric Dobroveanu <[email protected]>

Crazyglue · 2025-02-13T19:06:09Z

scripts/deploy_local_kind_registry.sh

This was added as a way to have the e2e tests include image download/upload. A registry is needed to perform the upload step, and so a local registry should be good enough to perform this test.

Crazyglue · 2025-02-13T19:07:54Z

clients/python/src/model_registry/utils.py

+    if backend == 'skopeo':
+        from olot.backend.skopeo import is_skopeo, skopeo_pull, skopeo_push
+
+        if not is_skopeo():
+            raise ValueError('skopeo is selected, but it is not present on the machine. Please validate the skopeo cli is installed and available in the PATH')
+
+        skopeo_pull(base_image, local_image_path)
+        oci_layers_on_top(local_image_path, model_files, modelcard)
+        skopeo_push(dest_dir, oci_ref)
+
+    elif backend == 'oras':
+        from olot.backend.oras_cp import is_oras, oras_pull, oras_push
+        if not is_oras():
+            raise ValueError('oras is selected, but it is not present on the machine. Please validate the oras cli is installed and available in the PATH')
+
+        oras_pull(base_image, local_image_path)
+        oci_layers_on_top(local_image_path, model_files, modelcard)
+        oras_push(local_image_path, oci_ref)


I chose not to abstract this away into an interface/facade because the steps are very straightforward, and i think it would over-engineer what is effectively calling 3 separate functions.

If olot supports more backends, this will probably need to be refactored to include an interface of some kind.

yeah, but as in #800 (comment)
I would consider passing to a monkeypatched method all params, so that at least we steer users to a provider mechanism of sort

Crazyglue · 2025-02-13T19:08:54Z

clients/python/pyproject.toml

@@ -27,9 +27,11 @@ nest-asyncio = "^1.6.0"
 eval-type-backport = "^0.2.0"

 huggingface-hub = { version = ">=0.20.1,<0.29.0", optional = true }
+olot = { version = "^0.1.2", optional = true }


This is following the same pattern by huggingface-hub above. This particular dependency is optional because if you choose not to use this new "save" method, you would never need this dependency. The README was updated to reflect this

scripts/deploy_on_kind.sh

Signed-off-by: Eric Dobroveanu <[email protected]>

tarilabs

some early comment, thank you @Crazyglue

tarilabs · 2025-02-14T14:46:42Z

clients/python/pyproject.toml

@@ -27,9 +27,11 @@ nest-asyncio = "^1.6.0"
 eval-type-backport = "^0.2.0"

 huggingface-hub = { version = ">=0.20.1,<0.29.0", optional = true }
+olot = { version = "^0.1.2", optional = true }


clients/python/src/model_registry/utils.py

clients/python/tests/test_utils.py

scripts/deploy_on_kind.sh

scripts/deploy_local_kind_registry.sh

clients/python/Makefile

…ders Signed-off-by: Eric Dobroveanu <[email protected]>

Signed-off-by: Eric Dobroveanu <[email protected]>

dhirajsb

/lgtm
reviewed at a high level, and I don't see any red flags
@tarilabs will have to approve to merge this initial version

google-oss-prow · 2025-02-20T20:34:41Z

@dhirajsb: changing LGTM is restricted to collaborators

In response to this:

/lgtm
reviewed at a high level, and I don't see any red flags
@tarilabs will have to approve to merge this initial version

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

google-oss-prow · 2025-02-20T20:34:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dhirajsb
Once this PR has been reviewed and has the lgtm label, please assign rareddy for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Crazyglue added 4 commits February 13, 2025 13:56

chore: add olot as a dependency

ef5deab

Signed-off-by: Eric Dobroveanu <[email protected]>

chore: update default local kind cluster name to be more MR specific

848f9ba

Signed-off-by: Eric Dobroveanu <[email protected]>

feat: add save_to_oci_registry method to python client

9573ffb

Signed-off-by: Eric Dobroveanu <[email protected]>

test: add e2e test for save_to_oci_registry python client method

2e40108

Signed-off-by: Eric Dobroveanu <[email protected]>

google-oss-prow bot requested review from andreyvelich and zijianjoy February 13, 2025 19:05

github-actions bot added the Area/MR Python client label Feb 13, 2025

google-oss-prow bot added the size/L label Feb 13, 2025

Crazyglue commented Feb 13, 2025

View reviewed changes

scripts/deploy_on_kind.sh Show resolved Hide resolved

tarilabs mentioned this pull request Feb 14, 2025

Export Models to Kubeflow Model Registry kubeflow/trainer#2438

Open

Crazyglue added 2 commits February 14, 2025 09:19

chore: tidy python client

7311260

Signed-off-by: Eric Dobroveanu <[email protected]>

chore: update noxfile to include olot

b0dba7b

Signed-off-by: Eric Dobroveanu <[email protected]>

Crazyglue force-pushed the feat/oci-registry-client branch from f606e01 to b0dba7b Compare February 14, 2025 14:19

github-actions bot added the Area/GitHub label Feb 14, 2025

chore: update ci to install skopeo in the runtime

2678e81

Signed-off-by: Eric Dobroveanu <[email protected]>

Crazyglue force-pushed the feat/oci-registry-client branch from 40dec90 to 2678e81 Compare February 14, 2025 14:45

tarilabs reviewed Feb 14, 2025

View reviewed changes

chore: rework save_to_oci_registry to incorporate user-provided provi…

51a7812

…ders Signed-off-by: Eric Dobroveanu <[email protected]>

Crazyglue force-pushed the feat/oci-registry-client branch from 4554ce2 to 51a7812 Compare February 17, 2025 15:09

Crazyglue added 3 commits February 18, 2025 10:37

chore: make methods internal

ae3a949

Signed-off-by: Eric Dobroveanu <[email protected]>

chore: improve e2e testing

46c4b68

Signed-off-by: Eric Dobroveanu <[email protected]>

test: add additional unit tests

010ccbb

Signed-off-by: Eric Dobroveanu <[email protected]>

Crazyglue force-pushed the feat/oci-registry-client branch 5 times, most recently from b8e68e6 to 9795f0b Compare February 18, 2025 19:07

Crazyglue force-pushed the feat/oci-registry-client branch from 9795f0b to 7fb8bef Compare February 18, 2025 19:28

chore: set default dest_dir

47a7b88

Signed-off-by: Eric Dobroveanu <[email protected]>

Crazyglue force-pushed the feat/oci-registry-client branch from 7fb8bef to 47a7b88 Compare February 18, 2025 19:29

Crazyglue added 2 commits February 18, 2025 14:44

chore: formatting for linter

0cd10af

Signed-off-by: Eric Dobroveanu <[email protected]>

chore: ensure local registry is deployed

c736e7e

Signed-off-by: Eric Dobroveanu <[email protected]>

Crazyglue force-pushed the feat/oci-registry-client branch 2 times, most recently from 3287526 to 37b60e4 Compare February 20, 2025 20:14

chore: ensure uri is returned from function

0fcdde7

Signed-off-by: Eric Dobroveanu <[email protected]>

Crazyglue force-pushed the feat/oci-registry-client branch from 37b60e4 to 0fcdde7 Compare February 20, 2025 20:22

dhirajsb approved these changes Feb 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add save_to_oci_registry ptyhon client method #800

Add save_to_oci_registry ptyhon client method #800

Crazyglue commented Feb 13, 2025

Crazyglue Feb 13, 2025

Crazyglue Feb 13, 2025

tarilabs Feb 14, 2025

Crazyglue Feb 13, 2025

tarilabs Feb 14, 2025

tarilabs left a comment

tarilabs Feb 14, 2025

dhirajsb left a comment

google-oss-prow bot commented Feb 20, 2025

google-oss-prow bot commented Feb 20, 2025

Add save_to_oci_registry ptyhon client method #800

Are you sure you want to change the base?

Add save_to_oci_registry ptyhon client method #800

Conversation

Crazyglue commented Feb 13, 2025

Description

How Has This Been Tested?

Merge criteria:

Crazyglue Feb 13, 2025

Choose a reason for hiding this comment

Crazyglue Feb 13, 2025

Choose a reason for hiding this comment

tarilabs Feb 14, 2025

Choose a reason for hiding this comment

Crazyglue Feb 13, 2025

Choose a reason for hiding this comment

tarilabs Feb 14, 2025

Choose a reason for hiding this comment

tarilabs left a comment

Choose a reason for hiding this comment

tarilabs Feb 14, 2025

Choose a reason for hiding this comment

dhirajsb left a comment

Choose a reason for hiding this comment

google-oss-prow bot commented Feb 20, 2025

google-oss-prow bot commented Feb 20, 2025