fix(cvi): fix cleanup for resources (#363)

Signed-off-by: Dmitry Lopatin <dmitry.lopatin@flant.com>

images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go

@@ -61,12 +61,11 @@ func NewController(
 	protection := service.NewProtectionService(mgr.GetClient(), virtv2.FinalizerCVIProtection)
 	importer := service.NewImporterService(dvcr, mgr.GetClient(), importerImage, requirements, PodPullPolicy, PodVerbose, ControllerName, protection)
 	uploader := service.NewUploaderService(dvcr, mgr.GetClient(), uploaderImage, requirements, PodPullPolicy, PodVerbose, ControllerName, protection)
-	disk := service.NewDiskService(mgr.GetClient(), dvcr, protection)
 
 	sources := source.NewSources()
 	sources.Set(virtv2.DataSourceTypeHTTP, source.NewHTTPDataSource(stat, importer, dvcr, ns))
 	sources.Set(virtv2.DataSourceTypeContainerImage, source.NewRegistryDataSource(stat, importer, dvcr, mgr.GetClient(), ns))
-	sources.Set(virtv2.DataSourceTypeObjectRef, source.NewObjectRefDataSource(stat, importer, disk, dvcr, mgr.GetClient(), ns))
+	sources.Set(virtv2.DataSourceTypeObjectRef, source.NewObjectRefDataSource(stat, importer, dvcr, mgr.GetClient(), ns))
 	sources.Set(virtv2.DataSourceTypeUpload, source.NewUploadDataSource(stat, uploader, dvcr, ns))
 
 	reconciler := NewReconciler(

images/virtualization-artifact/pkg/controller/cvi/internal/source/interfaces.go

@@ -40,6 +40,7 @@ type Importer interface {
 	CleanUp(ctx context.Context, sup *supplements.Generator) (bool, error)
 	CleanUpSupplements(ctx context.Context, sup *supplements.Generator) (bool, error)
 	GetPod(ctx context.Context, sup *supplements.Generator) (*corev1.Pod, error)
+	DeletePod(ctx context.Context, obj service.ObjectKind, controllerName string) (bool, error)
 	Protect(ctx context.Context, pod *corev1.Pod) error
 	Unprotect(ctx context.Context, pod *corev1.Pod) error
 	GetPodSettingsWithPVC(ownerRef *metav1.OwnerReference, sup *supplements.Generator, pvcName, pvcNamespace string) *importer.PodSettings

images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref.go

@@ -52,7 +52,6 @@ type ObjectRefDataSource struct {
 func NewObjectRefDataSource(
 	statService Stat,
 	importerService Importer,
-	diskService *service.DiskService,
 	dvcrSettings *dvcr.Settings,
 	client client.Client,
 	controllerNamespace string,
@@ -64,8 +63,8 @@ func NewObjectRefDataSource(
 		client:              client,
 		controllerNamespace: controllerNamespace,
 
-		viOnPvcSyncer: NewObjectRefVirtualImageOnPvc(importerService, diskService, controllerNamespace, dvcrSettings, statService),
-		vdSyncer:      NewObjectRefVirtualDisk(importerService, diskService, controllerNamespace, dvcrSettings, statService),
+		viOnPvcSyncer: NewObjectRefVirtualImageOnPvc(importerService, dvcrSettings, statService),
+		vdSyncer:      NewObjectRefVirtualDisk(importerService, client, controllerNamespace, dvcrSettings, statService),
 	}
 }
 
@@ -231,37 +230,24 @@ func (ds ObjectRefDataSource) Sync(ctx context.Context, cvi *virtv2.ClusterVirtu
 }
 
 func (ds ObjectRefDataSource) CleanUp(ctx context.Context, cvi *virtv2.ClusterVirtualImage) (bool, error) {
-	if cvi.Spec.DataSource.ObjectRef == nil {
-		return false, fmt.Errorf("nil object ref: %s", cvi.Spec.DataSource.Type)
+	viRefRequeue, err := ds.viOnPvcSyncer.CleanUp(ctx, cvi)
+	if err != nil {
+		return false, err
 	}
 
-	switch cvi.Spec.DataSource.ObjectRef.Kind {
-	case virtv2.ClusterVirtualImageObjectRefKindVirtualImage:
-		viKey := types.NamespacedName{Name: cvi.Spec.DataSource.ObjectRef.Name, Namespace: cvi.Spec.DataSource.ObjectRef.Namespace}
-		vi, err := helper.FetchObject(ctx, viKey, ds.client, &virtv2.VirtualImage{})
-		if err != nil {
-			return false, fmt.Errorf("unable to get VI %s: %w", viKey, err)
-		}
-
-		if vi == nil {
-			return false, NewImageNotReadyError(cvi.Spec.DataSource.ObjectRef.Name)
-		}
-
-		if vi.Spec.Storage == virtv2.StorageKubernetes {
-			return ds.viOnPvcSyncer.CleanUp(ctx, cvi)
-		}
-	case virtv2.ClusterVirtualImageObjectRefKindVirtualDisk:
-		return ds.vdSyncer.CleanUp(ctx, cvi)
+	vdRefRequeue, err := ds.vdSyncer.CleanUp(ctx, cvi)
+	if err != nil {
+		return false, err
 	}
 
 	supgen := supplements.NewGenerator(common.CVIShortName, cvi.Name, ds.controllerNamespace, cvi.UID)
 
-	requeue, err := ds.importerService.CleanUp(ctx, supgen)
+	objRefRequeue, err := ds.importerService.CleanUp(ctx, supgen)
 	if err != nil {
 		return false, err
 	}
 
-	return requeue, nil
+	return objRefRequeue || vdRefRequeue || viRefRequeue, nil
 }
 
 func (ds ObjectRefDataSource) Validate(ctx context.Context, cvi *virtv2.ClusterVirtualImage) error {

images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go

@@ -22,6 +22,8 @@ import (
 	"fmt"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/deckhouse/virtualization-controller/pkg/common/datasource"
 	cc "github.com/deckhouse/virtualization-controller/pkg/controller/common"
@@ -30,24 +32,25 @@ import (
 	"github.com/deckhouse/virtualization-controller/pkg/controller/supplements"
 	"github.com/deckhouse/virtualization-controller/pkg/dvcr"
 	"github.com/deckhouse/virtualization-controller/pkg/logger"
+	"github.com/deckhouse/virtualization-controller/pkg/sdk/framework/helper"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2/vicondition"
 )
 
 type ObjectRefVirtualDisk struct {
 	importerService     Importer
-	diskService         *service.DiskService
+	client              client.Client
 	statService         Stat
 	dvcrSettings        *dvcr.Settings
 	controllerNamespace string
 }
 
-func NewObjectRefVirtualDisk(importerService Importer, diskService *service.DiskService, controllerNamespace string, dvcrSettings *dvcr.Settings, statService Stat) *ObjectRefVirtualDisk {
+func NewObjectRefVirtualDisk(importerService Importer, client client.Client, controllerNamespace string, dvcrSettings *dvcr.Settings, statService Stat) *ObjectRefVirtualDisk {
 	return &ObjectRefVirtualDisk{
 		importerService:     importerService,
-		diskService:         diskService,
 		statService:         statService,
 		dvcrSettings:        dvcrSettings,
+		client:              client,
 		controllerNamespace: controllerNamespace,
 	}
 }
@@ -169,19 +172,12 @@ func (ds ObjectRefVirtualDisk) Sync(ctx context.Context, cvi *virtv2.ClusterVirt
 }
 
 func (ds ObjectRefVirtualDisk) CleanUp(ctx context.Context, cvi *virtv2.ClusterVirtualImage) (bool, error) {
-	supgen := supplements.NewGenerator(cc.CVIShortName, cvi.Name, cvi.Spec.DataSource.ObjectRef.Namespace, cvi.UID)
-
-	importerRequeue, err := ds.importerService.CleanUp(ctx, supgen)
-	if err != nil {
-		return false, err
-	}
-
-	diskRequeue, err := ds.diskService.CleanUp(ctx, supgen)
+	importerRequeue, err := ds.importerService.DeletePod(ctx, cvi, controllerName)
 	if err != nil {
 		return false, err
 	}
 
-	return importerRequeue || diskRequeue, nil
+	return importerRequeue, nil
 }
 
 func (ds ObjectRefVirtualDisk) getEnvSettings(cvi *virtv2.ClusterVirtualImage, sup *supplements.Generator) *importer.Settings {
@@ -202,7 +198,7 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, cvi *virtv2.Cluster
 		return fmt.Errorf("not a %s data source", virtv2.ClusterVirtualImageObjectRefKindVirtualDisk)
 	}
 
-	vd, err := ds.diskService.GetVirtualDisk(ctx, cvi.Spec.DataSource.ObjectRef.Name, cvi.Spec.DataSource.ObjectRef.Namespace)
+	vd, err := helper.FetchObject(ctx, types.NamespacedName{Name: cvi.Spec.DataSource.ObjectRef.Name, Namespace: cvi.Spec.DataSource.ObjectRef.Namespace}, ds.client, &virtv2.VirtualDisk{})
 	if err != nil {
 		return err
 	}
@@ -213,7 +209,8 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, cvi *virtv2.Cluster
 
 	if len(vd.Status.AttachedToVirtualMachines) != 0 {
 		vmName := vd.Status.AttachedToVirtualMachines[0]
-		vm, err := ds.diskService.GetVirtualMachine(ctx, vmName.Name, vd.Namespace)
+
+		vm, err := helper.FetchObject(ctx, types.NamespacedName{Name: vmName.Name, Namespace: vd.Namespace}, ds.client, &virtv2.VirtualMachine{})
 		if err != nil {
 			return err
 		}

images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vi_on_pvc.go

@@ -33,21 +33,19 @@ import (
 	"github.com/deckhouse/virtualization/api/core/v1alpha2/cvicondition"
 )
 
+const controllerName = "cvi-controller"
+
 type ObjectRefVirtualImageOnPvc struct {
-	importerService     Importer
-	diskService         *service.DiskService
-	statService         Stat
-	dvcrSettings        *dvcr.Settings
-	controllerNamespace string
+	importerService Importer
+	statService     Stat
+	dvcrSettings    *dvcr.Settings
 }
 
-func NewObjectRefVirtualImageOnPvc(importerService Importer, diskService *service.DiskService, controllerNamespace string, dvcrSettings *dvcr.Settings, statService Stat) *ObjectRefVirtualImageOnPvc {
+func NewObjectRefVirtualImageOnPvc(importerService Importer, dvcrSettings *dvcr.Settings, statService Stat) *ObjectRefVirtualImageOnPvc {
 	return &ObjectRefVirtualImageOnPvc{
-		importerService:     importerService,
-		diskService:         diskService,
-		statService:         statService,
-		dvcrSettings:        dvcrSettings,
-		controllerNamespace: controllerNamespace,
+		importerService: importerService,
+		statService:     statService,
+		dvcrSettings:    dvcrSettings,
 	}
 }
 
@@ -168,19 +166,12 @@ func (ds ObjectRefVirtualImageOnPvc) Sync(ctx context.Context, cvi *virtv2.Clust
 }
 
 func (ds ObjectRefVirtualImageOnPvc) CleanUp(ctx context.Context, cvi *virtv2.ClusterVirtualImage) (bool, error) {
-	supgen := supplements.NewGenerator(common.CVIShortName, cvi.Name, cvi.Spec.DataSource.ObjectRef.Namespace, cvi.UID)
-
-	importerRequeue, err := ds.importerService.CleanUp(ctx, supgen)
-	if err != nil {
-		return false, err
-	}
-
-	diskRequeue, err := ds.diskService.CleanUp(ctx, supgen)
+	importerRequeue, err := ds.importerService.DeletePod(ctx, cvi, controllerName)
 	if err != nil {
 		return false, err
 	}
 
-	return importerRequeue || diskRequeue, nil
+	return importerRequeue, nil
 }
 
 func (ds ObjectRefVirtualImageOnPvc) getEnvSettings(cvi *virtv2.ClusterVirtualImage, sup *supplements.Generator) *importer.Settings {

images/virtualization-artifact/pkg/controller/service/disk_service.go

@@ -439,14 +439,6 @@ func (s DiskService) GetVirtualDiskSnapshot(ctx context.Context, name, namespace
 	return helper.FetchObject(ctx, types.NamespacedName{Name: name, Namespace: namespace}, s.client, &virtv2.VirtualDiskSnapshot{})
 }
 
-func (s DiskService) GetVirtualDisk(ctx context.Context, name, namespace string) (*virtv2.VirtualDisk, error) {
-	return helper.FetchObject(ctx, types.NamespacedName{Name: name, Namespace: namespace}, s.client, &virtv2.VirtualDisk{})
-}
-
-func (s DiskService) GetVirtualMachine(ctx context.Context, name, namespace string) (*virtv2.VirtualMachine, error) {
-	return helper.FetchObject(ctx, types.NamespacedName{Name: name, Namespace: namespace}, s.client, &virtv2.VirtualMachine{})
-}
-
 func (s DiskService) CheckImportProcess(ctx context.Context, dv *cdiv1.DataVolume, pvc *corev1.PersistentVolumeClaim) error {
 	if dv == nil {
 		return nil

images/virtualization-artifact/pkg/controller/service/importer_service.go

@@ -25,6 +25,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"github.com/deckhouse/virtualization-controller/pkg/common"
 	"github.com/deckhouse/virtualization-controller/pkg/common/datasource"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/importer"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/supplements"
@@ -91,6 +92,39 @@ func (s ImporterService) CleanUp(ctx context.Context, sup *supplements.Generator
 	return s.CleanUpSupplements(ctx, sup)
 }
 
+func (s ImporterService) DeletePod(ctx context.Context, obj ObjectKind, controllerName string) (bool, error) {
+	labelSelector := client.MatchingLabels{common.AppKubernetesManagedByLabel: controllerName}
+
+	podList := &corev1.PodList{}
+	if err := s.client.List(ctx, podList, labelSelector); err != nil {
+		return false, err
+	}
+
+	for _, pod := range podList.Items {
+		for _, ownerRef := range pod.OwnerReferences {
+			if ownerRef.Kind == obj.GroupVersionKind().Kind && ownerRef.Name == obj.GetName() && ownerRef.UID == obj.GetUID() {
+				err := s.protection.RemoveProtection(ctx, &pod)
+				if err != nil {
+					return false, err
+				}
+
+				err = s.client.Delete(ctx, &pod)
+				if err != nil {
+					if k8serrors.IsNotFound(err) {
+						return false, nil
+					}
+
+					return false, err
+				}
+
+				return true, nil
+			}
+		}
+	}
+
+	return false, nil
+}
+
 func (s ImporterService) CleanUpSupplements(ctx context.Context, sup *supplements.Generator) (bool, error) {
 	pod, err := s.GetPod(ctx, sup)
 	if err != nil {

images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref.go

@@ -72,7 +72,7 @@ func NewObjectRefDataSource(
 		diskService:        diskService,
 		storageClassForPVC: storageClassForPVC,
 		viObjectRefOnPvc:   NewObjectRefDataVirtualImageOnPVC(statService, importerService, dvcrSettings, client, diskService, storageClassForPVC),
-		vdSyncer:           NewObjectRefVirtualDisk(importerService, diskService, dvcrSettings, statService, storageClassForPVC),
+		vdSyncer:           NewObjectRefVirtualDisk(importerService, client, diskService, dvcrSettings, statService, storageClassForPVC),
 	}
 }