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

[release/1.7] Fix CRI snapshotter root path when not under containerd root #10096

Merged
merged 4 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/cri/cri.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func init() {
plugin.ServicePlugin,
plugin.NRIApiPlugin,
plugin.WarningPlugin,
plugin.SnapshotPlugin,
},
InitFn: initCRIService,
})
Expand Down
49 changes: 46 additions & 3 deletions pkg/cri/server/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
package server

import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -126,6 +128,11 @@ type criService struct {
func NewCRIService(config criconfig.Config, client *containerd.Client, nri *nri.API, warn warning.Service) (CRIService, error) {
var err error
labels := label.NewStore()

if client.SnapshotService(config.ContainerdConfig.Snapshotter) == nil {
return nil, fmt.Errorf("failed to find snapshotter %q", config.ContainerdConfig.Snapshotter)
}

c := &criService{
config: config,
client: client,
Expand All @@ -149,7 +156,11 @@ func NewCRIService(config criconfig.Config, client *containerd.Client, nri *nri.
return nil, fmt.Errorf("failed to find snapshotter %q", c.config.ContainerdConfig.Snapshotter)
}

c.imageFSPath = imageFSPath(config.ContainerdRootDir, config.ContainerdConfig.Snapshotter)
c.imageFSPath = imageFSPath(
config.ContainerdRootDir,
config.ContainerdConfig.Snapshotter,
client,
)
logrus.Infof("Get image filesystem path %q", c.imageFSPath)

if err := c.initPlatform(); err != nil {
Expand Down Expand Up @@ -341,8 +352,40 @@ func (c *criService) register(s *grpc.Server) error {

// imageFSPath returns containerd image filesystem path.
// Note that if containerd changes directory layout, we also needs to change this.
func imageFSPath(rootDir, snapshotter string) string {
return filepath.Join(rootDir, fmt.Sprintf("%s.%s", plugin.SnapshotPlugin, snapshotter))
func imageFSPath(rootDir, snapshotter string, client *containerd.Client) string {
introspection := func() (string, error) {
filters := []string{fmt.Sprintf("type==%s, id==%s", plugin.SnapshotPlugin, snapshotter)}
in := client.IntrospectionService()

resp, err := in.Plugins(context.Background(), filters)
if err != nil {
return "", err
}

if len(resp.Plugins) <= 0 {
return "", fmt.Errorf("inspection service could not find snapshotter %s plugin", snapshotter)
}

sn := resp.Plugins[0]
if root, ok := sn.Exports[plugin.SnapshotterRootDir]; ok {
return root, nil
}
return "", errors.New("snapshotter does not export root path")
}

var imageFSPath string
path, err := introspection()
if err != nil {
logrus.WithError(err).WithField("snapshotter", snapshotter).Warn("snapshotter doesn't export root path")
imageFSPath = filepath.Join(
rootDir,
plugin.SnapshotPlugin.String()+"."+snapshotter,
)
} else {
imageFSPath = path
}

return imageFSPath
}

func loadOCISpec(filename string) (*oci.Spec, error) {
Expand Down
4 changes: 4 additions & 0 deletions plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ const (
DeprecationsPlugin = "deprecations"
)

const (
SnapshotterRootDir = "root"
)

// Registration contains information for registering a plugin
type Registration struct {
// Type of the plugin
Expand Down
6 changes: 4 additions & 2 deletions services/server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,10 @@ type CgroupConfig struct {

// ProxyPlugin provides a proxy plugin configuration
type ProxyPlugin struct {
Type string `toml:"type"`
Address string `toml:"address"`
Type string `toml:"type"`
Address string `toml:"address"`
Platform string `toml:"platform"`
Exports map[string]string `toml:"exports"`
Comment on lines +169 to +170
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need these? It's better to keep the existing configs for compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do need Exports, but we don't need Platform.

I kept the Platform change because it was implemented before Exports and so I thought it would be cleaner (for a vague sense of the word "clean") to take both rather than just the newer piece that I needed.

I can remove the Platform backport if needed.

}

// Decode unmarshals a plugin specific configuration by plugin id
Expand Down
21 changes: 20 additions & 1 deletion services/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"github.com/docker/go-metrics"
grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
v1 "github.com/opencontainers/image-spec/specs-go/v1"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"google.golang.org/grpc"
"google.golang.org/grpc/backoff"
Expand All @@ -57,6 +58,7 @@ import (
"github.com/containerd/containerd/pkg/deprecation"
"github.com/containerd/containerd/pkg/dialer"
"github.com/containerd/containerd/pkg/timeout"
"github.com/containerd/containerd/platforms"
"github.com/containerd/containerd/plugin"
srvconfig "github.com/containerd/containerd/services/server/config"
"github.com/containerd/containerd/services/warning"
Expand Down Expand Up @@ -454,6 +456,8 @@ func LoadPlugins(ctx context.Context, config *srvconfig.Config) ([]*plugin.Regis
f func(*grpc.ClientConn) interface{}

address = pp.Address
p v1.Platform
err error
)

switch pp.Type {
Expand All @@ -477,12 +481,27 @@ func LoadPlugins(ctx context.Context, config *srvconfig.Config) ([]*plugin.Regis
default:
log.G(ctx).WithField("type", pp.Type).Warn("unknown proxy plugin type")
}
if pp.Platform != "" {
p, err = platforms.Parse(pp.Platform)
if err != nil {
log.G(ctx).WithError(err).WithField("plugin", name).Warn("skipping proxy platform with bad platform")
}
} else {
p = platforms.DefaultSpec()
}

exports := pp.Exports
if exports == nil {
exports = map[string]string{}
}
exports["address"] = address

plugin.Register(&plugin.Registration{
Type: t,
ID: name,
InitFn: func(ic *plugin.InitContext) (interface{}, error) {
ic.Meta.Exports["address"] = address
ic.Meta.Exports = exports
ic.Meta.Platforms = append(ic.Meta.Platforms, p)
conn, err := clients.getClient(address)
if err != nil {
return nil, err
Expand Down
1 change: 1 addition & 0 deletions snapshots/blockfile/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func init() {
opts = append(opts, blockfile.WithMountOptions(config.MountOptions))
}

ic.Meta.Exports[plugin.SnapshotterRootDir] = root
return blockfile.NewSnapshotter(root, opts...)
},
})
Expand Down
2 changes: 1 addition & 1 deletion snapshots/btrfs/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func init() {
root = config.RootPath
}

ic.Meta.Exports = map[string]string{"root": root}
ic.Meta.Exports[plugin.SnapshotterRootDir] = root
return btrfs.NewSnapshotter(root)
},
})
Expand Down
1 change: 1 addition & 0 deletions snapshots/devmapper/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func init() {
config.RootPath = ic.Root
}

ic.Meta.Exports[plugin.SnapshotterRootDir] = config.RootPath
return devmapper.NewSnapshotter(ic.Context, config)
},
})
Expand Down
1 change: 1 addition & 0 deletions snapshots/native/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func init() {
root = config.RootPath
}

ic.Meta.Exports[plugin.SnapshotterRootDir] = root
return native.NewSnapshotter(root)
},
})
Expand Down
2 changes: 1 addition & 1 deletion snapshots/overlay/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func init() {
oOpts = append(oOpts, overlay.WithMountOptions(config.MountOptions))
}

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