diff --git a/chart/templates/clusterrole.yaml b/chart/templates/clusterrole.yaml index 14ee38e623..c86454d58b 100644 --- a/chart/templates/clusterrole.yaml +++ b/chart/templates/clusterrole.yaml @@ -78,6 +78,14 @@ rules: resources: ["persistentvolumes"] verbs: ["create", "delete", "patch", "update", "watch"] {{- end }} + {{- if and .Values.sync.toHost.persistentVolumeClaims.enabled (not .Values.sync.toHost.persistentVolumes.enabled) }} + # The external populator restore bridge re-binds populated host PVs from the + # populate helper PVC to the target PVC, which requires reading and patching + # host PVs even when PV sync to host is disabled (fake PV mode). + - apiGroups: [ "" ] + resources: [ "persistentvolumes" ] + verbs: [ "get", "list", "watch", "patch" ] + {{- end }} {{- if .Values.sync.fromHost.ingressClasses.enabled }} - apiGroups: ["networking.k8s.io"] resources: ["ingressclasses"] diff --git a/pkg/controllers/resources/persistentvolumeclaims/syncer.go b/pkg/controllers/resources/persistentvolumeclaims/syncer.go index 466887f476..a6b787f71c 100644 --- a/pkg/controllers/resources/persistentvolumeclaims/syncer.go +++ b/pkg/controllers/resources/persistentvolumeclaims/syncer.go @@ -215,6 +215,19 @@ func (s *persistentVolumeClaimSyncer) Sync(ctx *synccontext.SyncContext, event * return ctrl.Result{}, nil } else if event.Virtual.DeletionTimestamp != nil { + // Deleting the host populate helper PVC before the populated host PV has + // been re-bound to the target PVC releases the PV and lets the host CSI + // driver reclaim it, which breaks the restore. Keep the host helper PVC + // until the bridge to the target PVC has converged. + preserveHelper, err := s.shouldPreserveExternalPopulatorPopulateHelperHostPVC(ctx, event.Host, event.Virtual.Namespace, event.Virtual.Name, event.Virtual.Spec.VolumeName) + if err != nil { + return ctrl.Result{}, err + } + if preserveHelper { + ctx.Log.Infof("preserve host populate helper pvc %s/%s because restore populate bridge has not converged yet", event.Host.Namespace, event.Host.Name) + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + } + return patcher.DeleteHostObjectWithOptions(ctx, event.Host, event.Virtual, "virtual persistent volume claim is being deleted", &client.DeleteOptions{ GracePeriodSeconds: event.Virtual.DeletionGracePeriodSeconds, Preconditions: metav1.NewUIDPreconditions(string(event.Host.UID)), @@ -283,11 +296,17 @@ func (s *persistentVolumeClaimSyncer) Sync(ctx *synccontext.SyncContext, event * return ctrl.Result{}, err } if preserveVirtualStatus { - err = s.ensureExternalPopulatorHostMaterialization(ctx, event.Host, event.Virtual, vPV) + hostConverged, err := s.ensureExternalPopulatorHostMaterialization(ctx, event.Host, event.Virtual, vPV) if err != nil { return ctrl.Result{}, err } - ensureExternalPopulatorVirtualPopulateStatus(event.Virtual, vPV) + // Only report the virtual PVC as Bound once the populated PV actually + // exists on the host and is re-bound to the host target PVC. Doing it + // earlier makes the guest (and the restore controller inside it) see a + // successful restore while the host has no volume at all. + if hostConverged { + ensureExternalPopulatorVirtualPopulateStatus(event.Virtual, vPV) + } } else { preserveExternalPopulatorStatus, err := s.shouldPreserveExternalPopulatorVirtualStatus(ctx, event.Host, event.Virtual) if err != nil { @@ -310,6 +329,21 @@ func (s *persistentVolumeClaimSyncer) Sync(ctx *synccontext.SyncContext, event * func (s *persistentVolumeClaimSyncer) SyncToVirtual(ctx *synccontext.SyncContext, event *synccontext.SyncToVirtualEvent[*corev1.PersistentVolumeClaim]) (_ ctrl.Result, retErr error) { if event.VirtualOld != nil || translate.ShouldDeleteHostObject(event.Host) { + // The virtual populate helper PVC may already be gone while the populated + // host PV is still bound to the host helper PVC. Deleting the host helper + // PVC now would release the PV before it is re-bound to the target PVC. + vName := s.HostToVirtual(ctx, types.NamespacedName{Name: event.Host.Name, Namespace: event.Host.Namespace}, event.Host) + if vName.Name != "" { + preserveHelper, err := s.shouldPreserveExternalPopulatorPopulateHelperHostPVC(ctx, event.Host, vName.Namespace, vName.Name, "") + if err != nil { + return ctrl.Result{}, err + } + if preserveHelper { + ctx.Log.Infof("preserve host populate helper pvc %s/%s because restore populate bridge has not converged yet", event.Host.Namespace, event.Host.Name) + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + } + } + // virtual object is not here anymore, so we delete return patcher.DeleteHostObject(ctx, event.Host, event.VirtualOld, "virtual object was deleted") } @@ -451,24 +485,36 @@ func (s *persistentVolumeClaimSyncer) findExternalPopulatorPersistentVolumeByCla return match, true, nil } -func (s *persistentVolumeClaimSyncer) ensureExternalPopulatorHostMaterialization(ctx *synccontext.SyncContext, pObj, vObj *corev1.PersistentVolumeClaim, vPV *corev1.PersistentVolume) error { +// ensureExternalPopulatorHostMaterialization bridges the populated PV to the +// host target PVC. It reports whether the host side has converged: only then +// may the virtual PVC's status be synthesized as Bound. When the populated +// host PV does not exist (e.g. it was reclaimed before the bridge could run), +// a materialization request is emitted for an external materializer and false +// is returned — the guest must keep seeing a pending claim, not a false +// success. +func (s *persistentVolumeClaimSyncer) ensureExternalPopulatorHostMaterialization(ctx *synccontext.SyncContext, pObj, vObj *corev1.PersistentVolumeClaim, vPV *corev1.PersistentVolume) (bool, error) { hostPVName := s.externalPopulatorHostPersistentVolumeName(ctx, vPV) hostPV := &corev1.PersistentVolume{} err := ctx.HostClient.Get(ctx.Context, types.NamespacedName{Name: hostPVName}, hostPV) if err != nil { if kerrors.IsNotFound(err) { - return s.upsertExternalPopulatorMaterializationRequest(ctx, pObj, vObj, vPV) + return false, s.upsertExternalPopulatorMaterializationRequest(ctx, pObj, vObj, vPV) } - return err + return false, err } helperPVC, helperFound, err := s.findExternalPopulatorHelperPVC(ctx, vObj, vPV) if err != nil { - return err + return false, err } pObj.Spec.VolumeName = hostPVName - return s.ensureExternalPopulatorHostPVClaimRef(ctx, hostPVName, pObj, helperPVC, helperFound) + err = s.ensureExternalPopulatorHostPVClaimRef(ctx, hostPVName, pObj, helperPVC, helperFound) + if err != nil { + return false, err + } + + return true, nil } func (s *persistentVolumeClaimSyncer) upsertExternalPopulatorMaterializationRequest(ctx *synccontext.SyncContext, pObj, vObj *corev1.PersistentVolumeClaim, vPV *corev1.PersistentVolume) error { @@ -548,14 +594,11 @@ func (s *persistentVolumeClaimSyncer) ensureExternalPopulatorHostPVClaimRef(ctx return nil } } else if hostPV.Spec.ClaimRef != nil { - if !helperFound { - return fmt.Errorf("host pv %s is bound to %s/%s, but no virtual populate helper pvc was found for target pvc %s/%s", hostPVName, hostPV.Spec.ClaimRef.Namespace, hostPV.Spec.ClaimRef.Name, pObj.Namespace, pObj.Name) - } - ok, err := s.hostPVClaimRefMatchesVirtualPVC(ctx, hostPV.Spec.ClaimRef, helperPVC) + ok, err := s.canReleaseExternalPopulatorHostPVClaimRef(ctx, hostPV, helperPVC, helperFound) if err != nil { return err } else if !ok { - return fmt.Errorf("host pv %s claimRef %s/%s does not match target pvc %s/%s or expected populate helper pvc %s/%s", hostPVName, hostPV.Spec.ClaimRef.Namespace, hostPV.Spec.ClaimRef.Name, pObj.Namespace, pObj.Name, helperPVC.Namespace, helperPVC.Name) + return fmt.Errorf("host pv %s claimRef %s/%s does not match target pvc %s/%s or an expected populate helper pvc", hostPVName, hostPV.Spec.ClaimRef.Namespace, hostPV.Spec.ClaimRef.Name, pObj.Namespace, pObj.Name) } } @@ -564,6 +607,56 @@ func (s *persistentVolumeClaimSyncer) ensureExternalPopulatorHostPVClaimRef(ctx return ctx.HostClient.Patch(ctx.Context, updated, client.MergeFrom(hostPV)) } +// canReleaseExternalPopulatorHostPVClaimRef reports whether the populated host +// PV's claimRef may be re-pointed to the target PVC. That is the case when the +// claimRef references the populate helper PVC, a stale incarnation of a host +// PVC, or an orphaned vcluster-managed helper whose virtual PVC is already gone. +// It must never release a claim that a live, unrelated host PVC still holds. +func (s *persistentVolumeClaimSyncer) canReleaseExternalPopulatorHostPVClaimRef(ctx *synccontext.SyncContext, hostPV *corev1.PersistentVolume, helperPVC *corev1.PersistentVolumeClaim, helperFound bool) (bool, error) { + ref := hostPV.Spec.ClaimRef + refPVC := &corev1.PersistentVolumeClaim{} + err := ctx.HostClient.Get(ctx.Context, types.NamespacedName{Namespace: ref.Namespace, Name: ref.Name}, refPVC) + if err != nil { + if kerrors.IsNotFound(err) { + // the claimRef is dangling (e.g. the host helper PVC was already + // deleted), re-binding to the target recovers the populated PV + return true, nil + } + return false, err + } + if ref.UID != "" && ref.UID != refPVC.UID { + // the claimRef points at an old incarnation of that host PVC + return true, nil + } + + if helperFound { + ok, err := s.hostPVClaimRefMatchesVirtualPVC(ctx, ref, helperPVC) + if err != nil || ok { + return ok, err + } + } + + // orphaned populate helper: a vcluster managed host PVC bound to this PV + // whose virtual counterpart is gone or being deleted + if refPVC.Labels[translate.MarkerLabel] == "" || refPVC.Spec.VolumeName != hostPV.Name { + return false, nil + } + vName := s.HostToVirtual(ctx, types.NamespacedName{Name: refPVC.Name, Namespace: refPVC.Namespace}, refPVC) + if vName.Name == "" { + return true, nil + } + vPVC := &corev1.PersistentVolumeClaim{} + err = ctx.VirtualClient.Get(ctx, vName, vPVC) + if err != nil { + if kerrors.IsNotFound(err) { + return true, nil + } + return false, err + } + + return vPVC.DeletionTimestamp != nil, nil +} + func (s *persistentVolumeClaimSyncer) hostPVClaimRefMatchesVirtualPVC(ctx *synccontext.SyncContext, ref *corev1.ObjectReference, vPVC *corev1.PersistentVolumeClaim) (bool, error) { if ref == nil || vPVC == nil { return false, nil @@ -800,6 +893,111 @@ func (s *persistentVolumeClaimSyncer) shouldRecreateExternalPopulatorHostNoDataR return true, nil } +// shouldPreserveExternalPopulatorPopulateHelperHostPVC reports whether pObj is the +// host populate helper PVC of an in-flight external populator restore whose +// populated PV has not been re-bound to the target PVC yet. While that bridge is +// pending, the host helper PVC must not be deleted: releasing the PV would let +// the host CSI driver reclaim it and the restored data would be lost. +// +// helperVolumeName is the virtual helper PVC's spec.volumeName if the virtual +// object is still available, otherwise empty. +func (s *persistentVolumeClaimSyncer) shouldPreserveExternalPopulatorPopulateHelperHostPVC(ctx *synccontext.SyncContext, pObj *corev1.PersistentVolumeClaim, vNamespace, vName, helperVolumeName string) (bool, error) { + hostPVName := pObj.Spec.VolumeName + if hostPVName == "" { + // the host helper PVC never bound, so there is no populated PV to protect + return false, nil + } + + vPVName := helperVolumeName + if vPVName == "" { + vPVName = mappings.HostToVirtual(ctx, hostPVName, "", nil, mappings.PersistentVolumes()).Name + if vPVName == "" { + // host created PVs keep their name in the virtual cluster + vPVName = hostPVName + } + } + + target, found, err := s.findExternalPopulatorPopulateTargetPVC(ctx, vNamespace, vName, vPVName) + if err != nil || !found { + return false, err + } + + return s.isExternalPopulatorPopulateBridgePending(ctx, hostPVName, target) +} + +// findExternalPopulatorPopulateTargetPVC looks for a live external populator +// target PVC in the virtual namespace that expects the populated virtual PV +// vPVName, either because its own volumeName already points there or because the +// virtual PV's claimRef was re-pointed to it by the populator. +func (s *persistentVolumeClaimSyncer) findExternalPopulatorPopulateTargetPVC(ctx *synccontext.SyncContext, vNamespace, helperName, vPVName string) (*corev1.PersistentVolumeClaim, bool, error) { + vPV := &corev1.PersistentVolume{} + err := ctx.VirtualClient.Get(ctx, types.NamespacedName{Name: vPVName}, vPV) + if err != nil { + if !kerrors.IsNotFound(err) { + return nil, false, err + } + vPV = nil + } + + pvcList := &corev1.PersistentVolumeClaimList{} + err = ctx.VirtualClient.List(ctx.Context, pvcList, client.InNamespace(vNamespace)) + if err != nil { + return nil, false, err + } + + for i := range pvcList.Items { + pvc := &pvcList.Items[i] + if pvc.Name == helperName || pvc.DeletionTimestamp != nil || !isExternalPopulatorPVC(pvc) { + continue + } + if pvc.Spec.VolumeName == vPVName { + return pvc.DeepCopy(), true, nil + } + if vPV != nil && isExternalPopulatorPersistentVolumeForPVC(vPV, pvc, false) { + return pvc.DeepCopy(), true, nil + } + } + + return nil, false, nil +} + +// isExternalPopulatorPopulateBridgePending reports whether the populated host PV +// still has to be re-bound from the host populate helper PVC to the target PVC. +// The bridge has converged once the host PV's claimRef points at the host target +// PVC and the host target PVC's volumeName points back at the host PV. +func (s *persistentVolumeClaimSyncer) isExternalPopulatorPopulateBridgePending(ctx *synccontext.SyncContext, hostPVName string, target *corev1.PersistentVolumeClaim) (bool, error) { + hostTargetName := s.VirtualToHost(ctx, types.NamespacedName{Name: target.Name, Namespace: target.Namespace}, target) + hostTarget := &corev1.PersistentVolumeClaim{} + err := ctx.HostClient.Get(ctx.Context, hostTargetName, hostTarget) + if err != nil { + if kerrors.IsNotFound(err) { + // the host target PVC does not exist yet, so the bridge cannot have converged + return true, nil + } + return false, err + } + + if !isHostPVCWaitingForVolume(hostTarget) { + // the host target PVC is already bound, the helper is not needed anymore + return false, nil + } + if hostTarget.Spec.VolumeName != hostPVName { + return true, nil + } + + hostPV := &corev1.PersistentVolume{} + err = ctx.HostClient.Get(ctx.Context, types.NamespacedName{Name: hostPVName}, hostPV) + if err != nil { + if kerrors.IsNotFound(err) { + // the populated host PV is gone, so preserving the helper cannot protect it anymore + return false, nil + } + return false, err + } + + return !claimRefMatchesPersistentVolumeClaim(hostPV.Spec.ClaimRef, hostTarget), nil +} + func (s *persistentVolumeClaimSyncer) shouldPreserveExternalPopulatorNoDataRestorePVCWhileHostDeleting(ctx *synccontext.SyncContext, vObj *corev1.PersistentVolumeClaim) (bool, error) { return s.isExternalPopulatorNoDataRestorePVC(ctx, vObj) } diff --git a/pkg/controllers/resources/persistentvolumeclaims/syncer_test.go b/pkg/controllers/resources/persistentvolumeclaims/syncer_test.go index 3e10957e70..99a9f66e5f 100644 --- a/pkg/controllers/resources/persistentvolumeclaims/syncer_test.go +++ b/pkg/controllers/resources/persistentvolumeclaims/syncer_test.go @@ -381,6 +381,9 @@ func TestSync(t *testing.T) { dataProtectionHostPVBoundToTargetStaleUID.Spec.ClaimRef.UID = types.UID("stale-host-target-pvc-uid") dataProtectionHostPVBoundToTargetFreshUID := dataProtectionHostPVBoundToTarget.DeepCopy() dataProtectionHostPVBoundToTargetFreshUID.Spec.ClaimRef.UID = dataProtectionHostPendingPvcWithObjectUID.UID + dataProtectionPopulateHelperPvcDeleting := dataProtectionPopulateHelperPvc.DeepCopy() + dataProtectionPopulateHelperPvcDeleting.Finalizers = []string{"kubernetes.io/pvc-protection"} + dataProtectionPopulateHelperPvcDeleting.DeletionTimestamp = &metav1.Time{Time: time.Now()} syncertesting.RunTestsWithContext(t, func(vConfig *config.VirtualClusterConfig, pClient *testingutil.FakeIndexClient, vClient *testingutil.FakeIndexClient) *synccontext.RegisterContext { ctx := syncertesting.NewFakeRegisterContext(vConfig, pClient, vClient) @@ -704,11 +707,14 @@ func TestSync(t *testing.T) { }, InitialPhysicalState: []runtime.Object{dataProtectionNoDataHostPendingWithBackupSourceAndFakeVolumeName.DeepCopy()}, ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ - corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPendingPvcWithVolumeNameBoundStatus.DeepCopy()}, + // the populated host PV does not exist, so the virtual PVC must + // not be reported Bound + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPendingPvcWithVolumeName.DeepCopy()}, corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionPopulatedPV.DeepCopy()}, }, ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{ corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionNoDataHostPendingWithBackupSourceAndFakeVolumeName.DeepCopy()}, + corev1.SchemeGroupVersion.WithKind("ConfigMap"): {dataProtectionMaterializationRequestCM.DeepCopy()}, }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) @@ -953,6 +959,167 @@ func TestSync(t *testing.T) { assert.NilError(t, err) }, }, + { + Name: "Preserve host populate helper pvc while virtual helper is deleting and bridge is pending", + InitialVirtualState: []runtime.Object{ + dataProtectionBackupPvc.DeepCopy(), + dataProtectionPopulatedPV.DeepCopy(), + }, + InitialPhysicalState: []runtime.Object{ + dataProtectionHostPendingPvcWithUID.DeepCopy(), + dataProtectionHostPopulateHelperPvc.DeepCopy(), + dataProtectionHostPVBoundToHelper.DeepCopy(), + }, + ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPvc.DeepCopy()}, + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionPopulatedPV.DeepCopy()}, + }, + ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): { + dataProtectionHostPendingPvcWithUID.DeepCopy(), + dataProtectionHostPopulateHelperPvc.DeepCopy(), + }, + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionHostPVBoundToHelper.DeepCopy()}, + }, + Sync: func(ctx *synccontext.RegisterContext) { + syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) + syncer.(*persistentVolumeClaimSyncer).useFakePersistentVolumes = true + + result, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + dataProtectionHostPopulateHelperPvc.DeepCopy(), + dataProtectionHostPopulateHelperPvc.DeepCopy(), + dataProtectionPopulateHelperPvcDeleting.DeepCopy(), + dataProtectionPopulateHelperPvcDeleting.DeepCopy(), + )) + assert.NilError(t, err) + assert.Check(t, result.RequeueAfter > 0) + }, + }, + { + Name: "Delete host populate helper pvc after populate bridge converged", + InitialVirtualState: []runtime.Object{ + dataProtectionBackupPvc.DeepCopy(), + dataProtectionPopulatedPV.DeepCopy(), + }, + InitialPhysicalState: []runtime.Object{ + dataProtectionHostMaterializedTargetPvc.DeepCopy(), + dataProtectionHostPopulateHelperPvc.DeepCopy(), + dataProtectionHostPVBoundToTarget.DeepCopy(), + }, + ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPvc.DeepCopy()}, + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionPopulatedPV.DeepCopy()}, + }, + ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionHostMaterializedTargetPvc.DeepCopy()}, + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionHostPVBoundToTarget.DeepCopy()}, + }, + Sync: func(ctx *synccontext.RegisterContext) { + syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) + syncer.(*persistentVolumeClaimSyncer).useFakePersistentVolumes = true + + _, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + dataProtectionHostPopulateHelperPvc.DeepCopy(), + dataProtectionHostPopulateHelperPvc.DeepCopy(), + dataProtectionPopulateHelperPvcDeleting.DeepCopy(), + dataProtectionPopulateHelperPvcDeleting.DeepCopy(), + )) + assert.NilError(t, err) + }, + }, + { + Name: "Preserve orphaned host populate helper pvc while populate bridge is pending", + InitialVirtualState: []runtime.Object{ + dataProtectionBackupPvc.DeepCopy(), + dataProtectionPopulatedPV.DeepCopy(), + }, + InitialPhysicalState: []runtime.Object{ + dataProtectionHostPendingPvcWithUID.DeepCopy(), + dataProtectionHostPopulateHelperPvc.DeepCopy(), + dataProtectionHostPVBoundToHelper.DeepCopy(), + }, + ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPvc.DeepCopy()}, + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionPopulatedPV.DeepCopy()}, + }, + ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): { + dataProtectionHostPendingPvcWithUID.DeepCopy(), + dataProtectionHostPopulateHelperPvc.DeepCopy(), + }, + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionHostPVBoundToHelper.DeepCopy()}, + }, + Sync: func(ctx *synccontext.RegisterContext) { + syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) + syncer.(*persistentVolumeClaimSyncer).useFakePersistentVolumes = true + + result, err := syncer.(*persistentVolumeClaimSyncer).SyncToVirtual(syncCtx, synccontext.NewSyncToVirtualEvent(dataProtectionHostPopulateHelperPvc.DeepCopy())) + assert.NilError(t, err) + assert.Check(t, result.RequeueAfter > 0) + }, + }, + { + Name: "Delete orphaned host populate helper pvc after populate bridge converged", + InitialVirtualState: []runtime.Object{ + dataProtectionBackupPvc.DeepCopy(), + dataProtectionPopulatedPV.DeepCopy(), + }, + InitialPhysicalState: []runtime.Object{ + dataProtectionHostMaterializedTargetPvc.DeepCopy(), + dataProtectionHostPopulateHelperPvc.DeepCopy(), + dataProtectionHostPVBoundToTarget.DeepCopy(), + }, + ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPvc.DeepCopy()}, + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionPopulatedPV.DeepCopy()}, + }, + ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionHostMaterializedTargetPvc.DeepCopy()}, + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionHostPVBoundToTarget.DeepCopy()}, + }, + Sync: func(ctx *synccontext.RegisterContext) { + syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) + syncer.(*persistentVolumeClaimSyncer).useFakePersistentVolumes = true + + _, err := syncer.(*persistentVolumeClaimSyncer).SyncToVirtual(syncCtx, synccontext.NewSyncToVirtualEvent(dataProtectionHostPopulateHelperPvc.DeepCopy())) + assert.NilError(t, err) + }, + }, + { + Name: "Bridge data protection populated host pv to target pvc after virtual helper is gone", + InitialVirtualState: []runtime.Object{ + dataProtectionBackupPvc.DeepCopy(), + dataProtectionPopulatedPV.DeepCopy(), + }, + InitialPhysicalState: []runtime.Object{ + dataProtectionHostPendingPvc.DeepCopy(), + dataProtectionHostPopulateHelperPvc.DeepCopy(), + dataProtectionHostPVBoundToHelper.DeepCopy(), + }, + ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPvc.DeepCopy()}, + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionPopulatedPV.DeepCopy()}, + }, + ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): { + dataProtectionHostMaterializedTargetPvc.DeepCopy(), + dataProtectionHostPopulateHelperPvc.DeepCopy(), + }, + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionHostPVBoundToTarget.DeepCopy()}, + }, + Sync: func(ctx *synccontext.RegisterContext) { + syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) + syncer.(*persistentVolumeClaimSyncer).useFakePersistentVolumes = true + + _, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + dataProtectionHostPendingPvc.DeepCopy(), + dataProtectionHostPendingPvc.DeepCopy(), + dataProtectionBackupPvc.DeepCopy(), + dataProtectionBackupPvc.DeepCopy(), + )) + assert.NilError(t, err) + }, + }, { Name: "Requeue after deriving data protection pvc volume name from populated pv claim ref", InitialVirtualState: []runtime.Object{ @@ -982,14 +1149,17 @@ func TestSync(t *testing.T) { }, }, { - Name: "Derive data protection populated virtual status from bound populated pv while host pvc waits", + Name: "Do not derive populated virtual status while populated host pv is missing", InitialVirtualState: []runtime.Object{ dataProtectionBackupPendingPvcWithVolumeName.DeepCopy(), dataProtectionPopulatedPV.DeepCopy(), }, InitialPhysicalState: []runtime.Object{dataProtectionHostPendingPvc.DeepCopy()}, ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ - corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPendingPvcWithVolumeNameBoundStatus.DeepCopy()}, + // only a materialization request is emitted; reporting Bound here + // would tell the guest the restore succeeded while the host has no + // volume at all + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPendingPvcWithVolumeName.DeepCopy()}, corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionPopulatedPV.DeepCopy()}, }, ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{ @@ -1010,14 +1180,14 @@ func TestSync(t *testing.T) { }, }, { - Name: "Derive data protection populated virtual status while host pvc waits with stale fake volume name", + Name: "Do not derive populated virtual status with stale fake volume name while populated host pv is missing", InitialVirtualState: []runtime.Object{ dataProtectionBackupPendingPvcWithVolumeName.DeepCopy(), dataProtectionPopulatedPV.DeepCopy(), }, InitialPhysicalState: []runtime.Object{dataProtectionHostPendingPvcWithFakeVolumeName.DeepCopy()}, ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ - corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPendingPvcWithVolumeNameBoundStatus.DeepCopy()}, + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {dataProtectionBackupPendingPvcWithVolumeName.DeepCopy()}, corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {dataProtectionPopulatedPV.DeepCopy()}, }, ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{ diff --git a/pkg/controllers/resources/persistentvolumes/fake_syncer.go b/pkg/controllers/resources/persistentvolumes/fake_syncer.go index 75810a0820..e4213f438d 100644 --- a/pkg/controllers/resources/persistentvolumes/fake_syncer.go +++ b/pkg/controllers/resources/persistentvolumes/fake_syncer.go @@ -127,8 +127,39 @@ func (r *fakePersistentVolumeSyncer) pvNeeded(ctx *synccontext.SyncContext, pvNa if err != nil { return false, err } + if len(pvcList.Items) > 0 { + return true, nil + } + + // A fake PV whose claimRef still references a live PVC is still needed. An + // external populator re-points the PV's claimRef to the restore target PVC + // before the target's volumeName is derived from it; deleting the fake PV in + // that window would break the restore bridge to the populated host PV. + pv := &corev1.PersistentVolume{} + err = ctx.VirtualClient.Get(ctx, types.NamespacedName{Name: pvName}, pv) + if err != nil { + if kerrors.IsNotFound(err) { + return false, nil + } + return false, err + } + if pv.Spec.ClaimRef == nil || pv.Spec.ClaimRef.Kind != "PersistentVolumeClaim" { + return false, nil + } + + pvc := &corev1.PersistentVolumeClaim{} + err = ctx.VirtualClient.Get(ctx, types.NamespacedName{Name: pv.Spec.ClaimRef.Name, Namespace: pv.Spec.ClaimRef.Namespace}, pvc) + if err != nil { + if kerrors.IsNotFound(err) { + return false, nil + } + return false, err + } + if pvc.DeletionTimestamp != nil { + return false, nil + } - return len(pvcList.Items) > 0, nil + return pv.Spec.ClaimRef.UID == "" || pv.Spec.ClaimRef.UID == pvc.UID, nil } func CreateFakePersistentVolume(ctx context.Context, virtualClient client.Client, name types.NamespacedName, vPvc *corev1.PersistentVolumeClaim) error { diff --git a/pkg/controllers/resources/persistentvolumes/fake_syncer_test.go b/pkg/controllers/resources/persistentvolumes/fake_syncer_test.go index 4287b48c59..322c960b1e 100644 --- a/pkg/controllers/resources/persistentvolumes/fake_syncer_test.go +++ b/pkg/controllers/resources/persistentvolumes/fake_syncer_test.go @@ -82,8 +82,25 @@ func TestFakeSync(t *testing.T) { } pvWithFinalizers := basePv.DeepCopy() pvWithFinalizers.Finalizers = []string{"myfinalizer"} + // a restore target PVC whose volumeName is not derived yet, while the fake + // PV's claimRef was already re-pointed to it by an external populator + rePointedTargetPvc := basePvc.DeepCopy() + rePointedTargetPvc.Spec.VolumeName = "" syncertesting.RunTests(t, []*syncertesting.SyncTest{ + { + Name: "Keep pv while claim ref references live pvc without volume name", + InitialVirtualState: []runtime.Object{basePv, rePointedTargetPvc}, + ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("PersistentVolume"): {basePv}, + corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): {rePointedTargetPvc}, + }, + Sync: func(ctx *synccontext.RegisterContext) { + syncContext, syncer := newFakeFakeSyncer(t, ctx) + _, err := syncer.FakeSync(syncContext, basePv.DeepCopy()) + assert.NilError(t, err) + }, + }, { Name: "Create", InitialVirtualState: []runtime.Object{basePvc}, diff --git a/pkg/controllers/resources/persistentvolumes/syncer.go b/pkg/controllers/resources/persistentvolumes/syncer.go index f32027fe48..992073f428 100644 --- a/pkg/controllers/resources/persistentvolumes/syncer.go +++ b/pkg/controllers/resources/persistentvolumes/syncer.go @@ -297,6 +297,9 @@ func (s *persistentVolumeSyncer) shouldSync(ctx *synccontext.SyncContext, pObj * if translate.Default.IsManaged(ctx, pObj) { return true, nil, nil } + if target, ok, err := s.findExternalPopulatorTargetForPersistentVolume(ctx, pObj); err != nil || ok { + return ok, target, err + } return translate.Default.IsTargetedNamespace(ctx, pObj.Spec.ClaimRef.Namespace) && pObj.Spec.PersistentVolumeReclaimPolicy == corev1.PersistentVolumeReclaimRetain, nil, nil } @@ -309,6 +312,9 @@ func (s *persistentVolumeSyncer) shouldSync(ctx *synccontext.SyncContext, pObj * } else if translate.Default.IsManaged(ctx, pObj) { return true, nil, nil } + if target, ok, err := s.findExternalPopulatorTargetForPersistentVolume(ctx, pObj); err != nil || ok { + return ok, target, err + } if translate.Default.IsTargetedNamespace(ctx, pObj.Spec.ClaimRef.Namespace) && pObj.Status.Phase == corev1.VolumeReleased { return true, nil, nil } @@ -319,6 +325,71 @@ func (s *persistentVolumeSyncer) shouldSync(ctx *synccontext.SyncContext, pObj * return true, vPvc, nil } +// findExternalPopulatorTargetForPersistentVolume looks for a live external +// populator target PVC in the virtual cluster that still expects this populated +// PV. While the populate helper PVC is being torn down, the host PV's claimRef +// still references the helper, so the regular claimRef based resolution above +// fails. Without this, the virtual PV would be deleted (or never recreated) +// while the target PVC still needs it to finish the restore bridge. +func (s *persistentVolumeSyncer) findExternalPopulatorTargetForPersistentVolume(ctx *synccontext.SyncContext, pObj *corev1.PersistentVolume) (*corev1.PersistentVolumeClaim, bool, error) { + vPVName := mappings.HostToVirtual(ctx, pObj.Name, "", nil, mappings.PersistentVolumes()).Name + if vPVName == "" { + // host created PVs keep their name in the virtual cluster + vPVName = pObj.Name + } + + // prefer the virtual PV's claimRef, the populator re-points it to the target + vPV := &corev1.PersistentVolume{} + err := s.virtualClient.Get(ctx, types.NamespacedName{Name: vPVName}, vPV) + if err != nil { + if !kerrors.IsNotFound(err) { + return nil, false, err + } + } else if vPV.Spec.ClaimRef != nil { + vPvc := &corev1.PersistentVolumeClaim{} + err = s.virtualClient.Get(ctx, types.NamespacedName{Name: vPV.Spec.ClaimRef.Name, Namespace: vPV.Spec.ClaimRef.Namespace}, vPvc) + if err != nil { + if !kerrors.IsNotFound(err) { + return nil, false, err + } + } else if vPvc.DeletionTimestamp == nil && + hasExternalPopulatorDataSource(vPvc) && + (vPV.Spec.ClaimRef.UID == "" || vPV.Spec.ClaimRef.UID == vPvc.UID) { + return vPvc, true, nil + } + } + + // fall back to a target PVC whose volumeName already points at the populated PV + pvcList := &corev1.PersistentVolumeClaimList{} + err = s.virtualClient.List(ctx, pvcList) + if err != nil { + return nil, false, err + } + for i := range pvcList.Items { + vPvc := &pvcList.Items[i] + if vPvc.DeletionTimestamp == nil && hasExternalPopulatorDataSource(vPvc) && vPvc.Spec.VolumeName == vPVName { + return vPvc.DeepCopy(), true, nil + } + } + + return nil, false, nil +} + +// hasExternalPopulatorDataSource mirrors the check in the persistentvolumeclaims +// package, which cannot be imported here without creating an import cycle. +func hasExternalPopulatorDataSource(pvc *corev1.PersistentVolumeClaim) bool { + if pvc.Spec.DataSourceRef == nil || pvc.Spec.DataSourceRef.Name == "" { + return false + } + + switch pvc.Spec.DataSourceRef.Kind { + case "VolumeSnapshot", "PersistentVolumeClaim": + return false + default: + return true + } +} + func (s *persistentVolumeSyncer) IsManaged(ctx *synccontext.SyncContext, pObj client.Object) (bool, error) { pPv, ok := pObj.(*corev1.PersistentVolume) if !ok { diff --git a/pkg/controllers/resources/persistentvolumes/syncer_test.go b/pkg/controllers/resources/persistentvolumes/syncer_test.go index bc8d52c700..c8c07f1952 100644 --- a/pkg/controllers/resources/persistentvolumes/syncer_test.go +++ b/pkg/controllers/resources/persistentvolumes/syncer_test.go @@ -576,3 +576,122 @@ func TestTranslateUpdateBackwards_ClaimRefResourceVersionPreserved(t *testing.T) test.Run(t, createContext) } + +func externalPopulatorRestoreFixtures() (*corev1.PersistentVolumeClaim, *corev1.PersistentVolume, *corev1.PersistentVolume) { + dataProtectionGroup := "dataprotection.kubeblocks.io" + targetPvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testpvc", + Namespace: "test", + UID: types.UID("target-pvc-uid"), + }, + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "restore-populated-pv", + DataSourceRef: &corev1.TypedObjectReference{ + APIGroup: &dataProtectionGroup, + Kind: "Backup", + Name: "backup-1", + }, + }, + } + vPv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restore-populated-pv", + Annotations: map[string]string{ + constants.HostClusterPersistentVolumeAnnotation: "restore-populated-pv", + }, + }, + Spec: corev1.PersistentVolumeSpec{ + ClaimRef: &corev1.ObjectReference{ + Name: targetPvc.Name, + Namespace: targetPvc.Namespace, + UID: targetPvc.UID, + }, + }, + Status: corev1.PersistentVolumeStatus{ + Phase: corev1.VolumeBound, + }, + } + // the populated host PV is still bound to the host populate helper PVC whose + // virtual counterpart is already gone + pPv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restore-populated-pv", + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, + ClaimRef: &corev1.ObjectReference{ + Name: "kb-populate-host-helper", + Namespace: "test", + UID: types.UID("host-helper-uid"), + }, + }, + Status: corev1.PersistentVolumeStatus{ + Phase: corev1.VolumeBound, + }, + } + + return targetPvc, vPv, pPv +} + +func TestSyncKeepsVirtualPVForExternalPopulatorTarget(t *testing.T) { + targetPvc, vPv, pPv := externalPopulatorRestoreFixtures() + + test := &syncertesting.SyncTest{ + Name: "Keep virtual pv for external populator target while host claim ref is orphaned", + InitialVirtualState: []runtime.Object{targetPvc.DeepCopy(), vPv.DeepCopy()}, + InitialPhysicalState: []runtime.Object{pPv.DeepCopy()}, + Sync: func(ctx *synccontext.RegisterContext) { + syncContext, syncer := newFakeSyncer(t, ctx) + + _, err := syncer.Sync(syncContext, synccontext.NewSyncEventWithOld( + pPv.DeepCopy(), + pPv.DeepCopy(), + vPv.DeepCopy(), + vPv.DeepCopy(), + )) + assert.NilError(t, err) + + keptPv := &corev1.PersistentVolume{} + err = syncer.virtualClient.Get(syncContext, types.NamespacedName{Name: vPv.Name}, keptPv) + assert.NilError(t, err) + assert.Assert(t, keptPv.Spec.ClaimRef != nil) + assert.Equal(t, keptPv.Spec.ClaimRef.Name, targetPvc.Name) + assert.Equal(t, keptPv.Spec.ClaimRef.Namespace, targetPvc.Namespace) + }, + } + + test.Run(t, func(vConfig *config.VirtualClusterConfig, pClient *testingutil.FakeIndexClient, vClient *testingutil.FakeIndexClient) *synccontext.RegisterContext { + vConfig.Sync.ToHost.PersistentVolumes.Enabled = true + return syncertesting.NewFakeRegisterContext(vConfig, pClient, vClient) + }) +} + +func TestSyncToVirtualRecreatesExternalPopulatorTargetPV(t *testing.T) { + targetPvc, vPv, pPv := externalPopulatorRestoreFixtures() + + test := &syncertesting.SyncTest{ + Name: "Recreate virtual pv for external populator target after it was deleted", + InitialVirtualState: []runtime.Object{targetPvc.DeepCopy()}, + InitialPhysicalState: []runtime.Object{pPv.DeepCopy()}, + Sync: func(ctx *synccontext.RegisterContext) { + syncContext, syncer := newFakeSyncer(t, ctx) + + _, err := syncer.SyncToVirtual(syncContext, synccontext.NewSyncToVirtualEvent(pPv.DeepCopy())) + assert.NilError(t, err) + + recreatedPv := &corev1.PersistentVolume{} + err = syncer.virtualClient.Get(syncContext, types.NamespacedName{Name: vPv.Name}, recreatedPv) + assert.NilError(t, err) + assert.Assert(t, recreatedPv.Spec.ClaimRef != nil) + assert.Equal(t, recreatedPv.Spec.ClaimRef.Name, targetPvc.Name) + assert.Equal(t, recreatedPv.Spec.ClaimRef.Namespace, targetPvc.Namespace) + assert.Equal(t, string(recreatedPv.Spec.ClaimRef.UID), string(targetPvc.UID)) + }, + } + + test.Run(t, func(vConfig *config.VirtualClusterConfig, pClient *testingutil.FakeIndexClient, vClient *testingutil.FakeIndexClient) *synccontext.RegisterContext { + vConfig.Sync.ToHost.PersistentVolumes.Enabled = true + return syncertesting.NewFakeRegisterContext(vConfig, pClient, vClient) + }) +}