LabelSelector should not be added while restoring pvc

[muxuelanKK]

When old pvc had pv selector, new pvc should not include it.

[kubevirt-bot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

[kubevirt-bot]

Hi @muxuelanKK. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

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.

[kubevirt-bot]

[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 alicefr for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• pkg/storage/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mhenriks]

/test all

[akalenyu]

Thanks for the PR!

I think the labelselector is just one symptom of a broader issue where we just copy out the entire spec of the source PVC.
Maybe we should take look at only setting fields we care about instead of

kubevirt/pkg/storage/snapshot/restore.go

Line 998 in 817c3ad

Spec: sourcePVC.Spec,

[mhenriks]

@muxuelanKK is this affecting the restore at all? Curious if/how PV label selectors could have any effect when creating a PVC with VolumeSnapshot datasource

@akalenyu I think this is the only field that could potentially be problematic. What do you think?

[akalenyu]

@muxuelanKK is this affecting the restore at all? Curious if/how PV label selectors could have any effect when creating a PVC with VolumeSnapshot datasource

@akalenyu I think this is the only field that could potentially be problematic. What do you think?

This

kubevirt/pkg/storage/snapshot/restore.go

Line 1041 in 817c3ad

pvc.Spec.VolumeName = ""

is a bit clunky too, alongside datasource&datasourceref.
Maybe we could just set them all explicitly without copying, that would be nicer IMO

[muxuelanKK]

@muxuelanKK is this affecting the restore at all? Curious if/how PV label selectors could have any effect when creating a PVC with VolumeSnapshot datasource
@akalenyu I think this is the only field that could potentially be problematic. What do you think?

This

kubevirt/pkg/storage/snapshot/restore.go

Line 1041 in 817c3ad

pvc.Spec.VolumeName = ""

is a bit clunky too, alongside datasource&datasourceref.
Maybe we could just set them all explicitly without copying, that would be nicer IMO

Yes, hold on

[muxuelanKK]

@muxuelanKK is this affecting the restore at all? Curious if/how PV label selectors could have any effect when creating a PVC with VolumeSnapshot datasource
@akalenyu I think this is the only field that could potentially be problematic. What do you think?

This

kubevirt/pkg/storage/snapshot/restore.go

Line 1041 in 817c3ad

pvc.Spec.VolumeName = ""

is a bit clunky too, alongside datasource&datasourceref.
Maybe we could just set them all explicitly without copying, that would be nicer IMO

Yes, hold on

done

[akalenyu]

/lgtm
/ok-to-test

[akalenyu]

one note - may want to add a test in pkg/storage/snapshot/restore_test.go to make sure we don't regress over this in the future

[mhenriks]

Given this new pattern of explicitly setting the fields we care about I think it makes sense to set dataSource and dataSourceRef in in this block as well

[mhenriks]

@muxuelanKK you planning to come back to this?