Add fuse-manager #1892

wswsmao · 2024-12-09T09:19:04Z

Thanks to @ilyee, the author of PR #324 for his permission, this work can be resumed.

In light of the project's evolution over the past two years, the following adjustments have been made compared to the original content:

Two configuration packages have been added, including fsopts and keychainconfig. The main file now utilizes the interfaces provided by the configuration package to set up the main process, eliminating the need for each main package to repeat the configuration steps.
Resource cleanup has been implemented for fusemanager.
Feedback from the review of PR #324 has been addressed.

ktock

Thanks. Could you add Co-authored-by for @ilyee in the commit messages?

ktock · 2024-12-11T14:04:07Z

cmd/containerd-stargz-grpc/main.go

+		fmAddr := config.FuseManagerAddress
+		if fmAddr == "" {
+			var err error
+			fmAddr, err = exec.LookPath(fuseManagerAddress)


Please always expect an absolute path.

ktock · 2024-12-11T14:06:09Z

cmd/go.mod

 	github.com/opencontainers/go-digest v1.0.0
 	github.com/opencontainers/image-spec v1.1.0
 	github.com/opencontainers/runtime-spec v1.2.0
 	github.com/pelletier/go-toml v1.9.5
+	github.com/pkg/errors v0.9.1


Please do not use this package because this has been archived

ktock · 2024-12-11T14:08:33Z

cmd/stargz-fuse-manager/fusemanager/service.go

+	Config                     *service.Config
+	IPFS                       bool
+	MetadataStore              string
+	DefaultImageServiceAddress string


Can you add JSON tags?

ktock · 2024-12-11T14:09:08Z

cmd/stargz-fuse-manager/fusemanager/api/api.proto

@@ -0,0 +1,58 @@
+/*


Can we have fusemanager package in the repo root?

Can we have fusemanager package in the repo root?

I have tried two approaches:

Placing the fusemanager in the repository root. However, since the fsopts package is located in the cmd directory, building directly leads to issues where fusemanager cannot find fsopts. This requires adding fsopts separately to the go.mod file, which would introduce a significant number of dependencies.

Keeping the service.go file that depends on fsopts, along with the fusemanager.go and fusestore.go files, in the cmd/stargz-fuse-manager directory, while only moving the client-side content to the repository root.

I was wondering if there might be a better solution

Can we pass fsopts.ConfigFsOpts to fusemanager as a callback function so that we can eliminate direct dependency from fusemanager to fsopts?

ktock · 2024-12-11T14:11:43Z

service/service.go

-	snOpts := []snbase.Opt{snbase.AsynchronousRemove}
-	if config.SnapshotterConfig.AllowInvalidMountsOnRestart {
-		snOpts = append(snOpts, snbase.AllowInvalidMountsOnRestart)
+func StartFuseManager(ctx context.Context, executable, address, fusestore, logLevel, logPath string) error {


Can we put this in fusemounter package because this seem to be independent to service package.

done，just mv it to fusemanager/fusemanager.go

ktock · 2024-12-11T14:12:24Z

cmd/containerd-stargz-grpc/fsopts/fsopts.go

+
+	mt, err := getMetadataStore(rootDir, config)
+	if err != nil {
+		log.G(ctx).WithError(err).Fatalf("failed to configure metadata store")


Return an error.

Fatal exits the process here. This line isn't needed and just do return nil, fmt.Errorf("failed to configure metadata store: %w", err)
The same applies to other places as well.

ktock · 2024-12-11T14:12:42Z

cmd/stargz-fuse-manager/fusemanager/fusemanager.go

+func runFuseManager(ctx context.Context) error {
+	lvl, err := logrus.ParseLevel(logLevel)
+	if err != nil {
+		log.L.WithError(err).Fatal("failed to prepare logger")


Return an error.

ktock · 2024-12-11T14:12:52Z

cmd/stargz-fuse-manager/fusemanager/service.go

+
+	credsFuncs, err := keychainconfig.ConfigKeychain(ctx, fm.server, &keyChainConfig)
+	if err != nil {
+		log.G(ctx).WithError(err).Fatalf("failed to configure keychain")


Return an error (also the same for other places in this file)

ktock · 2024-12-11T14:13:33Z

service/service.go

@@ -66,6 +69,27 @@ func WithFilesystemOptions(opts ...stargzfs.Option) Option {

 // NewStargzSnapshotterService returns stargz snapshotter.
 func NewStargzSnapshotterService(ctx context.Context, root string, config *Config, opts ...Option) (snapshots.Snapshotter, error) {
+	fs, err := NewFileSystem(ctx, root, config, opts...)
+	if err != nil {
+		log.G(ctx).WithError(err).Fatalf("failed to configure filesystem")


Return an error (also the same for other places in this file)

wswsmao · 2024-12-12T03:57:10Z

Thanks. Could you add Co-authored-by for @ilyee in the commit messages?

done， I have squash these commit and add Co-authored-by for @ilyee

ktock

We need to enable fusemanager in the tests in our CI (can be following up though)

ktock · 2024-12-13T07:55:10Z

fusemanager/service.go

+	return nil
+}
+
+func (fm *Server) clearMounts(ctx context.Context) error {


Stargz snapshotter (snapshot/snapshot.go) already has the similar cleanup logic. Can we just rely on it or why we need this in fusemanager? It needs documents about how to restart fusemanager (e.g. for upgrading).

done

clearMounts is unnecessary and has been deleted.

add Upgrading Fuse Manager to overview.md

ktock · 2024-12-13T08:00:55Z

fusemanager/fusemanager.go

+func runFuseManager(ctx context.Context) error {
+	lvl, err := logrus.ParseLevel(logLevel)
+	if err != nil {
+		log.L.WithError(err).Fatal("failed to prepare logger")


Same as #1892 (comment)

ktock · 2024-12-13T08:01:07Z

service/service.go

+
+	snapshotter, err = snbase.NewSnapshotter(ctx, snapshotterRoot(root), fs, snOpts...)
+	if err != nil {
+		log.G(ctx).WithError(err).Fatalf("failed to create new snapshotter")


Same as #1892 (comment)

ktock · 2024-12-13T08:03:26Z

service/service.go

@@ -66,6 +67,29 @@ func WithFilesystemOptions(opts ...stargzfs.Option) Option {

 // NewStargzSnapshotterService returns stargz snapshotter.
 func NewStargzSnapshotterService(ctx context.Context, root string, config *Config, opts ...Option) (snapshots.Snapshotter, error) {
+	fs, err := NewFileSystem(ctx, root, config, opts...)
+	if err != nil {
+		log.G(ctx).WithError(err).Fatalf("failed to configure filesystem")


Same as #1892 (comment)

Signed-off-by: abushwang <[email protected]> Co-authored-by: Zuti He <[email protected]>

ktock

Thanks

wswsmao force-pushed the main branch 11 times, most recently from 80cee15 to e015180 Compare December 11, 2024 07:51

ktock reviewed Dec 11, 2024

View reviewed changes

wswsmao force-pushed the main branch from e015180 to c46cbf2 Compare December 12, 2024 03:53

wswsmao force-pushed the main branch 2 times, most recently from ffc8b65 to 58a325c Compare December 12, 2024 09:26

ktock reviewed Dec 13, 2024

View reviewed changes

wswsmao force-pushed the main branch 3 times, most recently from 48731b5 to 29532df Compare December 13, 2024 09:02

Add fuse-manager

ce7fc7b

Signed-off-by: abushwang <[email protected]> Co-authored-by: Zuti He <[email protected]>

wswsmao force-pushed the main branch from 29532df to ce7fc7b Compare December 13, 2024 09:08

ktock approved these changes Dec 13, 2024

View reviewed changes

ktock merged commit 4e10317 into containerd:main Dec 13, 2024
26 checks passed

ktock mentioned this pull request Dec 13, 2024

[WIP] Add fuse-manager #324

Closed

wswsmao mentioned this pull request Dec 17, 2024

fuse-manager: add tests #1912

Merged

ktock mentioned this pull request Dec 19, 2024

CI: test fuse manager #1914

Merged

ktock mentioned this pull request Jul 8, 2025

Make graceful restarting available in wider configurations #2077

Merged

Add fuse-manager #1892

Add fuse-manager #1892

Conversation

wswsmao commented Dec 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ktock left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wswsmao Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wswsmao commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ktock left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktock left a comment

Choose a reason for hiding this comment

Uh oh!

wswsmao commented Dec 9, 2024 •

edited

Loading

wswsmao Dec 12, 2024 •

edited

Loading

wswsmao commented Dec 12, 2024 •

edited

Loading