Skip to content

Commit

Permalink
Report succeeded phase for block blank import more accurately
Browse files Browse the repository at this point in the history
The import controller prematurely reports that a blank block import is succeeded,
Even before the target PVC is Bound.
This makes it possible for the import populator to enter
an endless loop of trying to rebind:
```
2023-10-04T11:18:47Z	ERROR	Reconciler error	{"controller": "import-populator", "object": {"name":"create-blank-image-to-block-pvc","namespace":"cdi-e2e-tests-importer-fmdj5"}, "namespace": "cdi-e2e-tests-importer-fmdj5", "name": "create-blank-image-to-block-pvc", "reconcileID": "015fcf2b-afbd-48ba-b35f-00cc37ef548a", "error": "PersistentVolume \"\" not found"}
```

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
  • Loading branch information
akalenyu committed Oct 4, 2023
1 parent d3e1778 commit f7839c4
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 9 deletions.
13 changes: 6 additions & 7 deletions pkg/controller/import-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,13 @@ func (r *ImportReconciler) Reconcile(_ context.Context, req reconcile.Request) (
volumeMode := cc.GetVolumeMode(pvc)
if volumeMode == corev1.PersistentVolumeBlock && pvc.GetAnnotations()[cc.AnnSource] == cc.SourceNone && pvc.GetAnnotations()[cc.AnnPreallocationRequested] != "true" {
log.V(1).Info("attempting to create blank disk for block mode, this is a no-op, marking pvc with pod-phase succeeded")
if pvc.GetAnnotations() == nil {
pvc.SetAnnotations(make(map[string]string, 0))
}
pvc.GetAnnotations()[cc.AnnPodPhase] = string(corev1.PodSucceeded)
if err := r.updatePVC(pvc, log); err != nil {
return reconcile.Result{}, errors.WithMessage(err, fmt.Sprintf("could not update pvc %q annotation and/or label", pvc.Name))
if pvc.Status.Phase == corev1.ClaimBound {
cc.AddAnnotation(pvc, cc.AnnPodPhase, string(corev1.PodSucceeded))
if err := r.updatePVC(pvc, log); err != nil {
return reconcile.Result{}, errors.WithMessage(err, fmt.Sprintf("could not update pvc %q annotation and/or label", pvc.Name))
}
return reconcile.Result{}, nil
}
return reconcile.Result{}, nil
}
return r.reconcilePvc(pvc, log)
}
Expand Down
18 changes: 16 additions & 2 deletions tests/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -587,18 +587,23 @@ var _ = Describe("Importer Test Suite-Block_device", func() {

})

It("[test_id:4972]Should create blank raw image for block PV", func() {
DescribeTable("Should create blank raw image for block PV", func(consumer bool) {
if !f.IsBlockVolumeStorageClassAvailable() {
Skip("Storage Class for block volume is not available")
}
dv := utils.NewDataVolumeForBlankRawImageBlock("create-blank-image-to-block-pvc", "500Mi", f.BlockSCName)
if !consumer {
controller.AddAnnotation(dv, controller.AnnImmediateBinding, "true")
}
dv, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dv)
Expect(err).ToNot(HaveOccurred())

By("verifying pvc was created")
pvc, err := utils.WaitForPVC(f.K8sClient, dv.Namespace, dv.Name)
Expect(err).ToNot(HaveOccurred())
f.ForceBindIfWaitForFirstConsumer(pvc)
if consumer {
f.ForceBindIfWaitForFirstConsumer(pvc)
}

By("Waiting for import to be completed")
err = utils.WaitForDataVolumePhase(f, f.Namespace.Name, cdiv1.Succeeded, dv.Name)
Expand All @@ -611,6 +616,15 @@ var _ = Describe("Importer Test Suite-Block_device", func() {
return strings.Contains(log, "attempting to create blank disk for block mode")
}, controllerSkipPVCCompleteTimeout, assertionPollInterval).Should(BeTrue())
Expect(err).ToNot(HaveOccurred())

err = utils.WaitForPersistentVolumeClaimPhase(f.K8sClient, pvc.Namespace, v1.ClaimBound, pvc.Name)
Expect(err).ToNot(HaveOccurred())
},
Entry("[test_id:4972] with first consumer", true),
Entry("with bind immediate annotation", false),
)
It("Should create blank raw image for block PV", func() {

})

It("Should perform fsync syscall after qemu-img convert to raw", func() {
Expand Down

0 comments on commit f7839c4

Please sign in to comment.