Skip to content

Commit

Permalink
Adjust functional tests 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 committed May 31, 2023
1 parent ee95951 commit 2a16983
Show file tree
Hide file tree
Showing 10 changed files with 151 additions and 48 deletions.
6 changes: 4 additions & 2 deletions pkg/controller/datavolume/controller-base.go
Original file line number Diff line number Diff line change
Expand Up @@ -687,9 +687,11 @@ 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)
} else {
datavolume.Status.Progress = "N/A"
}
return nil
}
Expand Down Expand Up @@ -812,7 +814,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
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,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
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
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
5 changes: 4 additions & 1 deletion tests/external_population_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,14 @@ var _ = Describe("Population tests", func() {
By("Verifying pvc was created")
pvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name)
Expect(err).ToNot(HaveOccurred())
f.ForceBindIfWaitForFirstConsumer(pvc)
executorPod := f.CreateConsumerPod(pvc)
// We check the expected event
By("Wait for expected no cdi driver event")
f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(dvc.NoCSIDriverForExternalPopulation))

By("Cleaning up")
err = utils.DeletePodNoGrace(f.K8sClient, executorPod, dataVolume.Namespace)
Expect(err).ToNot(HaveOccurred())
err = utils.DeleteDataVolume(f.CdiClient, f.Namespace.Name, dataVolume.Name)
Expect(err).ToNot(HaveOccurred())
Eventually(func() bool {
Expand Down
44 changes: 29 additions & 15 deletions tests/framework/pvc.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,16 @@ 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)
gomega.Expect(err).ToNot(gomega.HaveOccurred())
createConsumerPod(pvc, f)
if pvc.Spec.DataSourceRef != nil &&
dv.Spec.Source.PVC == nil && dv.Spec.Source.Snapshot == nil {
err = utils.WaitForDataVolumePhase(f, dv.Namespace, cdiv1.PendingPopulation, dv.Name)
gomega.Expect(err).ToNot(gomega.HaveOccurred())
createConsumerPodForPopulationPVC(pvc, f)
} else {
err = utils.WaitForDataVolumePhase(f, dv.Namespace, cdiv1.WaitForFirstConsumer, dv.Name)
gomega.Expect(err).ToNot(gomega.HaveOccurred())
createConsumerPod(pvc, f)
}
}
}

Expand All @@ -91,7 +98,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 All @@ -102,8 +113,8 @@ func (f *Framework) ForceSchedulingIfWaitForFirstConsumerPopulationPVC(targetPvc
}
}

func createConsumerPod(targetPvc *k8sv1.PersistentVolumeClaim, f *Framework) {
fmt.Fprintf(ginkgo.GinkgoWriter, "INFO: creating \"consumer-pod\" to force binding PVC: %s\n", targetPvc.Name)
// CreateConsumerPod create a pod that consumes the given PVC
func (f *Framework) CreateConsumerPod(targetPvc *k8sv1.PersistentVolumeClaim) *k8sv1.Pod {
namespace := targetPvc.Namespace

err := utils.WaitForPersistentVolumeClaimPhase(f.K8sClient, targetPvc.Namespace, k8sv1.ClaimPending, targetPvc.Name)
Expand All @@ -112,7 +123,15 @@ func createConsumerPod(targetPvc *k8sv1.PersistentVolumeClaim, f *Framework) {
podName := naming.GetResourceName("consumer-pod", targetPvc.Name)
executorPod, err := f.CreateNoopPodWithPVC(podName, namespace, targetPvc)
gomega.Expect(err).ToNot(gomega.HaveOccurred())
err = utils.WaitTimeoutForPodSucceeded(f.K8sClient, executorPod.Name, namespace, utils.PodWaitForTime)
return executorPod
}

func createConsumerPod(targetPvc *k8sv1.PersistentVolumeClaim, f *Framework) {
fmt.Fprintf(ginkgo.GinkgoWriter, "INFO: creating \"consumer-pod\" to force binding PVC: %s\n", targetPvc.Name)
executorPod := f.CreateConsumerPod(targetPvc)

namespace := targetPvc.Namespace
err := utils.WaitTimeoutForPodSucceeded(f.K8sClient, executorPod.Name, namespace, utils.PodWaitForTime)
gomega.Expect(err).ToNot(gomega.HaveOccurred())

err = utils.WaitForPersistentVolumeClaimPhase(f.K8sClient, namespace, k8sv1.ClaimBound, targetPvc.Name)
Expand All @@ -123,15 +142,9 @@ func createConsumerPod(targetPvc *k8sv1.PersistentVolumeClaim, f *Framework) {

func createConsumerPodForPopulationPVC(targetPvc *k8sv1.PersistentVolumeClaim, f *Framework) {
fmt.Fprintf(ginkgo.GinkgoWriter, "INFO: creating \"consumer-pod\" to get 'selected-node' annotation on PVC: %s\n", targetPvc.Name)
namespace := targetPvc.Namespace

err := utils.WaitForPersistentVolumeClaimPhase(f.K8sClient, targetPvc.Namespace, k8sv1.ClaimPending, targetPvc.Name)
gomega.Expect(err).ToNot(gomega.HaveOccurred())

podName := naming.GetResourceName("consumer-pod", targetPvc.Name)
executorPod, err := f.CreateNoopPodWithPVC(podName, namespace, targetPvc)
gomega.Expect(err).ToNot(gomega.HaveOccurred())
executorPod := f.CreateConsumerPod(targetPvc)

namespace := targetPvc.Namespace
selectedNode, status, err := utils.WaitForPVCAnnotation(f.K8sClient, namespace, targetPvc, controller.AnnSelectedNode)
gomega.Expect(err).ToNot(gomega.HaveOccurred())
gomega.Expect(status).To(gomega.BeTrue())
Expand Down Expand Up @@ -299,6 +312,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
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
6 changes: 5 additions & 1 deletion tests/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,9 +421,13 @@ var _ = Describe("ALL Operator tests", func() {
Expect(err).ToNot(HaveOccurred())
f.ForceBindPvcIfDvIsWaitForFirstConsumer(dv)

pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(dv.Namespace).Get(context.TODO(), dv.Name, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
uploadPodName := utils.UploadPodName(pvc)

By("Waiting for pod to be running")
Eventually(func() bool {
pod, err := f.K8sClient.CoreV1().Pods(dv.Namespace).Get(context.TODO(), "cdi-upload-"+dv.Name, metav1.GetOptions{})
pod, err := f.K8sClient.CoreV1().Pods(dv.Namespace).Get(context.TODO(), uploadPodName, metav1.GetOptions{})
if errors.IsNotFound(err) {
return false
}
Expand Down
1 change: 1 addition & 0 deletions tests/utils/BUILD.bazel
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
Loading

0 comments on commit 2a16983

Please sign in to comment.