OCPBUGS-83492: Allow :ref suffix in additionalLayerStores path for stargz-store to support lazy image pulling#2806
Conversation
|
Skipping CI for Draft Pull Request. |
|
Hello @BhargaviGudi! Some important instructions when contributing to openshift/api: |
|
@BhargaviGudi: This pull request references Jira Issue OCPBUGS-83492, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughIntroduces a new Go type 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.11.4)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/jira refresh |
|
@BhargaviGudi: This pull request references Jira Issue OCPBUGS-83492, which is invalid:
Comment 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@machineconfiguration/v1/tests/containerruntimeconfigs.machineconfiguration.openshift.io/AdditionalStorageConfig.yaml`:
- Around line 103-114: Update the outdated expectedError strings in the
ContainerRuntimeConfig test cases that assert additionalLayerStores validation:
find the test entries referencing additionalLayerStores (look for the initial
YAML blocks under ContainerRuntimeConfig and the existing expectedError values)
and change the older message fragments ("path must be absolute and contain only
alphanumeric characters, '/', '.', '_', and '-'") to the new message including
the optional ':ref' wording ("path must be absolute and contain only
alphanumeric characters, '/', '.', '_', '-', and optionally end with ':ref'") so
they match LayerStorePath's current error output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5825084f-d1fd-4ab7-a6a1-9bff27fa4b7d
⛔ Files ignored due to path filters (5)
machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_containerruntimeconfigs-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_containerruntimeconfigs-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_containerruntimeconfigs-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.featuregated-crd-manifests/containerruntimeconfigs.machineconfiguration.openshift.io/AdditionalStorageConfig.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*
📒 Files selected for processing (5)
machineconfiguration/v1/tests/containerruntimeconfigs.machineconfiguration.openshift.io/AdditionalStorageConfig.yamlmachineconfiguration/v1/types.gopayload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-TechPreviewNoUpgrade.crd.yaml
|
/jira refresh |
|
@BhargaviGudi: This pull request references Jira Issue OCPBUGS-83492, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (bgudi@redhat.com), skipping review request. 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. |
|
/test all |
942df24 to
9abf08e
Compare
|
/retest-required |
everettraven
left a comment
There was a problem hiding this comment.
Aside from a small nit to make the godoc and the generated documentation read a bit better, this LGTM.
…upport lazy image pulling Introduced LayerStorePath type for additionalLayerStores that allows paths ending with :ref (e.g., /var/lib/stargz-store/store:ref), while keeping StorePath unchanged for additionalImageStores and additionalArtifactStores. Fixes: https://issues.redhat.com/browse/OCPBUGS-83492
9abf08e to
590dfad
Compare
|
@BhargaviGudi: all tests passed! 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. |
Summary
:refsuffix inadditionalLayerStorespath to support lazy image pulling with stargz-store.Problem
ContainerRuntimeConfig API validation rejects the
:refsuffix inadditionalLayerStorespaths, preventing proper configuration of stargz-store for lazy pulling functionality.The path
/var/lib/stargz-store/store:refis required for stargz-store to work with CRI-O's additionalLayerStores mechanism, as documented in the upstream stargz-snapshotter installation guide.Current behavior:
Testing confirmed:
Solution
Changes:
Testing
Test cases added to verify: