Skip to content

Add support for overlay volatile option #8402

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.

Sign up for GitHub

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

Closed
natefive wants to merge 1 commit into from
Closed

Add support for overlay volatile option #8402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions mount/temp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,24 @@ func WithTempMount(ctx context.Context, mounts []Mount, f func(root string) erro
}
}
}()

// The volatile option of overlayfs doesn't allow to mount again using the
Copy link
Member

@fuweid fuweid May 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should change the option here. If there is conflict, we should modify the option before call the WithTempMount, for instance, https://github.com/containerd/containerd/pull/8259/files.

Copy link
Author

@natefive natefive Jun 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are lots of calls to WithTempMount? should I create another function which modifies the options calls before calling WithTempMount then update those other calls to call the new function? Feels like pointless nesting maybe I am misunderstanding, would be helpful if that link has some line numbers :) ty

Copy link
Member

@dmcgowan dmcgowan Jun 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is probably the right place to strip the option. Need to ensure the input mount array is not overwritten though.

Copy link
Member

@fuweid fuweid Jun 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@natefive Sorry for late reply.

should I create another function which modifies the options calls before calling WithTempMount then update those other calls to call the new function?

For my original comment, yes. But recently, I checked the code about starting container. It's unlikely to change the mount since there is no entrypoint for us to update it before passing to shim.

containerd/container.go

Lines 244 to 267 in b6abda7

s, err := c.client.getSnapshotter(ctx, r.Snapshotter)
if err != nil {
return nil, err
}
mounts, err := s.Mounts(ctx, r.SnapshotKey)
if err != nil {
return nil, err
}
spec, err := c.Spec(ctx)
if err != nil {
return nil, err
}
for _, m := range mounts {
if spec.Linux != nil && spec.Linux.MountLabel != "" {
if ml := label.FormatMountLabel("", spec.Linux.MountLabel); ml != "" {
m.Options = append(m.Options, ml)
}
}
request.Rootfs = append(request.Rootfs, &types.Mount{
Type: m.Type,
Source: m.Source,
Target: m.Target,
Options: m.Options,
})

I think we should introduce a label like #6528.

Maybe we don't need to persist the label in boltdb. We can pass the label by gRPC context and get mount with the option when we want to get the mount.

cc @dmcgowan

// same upper / work dirs. Since it's a temp mount, avoid using that
// option here.
// TODO: Make this logic conditional once the kernel supports reusing
// overlayfs volatile mounts.
for _, m := range mounts {
if m.Type != "overlay" {
continue
}
for i, opt := range m.Options {
if opt == "volatile" {
m.Options[i] = ""
break
}
}
}

if uerr = All(mounts, root); uerr != nil {
return fmt.Errorf("failed to mount %s: %w", root, uerr)
}
Expand Down
21 changes: 21 additions & 0 deletions snapshots/overlay/overlay.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const upperdirKey = "containerd.io/snapshot/overlay.upperdir"
type SnapshotterConfig struct {
asyncRemove bool
upperdirLabel bool
volatile bool
}

// Opt is an option to configure the overlay snapshotter
Expand All @@ -67,13 +68,21 @@ func WithUpperdirLabel(config *SnapshotterConfig) error {
return nil
}

// Volatile enables the volatile option of overlayfs to omit all
// forms of sync calls to the upper layer filesystem.
func Volatile(config *SnapshotterConfig) error {
config.volatile = true
return nil
}

type snapshotter struct {
root string
ms *storage.MetaStore
asyncRemove bool
upperdirLabel bool
indexOff bool
userxattr bool // whether to enable "userxattr" mount option
volatile bool
}

// NewSnapshotter returns a Snapshotter which uses overlayfs. The overlayfs
Expand Down Expand Up @@ -111,13 +120,21 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) {
logrus.WithError(err).Warnf("cannot detect whether \"userxattr\" option needs to be used, assuming to be %v", userxattr)
}

// check volatile option
if config.volatile {
if err := overlayutils.SupportsVolatileMount(root); err != nil {
return nil, err
}
}

return &snapshotter{
root: root,
ms: ms,
asyncRemove: config.asyncRemove,
upperdirLabel: config.upperdirLabel,
indexOff: supportsIndex(),
userxattr: userxattr,
volatile: config.volatile,
}, nil
}

Expand Down Expand Up @@ -464,6 +481,10 @@ func (o *snapshotter) mounts(s storage.Snapshot) []mount.Mount {
options = append(options, "userxattr")
}

if o.volatile {
options = append(options, "volatile")
Copy link
Member

@fuweid fuweid May 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overlayfs snapshot is service and this option is global setting. I think there are existing cases which required the rootfs is synced after umount. Currently, the kubernetes container is temporary and we can't restart a exited container. kubernetes always recreates container for pod. So, I would like to add the volatile option from caller, for example, CRI plugin.

I was thinking that we can go in this way:

a. When the overlayfs snapshot inits, it use SupportsVolatileMount to check and add the capability in Meta.

containerd/snapshots/overlay/plugin/plugin.go

Lines 41 to 61 in e52fbfd

InitFn: func(ic *plugin.InitContext) (interface{}, error) {
ic.Meta.Platforms = append(ic.Meta.Platforms, platforms.DefaultSpec())
config, ok := ic.Config.(*Config)
if !ok {
return nil, errors.New("invalid overlay configuration")
}
root := ic.Root
if config.RootPath != "" {
root = config.RootPath
}
var oOpts []overlay.Opt
if config.UpperdirLabel {
oOpts = append(oOpts, overlay.WithUpperdirLabel)
}
ic.Meta.Exports["root"] = root
return overlay.NewSnapshotter(root, append(oOpts, overlay.AsynchronousRemove)...)
},

b. Add the mount option to add volatile if snapshot has the capability. Currently, the CRI UserNS feature is working with capability. You can check the main branch for this. And just in case, we should add CRI option to turn this feature on.

}

if s.Kind == snapshots.KindActive {
options = append(options,
fmt.Sprintf("workdir=%s", o.workPath(s.ID)),
Expand Down
36 changes: 36 additions & 0 deletions snapshots/overlay/overlayutils/check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,42 @@ const (
tmpfsMagic = 0x01021994
)

// SupportsVolatileMount checks if the system supports the volatile mount option with the overlay filesystem.
// This function creates a temporary directory, mounts an overlay filesystem on it with the volatile mount option enabled,
// and then unmounts it.
func SupportsVolatileMount(d string) error {
td, err := os.MkdirTemp(d, "volatile-check")
if err != nil {
return err
}
defer func() {
if err := os.RemoveAll(td); err != nil {
log.L.WithError(err).Warnf("Failed to remove volatile check directory %v", td)
}
}()

for _, dir := range []string{"lower", "upper", "work", "merged"} {
if err := os.Mkdir(filepath.Join(td, dir), 0755); err != nil {
return err
}
}

opts := fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s,volatile", filepath.Join(td, "lower"), filepath.Join(td, "upper"), filepath.Join(td, "work"))
m := mount.Mount{
Type: "overlay",
Source: "overlay",
Options: []string{opts},
}
dest := filepath.Join(td, "merged")
if err := m.Mount(dest); err != nil {
return fmt.Errorf("failed to mount volatile overlay: %w", err)
}
if err := mount.UnmountAll(dest, 0); err != nil {
log.L.WithError(err).Warnf("Failed to unmount volatile check directory %v", dest)
}
return nil
}

// SupportsMultipleLowerDir checks if the system supports multiple lowerdirs,
// which is required for the overlay snapshotter. On 4.x kernels, multiple lowerdirs
// are always available (so this check isn't needed), and backported to RHEL and
Expand Down
5 changes: 5 additions & 0 deletions snapshots/overlay/plugin/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type Config struct {
// Root directory for the plugin
RootPath string `toml:"root_path"`
UpperdirLabel bool `toml:"upperdir_label"`
Volatile bool `toml:"volatile"`
}

func init() {
Expand All @@ -56,6 +57,10 @@ func init() {
oOpts = append(oOpts, overlay.WithUpperdirLabel)
}

if config.Volatile {
oOpts = append(oOpts, overlay.Volatile)
}

ic.Meta.Exports["root"] = root
return overlay.NewSnapshotter(root, append(oOpts, overlay.AsynchronousRemove)...)
},
Expand Down