diff --git a/api/core/v1alpha2/cvicondition/condition.go b/api/core/v1alpha2/cvicondition/condition.go index 3ddd38d68e..1b302b707f 100644 --- a/api/core/v1alpha2/cvicondition/condition.go +++ b/api/core/v1alpha2/cvicondition/condition.go @@ -28,6 +28,8 @@ const ( DatasourceReadyType Type = "DatasourceReady" // ReadyType indicates whether the import process succeeded and the `ClusterVirtualImage` is ready for use. ReadyType Type = "Ready" + // DeletingType indicates whether the ClusterVirtualImage deletion can be completed. + DeletingType Type = "Deleting" ) type ( @@ -35,6 +37,8 @@ type ( DatasourceReadyReason string // ReadyReason represents the various reasons for the Ready condition type. ReadyReason string + // DeletingReason represents the various reasons for the Deleting condition type. + DeletingReason string ) func (s DatasourceReadyReason) String() string { @@ -45,6 +49,10 @@ func (s ReadyReason) String() string { return string(s) } +func (s DeletingReason) String() string { + return string(s) +} + const ( // DatasourceReady indicates that the datasource is ready for use, allowing the import process to start. DatasourceReady DatasourceReadyReason = "DatasourceReady" @@ -75,4 +83,9 @@ const ( Ready ReadyReason = "Ready" // ImageLost indicates that the image in DVCR has been lost and the `ClusterVirtualImage` can no longer be used. ImageLost ReadyReason = "ImageLost" + + // DeletionBlockedByProtection indicates that the ClusterVirtualImage cannot be deleted while it is protected. + DeletionBlockedByProtection DeletingReason = "DeletionBlockedByProtection" + // DeletionCleanupPending indicates that the ClusterVirtualImage cleanup is still in progress. + DeletionCleanupPending DeletingReason = "CleanupPending" ) diff --git a/api/core/v1alpha2/vdcondition/condition.go b/api/core/v1alpha2/vdcondition/condition.go index 76606f7a54..76411136d8 100644 --- a/api/core/v1alpha2/vdcondition/condition.go +++ b/api/core/v1alpha2/vdcondition/condition.go @@ -38,6 +38,8 @@ const ( InUseType Type = "InUse" // MigratingType indicates that the virtual disk is in the process of migrating data from one volume to another (during the migration of a local disk or migration to another storage class). MigratingType Type = "Migrating" + // DeletingType indicates whether the VirtualDisk deletion can be completed. + DeletingType Type = "Deleting" ) type ( @@ -55,6 +57,8 @@ type ( InUseReason string // MigratingReason represents the various reasons for the Migration condition type. MigratingReason string + // DeletingReason represents the various reasons for the Deleting condition type. + DeletingReason string ) func (s DatasourceReadyReason) String() string { @@ -85,6 +89,10 @@ func (s MigratingReason) String() string { return string(s) } +func (s DeletingReason) String() string { + return string(s) +} + const ( // DatasourceReady indicates that the datasource is ready for use, allowing the import process to start. DatasourceReady DatasourceReadyReason = "DatasourceReady" @@ -189,3 +197,10 @@ const ( SnapshottingInProgressReason MigratingReason = "SnapshottingInProgress" StorageClassNotFoundReason MigratingReason = "StorageClassNotFound" ) + +const ( + // DeletionBlockedByProtection indicates that the VirtualDisk cannot be deleted while it is protected. + DeletionBlockedByProtection DeletingReason = "DeletionBlockedByProtection" + // DeletionCleanupPending indicates that the VirtualDisk cleanup is still in progress. + DeletionCleanupPending DeletingReason = "CleanupPending" +) diff --git a/api/core/v1alpha2/vicondition/condition.go b/api/core/v1alpha2/vicondition/condition.go index 3062c2a5cb..6e19bc91c2 100644 --- a/api/core/v1alpha2/vicondition/condition.go +++ b/api/core/v1alpha2/vicondition/condition.go @@ -30,6 +30,8 @@ const ( ReadyType Type = "Ready" // StorageClassReadyType indicates whether the storageClass ready. StorageClassReadyType Type = "StorageClassReady" + // DeletingType indicates whether the VirtualImage deletion can be completed. + DeletingType Type = "Deleting" ) type ( @@ -39,6 +41,8 @@ type ( ReadyReason string // StorageClassReadyReason represents the various reasons for the StorageClassReady condition type. StorageClassReadyReason string + // DeletingReason represents the various reasons for the Deleting condition type. + DeletingReason string ) func (s DatasourceReadyReason) String() string { @@ -53,6 +57,10 @@ func (s StorageClassReadyReason) String() string { return string(s) } +func (s DeletingReason) String() string { + return string(s) +} + const ( // DatasourceReady indicates that the datasource is ready for use, allowing the import process to start. DatasourceReady DatasourceReadyReason = "DatasourceReady" @@ -103,4 +111,9 @@ const ( StorageClassNotFound StorageClassReadyReason = "StorageClassNotFound" // DVCRTypeUsed indicates that the DVCR provisioning chosen. DVCRTypeUsed StorageClassReadyReason = "DVCRTypeUsed" + + // DeletionBlockedByProtection indicates that the VirtualImage cannot be deleted while it is protected. + DeletionBlockedByProtection DeletingReason = "DeletionBlockedByProtection" + // DeletionCleanupPending indicates that the VirtualImage cleanup is still in progress. + DeletionCleanupPending DeletingReason = "CleanupPending" ) diff --git a/api/core/v1alpha2/vmbdacondition/condition.go b/api/core/v1alpha2/vmbdacondition/condition.go index 19d30e18cd..b25514622f 100644 --- a/api/core/v1alpha2/vmbdacondition/condition.go +++ b/api/core/v1alpha2/vmbdacondition/condition.go @@ -28,6 +28,8 @@ const ( AttachedType Type = "Attached" // DiskAttachmentCapacityAvailableType indicates that the entity has not yet reached its predefined limit for block device attachments. DiskAttachmentCapacityAvailableType Type = "DiskAttachmentCapacityAvailableType" + // DeletingType indicates whether the VirtualMachineBlockDeviceAttachment deletion can be completed. + DeletingType Type = "Deleting" ) type ( @@ -39,6 +41,8 @@ type ( AttachedReason string // DiskAttachmentCapacityAvailableReason represent the various reasons for the `DiskAttachmentCapacityAvailableType` condition type. DiskAttachmentCapacityAvailableReason string + // DeletingReason represents the various reasons for the `Deleting` condition type. + DeletingReason string ) const ( @@ -72,6 +76,9 @@ const ( CapacityReached DiskAttachmentCapacityAvailableReason = "CapacityReached" // CapacityUnknown represents unknown condition state CapacityUnknown DiskAttachmentCapacityAvailableReason = "CapacityUnknown" + + // DeletionCleanupPending indicates that the VirtualMachineBlockDeviceAttachment cleanup is still in progress. + DeletionCleanupPending DeletingReason = "CleanupPending" ) func (t Type) String() string { @@ -93,3 +100,7 @@ func (t AttachedReason) String() string { func (t DiskAttachmentCapacityAvailableReason) String() string { return string(t) } + +func (t DeletingReason) String() string { + return string(t) +} diff --git a/images/virtualization-artifact/pkg/common/vm/vm.go b/images/virtualization-artifact/pkg/common/vm/vm.go index c8253af4fc..d14c39346f 100644 --- a/images/virtualization-artifact/pkg/common/vm/vm.go +++ b/images/virtualization-artifact/pkg/common/vm/vm.go @@ -116,6 +116,38 @@ func IsVMActive(ctx context.Context, cli client.Client, vm v1alpha2.VirtualMachi return false, nil } +// BlockDeviceUsage reports whether a VM status references a block device and whether that reference is actively mounted. +func BlockDeviceUsage(ctx context.Context, cli client.Client, vm v1alpha2.VirtualMachine, kind v1alpha2.BlockDeviceKind, name string) (referenced bool, mounted bool, err error) { + if !HasBlockDeviceStatusRef(vm, kind, name) { + return false, false, nil + } + + switch vm.Status.Phase { + case "": + return true, false, nil + case v1alpha2.MachineStopped: + vmIsActive, err := IsVMActive(ctx, cli, vm) + if err != nil { + return true, false, err + } + + return true, vmIsActive, nil + default: + return true, true, nil + } +} + +// HasBlockDeviceStatusRef reports whether VM status contains a block device reference with the provided kind and name. +func HasBlockDeviceStatusRef(vm v1alpha2.VirtualMachine, kind v1alpha2.BlockDeviceKind, name string) bool { + for _, bd := range vm.Status.BlockDeviceRefs { + if bd.Kind == kind && bd.Name == name { + return true + } + } + + return false +} + func GetActivePodName(vm *v1alpha2.VirtualMachine) (string, bool) { for _, pod := range vm.Status.VirtualMachinePods { if pod.Active { diff --git a/images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go b/images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go index c3fa276347..b2365403cb 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go +++ b/images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go @@ -85,7 +85,7 @@ func NewController( internal.NewDatasourceReadyHandler(sources), internal.NewLifeCycleHandler(sources, mgr.GetClient()), internal.NewImagePresenceHandler(recorder, dvcr.NewImageChecker(mgr.GetClient(), dvcrSettings)), - internal.NewDeletionHandler(sources), + internal.NewDeletionHandler(sources, mgr.GetClient()), internal.NewAttacheeHandler(mgr.GetClient()), ) diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/attachee.go b/images/virtualization-artifact/pkg/controller/cvi/internal/attachee.go index 3c9e737dd7..64b9c0ca70 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/attachee.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/attachee.go @@ -69,35 +69,15 @@ func (h AttacheeHandler) hasAttachedVM(ctx context.Context, cvi client.Object) ( } for _, vm := range vms.Items { - if vm.Status.Phase == "" { - continue + _, mounted, err := commonvm.BlockDeviceUsage(ctx, h.client, vm, v1alpha2.ClusterImageDevice, cvi.GetName()) + if err != nil { + return false, err } - if vm.Status.Phase == v1alpha2.MachineStopped { - vmIsActive, err := commonvm.IsVMActive(ctx, h.client, vm) - if err != nil { - return false, err - } - - if !vmIsActive { - continue - } - } - - if h.isCVIAttachedToVM(cvi.GetName(), vm) { + if mounted { return true, nil } } return false, nil } - -func (h AttacheeHandler) isCVIAttachedToVM(cviName string, vm v1alpha2.VirtualMachine) bool { - for _, bda := range vm.Status.BlockDeviceRefs { - if bda.Kind == v1alpha2.ClusterImageDevice && bda.Name == cviName { - return true - } - } - - return false -} diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/deletion.go b/images/virtualization-artifact/pkg/controller/cvi/internal/deletion.go index 80c94dfb71..e90135b4f1 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/deletion.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/deletion.go @@ -18,25 +18,36 @@ package internal import ( "context" + "log/slog" + "sort" + "strings" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" + commonvm "github.com/deckhouse/virtualization-controller/pkg/common/vm" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/cvi/internal/source" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/cvicondition" ) const deletionHandlerName = "DeletionHandler" type DeletionHandler struct { sources *source.Sources + client client.Client } -func NewDeletionHandler(sources *source.Sources) *DeletionHandler { +func NewDeletionHandler(sources *source.Sources, client client.Client) *DeletionHandler { return &DeletionHandler{ sources: sources, + client: client, } } @@ -44,20 +55,86 @@ func (h DeletionHandler) Handle(ctx context.Context, cvi *v1alpha2.ClusterVirtua log := logger.FromContext(ctx).With(logger.SlogHandler(deletionHandlerName)) if cvi.DeletionTimestamp != nil { - requeue, err := h.sources.CleanUp(ctx, cvi) + if controllerutil.ContainsFinalizer(cvi, v1alpha2.FinalizerCVIProtection) { + attachedVMs, err := h.attachedVirtualMachineNames(ctx, cvi) + if err != nil { + return reconcile.Result{}, err + } + + h.setDeletingCondition( + cvi, + cvicondition.DeletionBlockedByProtection, + deletionBlockedByProtectionMessage(cvi, attachedVMs), + ) + return reconcile.Result{}, nil + } + + requeue, reason, err := h.sources.CleanUp(ctx, cvi) if err != nil { return reconcile.Result{}, err } if requeue { + if reason == "" { + reason = "Waiting for cleanup to finish" + } + h.setDeletingCondition(cvi, cvicondition.DeletionCleanupPending, reason) + log.Info("ClusterVirtualImage cleanup is pending", slog.String("reason", reason)) return reconcile.Result{RequeueAfter: time.Second}, nil } + conditions.RemoveCondition(cvicondition.DeletingType, &cvi.Status.Conditions) log.Info("Deletion observed: remove cleanup finalizer from ClusterVirtualImage") controllerutil.RemoveFinalizer(cvi, v1alpha2.FinalizerCVICleanup) return reconcile.Result{}, nil } + conditions.RemoveCondition(cvicondition.DeletingType, &cvi.Status.Conditions) controllerutil.AddFinalizer(cvi, v1alpha2.FinalizerCVICleanup) return reconcile.Result{}, nil } + +func (h DeletionHandler) setDeletingCondition(cvi *v1alpha2.ClusterVirtualImage, reason cvicondition.DeletingReason, message string) { + conditions.SetCondition( + conditions.NewConditionBuilder(cvicondition.DeletingType). + Generation(cvi.Generation). + Status(metav1.ConditionFalse). + Reason(reason). + Message(service.CapitalizeFirstLetter(message)+"."), + &cvi.Status.Conditions, + ) +} + +func (h DeletionHandler) attachedVirtualMachineNames(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) ([]string, error) { + var vms v1alpha2.VirtualMachineList + err := h.client.List(ctx, &vms, &client.ListOptions{}) + if err != nil { + return nil, err + } + + var attachedVMs []string + for _, vm := range vms.Items { + _, mounted, err := commonvm.BlockDeviceUsage(ctx, h.client, vm, v1alpha2.ClusterImageDevice, cvi.GetName()) + if err != nil { + return nil, err + } + + if mounted { + attachedVMs = append(attachedVMs, vm.Namespace+"/"+vm.Name) + } + } + + sort.Strings(attachedVMs) + return attachedVMs, nil +} + +func deletionBlockedByProtectionMessage(cvi *v1alpha2.ClusterVirtualImage, attachedVMs []string) string { + switch len(attachedVMs) { + case 0: + return "The ClusterVirtualImage is protected from deletion by the protection finalizer" + case 1: + return "The ClusterVirtualImage is protected from deletion because it is attached to VirtualMachine " + attachedVMs[0] + default: + return "The ClusterVirtualImage is protected from deletion because it is attached to VirtualMachines: " + strings.Join(attachedVMs, ", ") + } +} diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/deletion_test.go b/images/virtualization-artifact/pkg/controller/cvi/internal/deletion_test.go new file mode 100644 index 0000000000..643b12f7c7 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/deletion_test.go @@ -0,0 +1,120 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package internal + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/controller/cvi/internal/source" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/cvicondition" +) + +var _ = Describe("DeletionHandler", func() { + It("sets Deleting condition with cleanup pending reason", func() { + now := metav1.Now() + cvi := &v1alpha2.ClusterVirtualImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cvi", + DeletionTimestamp: &now, + }, + } + + sources := source.NewSources() + sources.Set(v1alpha2.DataSourceTypeHTTP, cviSourceHandler{ + cleanUp: func(context.Context, *v1alpha2.ClusterVirtualImage) (bool, string, error) { + return true, "waiting for PersistentVolumeClaim deletion default/cvi", nil + }, + }) + + handler := NewDeletionHandler(sources, nil) + result, err := handler.Handle(context.Background(), cvi) + + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(time.Second)) + + cond, ok := conditions.GetCondition(cvicondition.DeletingType, cvi.Status.Conditions) + Expect(ok).To(BeTrue()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(cvicondition.DeletionCleanupPending.String())) + Expect(cond.Message).To(Equal("Waiting for PersistentVolumeClaim deletion default/cvi.")) + }) + + It("sets Deleting condition when protection finalizer blocks deletion", func() { + scheme := runtime.NewScheme() + Expect(v1alpha2.AddToScheme(scheme)).To(Succeed()) + + now := metav1.Now() + cvi := &v1alpha2.ClusterVirtualImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cvi", + DeletionTimestamp: &now, + Finalizers: []string{v1alpha2.FinalizerCVIProtection}, + }, + } + vm := &v1alpha2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vm-a", + Namespace: "default", + }, + Status: v1alpha2.VirtualMachineStatus{ + Phase: v1alpha2.MachineRunning, + BlockDeviceRefs: []v1alpha2.BlockDeviceStatusRef{ + {Kind: v1alpha2.ClusterImageDevice, Name: "cvi"}, + }, + }, + } + + client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vm).Build() + handler := NewDeletionHandler(source.NewSources(), client) + result, err := handler.Handle(context.Background(), cvi) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeZero()) + + cond, ok := conditions.GetCondition(cvicondition.DeletingType, cvi.Status.Conditions) + Expect(ok).To(BeTrue()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(cvicondition.DeletionBlockedByProtection.String())) + Expect(cond.Message).To(Equal("The ClusterVirtualImage is protected from deletion because it is attached to VirtualMachine default/vm-a.")) + }) +}) + +type cviSourceHandler struct { + cleanUp func(context.Context, *v1alpha2.ClusterVirtualImage) (bool, string, error) +} + +func (h cviSourceHandler) Sync(context.Context, *v1alpha2.ClusterVirtualImage) (reconcile.Result, error) { + return reconcile.Result{}, nil +} + +func (h cviSourceHandler) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) { + return h.cleanUp(ctx, cvi) +} + +func (h cviSourceHandler) Validate(context.Context, *v1alpha2.ClusterVirtualImage) error { + return nil +} diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/cvi/internal/life_cycle.go index b88d972a07..3dcd1ec1c2 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/life_cycle.go @@ -83,7 +83,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, cvi *v1alpha2.ClusterVirtu ObservedGeneration: cvi.Status.ObservedGeneration, } - _, err := h.sources.CleanUp(ctx, cvi) + _, _, err := h.sources.CleanUp(ctx, cvi) if err != nil { return reconcile.Result{}, err } diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/http.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/http.go index d42aed6db9..a89204ce74 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/http.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/http.go @@ -100,7 +100,7 @@ func (ds HTTPDataSource) Sync(ctx context.Context, cvi *v1alpha2.ClusterVirtualI return reconcile.Result{}, err } - _, err = CleanUp(ctx, cvi, ds) + _, _, err = CleanUp(ctx, cvi, ds) if err != nil { return reconcile.Result{}, err } @@ -220,15 +220,15 @@ func (ds HTTPDataSource) Sync(ctx context.Context, cvi *v1alpha2.ClusterVirtualI return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds HTTPDataSource) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, error) { +func (ds HTTPDataSource) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.CVIShortName, cvi.Name, ds.controllerNamespace, cvi.UID) - requeue, err := ds.importerService.CleanUp(ctx, supgen) + requeue, reason, err := ds.importerService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return requeue, nil + return requeue, reason, nil } func (ds HTTPDataSource) Validate(_ context.Context, _ *v1alpha2.ClusterVirtualImage) error { 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 319c54e031..9f462b5e98 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/interfaces.go @@ -38,8 +38,8 @@ import ( type Importer interface { Start(ctx context.Context, settings *importer.Settings, obj client.Object, sup supplements.Generator, caBundle *datasource.CABundle, opts ...service.Option) error StartWithPodSetting(ctx context.Context, settings *importer.Settings, sup supplements.Generator, caBundle *datasource.CABundle, podSettings *importer.PodSettings, opts ...service.Option) error - CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) - CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) + CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) GetPod(ctx context.Context, sup supplements.Generator) (*corev1.Pod, error) DeletePod(ctx context.Context, obj client.Object, controllerName string, sup supplements.Generator) (bool, error) Protect(ctx context.Context, pod *corev1.Pod, sup supplements.Generator) error @@ -49,7 +49,7 @@ type Importer interface { type Uploader interface { Start(ctx context.Context, settings *uploader.Settings, obj client.Object, sup supplements.Generator, caBundle *datasource.CABundle, opts ...service.Option) error - CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) GetPod(ctx context.Context, sup supplements.Generator) (*corev1.Pod, error) GetIngress(ctx context.Context, sup supplements.Generator) (*netv1.Ingress, error) GetService(ctx context.Context, sup supplements.Generator) (*corev1.Service, error) 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 9aacfa0a69..0b71198dda 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/mock.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/mock.go @@ -29,10 +29,10 @@ var _ Importer = &ImporterMock{} // // // make and configure a mocked Importer // mockedImporter := &ImporterMock{ -// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUp method") // }, -// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUpSupplements method") // }, // DeletePodFunc: func(ctx context.Context, obj client.Object, controllerName string, sup supplements.Generator) (bool, error) { @@ -64,10 +64,10 @@ var _ Importer = &ImporterMock{} // } type ImporterMock struct { // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // CleanUpSupplementsFunc mocks the CleanUpSupplements method. - CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // DeletePodFunc mocks the DeletePod method. DeletePodFunc func(ctx context.Context, obj client.Object, controllerName string, sup supplements.Generator) (bool, error) @@ -196,7 +196,7 @@ type ImporterMock struct { } // CleanUp calls CleanUpFunc. -func (mock *ImporterMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *ImporterMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpFunc == nil { panic("ImporterMock.CleanUpFunc: method is nil but Importer.CleanUp was just called") } @@ -232,7 +232,7 @@ func (mock *ImporterMock) CleanUpCalls() []struct { } // CleanUpSupplements calls CleanUpSupplementsFunc. -func (mock *ImporterMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *ImporterMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpSupplementsFunc == nil { panic("ImporterMock.CleanUpSupplementsFunc: method is nil but Importer.CleanUpSupplements was just called") } @@ -585,7 +585,7 @@ var _ Uploader = &UploaderMock{} // // // make and configure a mocked Uploader // mockedUploader := &UploaderMock{ -// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUp method") // }, // EnsureIngressFunc: func(ctx context.Context, obj client.Object, sup supplements.Generator) (*netv1.Ingress, error) { @@ -629,7 +629,7 @@ var _ Uploader = &UploaderMock{} // } type UploaderMock struct { // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // EnsureIngressFunc mocks the EnsureIngress method. EnsureIngressFunc func(ctx context.Context, obj client.Object, sup supplements.Generator) (*netv1.Ingress, error) @@ -782,7 +782,7 @@ type UploaderMock struct { } // CleanUp calls CleanUpFunc. -func (mock *UploaderMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *UploaderMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpFunc == nil { panic("UploaderMock.CleanUpFunc: method is nil but Uploader.CleanUp was just called") } 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 1db6dffcb5..6ff652bdfb 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 @@ -160,7 +160,7 @@ func (ds ObjectRefDataSource) Sync(ctx context.Context, cvi *v1alpha2.ClusterVir return reconcile.Result{}, err } - _, err = CleanUp(ctx, cvi, ds) + _, _, err = CleanUp(ctx, cvi, ds) if err != nil { return reconcile.Result{}, err } @@ -294,25 +294,25 @@ func (ds ObjectRefDataSource) Sync(ctx context.Context, cvi *v1alpha2.ClusterVir return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds ObjectRefDataSource) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, error) { - viRefResult, err := ds.viOnPvcSyncer.CleanUp(ctx, cvi) +func (ds ObjectRefDataSource) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) { + viRefRequeue, viRefReason, err := ds.viOnPvcSyncer.CleanUp(ctx, cvi) if err != nil { - return false, err + return false, "", err } - vdRefResult, err := ds.vdSyncer.CleanUp(ctx, cvi) + vdRefRequeue, vdRefReason, err := ds.vdSyncer.CleanUp(ctx, cvi) if err != nil { - return false, err + return false, "", err } supgen := supplements.NewGenerator(annotations.CVIShortName, cvi.Name, ds.controllerNamespace, cvi.UID) - objRefRequeue, err := ds.importerService.CleanUp(ctx, supgen) + objRefRequeue, objRefReason, err := ds.importerService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return viRefResult || vdRefResult || objRefRequeue, nil + return viRefRequeue || vdRefRequeue || objRefRequeue, service.MergeCleanUpReasons(viRefReason, vdRefReason, objRefReason), nil } func (ds ObjectRefDataSource) Validate(ctx context.Context, cvi *v1alpha2.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 72159c783d..e5df8bbe8d 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 @@ -91,7 +91,7 @@ func (ds ObjectRefVirtualDisk) Sync(ctx context.Context, cvi *v1alpha2.ClusterVi return reconcile.Result{}, err } - _, err = CleanUp(ctx, cvi, ds) + _, _, err = CleanUp(ctx, cvi, ds) if err != nil { return reconcile.Result{}, err } @@ -216,9 +216,17 @@ func (ds ObjectRefVirtualDisk) Sync(ctx context.Context, cvi *v1alpha2.ClusterVi return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds ObjectRefVirtualDisk) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, error) { +func (ds ObjectRefVirtualDisk) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.CVIShortName, cvi.Name, ds.controllerNamespace, cvi.UID) - return ds.importerService.DeletePod(ctx, cvi, controllerName, supgen) + deleted, err := ds.importerService.DeletePod(ctx, cvi, controllerName, supgen) + if err != nil { + return false, "", err + } + if !deleted { + return false, "", nil + } + + return true, "waiting for object-ref importer Pod deletion", nil } func (ds ObjectRefVirtualDisk) getEnvSettings(cvi *v1alpha2.ClusterVirtualImage, sup supplements.Generator, volumeMode *corev1.PersistentVolumeMode) *importer.Settings { diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vdsnapshot.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vdsnapshot.go index 8db4ab7abe..970a2e4ae7 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vdsnapshot.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vdsnapshot.go @@ -319,42 +319,42 @@ func (ds ObjectRefVirtualDiskSnapshot) Sync(ctx context.Context, cvi *v1alpha2.C func (ds ObjectRefVirtualDiskSnapshot) CleanUpSupplements(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (reconcile.Result, error) { supgen := supplements.NewGenerator(annotations.CVIShortName, cvi.Name, cvi.Spec.DataSource.ObjectRef.Namespace, cvi.UID) - importerRequeue, err := ds.importerService.CleanUpSupplements(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - diskRequeue, err := ds.diskService.CleanUpSupplements(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - pvcCleanupRequeue, err := ds.diskService.CleanUp(ctx, supgen) + pvcCleanupRequeue, pvcCleanupReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { return reconcile.Result{}, err } - if importerRequeue || diskRequeue || pvcCleanupRequeue { + if reason := service.MergeCleanUpReasons(importerReason, diskReason, pvcCleanupReason); reason != "" || importerRequeue || diskRequeue || pvcCleanupRequeue { return reconcile.Result{RequeueAfter: time.Second}, nil } else { return reconcile.Result{}, nil } } -func (ds ObjectRefVirtualDiskSnapshot) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, error) { +func (ds ObjectRefVirtualDiskSnapshot) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.CVIShortName, cvi.Name, cvi.Spec.DataSource.ObjectRef.Namespace, cvi.UID) - importerRequeue, err := ds.importerService.CleanUp(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - diskRequeue, err := ds.diskService.CleanUp(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return importerRequeue || diskRequeue, nil + return importerRequeue || diskRequeue, service.MergeCleanUpReasons(importerReason, diskReason), nil } func (ds ObjectRefVirtualDiskSnapshot) getEnvSettings(cvi *v1alpha2.ClusterVirtualImage, sup supplements.Generator, volumeMode *corev1.PersistentVolumeMode) *importer.Settings { 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 43fee2c235..794bae32fe 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 @@ -85,7 +85,7 @@ func (ds ObjectRefVirtualImageOnPvc) Sync(ctx context.Context, cvi *v1alpha2.Clu return reconcile.Result{}, err } - _, err = CleanUp(ctx, cvi, ds) + _, _, err = CleanUp(ctx, cvi, ds) if err != nil { return reconcile.Result{}, err } @@ -205,9 +205,17 @@ func (ds ObjectRefVirtualImageOnPvc) Sync(ctx context.Context, cvi *v1alpha2.Clu return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds ObjectRefVirtualImageOnPvc) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, error) { +func (ds ObjectRefVirtualImageOnPvc) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.CVIShortName, cvi.Name, cvi.Namespace, cvi.UID) - return ds.importerService.DeletePod(ctx, cvi, controllerName, supgen) + deleted, err := ds.importerService.DeletePod(ctx, cvi, controllerName, supgen) + if err != nil { + return false, "", err + } + if !deleted { + return false, "", nil + } + + return true, "waiting for object-ref importer Pod deletion", nil } func (ds ObjectRefVirtualImageOnPvc) getEnvSettings(cvi *v1alpha2.ClusterVirtualImage, sup supplements.Generator) *importer.Settings { diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/registry.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/registry.go index 9bf03d71a7..04edc1012b 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/registry.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/registry.go @@ -106,7 +106,7 @@ func (ds RegistryDataSource) Sync(ctx context.Context, cvi *v1alpha2.ClusterVirt return reconcile.Result{}, err } - _, err = CleanUp(ctx, cvi, ds) + _, _, err = CleanUp(ctx, cvi, ds) if err != nil { return reconcile.Result{}, err } @@ -224,7 +224,7 @@ func (ds RegistryDataSource) Sync(ctx context.Context, cvi *v1alpha2.ClusterVirt return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds RegistryDataSource) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, error) { +func (ds RegistryDataSource) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.CVIShortName, cvi.Name, ds.controllerNamespace, cvi.UID) return ds.importerService.CleanUp(ctx, supgen) diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/sources.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/sources.go index 744b915c9a..b14324e7ba 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/sources.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/sources.go @@ -33,7 +33,7 @@ import ( type Handler interface { Sync(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (reconcile.Result, error) - CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, error) + CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) Validate(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) error } @@ -60,31 +60,33 @@ func (s Sources) Changed(_ context.Context, cvi *v1alpha2.ClusterVirtualImage) b return cvi.Generation != cvi.Status.ObservedGeneration } -func (s Sources) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, error) { +func (s Sources) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) { var requeue bool + var reason string for _, source := range s.sources { - ok, err := source.CleanUp(ctx, cvi) + sourceRequeue, sourceReason, err := source.CleanUp(ctx, cvi) if err != nil { - return false, err + return false, "", err } - requeue = requeue || ok + requeue = requeue || sourceRequeue + reason = service.MergeCleanUpReasons(reason, sourceReason) } - return requeue, nil + return requeue, reason, nil } type Cleaner interface { - CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, error) + CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) } -func CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage, c Cleaner) (bool, error) { +func CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage, c Cleaner) (bool, string, error) { if object.ShouldCleanupSubResources(cvi) { return c.CleanUp(ctx, cvi) } - return false, nil + return false, "", nil } func isDiskProvisioningFinished(c metav1.Condition) bool { diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/upload.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/upload.go index 6391d7779a..fe1f611ec5 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/upload.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/upload.go @@ -139,7 +139,7 @@ func (ds UploadDataSource) Sync(ctx context.Context, cvi *v1alpha2.ClusterVirtua return reconcile.Result{}, err } - _, err = CleanUp(ctx, cvi, ds) + _, _, err = CleanUp(ctx, cvi, ds) if err != nil { return reconcile.Result{}, err } @@ -279,7 +279,7 @@ func (ds UploadDataSource) Sync(ctx context.Context, cvi *v1alpha2.ClusterVirtua return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds UploadDataSource) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, error) { +func (ds UploadDataSource) CleanUp(ctx context.Context, cvi *v1alpha2.ClusterVirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.CVIShortName, cvi.Name, ds.controllerNamespace, cvi.UID) return ds.uploaderService.CleanUp(ctx, supgen) diff --git a/images/virtualization-artifact/pkg/controller/service/bounder_service.go b/images/virtualization-artifact/pkg/controller/service/bounder_service.go index a6b7fffb65..bdfc90e915 100644 --- a/images/virtualization-artifact/pkg/controller/service/bounder_service.go +++ b/images/virtualization-artifact/pkg/controller/service/bounder_service.go @@ -79,32 +79,32 @@ func (s BounderPodService) Start(ctx context.Context, ownerRef *metav1.OwnerRefe return nil } -func (s BounderPodService) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (s BounderPodService) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { return s.CleanUpSupplements(ctx, sup) } -func (s BounderPodService) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (s BounderPodService) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { pod, err := s.GetPod(ctx, sup) if err != nil { - return false, err + return false, "", err } err = s.protection.RemoveProtection(ctx, pod) if err != nil { - return false, err + return false, "", err } - var hasDeleted bool + var reason string if pod != nil { - hasDeleted = true + reason = CleanUpReasonForObject("waiting for bounder Pod deletion", pod) err = s.client.Delete(ctx, pod) if err != nil && !k8serrors.IsNotFound(err) { - return false, err + return false, "", err } } - return hasDeleted, nil + return reason != "", reason, nil } func (s BounderPodService) GetPod(ctx context.Context, sup supplements.Generator) (*corev1.Pod, error) { diff --git a/images/virtualization-artifact/pkg/controller/service/cleanup.go b/images/virtualization-artifact/pkg/controller/service/cleanup.go new file mode 100644 index 0000000000..5e3e5e0c06 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/service/cleanup.go @@ -0,0 +1,52 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "fmt" + "strings" + + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func CleanUpReasonForObject(action string, obj client.Object) string { + if obj == nil { + return "" + } + + return fmt.Sprintf("%s %s/%s", action, obj.GetNamespace(), obj.GetName()) +} + +func MergeCleanUpReasons(reasons ...string) string { + var merged []string + seen := make(map[string]struct{}, len(reasons)) + + for _, reason := range reasons { + if reason == "" { + continue + } + + if _, ok := seen[reason]; ok { + continue + } + + seen[reason] = struct{}{} + merged = append(merged, reason) + } + + return strings.Join(merged, "; ") +} diff --git a/images/virtualization-artifact/pkg/controller/service/cleanup_test.go b/images/virtualization-artifact/pkg/controller/service/cleanup_test.go new file mode 100644 index 0000000000..66ec38ebc4 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/service/cleanup_test.go @@ -0,0 +1,35 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Cleanup", func() { + Describe("MergeCleanUpReasons", func() { + It("skips empty reasons and keeps unique reasons in order", func() { + Expect(MergeCleanUpReasons( + "", + "waiting for PersistentVolumeClaim deletion default/vd", + "waiting for PersistentVolumeClaim deletion default/vd", + "waiting for Pod deletion default/importer", + )).To(Equal("waiting for PersistentVolumeClaim deletion default/vd; waiting for Pod deletion default/importer")) + }) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/service/disk_service.go b/images/virtualization-artifact/pkg/controller/service/disk_service.go index ee6651540b..7e1820d7af 100644 --- a/images/virtualization-artifact/pkg/controller/service/disk_service.go +++ b/images/virtualization-artifact/pkg/controller/service/disk_service.go @@ -207,41 +207,41 @@ func (s DiskService) CreatePersistentVolumeClaim(ctx context.Context, pvc *corev return nil } -func (s DiskService) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { - subResourcesHaveDeleted, err := s.CleanUpSupplements(ctx, sup) +func (s DiskService) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { + subResourcesRequeue, subResourcesReason, err := s.CleanUpSupplements(ctx, sup) if err != nil { - return false, err + return false, "", err } pvc, err := s.GetPersistentVolumeClaim(ctx, sup) if err != nil { - return false, err + return false, "", err } - var resourcesHaveDeleted bool + var reason string if pvc != nil { - resourcesHaveDeleted = true + reason = CleanUpReasonForObject("waiting for PersistentVolumeClaim deletion", pvc) err = s.protection.RemoveProtection(ctx, pvc) if err != nil { - return false, err + return false, "", err } err = s.client.Delete(ctx, pvc) if err != nil && !k8serrors.IsNotFound(err) { - return false, err + return false, "", err } } - return resourcesHaveDeleted || subResourcesHaveDeleted, nil + return reason != "" || subResourcesRequeue, MergeCleanUpReasons(reason, subResourcesReason), nil } -func (s DiskService) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (s DiskService) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { // 1. Update owner ref of pvc. pvc, err := s.GetPersistentVolumeClaim(ctx, sup) if err != nil { - return false, err + return false, "", err } if pvc != nil { @@ -253,7 +253,7 @@ func (s DiskService) CleanUpSupplements(ctx context.Context, sup supplements.Gen pvc.OwnerReferences = ownerReferences err = s.client.Update(ctx, pvc) if err != nil && !k8serrors.IsNotFound(err) { - return false, fmt.Errorf("update owner ref of pvc: %w", err) + return false, "", fmt.Errorf("update owner ref of pvc: %w", err) } } } @@ -261,43 +261,47 @@ func (s DiskService) CleanUpSupplements(ctx context.Context, sup supplements.Gen // 2. Delete network policy. networkPolicy, err := networkpolicy.GetNetworkPolicy(ctx, s.client, sup.LegacyDataVolume(), sup) if err != nil { - return false, err + return false, "", err } + var networkPolicyReason string if networkPolicy != nil { + networkPolicyReason = CleanUpReasonForObject("waiting for NetworkPolicy deletion", networkPolicy) + err = s.protection.RemoveProtection(ctx, networkPolicy) if err != nil { - return false, fmt.Errorf("remove protection from network policy: %w", err) + return false, "", fmt.Errorf("remove protection from network policy: %w", err) } err = s.client.Delete(ctx, networkPolicy) if err != nil && !k8serrors.IsNotFound(err) { - return false, fmt.Errorf("delete network policy: %w", err) + return false, "", fmt.Errorf("delete network policy: %w", err) } } // 3. Delete DataVolume. - var hasDeleted bool + var reason string dv, err := s.GetDataVolume(ctx, sup) if err != nil { - return false, fmt.Errorf("get dv: %w", err) + return false, "", fmt.Errorf("get dv: %w", err) } if dv != nil { err = s.protection.RemoveProtection(ctx, dv) if err != nil { - return false, fmt.Errorf("remove protection from dv: %w", err) + return false, "", fmt.Errorf("remove protection from dv: %w", err) } err = s.client.Delete(ctx, dv) if err != nil && !k8serrors.IsNotFound(err) { - return false, fmt.Errorf("delete dv: %w", err) + return false, "", fmt.Errorf("delete dv: %w", err) } - hasDeleted = true + reason = CleanUpReasonForObject("waiting for DataVolume deletion", dv) } - return hasDeleted, supplements.CleanupForDataVolume(ctx, s.client, sup, s.dvcrSettings) + mergedReason := MergeCleanUpReasons(networkPolicyReason, reason) + return mergedReason != "", mergedReason, supplements.CleanupForDataVolume(ctx, s.client, sup, s.dvcrSettings) } func (s DiskService) Protect(ctx context.Context, sup supplements.Generator, owner client.Object, dv *cdiv1.DataVolume, pvc *corev1.PersistentVolumeClaim) error { diff --git a/images/virtualization-artifact/pkg/controller/service/importer_service.go b/images/virtualization-artifact/pkg/controller/service/importer_service.go index 13e23b09bc..6d5826622f 100644 --- a/images/virtualization-artifact/pkg/controller/service/importer_service.go +++ b/images/virtualization-artifact/pkg/controller/service/importer_service.go @@ -121,7 +121,7 @@ func (s ImporterService) StartWithPodSetting( return supplements.EnsureForPod(ctx, s.client, sup, pod, caBundle, s.dvcrSettings, importerTokenScope(s.dvcrSettings, settings)) } -func (s ImporterService) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (s ImporterService) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { return s.CleanUpSupplements(ctx, sup) } @@ -170,45 +170,49 @@ func (s ImporterService) DeletePod(ctx context.Context, obj client.Object, contr return false, nil } -func (s ImporterService) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (s ImporterService) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { networkPolicy, err := networkpolicy.GetNetworkPolicy(ctx, s.client, sup.LegacyImporterPod(), sup) if err != nil { - return false, err + return false, "", err } + var networkPolicyReason string if networkPolicy != nil { + networkPolicyReason = CleanUpReasonForObject("waiting for NetworkPolicy deletion", networkPolicy) + err = s.protection.RemoveProtection(ctx, networkPolicy) if err != nil { - return false, err + return false, "", err } err = s.client.Delete(ctx, networkPolicy) if err != nil && !k8serrors.IsNotFound(err) { - return false, err + return false, "", err } } pod, err := s.GetPod(ctx, sup) if err != nil { - return false, err + return false, "", err } err = s.protection.RemoveProtection(ctx, pod) if err != nil { - return false, err + return false, "", err } - var hasDeleted bool + var podReason string if pod != nil { - hasDeleted = true + podReason = CleanUpReasonForObject("waiting for importer Pod deletion", pod) err = s.client.Delete(ctx, pod) if err != nil && !k8serrors.IsNotFound(err) { - return false, err + return false, "", err } } - return hasDeleted, nil + reason := MergeCleanUpReasons(networkPolicyReason, podReason) + return reason != "", reason, nil } func (s ImporterService) Protect(ctx context.Context, pod *corev1.Pod, sup supplements.Generator) (err error) { diff --git a/images/virtualization-artifact/pkg/controller/service/uploader_service.go b/images/virtualization-artifact/pkg/controller/service/uploader_service.go index 73335d983f..d4d36709b1 100644 --- a/images/virtualization-artifact/pkg/controller/service/uploader_service.go +++ b/images/virtualization-artifact/pkg/controller/service/uploader_service.go @@ -114,68 +114,69 @@ func (s UploaderService) Start( return supplements.EnsureForIngress(ctx, s.client, sup, ing, s.dvcrSettings) } -func (s UploaderService) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (s UploaderService) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { return s.CleanUpSupplements(ctx, sup) } -func (s UploaderService) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (s UploaderService) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { pod, err := s.GetPod(ctx, sup) if err != nil { - return false, err + return false, "", err } svc, err := s.GetService(ctx, sup) if err != nil { - return false, err + return false, "", err } ing, err := s.GetIngress(ctx, sup) if err != nil { - return false, err + return false, "", err } networkPolicy, err := networkpolicy.GetNetworkPolicy(ctx, s.client, sup.LegacyUploaderPod(), sup) if err != nil { - return false, err + return false, "", err } err = s.protection.RemoveProtection(ctx, pod, svc, ing, networkPolicy) if err != nil { - return false, err + return false, "", err } - var haveDeleted bool + var reasons []string if pod != nil { - haveDeleted = true + reasons = append(reasons, CleanUpReasonForObject("waiting for uploader Pod deletion", pod)) err = s.client.Delete(ctx, pod) if err != nil && !k8serrors.IsNotFound(err) { - return false, err + return false, "", err } } if networkPolicy != nil { - haveDeleted = true + reasons = append(reasons, CleanUpReasonForObject("waiting for NetworkPolicy deletion", networkPolicy)) err = s.client.Delete(ctx, networkPolicy) if err != nil && !k8serrors.IsNotFound(err) { - return false, err + return false, "", err } } if svc != nil { - haveDeleted = true + reasons = append(reasons, CleanUpReasonForObject("waiting for Service deletion", svc)) err = s.client.Delete(ctx, svc) if err != nil && !k8serrors.IsNotFound(err) { - return false, err + return false, "", err } } if ing != nil { - haveDeleted = true + reasons = append(reasons, CleanUpReasonForObject("waiting for Ingress deletion", ing)) err = s.client.Delete(ctx, ing) if err != nil && !k8serrors.IsNotFound(err) { - return false, err + return false, "", err } } - return haveDeleted, nil + reason := MergeCleanUpReasons(reasons...) + return reason != "", reason, nil } func (s UploaderService) Protect(ctx context.Context, sup supplements.Generator, pod *corev1.Pod, svc *corev1.Service, ing *netv1.Ingress) (err error) { diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/deletion.go b/images/virtualization-artifact/pkg/controller/vd/internal/deletion.go index eb4e008726..d1b8012d68 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/deletion.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/deletion.go @@ -19,15 +19,22 @@ package internal import ( "context" "errors" + "log/slog" + "sort" + "strings" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vd/internal/source" "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" ) const deletionHandlerName = "DeletionHandler" @@ -49,15 +56,25 @@ func (h DeletionHandler) Handle(ctx context.Context, vd *v1alpha2.VirtualDisk) ( if vd.DeletionTimestamp != nil { if controllerutil.ContainsFinalizer(vd, v1alpha2.FinalizerVDProtection) { + h.setDeletingCondition( + vd, + vdcondition.DeletionBlockedByProtection, + deletionBlockedByProtectionMessage(vd), + ) return reconcile.Result{}, nil } - requeue, err := h.sources.CleanUp(ctx, vd) + requeue, reason, err := h.sources.CleanUp(ctx, vd) if err != nil { return reconcile.Result{}, err } if requeue { + if reason == "" { + reason = "Waiting for cleanup to finish" + } + h.setDeletingCondition(vd, vdcondition.DeletionCleanupPending, reason) + log.Info("VirtualDisk cleanup is pending", slog.String("reason", reason)) return reconcile.Result{RequeueAfter: time.Second}, nil } @@ -65,15 +82,28 @@ func (h DeletionHandler) Handle(ctx context.Context, vd *v1alpha2.VirtualDisk) ( return reconcile.Result{}, err } + conditions.RemoveCondition(vdcondition.DeletingType, &vd.Status.Conditions) log.Info("Deletion observed: remove cleanup finalizer from VirtualDisk") controllerutil.RemoveFinalizer(vd, v1alpha2.FinalizerVDCleanup) return reconcile.Result{}, nil } + conditions.RemoveCondition(vdcondition.DeletingType, &vd.Status.Conditions) controllerutil.AddFinalizer(vd, v1alpha2.FinalizerVDCleanup) return reconcile.Result{}, nil } +func (h DeletionHandler) setDeletingCondition(vd *v1alpha2.VirtualDisk, reason vdcondition.DeletingReason, message string) { + conditions.SetCondition( + conditions.NewConditionBuilder(vdcondition.DeletingType). + Generation(vd.Generation). + Status(metav1.ConditionFalse). + Reason(reason). + Message(service.CapitalizeFirstLetter(message)+"."), + &vd.Status.Conditions, + ) +} + func (h DeletionHandler) cleanupPersistentVolumeClaims(ctx context.Context, vd *v1alpha2.VirtualDisk) error { pvcs, err := listPersistentVolumeClaims(ctx, vd, h.client) if err != nil { @@ -91,3 +121,23 @@ func (h DeletionHandler) cleanupPersistentVolumeClaims(ctx context.Context, vd * return errs } + +func deletionBlockedByProtectionMessage(vd *v1alpha2.VirtualDisk) string { + mountedVMs := make([]string, 0, len(vd.Status.AttachedToVirtualMachines)) + for _, vm := range vd.Status.AttachedToVirtualMachines { + if vm.Mounted { + mountedVMs = append(mountedVMs, vm.Name) + } + } + + sort.Strings(mountedVMs) + + switch len(mountedVMs) { + case 0: + return "The VirtualDisk is protected from deletion by the protection finalizer" + case 1: + return "The VirtualDisk is protected from deletion because it is attached to VirtualMachine " + mountedVMs[0] + default: + return "The VirtualDisk is protected from deletion because it is attached to VirtualMachines: " + strings.Join(mountedVMs, ", ") + } +} diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/deletion_test.go b/images/virtualization-artifact/pkg/controller/vd/internal/deletion_test.go new file mode 100644 index 0000000000..db9e17827a --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vd/internal/deletion_test.go @@ -0,0 +1,99 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package internal + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/controller/vd/internal/source" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" +) + +var _ = Describe("DeletionHandler", func() { + It("sets Deleting condition when protection finalizer blocks deletion", func() { + now := metav1.Now() + vd := &v1alpha2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vd", + Namespace: "default", + DeletionTimestamp: &now, + Finalizers: []string{v1alpha2.FinalizerVDProtection}, + }, + Status: v1alpha2.VirtualDiskStatus{ + AttachedToVirtualMachines: []v1alpha2.AttachedVirtualMachine{ + {Name: "vm-a", Mounted: false}, + {Name: "vm-b", Mounted: true}, + }, + }, + } + + handler := NewDeletionHandler(source.NewSources(), nil) + result, err := handler.Handle(context.Background(), vd) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + + cond, ok := conditions.GetCondition(vdcondition.DeletingType, vd.Status.Conditions) + Expect(ok).To(BeTrue()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vdcondition.DeletionBlockedByProtection.String())) + Expect(cond.Message).To(Equal("The VirtualDisk is protected from deletion because it is attached to VirtualMachine vm-b.")) + }) + + It("sets Deleting condition with cleanup pending reason", func() { + now := metav1.Now() + vd := &v1alpha2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vd", + Namespace: "default", + DeletionTimestamp: &now, + }, + Spec: v1alpha2.VirtualDiskSpec{ + DataSource: &v1alpha2.VirtualDiskDataSource{ + Type: v1alpha2.DataSourceTypeHTTP, + }, + }, + } + + sources := source.NewSources() + sources.Set(v1alpha2.DataSourceTypeHTTP, &source.HandlerMock{ + CleanUpFunc: func(context.Context, *v1alpha2.VirtualDisk) (bool, string, error) { + return true, "waiting for PersistentVolumeClaim deletion default/vd", nil + }, + }) + + handler := NewDeletionHandler(sources, nil) + result, err := handler.Handle(context.Background(), vd) + + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(time.Second)) + + cond, ok := conditions.GetCondition(vdcondition.DeletingType, vd.Status.Conditions) + Expect(ok).To(BeTrue()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vdcondition.DeletionCleanupPending.String())) + Expect(cond.Message).To(Equal("Waiting for PersistentVolumeClaim deletion default/vd.")) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vd/internal/interfaces.go index 7f4bb44e89..7f806cee9b 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/interfaces.go @@ -35,7 +35,7 @@ type Handler = source.Handler type Sources interface { Changed(_ context.Context, vi *v1alpha2.VirtualDisk) bool Get(dsType v1alpha2.DataSourceType) (source.Handler, bool) - CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) + CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) } type DiskService interface { diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go b/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go index 0994617bc4..c4116f5691 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go @@ -102,16 +102,6 @@ func (h InUseHandler) Handle(ctx context.Context, vd *v1alpha2.VirtualDisk) (rec return reconcile.Result{}, nil } -func (h InUseHandler) isVDAttachedToVM(vdName string, vm v1alpha2.VirtualMachine) bool { - for _, bda := range vm.Status.BlockDeviceRefs { - if bda.Kind == v1alpha2.DiskDevice && bda.Name == vdName { - return true - } - } - - return false -} - func (h InUseHandler) checkDataExportUsage(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { pvcName := vd.Status.Target.PersistentVolumeClaim if pvcName == "" { @@ -171,25 +161,16 @@ func (h InUseHandler) getVirtualMachineUsageMap(ctx context.Context, vd *v1alpha usageMap := make(map[string]bool) for _, vm := range vms.Items { - if !h.isVDAttachedToVM(vd.GetName(), vm) { - continue + referenced, mounted, err := commonvm.BlockDeviceUsage(ctx, h.client, vm, v1alpha2.DiskDevice, vd.GetName()) + if err != nil { + return nil, err } - switch vm.Status.Phase { - case "": - usageMap[vm.GetName()] = false - case v1alpha2.MachinePending: - usageMap[vm.GetName()] = true - case v1alpha2.MachineStopped: - vmIsActive, err := commonvm.IsVMActive(ctx, h.client, vm) - if err != nil { - return nil, err - } - - usageMap[vm.GetName()] = vmIsActive - default: - usageMap[vm.GetName()] = true + if !referenced { + continue } + + usageMap[vm.GetName()] = mounted } return usageMap, nil diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vd/internal/life_cycle.go index d9ea5290e8..e76ae098c6 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/life_cycle.go @@ -90,7 +90,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vd *v1alpha2.VirtualDisk) Phase: v1alpha2.DiskPending, } - _, err := h.sources.CleanUp(ctx, vd) + _, _, err := h.sources.CleanUp(ctx, vd) if err != nil { return reconcile.Result{}, fmt.Errorf("failed to clean up to restart import process: %w", err) } diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/life_cycle_test.go b/images/virtualization-artifact/pkg/controller/vd/internal/life_cycle_test.go index 08d13e55de..2aa4378207 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/life_cycle_test.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/life_cycle_test.go @@ -67,9 +67,9 @@ var _ = Describe("LifeCycleHandler Run", func() { }, } - sourcesMock.CleanUpFunc = func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { + sourcesMock.CleanUpFunc = func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { cleanUpCalled = true - return false, nil + return false, "", nil } sourcesMock.ChangedFunc = func(ctx context.Context, vd *v1alpha2.VirtualDisk) bool { @@ -168,9 +168,9 @@ var _ = Describe("LifeCycleHandler Run", func() { }, } - sourcesMock.CleanUpFunc = func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { + sourcesMock.CleanUpFunc = func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { cleanUpCalled = true - return false, nil + return false, "", nil } sourcesMock.ChangedFunc = func(ctx context.Context, vd *v1alpha2.VirtualDisk) bool { diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/mock.go b/images/virtualization-artifact/pkg/controller/vd/internal/mock.go index 1f183e3b62..2979705353 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/mock.go @@ -25,7 +25,7 @@ var _ Handler = &HandlerMock{} // // // make and configure a mocked Handler // mockedHandler := &HandlerMock{ -// CleanUpFunc: func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +// CleanUpFunc: func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { // panic("mock out the CleanUp method") // }, // NameFunc: func() string { @@ -45,7 +45,7 @@ var _ Handler = &HandlerMock{} // } type HandlerMock struct { // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) + CleanUpFunc func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) // NameFunc mocks the Name method. NameFunc func() string @@ -90,7 +90,7 @@ type HandlerMock struct { } // CleanUp calls CleanUpFunc. -func (mock *HandlerMock) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +func (mock *HandlerMock) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { if mock.CleanUpFunc == nil { panic("HandlerMock.CleanUpFunc: method is nil but Handler.CleanUp was just called") } @@ -237,7 +237,7 @@ var _ Sources = &SourcesMock{} // ChangedFunc: func(contextMoqParam context.Context, vi *v1alpha2.VirtualDisk) bool { // panic("mock out the Changed method") // }, -// CleanUpFunc: func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +// CleanUpFunc: func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { // panic("mock out the CleanUp method") // }, // GetFunc: func(dsType v1alpha2.DataSourceType) (source.Handler, bool) { @@ -254,7 +254,7 @@ type SourcesMock struct { ChangedFunc func(contextMoqParam context.Context, vi *v1alpha2.VirtualDisk) bool // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) + CleanUpFunc func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) // GetFunc mocks the Get method. GetFunc func(dsType v1alpha2.DataSourceType) (source.Handler, bool) @@ -323,7 +323,7 @@ func (mock *SourcesMock) ChangedCalls() []struct { } // CleanUp calls CleanUpFunc. -func (mock *SourcesMock) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +func (mock *SourcesMock) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { if mock.CleanUpFunc == nil { panic("SourcesMock.CleanUpFunc: method is nil but Sources.CleanUp was just called") } 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 a1e722f7d4..a12f00e2db 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/blank.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/blank.go @@ -80,15 +80,15 @@ func (ds BlankDataSource) Validate(_ context.Context, _ *v1alpha2.VirtualDisk) e return nil } -func (ds BlankDataSource) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +func (ds BlankDataSource) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { supgen := vdsupplements.NewGenerator(vd) - requeue, err := ds.diskService.CleanUp(ctx, supgen) + requeue, reason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return requeue, nil + return requeue, reason, nil } func (ds BlankDataSource) Name() string { diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/blank_test.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/blank_test.go index be858ba5c5..a1179b3f43 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/blank_test.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/blank_test.go @@ -73,8 +73,8 @@ var _ = Describe("Blank", func() { GetCapacityFunc: func(_ *corev1.PersistentVolumeClaim) string { return vd.Spec.PersistentVolumeClaim.Size.String() }, - CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, error) { - return false, nil + CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, string, error) { + return false, "", nil }, ProtectFunc: func(_ context.Context, _ supplements.Generator, _ client.Object, _ *cdiv1.DataVolume, _ *corev1.PersistentVolumeClaim) error { return nil 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 1eb228523c..4ee6240843 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/http.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/http.go @@ -328,20 +328,20 @@ func (ds HTTPDataSource) Sync(ctx context.Context, vd *v1alpha2.VirtualDisk) (re return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds HTTPDataSource) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +func (ds HTTPDataSource) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { supgen := vdsupplements.NewGenerator(vd) - importerRequeue, err := ds.importerService.CleanUp(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - diskRequeue, err := ds.diskService.CleanUp(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return importerRequeue || diskRequeue, nil + return importerRequeue || diskRequeue, service.MergeCleanUpReasons(importerReason, diskReason), nil } func (ds HTTPDataSource) Validate(_ context.Context, _ *v1alpha2.VirtualDisk) error { @@ -351,17 +351,17 @@ func (ds HTTPDataSource) Validate(_ context.Context, _ *v1alpha2.VirtualDisk) er func (ds HTTPDataSource) CleanUpSupplements(ctx context.Context, vd *v1alpha2.VirtualDisk) (reconcile.Result, error) { supgen := vdsupplements.NewGenerator(vd) - importerRequeue, err := ds.importerService.CleanUpSupplements(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - diskRequeue, err := ds.diskService.CleanUpSupplements(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - if importerRequeue || diskRequeue { + if reason := service.MergeCleanUpReasons(importerReason, diskReason); reason != "" || importerRequeue || diskRequeue { return reconcile.Result{RequeueAfter: time.Second}, nil } else { return reconcile.Result{}, nil diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/interfaces.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/interfaces.go index cd26fd373e..9b2431f643 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/interfaces.go @@ -31,7 +31,7 @@ import ( type Handler interface { Name() string Sync(ctx context.Context, vd *v1alpha2.VirtualDisk) (reconcile.Result, error) - CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) + CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) Validate(ctx context.Context, vd *v1alpha2.VirtualDisk) error } @@ -39,7 +39,7 @@ type BlankDataSourceDiskService interface { step.VolumeAndAccessModesGetter step.ReadyStepDiskService - CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) } type ObjectRefVirtualImageDiskService interface { diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/mock.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/mock.go index 7e1e92e2e9..da549dee64 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/mock.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/mock.go @@ -27,7 +27,7 @@ var _ Handler = &HandlerMock{} // // // make and configure a mocked Handler // mockedHandler := &HandlerMock{ -// CleanUpFunc: func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +// CleanUpFunc: func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { // panic("mock out the CleanUp method") // }, // NameFunc: func() string { @@ -47,7 +47,7 @@ var _ Handler = &HandlerMock{} // } type HandlerMock struct { // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) + CleanUpFunc func(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) // NameFunc mocks the Name method. NameFunc func() string @@ -92,7 +92,7 @@ type HandlerMock struct { } // CleanUp calls CleanUpFunc. -func (mock *HandlerMock) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +func (mock *HandlerMock) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { if mock.CleanUpFunc == nil { panic("HandlerMock.CleanUpFunc: method is nil but Handler.CleanUp was just called") } @@ -236,10 +236,10 @@ var _ BlankDataSourceDiskService = &BlankDataSourceDiskServiceMock{} // // // make and configure a mocked BlankDataSourceDiskService // mockedBlankDataSourceDiskService := &BlankDataSourceDiskServiceMock{ -// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUp method") // }, -// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUpSupplements method") // }, // GetCapacityFunc: func(pvc *corev1.PersistentVolumeClaim) string { @@ -259,10 +259,10 @@ var _ BlankDataSourceDiskService = &BlankDataSourceDiskServiceMock{} // } type BlankDataSourceDiskServiceMock struct { // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // CleanUpSupplementsFunc mocks the CleanUpSupplements method. - CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // GetCapacityFunc mocks the GetCapacity method. GetCapacityFunc func(pvc *corev1.PersistentVolumeClaim) string @@ -325,7 +325,7 @@ type BlankDataSourceDiskServiceMock struct { } // CleanUp calls CleanUpFunc. -func (mock *BlankDataSourceDiskServiceMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *BlankDataSourceDiskServiceMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpFunc == nil { panic("BlankDataSourceDiskServiceMock.CleanUpFunc: method is nil but BlankDataSourceDiskService.CleanUp was just called") } @@ -361,7 +361,7 @@ func (mock *BlankDataSourceDiskServiceMock) CleanUpCalls() []struct { } // CleanUpSupplements calls CleanUpSupplementsFunc. -func (mock *BlankDataSourceDiskServiceMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *BlankDataSourceDiskServiceMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpSupplementsFunc == nil { panic("BlankDataSourceDiskServiceMock.CleanUpSupplementsFunc: method is nil but BlankDataSourceDiskService.CleanUpSupplements was just called") } @@ -529,10 +529,10 @@ var _ ObjectRefVirtualImageDiskService = &ObjectRefVirtualImageDiskServiceMock{} // CheckProvisioningFunc: func(ctx context.Context, pvc *corev1.PersistentVolumeClaim) error { // panic("mock out the CheckProvisioning method") // }, -// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUp method") // }, -// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUpSupplements method") // }, // GetCapacityFunc: func(pvc *corev1.PersistentVolumeClaim) string { @@ -558,10 +558,10 @@ type ObjectRefVirtualImageDiskServiceMock struct { CheckProvisioningFunc func(ctx context.Context, pvc *corev1.PersistentVolumeClaim) error // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // CleanUpSupplementsFunc mocks the CleanUpSupplements method. - CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // GetCapacityFunc mocks the GetCapacity method. GetCapacityFunc func(pvc *corev1.PersistentVolumeClaim) string @@ -689,7 +689,7 @@ func (mock *ObjectRefVirtualImageDiskServiceMock) CheckProvisioningCalls() []str } // CleanUp calls CleanUpFunc. -func (mock *ObjectRefVirtualImageDiskServiceMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *ObjectRefVirtualImageDiskServiceMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpFunc == nil { panic("ObjectRefVirtualImageDiskServiceMock.CleanUpFunc: method is nil but ObjectRefVirtualImageDiskService.CleanUp was just called") } @@ -725,7 +725,7 @@ func (mock *ObjectRefVirtualImageDiskServiceMock) CleanUpCalls() []struct { } // CleanUpSupplements calls CleanUpSupplementsFunc. -func (mock *ObjectRefVirtualImageDiskServiceMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *ObjectRefVirtualImageDiskServiceMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpSupplementsFunc == nil { panic("ObjectRefVirtualImageDiskServiceMock.CleanUpSupplementsFunc: method is nil but ObjectRefVirtualImageDiskService.CleanUpSupplements was just called") } @@ -949,10 +949,10 @@ var _ ObjectRefClusterVirtualImageDiskService = &ObjectRefClusterVirtualImageDis // CheckProvisioningFunc: func(ctx context.Context, pvc *corev1.PersistentVolumeClaim) error { // panic("mock out the CheckProvisioning method") // }, -// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUp method") // }, -// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUpSupplements method") // }, // GetCapacityFunc: func(pvc *corev1.PersistentVolumeClaim) string { @@ -978,10 +978,10 @@ type ObjectRefClusterVirtualImageDiskServiceMock struct { CheckProvisioningFunc func(ctx context.Context, pvc *corev1.PersistentVolumeClaim) error // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // CleanUpSupplementsFunc mocks the CleanUpSupplements method. - CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // GetCapacityFunc mocks the GetCapacity method. GetCapacityFunc func(pvc *corev1.PersistentVolumeClaim) string @@ -1109,7 +1109,7 @@ func (mock *ObjectRefClusterVirtualImageDiskServiceMock) CheckProvisioningCalls( } // CleanUp calls CleanUpFunc. -func (mock *ObjectRefClusterVirtualImageDiskServiceMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *ObjectRefClusterVirtualImageDiskServiceMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpFunc == nil { panic("ObjectRefClusterVirtualImageDiskServiceMock.CleanUpFunc: method is nil but ObjectRefClusterVirtualImageDiskService.CleanUp was just called") } @@ -1145,7 +1145,7 @@ func (mock *ObjectRefClusterVirtualImageDiskServiceMock) CleanUpCalls() []struct } // CleanUpSupplements calls CleanUpSupplementsFunc. -func (mock *ObjectRefClusterVirtualImageDiskServiceMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *ObjectRefClusterVirtualImageDiskServiceMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpSupplementsFunc == nil { panic("ObjectRefClusterVirtualImageDiskServiceMock.CleanUpSupplementsFunc: method is nil but ObjectRefClusterVirtualImageDiskService.CleanUpSupplements was just called") } @@ -1366,7 +1366,7 @@ var _ ObjectRefVirtualDiskSnapshotDiskService = &ObjectRefVirtualDiskSnapshotDis // // // make and configure a mocked ObjectRefVirtualDiskSnapshotDiskService // mockedObjectRefVirtualDiskSnapshotDiskService := &ObjectRefVirtualDiskSnapshotDiskServiceMock{ -// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUpSupplements method") // }, // GetCapacityFunc: func(pvc *corev1.PersistentVolumeClaim) string { @@ -1383,7 +1383,7 @@ var _ ObjectRefVirtualDiskSnapshotDiskService = &ObjectRefVirtualDiskSnapshotDis // } type ObjectRefVirtualDiskSnapshotDiskServiceMock struct { // CleanUpSupplementsFunc mocks the CleanUpSupplements method. - CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // GetCapacityFunc mocks the GetCapacity method. GetCapacityFunc func(pvc *corev1.PersistentVolumeClaim) string @@ -1425,7 +1425,7 @@ type ObjectRefVirtualDiskSnapshotDiskServiceMock struct { } // CleanUpSupplements calls CleanUpSupplementsFunc. -func (mock *ObjectRefVirtualDiskSnapshotDiskServiceMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *ObjectRefVirtualDiskSnapshotDiskServiceMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpSupplementsFunc == nil { panic("ObjectRefVirtualDiskSnapshotDiskServiceMock.CleanUpSupplementsFunc: method is nil but ObjectRefVirtualDiskSnapshotDiskService.CleanUpSupplements was just called") } 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 a3586326cd..55a77b817f 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 @@ -68,15 +68,15 @@ func (ds ObjectRefDataSource) Sync(ctx context.Context, vd *v1alpha2.VirtualDisk return reconcile.Result{}, fmt.Errorf("unexpected object ref kind %s, please report a bug", vd.Spec.DataSource.ObjectRef.Kind) } -func (ds ObjectRefDataSource) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +func (ds ObjectRefDataSource) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { supgen := vdsupplements.NewGenerator(vd) - requeue, err := ds.diskService.CleanUp(ctx, supgen) + requeue, reason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return requeue, nil + return requeue, reason, nil } func (ds ObjectRefDataSource) Validate(ctx context.Context, vd *v1alpha2.VirtualDisk) error { diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_cvi_test.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_cvi_test.go index bba5d04a43..444fe5a9e9 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_cvi_test.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_cvi_test.go @@ -68,8 +68,8 @@ var _ = Describe("ObjectRef ClusterVirtualImage", func() { GetCapacityFunc: func(_ *corev1.PersistentVolumeClaim) string { return "100Mi" }, - CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, error) { - return false, nil + CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, string, error) { + return false, "", nil }, ProtectFunc: func(_ context.Context, _ supplements.Generator, _ client.Object, _ *cdiv1.DataVolume, _ *corev1.PersistentVolumeClaim) error { return nil diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_vdsnapshot_test.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_vdsnapshot_test.go index 446aedc7ae..e282c656e3 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_vdsnapshot_test.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_vdsnapshot_test.go @@ -79,8 +79,8 @@ var _ = Describe("ObjectRef VirtualDiskSnapshot", func() { GetCapacityFunc: func(_ *corev1.PersistentVolumeClaim) string { return "1Mi" }, - CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, error) { - return false, nil + CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, string, error) { + return false, "", nil }, ProtectFunc: func(_ context.Context, _ supplements.Generator, _ client.Object, _ *cdiv1.DataVolume, _ *corev1.PersistentVolumeClaim) error { return nil diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_vi_test.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_vi_test.go index 5eacabf594..9c4d12c447 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_vi_test.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/object_ref_vi_test.go @@ -68,8 +68,8 @@ var _ = Describe("ObjectRef VirtualImage", func() { GetCapacityFunc: func(_ *corev1.PersistentVolumeClaim) string { return "100Mi" }, - CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, error) { - return false, nil + CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, string, error) { + return false, "", nil }, ProtectFunc: func(_ context.Context, _ supplements.Generator, _ client.Object, _ *cdiv1.DataVolume, _ *corev1.PersistentVolumeClaim) error { return nil 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 59340dbaa6..1fe453b017 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/registry.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/registry.go @@ -334,36 +334,36 @@ func (ds RegistryDataSource) Sync(ctx context.Context, vd *v1alpha2.VirtualDisk) return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds RegistryDataSource) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +func (ds RegistryDataSource) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { supgen := vdsupplements.NewGenerator(vd) - importerRequeue, err := ds.importerService.CleanUp(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - diskRequeue, err := ds.diskService.CleanUp(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return importerRequeue || diskRequeue, nil + return importerRequeue || diskRequeue, service.MergeCleanUpReasons(importerReason, diskReason), nil } func (ds RegistryDataSource) CleanUpSupplements(ctx context.Context, vd *v1alpha2.VirtualDisk) (reconcile.Result, error) { supgen := vdsupplements.NewGenerator(vd) - importerRequeue, err := ds.importerService.CleanUpSupplements(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - diskRequeue, err := ds.diskService.CleanUpSupplements(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - if importerRequeue || diskRequeue { + if reason := service.MergeCleanUpReasons(importerReason, diskReason); reason != "" || importerRequeue || diskRequeue { return reconcile.Result{RequeueAfter: time.Second}, nil } else { return reconcile.Result{}, nil diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/sources.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/sources.go index be0fd05552..387a40ead1 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/sources.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/sources.go @@ -69,19 +69,21 @@ func (s Sources) Changed(_ context.Context, vd *v1alpha2.VirtualDisk) bool { return vd.Generation != vd.Status.ObservedGeneration } -func (s Sources) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +func (s Sources) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { var requeue bool + var reason string for _, source := range s.sources { - ok, err := source.CleanUp(ctx, vd) + sourceRequeue, sourceReason, err := source.CleanUp(ctx, vd) if err != nil { - return false, fmt.Errorf("clean up failed for data source %s: %w", source.Name(), err) + return false, "", fmt.Errorf("clean up failed for data source %s: %w", source.Name(), err) } - requeue = requeue || ok + requeue = requeue || sourceRequeue + reason = service.MergeCleanUpReasons(reason, sourceReason) } - return requeue, nil + return requeue, reason, nil } type SupplementsCleaner interface { @@ -293,7 +295,7 @@ func setPhaseConditionFromPodError( } type Cleaner interface { - CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) } func setPhaseConditionFromProvisioningError( @@ -326,7 +328,7 @@ func setPhaseConditionFromProvisioningError( if isChanged { supgen := vdsupplements.NewGenerator(vd) - _, err = cleaner.CleanUp(ctx, supgen) + _, _, err = cleaner.CleanUp(ctx, supgen) if err != nil { err = errors.Join(provisioningErr, err) setPhaseConditionToFailed(vd, cb, &vd.Status.Phase, err) diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/step/ensure_node_placement.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/step/ensure_node_placement.go index c0a9d3a881..9e675da9a7 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/step/ensure_node_placement.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/step/ensure_node_placement.go @@ -38,7 +38,7 @@ import ( type EnsureNodePlacementStepDiskService interface { CheckProvisioning(ctx context.Context, pvc *corev1.PersistentVolumeClaim) error - CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) } // EnsureNodePlacementStep supports changing the node placement only if the PVC is created using a DataVolume. @@ -104,7 +104,7 @@ func (s EnsureNodePlacementStep) Take(ctx context.Context, vd *v1alpha2.VirtualD supgen := vdsupplements.NewGenerator(vd) - _, err = s.disk.CleanUp(ctx, supgen) + _, _, err = s.disk.CleanUp(ctx, supgen) if err != nil { return nil, fmt.Errorf("clean up due to changes in the virtual machine tolerations: %w", err) } diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/source/step/ready_step.go b/images/virtualization-artifact/pkg/controller/vd/internal/source/step/ready_step.go index 093cd1e423..d8cdc7172b 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/step/ready_step.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/step/ready_step.go @@ -40,7 +40,7 @@ const readyStep = "ready" type ReadyStepDiskService interface { GetCapacity(pvc *corev1.PersistentVolumeClaim) string - CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) Protect(ctx context.Context, sup supplements.Generator, owner client.Object, dv *cdiv1.DataVolume, pvc *corev1.PersistentVolumeClaim) error } @@ -115,7 +115,7 @@ func (s ReadyStep) Take(ctx context.Context, vd *v1alpha2.VirtualDisk) (*reconci } if object.ShouldCleanupSubResources(vd) { - _, err = s.diskService.CleanUpSupplements(ctx, supgen) + _, _, err = s.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return nil, fmt.Errorf("clean up supplements: %w", err) } 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 b286ee3fa7..b26aae207d 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/source/upload.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/source/upload.go @@ -392,36 +392,36 @@ func (ds UploadDataSource) Sync(ctx context.Context, vd *v1alpha2.VirtualDisk) ( return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds UploadDataSource) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, error) { +func (ds UploadDataSource) CleanUp(ctx context.Context, vd *v1alpha2.VirtualDisk) (bool, string, error) { supgen := vdsupplements.NewGenerator(vd) - uploaderRequeue, err := ds.uploaderService.CleanUp(ctx, supgen) + uploaderRequeue, uploaderReason, err := ds.uploaderService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - diskRequeue, err := ds.diskService.CleanUp(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return uploaderRequeue || diskRequeue, nil + return uploaderRequeue || diskRequeue, service.MergeCleanUpReasons(uploaderReason, diskReason), nil } func (ds UploadDataSource) CleanUpSupplements(ctx context.Context, vd *v1alpha2.VirtualDisk) (reconcile.Result, error) { supgen := vdsupplements.NewGenerator(vd) - uploaderRequeue, err := ds.uploaderService.CleanUpSupplements(ctx, supgen) + uploaderRequeue, uploaderReason, err := ds.uploaderService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - diskRequeue, err := ds.diskService.CleanUpSupplements(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - if uploaderRequeue || diskRequeue { + if reason := service.MergeCleanUpReasons(uploaderReason, diskReason); reason != "" || uploaderRequeue || diskRequeue { return reconcile.Result{RequeueAfter: time.Second}, nil } else { return reconcile.Result{}, nil diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/attachee.go b/images/virtualization-artifact/pkg/controller/vi/internal/attachee.go index c35cf184a2..a06b5d48a3 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/attachee.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/attachee.go @@ -75,35 +75,15 @@ func (h AttacheeHandler) hasAttachedVM(ctx context.Context, vi client.Object) (b } for _, vm := range vms.Items { - if vm.Status.Phase == "" { - continue + _, mounted, err := commonvm.BlockDeviceUsage(ctx, h.client, vm, v1alpha2.ImageDevice, vi.GetName()) + if err != nil { + return false, err } - if vm.Status.Phase == v1alpha2.MachineStopped { - vmIsActive, err := commonvm.IsVMActive(ctx, h.client, vm) - if err != nil { - return false, err - } - - if !vmIsActive { - continue - } - } - - if h.isVIAttachedToVM(vi.GetName(), vm) { + if mounted { return true, nil } } return false, nil } - -func (h AttacheeHandler) isVIAttachedToVM(viName string, vm v1alpha2.VirtualMachine) bool { - for _, bda := range vm.Status.BlockDeviceRefs { - if bda.Kind == v1alpha2.ImageDevice && bda.Name == viName { - return true - } - } - - return false -} diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/deletion.go b/images/virtualization-artifact/pkg/controller/vi/internal/deletion.go index b9240254f2..394143f6e7 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/deletion.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/deletion.go @@ -18,25 +18,36 @@ package internal import ( "context" + "log/slog" + "sort" + "strings" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" + commonvm "github.com/deckhouse/virtualization-controller/pkg/common/vm" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vi/internal/source" "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vicondition" ) const deletionHandlerName = "DeletionHandler" type DeletionHandler struct { sources *source.Sources + client client.Client } -func NewDeletionHandler(sources *source.Sources) *DeletionHandler { +func NewDeletionHandler(sources *source.Sources, client client.Client) *DeletionHandler { return &DeletionHandler{ sources: sources, + client: client, } } @@ -44,24 +55,90 @@ func (h DeletionHandler) Handle(ctx context.Context, vi *v1alpha2.VirtualImage) log := logger.FromContext(ctx).With(logger.SlogHandler(deletionHandlerName)) if vi.DeletionTimestamp != nil { - requeue, err := h.sources.CleanUp(ctx, vi) + if controllerutil.ContainsFinalizer(vi, v1alpha2.FinalizerVIProtection) { + attachedVMs, err := h.attachedVirtualMachineNames(ctx, vi) + if err != nil { + return reconcile.Result{}, err + } + + h.setDeletingCondition( + vi, + vicondition.DeletionBlockedByProtection, + deletionBlockedByProtectionMessage(vi, attachedVMs), + ) + return reconcile.Result{}, nil + } + + requeue, reason, err := h.sources.CleanUp(ctx, vi) if err != nil { return reconcile.Result{}, err } if requeue { + if reason == "" { + reason = "Waiting for cleanup to finish" + } + h.setDeletingCondition(vi, vicondition.DeletionCleanupPending, reason) + log.Info("VirtualImage cleanup is pending", slog.String("reason", reason)) return reconcile.Result{RequeueAfter: time.Second}, nil } + conditions.RemoveCondition(vicondition.DeletingType, &vi.Status.Conditions) log.Info("Deletion observed: remove cleanup finalizer from VirtualImage") controllerutil.RemoveFinalizer(vi, v1alpha2.FinalizerVICleanup) return reconcile.Result{}, nil } + conditions.RemoveCondition(vicondition.DeletingType, &vi.Status.Conditions) controllerutil.AddFinalizer(vi, v1alpha2.FinalizerVICleanup) return reconcile.Result{}, nil } +func (h DeletionHandler) setDeletingCondition(vi *v1alpha2.VirtualImage, reason vicondition.DeletingReason, message string) { + conditions.SetCondition( + conditions.NewConditionBuilder(vicondition.DeletingType). + Generation(vi.Generation). + Status(metav1.ConditionFalse). + Reason(reason). + Message(service.CapitalizeFirstLetter(message)+"."), + &vi.Status.Conditions, + ) +} + +func (h DeletionHandler) attachedVirtualMachineNames(ctx context.Context, vi *v1alpha2.VirtualImage) ([]string, error) { + var vms v1alpha2.VirtualMachineList + err := h.client.List(ctx, &vms, &client.ListOptions{Namespace: vi.GetNamespace()}) + if err != nil { + return nil, err + } + + var attachedVMs []string + for _, vm := range vms.Items { + _, mounted, err := commonvm.BlockDeviceUsage(ctx, h.client, vm, v1alpha2.ImageDevice, vi.GetName()) + if err != nil { + return nil, err + } + + if mounted { + attachedVMs = append(attachedVMs, vm.Name) + } + } + + sort.Strings(attachedVMs) + return attachedVMs, nil +} + +func deletionBlockedByProtectionMessage(vi *v1alpha2.VirtualImage, attachedVMs []string) string { + switch len(attachedVMs) { + case 0: + return "The VirtualImage is protected from deletion by the protection finalizer" + case 1: + return "The VirtualImage is protected from deletion because it is attached to VirtualMachine " + attachedVMs[0] + default: + return "The VirtualImage is protected from deletion because it is attached to VirtualMachines: " + strings.Join(attachedVMs, ", ") + } +} + func (h DeletionHandler) Name() string { return "DeletionHandler" } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/deletion_test.go b/images/virtualization-artifact/pkg/controller/vi/internal/deletion_test.go new file mode 100644 index 0000000000..51be07cec9 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vi/internal/deletion_test.go @@ -0,0 +1,105 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package internal + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/controller/vi/internal/source" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vicondition" +) + +var _ = Describe("DeletionHandler", func() { + It("sets Deleting condition with cleanup pending reason", func() { + now := metav1.Now() + vi := &v1alpha2.VirtualImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vi", + Namespace: "default", + DeletionTimestamp: &now, + }, + } + + sources := source.NewSources() + sources.Set(v1alpha2.DataSourceTypeHTTP, &source.HandlerMock{ + CleanUpFunc: func(context.Context, *v1alpha2.VirtualImage) (bool, string, error) { + return true, "waiting for PersistentVolumeClaim deletion default/vi", nil + }, + }) + + handler := NewDeletionHandler(sources, nil) + result, err := handler.Handle(context.Background(), vi) + + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(time.Second)) + + cond, ok := conditions.GetCondition(vicondition.DeletingType, vi.Status.Conditions) + Expect(ok).To(BeTrue()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vicondition.DeletionCleanupPending.String())) + Expect(cond.Message).To(Equal("Waiting for PersistentVolumeClaim deletion default/vi.")) + }) + + It("sets Deleting condition when protection finalizer blocks deletion", func() { + scheme := runtime.NewScheme() + Expect(v1alpha2.AddToScheme(scheme)).To(Succeed()) + + now := metav1.Now() + vi := &v1alpha2.VirtualImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vi", + Namespace: "default", + DeletionTimestamp: &now, + Finalizers: []string{v1alpha2.FinalizerVIProtection}, + }, + } + vm := &v1alpha2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vm-a", + Namespace: "default", + }, + Status: v1alpha2.VirtualMachineStatus{ + Phase: v1alpha2.MachineRunning, + BlockDeviceRefs: []v1alpha2.BlockDeviceStatusRef{ + {Kind: v1alpha2.ImageDevice, Name: "vi"}, + }, + }, + } + + client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vm).Build() + handler := NewDeletionHandler(source.NewSources(), client) + result, err := handler.Handle(context.Background(), vi) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeZero()) + + cond, ok := conditions.GetCondition(vicondition.DeletingType, vi.Status.Conditions) + Expect(ok).To(BeTrue()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vicondition.DeletionBlockedByProtection.String())) + Expect(cond.Message).To(Equal("The VirtualImage is protected from deletion because it is attached to VirtualMachine vm-a.")) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vi/internal/interfaces.go index 59fe2e0934..5b3c60b035 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/interfaces.go @@ -33,7 +33,7 @@ import ( type Sources interface { Changed(ctx context.Context, vi *v1alpha2.VirtualImage) bool For(dsType v1alpha2.DataSourceType) (source.Handler, bool) - CleanUp(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, error) + CleanUp(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, string, error) } type DiskService interface { diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle.go index f2df5c2cb6..faf32dcf52 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle.go @@ -84,7 +84,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vi *v1alpha2.VirtualImage) Phase: v1alpha2.ImagePending, } - _, err := h.sources.CleanUp(ctx, vi) + _, _, err := h.sources.CleanUp(ctx, vi) if err != nil { return reconcile.Result{}, err } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle_test.go b/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle_test.go index f6d185dd23..a3b80f4346 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle_test.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle_test.go @@ -54,9 +54,9 @@ var _ = Describe("LifeCycleHandler Run", func() { }, } - sourcesMock.CleanUpFunc = func(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, error) { + sourcesMock.CleanUpFunc = func(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, string, error) { cleanUpCalled = true - return false, nil + return false, "", nil } sourcesMock.ChangedFunc = func(contextMoqParam context.Context, vi *v1alpha2.VirtualImage) bool { @@ -139,9 +139,9 @@ var _ = Describe("LifeCycleHandler Run", func() { }, } - sourcesMock.CleanUpFunc = func(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, error) { + sourcesMock.CleanUpFunc = func(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, string, error) { cleanUpCalled = true - return false, nil + return false, "", nil } sourcesMock.ChangedFunc = func(contextMoqParam context.Context, vi *v1alpha2.VirtualImage) bool { diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/mock.go b/images/virtualization-artifact/pkg/controller/vi/internal/mock.go index 4f8ef5285e..d74b829914 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/mock.go @@ -149,7 +149,7 @@ var _ Sources = &SourcesMock{} // ChangedFunc: func(ctx context.Context, vi *v1alpha2.VirtualImage) bool { // panic("mock out the Changed method") // }, -// CleanUpFunc: func(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, error) { +// CleanUpFunc: func(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, string, error) { // panic("mock out the CleanUp method") // }, // ForFunc: func(dsType v1alpha2.DataSourceType) (source.Handler, bool) { @@ -166,7 +166,7 @@ type SourcesMock struct { ChangedFunc func(ctx context.Context, vi *v1alpha2.VirtualImage) bool // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, error) + CleanUpFunc func(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, string, error) // ForFunc mocks the For method. ForFunc func(dsType v1alpha2.DataSourceType) (source.Handler, bool) @@ -235,7 +235,7 @@ func (mock *SourcesMock) ChangedCalls() []struct { } // CleanUp calls CleanUpFunc. -func (mock *SourcesMock) CleanUp(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, error) { +func (mock *SourcesMock) CleanUp(ctx context.Context, vd *v1alpha2.VirtualImage) (bool, string, error) { if mock.CleanUpFunc == nil { panic("SourcesMock.CleanUpFunc: method is nil but Sources.CleanUp was just called") } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/http.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/http.go index 763caa784a..d54d1230d4 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/http.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/http.go @@ -410,36 +410,36 @@ func (ds HTTPDataSource) StoreToPVC(ctx context.Context, vi *v1alpha2.VirtualIma return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds HTTPDataSource) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) { +func (ds HTTPDataSource) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.importerService.CleanUp(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - diskRequeue, err := ds.diskService.CleanUp(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return importerRequeue || diskRequeue, nil + return importerRequeue || diskRequeue, service.MergeCleanUpReasons(importerReason, diskReason), nil } func (ds HTTPDataSource) CleanUpSupplements(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.importerService.CleanUpSupplements(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - diskRequeue, err := ds.diskService.CleanUpSupplements(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - if importerRequeue || diskRequeue { + if reason := service.MergeCleanUpReasons(importerReason, diskReason); reason != "" || importerRequeue || diskRequeue { return reconcile.Result{RequeueAfter: time.Second}, nil } else { return reconcile.Result{}, nil diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/interfaces.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/interfaces.go index 7ef5263749..64e51fee4f 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/interfaces.go @@ -38,8 +38,8 @@ import ( type Importer interface { step.CreatePodStepImporter step.ReadyContainerRegistryStepImporter - CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) - CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) + CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) GetPod(ctx context.Context, sup supplements.Generator) (*corev1.Pod, error) Protect(ctx context.Context, pod *corev1.Pod, sup supplements.Generator) error Unprotect(ctx context.Context, pod *corev1.Pod, sup supplements.Generator) error @@ -48,8 +48,8 @@ type Importer interface { type Uploader interface { Start(ctx context.Context, settings *uploader.Settings, obj client.Object, sup supplements.Generator, caBundle *datasource.CABundle, opts ...service.Option) error - CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) - CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) + CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) GetPod(ctx context.Context, sup supplements.Generator) (*corev1.Pod, error) GetIngress(ctx context.Context, sup supplements.Generator) (*netv1.Ingress, error) GetService(ctx context.Context, sup supplements.Generator) (*corev1.Service, error) @@ -73,11 +73,11 @@ type Stat interface { type Bounder interface { step.CreateBounderPodStepBounder - CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) - CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) + CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) } type Disk interface { GetPersistentVolumeClaim(ctx context.Context, sup supplements.Generator) (*corev1.PersistentVolumeClaim, error) - CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/mock.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/mock.go index 171212b6c6..4583669fe2 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/mock.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/mock.go @@ -30,10 +30,10 @@ var _ Importer = &ImporterMock{} // // // make and configure a mocked Importer // mockedImporter := &ImporterMock{ -// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUp method") // }, -// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUpSupplements method") // }, // GetPodFunc: func(ctx context.Context, sup supplements.Generator) (*corev1.Pod, error) { @@ -62,10 +62,10 @@ var _ Importer = &ImporterMock{} // } type ImporterMock struct { // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // CleanUpSupplementsFunc mocks the CleanUpSupplements method. - CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // GetPodFunc mocks the GetPod method. GetPodFunc func(ctx context.Context, sup supplements.Generator) (*corev1.Pod, error) @@ -179,7 +179,7 @@ type ImporterMock struct { } // CleanUp calls CleanUpFunc. -func (mock *ImporterMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *ImporterMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpFunc == nil { panic("ImporterMock.CleanUpFunc: method is nil but Importer.CleanUp was just called") } @@ -215,7 +215,7 @@ func (mock *ImporterMock) CleanUpCalls() []struct { } // CleanUpSupplements calls CleanUpSupplementsFunc. -func (mock *ImporterMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *ImporterMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpSupplementsFunc == nil { panic("ImporterMock.CleanUpSupplementsFunc: method is nil but Importer.CleanUpSupplements was just called") } @@ -524,10 +524,10 @@ var _ Uploader = &UploaderMock{} // // // make and configure a mocked Uploader // mockedUploader := &UploaderMock{ -// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUp method") // }, -// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUpSupplements method") // }, // EnsureIngressFunc: func(ctx context.Context, obj client.Object, sup supplements.Generator) (*netv1.Ingress, error) { @@ -571,10 +571,10 @@ var _ Uploader = &UploaderMock{} // } type UploaderMock struct { // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // CleanUpSupplementsFunc mocks the CleanUpSupplements method. - CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // EnsureIngressFunc mocks the EnsureIngress method. EnsureIngressFunc func(ctx context.Context, obj client.Object, sup supplements.Generator) (*netv1.Ingress, error) @@ -735,7 +735,7 @@ type UploaderMock struct { } // CleanUp calls CleanUpFunc. -func (mock *UploaderMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *UploaderMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpFunc == nil { panic("UploaderMock.CleanUpFunc: method is nil but Uploader.CleanUp was just called") } @@ -771,7 +771,7 @@ func (mock *UploaderMock) CleanUpCalls() []struct { } // CleanUpSupplements calls CleanUpSupplementsFunc. -func (mock *UploaderMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *UploaderMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpSupplementsFunc == nil { panic("UploaderMock.CleanUpSupplementsFunc: method is nil but Uploader.CleanUpSupplements was just called") } @@ -1709,10 +1709,10 @@ var _ Bounder = &BounderMock{} // // // make and configure a mocked Bounder // mockedBounder := &BounderMock{ -// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUp method") // }, -// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUpSupplements method") // }, // StartFunc: func(ctx context.Context, ownerRef *metav1.OwnerReference, sup supplements.Generator, opts ...service.Option) error { @@ -1726,10 +1726,10 @@ var _ Bounder = &BounderMock{} // } type BounderMock struct { // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // CleanUpSupplementsFunc mocks the CleanUpSupplements method. - CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // StartFunc mocks the Start method. StartFunc func(ctx context.Context, ownerRef *metav1.OwnerReference, sup supplements.Generator, opts ...service.Option) error @@ -1768,7 +1768,7 @@ type BounderMock struct { } // CleanUp calls CleanUpFunc. -func (mock *BounderMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *BounderMock) CleanUp(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpFunc == nil { panic("BounderMock.CleanUpFunc: method is nil but Bounder.CleanUp was just called") } @@ -1804,7 +1804,7 @@ func (mock *BounderMock) CleanUpCalls() []struct { } // CleanUpSupplements calls CleanUpSupplementsFunc. -func (mock *BounderMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *BounderMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpSupplementsFunc == nil { panic("BounderMock.CleanUpSupplementsFunc: method is nil but Bounder.CleanUpSupplements was just called") } @@ -1893,7 +1893,7 @@ var _ Handler = &HandlerMock{} // // // make and configure a mocked Handler // mockedHandler := &HandlerMock{ -// CleanUpFunc: func(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) { +// CleanUpFunc: func(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) { // panic("mock out the CleanUp method") // }, // StoreToDVCRFunc: func(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) { @@ -1913,7 +1913,7 @@ var _ Handler = &HandlerMock{} // } type HandlerMock struct { // CleanUpFunc mocks the CleanUp method. - CleanUpFunc func(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) + CleanUpFunc func(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) // StoreToDVCRFunc mocks the StoreToDVCR method. StoreToDVCRFunc func(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) @@ -1962,7 +1962,7 @@ type HandlerMock struct { } // CleanUp calls CleanUpFunc. -func (mock *HandlerMock) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) { +func (mock *HandlerMock) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) { if mock.CleanUpFunc == nil { panic("HandlerMock.CleanUpFunc: method is nil but Handler.CleanUp was just called") } @@ -2115,7 +2115,7 @@ var _ Disk = &DiskMock{} // // // make and configure a mocked Disk // mockedDisk := &DiskMock{ -// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { +// CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { // panic("mock out the CleanUpSupplements method") // }, // GetPersistentVolumeClaimFunc: func(ctx context.Context, sup supplements.Generator) (*corev1.PersistentVolumeClaim, error) { @@ -2129,7 +2129,7 @@ var _ Disk = &DiskMock{} // } type DiskMock struct { // CleanUpSupplementsFunc mocks the CleanUpSupplements method. - CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplementsFunc func(ctx context.Context, sup supplements.Generator) (bool, string, error) // GetPersistentVolumeClaimFunc mocks the GetPersistentVolumeClaim method. GetPersistentVolumeClaimFunc func(ctx context.Context, sup supplements.Generator) (*corev1.PersistentVolumeClaim, error) @@ -2156,7 +2156,7 @@ type DiskMock struct { } // CleanUpSupplements calls CleanUpSupplementsFunc. -func (mock *DiskMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) { +func (mock *DiskMock) CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) { if mock.CleanUpSupplementsFunc == nil { panic("DiskMock.CleanUpSupplementsFunc: method is nil but Disk.CleanUpSupplements was just called") } 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 2d11d4d410..a305fe996f 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 @@ -466,25 +466,25 @@ func (ds ObjectRefDataSource) StoreToDVCR(ctx context.Context, vi *v1alpha2.Virt return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds ObjectRefDataSource) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) { +func (ds ObjectRefDataSource) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.importerService.CleanUp(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - bounderRequeue, err := ds.bounderService.CleanUp(ctx, supgen) + bounderRequeue, bounderReason, err := ds.bounderService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - diskRequeue, err := ds.diskService.CleanUp(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return importerRequeue || bounderRequeue || diskRequeue, nil + return importerRequeue || bounderRequeue || diskRequeue, service.MergeCleanUpReasons(importerReason, bounderReason, diskReason), nil } func (ds ObjectRefDataSource) Validate(ctx context.Context, vi *v1alpha2.VirtualImage) error { @@ -568,17 +568,17 @@ func (ds ObjectRefDataSource) getEnvSettings(vi *v1alpha2.VirtualImage, sup supp func (ds ObjectRefDataSource) CleanUpSupplements(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.importerService.CleanUpSupplements(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - diskRequeue, err := ds.diskService.CleanUpSupplements(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - if importerRequeue || diskRequeue { + if reason := service.MergeCleanUpReasons(importerReason, diskReason); reason != "" || importerRequeue || diskRequeue { return reconcile.Result{RequeueAfter: time.Second}, nil } else { return reconcile.Result{}, nil 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 e8e87ffdc7..18519bab9d 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 @@ -383,37 +383,37 @@ func (ds ObjectRefVirtualDisk) StoreToPVC(ctx context.Context, vi *v1alpha2.Virt func (ds ObjectRefVirtualDisk) CleanUpSupplements(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.importerService.CleanUpSupplements(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - diskRequeue, err := ds.diskService.CleanUpSupplements(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - if importerRequeue || diskRequeue { + if reason := service.MergeCleanUpReasons(importerReason, diskReason); reason != "" || importerRequeue || diskRequeue { return reconcile.Result{RequeueAfter: time.Second}, nil } else { return reconcile.Result{}, nil } } -func (ds ObjectRefVirtualDisk) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) { +func (ds ObjectRefVirtualDisk) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.importerService.CleanUp(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - diskRequeue, err := ds.diskService.CleanUp(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return importerRequeue || diskRequeue, nil + return importerRequeue || diskRequeue, service.MergeCleanUpReasons(importerReason, diskReason), nil } func (ds ObjectRefVirtualDisk) getEnvSettings(vi *v1alpha2.VirtualImage, sup supplements.Generator, volumeMode *corev1.PersistentVolumeMode) *importer.Settings { diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vdsnapshot_cr_test.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vdsnapshot_cr_test.go index 65b2014975..f45a74772d 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vdsnapshot_cr_test.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vdsnapshot_cr_test.go @@ -84,8 +84,8 @@ var _ = Describe("ObjectRef VirtualImageSnapshot ContainerRegistry", func() { } importer = &ImporterMock{ - CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, error) { - return false, nil + CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, string, error) { + return false, "", nil }, GetPodFunc: func(_ context.Context, _ supplements.Generator) (*corev1.Pod, error) { return pod, nil @@ -113,8 +113,8 @@ var _ = Describe("ObjectRef VirtualImageSnapshot ContainerRegistry", func() { } diskService = &DiskMock{ - CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, error) { - return false, nil + CleanUpSupplementsFunc: func(ctx context.Context, sup supplements.Generator) (bool, string, error) { + return false, "", nil }, GetPersistentVolumeClaimFunc: func(_ context.Context, _ supplements.Generator) (*corev1.PersistentVolumeClaim, error) { return pvc, nil diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vdsnapshot_pvc_test.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vdsnapshot_pvc_test.go index 0a6cf97dbb..764ff31770 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vdsnapshot_pvc_test.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vdsnapshot_pvc_test.go @@ -76,13 +76,13 @@ var _ = Describe("ObjectRef VirtualImageSnapshot PersistentVolumeClaim", func() } importer = &ImporterMock{ - CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, error) { - return false, nil + CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, string, error) { + return false, "", nil }, } bounder = &BounderMock{ - CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, error) { - return false, nil + CleanUpSupplementsFunc: func(_ context.Context, _ supplements.Generator) (bool, string, error) { + return false, "", nil }, } stat = &StatMock{ diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vi_on_pvc.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vi_on_pvc.go index 8218e2a304..736d4e8ce5 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vi_on_pvc.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vi_on_pvc.go @@ -342,20 +342,20 @@ func (ds ObjectRefDataVirtualImageOnPVC) StoreToPVC(ctx context.Context, vi, viR return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds ObjectRefDataVirtualImageOnPVC) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) { +func (ds ObjectRefDataVirtualImageOnPVC) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.importerService.CleanUp(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - diskRequeue, err := ds.diskService.CleanUp(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return importerRequeue || diskRequeue, nil + return importerRequeue || diskRequeue, service.MergeCleanUpReasons(importerReason, diskReason), nil } func (ds ObjectRefDataVirtualImageOnPVC) getEnvSettings(vi *v1alpha2.VirtualImage, sup supplements.Generator) *importer.Settings { @@ -374,17 +374,17 @@ func (ds ObjectRefDataVirtualImageOnPVC) getEnvSettings(vi *v1alpha2.VirtualImag func (ds ObjectRefDataVirtualImageOnPVC) CleanUpSupplements(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.importerService.CleanUpSupplements(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - diskRequeue, err := ds.diskService.CleanUpSupplements(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - if importerRequeue || diskRequeue { + if reason := service.MergeCleanUpReasons(importerReason, diskReason); reason != "" || importerRequeue || diskRequeue { return reconcile.Result{RequeueAfter: time.Second}, nil } else { return reconcile.Result{}, nil diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/registry.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/registry.go index c781e9a03d..f2b6b484e8 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/registry.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/registry.go @@ -404,20 +404,20 @@ func (ds RegistryDataSource) StoreToDVCR(ctx context.Context, vi *v1alpha2.Virtu return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds RegistryDataSource) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) { +func (ds RegistryDataSource) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.importerService.CleanUp(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - diskRequeue, err := ds.diskService.CleanUp(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return importerRequeue || diskRequeue, nil + return importerRequeue || diskRequeue, service.MergeCleanUpReasons(importerReason, diskReason), nil } func (ds RegistryDataSource) Validate(ctx context.Context, vi *v1alpha2.VirtualImage) error { @@ -464,17 +464,17 @@ func (ds RegistryDataSource) getEnvSettings(vi *v1alpha2.VirtualImage, supgen su func (ds RegistryDataSource) CleanUpSupplements(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.importerService.CleanUpSupplements(ctx, supgen) + importerRequeue, importerReason, err := ds.importerService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - diskRequeue, err := ds.diskService.CleanUpSupplements(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - if importerRequeue || diskRequeue { + if reason := service.MergeCleanUpReasons(importerReason, diskReason); reason != "" || importerRequeue || diskRequeue { return reconcile.Result{RequeueAfter: time.Second}, nil } else { return reconcile.Result{}, nil diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/sources.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/sources.go index 86b1be6fab..75e1f46488 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/sources.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/sources.go @@ -39,7 +39,7 @@ import ( type Handler interface { StoreToDVCR(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) StoreToPVC(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) - CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) + CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) Validate(ctx context.Context, vi *v1alpha2.VirtualImage) error } @@ -66,32 +66,34 @@ func (s Sources) Changed(_ context.Context, vi *v1alpha2.VirtualImage) bool { return vi.Generation != vi.Status.ObservedGeneration } -func (s Sources) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) { +func (s Sources) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) { var requeue bool + var reason string for _, source := range s.sources { - ok, err := source.CleanUp(ctx, vi) + sourceRequeue, sourceReason, err := source.CleanUp(ctx, vi) if err != nil { - return false, err + return false, "", err } - requeue = requeue || ok + requeue = requeue || sourceRequeue + reason = service.MergeCleanUpReasons(reason, sourceReason) } - return requeue, nil + return requeue, reason, nil } type Cleaner interface { - CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) + CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) CleanUpSupplements(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) } -func CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage, c Cleaner) (bool, error) { +func CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage, c Cleaner) (bool, string, error) { if object.ShouldCleanupSubResources(vi) { return c.CleanUp(ctx, vi) } - return false, nil + return false, "", nil } func CleanUpSupplements(ctx context.Context, vi *v1alpha2.VirtualImage, c Cleaner) (reconcile.Result, error) { diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/sources_test.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/sources_test.go index ddc11b00b5..cfbf371e3a 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/sources_test.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/sources_test.go @@ -38,9 +38,10 @@ import ( ) type sourcesHandlerStub struct { - cleanupResult bool - cleanupErr error - cleanupCalls int + cleanupRequeue bool + cleanupReason string + cleanupErr error + cleanupCalls int } func (s *sourcesHandlerStub) StoreToDVCR(context.Context, *v1alpha2.VirtualImage) (reconcile.Result, error) { @@ -51,9 +52,9 @@ func (s *sourcesHandlerStub) StoreToPVC(context.Context, *v1alpha2.VirtualImage) return reconcile.Result{}, nil } -func (s *sourcesHandlerStub) CleanUp(context.Context, *v1alpha2.VirtualImage) (bool, error) { +func (s *sourcesHandlerStub) CleanUp(context.Context, *v1alpha2.VirtualImage) (bool, string, error) { s.cleanupCalls++ - return s.cleanupResult, s.cleanupErr + return s.cleanupRequeue, s.cleanupReason, s.cleanupErr } func (s *sourcesHandlerStub) Validate(context.Context, *v1alpha2.VirtualImage) error { @@ -61,7 +62,8 @@ func (s *sourcesHandlerStub) Validate(context.Context, *v1alpha2.VirtualImage) e } type sourcesCleanerStub struct { - cleanupResult bool + cleanupRequeue bool + cleanupReason string cleanupErr error cleanupSupplementsResult reconcile.Result cleanupSupplementsErr error @@ -69,9 +71,9 @@ type sourcesCleanerStub struct { cleanupSupplementsCalls int } -func (s *sourcesCleanerStub) CleanUp(context.Context, *v1alpha2.VirtualImage) (bool, error) { +func (s *sourcesCleanerStub) CleanUp(context.Context, *v1alpha2.VirtualImage) (bool, string, error) { s.cleanupCalls++ - return s.cleanupResult, s.cleanupErr + return s.cleanupRequeue, s.cleanupReason, s.cleanupErr } func (s *sourcesCleanerStub) CleanUpSupplements(context.Context, *v1alpha2.VirtualImage) (reconcile.Result, error) { @@ -119,12 +121,12 @@ var _ = Describe("Sources helpers", func() { It("aggregates cleanup results from all handlers", func() { sources := NewSources() - first := &sourcesHandlerStub{cleanupResult: false} - second := &sourcesHandlerStub{cleanupResult: true} + first := &sourcesHandlerStub{} + second := &sourcesHandlerStub{cleanupRequeue: true, cleanupReason: "waiting for cleanup"} sources.Set(v1alpha2.DataSourceTypeHTTP, first) sources.Set(v1alpha2.DataSourceTypeObjectRef, second) - requeue, err := sources.CleanUp(context.Background(), newVI()) + requeue, _, err := sources.CleanUp(context.Background(), newVI()) Expect(err).ToNot(HaveOccurred()) Expect(requeue).To(BeTrue()) Expect(first.cleanupCalls).To(Equal(1)) @@ -136,7 +138,7 @@ var _ = Describe("Sources helpers", func() { broken := &sourcesHandlerStub{cleanupErr: errors.New("cleanup failed")} sources.Set(v1alpha2.DataSourceTypeHTTP, broken) - requeue, err := sources.CleanUp(context.Background(), newVI()) + requeue, _, err := sources.CleanUp(context.Background(), newVI()) Expect(err).To(MatchError("cleanup failed")) Expect(requeue).To(BeFalse()) Expect(broken.cleanupCalls).To(Equal(1)) @@ -146,22 +148,22 @@ var _ = Describe("Sources helpers", func() { Describe("cleanup wrappers", func() { It("runs cleanup only when subresources should be deleted", func() { vi := newVI() - cleaner := &sourcesCleanerStub{cleanupResult: true} + cleaner := &sourcesCleanerStub{cleanupRequeue: true, cleanupReason: "waiting for cleanup"} - shouldRequeue, err := CleanUp(context.Background(), vi, cleaner) + requeue, _, err := CleanUp(context.Background(), vi, cleaner) Expect(err).ToNot(HaveOccurred()) - Expect(shouldRequeue).To(BeTrue()) + Expect(requeue).To(BeTrue()) Expect(cleaner.cleanupCalls).To(Equal(1)) }) It("skips cleanup when retain annotation is set", func() { vi := newVI() vi.Annotations[annotations.AnnPodRetainAfterCompletion] = "true" - cleaner := &sourcesCleanerStub{cleanupResult: true} + cleaner := &sourcesCleanerStub{cleanupRequeue: true, cleanupReason: "waiting for cleanup"} - shouldRequeue, err := CleanUp(context.Background(), vi, cleaner) + requeue, _, err := CleanUp(context.Background(), vi, cleaner) Expect(err).ToNot(HaveOccurred()) - Expect(shouldRequeue).To(BeFalse()) + Expect(requeue).To(BeFalse()) Expect(cleaner.cleanupCalls).To(BeZero()) }) diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_cr_step.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_cr_step.go index e1a1ed8bd4..c393da8d26 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_cr_step.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_cr_step.go @@ -37,11 +37,11 @@ import ( ) type ReadyContainerRegistryStepDiskService interface { - CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) } type ReadyContainerRegistryStepImporter interface { - CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) } type ReadyContainerRegistryStepStat interface { @@ -149,12 +149,12 @@ func (s ReadyContainerRegistryStep) Take(ctx context.Context, vi *v1alpha2.Virtu func (s ReadyContainerRegistryStep) cleanUpSupplements(ctx context.Context, vi *v1alpha2.VirtualImage) error { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - _, err := s.importer.CleanUpSupplements(ctx, supgen) + _, _, err := s.importer.CleanUpSupplements(ctx, supgen) if err != nil { return err } - _, err = s.diskService.CleanUpSupplements(ctx, supgen) + _, _, err = s.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return err } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_cr_step_test.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_cr_step_test.go index 6f0123a557..aa7f9bf4a3 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_cr_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_cr_step_test.go @@ -41,13 +41,13 @@ type readyContainerRegistryStepCleanerStub struct { calls int } -func (s *readyContainerRegistryStepCleanerStub) CleanUpSupplements(context.Context, supplements.Generator) (bool, error) { +func (s *readyContainerRegistryStepCleanerStub) CleanUpSupplements(context.Context, supplements.Generator) (bool, string, error) { s.calls++ if s.cleanupErr != nil { - return false, s.cleanupErr + return false, "", s.cleanupErr } - return true, nil + return true, "waiting for cleanup", nil } type readyContainerRegistryStepStatStub struct { diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_pvc_step.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_pvc_step.go index c85e3f98aa..eb76e9d0a0 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_pvc_step.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_pvc_step.go @@ -37,7 +37,7 @@ import ( ) type ReadyPersistentVolumeClaimStepBounder interface { - CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, error) + CleanUpSupplements(ctx context.Context, sup supplements.Generator) (bool, string, error) } type ReadyPersistentVolumeClaimStep struct { @@ -140,7 +140,7 @@ func (s ReadyPersistentVolumeClaimStep) Take(ctx context.Context, vi *v1alpha2.V func (s ReadyPersistentVolumeClaimStep) cleanUpSupplements(ctx context.Context, vi *v1alpha2.VirtualImage) error { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - _, err := s.bounder.CleanUpSupplements(ctx, supgen) + _, _, err := s.bounder.CleanUpSupplements(ctx, supgen) if err != nil { return err } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_pvc_step_test.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_pvc_step_test.go index 2ed4b131ff..38eac0ef2a 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_pvc_step_test.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/step/ready_pvc_step_test.go @@ -35,8 +35,8 @@ import ( type noopReadyPersistentVolumeClaimStepBounder struct{} -func (noopReadyPersistentVolumeClaimStepBounder) CleanUpSupplements(context.Context, supplements.Generator) (bool, error) { - return true, nil +func (noopReadyPersistentVolumeClaimStepBounder) CleanUpSupplements(context.Context, supplements.Generator) (bool, string, error) { + return true, "waiting for cleanup", nil } func noopEvent(client.Object, string, string, string) {} diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/upload.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/upload.go index a8bdc0aba6..a2244d250b 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/upload.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/upload.go @@ -538,20 +538,20 @@ func (ds UploadDataSource) StoreToDVCR(ctx context.Context, vi *v1alpha2.Virtual return reconcile.Result{RequeueAfter: time.Second}, nil } -func (ds UploadDataSource) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, error) { +func (ds UploadDataSource) CleanUp(ctx context.Context, vi *v1alpha2.VirtualImage) (bool, string, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - importerRequeue, err := ds.uploaderService.CleanUp(ctx, supgen) + uploaderRequeue, uploaderReason, err := ds.uploaderService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - diskRequeue, err := ds.diskService.CleanUp(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUp(ctx, supgen) if err != nil { - return false, err + return false, "", err } - return importerRequeue || diskRequeue, nil + return uploaderRequeue || diskRequeue, service.MergeCleanUpReasons(uploaderReason, diskReason), nil } func (ds UploadDataSource) Validate(_ context.Context, _ *v1alpha2.VirtualImage) error { @@ -574,17 +574,17 @@ func (ds UploadDataSource) getEnvSettings(vi *v1alpha2.VirtualImage, supgen supp func (ds UploadDataSource) CleanUpSupplements(ctx context.Context, vi *v1alpha2.VirtualImage) (reconcile.Result, error) { supgen := supplements.NewGenerator(annotations.VIShortName, vi.Name, vi.Namespace, vi.UID) - uploaderRequeue, err := ds.uploaderService.CleanUpSupplements(ctx, supgen) + uploaderRequeue, uploaderReason, err := ds.uploaderService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - diskRequeue, err := ds.diskService.CleanUpSupplements(ctx, supgen) + diskRequeue, diskReason, err := ds.diskService.CleanUpSupplements(ctx, supgen) if err != nil { return reconcile.Result{}, err } - if uploaderRequeue || diskRequeue { + if reason := service.MergeCleanUpReasons(uploaderReason, diskReason); reason != "" || uploaderRequeue || diskRequeue { return reconcile.Result{RequeueAfter: time.Second}, nil } else { return reconcile.Result{}, nil diff --git a/images/virtualization-artifact/pkg/controller/vi/vi_controller.go b/images/virtualization-artifact/pkg/controller/vi/vi_controller.go index 4f80e6c62a..f106ccfe31 100644 --- a/images/virtualization-artifact/pkg/controller/vi/vi_controller.go +++ b/images/virtualization-artifact/pkg/controller/vi/vi_controller.go @@ -91,7 +91,7 @@ func NewController( internal.NewDatasourceReadyHandler(sources), internal.NewLifeCycleHandler(recorder, sources, mgr.GetClient()), internal.NewImagePresenceHandler(recorder, mgr.GetClient(), dvcr), - internal.NewDeletionHandler(sources), + internal.NewDeletionHandler(sources, mgr.GetClient()), internal.NewAttacheeHandler(mgr.GetClient()), ) diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/deletion.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/deletion.go index 30c5fb1f50..43e600ab11 100644 --- a/images/virtualization-artifact/pkg/controller/vmbda/internal/deletion.go +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/deletion.go @@ -23,6 +23,7 @@ import ( "log/slog" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -30,9 +31,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/deckhouse/virtualization-controller/pkg/common/object" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/kvbuilder" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmbdacondition" ) const deletionHandlerName = "DeletionHandler" @@ -57,6 +61,7 @@ func (h *DeletionHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac controllerutil.AddFinalizer(vmbda, v1alpha2.FinalizerVMBDACleanup) if vmbda.DeletionTimestamp == nil { + conditions.RemoveCondition(vmbdacondition.DeletingType, &vmbda.Status.Conditions) return reconcile.Result{}, nil } @@ -71,6 +76,8 @@ func (h *DeletionHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac } if h.unplug.IsAttached(vm, kvvm, vmbda) { + h.setDeletingCondition(vmbda, vmbdacondition.DeletionCleanupPending, detachPendingMessage(vmbda)) + var res reconcile.Result res, err = h.detach(ctx, kvvm, vmbda) if err != nil { @@ -80,11 +87,27 @@ func (h *DeletionHandler) Handle(ctx context.Context, vmbda *v1alpha2.VirtualMac } log := logger.FromContext(ctx).With(logger.SlogHandler(deletionHandlerName)) + conditions.RemoveCondition(vmbdacondition.DeletingType, &vmbda.Status.Conditions) log.Info("Deletion observed: remove cleanup finalizer from VirtualMachineBlockDeviceAttachment") controllerutil.RemoveFinalizer(vmbda, v1alpha2.FinalizerVMBDACleanup) return reconcile.Result{}, nil } +func (h *DeletionHandler) setDeletingCondition(vmbda *v1alpha2.VirtualMachineBlockDeviceAttachment, reason vmbdacondition.DeletingReason, message string) { + conditions.SetCondition( + conditions.NewConditionBuilder(vmbdacondition.DeletingType). + Generation(vmbda.Generation). + Status(metav1.ConditionFalse). + Reason(reason). + Message(service.CapitalizeFirstLetter(message)+"."), + &vmbda.Status.Conditions, + ) +} + +func detachPendingMessage(vmbda *v1alpha2.VirtualMachineBlockDeviceAttachment) string { + return fmt.Sprintf("Waiting for block device %s/%s to detach from VirtualMachine %s", vmbda.Spec.BlockDeviceRef.Kind, vmbda.Spec.BlockDeviceRef.Name, vmbda.Spec.VirtualMachineName) +} + func (h *DeletionHandler) detach(ctx context.Context, kvvm *virtv1.VirtualMachine, vmbda *v1alpha2.VirtualMachineBlockDeviceAttachment) (reconcile.Result, error) { if kvvm == nil { return reconcile.Result{}, errors.New("intvirtvm not found to unplug") diff --git a/images/virtualization-artifact/pkg/controller/vmbda/internal/deletion_test.go b/images/virtualization-artifact/pkg/controller/vmbda/internal/deletion_test.go new file mode 100644 index 0000000000..bebe40eefa --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vmbda/internal/deletion_test.go @@ -0,0 +1,103 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package internal + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + virtv1 "kubevirt.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmbdacondition" +) + +var _ = Describe("DeletionHandler", func() { + It("sets Deleting condition while waiting for block device detach", func() { + scheme := runtime.NewScheme() + Expect(v1alpha2.AddToScheme(scheme)).To(Succeed()) + Expect(virtv1.AddToScheme(scheme)).To(Succeed()) + + now := metav1.Now() + vmbda := &v1alpha2.VirtualMachineBlockDeviceAttachment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmbda", + Namespace: "default", + DeletionTimestamp: &now, + }, + Spec: v1alpha2.VirtualMachineBlockDeviceAttachmentSpec{ + VirtualMachineName: "vm", + BlockDeviceRef: v1alpha2.VMBDAObjectRef{ + Kind: v1alpha2.VMBDAObjectRefKindVirtualDisk, + Name: "vd", + }, + }, + } + vm := &v1alpha2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vm", + Namespace: "default", + }, + } + kvvm := &virtv1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vm", + Namespace: "default", + }, + } + + client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vm, kvvm).Build() + handler := NewDeletionHandler(vmbdaUnplug{ + isAttached: func(*v1alpha2.VirtualMachine, *virtv1.VirtualMachine, *v1alpha2.VirtualMachineBlockDeviceAttachment) bool { + return true + }, + unplugDisk: func(context.Context, *virtv1.VirtualMachine, string) error { + return nil + }, + }, client) + + result, err := handler.Handle(context.Background(), vmbda) + + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(BeZero()) + Expect(vmbda.Finalizers).To(ContainElement(v1alpha2.FinalizerVMBDACleanup)) + + cond, ok := conditions.GetCondition(vmbdacondition.DeletingType, vmbda.Status.Conditions) + Expect(ok).To(BeTrue()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(vmbdacondition.DeletionCleanupPending.String())) + Expect(cond.Message).To(Equal("Waiting for block device VirtualDisk/vd to detach from VirtualMachine vm.")) + }) +}) + +type vmbdaUnplug struct { + isAttached func(*v1alpha2.VirtualMachine, *virtv1.VirtualMachine, *v1alpha2.VirtualMachineBlockDeviceAttachment) bool + unplugDisk func(context.Context, *virtv1.VirtualMachine, string) error +} + +func (u vmbdaUnplug) IsAttached(vm *v1alpha2.VirtualMachine, kvvm *virtv1.VirtualMachine, vmbda *v1alpha2.VirtualMachineBlockDeviceAttachment) bool { + return u.isAttached(vm, kvvm, vmbda) +} + +func (u vmbdaUnplug) UnplugDisk(ctx context.Context, kvvm *virtv1.VirtualMachine, diskName string) error { + return u.unplugDisk(ctx, kvvm, diskName) +}