Result: Merged (2026-10-26 19:58, kubevirt/containerized-data-importer#4319), issue #3005 closed. As of 2026-10-29, there is no CDI release containing this change yet (the latest is v1.66.1 from 2026-09-06). In fact, this was taken from GitHub PRs, reviews, inline comments, body edit history, timeline, inspection logs, the CDI source (base commit of the PR
9cd4fbdb), the local-path-provisioner source (tag v0.0.32), and raw contribution ledger data (StorageProfile list, 48-hour log aggregation), as well as the draft. Times are in Korea Standard Time.
Table of Contents
- One-Line Summary
- 1. What Was the Problem: Empty StorageProfile
- 2. Cause: Which Line in the Source
- 3. What Was Changed: Three Diffs
- 4. Reaction: Group Them, Remove Them, and the Reopened Issue
- 5. Lessons Learned: Open Source Contribution Tips
- Links
One-Line Summary
| PR | kubevirt/containerized-data-importer#4319 Add storage capabilities for rancher.io/local-path |
| Changes | storagecapabilities.go One file. rancher.io/local-path item and one helper shared with the node local path provisioner. 12 lines added, 4 lines deleted |
| Feedback | One inline comment each from two reviewers (group them, remove the two tests), maintainer reopened and linked an old issue |
| Result | PR opened 10-17 23:38 → /approve 10-19 16:59 → /lgtm 10-26 15:15 → Merged 19:58, approx. 8 days, 20 hours, 20 minutes. 3 commits, 21 checks passed |
1. What Was the Problem: Empty StorageProfile
CDI (Containerized Data Importer) is a component that creates disks for KubeVirt VMs as PVCs. If you use DataVolume as spec.storage , you don't need to specify access modes or volume modes, and those defaults are fetched from the StorageProfile created for each StorageClass. CDI populates the StorageProfile defaults by looking at the provisioner name.
When inspecting the TT Lab cluster (CDI v1.66.0) with a read command on 2026-10-17, two out of four StorageProfiles were empty (ledger measure/kv).
| StorageProfile | Provisioner | claimPropertySets | Recognized |
|---|---|---|---|
| local-path | rancher.io/local-path |
None | UnrecognizedProvisioner |
| local-path-retain | rancher.io/local-path |
None | UnrecognizedProvisioner |
| Two NFS classes | nfs.csi.k8s.io |
RWX, Filesystem | RecognizedProvisioner |
The two local-path classes were created 57 days ago, and the provisioner image was v0.0.32. In the cdi-deployment 48-hour logs, for each of the two classes, there were
Set metric:local-path complete:false … degraded:true lines appearing ten times each. As stated in the PR body, this metric is CDIStorageProfilesIncomplete
monitored by alerts.
In this state, if you create a DataVolume with spec.storage leaving the access mode empty, it stops at ErrClaimNotValid: no accessMode specified in StorageProfile local-path
as written in the PR body. This was written in the draft as "commands for users to run and expected results," but there is no record in the ledger of it actually being run (unmeasured). There is no record in the ledger that TT Lab's VMs were halted by this error. It was not an operational incident, but a problem found during investigation.
2. Cause: Which Line in the Source
CDI line numbers are based on the PR's base commit 9cd4fbdb .
- The table of provisioner names and defaults is
storagecapabilities.gofrom line 39 ofCapabilitiesByProvisionerKey. hostpath-provisioner andk8s.io/minikube-hostpathare present as{{rwo, file}}, butrancher.io/local-pathwas missing. - If it is not in the table,
storageprofile-controller.golines 206–220 ofreconcileConditionssets Recognized toFalse, and the reason toUnrecognizedProvisioner(message "Provisioner is not recognized"). The log line from Section 1 is printed at line 462 of the same file.
The basis for the values to be inserted as {{rwo, file}} is the provisioner-side source (local-path-provisioner v0.0.32 provisioner.go).
- Line 345: If an access mode other than RWO or RWOP is requested for a node path volume, it is rejected with
NodePath only supports ReadWriteOnce and ReadWriteOncePod (1.22+) access modes. - Line 405·Line 458:
The PVs it creates are always
VolumeMode: &fs(Filesystem), so Block is never output. - RWX is possible if you configure a shared filesystem path separately, but it is not the default, and you cannot tell just by looking at the StorageClass. I wrote in the main text that such users can directly specify the StorageProfile's
spec.claimPropertySetsas shown now. Since there are no snapshot or CSI cloning features, the clone strategy leaves the default chosen now (copy) as is.
I wrote one more thing in the PR body. CDI's own kind development environment (cluster-up/cluster/kind/manifests/local-storage-class.yaml) also uses this provisioner.
Since that environment's StorageProfile will also be populated after this change, I asked to let me know if there are any e2e tests that expect it to be empty.
3. What was changed: Diff across three commits
The first commit (10-17) added a line to the table, added a test for the new provisioner and a Recognized table entry to the StorageProfile controller tests.
It looks similar to a precedent, #4286(Mayastor, merged 2026-09-27). The test results in the PR body were like this (Go 1.27.2, GOFLAGS=-mod=vendor, main 9cd4fbd).
| test | results |
|---|---|
| New test, without table entry | 2 failed out of 7 |
| New test, with table entry | Passed |
go test ./pkg/controller/ Fixed version / Before fixing |
385 passed, 3 failed / 383 passed, 3 failed |
I noted in the body that the three failures were the same VDDK-related tests on both sides (Update PVC from POD), so they are unrelated to this change.
The second commit (10-18) incorporated the reviewer's suggestion. The feedback was to group them since provisioners that create volumes with a single node directory are ultimately all {rwo, file} . So I created a createLocalPathCapabilities() helper and used it in hostpath-provisioner (CSI and legacy), and k8s.io/minikube-hostpath,
rancher.io/local-path . The overlay variant of hostpath-provisioner was left as is because it is RWX. DirectPV, which exposes an entire drive rather than a directory, was left out, and I replied that it could be added if desired. At this time, go test ./pkg/controller/ passed all 388 tests (value mentioned in reply).
The third commit (10-26) deleted two unit tests from the first commit per another reviewer's feedback. The point was that the new provisioner simply reuses a helper that already has tests, so there is no new logic. This commit only ran gofmt·go vet and did not rerun the full tests (ledger record). The CI tests passed before merging.
var CapabilitiesByProvisionerKey = map[string][]StorageCapabilities{
// hostpath-provisioner
- ProvisionerHPPCSI: {{rwo, file}},
+ ProvisionerHPPCSI: createLocalPathCapabilities(),
ProvisionerHPPOverlayCSI: {{rwx, file}},
- ProvisionerHPPLegacy: {{rwo, file}},
- "k8s.io/minikube-hostpath": {{rwo, file}},
+ ProvisionerHPPLegacy: createLocalPathCapabilities(),
+ // other node-local path provisioners
+ "k8s.io/minikube-hostpath": createLocalPathCapabilities(),
+ "rancher.io/local-path": createLocalPathCapabilities(), // local-path-provisioner, default in k3s and kind
...
+// createLocalPathCapabilities is shared by provisioners that back each volume
+// with a directory on a single node, so they only offer RWO filesystem volumes.
+func createLocalPathCapabilities() []StorageCapabilities {
+ return []StorageCapabilities{{rwo, file}}
+}
The final diff is one file, 12 additions and 4 deletions. It actually became smaller than the initial one.
AI usage was indicated in all three commits with a Assisted-by: Claude trailer and DCO signature, and at the end of the body with a paragraph stating: 「Claude was used to help read the provisioner source and write the change and test drafts, all lines were reviewed, and the StorageProfile output is from my cluster.」 This follows the recommendation of the KubeVirt organization's
AI contribution policy. SSH signatures are only attached to the first commit, while the second and third commits reflecting reviews appear as unsigned commits on GitHub.
4. Reactions: Group them, remove them, and the reopened issue
| Time | Day |
|---|---|
| 10-17 23:38 | PR published. prow adds size/S label, notice 「untrusted authors cannot run tests directly」, two reviewers assigned |
| 10-18 20:25 | Reviewer A's inline comment: These local path provisioners are ultimately all rwo and file, so we could group them |
| 10-18 21:03 | Second commit grouped with a helper, 21:05 reply. Label changed to size/M as |
| 10-19 16:57 | A maintainer reopened issue #3005 which was automatically closed in 2024-10 /reopen |
| 10-19 16:58, 17:00 | The same maintainer edited the 「Which issue(s)」 section of our PR body to add Fixes #3005 (PR body revision history) |
| 10-19 16:59 | Reviewer A added /approve and /test all. 17:13 coveralls coverage 50.288% (+0.08%) |
| 10-25 11:33 | For six days, there was no /lgtm , so I requested the two assigned reviewers once: 「There is an approve and tests passed, so only lgtm remains」 |
| 10-25 17:53 | Reviewer B's inline comment: I don't know why two unit tests are needed when there is no new logic |
| 10-26 00:04 | Third commit deleting the two tests, 00:07 reply. Label changed back to size/S |
| 10-26 15:15 | Reviewer B added /lgtm |
| 10-26 19:58 | tide merged, issue #3005 closed along with it |
The most educational moment in this PR was a two-line event on 10-19. When writing the draft, we noted that there were no issues or PRs addressing the same problem (「0 search results」 in the ledger's draft list), and also put NONE in the issue field of the PR body. However, #3005
「StorageProfile needs to be patched for the rancher local-path provisioner」 already existed. It was opened on 2023-11-24, and another maintainer replied in essence (2024-01) that 「this is the default behavior of an unrecognized provisioner, so you should specify the access mode in the DV or fix the profile, or you can submit a PR adding defaults」. Since no one submitted a PR afterward, the bot automatically closed it on 2024-10-03. Our search apparently only looked at open ones.
The maintainer did not point this out separately. Instead, they reopened the issue and linked it to our PR body, causing the issue that had been dormant for 2 years and 11 months to be closed upon merging.
Both inline comments were accepted immediately with short replies. The first was 「Thanks for the review, grouped with a helper in the second commit, removed DirectPV because it exposes a whole drive rather than a directory」, and the second was 「Agreed, deleted both since it just reuses a helper already covered by existing hostpath tests」. Both reviewers' feedback reduced both what was added in the first commit (tests) and the changed shape (helper), resulting in a smaller PR.
5. Lessons learned: Tips for open source contribution
- 「Include closed issues in duplicate searches.」 Automatically closed (rotten) issues often mean 「no one fixed it」 rather than 「resolved」. When looking for such issues
Fixes #번호with it and quoted the maintainer's response in the issue ("You can send a PR") in the body, the review likely would have been faster. - 「The basis for the value is found in the source of the relative project.」
{rwo, file}since the reason for choosing it seemed to be the error message of local-path-provisioner and the line creating the PV, no one in the review asked about the value itself. All questions were about code shape (grouping, test count). - 「If the review shrinks the PR, accept it as is.. Adding more tests is not always good. The reviewer's perspective was that adding tests without new logic just increases the code to maintain. The scope desired by the maintainer can be narrower than the scope decided by the original author.
- 「If lgtm is missing for a long time after approve, politely ask once with just the facts.. "There is an approve and tests passed, so only lgtm is left" was a single sentence, and the review moved again that day.
A word from TT. Our neighborhood warehouse keeper said he had never heard of the name "Dirt Road Warehouse." So he put a "Don't know how to store" label on every box left there. I found in the warehouse rulebook and let him know that Dirt Road Warehouse accepts only one person per slot and only in the shape of a box. The warehouse uncle suggested listing four similar warehouses in a row, and another uncle said two confirmation stamps aren't needed. And someone pulled a note with the exact same request written 3 years ago out of a drawer and put it into the "Resolved" box along with our note.
Link
All materials were accessed on 2026-10-29.
- PR: kubevirt/containerized-data-importer#4319 (Merge commit
fd4146ac) - Closed issue: #3005 (Opened 2023-11-24, automatically closed 2024-10-03, reopened 2026-10-19)
- Modified source (base commit):
storagecapabilities.go,storageprofile-controller.go - local-path-provisioner source (v0.0.32):
provisioner.goline 345, line 458 - Precedent: containerized-data-importer#4286 Mayastor default value
- PR from the same investigation on the same day: Part 6, kubevirt/kubevirt#19499