feat(containerd): only import local-path-provisioner when needed - #195
Merged
stevensbkang merged 2 commits intoSep 17, 2026
Merged
Conversation
… storage is enabled When local-path storage is disabled, kubesolo no longer imports the local-path-provisioner image into containerd. This avoids requiring that image (and a registry pull) on systems that do not use local-path storage, such as constrained or air-gapped devices.
There was a problem hiding this comment.
🟡 Changes recommended
Critical golden-fixture failures and missing enabled-case coverage remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Conditionally imports the local-path-provisioner image only when local-path storage is enabled.
Changes:
- Adds local-storage state to embedded configuration.
- Propagates the setting to containerd.
- Skips unnecessary image imports.
File summaries
| File | Reviewed change | Findings |
|---|---|---|
types/types.go |
Adds the embedded local-storage flag. | Critical (1 vote): Golden fixtures omit LocalStorage, causing exact-output tests to fail; update fixtures and cover both states. |
pkg/runtime/containerd/service.go |
Stores the local-storage setting in the containerd service. | No findings. |
pkg/runtime/containerd/image.go |
Conditionally imports the provisioner image. | No findings. |
internal/config/embedded.go |
Maps configuration into embedded runtime settings. | Critical (2 votes): Golden fixtures must include LocalStorage. Moderate (1 vote): Add an enabled case to cover the new mapping and import path. |
Review details
Suppressed comments (1)
internal/config/embedded.go:191
- The
goldenCasestable never setsc.Storage.LocalPath.Enabledto true, so this new mapping—and the enabled path that imports the provisioner image—has no test coverage. Add an enabled case and assert thatLocalStorageis true in its golden output.
LocalStorage: cfg.Storage.LocalPath.Enabled,
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Add explicit golden test cases for local storage enabled and disabled so the LocalStorage mapping in BuildEmbedded is directly covered. Regenerate all existing fixtures to include the new LocalStorage field. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Note PR image published: |
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, correctly gates the image import on the existing local-path enablement flag, and updates golden fixtures to lock in the new behavior.
Review details
- Files reviewed: 31/31 changed files
- Comments generated: 0 new
- Review effort level: Lite
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When local-path storage is disabled, kubesolo no longer imports the local-path-provisioner image into containerd. This avoids requiring that image (and a registry pull) on systems that do not use local-path storage.