IPFS: use ipfs binary instead of library #1653

ktock · 2022-12-14T04:25:49Z

This commit reduces dependencies for ipfs library by using ipfs binary.
nerdctl ipfs registry up and nerdctl ipfs registry down now uses systemd for managing nerdctl ipfs registry serve because it now depends on ipfs command available on the host rootfs.

TODO
- agreement on design
- ~~update examples~~ will be done after we release the next version
- ~~remove github.com/ipfs/go-cid v0.3.2 (CID parser) dependency (following up PR?)~~
  - ~~We need to reimplement the parser to remove that dependency. ~~

AkihiroSuda · 2022-12-16T18:12:34Z

Thanks, you can retain go-cid and call it a day

djdongjin · 2022-12-16T19:46:52Z

cmd/nerdctl/ipfs_linux_test.go

-	c, err := cid.Decode(lines[len(lines)-2])
-	assert.NilError(t, err)
-	return "ipfs://" + c.String()
+	return "ipfs://" + lines[len(lines)-2]
 }

 func requiresIPFS(t *testing.T) {


maybe put in testutil, similar to other RequireXXXX.

https://github.com/containerd/nerdctl/blob/e2d618f283372f19c4fc52b809d6f6e1944ef8d3/pkg/testutil/testutil.go

djdongjin · 2022-12-16T19:57:18Z

docs/ipfs.md

@@ -159,14 +167,19 @@ When you specify `--ipfs` option to `nerdctl build`, it automatically starts the
 By default, nerdctl exposes the registry at `localhost:5050`.
 You can change the address and can manually restart the registry using `nerdctl ipfs registry up` and `nerdctl ipfs registry down`.

+To make these commands work correctly, you need to install `nerdctl ipfs registry serve` command as a systemd service named `nerdctl-ipfs-registry-serve`.


Maybe we can have a sample systemd service file that can be used directly by users?

AkihiroSuda · 2022-12-20T04:14:11Z

Dockerfile.d/test-integration-nerdctl-ipfs-registry-serve.service

+Description=nerdctl ipfs registry for integration test
+
+[Service]
+EnvironmentFile=/run/nerdctl-ipfs-registry-serve/args


s/args/env/ maybe

AkihiroSuda · 2022-12-20T04:16:13Z

serve

~~server, service, or just drop it~~ (Existing command, so no need to change)

AkihiroSuda · 2022-12-20T04:19:29Z

cmd/nerdctl/ipfs_registry_up.go

+Instead of using this command, you can also directly launch the registry using "nerdctl ipfs registry serve".
+
+Use "nerdctl ipfs registry down" to stop the registry service launced by "nerdctl ipfs registry up".
+`,


Maybe we can just drop the up/down command and let the user install the systemd unit by themselves

AkihiroSuda · 2022-12-20T23:43:30Z

docs/ipfs.md


 ```console
-> nerdctl build --ipfs -t hello .


The document may note that this flag (and up/down) is being removed in nerdctl v1.2.0.

AkihiroSuda · 2022-12-20T23:44:40Z

cmd/nerdctl/build.go

@@ -70,7 +70,7 @@ If Dockerfile is not present and -f is not specified, it will look for Container
 	buildCommand.RegisterFlagCompletionFunc("platform", shellCompletePlatforms)
 	// #endregion

-	buildCommand.Flags().Bool("ipfs", false, "Allow pulling base images from IPFS")
+	// buildCommand.Flags().Bool("ipfs", false, "Allow pulling base images from IPFS")


AkihiroSuda · 2022-12-21T02:27:52Z

Still draft?

ktock · 2022-12-21T02:30:02Z

Thank you for the review.

djdongjin · 2022-12-21T02:39:20Z

pkg/testutil/testutil.go

@@ -604,6 +604,12 @@ func RequireExecutable(t testing.TB, name string) {
 	}
 }

+func RequireIPFS(t *testing.T) {


nit: maybe this function can be replaced by just using RequireExecutable("ipfs") above.

ktock · 2022-12-22T05:54:22Z

It looks like the ipfs registry sometimes breaks. I'll fix it.

WARN[0215] ipfs command failed                           error="exit status 1" stderr="Error: lock /home/rootless/.ipfs/repo.lock: someone else has the lock\n"
WARN[0216] ipfs command failed                           error="exit status 1" stderr="Error: lock /home/rootless/.ipfs/repo.lock: someone else has the lock\n"
WARN[0217] ipfs command failed                           error="exit status 1" stderr="Error: lock /home/rootless/.ipfs/repo.lock: someone else has the lock\n"
WARN[0218] ipfs command failed                           error="exit status 1" stderr="Error: lock /home/rootless/.ipfs/repo.lock: someone else has the lock\n"
WARN[0219] ipfs command failed                           error="exit status 1" stderr="Error: lock /home/rootless/.ipfs/repo.lock: someone else has the lock\n"

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

AkihiroSuda

Thanks

AkihiroSuda mentioned this pull request Dec 16, 2022

Bump github.com/ipfs/interface-go-ipfs-core from 0.8.0 to 0.8.1 #1647

Closed

AkihiroSuda added this to the v1.2.0 (tentative) milestone Dec 16, 2022

djdongjin reviewed Dec 16, 2022

View reviewed changes

AkihiroSuda added the area/ipfs IPFS label Dec 18, 2022

ktock force-pushed the kubo-bin branch 2 times, most recently from efc03a3 to cd9f43c Compare December 19, 2022 15:05

ktock marked this pull request as ready for review December 19, 2022 23:32

AkihiroSuda reviewed Dec 20, 2022

View reviewed changes

ktock force-pushed the kubo-bin branch 2 times, most recently from 6d4c784 to 72b95ea Compare December 20, 2022 14:34

ktock marked this pull request as draft December 20, 2022 22:35

ktock force-pushed the kubo-bin branch from 72b95ea to 2ae76b7 Compare December 20, 2022 23:21

AkihiroSuda reviewed Dec 20, 2022

View reviewed changes

AkihiroSuda approved these changes Dec 20, 2022

View reviewed changes

ktock force-pushed the kubo-bin branch 2 times, most recently from ce532c0 to 52d3859 Compare December 21, 2022 00:54

ktock marked this pull request as ready for review December 21, 2022 02:29

djdongjin reviewed Dec 21, 2022

View reviewed changes

ktock marked this pull request as draft December 22, 2022 05:54

ktock force-pushed the kubo-bin branch from 52d3859 to 234ed05 Compare December 22, 2022 07:03

IPFS: use ipfs binary instead of library

5546d6d

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

ktock force-pushed the kubo-bin branch from 234ed05 to 5546d6d Compare December 22, 2022 07:08

ktock marked this pull request as ready for review December 22, 2022 07:54

AkihiroSuda approved these changes Dec 22, 2022

View reviewed changes

AkihiroSuda merged commit 2735dfc into containerd:main Dec 22, 2022

ktock deleted the kubo-bin branch December 26, 2022 11:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IPFS: use ipfs binary instead of library #1653

IPFS: use ipfs binary instead of library #1653

ktock commented Dec 14, 2022 •

edited

Loading

AkihiroSuda commented Dec 16, 2022

djdongjin Dec 16, 2022

djdongjin Dec 16, 2022

AkihiroSuda Dec 20, 2022

AkihiroSuda commented Dec 20, 2022 •

edited

Loading

AkihiroSuda Dec 20, 2022

AkihiroSuda Dec 20, 2022

AkihiroSuda Dec 20, 2022

AkihiroSuda commented Dec 21, 2022

ktock commented Dec 21, 2022

djdongjin Dec 21, 2022

ktock commented Dec 22, 2022

AkihiroSuda left a comment

IPFS: use ipfs binary instead of library #1653

IPFS: use ipfs binary instead of library #1653

Conversation

ktock commented Dec 14, 2022 • edited Loading

AkihiroSuda commented Dec 16, 2022

djdongjin Dec 16, 2022

Choose a reason for hiding this comment

djdongjin Dec 16, 2022

Choose a reason for hiding this comment

AkihiroSuda Dec 20, 2022

Choose a reason for hiding this comment

AkihiroSuda commented Dec 20, 2022 • edited Loading

AkihiroSuda Dec 20, 2022

Choose a reason for hiding this comment

AkihiroSuda Dec 20, 2022

Choose a reason for hiding this comment

AkihiroSuda Dec 20, 2022

Choose a reason for hiding this comment

AkihiroSuda commented Dec 21, 2022

ktock commented Dec 21, 2022

djdongjin Dec 21, 2022

Choose a reason for hiding this comment

ktock commented Dec 22, 2022

AkihiroSuda left a comment

Choose a reason for hiding this comment

ktock commented Dec 14, 2022 •

edited

Loading

AkihiroSuda commented Dec 20, 2022 •

edited

Loading