WIP: Add additional storage configuration support for CRI-O

examples/containerruntimeconfig.crd.yaml

@@ -9,3 +9,19 @@ spec:
   containerRuntimeConfig:
     pidsLimit: 2048
     logLevel: debug
+---
+apiVersion: machineconfiguration.openshift.io/v1
+kind: ContainerRuntimeConfig
+metadata:
+  name: set-additional-stores
+spec:
+  machineConfigPoolSelector:
+    matchLabels:
+      custom-crio: config-additional-stores
+  containerRuntimeConfig:
+    additionalLayerStores:
+      - path: /var/lib/stargz-store
+    additionalImageStores:
+      - path: /mnt/nfs-images
+    additionalArtifactStores:
+      - path: /mnt/ssd-artifacts

pkg/controller/container-runtime-config/container_runtime_config_bootstrap.go

@@ -39,7 +39,9 @@ func RunContainerRuntimeBootstrap(templateDir string, crconfigs []*mcfgv1.Contai
 
 			var configFileList []generatedConfigFile
 			ctrcfg := cfg.Spec.ContainerRuntimeConfig
-			if ctrcfg.OverlaySize != nil && !ctrcfg.OverlaySize.IsZero() {
+			needsStorageConfig := (ctrcfg.OverlaySize != nil && !ctrcfg.OverlaySize.IsZero()) ||
+				len(ctrcfg.AdditionalLayerStores) > 0 || len(ctrcfg.AdditionalImageStores) > 0
+			if needsStorageConfig {
 				storageTOML, err := mergeConfigChanges(originalStorageIgn, cfg, updateStorageConfig)
 				if err != nil {
 					klog.V(2).Infoln(cfg, err, "error merging user changes to storage.conf: %v", err)
@@ -48,7 +50,9 @@ func RunContainerRuntimeBootstrap(templateDir string, crconfigs []*mcfgv1.Contai
 				}
 			}
 			// Create the cri-o drop-in files
-			if ctrcfg.LogLevel != "" || ctrcfg.PidsLimit != nil || (ctrcfg.LogSizeMax != nil && !ctrcfg.LogSizeMax.IsZero()) || ctrcfg.DefaultRuntime != mcfgv1.ContainerRuntimeDefaultRuntimeEmpty {
+			needsCRIODropin := ctrcfg.LogLevel != "" || ctrcfg.PidsLimit != nil || (ctrcfg.LogSizeMax != nil && !ctrcfg.LogSizeMax.IsZero()) || ctrcfg.DefaultRuntime != mcfgv1.ContainerRuntimeDefaultRuntimeEmpty ||
+				len(ctrcfg.AdditionalArtifactStores) > 0
+			if needsCRIODropin {
 				crioFileConfigs := createCRIODropinFiles(cfg)
 				configFileList = append(configFileList, crioFileConfigs...)
 			}

pkg/controller/container-runtime-config/container_runtime_config_controller.go

@@ -363,6 +363,10 @@ func (ctrl *Controller) sigstoreAPIEnabled() bool {
 	return ctrl.fgHandler.Enabled(features.FeatureGateSigstoreImageVerification)
 }
 
+func (ctrl *Controller) additionalStorageConfigEnabled() bool {
+	return ctrl.fgHandler.Enabled(features.FeatureGateAdditionalStorageConfig)
+}
+
 func (ctrl *Controller) updateContainerRuntimeConfig(oldObj, newObj interface{}) {
 	oldCtrCfg := oldObj.(*mcfgv1.ContainerRuntimeConfig)
 	newCtrCfg := newObj.(*mcfgv1.ContainerRuntimeConfig)
@@ -753,7 +757,10 @@ func (ctrl *Controller) syncContainerRuntimeConfig(key string) error {
 
 		var configFileList []generatedConfigFile
 		ctrcfg := cfg.Spec.ContainerRuntimeConfig
-		if ctrcfg.OverlaySize != nil && !ctrcfg.OverlaySize.IsZero() {
+		additionalStorageEnabled := ctrl.additionalStorageConfigEnabled()
+		needsStorageConfig := (ctrcfg.OverlaySize != nil && !ctrcfg.OverlaySize.IsZero()) ||
+			(additionalStorageEnabled && (len(ctrcfg.AdditionalLayerStores) > 0 || len(ctrcfg.AdditionalImageStores) > 0))
+		if needsStorageConfig {
 			storageTOML, err := mergeConfigChanges(originalStorageIgn, cfg, updateStorageConfig)
 			if err != nil {
 				klog.V(2).Infoln(cfg, err, "error merging user changes to storage.conf: %v", err)
@@ -765,7 +772,9 @@ func (ctrl *Controller) syncContainerRuntimeConfig(key string) error {
 		}
 
 		// Create the cri-o drop-in files
-		if ctrcfg.LogLevel != "" || ctrcfg.PidsLimit != nil || (ctrcfg.LogSizeMax != nil && !ctrcfg.LogSizeMax.IsZero()) || ctrcfg.DefaultRuntime != mcfgv1.ContainerRuntimeDefaultRuntimeEmpty {
+		needsCRIODropin := ctrcfg.LogLevel != "" || ctrcfg.PidsLimit != nil || (ctrcfg.LogSizeMax != nil && !ctrcfg.LogSizeMax.IsZero()) || ctrcfg.DefaultRuntime != mcfgv1.ContainerRuntimeDefaultRuntimeEmpty ||
+			(additionalStorageEnabled && len(ctrcfg.AdditionalArtifactStores) > 0)
+		if needsCRIODropin {
 			crioFileConfigs := createCRIODropinFiles(cfg)
 			configFileList = append(configFileList, crioFileConfigs...)
 		}

pkg/controller/container-runtime-config/container_runtime_config_controller_test.go

@@ -2026,3 +2026,112 @@ func TestImagePolicyCreate(t *testing.T) {
 		})
 	}
 }
+
+func TestContainerRuntimeConfigAdditionalStorageConfig(t *testing.T) {
+	for _, platform := range []apicfgv1.PlatformType{apicfgv1.AWSPlatformType, apicfgv1.NonePlatformType, "unrecognized"} {
+		t.Run(string(platform), func(t *testing.T) {
+			f := newFixture(t)
+			// Enable the AdditionalStorageConfig feature gate
+			f.fgHandler = ctrlcommon.NewFeatureGatesHardcodedHandler(
+				[]apicfgv1.FeatureGateName{
+					features.FeatureGateSigstoreImageVerification,
+					features.FeatureGateAdditionalStorageConfig,
+				},
+				[]apicfgv1.FeatureGateName{},
+			)
+			f.newController()
+
+			cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform)
+			mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0")
+			mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0")
+			ctrcfg := newContainerRuntimeConfig("set-additional-stores", &mcfgv1.ContainerRuntimeConfiguration{
+				AdditionalLayerStores: []mcfgv1.AdditionalLayerStore{
+					{Path: "/var/lib/stargz-store"},
+				},
+				AdditionalImageStores: []mcfgv1.AdditionalImageStore{
+					{Path: "/mnt/nfs-images"},
+				},
+				AdditionalArtifactStores: []mcfgv1.AdditionalArtifactStore{
+					{Path: "/mnt/ssd-artifacts"},
+				},
+			}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", ""))
+			ctrCfgKey, _ := getManagedKeyCtrCfg(mcp, f.client, ctrcfg)
+			mcs1 := helpers.NewMachineConfig(getManagedKeyCtrCfgDeprecated(mcp), map[string]string{"node-role": "master"}, "dummy://", []ign3types.File{{}})
+			mcs2 := mcs1.DeepCopy()
+			mcs2.Name = ctrCfgKey
+
+			f.ccLister = append(f.ccLister, cc)
+			f.mcpLister = append(f.mcpLister, mcp)
+			f.mcpLister = append(f.mcpLister, mcp2)
+			f.mccrLister = append(f.mccrLister, ctrcfg)
+			f.objects = append(f.objects, ctrcfg)
+
+			f.expectGetMachineConfigAction(mcs2)
+			f.expectGetMachineConfigAction(mcs1)
+			f.expectGetMachineConfigAction(mcs1)
+			f.expectUpdateContainerRuntimeConfig(ctrcfg)
+			f.expectUpdateContainerRuntimeConfigRoot(ctrcfg)
+			f.expectCreateMachineConfigAction(mcs1)
+			f.expectPatchContainerRuntimeConfig(ctrcfg, ctrcfgPatchBytes)
+			f.expectGetMachineConfigAction(mcs2)
+			f.expectUpdateContainerRuntimeConfig(ctrcfg)
+
+			f.run(getKey(ctrcfg, t))
+		})
+	}
+}
+
+func TestContainerRuntimeConfigAdditionalStorageConfigFeatureGateDisabled(t *testing.T) {
+	for _, platform := range []apicfgv1.PlatformType{apicfgv1.AWSPlatformType, apicfgv1.NonePlatformType, "unrecognized"} {
+		t.Run(string(platform), func(t *testing.T) {
+			f := newFixture(t)
+			// Disable the AdditionalStorageConfig feature gate
+			f.fgHandler = ctrlcommon.NewFeatureGatesHardcodedHandler(
+				[]apicfgv1.FeatureGateName{features.FeatureGateSigstoreImageVerification},
+				[]apicfgv1.FeatureGateName{features.FeatureGateAdditionalStorageConfig},
+			)
+			f.newController()
+
+			cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform)
+			mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0")
+			mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0")
+			// Even if these fields are set, the controller should not generate config
+			// because the feature gate is disabled
+			ctrcfg := newContainerRuntimeConfig("set-additional-stores-disabled", &mcfgv1.ContainerRuntimeConfiguration{
+				AdditionalLayerStores: []mcfgv1.AdditionalLayerStore{
+					{Path: "/var/lib/stargz-store"},
+				},
+				AdditionalImageStores: []mcfgv1.AdditionalImageStore{
+					{Path: "/mnt/nfs-images"},
+				},
+				AdditionalArtifactStores: []mcfgv1.AdditionalArtifactStore{
+					{Path: "/mnt/ssd-artifacts"},
+				},
+			}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", ""))
+			ctrCfgKey, _ := getManagedKeyCtrCfg(mcp, f.client, ctrcfg)
+			mcs1 := helpers.NewMachineConfig(getManagedKeyCtrCfgDeprecated(mcp), map[string]string{"node-role": "master"}, "dummy://", []ign3types.File{{}})
+			mcs2 := mcs1.DeepCopy()
+			mcs2.Name = ctrCfgKey
+
+			f.ccLister = append(f.ccLister, cc)
+			f.mcpLister = append(f.mcpLister, mcp)
+			f.mcpLister = append(f.mcpLister, mcp2)
+			f.mccrLister = append(f.mccrLister, ctrcfg)
+			f.objects = append(f.objects, ctrcfg)
+
+			// When only additional stores are set and the feature gate is disabled,
+			// the controller creates an MC with empty ignition config (no storage/crio files).
+			// No intermediate syncStatusOnly because needsStorageConfig is false.
+			f.expectGetMachineConfigAction(mcs2)
+			f.expectGetMachineConfigAction(mcs1)
+			f.expectGetMachineConfigAction(mcs1)
+			f.expectUpdateContainerRuntimeConfigRoot(ctrcfg)
+			f.expectCreateMachineConfigAction(mcs1)
+			f.expectPatchContainerRuntimeConfig(ctrcfg, ctrcfgPatchBytes)
+			f.expectGetMachineConfigAction(mcs2)
+			f.expectUpdateContainerRuntimeConfig(ctrcfg)
+
+			f.run(getKey(ctrcfg, t))
+		})
+	}
+}

pkg/controller/container-runtime-config/helpers.go

@@ -49,12 +49,15 @@ const (
 	policyConfigPath                       = "/etc/containers/policy.json"
 	// CRIODropInFilePathLogLevel is the path at which changes to the crio config for log-level
 	// will be dropped in this is exported so that we can use it in the e2e-tests
-	CRIODropInFilePathLogLevel       = "/etc/crio/crio.conf.d/01-ctrcfg-logLevel"
-	crioDropInFilePathPidsLimit      = "/etc/crio/crio.conf.d/01-ctrcfg-pidsLimit"
-	crioDropInFilePathLogSizeMax     = "/etc/crio/crio.conf.d/01-ctrcfg-logSizeMax"
-	CRIODropInFilePathDefaultRuntime = "/etc/crio/crio.conf.d/01-ctrcfg-defaultRuntime"
-	imagepolicyType                  = "sigstoreSigned"
-	sigstoreRegistriesConfigFilePath = "/etc/containers/registries.d/sigstore-registries.yaml"
+	CRIODropInFilePathLogLevel   = "/etc/crio/crio.conf.d/01-ctrcfg-logLevel"
+	crioDropInFilePathPidsLimit  = "/etc/crio/crio.conf.d/01-ctrcfg-pidsLimit"
+	crioDropInFilePathLogSizeMax = "/etc/crio/crio.conf.d/01-ctrcfg-logSizeMax"
+	// CRIODropInFilePathDefaultRuntime is the path at which changes to the crio config for default-runtime
+	// will be dropped in this is exported so that we can use it in the e2e-tests
+	CRIODropInFilePathDefaultRuntime           = "/etc/crio/crio.conf.d/01-ctrcfg-defaultRuntime"
+	crioDropInFilePathAdditionalArtifactStores = "/etc/crio/crio.conf.d/01-ctrcfg-additionalArtifactStores"
+	imagepolicyType                            = "sigstoreSigned"
+	sigstoreRegistriesConfigFilePath           = "/etc/containers/registries.d/sigstore-registries.yaml"
 )
 
 var (
@@ -125,6 +128,17 @@ type tomlConfigCRIODefaultRuntime struct {
 	} `toml:"crio"`
 }
 
+// tomlConfigCRIOAdditionalArtifactStores is used for conversions when additional-artifact-stores is changed
+// TOML-friendly (it has all of the explicit tables). It's just used for
+// conversions.
+type tomlConfigCRIOAdditionalArtifactStores struct {
+	Crio struct {
+		Runtime struct {
+			AdditionalArtifactStores []string `toml:"additional_artifact_stores,omitempty"`
+		} `toml:"runtime"`
+	} `toml:"crio"`
+}
+
 type dockerConfig struct {
 	UseSigstoreAttachments bool `json:"use-sigstore-attachments,omitempty"`
 }
@@ -387,6 +401,22 @@ func updateStorageConfig(data []byte, internal *mcfgv1.ContainerRuntimeConfigura
 		}
 	}
 
+	if len(internal.AdditionalLayerStores) > 0 {
+		paths := make([]string, 0, len(internal.AdditionalLayerStores))
+		for _, store := range internal.AdditionalLayerStores {
+			paths = append(paths, store.Path)
+		}
+		tomlConf.Storage.Options.AdditionalLayerStores = paths
+	}
+
+	if len(internal.AdditionalImageStores) > 0 {
+		paths := make([]string, 0, len(internal.AdditionalImageStores))
+		for _, store := range internal.AdditionalImageStores {
+			paths = append(paths, store.Path)
+		}
+		tomlConf.Storage.Options.AdditionalImageStores = paths
+	}
+
 	var newData bytes.Buffer
 	encoder := toml.NewEncoder(&newData)
 	if err := encoder.Encode(*tomlConf); err != nil {
@@ -448,6 +478,18 @@ func createCRIODropinFiles(cfg *mcfgv1.ContainerRuntimeConfig) []generatedConfig
 			klog.V(2).Infoln(cfg, err, "error updating user changes for default-runtime to crio.conf.d: %v", err)
 		}
 	}
+	if len(ctrcfg.AdditionalArtifactStores) > 0 {
+		tomlConf := tomlConfigCRIOAdditionalArtifactStores{}
+		paths := make([]string, 0, len(ctrcfg.AdditionalArtifactStores))
+		for _, store := range ctrcfg.AdditionalArtifactStores {
+			paths = append(paths, store.Path)
+		}
+		tomlConf.Crio.Runtime.AdditionalArtifactStores = paths
+		generatedConfigFileList, err = addTOMLgeneratedConfigFile(generatedConfigFileList, crioDropInFilePathAdditionalArtifactStores, tomlConf)
+		if err != nil {
+			klog.V(2).Infoln(cfg, err, "error updating user changes for additional-artifact-stores to crio.conf.d: %v", err)
+		}
+	}
 	return generatedConfigFileList
 }