diff --git a/changelogs/unreleased/9014-Lyndon-Li b/changelogs/unreleased/9014-Lyndon-Li new file mode 100644 index 000000000..b36740f9d --- /dev/null +++ b/changelogs/unreleased/9014-Lyndon-Li @@ -0,0 +1 @@ +Fix issue #8959, add VGDP MS PVR controller \ No newline at end of file diff --git a/pkg/builder/pod_volume_restore_builder.go b/pkg/builder/pod_volume_restore_builder.go index 965dd5a5d..124f116fe 100644 --- a/pkg/builder/pod_volume_restore_builder.go +++ b/pkg/builder/pod_volume_restore_builder.go @@ -99,31 +99,31 @@ func (b *PodVolumeRestoreBuilder) UploaderType(uploaderType string) *PodVolumeRe } // Cancel sets the DataDownload's Cancel. -func (d *PodVolumeRestoreBuilder) Cancel(cancel bool) *PodVolumeRestoreBuilder { - d.object.Spec.Cancel = cancel - return d +func (b *PodVolumeRestoreBuilder) Cancel(cancel bool) *PodVolumeRestoreBuilder { + b.object.Spec.Cancel = cancel + return b } // AcceptedTimestamp sets the PodVolumeRestore's AcceptedTimestamp. -func (d *PodVolumeRestoreBuilder) AcceptedTimestamp(acceptedTimestamp *metav1.Time) *PodVolumeRestoreBuilder { - d.object.Status.AcceptedTimestamp = acceptedTimestamp - return d +func (b *PodVolumeRestoreBuilder) AcceptedTimestamp(acceptedTimestamp *metav1.Time) *PodVolumeRestoreBuilder { + b.object.Status.AcceptedTimestamp = acceptedTimestamp + return b } // Finalizers sets the PodVolumeRestore's Finalizers. -func (d *PodVolumeRestoreBuilder) Finalizers(finalizers []string) *PodVolumeRestoreBuilder { - d.object.Finalizers = finalizers - return d +func (b *PodVolumeRestoreBuilder) Finalizers(finalizers []string) *PodVolumeRestoreBuilder { + b.object.Finalizers = finalizers + return b } // Message sets the PodVolumeRestore's Message. -func (d *PodVolumeRestoreBuilder) Message(msg string) *PodVolumeRestoreBuilder { - d.object.Status.Message = msg - return d +func (b *PodVolumeRestoreBuilder) Message(msg string) *PodVolumeRestoreBuilder { + b.object.Status.Message = msg + return b } // Message sets the PodVolumeRestore's Node. -func (d *PodVolumeRestoreBuilder) Node(node string) *PodVolumeRestoreBuilder { - d.object.Status.Node = node - return d +func (b *PodVolumeRestoreBuilder) Node(node string) *PodVolumeRestoreBuilder { + b.object.Status.Node = node + return b } diff --git a/pkg/controller/data_download_controller.go b/pkg/controller/data_download_controller.go index 75c04e377..53d83d98e 100644 --- a/pkg/controller/data_download_controller.go +++ b/pkg/controller/data_download_controller.go @@ -198,7 +198,9 @@ func (r *DataDownloadReconciler) Reconcile(ctx context.Context, req ctrl.Request if time.Since(spotted) > delay { log.Infof("Data download %s is canceled in Phase %s but not handled in rasonable time", dd.GetName(), dd.Status.Phase) - r.tryCancelDataDownload(ctx, dd, "") + if r.tryCancelDataDownload(ctx, dd, "") { + delete(r.cancelledDataDownload, dd.Name) + } return ctrl.Result{}, nil } @@ -524,7 +526,6 @@ func (r *DataDownloadReconciler) tryCancelDataDownload(ctx context.Context, dd * // success update r.metrics.RegisterDataDownloadCancel(r.nodeName) r.restoreExposer.CleanUp(ctx, getDataDownloadOwnerObject(dd)) - delete(r.cancelledDataDownload, dd.Name) log.Warn("data download is canceled") diff --git a/pkg/controller/data_upload_controller.go b/pkg/controller/data_upload_controller.go index 7dc19e855..6ab65f172 100644 --- a/pkg/controller/data_upload_controller.go +++ b/pkg/controller/data_upload_controller.go @@ -226,7 +226,9 @@ func (r *DataUploadReconciler) Reconcile(ctx context.Context, req ctrl.Request) if time.Since(spotted) > delay { log.Infof("Data upload %s is canceled in Phase %s but not handled in reasonable time", du.GetName(), du.Status.Phase) - r.tryCancelDataUpload(ctx, du, "") + if r.tryCancelDataUpload(ctx, du, "") { + delete(r.cancelledDataUpload, du.Name) + } return ctrl.Result{}, nil } @@ -564,7 +566,6 @@ func (r *DataUploadReconciler) tryCancelDataUpload(ctx context.Context, du *vele r.metrics.RegisterDataUploadCancel(r.nodeName) // cleans up any objects generated during the snapshot expose r.cleanUp(ctx, du, log) - delete(r.cancelledDataUpload, du.Name) log.Warn("data upload is canceled") diff --git a/pkg/controller/pod_volume_restore_controller.go b/pkg/controller/pod_volume_restore_controller.go index 46a00937d..41362871a 100644 --- a/pkg/controller/pod_volume_restore_controller.go +++ b/pkg/controller/pod_volume_restore_controller.go @@ -43,7 +43,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" veleroapishared "github.com/vmware-tanzu/velero/pkg/apis/velero/shared" - velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/constant" "github.com/vmware-tanzu/velero/pkg/datapath" @@ -186,7 +185,9 @@ func (r *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req if time.Since(spotted) > delay { log.Infof("PVR %s is canceled in Phase %s but not handled in rasonable time", pvr.GetName(), pvr.Status.Phase) - r.tryCancelPodVolumeRestore(ctx, pvr, "") + if r.tryCancelPodVolumeRestore(ctx, pvr, "") { + delete(r.cancelledPVR, pvr.Name) + } return ctrl.Result{}, nil } @@ -196,7 +197,7 @@ func (r *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req if pvr.Status.Phase == "" || pvr.Status.Phase == velerov1api.PodVolumeRestorePhaseNew { if pvr.Spec.Cancel { log.Infof("PVR %s is canceled in Phase %s", pvr.GetName(), pvr.Status.Phase) - r.tryCancelPodVolumeRestore(ctx, pvr, "") + _ = r.tryCancelPodVolumeRestore(ctx, pvr, "") return ctrl.Result{}, nil } @@ -234,7 +235,7 @@ func (r *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req } else if pvr.Status.Phase == velerov1api.PodVolumeRestorePhaseAccepted { if peekErr := r.exposer.PeekExposed(ctx, getPVROwnerObject(pvr)); peekErr != nil { log.Errorf("Cancel PVR %s/%s because of expose error %s", pvr.Namespace, pvr.Name, peekErr) - r.tryCancelPodVolumeRestore(ctx, pvr, fmt.Sprintf("found a PVR %s/%s with expose error: %s. mark it as cancel", pvr.Namespace, pvr.Name, peekErr)) + _ = r.tryCancelPodVolumeRestore(ctx, pvr, fmt.Sprintf("found a PVR %s/%s with expose error: %s. mark it as cancel", pvr.Namespace, pvr.Name, peekErr)) } else if pvr.Status.AcceptedTimestamp != nil { if time.Since(pvr.Status.AcceptedTimestamp.Time) >= r.preparingTimeout { r.onPrepareTimeout(ctx, pvr) @@ -250,7 +251,7 @@ func (r *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req } if pvr.Spec.Cancel { - log.Info("Prepared PVR is being cancelled") + log.Info("Prepared PVR is being canceled") r.OnDataPathCancelled(ctx, pvr.GetNamespace(), pvr.GetName()) return ctrl.Result{}, nil } @@ -346,7 +347,7 @@ func (r *PodVolumeRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Req // Update status to Canceling if err := UpdatePVRWithRetry(ctx, r.client, types.NamespacedName{Namespace: pvr.Namespace, Name: pvr.Name}, log, func(pvr *velerov1api.PodVolumeRestore) bool { if isPVRInFinalState(pvr) { - log.Warnf("PVR %s is terminated, abort setting it to cancelling", pvr.Name) + log.Warnf("PVR %s is terminated, abort setting it to canceling", pvr.Name) return false } @@ -399,7 +400,6 @@ func (r *PodVolumeRestoreReconciler) tryCancelPodVolumeRestore(ctx context.Conte } r.exposer.CleanUp(ctx, getPVROwnerObject(pvr)) - delete(r.cancelledPVR, pvr.Name) log.Warn("PVR is canceled") @@ -569,7 +569,7 @@ func (r *PodVolumeRestoreReconciler) SetupWithManager(mgr ctrl.Manager) error { }) pred := kube.NewAllEventPredicate(func(obj client.Object) bool { - pvr := obj.(*velerov1.PodVolumeRestore) + pvr := obj.(*velerov1api.PodVolumeRestore) return !isLegacyPVR(pvr) }) diff --git a/pkg/controller/pod_volume_restore_controller_test.go b/pkg/controller/pod_volume_restore_controller_test.go index 5d4a3522e..81800b805 100644 --- a/pkg/controller/pod_volume_restore_controller_test.go +++ b/pkg/controller/pod_volume_restore_controller_test.go @@ -774,9 +774,10 @@ func TestPodVolumeRestoreReconcile(t *testing.T) { expectedErr: "error accepting PVR pvr-1: error updating PVR velero/pvr-1: Update error", }, { - name: "pvr is cancel on accepted", - pvr: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, pvrName).Finalizers([]string{PodVolumeFinalizer}).Cancel(true).Result(), - expected: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, pvrName).Finalizers([]string{PodVolumeFinalizer}).Cancel(true).Phase(velerov1api.PodVolumeRestorePhaseCanceled).Result(), + name: "pvr is cancel on accepted", + pvr: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, pvrName).Finalizers([]string{PodVolumeFinalizer}).Cancel(true).Result(), + expectCancelRecord: true, + expected: builder.ForPodVolumeRestore(velerov1api.DefaultNamespace, pvrName).Finalizers([]string{PodVolumeFinalizer}).Cancel(true).Phase(velerov1api.PodVolumeRestorePhaseCanceled).Result(), }, { name: "pvr expose failed",