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 examples for repository push, pull, resolve and tags #119

Merged
merged 49 commits into from
Apr 15, 2022

Conversation

qweeah
Copy link
Contributor

@qweeah qweeah commented Mar 18, 2022

This PR works for item #60 and adds examples for:
Push & pull blobs
Tag a image
Resolve the tag
List tags in a repository

@qweeah qweeah marked this pull request as draft March 25, 2022 01:53
@qweeah qweeah marked this pull request as ready for review March 25, 2022 03:27
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
@Wwwsylvia
Copy link
Member

We will need examples for FetchReference once #124 gets merged.

content/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
if err != nil {
panic(err) // Handle error
}
// If you want to play with your local registry, try to override
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Try using objective wording and try to avoid words like you, he, she, I, we.

qweeah and others added 18 commits April 6, 2022 13:08
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
* pack
* fix pack
* add pack test

Signed-off-by: Lixia (Sylvia) Lei <lixia_lei@outlook.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Sylvia Lei <lixlei@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Sajay Antony <sajaya@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixia_lei@outlook.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
* pack
* fix pack
* add pack test

Signed-off-by: Lixia (Sylvia) Lei <lixia_lei@outlook.com>
Signed-off-by: Sajay Antony <sajaya@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@Wwwsylvia
Copy link
Member

Wwwsylvia commented Apr 7, 2022

In the example, I pulled a tagged layer from the registry

You actually pulled a manifest.
Ah I see what's not right, you used the same digest for mocking /v2/<repository>/manifests/<reference> API and /v2/<repository>/blobs/<reference> API (https://github.com/qweeah/oras-go/blob/repository-example/registry/remote/example_test.go#L77-L82), so you didn't notice which API you were accessing. You may refer to TestRepository_Fetch for the mocks.

I think manifest is also a blob with specific media type, which should also be the common sense of oras-go user (correct me if I am wrong). Thus we don't need to clarify that, at least not in the example.

Yeah but when querying remote registries, manifests and blobs go through different API endpoints. Just that functions like Repository.Fetch() automatically choose endpoints based on the media type.
For users familiar with Docker V2 API, they may want to know how the oras-go functions correspond to the V2 APIs? (Just a thought)

User can also resolve the descriptor of a (layer or config) blob by Repository.Resolve()

No they can't.
Repository.Resolve() calls Manifests().Resolve() behind the scenes, which queries the /v2/<repository>/manifests/<reference> API (See https://github.com/oras-project/oras-go/blob/main/registry/remote/repository.go#L760). So it only resolves manifests.

// Resolve resolves a reference to a manifest descriptor.
// See also `ManifestMediaTypes`.
func (r *Repository) Resolve(ctx context.Context, reference string) (ocispec.Descriptor, error) {
	return r.Manifests().Resolve(ctx, reference)
}

@qweeah
Copy link
Contributor Author

qweeah commented Apr 7, 2022

In the example, I pulled a tagged layer from the registry

You actually pulled a manifest. Ah I see what's not right, you used the same digest for mocking /v2/<repository>/manifests/<reference> API and /v2/<repository>/blobs/<reference> API (https://github.com/qweeah/oras-go/blob/repository-example/registry/remote/example_test.go#L77-L82), so you didn't notice which API you were accessing. You may refer to TestRepository_Fetch for the mocks.

I think manifest is also a blob with specific media type, which should also be the common sense of oras-go user (correct me if I am wrong). Thus we don't need to clarify that, at least not in the example.

Yeah but when querying remote registries, manifests and blobs go through different API endpoints. Just that functions like Repository.Fetch() automatically choose endpoints based on the media type. For users familiar with Docker V2 API, they may want to know how the oras-go functions correspond to the V2 APIs? (Just a thought)

User can also resolve the descriptor of a (layer or config) blob by Repository.Resolve()

No they can't. Repository.Resolve() calls Manifests().Resolve() behind the scenes, which queries the /v2/<repository>/manifests/<reference> API (See https://github.com/oras-project/oras-go/blob/main/registry/remote/repository.go#L760). So it only resolves manifests.

// Resolve resolves a reference to a manifest descriptor.
// See also `ManifestMediaTypes`.
func (r *Repository) Resolve(ctx context.Context, reference string) (ocispec.Descriptor, error) {
	return r.Manifests().Resolve(ctx, reference)
}

Yes you are right. Looks like I mixed Repository.Resolve with BlobStore.Resolve. I will separate blob pull and manifest pull.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Copy link
Contributor Author

qweeah commented Apr 8, 2022

@Wwwsylvia I separated examples for manifest and layer fetch and added new examples for manifes FetchReference. Please take a review when you got time, thanks

registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah requested a review from Wwwsylvia April 11, 2022 16:21
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Copy link
Member

@Wwwsylvia Wwwsylvia left a comment

Choose a reason for hiding this comment

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

LGTM

registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
Comment on lines 330 to 337
pulledBlob, err := io.ReadAll(rc)
if err != nil {
panic(err)
}
// verify the fetched content
if descriptor.Digest != ocidigest.FromBytes(pulledBlob) || descriptor.Size != int64(len(pulledBlob)) {
panic(err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we have lots of duplicated code like this.

@Wwwsylvia What do you think if we promote internal/ioutil.ReadAll() to the content package?

Copy link
Member

Choose a reason for hiding this comment

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

That would be useful.

Copy link
Member

Choose a reason for hiding this comment

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

Issue created: #128

registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
registry/remote/example_test.go Outdated Show resolved Hide resolved
Comment on lines +448 to +456
descriptor, err := repo.Resolve(ctx, digest)
if err != nil {
panic(err)
}
tag := "latest"
err = repo.Tag(ctx, descriptor, tag)
if err != nil {
panic(err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@Wwwsylvia Resolve and Tag will have 3 requests in total. We probably need a more efficient one by having a generic Tag() function by leveraging FetchReference() and PushReference().

Copy link
Member

Choose a reason for hiding this comment

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

Make sense.
Like func (r *Repository) ReTag(ctx context.Context, srcRef, destRef string) error? Need to figure out a new function name.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about func Tag(ctx context.Context, repo Repository, srcRef, dstRef string) error in package registry?

Copy link
Member

Choose a reason for hiding this comment

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

OK

Copy link
Member

Choose a reason for hiding this comment

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

@qweeah has created an issue for this: #120

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@shizhMSFT shizhMSFT merged commit c34895e into oras-project:main Apr 15, 2022
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.

None yet

4 participants