Skip to content
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

See if upstream wants to make overlayfs snapshotter embeddable #10

Closed
elpdt852 opened this issue Jul 10, 2023 · 2 comments · Fixed by #45
Closed

See if upstream wants to make overlayfs snapshotter embeddable #10

elpdt852 opened this issue Jul 10, 2023 · 2 comments · Fixed by #45

Comments

@elpdt852
Copy link
Collaborator

So at a high-level, nix-snapshotter is basically overlayfs snapshotter but with additional support for nix layers.

Currently there's a lot of code duplication between pkg/nix/nix.go and the overlayfs snapshotter from containerd: https://github.com/containerd/containerd/blob/main/snapshots/overlay/overlay.go

This is because we use private members like o.ms:

	ctx, t, err := o.ms.TransactionContext(ctx, false)

to handle the metadata layer's bolt transactions.

If we look around the remote snapshotter ecosystem, you'll see this is duplicated the same way:
https://github.com/containerd/stargz-snapshotter/blob/main/snapshot/snapshot.go

We should consider upstreaming a refactor to make it possible to embed it, thus deleting a lot of code from pkg/nix/nix.go.

@elpdt852
Copy link
Collaborator Author

Summarizing our offline discussion:

  • We have two types of dependency on containerd. An IPC-level one (based on GRPC protocol-level compatibility), and a library-level one (consuming containerd golang packages)
  • We should create an upstream GitHub issue as that will take a long time
  • We should fork containerd to make the changes to overlay snapshotter so that it's embedabble
  • We can point to this fork to greatly reduce code in pkg/nix/nix.go without breaking our IPC-level dependency (i.e. we can build nix-snapshotter against the fork but still run as a snapshotter for a release version of containerd)

@elpdt852
Copy link
Collaborator Author

elpdt852 commented Aug 22, 2023

@rbpdt This has been merged, congrats! We should unfork containerd in our go.mod to close this issue.

containerd/containerd#8945

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant