diff --git a/images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go b/images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go index 6952349b0..2db06b974 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go +++ b/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( diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/interfaces.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/interfaces.go index 472cc2d52..fa9c199df 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/interfaces.go +++ b/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 diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/mock.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/mock.go index dae3cccc7..5743c49c1 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/mock.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/mock.go @@ -34,6 +34,9 @@ var _ Importer = &ImporterMock{} // CleanUpSupplementsFunc: func(ctx context.Context, sup *supplements.Generator) (bool, error) { // panic("mock out the CleanUpSupplements method") // }, +// DeletePodFunc: func(ctx context.Context, obj service.ObjectKind, controllerName string) (bool, error) { +// panic("mock out the DeletePod method") +// }, // GetPodFunc: func(ctx context.Context, sup *supplements.Generator) (*corev1.Pod, error) { // panic("mock out the GetPod method") // }, @@ -65,6 +68,9 @@ type ImporterMock struct { // CleanUpSupplementsFunc mocks the CleanUpSupplements method. CleanUpSupplementsFunc func(ctx context.Context, sup *supplements.Generator) (bool, error) + // DeletePodFunc mocks the DeletePod method. + DeletePodFunc func(ctx context.Context, obj service.ObjectKind, controllerName string) (bool, error) + // GetPodFunc mocks the GetPod method. GetPodFunc func(ctx context.Context, sup *supplements.Generator) (*corev1.Pod, error) @@ -99,6 +105,15 @@ type ImporterMock struct { // Sup is the sup argument value. Sup *supplements.Generator } + // DeletePod holds details about calls to the DeletePod method. + DeletePod []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Obj is the obj argument value. + Obj service.ObjectKind + // ControllerName is the controllerName argument value. + ControllerName string + } // GetPod holds details about calls to the GetPod method. GetPod []struct { // Ctx is the ctx argument value. @@ -160,6 +175,7 @@ type ImporterMock struct { } lockCleanUp sync.RWMutex lockCleanUpSupplements sync.RWMutex + lockDeletePod sync.RWMutex lockGetPod sync.RWMutex lockGetPodSettingsWithPVC sync.RWMutex lockProtect sync.RWMutex @@ -240,6 +256,46 @@ func (mock *ImporterMock) CleanUpSupplementsCalls() []struct { return calls } +// DeletePod calls DeletePodFunc. +func (mock *ImporterMock) DeletePod(ctx context.Context, obj service.ObjectKind, controllerName string) (bool, error) { + if mock.DeletePodFunc == nil { + panic("ImporterMock.DeletePodFunc: method is nil but Importer.DeletePod was just called") + } + callInfo := struct { + Ctx context.Context + Obj service.ObjectKind + ControllerName string + }{ + Ctx: ctx, + Obj: obj, + ControllerName: controllerName, + } + mock.lockDeletePod.Lock() + mock.calls.DeletePod = append(mock.calls.DeletePod, callInfo) + mock.lockDeletePod.Unlock() + return mock.DeletePodFunc(ctx, obj, controllerName) +} + +// DeletePodCalls gets all the calls that were made to DeletePod. +// Check the length with: +// +// len(mockedImporter.DeletePodCalls()) +func (mock *ImporterMock) DeletePodCalls() []struct { + Ctx context.Context + Obj service.ObjectKind + ControllerName string +} { + var calls []struct { + Ctx context.Context + Obj service.ObjectKind + ControllerName string + } + mock.lockDeletePod.RLock() + calls = mock.calls.DeletePod + mock.lockDeletePod.RUnlock() + return calls +} + // GetPod calls GetPodFunc. func (mock *ImporterMock) GetPod(ctx context.Context, sup *supplements.Generator) (*corev1.Pod, error) { if mock.GetPodFunc == nil { diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref.go index 9828acac4..8ee282ae8 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref.go +++ b/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 { diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go index 60c758117..a92662033 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go +++ b/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 } diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vi_on_pvc.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vi_on_pvc.go index f367e5b2e..21ecb5a9a 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vi_on_pvc.go +++ b/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 { diff --git a/images/virtualization-artifact/pkg/controller/service/disk_service.go b/images/virtualization-artifact/pkg/controller/service/disk_service.go index bbf1c99fc..5b3e09cca 100644 --- a/images/virtualization-artifact/pkg/controller/service/disk_service.go +++ b/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 diff --git a/images/virtualization-artifact/pkg/controller/service/importer_service.go b/images/virtualization-artifact/pkg/controller/service/importer_service.go index 89011cda6..a97e3c148 100644 --- a/images/virtualization-artifact/pkg/controller/service/importer_service.go +++ b/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 { diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref.go index f11863824..77e1a8167 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref.go +++ b/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), } } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go index d4ad01200..eab24721f 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go @@ -22,7 +22,9 @@ import ( "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/deckhouse/virtualization-controller/pkg/common/datasource" "github.com/deckhouse/virtualization-controller/pkg/controller/common" @@ -32,6 +34,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/dvcr" "github.com/deckhouse/virtualization-controller/pkg/imageformat" "github.com/deckhouse/virtualization-controller/pkg/logger" + "github.com/deckhouse/virtualization-controller/pkg/sdk/framework/helper" "github.com/deckhouse/virtualization-controller/pkg/util" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vicondition" @@ -42,12 +45,14 @@ type ObjectRefVirtualDisk struct { diskService *service.DiskService statService Stat dvcrSettings *dvcr.Settings + client client.Client storageClassForPVC string } -func NewObjectRefVirtualDisk(importerService Importer, diskService *service.DiskService, dvcrSettings *dvcr.Settings, statService Stat, storageClassForPVC string) *ObjectRefVirtualDisk { +func NewObjectRefVirtualDisk(importerService Importer, client client.Client, diskService *service.DiskService, dvcrSettings *dvcr.Settings, statService Stat, storageClassForPVC string) *ObjectRefVirtualDisk { return &ObjectRefVirtualDisk{ importerService: importerService, + client: client, diskService: diskService, statService: statService, dvcrSettings: dvcrSettings, @@ -329,7 +334,7 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, vi *virtv2.VirtualI return fmt.Errorf("not a %s data source", virtv2.ClusterVirtualImageObjectRefKindVirtualDisk) } - vd, err := ds.diskService.GetVirtualDisk(ctx, vi.Spec.DataSource.ObjectRef.Name, vi.Namespace) + vd, err := helper.FetchObject(ctx, types.NamespacedName{Name: vi.Spec.DataSource.ObjectRef.Name, Namespace: vi.Namespace}, ds.client, &virtv2.VirtualDisk{}) if err != nil { return err } @@ -340,7 +345,7 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, vi *virtv2.VirtualI 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 }