Use containerd plugin config.

Signed-off-by: Lantao Liu <lantaol@google.com>

cmd/cri-containerd/cri_containerd.go

@@ -30,7 +30,6 @@ import (
 	"github.com/containerd/cgroups"
 	"github.com/containerd/containerd/sys"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/opencontainers/selinux/go-selinux"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"k8s.io/kubernetes/pkg/util/interrupt"
@@ -90,7 +89,6 @@ func main() {
 		if err := o.InitFlags(cmd.Flags()); err != nil {
 			return fmt.Errorf("failed to init CRI containerd flags: %v", err)
 		}
-		validateConfig(o)
 
 		if err := setLogLevel(o.LogLevel); err != nil {
 			return fmt.Errorf("failed to set log level: %v", err)
@@ -118,7 +116,7 @@ func main() {
 		}
 
 		logrus.Infof("Run cri-containerd grpc server on socket %q", o.SocketPath)
-		s, err := server.NewCRIContainerdService(o.Config)
+		s, err := server.NewCRIContainerdService(o.CRIConfig)
 		if err != nil {
 			return fmt.Errorf("failed to create CRI containerd service: %v", err)
 		}
@@ -138,16 +136,6 @@ func main() {
 	}
 }
 
-func validateConfig(o *options.CRIContainerdOptions) {
-	if o.EnableSelinux {
-		if !selinux.GetEnabled() {
-			logrus.Warn("Selinux is not supported")
-		}
-	} else {
-		selinux.SetDisabled()
-	}
-}
-
 func setupDumpStacksTrap() {
 	c := make(chan os.Signal, 1)
 	signal.Notify(c, syscall.SIGUSR1)

cmd/cri-containerd/options/options.go

@@ -35,13 +35,15 @@ const (
 	connectionTimeout = 10 * time.Second
 )
 
-// ContainerdConfig contains config related to containerd
+// ContainerdConfig contains toml config related to containerd
 type ContainerdConfig struct {
 	// RootDir is the root directory path for containerd.
+	// TODO(random-liu): Remove this field when no longer support cri-containerd standalone mode.
 	RootDir string `toml:"root_dir" json:"rootDir,omitempty"`
 	// Snapshotter is the snapshotter used by containerd.
 	Snapshotter string `toml:"snapshotter" json:"snapshotter,omitempty"`
 	// Endpoint is the containerd endpoint path.
+	// TODO(random-liu): Remove this field when no longer support cri-containerd standalone mode.
 	Endpoint string `toml:"endpoint" json:"endpoint,omitempty"`
 	// Runtime is the runtime to use in containerd. We may support
 	// other runtimes in the future.
@@ -55,7 +57,7 @@ type ContainerdConfig struct {
 	RuntimeRoot string `toml:"runtime_root" json:"runtimeRoot,omitempty"`
 }
 
-// CniConfig contains config related to cni
+// CniConfig contains toml config related to cni
 type CniConfig struct {
 	// NetworkPluginBinDir is the directory in which the binaries for the plugin is kept.
 	NetworkPluginBinDir string `toml:"bin_dir" json:"binDir,omitempty"`
@@ -63,23 +65,17 @@ type CniConfig struct {
 	NetworkPluginConfDir string `toml:"conf_dir" json:"confDir,omitempty"`
 }
 
-// Config contains cri-containerd toml config
+// PluginConfig contains toml config related to CRI plugin,
-type Config struct {
+// it is a subset of CRIConfig.
+type PluginConfig struct {
 	// ContainerdConfig contains config related to containerd
 	ContainerdConfig `toml:"containerd" json:"containerd,omitempty"`
 	// CniConfig contains config related to cni
 	CniConfig `toml:"cni" json:"cni,omitempty"`
-	// SocketPath is the path to the socket which cri-containerd serves on.
-	SocketPath string `toml:"socket_path" json:"socketPath,omitempty"`
-	// RootDir is the root directory path for managing cri-containerd files
-	// (metadata checkpoint etc.)
-	RootDir string `toml:"root_dir" json:"rootDir,omitempty"`
 	// StreamServerAddress is the ip address streaming server is listening on.
 	StreamServerAddress string `toml:"stream_server_address" json:"streamServerAddress,omitempty"`
 	// StreamServerPort is the port streaming server is listening on.
 	StreamServerPort string `toml:"stream_server_port" json:"streamServerPort,omitempty"`
-	// CgroupPath is the path for the cgroup that cri-containerd is placed in.
-	CgroupPath string `toml:"cgroup_path" json:"cgroupPath,omitempty"`
 	// EnableSelinux indicates to enable the selinux support.
 	EnableSelinux bool `toml:"enable_selinux" json:"enableSelinux,omitempty"`
 	// SandboxImage is the image used by sandbox container.
@@ -88,19 +84,45 @@ type Config struct {
 	StatsCollectPeriod int `toml:"stats_collect_period" json:"statsCollectPeriod,omitempty"`
 	// SystemdCgroup enables systemd cgroup support.
 	SystemdCgroup bool `toml:"systemd_cgroup" json:"systemdCgroup,omitempty"`
-	// OOMScore adjust the cri-containerd's oom score
-	OOMScore int `toml:"oom_score" json:"oomScore,omitempty"`
-	// EnableProfiling is used for enable profiling via host:port/debug/pprof/
-	EnableProfiling bool `toml:"profiling" json:"enableProfiling,omitempty"`
-	// ProfilingPort is the port for profiling via host:port/debug/pprof/
-	ProfilingPort string `toml:"profiling_port" json:"profilingPort,omitempty"`
-	// ProfilingAddress is address for profiling via host:port/debug/pprof/
-	ProfilingAddress string `toml:"profiling_addr" json:"profilingAddress,omitempty"`
 	// SkipImageFSUUID skips retrieving imagefs uuid.
 	// TODO(random-liu): Remove this after we find a generic way to get imagefs uuid.
 	SkipImageFSUUID bool `toml:"skip_imagefs_uuid" json:"skipImageFSUUID,omitempty"`
+}
+
+// CRIConfig contains toml config related to CRI service.
+// TODO(random-liu): Make this an internal config object when we no longer support cri-containerd
+// standalone mode. At that time, we can clean this up.
+type CRIConfig struct {
+	// PluginConfig is the config for CRI plugin.
+	PluginConfig
+	// ContainerdRootDir is the root directory path for containerd.
+	ContainerdRootDir string `toml:"-" json:"containerdRootDir,omitempty"`
+	// ContainerdEndpoint is the containerd endpoint path.
+	ContainerdEndpoint string `toml:"-" json:"containerdEndpoint,omitempty"`
+	// SocketPath is the path to the socket which cri-containerd serves on.
+	// TODO(random-liu): Remove SocketPath when no longer support cri-containerd
+	// standalone mode.
+	SocketPath string `toml:"socket_path" json:"socketPath,omitempty"`
+	// RootDir is the root directory path for managing cri-containerd files
+	// (metadata checkpoint etc.)
+	RootDir string `toml:"root_dir" json:"rootDir,omitempty"`
+}
+
+// Config contains toml config related cri-containerd daemon.
+type Config struct {
+	CRIConfig `toml:"-"`
+	// CgroupPath is the path for the cgroup that cri-containerd is placed in.
+	CgroupPath string `toml:"cgroup_path"`
+	// OOMScore adjust the cri-containerd's oom score
+	OOMScore int `toml:"oom_score"`
+	// EnableProfiling is used for enable profiling via host:port/debug/pprof/
+	EnableProfiling bool `toml:"profiling"`
+	// ProfilingPort is the port for profiling via host:port/debug/pprof/
+	ProfilingPort string `toml:"profiling_port"`
+	// ProfilingAddress is address for profiling via host:port/debug/pprof/
+	ProfilingAddress string `toml:"profiling_addr"`
 	// LogLevel is the logrus log level.
-	LogLevel string `toml:"log_level" json:"logLevel,omitempty"`
+	LogLevel string `toml:"log_level"`
 }
 
 // CRIContainerdOptions contains cri-containerd command line and toml options.
@@ -119,18 +141,18 @@ func NewCRIContainerdOptions() *CRIContainerdOptions {
 // AddFlags adds cri-containerd command line options to pflag.
 func (c *CRIContainerdOptions) AddFlags(fs *pflag.FlagSet) {
 	defaults := DefaultConfig()
-	fs.StringVar(&c.LogLevel, "log-level",
-		"info", "Set the logging level [trace, debug, info, warn, error, fatal, panic].")
 	fs.StringVar(&c.ConfigFilePath, configFilePathArgName,
 		defaultConfigFilePath, "Path to the config file.")
+	fs.StringVar(&c.LogLevel, "log-level",
+		defaults.LogLevel, "Set the logging level [trace, debug, info, warn, error, fatal, panic].")
 	fs.StringVar(&c.SocketPath, "socket-path",
 		defaults.SocketPath, "Path to the socket which cri-containerd serves on.")
 	fs.StringVar(&c.RootDir, "root-dir",
 		defaults.RootDir, "Root directory path for cri-containerd managed files (metadata checkpoint etc).")
-	fs.StringVar(&c.ContainerdConfig.RootDir, "containerd-root-dir",
+	fs.StringVar(&c.ContainerdRootDir, "containerd-root-dir",
-		defaults.ContainerdConfig.RootDir, "Root directory path where containerd stores persistent data.")
+		defaults.ContainerdRootDir, "Root directory path where containerd stores persistent data.")
-	fs.StringVar(&c.ContainerdConfig.Endpoint, "containerd-endpoint",
+	fs.StringVar(&c.ContainerdEndpoint, "containerd-endpoint",
-		defaults.ContainerdConfig.Endpoint, "Path to the containerd endpoint.")
+		defaults.ContainerdEndpoint, "Path to the containerd endpoint.")
 	fs.StringVar(&c.ContainerdConfig.Snapshotter, "containerd-snapshotter",
 		defaults.ContainerdConfig.Snapshotter, "The snapshotter used by containerd.")
 	fs.StringVar(&c.ContainerdConfig.Runtime, "containerd-runtime",
@@ -182,6 +204,10 @@ func (c *CRIContainerdOptions) InitFlags(fs *pflag.FlagSet) error {
 		}
 		return err
 	}
+	// Add this for backward compatibility.
+	// TODO(random-liu): Remove this when we no longer support cri-containerd standalone mode.
+	c.ContainerdRootDir = c.ContainerdConfig.RootDir
+	c.ContainerdEndpoint = c.ContainerdConfig.Endpoint
 
 	// What is the reason for applying the command line twice?
 	// Because the values from command line have the highest priority.
@@ -212,31 +238,36 @@ func AddGRPCFlags(fs *pflag.FlagSet) (*string, *time.Duration) {
 // DefaultConfig returns default configurations of cri-containerd.
 func DefaultConfig() Config {
 	return Config{
-		ContainerdConfig: ContainerdConfig{
+		CRIConfig: CRIConfig{
-			RootDir:       "/var/lib/containerd",
+			PluginConfig: PluginConfig{
-			Snapshotter:   containerd.DefaultSnapshotter,
+				CniConfig: CniConfig{
-			Endpoint:      "/run/containerd/containerd.sock",
+					NetworkPluginBinDir:  "/opt/cni/bin",
-			Runtime:       "io.containerd.runtime.v1.linux",
+					NetworkPluginConfDir: "/etc/cni/net.d",
-			RuntimeEngine: "",
+				},
-			RuntimeRoot:   "",
+				ContainerdConfig: ContainerdConfig{
+					Snapshotter:   containerd.DefaultSnapshotter,
+					Runtime:       "io.containerd.runtime.v1.linux",
+					RuntimeEngine: "",
+					RuntimeRoot:   "",
+				},
+				StreamServerAddress: "",
+				StreamServerPort:    "10010",
+				EnableSelinux:       false,
+				SandboxImage:        "gcr.io/google_containers/pause:3.0",
+				StatsCollectPeriod:  10,
+				SystemdCgroup:       false,
+				SkipImageFSUUID:     false,
+			},
+			ContainerdRootDir:  "/var/lib/containerd",
+			ContainerdEndpoint: "/run/containerd/containerd.sock",
+			SocketPath:         "/var/run/cri-containerd.sock",
+			RootDir:            "/var/lib/cri-containerd",
 		},
-		CniConfig: CniConfig{
+		CgroupPath:       "",
-			NetworkPluginBinDir:  "/opt/cni/bin",
+		OOMScore:         -999,
-			NetworkPluginConfDir: "/etc/cni/net.d",
+		EnableProfiling:  true,
-		},
+		ProfilingPort:    "10011",
-		SocketPath:          "/var/run/cri-containerd.sock",
+		ProfilingAddress: "127.0.0.1",
-		RootDir:             "/var/lib/cri-containerd",
+		LogLevel:         "info",
-		StreamServerAddress: "",
-		StreamServerPort:    "10010",
-		CgroupPath:          "",
-		EnableSelinux:       false,
-		SandboxImage:        "gcr.io/google_containers/pause:3.0",
-		StatsCollectPeriod:  10,
-		SystemdCgroup:       false,
-		OOMScore:            -999,
-		EnableProfiling:     true,
-		ProfilingPort:       "10011",
-		ProfilingAddress:    "127.0.0.1",
-		SkipImageFSUUID:     false,
 	}
 }

cri.go

@@ -17,6 +17,8 @@ limitations under the License.
 package cri
 
 import (
+	"path/filepath"
+
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/plugin"
 	"github.com/pkg/errors"
@@ -28,10 +30,13 @@ import (
 // TODO(random-liu): Use github.com/pkg/errors for our errors.
 // Register CRI service plugin
 func init() {
+	// TODO(random-liu): Make `containerd config default` print plugin default config.
+	config := options.DefaultConfig().PluginConfig
 	plugin.Register(&plugin.Registration{
 		// In fact, cri is not strictly a GRPC plugin now.
-		Type: plugin.GRPCPlugin,
+		Type:   plugin.GRPCPlugin,
-		ID:   "cri",
+		ID:     "cri",
+		Config: &config,
 		Requires: []plugin.Type{
 			plugin.RuntimePlugin,
 			plugin.SnapshotPlugin,
@@ -47,13 +52,17 @@ func init() {
 
 func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 	ctx := ic.Context
-	// TODO(random-liu): Support Config through Registration.Config.
-	// TODO(random-liu): Validate the configuration.
-	// TODO(random-liu): Leverage other fields in InitContext, such as Root.
-	// TODO(random-liu): Separate cri plugin config from cri-containerd server config,
-	// because many options only make sense to cri-containerd server.
 	// TODO(random-liu): Handle graceful stop.
-	c := options.DefaultConfig()
+	pluginConfig := ic.Config.(*options.PluginConfig)
+	c := options.CRIConfig{
+		PluginConfig: *pluginConfig,
+		// This is a hack. We assume that containerd root directory
+		// is one level above plugin directory.
+		// TODO(random-liu): Expose containerd config to plugin.
+		ContainerdRootDir:  filepath.Dir(ic.Root),
+		ContainerdEndpoint: ic.Address,
+		RootDir:            ic.Root,
+	}
 	log.G(ctx).Infof("Start cri plugin with config %+v", c)
 
 	s, err := server.NewCRIContainerdService(c)
@@ -61,8 +70,8 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 		return nil, errors.Wrap(err, "failed to create CRI service")
 	}
 
-	// Use a goroutine to start cri service. The reason is that currently
+	// Use a goroutine to initialize cri service. The reason is that currently
-	// cri service requires containerd to be running.
+	// cri service requires containerd to be initialize.
 	go func() {
 		if err := s.Run(false); err != nil {
 			log.G(ctx).WithError(err).Fatal("Failed to run CRI service")

hack/test-integration.sh

@@ -25,6 +25,11 @@ FOCUS=${FOCUS:-""}
 # REPORT_DIR is the the directory to store test logs.
 REPORT_DIR=${REPORT_DIR:-"/tmp/test-integration"}
 
+CRICONTAINERD_ROOT="/var/lib/cri-containerd"
+if ! ${STANDALONE_CRI_CONTAINERD}; then
+  CRICONTAINERD_ROOT="/var/lib/containerd/io.containerd.grpc.v1.cri"
+fi
+
 mkdir -p ${REPORT_DIR}
 test_setup ${REPORT_DIR}
 
@@ -33,7 +38,8 @@ test_setup ${REPORT_DIR}
 # Some integration test needs the env to skip itself.
 sudo ${ROOT}/_output/integration.test --test.run="${FOCUS}" --test.v \
   --standalone-cri-containerd=${STANDALONE_CRI_CONTAINERD} \
-  --cri-containerd-endpoint=${CRICONTAINERD_SOCK}
+  --cri-containerd-endpoint=${CRICONTAINERD_SOCK} \
+  --cri-containerd-root=${CRICONTAINERD_ROOT}
 
 test_exit_code=$?
 

integration/restart_test.go

@@ -201,7 +201,7 @@ func TestSandboxDeletionAcrossCRIContainerdRestart(t *testing.T) {
 	assert.Empty(t, loadedSandboxes)
 
 	t.Logf("Make sure sandbox root is removed")
-	sandboxRoot := filepath.Join(criContainerdRoot, "sandboxes", sb)
+	sandboxRoot := filepath.Join(*criContainerdRoot, "sandboxes", sb)
 	_, err = os.Stat(sandboxRoot)
 	assert.True(t, os.IsNotExist(err))
 }

integration/test_utils.go

@@ -39,7 +39,6 @@ const (
 	pauseImage         = "gcr.io/google_containers/pause:3.0" // This is the same with default sandbox image.
 	k8sNamespace       = "k8s.io"                             // This is the same with server.k8sContainerdNamespace.
 	containerdEndpoint = "/run/containerd/containerd.sock"
-	criContainerdRoot  = "/var/lib/cri-containerd"
 )
 
 var (
@@ -51,6 +50,7 @@ var (
 
 var standaloneCRIContainerd = flag.Bool("standalone-cri-containerd", true, "Whether cri-containerd is running in standalone mode.")
 var criContainerdEndpoint = flag.String("cri-containerd-endpoint", "/var/run/cri-containerd.sock", "The endpoint of cri-containerd.")
+var criContainerdRoot = flag.String("cri-containerd-root", "/var/lib/cri-containerd", "The root directory of cri-containerd.")
 
 func init() {
 	flag.Parse()

integration/volume_copy_up_test.go

@@ -70,7 +70,7 @@ func TestVolumeCopyUp(t *testing.T) {
 	assert.Equal(t, "test_content\n", string(stdout))
 
 	t.Logf("Check host path of the volume")
-	hostCmd := fmt.Sprintf("ls %s/containers/%s/volumes/*/test_file | xargs cat", criContainerdRoot, cn)
+	hostCmd := fmt.Sprintf("ls %s/containers/%s/volumes/*/test_file | xargs cat", *criContainerdRoot, cn)
 	output, err := exec.Command("sh", "-c", hostCmd).CombinedOutput()
 	require.NoError(t, err)
 	assert.Equal(t, "test_content\n", string(output))

pkg/server/service.go

@@ -29,6 +29,7 @@ import (
 	"github.com/cri-o/ocicni/pkg/ocicni"
 	runcapparmor "github.com/opencontainers/runc/libcontainer/apparmor"
 	runcseccomp "github.com/opencontainers/runc/libcontainer/seccomp"
+	"github.com/opencontainers/selinux/go-selinux"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/net/context"
 	"golang.org/x/sys/unix"
@@ -72,7 +73,7 @@ type CRIContainerdService interface {
 // criContainerdService implements CRIContainerdService.
 type criContainerdService struct {
 	// config contains all configurations.
-	config options.Config
+	config options.CRIConfig
 	// imageFSUUID is the device uuid of image filesystem.
 	imageFSUUID string
 	// apparmorEnabled indicates whether apparmor is enabled.
@@ -111,7 +112,7 @@ type criContainerdService struct {
 }
 
 // NewCRIContainerdService returns a new instance of CRIContainerdService
-func NewCRIContainerdService(config options.Config) (CRIContainerdService, error) {
+func NewCRIContainerdService(config options.CRIConfig) (CRIContainerdService, error) {
 	var err error
 	c := &criContainerdService{
 		config:             config,
@@ -127,8 +128,16 @@ func NewCRIContainerdService(config options.Config) (CRIContainerdService, error
 		initialized:        atomic.NewBool(false),
 	}
 
+	if c.config.EnableSelinux {
+		if !selinux.GetEnabled() {
+			logrus.Warn("Selinux is not supported")
+		}
+	} else {
+		selinux.SetDisabled()
+	}
+
 	if !c.config.SkipImageFSUUID {
-		imageFSPath := imageFSPath(config.ContainerdConfig.RootDir, config.ContainerdConfig.Snapshotter)
+		imageFSPath := imageFSPath(config.ContainerdRootDir, config.ContainerdConfig.Snapshotter)
 		c.imageFSUUID, err = c.getDeviceUUID(imageFSPath)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get imagefs uuid of %q: %v", imageFSPath, err)
@@ -180,10 +189,10 @@ func (c *criContainerdService) Run(startGRPC bool) error {
 	// Connect containerd service here, to get rid of the containerd dependency
 	// in `NewCRIContainerdService`. This is required for plugin mode bootstrapping.
 	logrus.Info("Connect containerd service")
-	client, err := containerd.New(c.config.ContainerdConfig.Endpoint, containerd.WithDefaultNamespace(k8sContainerdNamespace))
+	client, err := containerd.New(c.config.ContainerdEndpoint, containerd.WithDefaultNamespace(k8sContainerdNamespace))
 	if err != nil {
 		return fmt.Errorf("failed to initialize containerd client with endpoint %q: %v",
-			c.config.ContainerdConfig.Endpoint, err)
+			c.config.ContainerdEndpoint, err)
 	}
 	c.client = client
 

pkg/server/service_test.go

@@ -39,9 +39,11 @@ const (
 // newTestCRIContainerdService creates a fake criContainerdService for test.
 func newTestCRIContainerdService() *criContainerdService {
 	return &criContainerdService{
-		config: options.Config{
+		config: options.CRIConfig{
-			RootDir:      testRootDir,
+			RootDir: testRootDir,
-			SandboxImage: testSandboxImage,
+			PluginConfig: options.PluginConfig{
+				SandboxImage: testSandboxImage,
+			},
 		},
 		imageFSUUID:        testImageFSUUID,
 		os:                 ostesting.NewFakeOS(),