Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix discard job #74

Merged
merged 6 commits into from
Dec 6, 2024
Merged
12 changes: 12 additions & 0 deletions internal/controller/mantlebackup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1391,6 +1391,11 @@ func (r *MantleBackupReconciler) startImport(
backup *mantlev1.MantleBackup,
target *snapshotTarget,
) (ctrl.Result, error) {
if !r.doesMantleBackupHaveSyncModeAnnot(backup) {
// SetSynchronizingg is not called yet or the cache is stale.
return ctrl.Result{Requeue: true}, nil
}

if result, err := r.isExportDataAlreadyUploaded(ctx, backup); err != nil || !result.IsZero() {
return result, err
}
Expand All @@ -1415,6 +1420,12 @@ func (r *MantleBackupReconciler) startImport(
return ctrl.Result{}, nil
}

func (r *MantleBackupReconciler) doesMantleBackupHaveSyncModeAnnot(backup *mantlev1.MantleBackup) bool {
annots := backup.GetAnnotations()
syncMode, ok := annots[annotSyncMode]
return ok && (syncMode == syncModeFull || syncMode == syncModeIncremental)
}

func (r *MantleBackupReconciler) prepareObjectStorageClient(ctx context.Context) error {
if r.objectStorageClient != nil {
return nil
Expand Down Expand Up @@ -1678,6 +1689,7 @@ blkdiscard /dev/discard-rbd
DevicePath: "/dev/discard-rbd",
},
},
ImagePullPolicy: corev1.PullIfNotPresent,
},
}

Expand Down
8 changes: 5 additions & 3 deletions internal/controller/mantlerestore_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,11 @@ func (test *mantleRestoreControllerUnitTest) testDeleteRestoringPV() {
err = k8sClient.Update(ctx, &pv)
Expect(err).NotTo(HaveOccurred())

err = k8sClient.Get(ctx, client.ObjectKey{Name: test.reconciler.restoringPVName(restore)}, &pv)
Expect(err).To(HaveOccurred())
Expect(errors.IsNotFound(err)).To(BeTrue())
Eventually(ctx, func(g Gomega) {
err = k8sClient.Get(ctx, client.ObjectKey{Name: test.reconciler.restoringPVName(restore)}, &pv)
g.Expect(err).To(HaveOccurred())
g.Expect(errors.IsNotFound(err)).To(BeTrue())
}).Should(Succeed())
})

It("should skip deleting the PV if it does not exist", func(ctx SpecContext) {
Expand Down
10 changes: 5 additions & 5 deletions internal/controller/persistentvolume_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ func (r *PersistentVolumeReconciler) Reconcile(ctx context.Context, req ctrl.Req
return ctrl.Result{}, fmt.Errorf("failed to get PersistentVolume: %w", err)
}

// Make sure the PV has the finalizer.
if !controllerutil.ContainsFinalizer(&pv, RestoringPVFinalizerName) {
return ctrl.Result{}, nil
}

// Check if the PV is managed by the target Ceph cluster.
clusterID, err := getCephClusterIDFromSCName(ctx, r.client, pv.Spec.StorageClassName)
if err != nil {
Expand All @@ -77,11 +82,6 @@ func (r *PersistentVolumeReconciler) Reconcile(ctx context.Context, req ctrl.Req
return ctrl.Result{}, nil
}

// Make sure the PV has the finalizer.
if !controllerutil.ContainsFinalizer(&pv, RestoringPVFinalizerName) {
return ctrl.Result{}, nil
}

// Make sure the PV has a deletionTimestamp.
if pv.GetDeletionTimestamp().IsZero() {
return ctrl.Result{}, nil
Expand Down
11 changes: 5 additions & 6 deletions internal/controller/replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,11 @@ func (s *SecondaryServer) CreateOrUpdateMantleBackup(
return nil, fmt.Errorf("CreateOrUpdate failed: %w", err)
}

// Update the status here because ctrl.CreateOrUpdate doesn't change the status.
if err := updateStatus(ctx, s.client, &backup, func() error {
backup.Status.CreatedAt = backupReceived.Status.CreatedAt
return nil
}); err != nil {
return nil, fmt.Errorf("updateStatus failed: %w", err)
// Use Patch here because updateStatus is likely to fail due to "the object has been modified" error.
newBackup := backup.DeepCopy()
newBackup.Status.CreatedAt = backupReceived.Status.CreatedAt
if err := s.client.Status().Patch(ctx, newBackup, client.MergeFrom(&backup)); err != nil {
return nil, fmt.Errorf("status patch failed: %w", err)
}

return &proto.CreateOrUpdateMantleBackupResponse{}, nil
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/multik8s/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func replicationTestSuite() {
return errors.New("status of SyncedToRemote condition is not True")
}
return nil
}, "5m", "1s").Should(Succeed())
}, "10m", "1s").Should(Succeed())

By("checking PVC is replicated")
Eventually(func() error {
Expand Down