-
Notifications
You must be signed in to change notification settings - Fork 606
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
Enable image distribution on IPFS #505
Conversation
cmd/nerdctl/ipfs_pull.go
Outdated
ipfsPullCommand.PersistentFlags().Bool("all-platforms", false, "Pull content for all platforms") | ||
// #endregion | ||
|
||
ipfsPullCommand.PersistentFlags().String("scheme", "ipfs", "Scheme to pull the CID (\"ipfs\" or \"ipns\")") |
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.
nit: register flag completion func
cmd/nerdctl/ipfs_pull.go
Outdated
|
||
func newIPFSPullCommand() *cobra.Command { | ||
var ipfsPullCommand = &cobra.Command{ | ||
Use: "pull", |
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.
Explain argument
cmd/nerdctl/ipfs_push.go
Outdated
ipfsPushCommand.PersistentFlags().Bool("all-platforms", false, "Push content for all platforms") | ||
// #endregion | ||
|
||
ipfsPushCommand.PersistentFlags().Bool("estargz", true, "Convert the image into eStargz") |
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.
If we want --estargz
flag here, we should add that to the regular nerdctl image push
too, and deduplicate the code across nerdctl image push
and nerdctl ipfs push
.
(But do we really need nerdctl ipfs *
commands?)
3e1febb
to
a46e06a
Compare
97cba30
to
98edcd5
Compare
echo "ipfs = true" > ${HOME}/.config/containerd-stargz-grpc/config.toml | ||
systemctl --user restart stargz-snapshotter.service | ||
sleep 3 | ||
fi |
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.
The logic looks too much complicated.
Maybe we should just run IPFS as a systemd service?
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.
Rootless nerdctl (inside rootlesskit network namespace) can't access to IPFS daemon API (127.0.0.1:5001) on the host?
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.
Right. I thought we could just use a public IP.
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.
Wondering we should have containerd-rootless-setuptool.sh install-ipfs
to run IPFS daemon in the netns
nerdctl/extras/rootless/containerd-rootless-setuptool.sh
Lines 397 to 419 in 5b52e3b
usage() { | |
echo "Usage: ${ARG0} [OPTIONS] COMMAND" | |
echo | |
echo "A setup tool for Rootless containerd (${CONTAINERD_ROOTLESS_SH})." | |
echo | |
echo "Commands:" | |
echo " check Check prerequisites" | |
echo " nsenter Enter into RootlessKit namespaces (mostly for debugging)" | |
echo " install Install systemd unit and show how to manage the service" | |
echo " uninstall Uninstall systemd unit" | |
echo | |
echo "Add-on commands (BuildKit):" | |
echo " install-buildkit Install the systemd unit for BuildKit" | |
echo " uninstall-buildkit Uninstall the systemd unit for BuildKit" | |
echo | |
echo "Add-on commands (fuse-overlayfs):" | |
echo " install-fuse-overlayfs Install the systemd unit for fuse-overlayfs snapshotter" | |
echo " uninstall-fuse-overlayfs Uninstall the systemd unit for fuse-overlayfs snapshotter" | |
echo | |
echo "Add-on commands (stargz):" | |
echo " install-stargz Install the systemd unit for stargz snapshotter" | |
echo " uninstall-stargz Uninstall the systemd unit for stargz snapshotter" | |
} |
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
cmd/nerdctl/compose_up_test.go
Outdated
testutil.DockerIncompatible(t) | ||
base := testutil.NewBase(t) | ||
requiresStargz(base) | ||
base.Env = append(os.Environ(), "CONTAINERD_SNAPSHOTTER=stargz") |
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.
We should test this for overlayfs as well
cmd/nerdctl/push.go
Outdated
@@ -47,6 +58,11 @@ func newPushCommand() *cobra.Command { | |||
pushCommand.Flags().Bool("all-platforms", false, "Push content for all platforms") | |||
// #endregion | |||
|
|||
pushCommand.PersistentFlags().Bool("estargz", false, "Convert the image into eStargz (always true in IPFS mode)") |
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.
Not sure why this has to be always true in IPFS mode.
cmd/nerdctl/push.go
Outdated
pushCommand.PersistentFlags().Bool("estargz", false, "Convert the image into eStargz (always true in IPFS mode)") | ||
|
||
pushCommand.PersistentFlags().Bool("ipfs-no-estargz", false, "Disable conversion of the image into eStargz in IPFS mode") | ||
pushCommand.PersistentFlags().Bool("ipfs-ensure-image", true, "Ensure the IPFS image is fully pulled to containerd") |
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.
s/pulled/pushed/ (?)
e83b542
to
5dee562
Compare
CI seems to be cancelled?
|
e2d5a58
to
5eacb67
Compare
go.mod
Outdated
github.com/mattn/go-isatty v0.0.14 | ||
github.com/moby/sys/mount v0.2.0 | ||
github.com/opencontainers/go-digest v1.0.0 | ||
github.com/opencontainers/image-spec v1.0.2-0.20211102003311-9a7a9876500e | ||
github.com/opencontainers/runtime-spec v1.0.3-0.20211101234015-a3c33d663ebc | ||
github.com/pkg/errors v0.9.1 |
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.
Shouldn't be needed
docs/ipfs.md
Outdated
bafkreifajsysbvhtgd7fdgrfesszexdq6v5zbj5y2jnjfwxdjyqws2s3s4 | ||
``` | ||
|
||
You can pull the encrypted image from IPFS using `ipfs://` prefix and can decrypt it in the same way as described in [`/docs/ocycrypt.md`](/docs/ocicrypt.md). |
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.
/docs/
-> ./
docs/ipfs.md
Outdated
hello | ||
``` | ||
|
||
:information_source: Note that though the IPFS-enabled image is OCI compatible, some runtimes including [containerd](https://github.com/containerd/containerd/pull/6221) and [podman](https://github.com/containers/image/pull/1403)) had bugs and failed to pull that image. Containerd has already fixed 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.
:information_source: Note that though the IPFS-enabled image is OCI compatible, some runtimes including [containerd](https://github.com/containerd/containerd/pull/6221) and [podman](https://github.com/containers/image/pull/1403)) had bugs and failed to pull that image. Containerd has already fixed this. | |
:information_source: Note that though the IPFS-enabled image is OCI compatible, some runtimes including [containerd](https://github.com/containerd/containerd/pull/6221) and [podman](https://github.com/containers/image/pull/1403) had bugs and failed to pull that image. Containerd has already fixed this. |
Looks good but a couple of nits. |
Probably now we have to increase the CI timeout nerdctl/.github/workflows/test.yml Line 55 in f316402
|
I'll fix it in this PR. |
e085893
to
fdc5335
Compare
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
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.
Thanks
#465
This commit enables image distribution on IPFS when
ipfs://
prefix is specified before the image name.(
ipfs daemon
needs running)push: pushes the specified image to IPFS
The following example pushes
ubuntu:20.04
(pulled from DockerHub withnerdctl pull ubuntu:20.04
in advance) to IPFSpull: pulls the specified image from IFPS
run:
compose:
encryption (needs to disable estargz conversoin):
This commit also addsnerdctl ipfs push
andnerdctl ipfs pull
subcommands as the low-level CLI can be used for debugging.Lazy pulling with stagz snapshotter (doc) is enabled when
--snapshotter=stargz
is specified.nerdctl build
should support pulling base images from IPFS (can be a following-up PR)http(s)://
urls containerd#6221