Fix possible nil ptr in snapshot/pvc clone controllers #2900
Conversation
kubevirt-bot
added
release-note
|
/lgtm |
|
/cc @mhenriks |
| @@ -524,7 +524,7 @@ func isCrossNamespaceClone(dv *cdiv1.DataVolume) bool { | |||
| } | |||
|
|
|||
| // addCloneWithoutSourceWatch reconciles clones created without source once the matching PVC is created | |||
| func addCloneWithoutSourceWatch(mgr manager.Manager, datavolumeController controller.Controller, typeToWatch client.Object, indexingKey string) error { | |||
| func addCloneWithoutSourceWatch(mgr manager.Manager, datavolumeController controller.Controller, typeToWatch client.Object, indexingKey string, op dataVolumeOp) error { | |||
There was a problem hiding this comment.
It's the datavolume requested operation so one of these
containerized-data-importer/pkg/controller/datavolume/controller-base.go
Lines 157 to 164 in c512296
spec.source.snapshot index pointing to a spec.source.pvc data volume key
There was a problem hiding this comment.
Okay so Op means operation, could you just name it operation, so everyone understands what it means? I was trying to figure out what it stood for.
There was a problem hiding this comment.
So op is consistent across the project, should I refactor?
We slap on the clone-without-source index key regardless of source.PVC/source.Snapshot so it was possible for a DV with PVC source to get queued for the snapshot controller (and hence the nil ptr). The test addition demonstrates a real use case where this would happen. Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
akalenyu
force-pushed
the
nil-ptr-sourceless-index
branch
from
c512296
to
1db47ec
Compare
|
/retest-required |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: awels The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubevirt-bot
added
the
approved
|
/cherrypick release-v1.57 |
|
@akalenyu: new pull request created: #2905 In 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 kubernetes/test-infra repository. |
What this PR does / why we need it:
We slap on the clone-without-source index key regardless of source.PVC/source.Snapshot so
it was possible for a DV with PVC source to get queued for the snapshot controller
(and hence the nil ptr).
The test addition demonstrates a real use case where this would happen.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
https://issues.redhat.com/browse/CNV-33002
Special notes for your reviewer:
Release note: