diff --git a/cmd/cri-containerd/cri_containerd.go b/cmd/cri-containerd/cri_containerd.go index 80c47bb0c..cdfdd51a6 100644 --- a/cmd/cri-containerd/cri_containerd.go +++ b/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) diff --git a/cmd/cri-containerd/options/options.go b/cmd/cri-containerd/options/options.go index d290a07ad..945a1098e 100644 --- a/cmd/cri-containerd/options/options.go +++ b/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 -type Config struct { +// PluginConfig contains toml config related to CRI plugin, +// 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", - defaults.ContainerdConfig.RootDir, "Root directory path where containerd stores persistent data.") - fs.StringVar(&c.ContainerdConfig.Endpoint, "containerd-endpoint", - defaults.ContainerdConfig.Endpoint, "Path to the containerd endpoint.") + fs.StringVar(&c.ContainerdRootDir, "containerd-root-dir", + defaults.ContainerdRootDir, "Root directory path where containerd stores persistent data.") + fs.StringVar(&c.ContainerdEndpoint, "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{ - RootDir: "/var/lib/containerd", - Snapshotter: containerd.DefaultSnapshotter, - Endpoint: "/run/containerd/containerd.sock", - Runtime: "io.containerd.runtime.v1.linux", - RuntimeEngine: "", - RuntimeRoot: "", + CRIConfig: CRIConfig{ + PluginConfig: PluginConfig{ + CniConfig: CniConfig{ + NetworkPluginBinDir: "/opt/cni/bin", + NetworkPluginConfDir: "/etc/cni/net.d", + }, + 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{ - NetworkPluginBinDir: "/opt/cni/bin", - NetworkPluginConfDir: "/etc/cni/net.d", - }, - SocketPath: "/var/run/cri-containerd.sock", - RootDir: "/var/lib/cri-containerd", - 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, + CgroupPath: "", + OOMScore: -999, + EnableProfiling: true, + ProfilingPort: "10011", + ProfilingAddress: "127.0.0.1", + LogLevel: "info", } } diff --git a/cri.go b/cri.go index c894d8f7e..f4462b8ab 100644 --- a/cri.go +++ b/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, - ID: "cri", + Type: plugin.GRPCPlugin, + 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 - // cri service requires containerd to be running. + // Use a goroutine to initialize cri service. The reason is that currently + // 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") diff --git a/hack/test-integration.sh b/hack/test-integration.sh index 9d8738e1b..aef154e46 100755 --- a/hack/test-integration.sh +++ b/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=$? diff --git a/integration/restart_test.go b/integration/restart_test.go index af06b56b3..db4497635 100644 --- a/integration/restart_test.go +++ b/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)) } diff --git a/integration/test_utils.go b/integration/test_utils.go index 4f096fc18..1c975e4b6 100644 --- a/integration/test_utils.go +++ b/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() diff --git a/integration/volume_copy_up_test.go b/integration/volume_copy_up_test.go index 4665cf805..c26bd3eb1 100644 --- a/integration/volume_copy_up_test.go +++ b/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)) diff --git a/pkg/server/service.go b/pkg/server/service.go index 25ed52ff5..7d8edce9e 100644 --- a/pkg/server/service.go +++ b/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 diff --git a/pkg/server/service_test.go b/pkg/server/service_test.go index 34a234297..f1088f5f7 100644 --- a/pkg/server/service_test.go +++ b/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{ - RootDir: testRootDir, - SandboxImage: testSandboxImage, + config: options.CRIConfig{ + RootDir: testRootDir, + PluginConfig: options.PluginConfig{ + SandboxImage: testSandboxImage, + }, }, imageFSUUID: testImageFSUUID, os: ostesting.NewFakeOS(),