OCI Target #86
OCI Target #86
Conversation
Wwwsylvia
force-pushed
the
oci_target
branch
from
37944ad
to
8a098bc
Compare
shizhMSFT
requested review from
shizhMSFT,
deitch,
jdolitsky,
sajayantony and
SteveLasker
Wwwsylvia
force-pushed
the
oci_target
branch
from
8a098bc
to
8bd197b
Compare
Wwwsylvia
force-pushed
the
oci_target
branch
2 times, most recently
from
c6fe769
to
56386c9
Compare
content/oci/oci.go
Outdated
| if err := store.validateOCILayoutFile(); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if err := store.loadIndex(); err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
What component is supposed to do the initialization to create the layerout and index file?
There was a problem hiding this comment.
The layout file is initialized in validateOCILayoutFile() when loading. The index file will be initialized on first tag in saveIndex()
There was a problem hiding this comment.
I see. It is not clear from the name that validateOCILayeroutFile creates the layout file if it doesn't exist. Maybe you can rename it something like validateOrCreateOCILayoutFile
There was a problem hiding this comment.
Do you know where I can find a code snippet example of copy an oci artifact from registry to local?
There was a problem hiding this comment.
We shall update the examples once the implementation is done... Before that, @akashsinghal has written some code to copy some artifacts from OCI target to remote registry, which is helpful - https://github.com/akashsinghal/accelerated-container-image/blob/47ef7a728bed74020ef48814e8efb461f3db0236/cmd/ctr/overlaybd_conv.go#L740-L781
Wwwsylvia
force-pushed
the
oci_target
branch
2 times, most recently
from
842feab
to
463badf
Compare
| return s.storage.Exists(ctx, target) | ||
| } | ||
|
|
||
| // Tag tags a descriptor with a reference string. |
There was a problem hiding this comment.
What if the user invokes s.Tag(ctx, desc, "localhost:5000/myrepo:mytag") since it is a common scenario in Copy(). Will you parse it and only store mytag in org.opencontainers.image.ref.name? or consider localhost:5000/myrepo:mytag as an invalid reference?
content/oci/oci.go
Outdated
| indexFile, err := os.Open(s.indexPath) | ||
| if err != nil { | ||
| if !os.IsNotExist(err) { | ||
| return fmt.Errorf("failed to open index file: %v", err) |
There was a problem hiding this comment.
nit: it will be better to use %w instead of %v for wrapping errors. Same comment will be applied to all similar occourances.
There was a problem hiding this comment.
Please see fmt.Errorf() for the documentation of %w.
There was a problem hiding this comment.
What if the user invokes s.Tag(ctx, desc, "localhost:5000/myrepo:mytag") since it is a common scenario in Copy(). Will you parse it and only store mytag in org.opencontainers.image.ref.name? or consider localhost:5000/myrepo:mytag as an invalid reference?
We need to store the original user-provided reference in the org.opencontainers.image.ref.name as we use it to construct ref map when loading index. As per the OCI image spec, a reference like "localhost:5000/myrepo:mytag" is valid in grammar.
For the common Copy scenarios, isn't it like
// do something...
ref := "mytag"
err := oci.Push(ctx, desc, content)
err := oci.Tag(ctx, desc, ref)
repo := "localhost:5000/myrepo"
repository, err := remote.NewRepository(repo)
// do something...
root, err := oras.Copy(ctx, oci, ref, repository, ref)? The user would call oci.Tag(ctx, desc, "mytag") instead of oci.Tag(ctx, desc, "localhost:5000/myrepo:mytag")
There was a problem hiding this comment.
How about this scenario?
ref := "localhost:5000/hello:v1"
src, _ := remote.NewRepository(ref)
dst, _ := oci.New("hello")
oras.Copy(context.Background(), src, ref, dst, "")There was a problem hiding this comment.
I think it's ok. In this case we will have "org.opencontainers.image.ref.name": "localhost:5000/hello:v1".
The OCI image spec mentions that
No semantic restriction is given for the "org.opencontainers.image.ref.name" annotation of descriptors.
So it makes sense to store FQDN in org.opencontainers.image.ref.name.
After copy, the user can resolve by the same ref.
ref := "localhost:5000/hello:v1"
src, _ := remote.NewRepository(ref)
dst, _ := oci.New("hello")
oras.Copy(context.Background(), src, ref, dst, "")
desc, _ := dst.Resolve(ctx, ref)
Wwwsylvia
force-pushed
the
oci_target
branch
2 times, most recently
from
aea9977
to
a8079b9
Compare
|
@shizhMSFT please squash merge. Let’s discuss the API usability to consume these and clean up before release. |
Signed-off-by: Sylvia Lei <lixia_lei@outlook.com>
Signed-off-by: Sylvia Lei <lixia_lei@outlook.com>
Resolves #55