-
Notifications
You must be signed in to change notification settings - Fork 591
Comments
OCPNODE-4060: Add additional storage configuration fields to ContainerRuntimeConfig#2681
OCPNODE-4060: Add additional storage configuration fields to ContainerRuntimeConfig #2681saschagrunert wants to merge 1 commit intoopenshift:master from
Conversation
openshift-ci-robot
commented
Jan 30, 2026
Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.
For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.
This repository is configured in: LGTM mode
@saschagrunert: This pull request references OCPNODE-4060 which is a valid jira issue.
Details
In response to this:
This enhancement extends ContainerRuntimeConfig API with three features for improved container storage flexibility:
additionalLayerStores: Enable lazy image pulling via storage plugins (BYOS approach with stargz-snapshotter, nydus-storage-plugin)
additionalImageStores: Read-only container image caches on shared or high-performance storage (NFS, SSD) for faster startup and reduced network overhead
additionalArtifactStores: Configurable OCI artifact storage locations (SSD-backed storage, pre-populated caches, air-gapped deployments)
All features target AI/ML workload performance improvements and will ship as Tech Preview in 4.22 behind TechPreviewNoUpgrade feature gate.
Path-based configuration with graceful fallback to standard behavior on failure. Unified API design pattern across all three features.
Requires openshift/enhancements#1934
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds three new container storage types and fields to ContainerRuntimeConfiguration: AdditionalLayerStores, AdditionalImageStores, and AdditionalArtifactStores, plus types AdditionalLayerStore, AdditionalImageStore, and AdditionalArtifactStore (each with a Path). Regenerates deepcopy and Swagger doc entries for the new types. Introduces a FeatureGate AdditionalStorageConfig and updates feature-gate payloads. Adds multiple ContainerRuntimeConfig CRD manifests (Default, DevPreviewNoUpgrade, TechPreviewNoUpgrade, OKD, CustomNoUpgrade) and a YAML test manifest validating create and path/count validations. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
Hello @saschagrunert! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.
|
@saschagrunert: This pull request references OCPNODE-4060 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
PR Compliance Guide 🔍
Below is a summary of compliance checks for this PR:
No security concerns identified
No security vulnerabilities detected by AI analysis. Human verification advised for critical code.🎫 No ticket provided
- Create ticket/issue
Codebase context is not defined
Follow the guide to enable codebase context checks.
Generic: Comprehensive Audit Trails
Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
Generic: Meaningful Naming and Self-Documenting Code
Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
Generic: Robust Error Handling and Edge Case Management
Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
Generic: Secure Error Handling
Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
Generic: Secure Logging Practices
Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
Generic: Security-First Input Validation and Data Handling
Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities
Status: Passed
Learn more about managing compliance generic rules or creating your own custom rules
- Update
Compliance status legend
🟢 - Fully Compliant🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign joelspeed for approval. For more information see the Code Review Process.
The full list of commands accepted by this bot can be found here.
Details
Needs approval from an approver in each of these files:Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@machineconfiguration/v1/types.go`:
- Around line 895-939: The new storage fields AdditionalLayerStores,
AdditionalImageStores, and AdditionalArtifactStores must be gated behind the
TechPreviewNoUpgrade feature gate; add the openshift feature-gate annotation
+openshift:enable:FeatureGate=TechPreviewNoUpgrade directly above each of those
field declarations in types.go so the CRD schema is only emitted when the
TechPreviewNoUpgrade gate is enabled (i.e., add the annotation to the comment
block for AdditionalLayerStores, AdditionalImageStores and
AdditionalArtifactStores).
🧹 Nitpick comments (1)
machineconfiguration/v1/tests/containerruntimeconfigs.machineconfiguration.openshift.io/AdditionalStorageConfig.yaml (1)
6-173: Add a couple of path-validation cases for image/artifact stores.Layer-store paths are thoroughly validated, but image/artifact store path constraints are only covered by max-items tests. A minimal invalid-path case for each would guard against drift.
✅ Proposed test additions
@@ - 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: "additionalImageStores in body should 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 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: "additionalArtifactStores in body should 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 '-'"
37efaf4 to
a5273ad
Compare
...ation This commit addresses CodeRabbit PR review feedback by: 1. Feature Gate Definition (AdditionalStorageConfig): - Created proper feature gate "AdditionalStorageConfig" in features/features.go - Gate enabled in DevPreviewNoUpgrade and TechPreviewNoUpgrade feature sets - Linked to enhancement PR openshift/enhancements#1934 - Contact: saschagrunert, Component: MachineConfigOperator 2. Feature Gating Applied: - Added +openshift:enable:FeatureGate=AdditionalStorageConfig to all three storage field declarations (AdditionalLayerStores, AdditionalImageStores, AdditionalArtifactStores) - Fields now only appear in CRD schema when AdditionalStorageConfig feature gate is enabled (DevPreview/TechPreview) - Generated feature-gated CRD manifest: AdditionalStorageConfig.yaml 3. Enhanced Test Coverage: - Added path validation tests for additionalImageStores (non-absolute path, empty struct/MinProperties) - Added path validation tests for additionalArtifactStores (non-absolute path, empty struct/MinProperties) - Ensures all three store types have consistent validation coverage - Total test count: 15 tests 4. Path Validation Enhancement: - Updated regex to allow dots in paths: ^/[a-zA-Z0-9/._-]+$ - Documentation updated to reflect dot support - Test paths include dots (e.g., /opt/layer_store-v1.0) Verification: - Fields present in AdditionalStorageConfig.yaml (feature-gated) ✓ - Fields NOT present in AAA_ungated.yaml ✓ - Linter passes with 0 issues ✓ Addresses: openshift#2681 (review) Addresses: openshift#2681 (comment) Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
ec4b294 to
69ba824
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@features.md`:
- Line 27: The new table row with "AdditionalStorageConfig" violates MD060
(inconsistent pipe spacing); update the row so pipe characters align with the
table's existing spacing style (match spaces around pipes used by other rows),
ensuring consistent left/right spacing around each '|' and the same number of
cells as the header to fix the lint error for the line containing
"AdditionalStorageConfig".
In
`@payload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-CustomNoUpgrade.crd.yaml`:
- Around line 224-227: Fix the typo in the CRD description for
machineConfigPoolSelector under the ContainerRuntimeConfig resource: change
"shoud" to "should" in the description string for machineConfigPoolSelector to
read "machineConfigPoolSelector selects which pools the ContainerRuntimeConfig
should apply to."
In
`@payload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-Default.crd.yaml`:
- Around line 98-101: Typo: the description for machineConfigPoolSelector in the
ContainerRuntimeConfig CRD contains "shoud" instead of "should"; update the
source type definition that generates these CRDs (the ContainerRuntimeConfig
description field / machineConfigPoolSelector docstring) to correct "shoud" →
"should" so all generated CRD variants (machineConfigPoolSelector in
ContainerRuntimeConfig) get the fixed wording.
In
`@payload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-DevPreviewNoUpgrade.crd.yaml`:
- Around line 52-97: The CRD is missing the containerRuntimeConfig fields
additionalLayerStores, additionalImageStores, and additionalArtifactStores; add
these three properties under containerRuntimeConfig mirroring the
CustomNoUpgrade CRD's schema: each property should be an array of strings with
item length constraints (1-256 chars), pattern/validation matching the existing
CRD, and min/max item constraints (additionalLayerStores: minItems 1 maxItems 5;
additionalImageStores and additionalArtifactStores: minItems 1 maxItems 10);
ensure types and x-kubernetes validations align with
machineconfiguration/v1/types.go and the CustomNoUpgrade CRD definitions so
schema and validation are consistent.
In
`@payload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-TechPreviewNoUpgrade.crd.yaml`:
- Around line 98-101: Fix the typo in the CRD description for the field
machineConfigPoolSelector by changing "shoud" to "should" in both the
ContainerRuntimeConfig and the CustomNoUpgrade variant descriptions; locate the
description block under the machineConfigPoolSelector key and update the wording
to "machineConfigPoolSelector selects which pools the ContainerRuntimeConfig
should apply to." for both occurrences.
- Around line 1-97: The TechPreviewNoUpgrade CRD is missing the storage fields
controlled by the AdditionalStorageConfig feature gate (additionalLayerStores,
additionalImageStores, additionalArtifactStores); update the
ContainerRuntimeConfig schema under spec.containerRuntimeConfig in this CRD to
add those three fields with the same types/anyOf/x-kubernetes-int-or-string and
validation as the CustomNoUpgrade variant and annotate them the same way they
are annotated for the feature gate (i.e. the
+openshift:enable:FeatureGate=AdditionalStorageConfig behavior reflected in the
OpenAPI schema), or alternatively restrict the feature gate scope in
features/features.go to exclude TechPreviewNoUpgrade to keep schemas consistent
with the gate.
🧹 Nitpick comments (2)
machineconfiguration/v1/tests/containerruntimeconfigs.machineconfiguration.openshift.io/AdditionalStorageConfig.yaml (2)
102-142: Consider adding positive test case for additionalImageStores.The tests for
additionalImageStorescover validation errors well (max items, non-absolute path, missing path), but there's no positive test case showing successful creation with valid image store paths, unlikeadditionalLayerStoreswhich has one at lines 7-25.
143-183: Consider adding positive test case for additionalArtifactStores.Similarly,
additionalArtifactStoresvalidation tests are present but a positive test case demonstrating successful creation with valid artifact store paths would provide more complete coverage.
...ad-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-CustomNoUpgrade.crd.yaml
Show resolved
Hide resolved
payload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-Default.crd.yaml
Show resolved
Hide resolved
...anifests/crds/0000_80_machine-config_01_containerruntimeconfigs-DevPreviewNoUpgrade.crd.yaml
Show resolved
Hide resolved
...nifests/crds/0000_80_machine-config_01_containerruntimeconfigs-TechPreviewNoUpgrade.crd.yaml
Show resolved
Hide resolved
...nifests/crds/0000_80_machine-config_01_containerruntimeconfigs-TechPreviewNoUpgrade.crd.yaml
Show resolved
Hide resolved
69ba824 to
d9643eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@machineconfiguration/v1/tests/containerruntimeconfigs.machineconfiguration.openshift.io/AdditionalStorageConfig.yaml`:
- Line 45: Update the test expectations in AdditionalStorageConfig.yaml where
the expectedError string omits the '.' character: change the three occurrences
of expectedError ("path must be absolute and contain only alphanumeric
characters, '/', '_', and '-'") to include the '.' in the allowed-character list
(e.g., "path must be absolute and contain only alphanumeric characters, '/',
'_', '-', and '.'") so they match the CRD regex; the affected values are the
expectedError fields used in the test cases around the three entries currently
failing.
In
`@payload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-CustomNoUpgrade.crd.yaml`:
- Around line 164-174: The validation message for the additionalLayerStores
field is inconsistent: the description and the x-kubernetes-validations regular
expression (rule: self.matches('^/[a-zA-Z0-9/._-]+$')) allow the '.' character
but the message omits it; update the x-kubernetes-validations message to list
'/', '.', '_', and '-' (e.g., "path must be absolute and contain only
alphanumeric characters, '/', '.', '_', and '-'") so it matches the description
and the regex for additionalLayerStores and the rule
self.matches('^/[a-zA-Z0-9/._-]+$').
In
`@payload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-DevPreviewNoUpgrade.crd.yaml`:
- Around line 224-227: Fix the typo in the CRD description for the
machineConfigPoolSelector field: change "shoud" to "should" in the description
string for machineConfigPoolSelector in the containerruntimeconfigs CRD (the
multi-line description under machineConfigPoolSelector).
...tests/containerruntimeconfigs.machineconfiguration.openshift.io/AdditionalStorageConfig.yaml
Outdated
Show resolved
Hide resolved
...ad-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-CustomNoUpgrade.crd.yaml
Show resolved
Hide resolved
...anifests/crds/0000_80_machine-config_01_containerruntimeconfigs-DevPreviewNoUpgrade.crd.yaml
Show resolved
Hide resolved
d9643eb to
50e1e8a
Compare
saschagrunert
commented
Jan 30, 2026
@qodo-code-review:
- "Allow dots in path regex": Should be addressed
- "Consolidate redundant storage types into one": I'm going to keep separate types for future API flexibility.
- "Always serialize required path": Omitempty is required, otherwise the linter will complain on purpose.
4520cd0 to
3ca284c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@machineconfiguration/v1/types.go`:
- Around line 895-945: The fields AdditionalLayerStores, AdditionalImageStores,
and AdditionalArtifactStores document an ordered lookup but are annotated with
+listType=map (+listMapKey=path), which makes identity key-based and means
reordering won’t be treated as an update; if ordering is functionally critical
change the list semantics to atomic by replacing +listType=map and
+listMapKey=path with +listType=atomic for each of AdditionalLayerStores,
AdditionalImageStores, and AdditionalArtifactStores (and ensure validations
still make sense), otherwise update the field comments to explicitly state that
order is not enforced by the API and consumers must determine priority
themselves.
3ca284c to
5bfc936
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@machineconfiguration/v1/types.go`:
- Around line 929-930: The doc comment for the field additionalArtifactStores
uses "OCI volumes" but the store type below uses "OCI artifacts"—pick one term
for consistency (prefer using "OCI artifacts" to match the store type) and
update the comment text accordingly; locate the comment attached to
additionalArtifactStores in machineconfiguration/v1/types.go and change "OCI
volumes" to "OCI artifacts" (or vice versa if you prefer that term) so generated
docs use a single consistent phrase.
259a7fe to
628b5f1
Compare
saschagrunert
commented
Feb 5, 2026
@yuqi-zhang PTAL for shadow review if feasible.
@yuqi-zhang
yuqi-zhang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General structure makes sense based on enhancement. Some suggestions/questions inline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's quite a bit of duplication here. What do you think about either:
- have Path be of a new type, with validation on the type directly (e.g. https://github.com/openshift/api/blob/master/machineconfiguration/v1/types_machineosconfig.go#L198)
- collapse AdditionalLayerStore, AdditionalImageStore, AdditionalArtifactStore into one type called AdditionalStore, and have the difference in godoc reflect via the subfields in ContainerRuntimeConfiguration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main idea was to be able to separate the configurations later on when required. Means that AdditionalLayerStore may have different configuration additions than AdditionalArtifactStore. This would allow us the flexibility to develop the features into certain directions without coupling them together from day 1. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with that. Curious if Joel or Bryce has any better suggestions as well, so will wait for their review here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was thinking into a similar direction. Unfortunately we don't have much upstream docs and would require extensive OpenShift documentation about the features functionalities.
additionalimagestores=[] Paths to additional container image stores. Usually these are read/only and stored on remote network shares.
And even less for the additional layer store...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, any additional godoc here that you think makes sense would be helpful, but matching what we have in container-libs seem fine as a start.
This enhancement extends ContainerRuntimeConfig API with three features for improved container storage flexibility: - additionalLayerStores: Enable lazy image pulling via storage plugins (BYOS approach with stargz-snapshotter, nydus-storage-plugin) - additionalImageStores: Read-only container image caches on shared or high-performance storage (NFS, SSD) for faster startup and reduced network overhead - additionalArtifactStores: Configurable OCI artifact storage locations (SSD-backed storage, pre-populated caches, air-gapped deployments) All features target AI/ML workload performance improvements and will ship as Tech Preview in 4.22 behind TechPreviewNoUpgrade feature gate. Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
628b5f1 to
0fbbf8e
Compare
saschagrunert
commented
Feb 12, 2026
/pipeline auto
openshift-ci-robot
commented
Feb 12, 2026
Pipeline controller notification
The pipeline-auto label has been added to this PR. Second-stage tests will be triggered automatically when all first-stage tests pass.
openshift-ci-robot
commented
Feb 12, 2026
Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn
/test e2e-aws-ovn-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2
/test e2e-aws-serial-techpreview-1of2
/test e2e-aws-serial-techpreview-2of2
/test e2e-azure
/test e2e-gcp
/test e2e-upgrade
/test e2e-upgrade-out-of-change
/test minor-e2e-upgrade-minor
|
@saschagrunert: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
openshift-ci-robot
commented
Feb 12, 2026
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage.
@yuqi-zhang
yuqi-zhang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally seems fine to me, will mark as shadow review complete. Will follow to see if we have suggestions on simplification of the types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recall I made a suggestion to lift this into a separate type. But for some reason I can't see it in the comment history
Uh oh!
There was an error while loading. Please reload this page.
User description
This enhancement extends ContainerRuntimeConfig API with three features for improved container storage flexibility:
additionalLayerStores: Enable lazy image pulling via storage plugins (BYOS approach with stargz-snapshotter, nydus-storage-plugin)
additionalImageStores: Read-only container image caches on shared or high-performance storage (NFS, SSD) for faster startup and reduced network overhead
additionalArtifactStores: Configurable OCI artifact storage locations (SSD-backed storage, pre-populated caches, air-gapped deployments)
All features target AI/ML workload performance improvements and will ship as Tech Preview in 4.22 behind TechPreviewNoUpgrade feature gate.
Path-based configuration with graceful fallback to standard behavior on failure. Unified API design pattern across all three features.
Requires openshift/enhancements#1934
PR Type
Enhancement
Description
Add three new storage configuration fields to ContainerRuntimeConfig API
Implement comprehensive validation with path constraints and item limits
Generate required Kubernetes API machinery code and CRD manifests
Add comprehensive test suite validating all storage configuration scenarios
Diagram Walkthrough
File Walkthrough
types.go
Define storage configuration types and validation rulesmachineconfiguration/v1/types.go
AdditionalLayerStores,AdditionalImageStores,AdditionalArtifactStoresAdditionalLayerStore,AdditionalImageStore,AdditionalArtifactStorewithpathfield(MinLength, MaxLength, XValidation regex, MinItems, MaxItems)
constraints, and behavior of each field
zz_generated.deepcopy.go
Auto-generate deepcopy functions for storage typesmachineconfiguration/v1/zz_generated.deepcopy.go
AdditionalLayerStoretype
AdditionalImageStoretype
AdditionalArtifactStoretype
storage configuration fields
zz_generated.swagger_doc_generated.go
Generate swagger documentation for storage typesmachineconfiguration/v1/zz_generated.swagger_doc_generated.go
AdditionalLayerStore,AdditionalImageStore,AdditionalArtifactStorestorage configuration fields
AdditionalStorageConfig.yaml
Add comprehensive validation test suitemachineconfiguration/v1/tests/containerruntimeconfigs.machineconfiguration.openshift.io/AdditionalStorageConfig.yaml
configuration
additionalArtifactStores max item limits
existing fields
non-absolute paths, spaces, invalid characters, length limits, item
count limits, missing required fields
0000_80_machine-config_01_containerruntimeconfigs.crd.yaml
Update CRD manifest with storage configuration schemamachineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_containerruntimeconfigs.crd.yaml
additionalLayerStores,additionalImageStores,additionalArtifactStoresfieldsminItems, maxItems, x-kubernetes-validations regex
AAA_ungated.yaml
Update featuregated CRD manifest with storage configurationmachineconfiguration/v1/zz_generated.featuregated-crd-manifests/containerruntimeconfigs.machineconfiguration.openshift.io/AAA_ungated.yaml
additionalLayerStores,additionalImageStores,additionalArtifactStoresfieldsminItems, maxItems, x-kubernetes-validations regex
manifest
0000_80_machine-config_01_containerruntimeconfigs.crd.yaml
Update payload CRD manifest with storage configurationpayload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs.crd.yaml
additionalLayerStores,additionalImageStores,additionalArtifactStoresfieldsminItems, maxItems, x-kubernetes-validations regex
manifest