Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When clone dv from an existing pvc use smart clone, some ann of dv can't be inherited by the new pvc. #2391

Closed
Longchuanzheng opened this issue Aug 15, 2022 · 2 comments · Fixed by #2397
Labels

Comments

@Longchuanzheng
Copy link
Contributor

Longchuanzheng commented Aug 15, 2022

/kind bug

Is your feature request related to a problem? Please describe:
When clone dv from an existing pvc use smart clone, some ann of dv can't be inherited by the new pvc.

Describe the solution you'd like:
Some new anns be added to the new dv should be added to the new pvc, when using smart clone.

Describe alternatives you've considered:
add some ann in the yaml used to create the new dv from an existing pvc.

kubectl create -f

the new pvc relate to the new dv doesn't have these anns.

Additional context:
maybe in some cases, customs want to do this, I add some code in /pkg/controller/smart-clone-controller.go

	if err := setAnnOwnedByDataVolume(newPvc, dataVolume); err != nil {
		return reconcile.Result{}, err
	}
	if len(dataVolume.GetAnnotations()) > 0 {
		for k, v := range dataVolume.GetAnnotations() {
			if !strings.Contains(k, common.CDIAnnKey) {
				newPvc.Annotations[k] = v
			}
		}
	}
	if snapshot.Spec.Source.PersistentVolumeClaimName != nil {

these code works, but may have some pontional problems.

Add any other context or screenshots about the feature request here.

@awels
Copy link
Member

awels commented Aug 15, 2022

Hi, thanks for the report, you are absolutely right this is a bug. We support passing annotations on all other kinds of clones (host assisted/csi clone) and there is no reason we shouldn't allow this on a smart clone as well. What you are suggesting seems reasonable for passing annotations from the target DV to the matching target PVC. If you have a PR already feel free to submit it. I might suggest adding a functional test (or modifying a functional test to include annotations)

@Longchuanzheng
Copy link
Contributor Author

Hi, thanks for the report, you are absolutely right this is a bug. We support passing annotations on all other kinds of clones (host assisted/csi clone) and there is no reason we shouldn't allow this on a smart clone as well. What you are suggesting seems reasonable for passing annotations from the target DV to the matching target PVC. If you have a PR already feel free to submit it. I might suggest adding a functional test (or modifying a functional test to include annotations)

Thank you very much for your timely response,I have create a PR and try to add a 'DCO signoff' to my commits and etc. Although my code works, but I think these code may not meet the logic of reconcile and need to improve. Meanwhile, I need sometime to adding a functional test.

kubevirt-bot pushed a commit that referenced this issue Aug 25, 2022
#2397)

* fix issue #2391 When clone dv from an existing pvc use smart clone, some ann of dv can't be inherited by the new pvc.

Signed-off-by: zhuanlan <zhuanlan_yewu@cmss.chinamobile.com>

* fix issue #2391 modify a functional test to verify the annotations are properly passed to the PVC

Signed-off-by: zhuanlan <zhuanlan_yewu@cmss.chinamobile.com>

Signed-off-by: zhuanlan <zhuanlan_yewu@cmss.chinamobile.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants