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

[release-v1.56] Pass DV labels to PVC #2646

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 3 additions & 16 deletions pkg/controller/datavolume/controller-base.go
Original file line number Diff line number Diff line change
Expand Up @@ -982,21 +982,6 @@ func buildHTTPClient() *http.Client {
return httpClient
}

func passDataVolumeInstancetypeLabelstoPVC(dataVolumeLabels, pvcLabels map[string]string) map[string]string {
instancetypeLabels := []string{
cc.LabelDefaultInstancetype,
cc.LabelDefaultInstancetypeKind,
cc.LabelDefaultPreference,
cc.LabelDefaultPreferenceKind,
}
for _, label := range instancetypeLabels {
if dvLabel, hasLabel := dataVolumeLabels[label]; hasLabel {
pvcLabels[label] = dvLabel
}
}
return pvcLabels
}

// newPersistentVolumeClaim creates a new PVC for the DataVolume resource.
// It also sets the appropriate OwnerReferences on the resource
// which allows handleObject to discover the DataVolume resource
Expand All @@ -1008,7 +993,9 @@ func (r *ReconcilerBase) newPersistentVolumeClaim(dataVolume *cdiv1.DataVolume,
if util.ResolveVolumeMode(targetPvcSpec.VolumeMode) == corev1.PersistentVolumeFilesystem {
labels[common.KubePersistentVolumeFillingUpSuppressLabelKey] = common.KubePersistentVolumeFillingUpSuppressLabelValue
}
labels = passDataVolumeInstancetypeLabelstoPVC(dataVolume.GetLabels(), labels)
for k, v := range dataVolume.Labels {
labels[k] = v
}

annotations := make(map[string]string)
for k, v := range dataVolume.ObjectMeta.Annotations {
Expand Down
5 changes: 4 additions & 1 deletion pkg/controller/datavolume/import-controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,14 +464,16 @@ var _ = Describe("All DataVolume Tests", func() {
Expect(pvc.Spec.Resources.Requests.Storage().Value()).To(Equal(expectedSize.Value()))
})

It("Should pass annotation from DV to created a PVC on a DV", func() {
It("Should pass annotations and labels from DV to created PVC", func() {
dv := NewImportDataVolume("test-dv")
dv.SetAnnotations(make(map[string]string))
dv.GetAnnotations()["test-ann-1"] = "test-value-1"
dv.GetAnnotations()["test-ann-2"] = "test-value-2"
dv.GetAnnotations()[AnnSource] = "invalid phase should not copy"
dv.GetAnnotations()[AnnPodNetwork] = "data-network"
dv.GetAnnotations()[AnnPodSidecarInjection] = "false"
dv.SetLabels(make(map[string]string))
dv.GetLabels()["test"] = "test-label"
reconciler = createImportReconciler(dv)
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
Expand All @@ -486,6 +488,7 @@ var _ = Describe("All DataVolume Tests", func() {
Expect(pvc.GetAnnotations()[AnnPodNetwork]).To(Equal("data-network"))
Expect(pvc.GetAnnotations()[AnnPodSidecarInjection]).To(Equal("false"))
Expect(pvc.GetAnnotations()[AnnPriorityClassName]).To(Equal("p0"))
Expect(pvc.Labels["test"]).To(Equal("test-label"))
})

It("Should pass annotation from DV with S3 source to created a PVC on a DV", func() {
Expand Down
34 changes: 22 additions & 12 deletions pkg/controller/datavolume/smart-clone-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ func (r *SmartCloneReconciler) reconcileSnapshot(log logr.Logger, snapshot *snap
if err != nil {
return reconcile.Result{}, err
}
newPvc, err := newPvcFromSnapshot(snapshot.Name, snapshot, targetPvcSpec)

newPvc, err := newPvcFromSnapshot(dataVolume, snapshot.Name, snapshot, targetPvcSpec)
if err != nil {
return reconcile.Result{}, err
}
Expand Down Expand Up @@ -352,7 +353,7 @@ func (r *SmartCloneReconciler) getTargetPVC(dataVolume *cdiv1.DataVolume) (*core
return pvc, nil
}

func newPvcFromSnapshot(name string, snapshot *snapshotv1.VolumeSnapshot, targetPvcSpec *corev1.PersistentVolumeClaimSpec) (*corev1.PersistentVolumeClaim, error) {
func newPvcFromSnapshot(dv *cdiv1.DataVolume, name string, snapshot *snapshotv1.VolumeSnapshot, targetPvcSpec *corev1.PersistentVolumeClaimSpec) (*corev1.PersistentVolumeClaim, error) {
targetPvcSpecCopy := targetPvcSpec.DeepCopy()
restoreSize := snapshot.Status.RestoreSize
if restoreSize == nil {
Expand All @@ -369,22 +370,31 @@ func newPvcFromSnapshot(name string, snapshot *snapshotv1.VolumeSnapshot, target
common.CDILabelKey: common.CDILabelValue,
common.CDIComponentLabel: common.SmartClonerCDILabel,
}
for k, v := range dv.Labels {
labels[k] = v
}

annotations := map[string]string{
AnnSmartCloneRequest: "true",
cc.AnnRunningCondition: string(corev1.ConditionFalse),
cc.AnnRunningConditionMessage: cc.CloneComplete,
cc.AnnRunningConditionReason: "Completed",
annSmartCloneSnapshot: key,
}
for k, v := range dv.ObjectMeta.Annotations {
annotations[k] = v
}

if util.ResolveVolumeMode(targetPvcSpecCopy.VolumeMode) == corev1.PersistentVolumeFilesystem {
labels[common.KubePersistentVolumeFillingUpSuppressLabelKey] = common.KubePersistentVolumeFillingUpSuppressLabelValue
}

target := &corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: snapshot.Namespace,
Labels: labels,
Annotations: map[string]string{
AnnSmartCloneRequest: "true",
cc.AnnRunningCondition: string(corev1.ConditionFalse),
cc.AnnRunningConditionMessage: cc.CloneComplete,
cc.AnnRunningConditionReason: "Completed",
annSmartCloneSnapshot: key,
},
Name: name,
Namespace: snapshot.Namespace,
Labels: labels,
Annotations: annotations,
},
Spec: corev1.PersistentVolumeClaimSpec{
DataSource: &corev1.TypedLocalObjectReference{
Expand Down
9 changes: 6 additions & 3 deletions pkg/controller/datavolume/smart-clone-controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,10 @@ var _ = Describe("All smart clone tests", func() {
It("Should create PVC if snapshot ready", func() {
dv := newCloneDataVolume("test-dv")
q, _ := resource.ParseQuantity("500Mi")
// Set annotation on DV which we can verify on PVC later
dv.GetAnnotations()["test"] = "test-value"
// Set annotation and label on DV which we can verify on PVC later
dv.Annotations["test"] = "test-value"
dv.Labels = map[string]string{"test": "test-label"}

snapshot := createSnapshotVolume(dv.Name, dv.Namespace, nil)
snapshot.Spec.Source = snapshotv1.VolumeSnapshotSource{
PersistentVolumeClaimName: &[]string{"source"}[0],
Expand All @@ -234,8 +236,9 @@ var _ = Describe("All smart clone tests", func() {
Expect(err).ToNot(HaveOccurred())
Expect(pvc.Labels[common.AppKubernetesVersionLabel]).To(Equal("v0.0.0-tests"))
Expect(pvc.Labels[common.KubePersistentVolumeFillingUpSuppressLabelKey]).To(Equal(common.KubePersistentVolumeFillingUpSuppressLabelValue))
Expect(pvc.Labels["test"]).To(Equal("test-label"))
// Verify PVC's annotation
Expect(pvc.GetAnnotations()["test"]).To(Equal("test-value"))
Expect(pvc.Annotations["test"]).To(Equal("test-value"))
event := <-reconciler.recorder.(*record.FakeRecorder).Events
Expect(event).To(ContainSubstring("Creating PVC for smart-clone is in progress"))
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/datavolume/snapshot-clone-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ func (r *SnapshotCloneReconciler) getStorageClassCorrespondingToSnapClass(driver
}

func (r *SnapshotCloneReconciler) makePvcFromSnapshot(pvcName string, dv *cdiv1.DataVolume, snapshot *snapshotv1.VolumeSnapshot, targetPvcSpec *corev1.PersistentVolumeClaimSpec) (*corev1.PersistentVolumeClaim, error) {
newPvc, err := newPvcFromSnapshot(pvcName, snapshot, targetPvcSpec)
newPvc, err := newPvcFromSnapshot(dv, pvcName, snapshot, targetPvcSpec)
if err != nil {
return nil, err
}
Expand Down
20 changes: 20 additions & 0 deletions tests/cloner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,11 +281,13 @@ var _ = Describe("all clone tests", func() {
DescribeTable("[test_id:1355]Should clone data across different namespaces", func(targetSize string) {
pvcDef := utils.NewPVCDefinition(sourcePVCName, "1Gi", nil, nil)
pvcDef.Namespace = f.Namespace.Name

sourcePvc = f.CreateAndPopulateSourcePVC(pvcDef, sourcePodFillerName, fillCommand+testFile+"; chmod 660 "+testBaseDir+testFile)
targetNs, err := f.CreateNamespace(f.NsPrefix, map[string]string{
framework.NsPrefixLabel: f.NsPrefix,
})
Expect(err).NotTo(HaveOccurred())

f.AddNamespaceToDelete(targetNs)
doFileBasedCloneTest(f, pvcDef, targetNs, "target-dv", targetSize)
},
Expand Down Expand Up @@ -2653,6 +2655,8 @@ var _ = Describe("all clone tests", func() {

for i = 0; i < repeat; i++ {
dataVolume := utils.NewDataVolumeForSnapshotCloningAndStorageSpec(fmt.Sprintf("clone-from-snap-%d", i), size, snapshot.Namespace, snapshot.Name, nil, &volumeMode)
dataVolume.Labels = map[string]string{"test-label-1": "test-label-value-1"}
dataVolume.Annotations = map[string]string{"test-annotation-1": "test-annotation-value-1"}
By(fmt.Sprintf("Create new datavolume %s which will clone from volumesnapshot", dataVolume.Name))
dataVolume, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, targetNs.Name, dataVolume)
Expect(err).ToNot(HaveOccurred())
Expand All @@ -2670,6 +2674,9 @@ var _ = Describe("all clone tests", func() {
Expect(ok).To(BeFalse())
Expect(pvc.Spec.DataSource.Kind).To(Equal("VolumeSnapshot"))
Expect(pvc.Spec.DataSourceRef.Kind).To(Equal("VolumeSnapshot"))
// All labels and annotations passed
Expect(pvc.Labels["test-label-1"]).To(Equal("test-label-value-1"))
Expect(pvc.Annotations["test-annotation-1"]).To(Equal("test-annotation-value-1"))
}

By("Verify MD5 on one of the DVs")
Expand Down Expand Up @@ -2895,6 +2902,15 @@ func doFileBasedCloneTest(f *framework.Framework, srcPVCDef *v1.PersistentVolume
}
// Create targetPvc in new NS.
targetDV := utils.NewCloningDataVolume(targetDv, targetSize[0], srcPVCDef)
if targetDV.GetLabels() == nil {
targetDV.SetLabels(make(map[string]string))
}
if targetDV.GetAnnotations() == nil {
targetDV.SetAnnotations(make(map[string]string))
}
targetDV.Labels["test-label-1"] = "test-label-key-1"
targetDV.Annotations["test-annotation-1"] = "test-annotation-key-1"

dataVolume, err := utils.CreateDataVolumeFromDefinition(f.CdiClient, targetNs.Name, targetDV)
Expect(err).ToNot(HaveOccurred())

Expand All @@ -2915,6 +2931,10 @@ func doFileBasedCloneTest(f *framework.Framework, srcPVCDef *v1.PersistentVolume

es := resource.MustParse(targetSize[0])
Expect(es.Cmp(*targetPvc.Status.Capacity.Storage()) <= 0).To(BeTrue())

// All labels and annotations passed
Expect(targetPvc.Labels["test-label-1"]).To(Equal("test-label-key-1"))
Expect(targetPvc.Annotations["test-annotation-1"]).To(Equal("test-annotation-key-1"))
}

func doInUseCloneTest(f *framework.Framework, srcPVCDef *v1.PersistentVolumeClaim, targetNs *v1.Namespace, targetDv string) {
Expand Down
25 changes: 25 additions & 0 deletions tests/datavolume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1134,6 +1134,31 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
table.Entry("[test_id:8045]for clone DataVolume", createCloneDataVolume, fillCommand),
)

table.DescribeTable("Should pass all DV labels and annotations to the PVC", func(dvFunc func(string, string, string) *cdiv1.DataVolume, url string) {
dataVolume := dvFunc("pass-all-labels-dv", "1Gi", url)
dataVolume.Labels = map[string]string{"test-label-1": "test-label-1", "test-label-2": "test-label-2"}
dataVolume.Annotations = map[string]string{"test-annotation-1": "test-annotation-1", "test-annotation-2": "test-annotation-2"}

By(fmt.Sprintf("creating new datavolume %s", dataVolume.Name))
dataVolume, err := utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dataVolume)
Expect(err).ToNot(HaveOccurred())

// verify PVC was created
By("verifying pvc was created and is Bound")
pvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name)
Expect(err).ToNot(HaveOccurred())

// All labels and annotations passed
Expect(pvc.Labels["test-label-1"]).To(Equal("test-label-1"))
Expect(pvc.Labels["test-label-2"]).To(Equal("test-label-2"))
Expect(pvc.Annotations["test-annotation-1"]).To(Equal("test-annotation-1"))
Expect(pvc.Annotations["test-annotation-2"]).To(Equal("test-annotation-2"))
},
table.Entry("for import DataVolume", utils.NewDataVolumeWithHTTPImport, tinyCoreIsoURL()),
table.Entry("for upload DataVolume", createUploadDataVolume, tinyCoreIsoURL()),
table.Entry("for clone DataVolume", createCloneDataVolume, fillCommand),
)

It("Should handle a pre populated DV", func() {
By(fmt.Sprintf("initializing dataVolume marked as prePopulated %s", dataVolumeName))
dataVolume := utils.NewDataVolumeWithHTTPImport(dataVolumeName, "1Gi", cirrosURL())
Expand Down