Split image config from CRI plugin

Signed-off-by: Derek McGowan <[email protected]>

Author: dmcgowan

contrib/fuzz/cri_server_fuzzer.go

@@ -44,7 +44,7 @@ func FuzzCRIServer(data []byte) int {
 
 	config := criconfig.Config{}
 
-	imageService, err := images.NewService(config, map[string]string{}, client)
+	imageService, err := images.NewService(config.ImageConfig, map[string]string{}, map[string]images.RuntimePlatform{}, client)
 	if err != nil {
 		panic(err)
 	}

integration/image_pull_timeout_test.go

@@ -478,17 +478,17 @@ func initLocalCRIPlugin(client *containerd.Client, tmpDir string, registryCfg cr
 
 	cfg := criconfig.Config{
 		PluginConfig: criconfig.PluginConfig{
-			ContainerdConfig: criconfig.ContainerdConfig{
-				Snapshotter: containerd.DefaultSnapshotter,
+			ImageConfig: criconfig.ImageConfig{
+				Snapshotter:              containerd.DefaultSnapshotter,
+				Registry:                 registryCfg,
+				ImagePullProgressTimeout: defaultImagePullProgressTimeout.String(),
+				StatsCollectPeriod:       10,
 			},
-			Registry:                 registryCfg,
-			ImagePullProgressTimeout: defaultImagePullProgressTimeout.String(),
-			StatsCollectPeriod:       10,
 		},
 		ContainerdRootDir: containerdRootDir,
 		RootDir:           filepath.Join(criWorkDir, "root"),
 		StateDir:          filepath.Join(criWorkDir, "state"),
 	}
 
-	return images.NewService(cfg, map[string]string{}, client)
+	return images.NewService(cfg.ImageConfig, map[string]string{}, map[string]images.RuntimePlatform{}, client)
 }

pkg/cri/config/config.go

@@ -116,25 +116,13 @@ type Runtime struct {
 
 // ContainerdConfig contains toml config related to containerd
 type ContainerdConfig struct {
-	// Snapshotter is the snapshotter used by containerd.
-	Snapshotter string `toml:"snapshotter" json:"snapshotter"`
 	// DefaultRuntimeName is the default runtime name to use from the runtimes table.
 	DefaultRuntimeName string `toml:"default_runtime_name" json:"defaultRuntimeName"`
 
 	// Runtimes is a map from CRI RuntimeHandler strings, which specify types of runtime
 	// configurations, to the matching configurations.
 	Runtimes map[string]Runtime `toml:"runtimes" json:"runtimes"`
 
-	// DisableSnapshotAnnotations disables to pass additional annotations (image
-	// related information) to snapshotters. These annotations are required by
-	// stargz snapshotter (https://github.com/containerd/stargz-snapshotter).
-	DisableSnapshotAnnotations bool `toml:"disable_snapshot_annotations" json:"disableSnapshotAnnotations"`
-
-	// DiscardUnpackedLayers is a boolean flag to specify whether to allow GC to
-	// remove layers from the content store after successfully unpacking these
-	// layers to the snapshotter.
-	DiscardUnpackedLayers bool `toml:"discard_unpacked_layers" json:"discardUnpackedLayers"`
-
 	// IgnoreBlockIONotEnabledErrors is a boolean flag to ignore
 	// blockio related errors when blockio support has not been
 	// enabled.
@@ -249,17 +237,57 @@ type ImageDecryption struct {
 	KeyModel string `toml:"key_model" json:"keyModel"`
 }
 
+type ImageConfig struct {
+	// Snapshotter is the snapshotter used by containerd.
+	Snapshotter string `toml:"snapshotter" json:"snapshotter"`
+
+	// DisableSnapshotAnnotations disables to pass additional annotations (image
+	// related information) to snapshotters. These annotations are required by
+	// stargz snapshotter (https://github.com/containerd/stargz-snapshotter).
+	DisableSnapshotAnnotations bool `toml:"disable_snapshot_annotations" json:"disableSnapshotAnnotations"`
+
+	// DiscardUnpackedLayers is a boolean flag to specify whether to allow GC to
+	// remove layers from the content store after successfully unpacking these
+	// layers to the snapshotter.
+	DiscardUnpackedLayers bool `toml:"discard_unpacked_layers" json:"discardUnpackedLayers"`
+
+	// Registry contains config related to the registry
+	Registry Registry `toml:"registry" json:"registry"`
+
+	// ImageDecryption contains config related to handling decryption of encrypted container images
+	ImageDecryption `toml:"image_decryption" json:"imageDecryption"`
+
+	// MaxConcurrentDownloads restricts the number of concurrent downloads for each image.
+	// TODO: Migrate to transfer service
+	MaxConcurrentDownloads int `toml:"max_concurrent_downloads" json:"maxConcurrentDownloads"`
+
+	// ImagePullProgressTimeout is the maximum duration that there is no
+	// image data read from image registry in the open connection. It will
+	// be reset whatever a new byte has been read. If timeout, the image
+	// pulling will be cancelled. A zero value means there is no timeout.
+	//
+	// The string is in the golang duration format, see:
+	//   https://golang.org/pkg/time/#ParseDuration
+	ImagePullProgressTimeout string `toml:"image_pull_progress_timeout" json:"imagePullProgressTimeout"`
+
+	// ImagePullWithSyncFs is an experimental setting. It's to force sync
+	// filesystem during unpacking to ensure that data integrity.
+	// TODO: Migrate to transfer service
+	ImagePullWithSyncFs bool `toml:"image_pull_with_sync_fs" json:"imagePullWithSyncFs"`
+
+	// StatsCollectPeriod is the period (in seconds) of snapshots stats collection.
+	StatsCollectPeriod int `toml:"stats_collect_period" json:"statsCollectPeriod"`
+}
+
 // PluginConfig contains toml config related to CRI plugin,
 // it is a subset of Config.
 type PluginConfig struct {
+	// ImageConfig is the image service configuration
+	ImageConfig
 	// ContainerdConfig contains config related to containerd
 	ContainerdConfig `toml:"containerd" json:"containerd"`
 	// CniConfig contains config related to cni
 	CniConfig `toml:"cni" json:"cni"`
-	// Registry contains config related to the registry
-	Registry Registry `toml:"registry" json:"registry"`
-	// ImageDecryption contains config related to handling decryption of encrypted container images
-	ImageDecryption `toml:"image_decryption" json:"imageDecryption"`
 	// DisableTCPService disables serving CRI on the TCP server.
 	DisableTCPService bool `toml:"disable_tcp_service" json:"disableTCPService"`
 	// StreamServerAddress is the ip address streaming server is listening on.
@@ -278,8 +306,6 @@ type PluginConfig struct {
 	SelinuxCategoryRange int `toml:"selinux_category_range" json:"selinuxCategoryRange"`
 	// SandboxImage is the image used by sandbox container.
 	SandboxImage string `toml:"sandbox_image" json:"sandboxImage"`
-	// StatsCollectPeriod is the period (in seconds) of snapshots stats collection.
-	StatsCollectPeriod int `toml:"stats_collect_period" json:"statsCollectPeriod"`
 	// EnableTLSStreaming indicates to enable the TLS streaming support.
 	EnableTLSStreaming bool `toml:"enable_tls_streaming" json:"enableTLSStreaming"`
 	// X509KeyPairStreaming is a x509 key pair used for TLS streaming
@@ -298,8 +324,6 @@ type PluginConfig struct {
 	// current OOMScoreADj.
 	// This is useful when the containerd does not have permission to decrease OOMScoreAdj.
 	RestrictOOMScoreAdj bool `toml:"restrict_oom_score_adj" json:"restrictOOMScoreAdj"`
-	// MaxConcurrentDownloads restricts the number of concurrent downloads for each image.
-	MaxConcurrentDownloads int `toml:"max_concurrent_downloads" json:"maxConcurrentDownloads"`
 	// DisableProcMount disables Kubernetes ProcMount support. This MUST be set to `true`
 	// when using containerd with Kubernetes <=1.11.
 	DisableProcMount bool `toml:"disable_proc_mount" json:"disableProcMount"`
@@ -345,14 +369,7 @@ type PluginConfig struct {
 	// For more details about CDI configuration please refer to
 	// https://github.com/container-orchestrated-devices/container-device-interface#containerd-configuration
 	CDISpecDirs []string `toml:"cdi_spec_dirs" json:"cdiSpecDirs"`
-	// ImagePullProgressTimeout is the maximum duration that there is no
-	// image data read from image registry in the open connection. It will
-	// be reset whatever a new byte has been read. If timeout, the image
-	// pulling will be cancelled. A zero value means there is no timeout.
-	//
-	// The string is in the golang duration format, see:
-	//   https://golang.org/pkg/time/#ParseDuration
-	ImagePullProgressTimeout string `toml:"image_pull_progress_timeout" json:"imagePullProgressTimeout"`
+
 	// DrainExecSyncIOTimeout is the maximum duration to wait for ExecSync
 	// API' IO EOF event after exec init process exits. A zero value means
 	// there is no timeout.
@@ -362,9 +379,6 @@ type PluginConfig struct {
 	//
 	// For example, the value can be '5h', '2h30m', '10s'.
 	DrainExecSyncIOTimeout string `toml:"drain_exec_sync_io_timeout" json:"drainExecSyncIOTimeout"`
-	// ImagePullWithSyncFs is an experimental setting. It's to force sync
-	// filesystem during unpacking to ensure that data integrity.
-	ImagePullWithSyncFs bool `toml:"image_pull_with_sync_fs" json:"imagePullWithSyncFs"`
 }
 
 // X509KeyPairStreaming contains the x509 configuration for streaming

pkg/cri/config/config_test.go

@@ -54,9 +54,11 @@ func TestValidateConfig(t *testing.T) {
 						RuntimeDefault: {},
 					},
 				},
-				Registry: Registry{
-					Auths: map[string]AuthConfig{
-						"https://gcr.io": {Username: "test"},
+				ImageConfig: ImageConfig{
+					Registry: Registry{
+						Auths: map[string]AuthConfig{
+							"https://gcr.io": {Username: "test"},
+						},
 					},
 				},
 			},
@@ -69,16 +71,18 @@ func TestValidateConfig(t *testing.T) {
 						},
 					},
 				},
-				Registry: Registry{
-					Configs: map[string]RegistryConfig{
-						"gcr.io": {
-							Auth: &AuthConfig{
-								Username: "test",
+				ImageConfig: ImageConfig{
+					Registry: Registry{
+						Configs: map[string]RegistryConfig{
+							"gcr.io": {
+								Auth: &AuthConfig{
+									Username: "test",
+								},
 							},
 						},
-					},
-					Auths: map[string]AuthConfig{
-						"https://gcr.io": {Username: "test"},
+						Auths: map[string]AuthConfig{
+							"https://gcr.io": {Username: "test"},
+						},
 					},
 				},
 			},
@@ -108,10 +112,12 @@ func TestValidateConfig(t *testing.T) {
 						},
 					},
 				},
-				Registry: Registry{
-					ConfigPath: "/etc/containerd/conf.d",
-					Mirrors: map[string]Mirror{
-						"something.io": {},
+				ImageConfig: ImageConfig{
+					Registry: Registry{
+						ConfigPath: "/etc/containerd/conf.d",
+						Mirrors: map[string]Mirror{
+							"something.io": {},
+						},
 					},
 				},
 			},
@@ -125,9 +131,11 @@ func TestValidateConfig(t *testing.T) {
 						RuntimeDefault: {},
 					},
 				},
-				Registry: Registry{
-					Mirrors: map[string]Mirror{
-						"example.com": {},
+				ImageConfig: ImageConfig{
+					Registry: Registry{
+						Mirrors: map[string]Mirror{
+							"example.com": {},
+						},
 					},
 				},
 			},
@@ -140,9 +148,11 @@ func TestValidateConfig(t *testing.T) {
 						},
 					},
 				},
-				Registry: Registry{
-					Mirrors: map[string]Mirror{
-						"example.com": {},
+				ImageConfig: ImageConfig{
+					Registry: Registry{
+						Mirrors: map[string]Mirror{
+							"example.com": {},
+						},
 					},
 				},
 			},
@@ -156,11 +166,13 @@ func TestValidateConfig(t *testing.T) {
 						RuntimeDefault: {},
 					},
 				},
-				Registry: Registry{
-					Configs: map[string]RegistryConfig{
-						"gcr.io": {
-							Auth: &AuthConfig{
-								Username: "test",
+				ImageConfig: ImageConfig{
+					Registry: Registry{
+						Configs: map[string]RegistryConfig{
+							"gcr.io": {
+								Auth: &AuthConfig{
+									Username: "test",
+								},
 							},
 						},
 					},
@@ -175,11 +187,13 @@ func TestValidateConfig(t *testing.T) {
 						},
 					},
 				},
-				Registry: Registry{
-					Configs: map[string]RegistryConfig{
-						"gcr.io": {
-							Auth: &AuthConfig{
-								Username: "test",
+				ImageConfig: ImageConfig{
+					Registry: Registry{
+						Configs: map[string]RegistryConfig{
+							"gcr.io": {
+								Auth: &AuthConfig{
+									Username: "test",
+								},
 							},
 						},
 					},

pkg/cri/config/config_unix.go

@@ -62,8 +62,18 @@ func DefaultConfig() PluginConfig {
 			NetworkPluginSetupSerially: false,
 			NetworkPluginConfTemplate:  "",
 		},
+		ImageConfig: ImageConfig{
+			Snapshotter:                containerd.DefaultSnapshotter,
+			DisableSnapshotAnnotations: true,
+			MaxConcurrentDownloads:     3,
+			ImageDecryption: ImageDecryption{
+				KeyModel: KeyModelNode,
+			},
+			ImagePullProgressTimeout: defaultImagePullProgressTimeoutDuration.String(),
+			ImagePullWithSyncFs:      false,
+			StatsCollectPeriod:       10,
+		},
 		ContainerdConfig: ContainerdConfig{
-			Snapshotter:        containerd.DefaultSnapshotter,
 			DefaultRuntimeName: "runc",
 			Runtimes: map[string]Runtime{
 				"runc": {
@@ -72,7 +82,6 @@ func DefaultConfig() PluginConfig {
 					Sandboxer: string(ModePodSandbox),
 				},
 			},
-			DisableSnapshotAnnotations: true,
 		},
 		DisableTCPService:    true,
 		StreamServerAddress:  "127.0.0.1",
@@ -86,22 +95,15 @@ func DefaultConfig() PluginConfig {
 			TLSCertFile: "",
 		},
 		SandboxImage:                     "registry.k8s.io/pause:3.9",
-		StatsCollectPeriod:               10,
 		MaxContainerLogLineSize:          16 * 1024,
-		MaxConcurrentDownloads:           3,
 		DisableProcMount:                 false,
 		TolerateMissingHugetlbController: true,
 		DisableHugetlbController:         true,
 		IgnoreImageDefinedVolumes:        false,
-		ImageDecryption: ImageDecryption{
-			KeyModel: KeyModelNode,
-		},
-		EnableCDI:                false,
-		CDISpecDirs:              []string{"/etc/cdi", "/var/run/cdi"},
-		ImagePullProgressTimeout: defaultImagePullProgressTimeoutDuration.String(),
-		DrainExecSyncIOTimeout:   "0s",
-		ImagePullWithSyncFs:      false,
-		EnableUnprivilegedPorts:  true,
-		EnableUnprivilegedICMP:   true,
+		EnableCDI:                        false,
+		CDISpecDirs:                      []string{"/etc/cdi", "/var/run/cdi"},
+		DrainExecSyncIOTimeout:           "0s",
+		EnableUnprivilegedPorts:          true,
+		EnableUnprivilegedICMP:           true,
 	}
 }

pkg/cri/config/config_windows.go

@@ -34,8 +34,16 @@ func DefaultConfig() PluginConfig {
 			NetworkPluginSetupSerially: false,
 			NetworkPluginConfTemplate:  "",
 		},
+		ImageConfig: ImageConfig{
+			Snapshotter:            containerd.DefaultSnapshotter,
+			StatsCollectPeriod:     10,
+			MaxConcurrentDownloads: 3,
+			ImageDecryption: ImageDecryption{
+				KeyModel: KeyModelNode,
+			},
+			ImagePullProgressTimeout: defaultImagePullProgressTimeoutDuration.String(),
+		},
 		ContainerdConfig: ContainerdConfig{
-			Snapshotter:        containerd.DefaultSnapshotter,
 			DefaultRuntimeName: "runhcs-wcow-process",
 			Runtimes: map[string]Runtime{
 				"runhcs-wcow-process": {
@@ -74,16 +82,10 @@ func DefaultConfig() PluginConfig {
 			TLSCertFile: "",
 		},
 		SandboxImage:              "registry.k8s.io/pause:3.9",
-		StatsCollectPeriod:        10,
 		MaxContainerLogLineSize:   16 * 1024,
-		MaxConcurrentDownloads:    3,
 		IgnoreImageDefinedVolumes: false,
 		// TODO(windows): Add platform specific config, so that most common defaults can be shared.
 
-		ImageDecryption: ImageDecryption{
-			KeyModel: KeyModelNode,
-		},
-		ImagePullProgressTimeout: defaultImagePullProgressTimeoutDuration.String(),
-		DrainExecSyncIOTimeout:   "0s",
+		DrainExecSyncIOTimeout: "0s",
 	}
 }

pkg/cri/cri.go

@@ -94,8 +94,8 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 	}
 
 	// TODO: Update this logic to use runtime snapshotter
-	if client.SnapshotService(c.ContainerdConfig.Snapshotter) == nil {
-		return nil, fmt.Errorf("failed to find snapshotter %q", c.ContainerdConfig.Snapshotter)
+	if client.SnapshotService(c.ImageConfig.Snapshotter) == nil {
+		return nil, fmt.Errorf("failed to find snapshotter %q", c.ImageConfig.Snapshotter)
 	}
 
 	// TODO(dmcgowan): Get the full list directly from configured plugins