From aae4b4e5aadd94e30aa7876008455b60e53ac07a Mon Sep 17 00:00:00 2001 From: Isteb4k Date: Wed, 26 Jun 2024 20:09:52 +0200 Subject: [PATCH] fix(vd): copy error from data volume Signed-off-by: Isteb4k --- .../pkg/controller/service/condition.go | 10 +++++++++ .../pkg/controller/service/disk_service.go | 21 +++++++++++++++---- .../pkg/controller/service/errors.go | 1 + .../controller/vd/internal/source/blank.go | 8 ++++++- .../pkg/controller/vd/internal/source/http.go | 8 ++++++- .../vd/internal/source/object_ref.go | 8 ++++++- .../controller/vd/internal/source/registry.go | 8 ++++++- .../controller/vd/internal/source/upload.go | 8 ++++++- .../pkg/controller/vd/vd_reconciler.go | 7 ++++++- 9 files changed, 69 insertions(+), 10 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/service/condition.go b/images/virtualization-artifact/pkg/controller/service/condition.go index 7b0203188..e725cf07d 100644 --- a/images/virtualization-artifact/pkg/controller/service/condition.go +++ b/images/virtualization-artifact/pkg/controller/service/condition.go @@ -20,6 +20,7 @@ import ( "unicode" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" "github.com/deckhouse/virtualization/api/core/v1alpha2/cvicondition" ) @@ -60,3 +61,12 @@ func CapitalizeFirstLetter(s string) string { return string(runes) } + +func GetDataVolumeCondition(conditionType cdiv1.DataVolumeConditionType, conditions []cdiv1.DataVolumeCondition) *cdiv1.DataVolumeCondition { + for i, condition := range conditions { + if condition.Type == conditionType { + return &conditions[i] + } + } + return nil +} diff --git a/images/virtualization-artifact/pkg/controller/service/disk_service.go b/images/virtualization-artifact/pkg/controller/service/disk_service.go index 6e82f1355..25a38aa28 100644 --- a/images/virtualization-artifact/pkg/controller/service/disk_service.go +++ b/images/virtualization-artifact/pkg/controller/service/disk_service.go @@ -235,12 +235,26 @@ func (s DiskService) GetPersistentVolume(ctx context.Context, pvc *corev1.Persis return helper.FetchObject(ctx, types.NamespacedName{Name: pvc.Spec.VolumeName}, s.client, &corev1.PersistentVolume{}) } -func (s DiskService) CheckStorageClass(ctx context.Context, storageClassName *string) error { +func (s DiskService) CheckImportProcess(ctx context.Context, dv *cdiv1.DataVolume, storageClassName *string) error { + var err error + if storageClassName == nil || *storageClassName == "" { - return s.checkDefaultStorageClass(ctx) + err = s.checkDefaultStorageClass(ctx) + } else { + err = s.checkStorageClass(ctx, *storageClassName) + } + if err != nil { + return err } - return s.checkStorageClass(ctx, *storageClassName) + if dv != nil { + dvRunning := GetDataVolumeCondition(cdiv1.DataVolumeRunning, dv.Status.Conditions) + if dvRunning != nil && dvRunning.Status == corev1.ConditionFalse && dvRunning.Reason == "Error" { + return fmt.Errorf("%w: %s", ErrDataVolumeNotRunning, dvRunning.Message) + } + } + + return nil } func (s DiskService) checkDefaultStorageClass(ctx context.Context) error { @@ -251,7 +265,6 @@ func (s DiskService) checkDefaultStorageClass(ctx context.Context) error { } for _, sc := range scs.Items { - // TODO comment. if sc.Annotations[common.AnnDefaultStorageClass] == "true" { return nil } diff --git a/images/virtualization-artifact/pkg/controller/service/errors.go b/images/virtualization-artifact/pkg/controller/service/errors.go index 3672b3554..e4ca53863 100644 --- a/images/virtualization-artifact/pkg/controller/service/errors.go +++ b/images/virtualization-artifact/pkg/controller/service/errors.go @@ -23,4 +23,5 @@ var ErrTooSmallDiskSize = errors.New("virtual disk size is too small") var ( ErrStorageClassNotFound = errors.New("storage class not found") ErrDefaultStorageClassNotFound = errors.New("default storage class not found") + ErrDataVolumeNotRunning = errors.New("pvc import is not running") ) diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/blank.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/blank.go index 83db6f731..31965c1bb 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/blank.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/blank.go @@ -150,7 +150,7 @@ func (ds BlankDataSource) Sync(ctx context.Context, vd *virtv2.VirtualDisk) (boo return false, err } - err = ds.diskService.CheckStorageClass(ctx, vd.Spec.PersistentVolumeClaim.StorageClass) + err = ds.diskService.CheckImportProcess(ctx, dv, vd.Spec.PersistentVolumeClaim.StorageClass) switch { case err == nil: vd.Status.Phase = virtv2.DiskProvisioning @@ -158,6 +158,12 @@ func (ds BlankDataSource) Sync(ctx context.Context, vd *virtv2.VirtualDisk) (boo condition.Reason = vdcondition.Provisioning condition.Message = "Import is in the process of provisioning to PVC." return false, nil + case errors.Is(err, service.ErrDataVolumeNotRunning): + vd.Status.Phase = virtv2.DiskFailed + condition.Status = metav1.ConditionFalse + condition.Reason = vdcondition.ProvisioningFailed + condition.Message = service.CapitalizeFirstLetter(err.Error()) + return false, nil case errors.Is(err, service.ErrStorageClassNotFound): vd.Status.Phase = virtv2.DiskFailed condition.Status = metav1.ConditionFalse diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/http.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/http.go index 885ed7dec..52f180adc 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/http.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/http.go @@ -239,7 +239,7 @@ func (ds HTTPDataSource) Sync(ctx context.Context, vd *virtv2.VirtualDisk) (bool return false, err } - err = ds.diskService.CheckStorageClass(ctx, vd.Spec.PersistentVolumeClaim.StorageClass) + err = ds.diskService.CheckImportProcess(ctx, dv, vd.Spec.PersistentVolumeClaim.StorageClass) switch { case err == nil: vd.Status.Phase = virtv2.DiskProvisioning @@ -247,6 +247,12 @@ func (ds HTTPDataSource) Sync(ctx context.Context, vd *virtv2.VirtualDisk) (bool condition.Reason = vdcondition.Provisioning condition.Message = "Import is in the process of provisioning to PVC." return false, nil + case errors.Is(err, service.ErrDataVolumeNotRunning): + vd.Status.Phase = virtv2.DiskFailed + condition.Status = metav1.ConditionFalse + condition.Reason = vdcondition.ProvisioningFailed + condition.Message = service.CapitalizeFirstLetter(err.Error()) + return false, nil case errors.Is(err, service.ErrStorageClassNotFound): vd.Status.Phase = virtv2.DiskFailed condition.Status = metav1.ConditionFalse diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref.go index 8e9f00e25..ee02baddd 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref.go @@ -163,7 +163,7 @@ func (ds ObjectRefDataSource) Sync(ctx context.Context, vd *virtv2.VirtualDisk) return false, err } - err = ds.diskService.CheckStorageClass(ctx, vd.Spec.PersistentVolumeClaim.StorageClass) + err = ds.diskService.CheckImportProcess(ctx, dv, vd.Spec.PersistentVolumeClaim.StorageClass) switch { case err == nil: vd.Status.Phase = virtv2.DiskProvisioning @@ -171,6 +171,12 @@ func (ds ObjectRefDataSource) Sync(ctx context.Context, vd *virtv2.VirtualDisk) condition.Reason = vdcondition.Provisioning condition.Message = "Import is in the process of provisioning to PVC." return false, nil + case errors.Is(err, service.ErrDataVolumeNotRunning): + vd.Status.Phase = virtv2.DiskFailed + condition.Status = metav1.ConditionFalse + condition.Reason = vdcondition.ProvisioningFailed + condition.Message = service.CapitalizeFirstLetter(err.Error()) + return false, nil case errors.Is(err, service.ErrStorageClassNotFound): vd.Status.Phase = virtv2.DiskFailed condition.Status = metav1.ConditionFalse diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/registry.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/registry.go index dddb66993..6f85467bf 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/registry.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/registry.go @@ -244,7 +244,7 @@ func (ds RegistryDataSource) Sync(ctx context.Context, vd *virtv2.VirtualDisk) ( return false, err } - err = ds.diskService.CheckStorageClass(ctx, vd.Spec.PersistentVolumeClaim.StorageClass) + err = ds.diskService.CheckImportProcess(ctx, dv, vd.Spec.PersistentVolumeClaim.StorageClass) switch { case err == nil: vd.Status.Phase = virtv2.DiskProvisioning @@ -252,6 +252,12 @@ func (ds RegistryDataSource) Sync(ctx context.Context, vd *virtv2.VirtualDisk) ( condition.Reason = vdcondition.Provisioning condition.Message = "Import is in the process of provisioning to PVC." return false, nil + case errors.Is(err, service.ErrDataVolumeNotRunning): + vd.Status.Phase = virtv2.DiskFailed + condition.Status = metav1.ConditionFalse + condition.Reason = vdcondition.ProvisioningFailed + condition.Message = service.CapitalizeFirstLetter(err.Error()) + return false, nil case errors.Is(err, service.ErrStorageClassNotFound): vd.Status.Phase = virtv2.DiskFailed condition.Status = metav1.ConditionFalse diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/upload.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/upload.go index 41643b1f5..fea99a822 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/upload.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/upload.go @@ -266,7 +266,7 @@ func (ds UploadDataSource) Sync(ctx context.Context, vd *virtv2.VirtualDisk) (bo return false, err } - err = ds.diskService.CheckStorageClass(ctx, vd.Spec.PersistentVolumeClaim.StorageClass) + err = ds.diskService.CheckImportProcess(ctx, dv, vd.Spec.PersistentVolumeClaim.StorageClass) switch { case err == nil: vd.Status.Phase = virtv2.DiskProvisioning @@ -274,6 +274,12 @@ func (ds UploadDataSource) Sync(ctx context.Context, vd *virtv2.VirtualDisk) (bo condition.Reason = vdcondition.Provisioning condition.Message = "Import is in the process of provisioning to PVC." return false, nil + case errors.Is(err, service.ErrDataVolumeNotRunning): + vd.Status.Phase = virtv2.DiskFailed + condition.Status = metav1.ConditionFalse + condition.Reason = vdcondition.ProvisioningFailed + condition.Message = service.CapitalizeFirstLetter(err.Error()) + return false, nil case errors.Is(err, service.ErrStorageClassNotFound): vd.Status.Phase = virtv2.DiskFailed condition.Status = metav1.ConditionFalse diff --git a/images/virtualization-artifact/pkg/controller/vd/vd_reconciler.go b/images/virtualization-artifact/pkg/controller/vd/vd_reconciler.go index a81715df7..a181b3767 100644 --- a/images/virtualization-artifact/pkg/controller/vd/vd_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vd/vd_reconciler.go @@ -149,7 +149,12 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr return false } - return oldDV.Status.Progress != newDV.Status.Progress + if oldDV.Status.Progress != newDV.Status.Progress { + return true + } + + dvRunning := service.GetDataVolumeCondition(cdiv1.DataVolumeRunning, newDV.Status.Conditions) + return dvRunning != nil && dvRunning.Reason == "Error" }, }, ); err != nil {