Skip to content

Commit

Permalink
More fixes and UT
Browse files Browse the repository at this point in the history 
Signed-off-by: Shelly Kagan <skagan@redhat.com>
  • Loading branch information
@ShellyKa13
ShellyKa13 committed Jun 6, 2023
1 parent 7a8ef69 commit 7776b9d
Show file tree
Hide file tree
Showing 4 changed files with 473 additions and 15 deletions.
28 changes: 17 additions & 11 deletions pkg/controller/datavolume/import-controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,13 @@ func addDataVolumeImportControllerWatches(mgr manager.Manager, datavolumeControl
}

func (r *ImportReconciler) updatePVCForPopulation(dataVolume *cdiv1.DataVolume, pvc *corev1.PersistentVolumeClaim) error {
if dataVolume.Spec.Source.HTTP != nil &&
dataVolume.Spec.Source.S3 != nil &&
dataVolume.Spec.Source.GCS != nil &&
dataVolume.Spec.Source.Registry != nil &&
dataVolume.Spec.Source.Imageio != nil &&
dataVolume.Spec.Source.VDDK != nil &&
dataVolume.Spec.Source.Blank != nil {
if dataVolume.Spec.Source.HTTP == nil &&
dataVolume.Spec.Source.S3 == nil &&
dataVolume.Spec.Source.GCS == nil &&
dataVolume.Spec.Source.Registry == nil &&
dataVolume.Spec.Source.Imageio == nil &&
dataVolume.Spec.Source.VDDK == nil &&
dataVolume.Spec.Source.Blank == nil {
return errors.Errorf("no source set for import datavolume")
}
apiGroup := cc.AnnAPIGroup
Expand Down Expand Up @@ -201,9 +201,11 @@ func (r *ImportReconciler) syncImport(log logr.Logger, req reconcile.Request) (d

pvcModifier := r.updateAnnotations
if syncState.usePopulator {
err := r.createVolumeImportSourceCR(&syncState)
if err != nil {
return syncState, err
if syncState.dvMutated.Status.Phase != cdiv1.Succeeded {
err := r.createVolumeImportSourceCR(&syncState)
if err != nil {
return syncState, err
}
}
pvcModifier = r.updatePVCForPopulation
}
Expand All @@ -224,7 +226,11 @@ func (r *ImportReconciler) cleanup(syncState *dvSyncState) error {
// The cleanup is to delete the volumeImportSourceCR which is used only with populators,
// it is owner by the DV so will be deleted when dv is deleted
// also we can already delete once dv is succeeded
if syncState.usePopulator && dv.Status.Phase == cdiv1.Succeeded {
usePopulator, err := checkDVUsingPopulators(syncState.dvMutated)
if err != nil {
return err
}
if usePopulator && dv.Status.Phase == cdiv1.Succeeded {
return r.deleteVolumeImportSourceCR(syncState)
}

Expand Down
243 changes: 243 additions & 0 deletions pkg/controller/datavolume/import-controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
corev1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/errors"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -86,6 +87,73 @@ var _ = Describe("All DataVolume Tests", func() {
}
})

It("Should create volumeImportSource if should use cdi populator", func() {
scName := "testSC"
sc := CreateStorageClassWithProvisioner(scName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, "csi-plugin")
csiDriver := &storagev1.CSIDriver{
ObjectMeta: metav1.ObjectMeta{
Name: "csi-plugin",
},
}
dv := NewImportDataVolume("test-dv")
dv.Spec.ContentType = cdiv1.DataVolumeArchive
preallocation := true
dv.Spec.Preallocation = &preallocation
reconciler = createImportReconciler(dv, sc, csiDriver)
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv)
Expect(err).ToNot(HaveOccurred())
Expect(dv.GetAnnotations()[AnnUsePopulator]).To(Equal("true"))

importSource := &cdiv1.VolumeImportSource{}
importSourceName := volumeImportSourceName(dv)
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: importSourceName, Namespace: metav1.NamespaceDefault}, importSource)
Expect(err).ToNot(HaveOccurred())
Expect(importSource.Spec.Source).ToNot(BeNil())
Expect(importSource.Spec.ContentType).To(Equal(dv.Spec.ContentType))
Expect(importSource.Spec.Preallocation).To(Equal(dv.Spec.Preallocation))
Expect(importSource.OwnerReferences).To(HaveLen(1))
or := importSource.OwnerReferences[0]
Expect(or.UID).To(Equal(dv.UID))
})

It("Should delete volumeImportSource if dv succeeded and we use cdi populator", func() {
scName := "testSC"
sc := CreateStorageClassWithProvisioner(scName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, "csi-plugin")
csiDriver := &storagev1.CSIDriver{
ObjectMeta: metav1.ObjectMeta{
Name: "csi-plugin",
},
}
dv := NewImportDataVolume("test-dv")
importSourceName := volumeImportSourceName(dv)
importSource := &cdiv1.VolumeImportSource{
ObjectMeta: metav1.ObjectMeta{
Name: importSourceName,
Namespace: dv.Namespace,
},
}
reconciler = createImportReconciler(dv, sc, csiDriver, importSource)
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv)
Expect(err).ToNot(HaveOccurred())
Expect(dv.GetAnnotations()[AnnUsePopulator]).To(Equal("true"))

dv.Status.Phase = cdiv1.Succeeded
err = reconciler.client.Update(context.TODO(), dv)
Expect(err).ToNot(HaveOccurred())

_, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())

deletedImportSource := &cdiv1.VolumeImportSource{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: importSourceName, Namespace: dv.Namespace}, deletedImportSource)
Expect(err).To(HaveOccurred())
Expect(errors.IsNotFound(err)).To(BeTrue())
})

It("Should create a PVC on a valid import DV", func() {
reconciler = createImportReconciler(NewImportDataVolume("test-dv"))
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expand All @@ -98,6 +166,86 @@ var _ = Describe("All DataVolume Tests", func() {
Expect(pvc.Labels[common.KubePersistentVolumeFillingUpSuppressLabelKey]).To(Equal(common.KubePersistentVolumeFillingUpSuppressLabelValue))
})

It("Should fail if dv source not import when use populators", func() {
scName := "testSC"
sc := CreateStorageClassWithProvisioner(scName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, "csi-plugin")
csiDriver := &storagev1.CSIDriver{
ObjectMeta: metav1.ObjectMeta{
Name: "csi-plugin",
},
}
dv := NewImportDataVolume("test-dv")
dv.Spec.Source.HTTP = nil
reconciler = createImportReconciler(dv, sc, csiDriver)
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("no source set for import datavolume"))
pvc := &corev1.PersistentVolumeClaim{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc)
Expect(err).To(HaveOccurred())
Expect(errors.IsNotFound(err)).To(BeTrue())
})

It("Should create a PVC with volumeImportSource when use populators", func() {
scName := "testSC"
sc := CreateStorageClassWithProvisioner(scName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, "csi-plugin")
csiDriver := &storagev1.CSIDriver{
ObjectMeta: metav1.ObjectMeta{
Name: "csi-plugin",
},
}
dv := NewImportDataVolume("test-dv")
reconciler = createImportReconciler(dv, sc, csiDriver)
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
pvc := &corev1.PersistentVolumeClaim{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc)
Expect(err).ToNot(HaveOccurred())
Expect(pvc.Name).To(Equal("test-dv"))
Expect(pvc.Labels[common.AppKubernetesPartOfLabel]).To(Equal("testing"))
Expect(pvc.Labels[common.KubePersistentVolumeFillingUpSuppressLabelKey]).To(Equal(common.KubePersistentVolumeFillingUpSuppressLabelValue))
Expect(pvc.Spec.DataSourceRef).ToNot(BeNil())
importSourceName := volumeImportSourceName(dv)
Expect(pvc.Spec.DataSourceRef.Name).To(Equal(importSourceName))
Expect(pvc.Spec.DataSourceRef.Kind).To(Equal(cdiv1.VolumeImportSourceRef))
Expect(pvc.GetAnnotations()[AnnUsePopulator]).To(Equal("true"))
})

It("Should report import population progress when use populators", func() {
scName := "testSC"
sc := CreateStorageClassWithProvisioner(scName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, "csi-plugin")
csiDriver := &storagev1.CSIDriver{
ObjectMeta: metav1.ObjectMeta{
Name: "csi-plugin",
},
}
dv := NewImportDataVolume("test-dv")
reconciler = createImportReconciler(dv, sc, csiDriver)
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
dv = &cdiv1.DataVolume{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv)
Expect(err).ToNot(HaveOccurred())
Expect(string(dv.Status.Progress)).To(Equal("N/A"))

pvc := &corev1.PersistentVolumeClaim{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc)
Expect(err).ToNot(HaveOccurred())
Expect(pvc.GetAnnotations()[AnnUsePopulator]).To(Equal("true"))

AddAnnotation(pvc, AnnPopulatorProgress, "13.45%")
err = reconciler.client.Update(context.TODO(), pvc)
Expect(err).ToNot(HaveOccurred())

_, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())

dv = &cdiv1.DataVolume{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv)
Expect(err).ToNot(HaveOccurred())
Expect(dv.Status.Progress).To(BeEquivalentTo("13.45%"))
})

It("Should pass instancetype labels from DV to PVC", func() {
dv := NewImportDataVolume("test-dv")
dv.Labels = map[string]string{}
Expand Down Expand Up @@ -968,6 +1116,101 @@ var _ = Describe("All DataVolume Tests", func() {
}
Expect(found).To(BeTrue())
})
It("Should set DV phase to pendingPopulation if use populators with storage class WFFC", func() {
scName := "pvc_sc_wffc"
bindingMode := storagev1.VolumeBindingWaitForFirstConsumer
sc := CreateStorageClassWithProvisioner(scName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, "csi-plugin")
sc.VolumeBindingMode = &bindingMode
csiDriver := &storagev1.CSIDriver{
ObjectMeta: metav1.ObjectMeta{
Name: "csi-plugin",
},
}
importDataVolume := NewImportDataVolume("test-dv")
importDataVolume.Spec.PVC.StorageClassName = &scName

reconciler = createImportReconciler(sc, csiDriver, importDataVolume)
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())
dv := &cdiv1.DataVolume{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv)
Expect(err).ToNot(HaveOccurred())

pvc := &corev1.PersistentVolumeClaim{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc)
Expect(err).ToNot(HaveOccurred())
Expect(pvc.Name).To(Equal("test-dv"))
pvc.Status.Phase = corev1.ClaimPending
err = reconciler.client.Update(context.TODO(), pvc)
Expect(err).ToNot(HaveOccurred())
_, err = reconciler.updateStatus(getReconcileRequest(dv), nil, reconciler)
Expect(err).ToNot(HaveOccurred())
dv = &cdiv1.DataVolume{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv)
Expect(err).ToNot(HaveOccurred())
Expect(dv.Status.Phase).To(Equal(cdiv1.PendingPopulation))

Expect(dv.Status.Conditions).To(HaveLen(3))
boundCondition := FindConditionByType(cdiv1.DataVolumeBound, dv.Status.Conditions)
Expect(boundCondition.Status).To(Equal(corev1.ConditionFalse))
Expect(boundCondition.Message).To(Equal("PVC test-dv Pending"))
By("Checking events recorded")
close(reconciler.recorder.(*record.FakeRecorder).Events)
found := false
for event := range reconciler.recorder.(*record.FakeRecorder).Events {
if strings.Contains(event, "PVC test-dv Pending") {
found = true
}
}
Expect(found).To(BeTrue())
})

It("Should set DV phase to ImportScheduled if use populators wffc storage class after scheduled node", func() {
scName := "pvc_sc_wffc"
bindingMode := storagev1.VolumeBindingWaitForFirstConsumer
sc := CreateStorageClassWithProvisioner(scName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, "csi-plugin")
sc.VolumeBindingMode = &bindingMode
csiDriver := &storagev1.CSIDriver{
ObjectMeta: metav1.ObjectMeta{
Name: "csi-plugin",
},
}
importDataVolume := NewImportDataVolume("test-dv")
importDataVolume.Spec.PVC.StorageClassName = &scName

reconciler = createImportReconciler(sc, csiDriver, importDataVolume)
_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
Expect(err).ToNot(HaveOccurred())

pvc := &corev1.PersistentVolumeClaim{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc)
Expect(err).ToNot(HaveOccurred())
Expect(pvc.Name).To(Equal("test-dv"))
pvc.Status.Phase = corev1.ClaimPending
AddAnnotation(pvc, AnnSelectedNode, "node01")
err = reconciler.client.Update(context.TODO(), pvc)
Expect(err).ToNot(HaveOccurred())
_, err = reconciler.updateStatus(getReconcileRequest(importDataVolume), nil, reconciler)
Expect(err).ToNot(HaveOccurred())
dv := &cdiv1.DataVolume{}
err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv)
Expect(err).ToNot(HaveOccurred())
Expect(dv.Status.Phase).To(Equal(cdiv1.ImportScheduled))

Expect(dv.Status.Conditions).To(HaveLen(3))
boundCondition := FindConditionByType(cdiv1.DataVolumeBound, dv.Status.Conditions)
Expect(boundCondition.Status).To(Equal(corev1.ConditionFalse))
Expect(boundCondition.Message).To(Equal("PVC test-dv Pending"))
By("Checking events recorded")
close(reconciler.recorder.(*record.FakeRecorder).Events)
found := false
for event := range reconciler.recorder.(*record.FakeRecorder).Events {
if strings.Contains(event, "PVC test-dv Pending") {
found = true
}
}
Expect(found).To(BeTrue())
})

It("Should switch to succeeded if PVC phase is pending, but pod phase is succeeded", func() {
reconciler = createImportReconciler(NewImportDataVolume("test-dv"))
Expand Down
14 changes: 10 additions & 4 deletions pkg/controller/datavolume/upload-controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,11 @@ func (r *UploadReconciler) syncUpload(log logr.Logger, req reconcile.Request) (d

pvcModifier := r.updateAnnotations
if syncState.usePopulator {
err := r.createVolumeUploadSourceCR(&syncState)
if err != nil {
return syncState, err
if syncState.dvMutated.Status.Phase != cdiv1.Succeeded {
err := r.createVolumeUploadSourceCR(&syncState)
if err != nil {
return syncState, err
}
}
pvcModifier = r.updatePVCForPopulation
}
Expand All @@ -175,7 +177,11 @@ func (r *UploadReconciler) cleanup(syncState *dvSyncState) error {
// The cleanup is to delete the volumeUploadSourceCR which is used only with populators,
// it is owner by the DV so will be deleted when dv is deleted
// also we can already delete once dv is succeeded
if syncState.usePopulator && dv.Status.Phase == cdiv1.Succeeded {
usePopulator, err := checkDVUsingPopulators(syncState.dvMutated)
if err != nil {
return err
}
if usePopulator && dv.Status.Phase == cdiv1.Succeeded {
return r.deleteVolumeUploadSourceCR(syncState)
}

Expand Down

0 comments on commit 7776b9d

Please sign in to comment.