OCPNODE-4060: Add additional storage configuration fields to ContainerRuntimeConfig #2681
+3,209
−7
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,248 @@ | ||
| apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this | ||
| name: "ContainerRuntimeConfig" | ||
| crdName: containerruntimeconfigs.machineconfiguration.openshift.io | ||
| featureGates: | ||
| - AdditionalStorageConfig | ||
| tests: | ||
| onCreate: | ||
| # AdditionalLayerStores - comprehensive validation tests | ||
| - name: Should be able to create ContainerRuntimeConfig with multiple additionalLayerStores | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalLayerStores: | ||
| - path: /var/lib/stargz-store | ||
| - path: /mnt/nydus-store | ||
| - path: /opt/layer_store-v1.0 | ||
| expected: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalLayerStores: | ||
| - path: /var/lib/stargz-store | ||
| - path: /mnt/nydus-store | ||
| - path: /opt/layer_store-v1.0 | ||
|
|
||
| - name: Should fail if additionalLayerStores path is empty | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalLayerStores: | ||
| - path: "" | ||
| expectedError: "path in body should be at least 1 chars long" | ||
|
|
||
| - name: Should fail if additionalLayerStores path is not absolute | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalLayerStores: | ||
| - path: var/lib/stargz-store | ||
| expectedError: "path must be absolute and contain only alphanumeric characters, '/', '.', '_', and '-'" | ||
|
|
||
| - name: Should fail if additionalLayerStores path contains spaces | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalLayerStores: | ||
| - path: /var/lib/stargz store | ||
| expectedError: "path must be absolute and contain only alphanumeric characters, '/', '.', '_', and '-'" | ||
|
|
||
| - name: Should fail if additionalLayerStores path contains invalid characters | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalLayerStores: | ||
| - path: /var/lib/stargz@store | ||
| expectedError: "path must be absolute and contain only alphanumeric characters, '/', '.', '_', and '-'" | ||
|
|
||
| - name: Should fail if additionalLayerStores path is too long | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalLayerStores: | ||
| - path: /aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | ||
| expectedError: "Too long: may not be more than 256 bytes" | ||
|
|
||
| - name: Should fail if additionalLayerStores exceeds maximum of 5 items | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalLayerStores: | ||
| - path: /var/lib/store1 | ||
| - path: /var/lib/store2 | ||
| - path: /var/lib/store3 | ||
| - path: /var/lib/store4 | ||
| - path: /var/lib/store5 | ||
| - path: /var/lib/store6 | ||
| expectedError: "Too many: 6: must have at most 5 items" | ||
|
|
||
| - name: Should fail if additionalLayerStores item has no path field | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalLayerStores: | ||
| - {} | ||
| expectedError: "path: Required value" | ||
|
|
||
| - name: Should fail if additionalLayerStores contains duplicate paths | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalLayerStores: | ||
| - path: /var/lib/stargz-store | ||
| - path: /var/lib/stargz-store | ||
| expectedError: "additionalLayerStores must not contain duplicate paths" | ||
|
|
||
| # AdditionalImageStores - test max items validation (different from layer stores) | ||
| - name: Should fail if additionalImageStores exceeds maximum of 10 items | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalImageStores: | ||
| - path: /var/lib/store1 | ||
| - path: /var/lib/store2 | ||
| - path: /var/lib/store3 | ||
| - path: /var/lib/store4 | ||
| - path: /var/lib/store5 | ||
| - path: /var/lib/store6 | ||
| - path: /var/lib/store7 | ||
| - path: /var/lib/store8 | ||
| - path: /var/lib/store9 | ||
| - path: /var/lib/store10 | ||
| - path: /var/lib/store11 | ||
| expectedError: "Too many: 11: must have at most 10 items" | ||
|
|
||
| - name: Should fail if additionalImageStores path is not absolute | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalImageStores: | ||
| - path: var/lib/images | ||
| expectedError: "path must be absolute and contain only alphanumeric characters, '/', '.', '_', and '-'" | ||
|
|
||
| - name: Should fail if additionalImageStores item has no path field | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalImageStores: | ||
| - {} | ||
| expectedError: "path: Required value" | ||
|
|
||
| - name: Should fail if additionalImageStores contains duplicate paths | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalImageStores: | ||
| - path: /mnt/nfs-images | ||
| - path: /mnt/nfs-images | ||
| expectedError: "additionalImageStores must not contain duplicate paths" | ||
|
|
||
| # AdditionalArtifactStores - test max items validation (different from layer stores) | ||
| - name: Should fail if additionalArtifactStores exceeds maximum of 10 items | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalArtifactStores: | ||
| - path: /var/lib/store1 | ||
| - path: /var/lib/store2 | ||
| - path: /var/lib/store3 | ||
| - path: /var/lib/store4 | ||
| - path: /var/lib/store5 | ||
| - path: /var/lib/store6 | ||
| - path: /var/lib/store7 | ||
| - path: /var/lib/store8 | ||
| - path: /var/lib/store9 | ||
| - path: /var/lib/store10 | ||
| - path: /var/lib/store11 | ||
| expectedError: "Too many: 11: must have at most 10 items" | ||
|
|
||
| - name: Should fail if additionalArtifactStores path is not absolute | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalArtifactStores: | ||
| - path: var/lib/artifacts | ||
| expectedError: "path must be absolute and contain only alphanumeric characters, '/', '.', '_', and '-'" | ||
|
|
||
| - name: Should fail if additionalArtifactStores item has no path field | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalArtifactStores: | ||
| - {} | ||
| expectedError: "path: Required value" | ||
|
|
||
| - name: Should fail if additionalArtifactStores contains duplicate paths | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| additionalArtifactStores: | ||
| - path: /mnt/ssd-artifacts | ||
| - path: /mnt/ssd-artifacts | ||
| expectedError: "additionalArtifactStores must not contain duplicate paths" | ||
|
|
||
| # Combined test - all storage types together with other fields | ||
| - name: Should be able to create ContainerRuntimeConfig with all storage types and existing fields | ||
| initial: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| defaultRuntime: crun | ||
| logLevel: info | ||
| additionalLayerStores: | ||
| - path: /var/lib/stargz-store | ||
| additionalImageStores: | ||
| - path: /mnt/nfs-images | ||
| - path: /mnt/ssd-images | ||
| additionalArtifactStores: | ||
| - path: /mnt/ssd-artifacts | ||
| expected: | | ||
| apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: ContainerRuntimeConfig | ||
| spec: | ||
| containerRuntimeConfig: | ||
| defaultRuntime: crun | ||
| logLevel: info | ||
| additionalLayerStores: | ||
| - path: /var/lib/stargz-store | ||
| additionalImageStores: | ||
| - path: /mnt/nfs-images | ||
| - path: /mnt/ssd-images | ||
| additionalArtifactStores: | ||
| - path: /mnt/ssd-artifacts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -892,6 +892,60 @@ type ContainerRuntimeConfiguration struct { | |
| // +kubebuilder:validation:Enum=crun;runc | ||
| // +optional | ||
| DefaultRuntime ContainerRuntimeDefaultRuntime `json:"defaultRuntime,omitempty"` | ||
|
|
||
| // additionalLayerStores configures additional read-only container image layer store locations for Open Container Initiative (OCI) images. | ||
| // | ||
| // Layers are checked in order: additional stores first, then the default location. | ||
|
Contributor
There was a problem hiding this comment. nit: reading the godoc of the three fields we're adding is a bit confusing, since they're phrased very similarly. Would it make sense to link to upstream docs (not sure if that's something we do) or have a bit more description on the differences of behaviour? |
||
| // Stores are read-only. | ||
| // Maximum of 5 stores allowed. | ||
| // Each path must be unique. | ||
| // | ||
| // When omitted, only the default layer location is used. | ||
| // When specified, at least one store must be provided. | ||
| // | ||
| // +openshift:enable:FeatureGate=AdditionalStorageConfig | ||
| // +optional | ||
| // +listType=atomic | ||
| // +kubebuilder:validation:MinItems=1 | ||
| // +kubebuilder:validation:MaxItems=5 | ||
| // +kubebuilder:validation:XValidation:rule="self.all(x, self.exists_one(y, x.path == y.path))",message="additionalLayerStores must not contain duplicate paths" | ||
| AdditionalLayerStores []AdditionalLayerStore `json:"additionalLayerStores,omitempty"` | ||
|
|
||
| // additionalImageStores configures additional read-only container image store locations for Open Container Initiative (OCI) images. | ||
| // | ||
| // Images are checked in order: additional stores first, then the default location. | ||
| // Stores are read-only. | ||
| // Maximum of 10 stores allowed. | ||
| // Each path must be unique. | ||
| // | ||
| // When omitted, only the default image location is used. | ||
| // When specified, at least one store must be provided. | ||
| // | ||
| // +openshift:enable:FeatureGate=AdditionalStorageConfig | ||
| // +optional | ||
| // +listType=atomic | ||
| // +kubebuilder:validation:MinItems=1 | ||
| // +kubebuilder:validation:MaxItems=10 | ||
| // +kubebuilder:validation:XValidation:rule="self.all(x, self.exists_one(y, x.path == y.path))",message="additionalImageStores must not contain duplicate paths" | ||
| AdditionalImageStores []AdditionalImageStore `json:"additionalImageStores,omitempty"` | ||
|
|
||
| // additionalArtifactStores configures additional read-only artifact storage locations for Open Container Initiative (OCI) artifacts. | ||
| // | ||
| // Artifacts are checked in order: additional stores first, then the default location (/var/lib/containers/storage/artifacts). | ||
| // Stores are read-only. | ||
| // Maximum of 10 stores allowed. | ||
| // Each path must be unique. | ||
| // | ||
| // When omitted, only the default artifact location is used. | ||
| // When specified, at least one store must be provided. | ||
| // | ||
| // +openshift:enable:FeatureGate=AdditionalStorageConfig | ||
| // +optional | ||
| // +listType=atomic | ||
| // +kubebuilder:validation:MinItems=1 | ||
| // +kubebuilder:validation:MaxItems=10 | ||
| // +kubebuilder:validation:XValidation:rule="self.all(x, self.exists_one(y, x.path == y.path))",message="additionalArtifactStores must not contain duplicate paths" | ||
| AdditionalArtifactStores []AdditionalArtifactStore `json:"additionalArtifactStores,omitempty"` | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| type ContainerRuntimeDefaultRuntime string | ||
|
|
@@ -904,6 +958,66 @@ const ( | |
| ContainerRuntimeDefaultRuntimeDefault = ContainerRuntimeDefaultRuntimeCrun | ||
| ) | ||
|
|
||
| // AdditionalLayerStore defines a read-only storage location for Open Container Initiative (OCI) container image layers. | ||
|
Contributor
There was a problem hiding this comment. There's quite a bit of duplication here. What do you think about either:
|
||
| type AdditionalLayerStore struct { | ||
| // path specifies the absolute location of the additional layer store. | ||
| // | ||
| // The path must exist on the node before configuration is applied. | ||
| // When a container image is requested, layers found at this location will be used instead of | ||
| // retrieving from the registry. | ||
| // | ||
| // This field is required and must: | ||
| // - Have length between 1 and 256 characters | ||
| // - Start with '/' (absolute path) | ||
| // - Contain only: a-z, A-Z, 0-9, '/', '.', '_', '-' (no spaces or special characters) | ||
| // | ||
| // +required | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:MaxLength=256 | ||
| // +kubebuilder:validation:XValidation:rule="self.matches('^/[a-zA-Z0-9/._-]+$')",message="path must be absolute and contain only alphanumeric characters, '/', '.', '_', and '-'" | ||
| Path string `json:"path,omitempty"` | ||
| } | ||
|
|
||
| // AdditionalImageStore defines an additional read-only storage location for Open Container Initiative (OCI) images. | ||
| type AdditionalImageStore struct { | ||
| // path specifies the absolute location of the additional image store. | ||
| // | ||
| // The path must exist on the node before configuration is applied. | ||
| // When a container image is requested, images found at this location will be used instead of | ||
| // retrieving from the registry. | ||
| // | ||
| // This field is required and must: | ||
| // - Have length between 1 and 256 characters | ||
| // - Start with '/' (absolute path) | ||
| // - Contain only: a-z, A-Z, 0-9, '/', '.', '_', '-' (no spaces or special characters) | ||
| // | ||
| // +required | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:MaxLength=256 | ||
| // +kubebuilder:validation:XValidation:rule="self.matches('^/[a-zA-Z0-9/._-]+$')",message="path must be absolute and contain only alphanumeric characters, '/', '.', '_', and '-'" | ||
| Path string `json:"path,omitempty"` | ||
| } | ||
|
|
||
| // AdditionalArtifactStore defines an additional read-only storage location for Open Container Initiative (OCI) artifacts. | ||
| type AdditionalArtifactStore struct { | ||
| // path specifies the absolute location of the additional artifact store. | ||
| // | ||
| // The path must exist on the node before configuration is applied. | ||
| // When an artifact is requested, artifacts found at this location will be used instead of | ||
| // retrieving from the registry. | ||
| // | ||
| // This field is required and must: | ||
| // - Have length between 1 and 256 characters | ||
| // - Start with '/' (absolute path) | ||
| // - Contain only: a-z, A-Z, 0-9, '/', '.', '_', '-' (no spaces or special characters) | ||
| // | ||
| // +required | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:MaxLength=256 | ||
| // +kubebuilder:validation:XValidation:rule="self.matches('^/[a-zA-Z0-9/._-]+$')",message="path must be absolute and contain only alphanumeric characters, '/', '.', '_', and '-'" | ||
| Path string `json:"path,omitempty"` | ||
| } | ||
|
|
||
| // ContainerRuntimeConfigStatus defines the observed state of a ContainerRuntimeConfig | ||
| type ContainerRuntimeConfigStatus struct { | ||
| // observedGeneration represents the generation observed by the controller. | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.