Skip to content

Commit 9be4daf

Browse files
leon-apeleon-ape
committed
fix(dataprotection): simplify restore deletion coordination
1 parent cd18bd6 commit 9be4daf

5 files changed

Lines changed: 169 additions & 390 deletions

File tree

‎cmd/dataprotection/main.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -341,9 +341,8 @@ func main() {
341341
}
342342

343343
if err = (&dpcontrollers.ClusterRestoreReconciler{
344-
Client: mgr.GetClient(),
345-
APIReader: mgr.GetAPIReader(),
346-
Recorder: mgr.GetEventRecorderFor("cluster-restore-controller"),
344+
Client: mgr.GetClient(),
345+
Recorder: mgr.GetEventRecorderFor("cluster-restore-controller"),
347346
}).SetupWithManager(mgr); err != nil {
348347
setupLog.Error(err, "unable to create controller", "controller", "ClusterRestore")
349348
os.Exit(1)

‎controllers/dataprotection/cluster_restore_controller.go‎

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ import (
4545
// VolumePopulator remains the owner of all PVC-scoped restore resources.
4646
type ClusterRestoreReconciler struct {
4747
client.Client
48-
APIReader client.Reader
49-
Recorder record.EventRecorder
48+
Recorder record.EventRecorder
5049
}
5150

5251
// +kubebuilder:rbac:groups=apps.kubeblocks.io,resources=clusters,verbs=get;list;watch;patch;update
@@ -82,24 +81,10 @@ func (r *ClusterRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Reque
8281
return intctrlutil.RequeueAfter(reconcileInterval, reqCtx.Log,
8382
"waiting for restore resource owners to finish Cluster termination")
8483
}
85-
86-
// A final uncached read closes informer visibility gaps before releasing the
87-
// only protection that keeps the Cluster identity available to resource owners.
88-
active, err = r.hasRestoreResources(ctx, r.directReader(), cluster)
89-
if err != nil {
90-
return intctrlutil.CheckedRequeueWithError(err, reqCtx.Log, "failed to confirm Cluster restore cleanup")
91-
}
92-
if active {
93-
return intctrlutil.RequeueAfter(reconcileInterval, reqCtx.Log,
94-
"waiting for API server to confirm Cluster restore cleanup")
95-
}
9684
return r.removeFinalizer(reqCtx, cluster)
9785
}
9886

9987
func (r *ClusterRestoreReconciler) SetupWithManager(mgr ctrl.Manager) error {
100-
if r.APIReader == nil {
101-
r.APIReader = mgr.GetAPIReader()
102-
}
10388
return intctrlutil.NewControllerManagedBy(mgr).
10489
Named("cluster_restore").
10590
For(&appsv1.Cluster{}).
@@ -145,7 +130,8 @@ func (r *ClusterRestoreReconciler) hasRestoreResources(ctx context.Context, read
145130
ownedByCurrentCluster := clusterUID == "" || clusterUID == string(cluster.UID)
146131
terminal := restore.Status.Phase == dpv1alpha1.RestorePhaseCompleted ||
147132
restore.Status.Phase == dpv1alpha1.RestorePhaseFailed
148-
if ownedByCurrentCluster && (!terminal || !restore.DeletionTimestamp.IsZero()) {
133+
if ownedByCurrentCluster && (!cluster.DeletionTimestamp.IsZero() ||
134+
!terminal || !restore.DeletionTimestamp.IsZero()) {
149135
return true, nil
150136
}
151137
}
@@ -196,13 +182,6 @@ func (r *ClusterRestoreReconciler) removeFinalizer(reqCtx intctrlutil.RequestCtx
196182
return intctrlutil.Reconciled()
197183
}
198184

199-
func (r *ClusterRestoreReconciler) directReader() client.Reader {
200-
if r.APIReader != nil {
201-
return r.APIReader
202-
}
203-
return r.Client
204-
}
205-
206185
func isClusterRestoreHelperPVC(pvc *corev1.PersistentVolumeClaim) bool {
207186
return pvc.Labels[dprestore.DataProtectionPopulatePVCLabelKey] != ""
208187
}

‎controllers/dataprotection/cluster_restore_controller_unit_test.go‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func TestClusterRestoreControllerOwnsOnlyClusterFinalizer(t *testing.T) {
4343
scheme := clusterRestoreTestScheme(t)
4444
cluster := activeRestoreCluster()
4545
cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster).Build()
46-
reconciler := &ClusterRestoreReconciler{Client: cli, APIReader: cli}
46+
reconciler := &ClusterRestoreReconciler{Client: cli}
4747

4848
_, err := reconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(cluster)})
4949
require.NoError(t, err)
@@ -64,7 +64,7 @@ func TestClusterRestoreControllerWaitsWithoutMutatingOwnerResources(t *testing.T
6464
restore := clusterExecutionRestore(cluster, target)
6565
restore.Finalizers = []string{"dataprotection.kubeblocks.io/restore-finalizer"}
6666
cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster, target, helper, restore).Build()
67-
reconciler := &ClusterRestoreReconciler{Client: cli, APIReader: cli}
67+
reconciler := &ClusterRestoreReconciler{Client: cli}
6868

6969
result, err := reconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(cluster)})
7070
require.NoError(t, err)
@@ -76,34 +76,14 @@ func TestClusterRestoreControllerWaitsWithoutMutatingOwnerResources(t *testing.T
7676
require.Contains(t, currentTarget.Finalizers, dptypes.DataProtectionFinalizerName)
7777
}
7878

79-
func TestClusterRestoreControllerUsesUncachedFinalCheck(t *testing.T) {
80-
scheme := clusterRestoreTestScheme(t)
81-
cluster := activeRestoreCluster()
82-
now := metav1.Now()
83-
cluster.DeletionTimestamp = &now
84-
cluster.Finalizers = []string{dptypes.RestoreProtectionFinalizerName}
85-
target := clusterRestoreTarget(cluster, "target-uid")
86-
target.Finalizers = []string{dptypes.DataProtectionFinalizerName}
87-
cached := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster).Build()
88-
direct := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster, target).Build()
89-
reconciler := &ClusterRestoreReconciler{Client: cached, APIReader: direct}
90-
91-
result, err := reconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(cluster)})
92-
require.NoError(t, err)
93-
require.NotZero(t, result.RequeueAfter)
94-
current := &appsv1.Cluster{}
95-
require.NoError(t, cached.Get(context.Background(), client.ObjectKeyFromObject(cluster), current))
96-
require.Contains(t, current.Finalizers, dptypes.RestoreProtectionFinalizerName)
97-
}
98-
9979
func TestClusterRestoreControllerReleasesFinalizerAfterOwnersFinish(t *testing.T) {
10080
scheme := clusterRestoreTestScheme(t)
10181
cluster := activeRestoreCluster()
10282
now := metav1.Now()
10383
cluster.DeletionTimestamp = &now
10484
cluster.Finalizers = []string{dptypes.RestoreProtectionFinalizerName, "example.io/keep"}
10585
cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster).Build()
106-
reconciler := &ClusterRestoreReconciler{Client: cli, APIReader: cli}
86+
reconciler := &ClusterRestoreReconciler{Client: cli}
10787

10888
_, err := reconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(cluster)})
10989
require.NoError(t, err)
@@ -124,7 +104,7 @@ func TestClusterRestoreControllerTreatsCompletedRestoreAsInactive(t *testing.T)
124104
restore := clusterExecutionRestore(cluster, target)
125105
restore.Status.Phase = dpv1alpha1.RestorePhaseCompleted
126106
cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster, target, restore).Build()
127-
reconciler := &ClusterRestoreReconciler{Client: cli, APIReader: cli}
107+
reconciler := &ClusterRestoreReconciler{Client: cli}
128108

129109
_, err := reconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(cluster)})
130110
require.NoError(t, err)
@@ -143,7 +123,7 @@ func TestClusterRestoreControllerKeepsProtectionAfterRestoreFailure(t *testing.T
143123
Type: appsv1.ConditionTypeRestore, Status: metav1.ConditionFalse,
144124
}}
145125
cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster).Build()
146-
reconciler := &ClusterRestoreReconciler{Client: cli, APIReader: cli}
126+
reconciler := &ClusterRestoreReconciler{Client: cli}
147127

148128
_, err := reconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(cluster)})
149129
require.NoError(t, err)
@@ -153,6 +133,26 @@ func TestClusterRestoreControllerKeepsProtectionAfterRestoreFailure(t *testing.T
153133
"failed restore intent must remain protected until Cluster deletion")
154134
}
155135

136+
func TestClusterRestoreControllerWaitsForTerminalRestoreDuringDeletion(t *testing.T) {
137+
scheme := clusterRestoreTestScheme(t)
138+
cluster := activeRestoreCluster()
139+
now := metav1.Now()
140+
cluster.DeletionTimestamp = &now
141+
cluster.Finalizers = []string{dptypes.RestoreProtectionFinalizerName}
142+
target := clusterRestoreTarget(cluster, "target-uid")
143+
restore := clusterExecutionRestore(cluster, target)
144+
restore.Status.Phase = dpv1alpha1.RestorePhaseFailed
145+
cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster, target, restore).Build()
146+
reconciler := &ClusterRestoreReconciler{Client: cli}
147+
148+
result, err := reconciler.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(cluster)})
149+
require.NoError(t, err)
150+
require.NotZero(t, result.RequeueAfter)
151+
current := &appsv1.Cluster{}
152+
require.NoError(t, cli.Get(context.Background(), client.ObjectKeyFromObject(cluster), current))
153+
require.Contains(t, current.Finalizers, dptypes.RestoreProtectionFinalizerName)
154+
}
155+
156156
func clusterRestoreTestScheme(t *testing.T) *runtime.Scheme {
157157
t.Helper()
158158
scheme := runtime.NewScheme()

0 commit comments

Comments
 (0)