fix(dvcr): make garbage collection completion more reliable

Use the dvcr garbage collection secret as the source of truth for
postponed VI/CVI/VD imports and requeue postponed resources when the
secret is created, updated, or deleted.

Split deployment status persistence from garbage collection secret
deletion to avoid losing the final GC result on deployment update
conflicts with hook-triggered Helm reconciliations. It should fix
problem when VI/CVI/VD stuck in "Postponed" state.

Add timeout handling for running garbage collection and provisioner wait
stages, and keep the cleanup result safe until it is persisted to
deployment status.

Minor:
- Handle nil garbage collection secret in dvcr-cleaner when
adding cleanup completion annotation.
- Refactor dvcr gc secret watcher: filter events for secret early in predicate functions.

Signed-off-by: Ivan Mikheykin <ivan.mikheykin@flant.com>

Author: diafour

images/dvcr-artifact/cmd/dvcr-cleaner/cmd/gc.go

@@ -380,6 +380,10 @@ func annotateGarbageCollectionSecretOnCleanupDone(ctx context.Context, result ma
 		return err
 	}
 
+	if secret == nil {
+		return fmt.Errorf("garbage collection secret not found")
+	}
+
 	if secret.Annotations == nil {
 		secret.Annotations = make(map[string]string)
 	}

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

@@ -33,7 +33,7 @@ import (
 	"github.com/deckhouse/deckhouse/pkg/log"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/cvi/internal"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/cvi/internal/source"
-	"github.com/deckhouse/virtualization-controller/pkg/controller/dvcr-garbage-collection/postponehandler"
+	"github.com/deckhouse/virtualization-controller/pkg/controller/dvcr-garbage-collection/postponeimporter"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/gc"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/indexer"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/service"
@@ -81,7 +81,7 @@ func NewController(
 
 	reconciler := NewReconciler(
 		mgr.GetClient(),
-		postponehandler.New[*v1alpha2.ClusterVirtualImage](dvcrService, recorder),
+		postponeimporter.NewHandler[*v1alpha2.ClusterVirtualImage](dvcrService, recorder),
 		internal.NewDatasourceReadyHandler(sources),
 		internal.NewLifeCycleHandler(sources, mgr.GetClient()),
 		internal.NewImagePresenceHandler(dvcr.NewImageChecker(mgr.GetClient(), dvcrSettings)),
@@ -100,7 +100,7 @@ func NewController(
 		return nil, err
 	}
 
-	err = reconciler.SetupController(ctx, mgr, cviController)
+	err = reconciler.SetupController(ctx, mgr, cviController, log)
 	if err != nil {
 		return nil, err
 	}

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

@@ -30,7 +30,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
+	"github.com/deckhouse/deckhouse/pkg/log"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/cvi/internal/watcher"
+	"github.com/deckhouse/virtualization-controller/pkg/controller/dvcr-garbage-collection/postponeimporter"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/reconciler"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/watchers"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2"
@@ -81,7 +83,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 	return rec.Reconcile(ctx)
 }
 
-func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr controller.Controller) error {
+func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr controller.Controller, logger *log.Logger) error {
 	if err := ctr.Watch(
 		source.Kind(
 			mgr.GetCache(),
@@ -102,6 +104,7 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr
 		watcher.NewVirtualMachineWatcher(),
 		watcher.NewVirtualDiskWatcher(mgrClient),
 		watcher.NewVirtualDiskSnapshotWatcher(mgrClient),
+		postponeimporter.NewWatcher[*v1alpha2.ClusterVirtualImage](mgrClient, logger),
 	} {
 		err := w.Watch(mgr, ctr)
 		if err != nil {

images/virtualization-artifact/pkg/controller/dvcr-garbage-collection/condition/deployment.go

@@ -71,3 +71,15 @@ func UpdateGarbageCollectionCondition(deploy *appsv1.Deployment, reason dvcrdepl
 	}
 	deploy.Status.Conditions = filteredConditions
 }
+
+func GetGarbageCollectionCondition(deploy *appsv1.Deployment) appsv1.DeploymentCondition {
+	if deploy == nil {
+		return appsv1.DeploymentCondition{}
+	}
+	for _, cond := range deploy.Status.Conditions {
+		if cond.Type == dvcrdeploymentcondition.GarbageCollectionType {
+			return cond
+		}
+	}
+	return appsv1.DeploymentCondition{}
+}

images/virtualization-artifact/pkg/controller/dvcr-garbage-collection/internal/life_cycle.go

@@ -37,6 +37,11 @@ type LifeCycleHandler struct {
 	provisioningLister dvcrtypes.ProvisioningLister
 }
 
+var (
+	gcStatusPollingInterval    = time.Second * 20
+	resultPersistRetryInterval = time.Second
+)
+
 func NewLifeCycleHandler(client client.Client, dvcrService dvcrtypes.DVCRService, provisioningLister dvcrtypes.ProvisioningLister) *LifeCycleHandler {
 	return &LifeCycleHandler{
 		client:             client,
@@ -74,11 +79,17 @@ func (h LifeCycleHandler) Handle(ctx context.Context, req reconcile.Request, dep
 			return reconcile.Result{}, fmt.Errorf("fetch garbage collection secret: %w", err)
 		}
 		if secret == nil || secret.GetDeletionTimestamp() != nil {
-			// Secret is gone, no action required.
+			// Secret is gone, nothing to clean up:
+			// - Keep deployment conditions for informational purposes.
+			// - Postponed CVI/VI/VD will start importers themselves.
 			return reconcile.Result{}, nil
 		}
 
 		if h.dvcrService.IsGarbageCollectionDone(secret) {
+			if h.dvcrService.IsGarbageCollectionResultPersisted(secret, deploy) {
+				return reconcile.Result{}, h.dvcrService.DeleteGarbageCollectionSecret(ctx)
+			}
+
 			// Extract error or success message from the result.
 			reason, msg, err := h.dvcrService.ParseGarbageCollectionResult(secret)
 			if err != nil {
@@ -87,17 +98,34 @@ func (h LifeCycleHandler) Handle(ctx context.Context, req reconcile.Request, dep
 			dvcrcondition.UpdateGarbageCollectionCondition(deploy, reason, "%s", msg)
 			// Put full result JSON into annotation on deployment.
 			annotations.AddAnnotation(deploy, annotations.AnnDVCRGarbageCollectionResult, h.dvcrService.GetGarbageCollectionResult(secret))
-			// It is now possible to delete a secret.
-			return reconcile.Result{}, h.dvcrService.DeleteGarbageCollectionSecret(ctx)
+			// Requeue to delete secret only after deployment update succeeds.
+			return reconcile.Result{RequeueAfter: resultPersistRetryInterval}, nil
 		}
 
 		if h.dvcrService.IsGarbageCollectionStarted(secret) {
+			hasCreationTimestamp := !secret.GetCreationTimestamp().Time.IsZero()
+			waitDuration := time.Since(secret.GetCreationTimestamp().Time)
+			if hasCreationTimestamp && waitDuration > dvcrtypes.GarbageCollectionTimeout {
+				if h.dvcrService.IsGarbageCollectionResultPersisted(secret, deploy) {
+					return reconcile.Result{}, h.dvcrService.DeleteGarbageCollectionSecret(ctx)
+				}
+
+				dvcrcondition.UpdateGarbageCollectionCondition(deploy,
+					dvcrdeploymentcondition.Error,
+					"Wait for garbage collection more than %s timeout: %s elapsed, garbage collection canceled",
+					dvcrtypes.GarbageCollectionTimeout.String(),
+					waitDuration.Truncate(time.Second).String(),
+				)
+				annotations.AddAnnotation(deploy, annotations.AnnDVCRGarbageCollectionResult, "")
+				return reconcile.Result{RequeueAfter: resultPersistRetryInterval}, nil
+			}
+
 			dvcrcondition.UpdateGarbageCollectionCondition(deploy,
 				dvcrdeploymentcondition.InProgress,
 				"Wait for garbage collection to finish.",
 			)
 			// Wait for done annotation appears on secret.
-			return reconcile.Result{}, nil
+			return reconcile.Result{RequeueAfter: gcStatusPollingInterval}, nil
 		}
 
 		// No special annotation, check for provisioners to finish.
@@ -116,30 +144,35 @@ func (h LifeCycleHandler) Handle(ctx context.Context, req reconcile.Request, dep
 				dvcrdeploymentcondition.InProgress,
 				"Wait for garbage collection to finish.",
 			)
-			return reconcile.Result{}, nil
+			return reconcile.Result{RequeueAfter: gcStatusPollingInterval}, nil
 		}
 
 		// Cancel garbage collection if wait for provisioners for too long.
 		hasCreationTimestamp := !secret.GetCreationTimestamp().Time.IsZero()
 		waitDuration := time.Since(secret.GetCreationTimestamp().Time)
 		if hasCreationTimestamp && waitDuration > dvcrtypes.WaitProvisionersTimeout {
+			if h.dvcrService.IsGarbageCollectionResultPersisted(secret, deploy) {
+				return reconcile.Result{}, h.dvcrService.DeleteGarbageCollectionSecret(ctx)
+			}
+
 			// Wait for provisioners timed out: report error and stop garbage collection.
 			dvcrcondition.UpdateGarbageCollectionCondition(deploy,
 				dvcrdeploymentcondition.Error,
-				"Wait for resources provisioners more than %s timeout: %s elapsed, garbage collection canceled",
+				"Wait for %d resources provisioners to finish more than %s timeout: %s elapsed, garbage collection canceled",
+				remainInProvisioning,
 				dvcrtypes.WaitProvisionersTimeout.String(),
-				waitDuration.String(),
+				waitDuration.Truncate(time.Second).String(),
 			)
 			annotations.AddAnnotation(deploy, annotations.AnnDVCRGarbageCollectionResult, "")
-			return reconcile.Result{}, h.dvcrService.DeleteGarbageCollectionSecret(ctx)
+			return reconcile.Result{RequeueAfter: resultPersistRetryInterval}, nil
 		}
 
 		// Use requeue to wait for provisioners to finish.
 		dvcrcondition.UpdateGarbageCollectionCondition(deploy,
 			dvcrdeploymentcondition.InProgress,
 			"Wait for cvi/vi/vd finish provisioning: %d resources remain.", remainInProvisioning,
 		)
-		return reconcile.Result{RequeueAfter: time.Second * 20}, nil
+		return reconcile.Result{RequeueAfter: gcStatusPollingInterval}, nil
 	}
 
 	return reconcile.Result{}, nil

images/virtualization-artifact/pkg/controller/dvcr-garbage-collection/internal/service/provisioning_lister.go

@@ -80,6 +80,10 @@ func (p *ProvisioningLister) ListClusterVirtualImagesInProvisioning(ctx context.
 
 	var provisioning []v1alpha2.ClusterVirtualImage
 	for _, cvi := range cviList.Items {
+		// Ignore if Terminating state.
+		if !cvi.GetDeletionTimestamp().IsZero() {
+			continue
+		}
 		cond, exists := conditions.GetCondition(cvicondition.ReadyType, cvi.Status.Conditions)
 		if exists && cond.Status == metav1.ConditionFalse && cond.Reason == cvicondition.Provisioning.String() {
 			provisioning = append(provisioning, cvi)
@@ -97,6 +101,10 @@ func (p *ProvisioningLister) ListVirtualImagesInProvisioning(ctx context.Context
 
 	var provisioning []v1alpha2.VirtualImage
 	for _, vi := range viList.Items {
+		// Ignore if Terminating state.
+		if !vi.GetDeletionTimestamp().IsZero() {
+			continue
+		}
 		cond, exists := conditions.GetCondition(vicondition.ReadyType, vi.Status.Conditions)
 		if exists && cond.Status == metav1.ConditionFalse && cond.Reason == vicondition.Provisioning.String() {
 			provisioning = append(provisioning, vi)
@@ -118,6 +126,10 @@ func (p *ProvisioningLister) ListVirtualDisksInProvisioning(ctx context.Context)
 		if !vdHasDVCRStage(&vd) {
 			continue
 		}
+		// Ignore if Terminating state.
+		if !vd.GetDeletionTimestamp().IsZero() {
+			continue
+		}
 		cond, exists := conditions.GetCondition(vdcondition.ReadyType, vd.Status.Conditions)
 		if exists && cond.Status == metav1.ConditionFalse && cond.Reason == vdcondition.Provisioning.String() {
 			provisioning = append(provisioning, vd)

images/virtualization-artifact/pkg/controller/dvcr-garbage-collection/internal/watcher/garbage_collection_secret.go

@@ -21,6 +21,7 @@ import (
 	"reflect"
 
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -50,16 +51,29 @@ func (w *DVCRGarbageCollectionSecretWatcher) Watch(mgr manager.Manager, ctr cont
 			mgr.GetCache(),
 			&corev1.Secret{},
 			handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, secret *corev1.Secret) []reconcile.Request {
-				if secret.GetNamespace() == dvcrtypes.ModuleNamespace && secret.GetName() == dvcrtypes.DVCRGarbageCollectionSecretName {
-					return []reconcile.Request{{NamespacedName: client.ObjectKeyFromObject(secret)}}
-				}
-				return nil
+				return []reconcile.Request{{NamespacedName: client.ObjectKeyFromObject(secret)}}
 			}),
 			predicate.TypedFuncs[*corev1.Secret]{
+				CreateFunc: func(e event.TypedCreateEvent[*corev1.Secret]) bool {
+					return IsDVCRGarbageCollectionSecret(e.Object)
+				},
 				UpdateFunc: func(e event.TypedUpdateEvent[*corev1.Secret]) bool {
+					if !IsDVCRGarbageCollectionSecret(e.ObjectNew) {
+						return false
+					}
 					return !reflect.DeepEqual(e.ObjectNew.GetAnnotations(), e.ObjectOld.GetAnnotations())
 				},
+				DeleteFunc: func(e event.TypedDeleteEvent[*corev1.Secret]) bool {
+					return IsDVCRGarbageCollectionSecret(e.Object)
+				},
 			},
 		),
 	)
 }
+
+func IsDVCRGarbageCollectionSecret(secret metav1.Object) bool {
+	if secret == nil {
+		return false
+	}
+	return secret.GetNamespace() == dvcrtypes.ModuleNamespace && secret.GetName() == dvcrtypes.DVCRGarbageCollectionSecretName
+}

…e-collection/postponehandler/postpone.go …ion/postponeimporter/postpone_handler.goimages/virtualization-artifact/pkg/controller/dvcr-garbage-collection/postponehandler/postpone.go renamed to images/virtualization-artifact/pkg/controller/dvcr-garbage-collection/postponeimporter/postpone_handler.go images/virtualization-artifact/pkg/controller/dvcr-garbage-collection/postponehandler/postpone.go renamed to images/virtualization-artifact/pkg/controller/dvcr-garbage-collection/postponeimporter/postpone_handler.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package postponehandler
+package postponeimporter
 
 import (
 	"context"
@@ -42,13 +42,13 @@ type DVCRService interface {
 
 var PostponePeriod = time.Second * 15
 
-type Postpone[object client.Object] struct {
+type PostponeHandler[object client.Object] struct {
 	dvcrService DVCRService
 	recorder    eventrecord.EventRecorderLogger
 }
 
-func New[T client.Object](dvcrService DVCRService, recorder eventrecord.EventRecorderLogger) *Postpone[T] {
-	return &Postpone[T]{
+func NewHandler[T client.Object](dvcrService DVCRService, recorder eventrecord.EventRecorderLogger) *PostponeHandler[T] {
+	return &PostponeHandler[T]{
 		dvcrService: dvcrService,
 		recorder:    recorder,
 	}
@@ -57,7 +57,7 @@ func New[T client.Object](dvcrService DVCRService, recorder eventrecord.EventRec
 // Handle sets Ready condition to Provisioning for newly created resources
 // if dvcr is in the garbage collection mode.
 // Applicable for ClusterVirtualImage, VirtualImage, and VirtualDisk.
-func (p *Postpone[T]) Handle(ctx context.Context, obj T) (reconcile.Result, error) {
+func (p *PostponeHandler[T]) Handle(ctx context.Context, obj T) (reconcile.Result, error) {
 	conditionsPtr := conditions.NewConditionsAccessor(obj).Conditions()
 
 	readyCondition, readyConditionPresent := conditions.GetCondition(getReadyType(obj), *conditionsPtr)
@@ -109,7 +109,7 @@ func (p *Postpone[T]) Handle(ctx context.Context, obj T) (reconcile.Result, erro
 	return reconcile.Result{RequeueAfter: PostponePeriod}, reconciler.ErrStopHandlerChain
 }
 
-func (p *Postpone[T]) Name() string {
+func (p *PostponeHandler[T]) Name() string {
 	return "postpone-on-dvcr-garbage-collection-handler"
 }