-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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 distributing OCI images on IPFS #6175
Conversation
Build succeeded.
|
images/converter/default.go
Outdated
} | ||
|
||
// IndexConvertFuncWithHook is the convert func used by Convert with hook functions support. | ||
func IndexConvertFuncWithHook(layerConvertFunc ConvertFunc, docker2oci bool, platformMC platforms.MatchComparer, hooks ConvertHooks) ConvertFunc { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you split this to another PR for ease of reviewing and merging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the review. Opened the converter hook PR on #6176.
// additional tracking overhead to a standard LRU cache, computationally | ||
// it is roughly 2x the cost, and the extra memory overhead is linear | ||
// with the size of the cache. ARC has been patented by IBM, but is | ||
// similar to the TwoQueueCache (2Q) which requires setting parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we actually using this patent-protected code?
Can we avoid importing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should consider temporarily forking the repo to ensure that we aren't importing this
// block Cids. This provides block access-time improvements, allowing | ||
// to short-cut many searches without query-ing the underlying datastore. | ||
type arccache struct { | ||
arc *lru.TwoQueueCache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this file has been patched to avoid hitting the ARC patent, but needs confirmation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened https://github.com/ipfs/go-ipfs-blockstore/issues/87 to request clarification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR can compile without vendor/github.com/hashicorp/golang-lru/arc.go
.
Should we import the forked version that doesn't contain vendor/github.com/hashicorp/golang-lru/arc.go
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed to use the forked golang-lru that removes vendor/github.com/hashicorp/golang-lru/arc.go
.
8f96335
to
dad1577
Compare
Build succeeded.
|
@@ -0,0 +1,105 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have ctr run --ipfs
in the same repo too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about ctr ipfs pull <CID>
? Then we can run it using the normal ctr run
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added ctr ipfs pull <CID>
.
images/converter/ipfs/ipfs.go
Outdated
limitations under the License. | ||
*/ | ||
|
||
package ipfs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this package should be under contrib/ipfs/converter
or contrib/converter/ipfs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved ipfs-related codes to contrib/ipfs
.
Build succeeded.
|
Is this a feature that could start out in nerdctl? |
Could we move #6176 forward at least? That is a generic patch and will be used by nerdctl and stargz-snapshotter. |
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Build succeeded.
|
IPFS will come to nerdctl soon containerd/nerdctl#505 |
@dmcgowan @kzys Thanks for the comments. I'm closing this PR in favor of the work in nerdctl (containerd/nerdctl#505). I'll reopen it when changes will be needed to containerd itself. |
@AkihiroSuda I'll complete the PR in nerdctl first. Thank you for your time to review this PR and the one in nerdctl. |
Moved to containerd/nerdctl#505
This commit enables containerd client to distribute OCI images on IPFS.
Push (ipfs daemon needs running):
Pull and run (ipfs daemon needs running):
This example uses overlayfs snapshotter but any snapshotters can be used in theory because IPFS is handled by the IPFS-aware
remotes.Resolver
not by snapshotters. For example, stargz snapshotter supports directly mounting eStargz from IPFS to containers's rootfs with experimental lazy pulling support. Please see https://github.com/containerd/stargz-snapshotter/blob/main/docs/ipfs.md for details.IPFS-enabled OCI Image
Each descriptor in an IPFS-enabled OCI image must contain the following IPFS URL in
urls
field.<CID>
is the case-insensitive CIDv1 of the blob that the descriptor points to.Code changes to containerd client
This commit adds a package
contrib/ipfs
which contains containerd client codes for distributing OCI images on IPFS.ipfs.Push
pushes the image with converting to the IPFS-enabled format described above. Each blob is pushed as an UnixFS file for leveraging the chunking support.ipfs.Resolver
. When pulling the image, this resolver seesurls
field in each descriptor and fetches the contents from IPFS.Next steps
We will add the following changes to enable containerd to fully support IPFS.
references
pkg of contianerd. (ipfs://<CID>
or/ipfs/<CID>
)nerdctl run /ipfs/QmfTVLXMG9TH7X523NytcXj35XtEDx4wgNWYepVumqpJZV
) nerdctl#465).Related discussions
nerdctl run /ipfs/QmfTVLXMG9TH7X523NytcXj35XtEDx4wgNWYepVumqpJZV
) nerdctl#465digest
field to the hash of the blob's root object but this PR leveragesurls
for keeping OCI compatibility.Thanks @AkihiroSuda for the discussion.