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

Support pulling/pushing VMs to OCI-compatible registries #32

Merged
merged 21 commits into from
May 3, 2022

Conversation

edigaryev
Copy link
Collaborator

@edigaryev edigaryev commented Apr 29, 2022

This still needs some things to be done before this gets merged:

Resolves #6.

@edigaryev edigaryev requested a review from fkorotkov April 29, 2022 13:52
Sources/tart/VMStorageOCI.swift Show resolved Hide resolved
Sources/tart/VMDirectory+OCI.swift Outdated Show resolved Hide resolved
Sources/tart/Commands/Push.swift Outdated Show resolved Hide resolved
Sources/tart/Commands/Clone.swift Show resolved Hide resolved
@fkorotkov
Copy link
Contributor

Another note. I think we should deprioritize tests in favor of a sooner release and follow up with more tests in a separate PR.

Also will be great to add some logging to the push/pull process since it can take quite a while and will be great to update users about progress.

@fkorotkov
Copy link
Contributor

Seems there is an issue with the reference parsing as well:

tart push monterey-vanilla ghcr.io/cirruslabs/macos-monterey-vanilla:12.3.1
error: unexpected input
 --> input:1:45
1 | …-monterey-vanilla:12.3.1
  |                      ^ expected end of input

@edigaryev edigaryev force-pushed the oci-registry-support branch from f21d4ce to 80645ff Compare May 3, 2022 11:40
@edigaryev
Copy link
Collaborator Author

Seems OK, made a couple of improvements (e.g. 0144913).

Also re-checked against https://docs.docker.com/registry/spec/auth/token/, the only thing that's missing is that we don't specify a client_id in the token request, but that parameter doesn't seem to be critical.

@edigaryev
Copy link
Collaborator Author

Seems there is an issue with the reference parsing as well:

Fixed in 20fffbb, further improvements in #35.

@edigaryev
Copy link
Collaborator Author

Another note. I think we should deprioritize tests in favor of a sooner release and follow up with more tests in a separate PR.

Deprioritized to #34.

Also will be great to add some logging to the push/pull process since it can take quite a while and will be great to update users about progress.

Implemented in 27bb3c5.

@edigaryev edigaryev requested a review from fkorotkov May 3, 2022 15:23
@edigaryev edigaryev enabled auto-merge (squash) May 3, 2022 15:26
@@ -67,6 +67,10 @@ extension VMDirectory {
}

func pushToRegistry(registry: Registry, reference: String) async throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can remove this method completely and remove the for loop where it's called.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think you can remove this method completely and remove the for loop where it's called.

Oh, good catch.

Fixed in 821a99b.

@edigaryev edigaryev force-pushed the oci-registry-support branch from ca94d6f to b6fd0ed Compare May 3, 2022 16:26
@edigaryev edigaryev requested a review from fkorotkov May 3, 2022 16:47
@edigaryev edigaryev merged commit 95316c0 into main May 3, 2022
@edigaryev edigaryev deleted the oci-registry-support branch May 3, 2022 18:30
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.

Push/pull images using OCI container registry
2 participants