Skip to content

Commit

Permalink
Adjust tests functions to handle dvs using populators
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 May 28, 2023
1 parent be77148 commit a98ea49
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 34 deletions.
4 changes: 2 additions & 2 deletions pkg/controller/datavolume/controller-base.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ func (r *ReconcilerBase) reconcileProgressUpdate(datavolume *cdiv1.DataVolume, p
datavolume.Status.Progress = "N/A"
}

if usePopulator, _ := checkPVCUsingPopulators(pvc); usePopulator {
if usePopulator, _ := CheckPVCUsingPopulators(pvc); usePopulator {
if progress, ok := pvc.Annotations[cc.AnnPopulatorProgress]; ok {
datavolume.Status.Progress = cdiv1.DataVolumeProgress(progress)
}
Expand Down Expand Up @@ -809,7 +809,7 @@ func (r ReconcilerBase) updateStatus(req reconcile.Request, phaseSync *statusPha
} else {
switch pvc.Status.Phase {
case corev1.ClaimPending:
usePopulator, err := checkPVCUsingPopulators(pvc)
usePopulator, err := CheckPVCUsingPopulators(pvc)
if err != nil {
return reconcile.Result{}, err
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/controller/datavolume/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,9 @@ func storageClassCSIDriverExists(client client.Client, log logr.Logger, storageC
return true, nil
}

func checkPVCUsingPopulators(pvc *v1.PersistentVolumeClaim) (bool, error) {
// CheckPVCUsingPopulators returns true if pvc has dataSourceRef and has
// the usePopulator annotation
func CheckPVCUsingPopulators(pvc *v1.PersistentVolumeClaim) (bool, error) {
if pvc.Spec.DataSourceRef == nil {
return false, nil
}
Expand Down
1 change: 1 addition & 0 deletions tests/api_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ var _ = Describe("[rfe_id:1130][crit:medium][posneg:negative][vendor:cnv-qe@redh
}
Expect(err).ToNot(HaveOccurred())
progressRegExp := regexp.MustCompile(`\d{1,3}\.?\d{1,2}%`)
By("Waiting for datavolume to indicate progress")
Eventually(func() bool {
dv, err := f.CdiClient.CdiV1beta1().DataVolumes(f.Namespace.Name).Get(context.TODO(), dataVolumeName, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expand Down
100 changes: 79 additions & 21 deletions tests/datavolume_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"kubevirt.io/containerized-data-importer/pkg/common"
controller "kubevirt.io/containerized-data-importer/pkg/controller/common"
dvc "kubevirt.io/containerized-data-importer/pkg/controller/datavolume"
"kubevirt.io/containerized-data-importer/pkg/controller/populators"
featuregates "kubevirt.io/containerized-data-importer/pkg/feature-gates"
"kubevirt.io/containerized-data-importer/pkg/util/naming"
"kubevirt.io/containerized-data-importer/tests/framework"
Expand Down Expand Up @@ -232,19 +233,20 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",

Describe("Verify DataVolume", func() {
type dataVolumeTestArguments struct {
name string
size string
url func() string
dvFunc func(string, string, string) *cdiv1.DataVolume
errorMessage string
errorMessageFunc func() string
eventReason string
phase cdiv1.DataVolumePhase
repeat int
checkPermissions bool
readyCondition *cdiv1.DataVolumeCondition
boundCondition *cdiv1.DataVolumeCondition
runningCondition *cdiv1.DataVolumeCondition
name string
size string
url func() string
dvFunc func(string, string, string) *cdiv1.DataVolume
errorMessage string
errorMessageFunc func() string
eventReason string
phase cdiv1.DataVolumePhase
repeat int
checkPermissions bool
readyCondition *cdiv1.DataVolumeCondition
boundCondition *cdiv1.DataVolumeCondition
boundConditionWithPopulators *cdiv1.DataVolumeCondition
runningCondition *cdiv1.DataVolumeCondition
}

createHTTPSDataVolume := func(dataVolumeName, size, url string) *cdiv1.DataVolume {
Expand Down Expand Up @@ -324,14 +326,20 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",

waitForDvPhase(args.phase, dataVolume, f)

By("Verifying the DV has the correct conditions and messages for those conditions")
utils.WaitForConditions(f, dataVolume.Name, f.Namespace.Name, timeout, pollingInterval, args.readyCondition, args.runningCondition, args.boundCondition)

// verify PVC was created
By("verifying pvc was created")
pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(dataVolume.Namespace).Get(context.TODO(), dataVolume.Name, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())

By("Verifying the DV has the correct conditions and messages for those conditions")
usePopulator, err := dvc.CheckPVCUsingPopulators(pvc)
Expect(err).ToNot(HaveOccurred())
if usePopulator && args.boundConditionWithPopulators != nil {
utils.WaitForConditions(f, dataVolume.Name, f.Namespace.Name, timeout, pollingInterval, args.readyCondition, args.runningCondition, args.boundConditionWithPopulators)
} else {
utils.WaitForConditions(f, dataVolume.Name, f.Namespace.Name, timeout, pollingInterval, args.readyCondition, args.runningCondition, args.boundCondition)
}

By("Verifying event occurred")
Eventually(func() bool {
// Only find DV events, we know the PVC gets the same events
Expand Down Expand Up @@ -404,20 +412,26 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
utils.WaitForConditions(f, dataVolume.Name, f.Namespace.Name, timeout, pollingInterval, boundCondition, readyCondition)

By("Increase quota")
err = f.UpdateStorageQuota(int64(2), int64(2*1024*1024*1024))
err = f.UpdateStorageQuota(int64(3), int64(4*1024*1024*1024))
Expect(err).ToNot(HaveOccurred())
f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume)

waitForDvPhase(args.phase, dataVolume, f)

By("Verifying the DV has the correct conditions and messages for those conditions")
utils.WaitForConditions(f, dataVolume.Name, f.Namespace.Name, timeout, pollingInterval, args.readyCondition, args.runningCondition, args.boundCondition)

// verify PVC was created
By("verifying pvc was created")
_, err = f.K8sClient.CoreV1().PersistentVolumeClaims(dataVolume.Namespace).Get(context.TODO(), dataVolume.Name, metav1.GetOptions{})
pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(dataVolume.Namespace).Get(context.TODO(), dataVolume.Name, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())

By("Verifying the DV has the correct conditions and messages for those conditions")
usePopulator, err := dvc.CheckPVCUsingPopulators(pvc)
Expect(err).ToNot(HaveOccurred())
if usePopulator && args.boundConditionWithPopulators != nil {
utils.WaitForConditions(f, dataVolume.Name, f.Namespace.Name, timeout, pollingInterval, args.readyCondition, args.runningCondition, args.boundConditionWithPopulators)
} else {
utils.WaitForConditions(f, dataVolume.Name, f.Namespace.Name, timeout, pollingInterval, args.readyCondition, args.runningCondition, args.boundCondition)
}

By("Verifying event occurred")
Eventually(func() bool {
// Only find DV events, we know the PVC gets the same events
Expand Down Expand Up @@ -485,6 +499,12 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
Message: "PVC dv-http-import-invalid-url Bound",
Reason: "Bound",
},
boundConditionWithPopulators: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeBound,
Status: v1.ConditionFalse,
Message: "PVC dv-http-import-invalid-url Pending",
Reason: "Pending",
},
runningCondition: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeRunning,
Status: v1.ConditionFalse,
Expand All @@ -509,6 +529,12 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
Message: "PVC dv-http-import-404 Bound",
Reason: "Bound",
},
boundConditionWithPopulators: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeBound,
Status: v1.ConditionFalse,
Message: "PVC dv-http-import-404 Pending",
Reason: "Pending",
},
runningCondition: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeRunning,
Status: v1.ConditionFalse,
Expand All @@ -533,6 +559,12 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
Message: "PVC dv-invalid-qcow-large-memory Bound",
Reason: "Bound",
},
boundConditionWithPopulators: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeBound,
Status: v1.ConditionFalse,
Message: "PVC dv-invalid-qcow-large-memory Pending",
Reason: "Pending",
},
runningCondition: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeRunning,
Status: v1.ConditionFalse,
Expand Down Expand Up @@ -749,6 +781,12 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
Message: "PVC upload-dv Bound",
Reason: "Bound",
},
boundConditionWithPopulators: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeBound,
Status: v1.ConditionFalse,
Message: "PVC upload-dv Pending",
Reason: "Pending",
},
runningCondition: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeRunning,
Status: v1.ConditionTrue,
Expand Down Expand Up @@ -795,6 +833,12 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
Message: "PVC dv-non-tar-archive Bound",
Reason: "Bound",
},
boundConditionWithPopulators: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeBound,
Status: v1.ConditionFalse,
Message: "PVC dv-non-tar-archive Pending",
Reason: "Pending",
},
runningCondition: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeRunning,
Status: v1.ConditionFalse,
Expand Down Expand Up @@ -1168,6 +1212,12 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
Message: "PVC upload-dv Bound",
Reason: "Bound",
},
boundConditionWithPopulators: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeBound,
Status: v1.ConditionFalse,
Message: "PVC upload-dv Pending",
Reason: "Pending",
},
runningCondition: &cdiv1.DataVolumeCondition{
Type: cdiv1.DataVolumeRunning,
Status: v1.ConditionTrue,
Expand Down Expand Up @@ -1502,7 +1552,15 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
By("verifying pvc and pod were created")
pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(dataVolume.Namespace).Get(context.TODO(), dataVolume.Name, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
usePopulator, err := dvc.CheckPVCUsingPopulators(pvc)
Expect(err).ToNot(HaveOccurred())
podName := pvc.Annotations[controller.AnnImportPod]
if usePopulator {
pvcPrimeName := populators.PVCPrimeName(pvc)
pvcPrime, err := f.K8sClient.CoreV1().PersistentVolumeClaims(dataVolume.Namespace).Get(context.TODO(), pvcPrimeName, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
podName = pvcPrime.Annotations[controller.AnnImportPod]
}

pod, err := f.K8sClient.CoreV1().Pods(f.Namespace.Name).Get(context.TODO(), podName, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expand Down
15 changes: 12 additions & 3 deletions tests/framework/pvc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,13 @@ func (f *Framework) ForceBindPvcIfDvIsWaitForFirstConsumer(dv *cdiv1.DataVolume)
pvc, err := utils.WaitForPVC(f.K8sClient, dv.Namespace, dv.Name)
gomega.Expect(err).ToNot(gomega.HaveOccurred(), "PVC should exist")
if f.IsBindingModeWaitForFirstConsumer(pvc.Spec.StorageClassName) {
err = utils.WaitForDataVolumePhase(f, dv.Namespace, cdiv1.WaitForFirstConsumer, dv.Name)
expectedPhase := cdiv1.WaitForFirstConsumer
if pvc.Spec.DataSourceRef != nil {
expectedPhase = cdiv1.PendingPopulation
}
err = utils.WaitForDataVolumePhase(f, dv.Namespace, expectedPhase, dv.Name)
gomega.Expect(err).ToNot(gomega.HaveOccurred())
createConsumerPod(pvc, f)
f.ForceBindIfWaitForFirstConsumer(pvc)
}
}

Expand All @@ -91,7 +95,11 @@ func (f *Framework) WaitPVCDeletedByUID(pvcSpec *k8sv1.PersistentVolumeClaim, ti
// ForceBindIfWaitForFirstConsumer creates a Pod with the passed in PVC mounted under /dev/pvc, which forces the PVC to be scheduled and bound.
func (f *Framework) ForceBindIfWaitForFirstConsumer(targetPvc *k8sv1.PersistentVolumeClaim) {
if targetPvc.Spec.VolumeName == "" && f.IsBindingModeWaitForFirstConsumer(targetPvc.Spec.StorageClassName) {
createConsumerPod(targetPvc, f)
if targetPvc.Spec.DataSourceRef != nil {
createConsumerPodForPopulationPVC(targetPvc, f)
} else {
createConsumerPod(targetPvc, f)
}
}
}

Expand Down Expand Up @@ -299,6 +307,7 @@ func (f *Framework) VerifyFSOverhead(namespace *k8sv1.Namespace, pvc *k8sv1.Pers
}

requestedSize := pvc.Spec.Resources.Requests[k8sv1.ResourceStorage]
fmt.Fprintf(ginkgo.GinkgoWriter, "INFO: VerifyFSOverhead comparison: Virtual: %d, Actual: %d, requestedSize: %d\n", info.VirtualSize, info.ActualSize, requestedSize.Value())
return info.VirtualSize <= info.ActualSize && info.VirtualSize < requestedSize.Value(), nil
}

Expand Down
22 changes: 18 additions & 4 deletions tests/import_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1244,9 +1244,17 @@ var _ = Describe("Preallocation", func() {
if dv.Spec.Source.Registry != nil && dv.Spec.Source.Registry.ImageStream != nil {
By("Verify image lookup annotation")
podName := pvc.Annotations[controller.AnnImportPod]
pod, err := f.K8sClient.CoreV1().Pods(f.Namespace.Name).Get(context.TODO(), podName, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(pod.Annotations[controller.AnnOpenShiftImageLookup]).To(Equal("*"))
if pvc.Spec.DataSourceRef != nil {
Expect(podName).To(BeEmpty())
} else {
// when using populators when the population completes PVC' and
// the importer pod are deleted, so can't check the annotation
// TODO: any suggestions? putting the check before dv completes is
// still racy
pod, err := f.K8sClient.CoreV1().Pods(f.Namespace.Name).Get(context.TODO(), podName, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(pod.Annotations[controller.AnnOpenShiftImageLookup]).To(Equal("*"))
}
}
},
Entry("HTTP import (ISO image)", true, utils.TinyCoreMD5, utils.DefaultImagePath, func() *cdiv1.DataVolume {
Expand Down Expand Up @@ -1376,7 +1384,13 @@ var _ = Describe("Preallocation", func() {
Expect(err).ToNot(HaveOccurred())
Expect(found).To(BeTrue())

Expect(f.VerifyFSOverhead(f.Namespace, pvc, preallocation)).To(BeTrue())
// incase of using populators the requested size with the fsoverhead
// is put only on the PVC' which at thisd point we can't check
// TODO: any suggestions? getting the requested size from PVC' in earlier
// point in the test seems to be racy
if pvc.Spec.DataSourceRef == nil {
Expect(f.VerifyFSOverhead(f.Namespace, pvc, preallocation)).To(BeTrue())
}

pvc, err = utils.FindPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name)
Expect(err).ToNot(HaveOccurred())
Expand Down
1 change: 1 addition & 0 deletions tests/utils/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ go_library(
"//pkg/common:go_default_library",
"//pkg/controller/common:go_default_library",
"//pkg/controller/datavolume:go_default_library",
"//pkg/controller/populators:go_default_library",
"//pkg/image:go_default_library",
"//pkg/util:go_default_library",
"//pkg/util/naming:go_default_library",
Expand Down
10 changes: 7 additions & 3 deletions tests/utils/upload.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package utils
import (
"context"

corev1 "k8s.io/api/core/v1"
k8sv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand All @@ -14,6 +13,7 @@ import (
cdiClientset "kubevirt.io/containerized-data-importer/pkg/client/clientset/versioned"
"kubevirt.io/containerized-data-importer/pkg/common"
cc "kubevirt.io/containerized-data-importer/pkg/controller/common"
"kubevirt.io/containerized-data-importer/pkg/controller/populators"
"kubevirt.io/containerized-data-importer/pkg/util/naming"
)

Expand Down Expand Up @@ -56,7 +56,11 @@ const (

// UploadPodName returns the name of the upload server pod associated with a PVC
func UploadPodName(pvc *k8sv1.PersistentVolumeClaim) string {
return naming.GetResourceName(common.UploadPodName, pvc.Name)
uploadPodNameSuffix := pvc.Name
if pvc.Spec.DataSourceRef != nil {
uploadPodNameSuffix = populators.PVCPrimeName(pvc)
}
return naming.GetResourceName(common.UploadPodName, uploadPodNameSuffix)
}

// UploadPVCDefinition creates a PVC with the upload target annotation
Expand Down Expand Up @@ -91,7 +95,7 @@ func UploadPopulationPVCDefinition() *k8sv1.PersistentVolumeClaim {
func UploadPopulationBlockPVCDefinition(storageClassName string) *k8sv1.PersistentVolumeClaim {
pvcDef := UploadPopulationPVCDefinition()
pvcDef.Spec.StorageClassName = &storageClassName
volumeMode := corev1.PersistentVolumeBlock
volumeMode := k8sv1.PersistentVolumeBlock
pvcDef.Spec.VolumeMode = &volumeMode
return pvcDef
}
Expand Down

0 comments on commit a98ea49

Please sign in to comment.